From 4b3eb9b7740b37151fdd734c5df20dafe596881e Mon Sep 17 00:00:00 2001 From: crypto-a Date: Tue, 18 Aug 2026 18:51:50 -0400 Subject: [PATCH] fix: give each OTLP signal its own path 0.5.0 handed one endpoint to both OTLPSpanExporter and OTLPMetricExporter. Passing endpoint= overrides the SDK and is used VERBATIM; only when the SDK reads OTEL_EXPORTER_OTLP_ENDPOINT itself does it append /v1/traces or /v1/metrics. So a service configured with http://collector:4318 POSTed both signals to the collector's root and got 404 for every batch. The service served perfectly and the only evidence was an export error in its own logs, which is why this reached a running cluster before anyone noticed. Found by deploying the template to DOKS, pointing it at a real Collector, and observing that no metric or trace ever arrived. Both endpoint forms are accepted now, because both exist: a base URL from an operator who read the OTel docs, and a full signal URL from one who read this kit's own tests. --- pyproject.toml | 2 +- src/kit/__init__.py | 2 +- src/kit/observability/_metrics.py | 6 ++++- src/kit/observability/_tracing.py | 28 ++++++++++++++++++++++- tests/test_propagation.py | 38 +++++++++++++++++++++++++++++++ uv.lock | 2 +- 6 files changed, 73 insertions(+), 5 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index ba6ebec..50c8452 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -9,7 +9,7 @@ [project] name = "scadable-kit" -version = "0.5.0" +version = "0.5.1" description = "Cross-cutting behaviour shared by every SCADABLE service" requires-python = ">=3.14,<3.15" license = { file = "LICENSE" } diff --git a/src/kit/__init__.py b/src/kit/__init__.py index 10da434..efe440e 100644 --- a/src/kit/__init__.py +++ b/src/kit/__init__.py @@ -20,4 +20,4 @@ across the fleet without opening every repository. """ -__version__ = "0.5.0" +__version__ = "0.5.1" diff --git a/src/kit/observability/_metrics.py b/src/kit/observability/_metrics.py index 8350091..a915a4a 100644 --- a/src/kit/observability/_metrics.py +++ b/src/kit/observability/_metrics.py @@ -21,6 +21,8 @@ import logging from typing import Any +from kit.observability._tracing import signal_endpoint + _meter: Any = None _provider: Any = None _instruments: dict[str, Any] = {} @@ -86,7 +88,9 @@ def start_metrics( # report why it stopped. Telemetry must never be the reason a shutdown is # not clean. reader = PeriodicExportingMetricReader( - OTLPMetricExporter(endpoint=endpoint, timeout=EXPORT_TIMEOUT_SECONDS) + OTLPMetricExporter( + endpoint=signal_endpoint(endpoint, "metrics"), timeout=EXPORT_TIMEOUT_SECONDS + ) ) provider = MeterProvider(resource=resource, metric_readers=[reader]) metrics.set_meter_provider(provider) diff --git a/src/kit/observability/_tracing.py b/src/kit/observability/_tracing.py index 00e66e4..eb90155 100644 --- a/src/kit/observability/_tracing.py +++ b/src/kit/observability/_tracing.py @@ -37,6 +37,28 @@ class of data we exist to keep track of. log = logging.getLogger("kit.observability") +def signal_endpoint(endpoint: str, signal: str) -> str: + """Append the signal path unless the caller already gave one. + + Passing `endpoint=` to an OTLP exporter overrides the SDK entirely and is + used VERBATIM. Only when the SDK reads OTEL_EXPORTER_OTLP_ENDPOINT itself + does it append /v1/traces or /v1/metrics. So handing both exporters one base + URL posts both signals to the collector's root, which answers 404, and the + only symptom is an export error in the logs while the service serves + perfectly. That is exactly how this shipped. + + Both forms are accepted because both exist in the wild: a base URL from an + operator who read the OTel docs, and a full signal URL from one who read + this kit's tests. + """ + if not endpoint: + return endpoint + trimmed = endpoint.rstrip("/") + if trimmed.endswith(("/v1/traces", "/v1/metrics", "/v1/logs")): + return trimmed + return f"{trimmed}/v1/{signal}" + + def start_telemetry( *, service_name: str, @@ -92,7 +114,11 @@ def start_telemetry( # unreachable collector must not outlast the pod's termination grace # period and turn a clean stop into a SIGKILL. provider.add_span_processor( - BatchSpanProcessor(OTLPSpanExporter(endpoint=endpoint, timeout=EXPORT_TIMEOUT_SECONDS)) + BatchSpanProcessor( + OTLPSpanExporter( + endpoint=signal_endpoint(endpoint, "traces"), timeout=EXPORT_TIMEOUT_SECONDS + ) + ) ) trace.set_tracer_provider(provider) diff --git a/tests/test_propagation.py b/tests/test_propagation.py index 3294c59..bd509c7 100644 --- a/tests/test_propagation.py +++ b/tests/test_propagation.py @@ -207,3 +207,41 @@ def missing(name: str, *args: Any, **kwargs: Any) -> Any: monkeypatch.setattr(builtins, "__import__", missing) assert start_telemetry(service_name="s", version="v", environment="test") is False + + +# --- the endpoint bug that shipped in 0.5.0 --------------------------------- + + +@pytest.mark.parametrize( + ("given", "signal", "expected"), + [ + ("http://collector:4318", "traces", "http://collector:4318/v1/traces"), + ("http://collector:4318", "metrics", "http://collector:4318/v1/metrics"), + ("http://collector:4318/", "traces", "http://collector:4318/v1/traces"), + # Already specific: left alone rather than doubled. + ("http://c:4318/v1/traces", "traces", "http://c:4318/v1/traces"), + ("", "traces", ""), + ], +) +def test_the_endpoint_gets_its_signal_path(given: str, signal: str, expected: str) -> None: + """Passing `endpoint=` to an OTLP exporter overrides the SDK and is used + VERBATIM; only reading OTEL_EXPORTER_OTLP_ENDPOINT itself makes the SDK + append the signal path. + + 0.5.0 handed both exporters one base URL, so traces and metrics both POSTed + to the collector's root and got 404. The service served perfectly and the + only evidence was an export error in the logs, which is why this reached a + cluster before anyone noticed. + """ + from kit.observability._tracing import signal_endpoint + + assert signal_endpoint(given, signal) == expected + + +def test_traces_and_metrics_do_not_share_one_url() -> None: + """The specific thing that was broken: one endpoint cannot be both signals.""" + from kit.observability._tracing import signal_endpoint + + base = "http://collector:4318" + + assert signal_endpoint(base, "traces") != signal_endpoint(base, "metrics") diff --git a/uv.lock b/uv.lock index 6e024b8..7e2b4e9 100644 --- a/uv.lock +++ b/uv.lock @@ -623,7 +623,7 @@ wheels = [ [[package]] name = "scadable-kit" -version = "0.5.0" +version = "0.5.1" source = { editable = "." } dependencies = [ { name = "fastapi" },