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" },