diff --git a/docs/introduction/configuration.md b/docs/introduction/configuration.md index 5285ff6..a920e78 100644 --- a/docs/introduction/configuration.md +++ b/docs/introduction/configuration.md @@ -116,7 +116,7 @@ Additional parameters: - `opentelemetry_log_traces` - traces will be logged to stdout. - `opentelemetry_sampler` - an `opentelemetry.sdk.trace.sampling.Sampler` deciding which traces are recorded. Unset, the SDK's own default applies: `parentbased_always_on`, which records every trace that is not the child of a non-recording remote parent, unless `OTEL_TRACES_SAMPLER` / `OTEL_TRACES_SAMPLER_ARG` say otherwise. - `opentelemetry_generate_health_check_spans` - generate spans for health check handlers if `True`. -- `opentelemetry_excluded_urls` - extra URLs excluded from tracing; the metrics path and (unless health-check spans are enabled) the health-check path are excluded automatically. +- `opentelemetry_excluded_urls` - extra regexes for URLs excluded from tracing; the metrics path and (unless health-check spans are enabled) the health-check path are excluded automatically. Litestar searches them in the request path and also honors `OTEL_PYTHON_LITESTAR_EXCLUDED_URLS`; FastAPI searches them in the full URL. - `opentelemetry_metrics_endpoint` - will be passed to `OTLPMetricExporter` as endpoint, and turns on the metrics signal. Its own field rather than a flag on `opentelemetry_endpoint`, so upgrading never starts exporting metrics you did not ask for. Under `opentelemetry_exporter_protocol="http"` this is a full URL (e.g. `http://collector:4318/v1/metrics`); `opentelemetry_exporter_protocol` and `opentelemetry_insecure` are shared with traces. ### Metrics diff --git a/lite_bootstrap/bootstrappers/litestar_bootstrapper.py b/lite_bootstrap/bootstrappers/litestar_bootstrapper.py index 247f754..5417afa 100644 --- a/lite_bootstrap/bootstrappers/litestar_bootstrapper.py +++ b/lite_bootstrap/bootstrappers/litestar_bootstrapper.py @@ -1,9 +1,7 @@ -import contextlib import dataclasses import pathlib import re import typing -import weakref from lite_bootstrap import import_checker from lite_bootstrap.bootstrappers.base import BaseBootstrapper @@ -49,11 +47,10 @@ if import_checker.is_litestar_opentelemetry_installed: from litestar.middleware import ASGIMiddleware + from litestar.plugins.opentelemetry import OpenTelemetryConfig as LitestarOpenTelemetryConfig + from litestar.plugins.opentelemetry import OpenTelemetryPlugin from litestar.types.asgi_types import ASGIApp, Receive, Scope, Send - from opentelemetry.instrumentation.asgi import OpenTelemetryMiddleware - from opentelemetry.metrics import MeterProvider - from opentelemetry.trace import TracerProvider - from opentelemetry.util.http import parse_excluded_urls + from opentelemetry import trace if import_checker.is_opentelemetry_installed: from opentelemetry.metrics import get_meter_provider @@ -75,9 +72,6 @@ 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. @@ -85,36 +79,21 @@ def build_span_name(method: str, route: str) -> str: def build_litestar_route_details_from_scope( - scope: typing.MutableMapping[str, typing.Any], + scope: "Scope", ) -> tuple[str, dict[str, str]]: - path_template: typing.Final = scope.get("path_template") method: typing.Final = str(scope.get("method", "HTTP")).strip() - if path_template is not None: - path_template_stripped: typing.Final = path_template.strip() - return build_span_name(method, path_template_stripped), {"http.route": path_template_stripped} - - path: typing.Final = scope.get("path") - if path is not None: - path_stripped: typing.Final = path.strip() - return build_span_name(method, path_stripped), {"http.route": path_stripped} - return method, {} + path_template: typing.Final = scope.get("path_template") + # Unmatched paths get no `http.route`: a raw path would let scanners explode span cardinality + if path_template is None: + return method, {} + path_template_stripped: typing.Final = path_template.strip() + return build_span_name(method, path_template_stripped), {"http.route": path_template_stripped} if import_checker.is_litestar_opentelemetry_installed: - class LitestarOpenTelemetryInstrumentationMiddleware(ASGIMiddleware): - def __init__( - self, tracer_provider: "TracerProvider", meter_provider: "MeterProvider", excluded_urls: set[str] - ) -> None: - self._tracer_provider = tracer_provider - self._meter_provider = meter_provider - # OpenTelemetryMiddleware only parses a raw string from 0.56b0; the floor is 0.49b0. - self._excluded_urls = parse_excluded_urls(",".join(excluded_urls)) - # WeakKeyDictionary so wrapper apps are evicted when Litestar drops the - # next_app reference (hot reload, plugin add/remove, AppConfig rebuild). - # Apps that don't support weak references are simply not cached. - self._otel_apps: weakref.WeakKeyDictionary[ASGIApp, ASGIApp] = weakref.WeakKeyDictionary() - + class LitestarOpenTelemetryRouteMiddleware(ASGIMiddleware): + # OpenTelemetryPlugin opens the server span before routing, so the route template is only known here async def handle( self, scope: "Scope", @@ -122,20 +101,12 @@ async def handle( send: "Send", next_app: "ASGIApp", ) -> None: - otel_app: ASGIApp | None = None - with contextlib.suppress(TypeError): - otel_app = self._otel_apps.get(next_app) - if otel_app is None: - otel_app = OpenTelemetryMiddleware( # ty: ignore[invalid-assignment] - app=next_app, - default_span_details=build_litestar_route_details_from_scope, - excluded_urls=self._excluded_urls, - tracer_provider=self._tracer_provider, - meter_provider=self._meter_provider, - ) - with contextlib.suppress(TypeError): - self._otel_apps[next_app] = otel_app # ty: ignore[invalid-assignment] - await otel_app(scope, receive, send) # ty: ignore[call-non-callable] + server_span: typing.Final = trace.get_current_span() + if server_span.is_recording(): + span_name, attributes = build_litestar_route_details_from_scope(scope) + server_span.update_name(span_name) + server_span.set_attributes(attributes) + await next_app(scope, receive, send) @dataclasses.dataclass(kw_only=True, slots=True, frozen=True) @@ -240,32 +211,43 @@ def _configure_structlog_loggers(self) -> None: @dataclasses.dataclass(kw_only=True) class LitestarOpenTelemetryInstrument(OpenTelemetryInstrument): bootstrap_config: LitestarConfig + missing_dependency_message = "opentelemetry-instrumentation-asgi or litestar>=2.22 is not installed" - def _build_excluded_url_patterns(self) -> set[str]: + @staticmethod + def dependencies_installed() -> bool: + return OpenTelemetryInstrument.dependencies_installed() and import_checker.is_litestar_opentelemetry_installed + + def _build_excluded_path_patterns(self) -> list[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. + The trailing slash is stripped and matched optionally, so ``/custom-health/`` excludes the + path with or without it. Anchoring is needed because Litestar searches its ``exclude`` + patterns unanchored: a bare ``/custom-health`` would also silence ``/custom-healthy``. + Caller-supplied entries stay untouched because they are regexes. """ anchored_patterns: typing.Final = { - rf"{_EXCLUDED_URL_SCHEME_AND_HOST}{re.escape(normalized_path)}(?:/|$)" + rf"^{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) + return sorted(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(), - meter_provider=get_meter_provider(), - excluded_urls=self._build_excluded_url_patterns(), + application_config: typing.Final = self.bootstrap_config.application_config + application_config.plugins.append( + OpenTelemetryPlugin( + LitestarOpenTelemetryConfig( + tracer_provider=get_tracer_provider(), + meter_provider=get_meter_provider(), + scope_span_details_extractor=build_litestar_route_details_from_scope, + # An empty list compiles to a pattern that matches, and so skips, every path + exclude=self._build_excluded_path_patterns() or None, + ) ) ) + application_config.middleware.append(LitestarOpenTelemetryRouteMiddleware()) @dataclasses.dataclass(kw_only=True) diff --git a/lite_bootstrap/import_checker.py b/lite_bootstrap/import_checker.py index e3b0e4b..55842df 100644 --- a/lite_bootstrap/import_checker.py +++ b/lite_bootstrap/import_checker.py @@ -31,7 +31,10 @@ def _safe_find_spec(module_name: str) -> bool: "opentelemetry.instrumentation.fastapi" ) is_litestar_opentelemetry_installed = ( - is_opentelemetry_installed and is_litestar_installed and _safe_find_spec("opentelemetry.instrumentation.asgi") + is_opentelemetry_installed + and is_litestar_installed + and _safe_find_spec("opentelemetry.instrumentation.asgi") + and _safe_find_spec("litestar.plugins.opentelemetry") ) is_otlp_grpc_exporter_installed = _safe_find_spec("opentelemetry.exporter.otlp.proto.grpc.trace_exporter") is_otlp_http_exporter_installed = _safe_find_spec("opentelemetry.exporter.otlp.proto.http.trace_exporter") diff --git a/pyproject.toml b/pyproject.toml index 55f4eda..46bb1fa 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -123,7 +123,7 @@ fastapi-all = [ ] litestar = [ # >=2.13 for AppConfig.request_max_body_size (LitestarBootstrapper._apply_config reads it); - # >=2.15 for litestar.middleware.ASGIMiddleware (LitestarOpenTelemetryInstrumentationMiddleware + # >=2.15 for litestar.middleware.ASGIMiddleware (LitestarOpenTelemetryRouteMiddleware # subclasses it); >=2.19 because litestar/concurrency.py imports sniffio and only 2.19 declares # it -- until then it arrived via anyio, which dropped the dependency in 4.14. The highest wins. "litestar>=2.19", @@ -137,6 +137,8 @@ litestar-sentry = [ ] litestar-otl = [ "lite-bootstrap[litestar,otl]", + # >=2.22 for litestar.plugins.opentelemetry (LitestarOpenTelemetryInstrument registers its OpenTelemetryPlugin). + "litestar>=2.22", # Raised from 0.46b0 with the rest of the train: every instrumentation package pins # `opentelemetry-instrumentation` to its own exact version, so they move together. "opentelemetry-instrumentation-asgi>=0.49b0", diff --git a/tests/test_litestar_bootstrap.py b/tests/test_litestar_bootstrap.py index de7955f..b3f55de 100644 --- a/tests/test_litestar_bootstrap.py +++ b/tests/test_litestar_bootstrap.py @@ -1,15 +1,12 @@ import contextlib import dataclasses -import gc import inspect import json import logging import sys import typing import warnings -import weakref from collections.abc import Generator -from unittest.mock import AsyncMock, patch import litestar import pytest @@ -17,21 +14,23 @@ from litestar import status_codes from litestar.config.app import AppConfig from litestar.middleware.logging import LoggingMiddlewareConfig +from litestar.plugins import InitPluginProtocol +from litestar.plugins.opentelemetry import OpenTelemetryPlugin from litestar.testing import TestClient +from litestar.types import Scope from opentelemetry.metrics import get_meter_provider from opentelemetry.sdk.metrics import MeterProvider as SDKMeterProvider -from opentelemetry.sdk.trace import TracerProvider +from opentelemetry.sdk.trace import ReadableSpan from opentelemetry.sdk.trace import TracerProvider as SDKTracerProvider from opentelemetry.sdk.trace.export import SimpleSpanProcessor from opentelemetry.sdk.trace.export.in_memory_span_exporter import InMemorySpanExporter -from opentelemetry.trace import get_tracer_provider +from opentelemetry.trace import SpanKind, get_tracer_provider from lite_bootstrap import LitestarBootstrapper, LitestarConfig, import_checker from lite_bootstrap.bootstrappers import litestar_bootstrapper from lite_bootstrap.bootstrappers.litestar_bootstrapper import ( _LITESTAR_DEFAULT_REQUEST_MAX_BODY_SIZE, LitestarLoggingInstrument, - LitestarOpenTelemetryInstrumentationMiddleware, build_litestar_route_details_from_scope, build_span_name, ) @@ -212,73 +211,20 @@ def test_build_span_name_no_route() -> None: assert build_span_name("GET", "") == "GET" -def test_build_litestar_route_details_from_scope_path_fallback() -> None: - scope = {"method": "POST", "path": "/fallback/path"} +def test_build_litestar_route_details_from_scope_ignores_raw_path() -> None: + scope = typing.cast("Scope", {"method": "POST", "path": "/fallback/path"}) name, attrs = build_litestar_route_details_from_scope(scope) - assert name == "POST /fallback/path" - assert attrs == {"http.route": "/fallback/path"} + assert name == "POST" + assert attrs == {} def test_build_litestar_route_details_from_scope_no_path() -> None: - scope = {"type": "lifespan"} + scope = typing.cast("Scope", {"type": "lifespan"}) name, attrs = build_litestar_route_details_from_scope(scope) assert name == "HTTP" assert attrs == {} -class _NotWeakrefable: - __slots__ = () - - async def __call__(self, scope: object, receive: object, send: object) -> None: # noqa: ARG002 - return None - - -async def test_litestar_otel_apps_cache_skips_non_weakrefable_app() -> None: - """Apps that don't support weak references are called but not cached.""" - tracer_provider = TracerProvider() - middleware = LitestarOpenTelemetryInstrumentationMiddleware( - tracer_provider=tracer_provider, - meter_provider=SDKMeterProvider(), - excluded_urls=set(), - ) - - non_weakrefable_app = _NotWeakrefable() - # Confirm the key really is non-weakrefable so the test is meaningful. - with pytest.raises(TypeError): - weakref.ref(non_weakrefable_app) - - # Call handle() twice with the non-weakrefable next_app. Both calls must succeed - # (TypeError suppressed on both lookup and store) and the cache stays empty. - scope: dict = {"type": "lifespan"} - await middleware.handle(scope, object(), object(), non_weakrefable_app) # type: ignore[arg-type] # ty: ignore[invalid-argument-type] - await middleware.handle(scope, object(), object(), non_weakrefable_app) # type: ignore[arg-type] # ty: ignore[invalid-argument-type] - - assert len(middleware._otel_apps) == 0 # noqa: SLF001 - - -def test_litestar_otel_apps_cache_evicts_dead_refs() -> None: - tracer_provider = TracerProvider() - middleware = LitestarOpenTelemetryInstrumentationMiddleware( - tracer_provider=tracer_provider, - meter_provider=SDKMeterProvider(), - excluded_urls=set(), - ) - - async def transient_app(scope: dict, receive: object, send: object) -> None: # noqa: ARG001 - return None # pragma: no cover - only a weakref target; the test never calls it - - weak_app = weakref.ref(transient_app) - middleware._otel_apps[transient_app] = "marker" # noqa: SLF001 # ty: ignore[invalid-assignment] - assert weak_app() is not None - assert len(middleware._otel_apps) == 1 # noqa: SLF001 - - del transient_app - gc.collect() - - assert weak_app() is None - assert len(middleware._otel_apps) == 0 # noqa: SLF001 - - def test_litestar_bootstrap_without_prometheus_client() -> None: # Regression: `import lite_bootstrap` with the `litestar` extra but not # prometheus_client crashed because litestar_bootstrapper imported @@ -541,26 +487,6 @@ def test_second_litestar_bootstrapper_bootstrap_raises(litestar_config: Litestar first.teardown() -def test_litestar_otel_middleware_hands_the_instrumentor_a_parsed_exclude_list() -> None: - """INVARIANT: `excluded_urls` reaches OpenTelemetryMiddleware parsed, never as a raw string. - - `OpenTelemetryMiddleware` only learned to parse a string itself in - opentelemetry-instrumentation 0.56b0. The declared floor is 0.49b0, where a string reaches - `self.excluded_urls.url_disabled(url)` and raises `AttributeError` on every request, so a - Litestar service with OpenTelemetry returns 500 for everything. - """ - middleware = LitestarOpenTelemetryInstrumentationMiddleware( - tracer_provider=TracerProvider(), - meter_provider=SDKMeterProvider(), - excluded_urls={"/custom-metrics"}, - ) - - excluded_urls = middleware._excluded_urls # noqa: SLF001 - assert not isinstance(excluded_urls, str) - 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: @@ -621,40 +547,128 @@ async def get_item(item_id: int) -> dict[str, int]: assert exporter.get_finished_spans() == () -async def test_litestar_otel_middleware_hands_the_instrumentor_the_meter_provider() -> None: - """opentelemetry-instrumentation-asgi builds duration histograms when given a meter provider.""" - meter_provider = SDKMeterProvider() - middleware = LitestarOpenTelemetryInstrumentationMiddleware( - tracer_provider=TracerProvider(), - meter_provider=meter_provider, - excluded_urls=set(), - ) - - async def next_app(scope: dict, receive: object, send: object) -> None: # noqa: ARG001 - return None # pragma: no cover - the patched middleware never calls through +def _bootstrap_with_span_exporter(config: LitestarConfig) -> tuple[litestar.Litestar, InMemorySpanExporter]: + 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)) + return application, exporter - scope: dict = {"type": "http"} - with patch.object(litestar_bootstrapper, "OpenTelemetryMiddleware") as mock_middleware: - mock_middleware.return_value = AsyncMock() - await middleware.handle(scope, object(), object(), next_app) # type: ignore[arg-type] # ty: ignore[invalid-argument-type] - assert mock_middleware.call_args.kwargs["meter_provider"] is meter_provider +def _server_spans(exporter: InMemorySpanExporter) -> list[ReadableSpan]: + return [span for span in exporter.get_finished_spans() if span.kind == SpanKind.SERVER] -def test_litestar_otel_instrument_passes_the_installed_meter_provider(litestar_config: LitestarConfig) -> None: +def test_litestar_otel_registers_litestars_own_plugin(litestar_config: LitestarConfig) -> None: config = dataclasses.replace(litestar_config, opentelemetry_metrics_endpoint="localhost:4317") bootstrapper = LitestarBootstrapper(bootstrap_config=config) try: bootstrapper.bootstrap() - appended = [ - one_middleware - for one_middleware in config.application_config.middleware - if isinstance(one_middleware, LitestarOpenTelemetryInstrumentationMiddleware) - ] - assert len(appended) == 1 - assert appended[0]._meter_provider is get_meter_provider() # noqa: SLF001 + plugins = [plugin for plugin in config.application_config.plugins if isinstance(plugin, OpenTelemetryPlugin)] + assert len(plugins) == 1 + assert plugins[0].config.tracer_provider is get_tracer_provider() + assert plugins[0].config.meter_provider is get_meter_provider() assert isinstance(get_meter_provider(), SDKMeterProvider) finally: bootstrapper.teardown() + + +def test_litestar_otel_keeps_user_plugins(litestar_config: LitestarConfig) -> None: + class UserPlugin(InitPluginProtocol): + def on_app_init(self, app_config: AppConfig) -> AppConfig: + return app_config + + config = dataclasses.replace(litestar_config, application_config=AppConfig(plugins=[UserPlugin()])) + application = LitestarBootstrapper(bootstrap_config=config).bootstrap() + + assert any(isinstance(plugin, UserPlugin) for plugin in application.plugins.init) + assert any(isinstance(plugin, OpenTelemetryPlugin) for plugin in application.plugins.init) + + +def test_litestar_otel_span_carries_route_template(litestar_config: LitestarConfig) -> None: + @litestar.get("/users/{user_id:int}") + async def get_user(user_id: int) -> dict[str, int]: + return {"user_id": user_id} + + config = dataclasses.replace(litestar_config, application_config=AppConfig(route_handlers=[get_user])) + application, exporter = _bootstrap_with_span_exporter(config) + + with TestClient(app=application) as client: + assert client.get("/users/123").status_code == status_codes.HTTP_200_OK + + server_spans = _server_spans(exporter) + assert [span.name for span in server_spans] == ["GET /users/{user_id}"] + assert server_spans[0].attributes is not None + assert server_spans[0].attributes["http.route"] == "/users/{user_id}" + + +def test_litestar_otel_unmatched_path_span_is_named_by_method_only(litestar_config: LitestarConfig) -> None: + """A 404 never reaches the route stack, so its span must not carry the raw path that scanners vary.""" + application, exporter = _bootstrap_with_span_exporter(litestar_config) + + with TestClient(app=application) as client: + assert client.get("/missing/abc").status_code == status_codes.HTTP_404_NOT_FOUND + + server_spans = _server_spans(exporter) + assert [span.name for span in server_spans] == ["GET"] + assert server_spans[0].attributes is not None + assert "http.route" not in server_spans[0].attributes + + +def test_litestar_otel_honors_excluded_urls_from_environment( + litestar_config: LitestarConfig, monkeypatch: pytest.MonkeyPatch +) -> None: + monkeypatch.setenv("OTEL_PYTHON_LITESTAR_EXCLUDED_URLS", "/from-env") + + @litestar.get("/from-env") + async def from_env() -> None: ... + + @litestar.get("/other") + async def other() -> None: ... + + config = dataclasses.replace(litestar_config, application_config=AppConfig(route_handlers=[from_env, other])) + application, exporter = _bootstrap_with_span_exporter(config) + + with TestClient(app=application) as client: + client.get("/from-env") + client.get("/other") + + assert [span.name for span in _server_spans(exporter)] == ["GET /other"] + + +def test_litestar_otel_traces_everything_when_nothing_is_excluded(litestar_config: LitestarConfig) -> None: + """An empty exclude list compiles to a pattern matching every path, so it must never reach the plugin.""" + + @litestar.get("/traced") + async def traced() -> None: ... + + config = dataclasses.replace( + litestar_config, + application_config=AppConfig(route_handlers=[traced]), + prometheus_metrics_path="", + opentelemetry_generate_health_check_spans=True, + ) + application, exporter = _bootstrap_with_span_exporter(config) + + with TestClient(app=application) as client: + client.get("/traced") + + assert [span.name for span in _server_spans(exporter)] == ["GET /traced"] + + +def test_litestar_otel_is_skipped_without_litestars_opentelemetry_plugin(litestar_config: LitestarConfig) -> None: + """Litestar below 2.22 has no `litestar.plugins.opentelemetry`; that must skip the instrument, not crash.""" + with emulate_package_missing_with_module_reload( + "litestar.plugins.opentelemetry", + ["lite_bootstrap.bootstrappers.litestar_bootstrapper"], + ): + with pytest.warns(UserWarning, match=r"litestar>=2\.22"): + bootstrapper = litestar_bootstrapper.LitestarBootstrapper(bootstrap_config=litestar_config) + bootstrapper.bootstrap() + + assert "OpenTelemetryPlugin" not in { + type(plugin).__name__ for plugin in litestar_config.application_config.plugins + }