From 1887fcf6ba55c829fb0ea214caa69622c7f4f98b Mon Sep 17 00:00:00 2001 From: Aaron Pham <29749331+aarnphm@users.noreply.github.com> Date: Sun, 20 Nov 2022 21:44:10 -0800 Subject: [PATCH] chore: move tracing to general config Signed-off-by: Aaron Pham <29749331+aarnphm@users.noreply.github.com> --- .../_internal/configuration/containers.py | 6 +- .../_internal/configuration/helpers.py | 2 +- .../_internal/configuration/v1/__init__.py | 18 ++---- .../v1/default_configuration.yaml | 60 +++++++++---------- .../_internal/runner/runner_handle/remote.py | 3 +- .../configuration/test_v1_migration.py | 8 +-- 6 files changed, 45 insertions(+), 52 deletions(-) diff --git a/src/bentoml/_internal/configuration/containers.py b/src/bentoml/_internal/configuration/containers.py index 79d5367bb54..e3cf2b63fba 100644 --- a/src/bentoml/_internal/configuration/containers.py +++ b/src/bentoml/_internal/configuration/containers.py @@ -277,7 +277,7 @@ def metrics_client( return PrometheusClient(multiproc_dir=multiproc_dir) - tracing = api_server_config.tracing + tracing = config.tracing @providers.SingletonFactory @staticmethod @@ -306,8 +306,8 @@ def tracer_provider( if sample_rate is None: sample_rate = 0.0 if sample_rate == 0.0: - logger.warning( - "'api_server.tracing.sample_rate' is set to zero. No traces will be collected. Please refer to https://docs.bentoml.org/en/latest/guides/tracing.html for more details." + logger.debug( + "'tracing.sample_rate' is set to zero. No traces will be collected. Please refer to https://docs.bentoml.org/en/latest/guides/tracing.html for more details." ) resource = {} # User can optionally configure the resource with the following environment variables. Only diff --git a/src/bentoml/_internal/configuration/helpers.py b/src/bentoml/_internal/configuration/helpers.py index 3a66be7961c..997eeca61ae 100644 --- a/src/bentoml/_internal/configuration/helpers.py +++ b/src/bentoml/_internal/configuration/helpers.py @@ -36,7 +36,7 @@ def depth(_: t.Any, _level: int = 0): # pragma: no cover @depth.register(dict) -def _dict_depth(d: dict[str, t.Any], level: int = 0, **kw: t.Any): # type: ignore # pylint: disable +def _(d: dict[str, t.Any], level: int = 0, **kw: t.Any): return max(depth(v, level + 1, **kw) for v in d.values()) diff --git a/src/bentoml/_internal/configuration/v1/__init__.py b/src/bentoml/_internal/configuration/v1/__init__.py index b054e93411b..2d9776edf7f 100644 --- a/src/bentoml/_internal/configuration/v1/__init__.py +++ b/src/bentoml/_internal/configuration/v1/__init__.py @@ -120,7 +120,6 @@ "max_message_length": s.Or(int, None), "maximum_concurrent_rpcs": s.Or(int, None), }, - "tracing": TRACING_CFG, s.Optional("ssl"): { "enabled": bool, s.Optional("certfile"): s.Or(str, None), @@ -169,6 +168,7 @@ **_RUNNER_CONFIG, s.Optional(str): _RUNNER_CONFIG, }, + "tracing": TRACING_CFG, s.Optional("monitoring"): { "enabled": bool, s.Optional("type"): s.Or(str, None), @@ -231,31 +231,25 @@ def migration(*, override_config: dict[str, t.Any]): rename_fields( override_config, current="tracing.type", - replace_with="api_server.tracing.exporter_type", + replace_with="tracing.exporter_type", ) - for f in ["sample_rate", "excluded_urls"]: - rename_fields( - override_config, - current=f"tracing.{f}", - replace_with=f"api_server.tracing.{f}", - ) # 5.2. for Zipkin and OTLP, migrate tracing.[exporter].url -> api_server.tracing.[exporter].endpoint for exporter in ["zipkin", "otlp"]: rename_fields( override_config, current=f"tracing.{exporter}.url", - replace_with=f"api_server.tracing.{exporter}.endpoint", + replace_with=f"tracing.{exporter}.endpoint", ) # 5.3. For Jaeger, migrate tracing.jaeger.[address|port] -> api_server.tracing.jaeger.thrift.[agent_host_name|agent_port] rename_fields( override_config, current="tracing.jaeger.address", - replace_with="api_server.tracing.jaeger.thrift.agent_host_name", + replace_with="tracing.jaeger.thrift.agent_host_name", ) rename_fields( override_config, current="tracing.jaeger.port", - replace_with="api_server.tracing.jaeger.thrift.agent_port", + replace_with="tracing.jaeger.thrift.agent_port", ) # we also need to choose which protocol to use for jaeger. if ( @@ -268,7 +262,7 @@ def migration(*, override_config: dict[str, t.Any]): ) != 0 ): - override_config["api_server.tracing.jaeger.protocol"] = "thrift" + override_config["tracing.jaeger.protocol"] = "thrift" # 6. Last but not least, moving logging.formatting.* -> api_server.logging.access.format.* for f in ["trace_id", "span_id"]: rename_fields( diff --git a/src/bentoml/_internal/configuration/v1/default_configuration.yaml b/src/bentoml/_internal/configuration/v1/default_configuration.yaml index 662730884c4..9ddd2741d50 100644 --- a/src/bentoml/_internal/configuration/v1/default_configuration.yaml +++ b/src/bentoml/_internal/configuration/v1/default_configuration.yaml @@ -72,36 +72,6 @@ api_server: metrics: host: 0.0.0.0 port: 3001 - tracing: - exporter_type: ~ - sample_rate: ~ - excluded_urls: ~ - timeout: ~ - max_tag_value_length: ~ - zipkin: - endpoint: ~ - local_node_ipv4: ~ - local_node_ipv6: ~ - local_node_port: ~ - jaeger: - protocol: thrift - collector_endpoint: ~ - thrift: - agent_host_name: ~ - agent_port: ~ - udp_split_oversized_batches: ~ - grpc: - insecure: ~ - otlp: - protocol: ~ - endpoint: ~ - compression: ~ - http: - certificate_file: ~ - headers: ~ - grpc: - headers: ~ - insecure: ~ runner_probe: # configure whether the API server's health check endpoints (readyz, livez, healthz) also check the runners enabled: true timeout: 1 @@ -123,6 +93,36 @@ runners: metrics: enabled: true namespace: bentoml_runner +tracing: + exporter_type: ~ + sample_rate: ~ + excluded_urls: ~ + timeout: ~ + max_tag_value_length: ~ + zipkin: + endpoint: ~ + local_node_ipv4: ~ + local_node_ipv6: ~ + local_node_port: ~ + jaeger: + protocol: thrift + collector_endpoint: ~ + thrift: + agent_host_name: ~ + agent_port: ~ + udp_split_oversized_batches: ~ + grpc: + insecure: ~ + otlp: + protocol: ~ + endpoint: ~ + compression: ~ + http: + certificate_file: ~ + headers: ~ + grpc: + headers: ~ + insecure: ~ monitoring: enabled: true type: default diff --git a/src/bentoml/_internal/runner/runner_handle/remote.py b/src/bentoml/_internal/runner/runner_handle/remote.py index 8ef5506f301..32a3dbdc5df 100644 --- a/src/bentoml/_internal/runner/runner_handle/remote.py +++ b/src/bentoml/_internal/runner/runner_handle/remote.py @@ -22,7 +22,6 @@ if TYPE_CHECKING: import yarl - import aiohttp from aiohttp import BaseConnector from aiohttp.client import ClientSession @@ -114,7 +113,7 @@ def strip_query_params(url: yarl.URL) -> str: trace_configs=[ create_trace_config( # Remove all query params from the URL attribute on the span. - url_filter=strip_query_params, # type: ignore + url_filter=strip_query_params, tracer_provider=BentoMLContainer.tracer_provider.get(), ) ], diff --git a/tests/unit/_internal/configuration/test_v1_migration.py b/tests/unit/_internal/configuration/test_v1_migration.py index d3ede9886aa..ad7eae29294 100644 --- a/tests/unit/_internal/configuration/test_v1_migration.py +++ b/tests/unit/_internal/configuration/test_v1_migration.py @@ -46,10 +46,10 @@ def test_backward_from_envvar( with caplog.at_level(logging.WARNING): bentoml_cfg = container_from_envvar(envvar) assert bentoml_cfg["version"] == 1 - assert bentoml_cfg["api_server"]["tracing"]["exporter_type"] == "jaeger" + assert bentoml_cfg["tracing"]["exporter_type"] == "jaeger" assert ( - "Field 'tracing.jaeger.address' is deprecated and has been renamed to 'api_server.tracing.jaeger.thrift.agent_host_name'" + "Field 'tracing.jaeger.address' is deprecated and has been renamed to 'tracing.jaeger.thrift.agent_host_name'" in caplog.text ) @@ -130,7 +130,7 @@ def test_backward_warning( with caplog.at_level(logging.WARNING): container_from_file(OLD_ZIPKIN_URL) assert ( - "Field 'tracing.zipkin.url' is deprecated and has been renamed to 'api_server.tracing.zipkin.endpoint'" + "Field 'tracing.zipkin.url' is deprecated and has been renamed to 'tracing.zipkin.endpoint'" in caplog.text ) caplog.clear() @@ -144,7 +144,7 @@ def test_backward_warning( with caplog.at_level(logging.WARNING): container_from_file(OLD_OTLP_URL) assert ( - "Field 'tracing.otlp.url' is deprecated and has been renamed to 'api_server.tracing.otlp.endpoint'" + "Field 'tracing.otlp.url' is deprecated and has been renamed to 'tracing.otlp.endpoint'" in caplog.text ) caplog.clear()