From c7fcbb9a7245b20e448594f21e92c4eb75029a46 Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Mon, 1 Jun 2026 09:57:09 +0300 Subject: [PATCH] refactor: OTel instrument touch-ups (REF-1, LOW-3, LOW-5) REF-1: _build_excluded_urls() was character-for-character identical in FastAPIOpenTelemetryInstrument and LitestarOpenTelemetryInstrument. Hoist to the base OpenTelemetryInstrument and use getattr with safe defaults so the method works when the base instrument runs against a config without the framework-specific fields (opentelemetry_excluded_urls, prometheus_metrics_path, health_checks_path). LOW-3: _format_span used os.linesep, which produces \r\n on Windows. The OTel SDK convention is plain \n; change to a literal. LOW-5: Add a one-line comment on LitestarOpenTelemetryInstrumentationMiddleware._otel_apps documenting the id(next_app) cache assumption (ASGI app instances are stable for the middleware lifetime). No behavior change. Closes REF-1, LOW-3, LOW-5 from the audit. --- .../bootstrappers/fastapi_bootstrapper.py | 8 -------- .../bootstrappers/litestar_bootstrapper.py | 10 ++-------- .../instruments/opentelemetry_instrument.py | 13 ++++++++++++- 3 files changed, 14 insertions(+), 17 deletions(-) diff --git a/lite_bootstrap/bootstrappers/fastapi_bootstrapper.py b/lite_bootstrap/bootstrappers/fastapi_bootstrapper.py index 838e6a0..39a371d 100644 --- a/lite_bootstrap/bootstrappers/fastapi_bootstrapper.py +++ b/lite_bootstrap/bootstrappers/fastapi_bootstrapper.py @@ -112,14 +112,6 @@ def bootstrap(self) -> None: class FastAPIOpenTelemetryInstrument(OpenTelemetryInstrument): bootstrap_config: FastAPIConfig - def _build_excluded_urls(self) -> set[str]: - excluded_urls = set(self.bootstrap_config.opentelemetry_excluded_urls) - excluded_urls.add(self.bootstrap_config.prometheus_metrics_path) - if not self.bootstrap_config.opentelemetry_generate_health_check_spans: - excluded_urls.add(self.bootstrap_config.health_checks_path) - - return excluded_urls - def bootstrap(self) -> None: super().bootstrap() FastAPIInstrumentor.instrument_app( diff --git a/lite_bootstrap/bootstrappers/litestar_bootstrapper.py b/lite_bootstrap/bootstrappers/litestar_bootstrapper.py index 7a026d9..dc381e1 100644 --- a/lite_bootstrap/bootstrappers/litestar_bootstrapper.py +++ b/lite_bootstrap/bootstrappers/litestar_bootstrapper.py @@ -76,6 +76,8 @@ class LitestarOpenTelemetryInstrumentationMiddleware(ASGIMiddleware): def __init__(self, tracer_provider: "TracerProvider", excluded_urls: set[str]) -> None: self._tracer_provider = tracer_provider self._excluded_urls = ",".join(excluded_urls) + # Cache keyed by id(next_app); Litestar's ASGI app instances are stable for + # the middleware lifetime, so id-reuse-after-GC isn't a concern. self._otel_apps: dict[int, ASGIApp] = {} async def handle( @@ -178,14 +180,6 @@ def bootstrap(self) -> None: class LitestarOpenTelemetryInstrument(OpenTelemetryInstrument): bootstrap_config: LitestarConfig - def _build_excluded_urls(self) -> set[str]: - excluded_urls = set(self.bootstrap_config.opentelemetry_excluded_urls) - excluded_urls.add(self.bootstrap_config.prometheus_metrics_path) - if not self.bootstrap_config.opentelemetry_generate_health_check_spans: - excluded_urls.add(self.bootstrap_config.health_checks_path) - - return excluded_urls - def bootstrap(self) -> None: super().bootstrap() self.bootstrap_config.application_config.middleware.append( diff --git a/lite_bootstrap/instruments/opentelemetry_instrument.py b/lite_bootstrap/instruments/opentelemetry_instrument.py index 1e7eeda..cf4d8f9 100644 --- a/lite_bootstrap/instruments/opentelemetry_instrument.py +++ b/lite_bootstrap/instruments/opentelemetry_instrument.py @@ -23,7 +23,7 @@ def _format_span(readable_span: "ReadableSpan") -> str: - return typing.cast("str", readable_span.to_json(indent=None)) + os.linesep + return typing.cast("str", readable_span.to_json(indent=None)) + "\n" @dataclasses.dataclass(kw_only=True, slots=True, frozen=True) @@ -92,6 +92,17 @@ def is_ready(self) -> bool: def check_dependencies() -> bool: return import_checker.is_opentelemetry_installed + def _build_excluded_urls(self) -> set[str]: + excluded_urls: set[str] = set(getattr(self.bootstrap_config, "opentelemetry_excluded_urls", [])) + prometheus_path = getattr(self.bootstrap_config, "prometheus_metrics_path", None) + if prometheus_path: + excluded_urls.add(prometheus_path) + if not self.bootstrap_config.opentelemetry_generate_health_check_spans: + health_path = getattr(self.bootstrap_config, "health_checks_path", None) + if health_path: + excluded_urls.add(health_path) + return excluded_urls + def bootstrap(self) -> None: logging.getLogger("opentelemetry.instrumentation.instrumentor").disabled = True logging.getLogger("opentelemetry.trace").disabled = True