Skip to content

Commit

Permalink
chore: move tracing to general config
Browse files Browse the repository at this point in the history
Signed-off-by: Aaron Pham <29749331+aarnphm@users.noreply.github.com>
  • Loading branch information
aarnphm committed Nov 21, 2022
1 parent ca27fc5 commit 1887fcf
Show file tree
Hide file tree
Showing 6 changed files with 45 additions and 52 deletions.
6 changes: 3 additions & 3 deletions src/bentoml/_internal/configuration/containers.py
Expand Up @@ -277,7 +277,7 @@ def metrics_client(

return PrometheusClient(multiproc_dir=multiproc_dir)

tracing = api_server_config.tracing
tracing = config.tracing

@providers.SingletonFactory
@staticmethod
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/bentoml/_internal/configuration/helpers.py
Expand Up @@ -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())


Expand Down
18 changes: 6 additions & 12 deletions src/bentoml/_internal/configuration/v1/__init__.py
Expand Up @@ -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),
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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 (
Expand All @@ -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(
Expand Down
60 changes: 30 additions & 30 deletions src/bentoml/_internal/configuration/v1/default_configuration.yaml
Expand Up @@ -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
Expand All @@ -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
Expand Down
3 changes: 1 addition & 2 deletions src/bentoml/_internal/runner/runner_handle/remote.py
Expand Up @@ -22,7 +22,6 @@

if TYPE_CHECKING:
import yarl
import aiohttp
from aiohttp import BaseConnector
from aiohttp.client import ClientSession

Expand Down Expand Up @@ -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(),
)
],
Expand Down
8 changes: 4 additions & 4 deletions tests/unit/_internal/configuration/test_v1_migration.py
Expand Up @@ -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
)

Expand Down Expand Up @@ -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()
Expand All @@ -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()

0 comments on commit 1887fcf

Please sign in to comment.