diff --git a/lite_bootstrap/bootstrappers/litestar_bootstrapper.py b/lite_bootstrap/bootstrappers/litestar_bootstrapper.py index a846c53..24ccb72 100644 --- a/lite_bootstrap/bootstrappers/litestar_bootstrapper.py +++ b/lite_bootstrap/bootstrappers/litestar_bootstrapper.py @@ -73,6 +73,9 @@ def build_span_name(method: str, route: str) -> str: _LOGGING_MIDDLEWARE_REQUEST_LOG_FIELDS: typing.Final = ("path", "method", "content_type", "path_params") _LOGGING_MIDDLEWARE_RESPONSE_LOG_FIELDS: typing.Final = ("status_code",) +# OpenTelemetryMiddleware matches its patterns against a full URL, not a bare path. +_EXCLUDED_URL_SCHEME_AND_HOST: typing.Final = r"^\w+://[^/]*" + # Litestar.from_config() passes every AppConfig field explicitly, so the default that # Litestar.__init__ applies never reaches an app built from a config. Pinned to Litestar's # own default by a guard test. See https://github.com/litestar-org/litestar/issues/4296. @@ -232,12 +235,28 @@ def _configure_structlog_loggers(self) -> None: class LitestarOpenTelemetryInstrument(OpenTelemetryInstrument): bootstrap_config: LitestarConfig + def _build_excluded_url_patterns(self) -> set[str]: + """Anchored patterns for the derived paths, plus the caller's own entries verbatim. + + Litestar normalizes the trailing slash out of ``scope["path"]``, so a derived path + carrying one never matches. Stripping it alone is not enough: ``ExcludeList`` searches + unanchored, so a bare ``/custom-health`` would also silence ``/custom-healthy``. + Caller-supplied entries stay untouched because OpenTelemetry documents them as regexes. + """ + anchored_patterns: typing.Final = { + rf"{_EXCLUDED_URL_SCHEME_AND_HOST}{re.escape(normalized_path)}(?:/|$)" + for excluded_path in self._build_infrastructure_excluded_paths() + # A bare "/" would anchor to every URL, so it is dropped along with empty values. + if (normalized_path := excluded_path.rstrip("/")) + } + return anchored_patterns | set(self.bootstrap_config.opentelemetry_excluded_urls) + def bootstrap(self) -> None: super().bootstrap() self.bootstrap_config.application_config.middleware.append( LitestarOpenTelemetryInstrumentationMiddleware( tracer_provider=get_tracer_provider(), - excluded_urls=self._build_excluded_urls(), + excluded_urls=self._build_excluded_url_patterns(), ) ) diff --git a/lite_bootstrap/instruments/opentelemetry_instrument.py b/lite_bootstrap/instruments/opentelemetry_instrument.py index 1f709c6..7ad2c52 100644 --- a/lite_bootstrap/instruments/opentelemetry_instrument.py +++ b/lite_bootstrap/instruments/opentelemetry_instrument.py @@ -163,17 +163,21 @@ def dependencies_installed() -> bool: # the sdk (opentelemetry.sdk.*), so it needs both distributions present. return import_checker.is_opentelemetry_installed and import_checker.is_opentelemetry_sdk_installed - def _build_excluded_urls(self) -> set[str]: + def _build_infrastructure_excluded_paths(self) -> set[str]: + """Paths this policy derives itself, without the caller-supplied entries.""" config = self.bootstrap_config - excluded_urls: set[str] = set(config.opentelemetry_excluded_urls) + excluded_paths: set[str] = set() prometheus_path = getattr(config, "prometheus_metrics_path", None) if prometheus_path: - excluded_urls.add(prometheus_path) + excluded_paths.add(prometheus_path) if not config.opentelemetry_generate_health_check_spans: health_path = getattr(config, "health_checks_path", None) if health_path: - excluded_urls.add(health_path) - return excluded_urls + excluded_paths.add(health_path) + return excluded_paths + + def _build_excluded_urls(self) -> set[str]: + return set(self.bootstrap_config.opentelemetry_excluded_urls) | self._build_infrastructure_excluded_paths() def _silence_otel_loggers(self) -> None: for logger_name in ("opentelemetry.instrumentation.instrumentor", "opentelemetry.trace"): diff --git a/tests/test_litestar_bootstrap.py b/tests/test_litestar_bootstrap.py index 17689a5..c07aad8 100644 --- a/tests/test_litestar_bootstrap.py +++ b/tests/test_litestar_bootstrap.py @@ -549,7 +549,65 @@ def test_litestar_otel_middleware_hands_the_instrumentor_a_parsed_exclude_list() excluded_urls = middleware._excluded_urls # noqa: SLF001 assert not isinstance(excluded_urls, str) - # Matched against the URL the middleware builds from `scope["path"]`, which Litestar has - # already normalized; see #248 for the trailing-slash entries that therefore never match. assert excluded_urls.url_disabled("http://test/custom-metrics") assert not excluded_urls.url_disabled("http://test/items/1") + + +def test_litestar_otel_excludes_infrastructure_paths_normalized_by_litestar( + litestar_config: LitestarConfig, +) -> None: + """REGRESSION #248: Litestar strips the trailing slash before the middleware sees the path. + + `health_checks_path="/custom-health/"` reaches `OpenTelemetryMiddleware` as + `http://host/custom-health`, so an exclude entry carrying the slash never matched and the + health check was traced anyway. The lookalike route guards the obvious over-correction: + `ExcludeList` regex-searches unanchored, so a bare `/custom-health` prefix would also + silence `/custom-healthy`. + """ + + @litestar.get("/custom-healthy") + async def lookalike_handler() -> dict[str, str]: + return {"status": "ok"} + + config = dataclasses.replace(litestar_config, application_config=AppConfig(route_handlers=[lookalike_handler])) + application = LitestarBootstrapper(bootstrap_config=config).bootstrap() + + tracer_provider = get_tracer_provider() + assert isinstance(tracer_provider, SDKTracerProvider) + exporter = InMemorySpanExporter() + tracer_provider.add_span_processor(SimpleSpanProcessor(exporter)) + + with TestClient(app=application) as client: + assert client.get(config.health_checks_path).status_code == status_codes.HTTP_200_OK + assert client.get(config.prometheus_metrics_path).status_code == status_codes.HTTP_200_OK + assert client.get("/custom-healthy").status_code == status_codes.HTTP_200_OK + + span_names = [span.name for span in exporter.get_finished_spans()] + assert "GET /custom-health" not in span_names + assert "GET /custom-metrics" not in span_names + assert "GET /custom-healthy" in span_names + + +def test_litestar_otel_keeps_caller_supplied_excluded_urls_as_regexes(litestar_config: LitestarConfig) -> None: + """`opentelemetry_excluded_urls` entries are OpenTelemetry regexes, so anchoring must not touch them.""" + + @litestar.get("/items/{item_id:int}") + async def get_item(item_id: int) -> dict[str, int]: + return {"item_id": item_id} + + config = dataclasses.replace( + litestar_config, + application_config=AppConfig(route_handlers=[get_item]), + opentelemetry_excluded_urls=[r"/items/\d+$"], + ) + application = LitestarBootstrapper(bootstrap_config=config).bootstrap() + + tracer_provider = get_tracer_provider() + assert isinstance(tracer_provider, SDKTracerProvider) + exporter = InMemorySpanExporter() + tracer_provider.add_span_processor(SimpleSpanProcessor(exporter)) + + with TestClient(app=application) as client: + assert client.get("/items/42").status_code == status_codes.HTTP_200_OK + + assert exporter.get_finished_spans() == ()