diff --git a/docs/introduction/configuration.md b/docs/introduction/configuration.md index e381dda..d4cc875 100644 --- a/docs/introduction/configuration.md +++ b/docs/introduction/configuration.md @@ -77,10 +77,7 @@ Additional parameters: - `opentelemetry_instrumentors` - a list of extra instrumentors. - `opentelemetry_log_traces` - traces will be logged to stdout. - `opentelemetry_generate_health_check_spans` - generate spans for health check handlers if `True`. - -Additional parameters for Litestar and FastAPI: - -- `opentelemetry_excluded_urls` - by default, heath checks and metrics paths will be excluded. +- `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. For FastStream you must provide additionally: diff --git a/lite_bootstrap/bootstrappers/fastapi_bootstrapper.py b/lite_bootstrap/bootstrappers/fastapi_bootstrapper.py index e9d51a0..67b6d91 100644 --- a/lite_bootstrap/bootstrappers/fastapi_bootstrapper.py +++ b/lite_bootstrap/bootstrappers/fastapi_bootstrapper.py @@ -50,7 +50,6 @@ class FastAPIConfig( ): application: "fastapi.FastAPI | UnsetType" = UNSET application_kwargs: dict[str, typing.Any] = dataclasses.field(default_factory=dict) - opentelemetry_excluded_urls: list[str] = dataclasses.field(default_factory=list) prometheus_instrumentator_params: dict[str, typing.Any] = dataclasses.field(default_factory=dict) prometheus_instrument_params: dict[str, typing.Any] = dataclasses.field(default_factory=dict) prometheus_expose_params: dict[str, typing.Any] = dataclasses.field(default_factory=dict) diff --git a/lite_bootstrap/bootstrappers/faststream_bootstrapper.py b/lite_bootstrap/bootstrappers/faststream_bootstrapper.py index 2f795e9..557ab20 100644 --- a/lite_bootstrap/bootstrappers/faststream_bootstrapper.py +++ b/lite_bootstrap/bootstrappers/faststream_bootstrapper.py @@ -66,7 +66,6 @@ class FastStreamConfig( ): application: "AsgiFastStream" = dataclasses.field(default_factory=_make_asgi_faststream) opentelemetry_middleware_cls: type[FastStreamTelemetryMiddlewareProtocol] | None = None - opentelemetry_excluded_urls: list[str] = dataclasses.field(default_factory=list) prometheus_middleware_cls: type[FastStreamPrometheusMiddlewareProtocol] | None = None prometheus_collector_registry: "prometheus_client.CollectorRegistry | None" = None faststream_log_level: int = logging.WARNING diff --git a/lite_bootstrap/bootstrappers/litestar_bootstrapper.py b/lite_bootstrap/bootstrappers/litestar_bootstrapper.py index a0edc33..49cebb2 100644 --- a/lite_bootstrap/bootstrappers/litestar_bootstrapper.py +++ b/lite_bootstrap/bootstrappers/litestar_bootstrapper.py @@ -117,7 +117,6 @@ class LitestarConfig( SwaggerConfig, ): application_config: "AppConfig" = dataclasses.field(default_factory=lambda: AppConfig()) # noqa: PLW0108 - opentelemetry_excluded_urls: list[str] = dataclasses.field(default_factory=list) prometheus_additional_params: dict[str, typing.Any] = dataclasses.field(default_factory=dict) swagger_extra_params: dict[str, typing.Any] = dataclasses.field(default_factory=dict) diff --git a/lite_bootstrap/instruments/opentelemetry_instrument.py b/lite_bootstrap/instruments/opentelemetry_instrument.py index a82e025..e4071f4 100644 --- a/lite_bootstrap/instruments/opentelemetry_instrument.py +++ b/lite_bootstrap/instruments/opentelemetry_instrument.py @@ -56,6 +56,7 @@ class OpenTelemetryConfig(OpenTelemetryServiceFieldsConfig): ) opentelemetry_log_traces: bool = False opentelemetry_generate_health_check_spans: bool = True + opentelemetry_excluded_urls: list[str] = dataclasses.field(default_factory=list) def __post_init__(self) -> None: host = self._parse_remote_insecure_host() @@ -142,7 +143,7 @@ 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", [])) + excluded_urls: set[str] = set(self.bootstrap_config.opentelemetry_excluded_urls) prometheus_path = getattr(self.bootstrap_config, "prometheus_metrics_path", None) if prometheus_path: excluded_urls.add(prometheus_path) diff --git a/planning/changes/2026-06-24.02-otel-excluded-urls-home/design.md b/planning/changes/2026-06-24.02-otel-excluded-urls-home/design.md new file mode 100644 index 0000000..7f03cac --- /dev/null +++ b/planning/changes/2026-06-24.02-otel-excluded-urls-home/design.md @@ -0,0 +1,144 @@ +--- +status: shipped +date: 2026-06-24 +slug: otel-excluded-urls-home +summary: Move OTel's own opentelemetry_excluded_urls field onto OpenTelemetryConfig (typed access, one declaration) and pin the genuine prometheus/health cross-config exclusion reads with a regression test. +supersedes: null +superseded_by: null +pr: 132 +outcome: Shipped as designed — opentelemetry_excluded_urls moved to OpenTelemetryConfig (3 declarations → 1, typed access), sibling prometheus/health reads kept and pinned by a new test, contribution-mechanism refactor rejected; 100% coverage held. +--- + +# Design: Move `opentelemetry_excluded_urls` onto its own config; pin the sibling-path exclusions + +## Summary + +`OpenTelemetryInstrument._build_excluded_urls` reads four config fields by string via +`getattr(..., default)`. One of them — `opentelemetry_excluded_urls` — is OTel's *own* +setting, yet it is declared three times on the framework configs and never on +`OpenTelemetryConfig`, which is *why* OTel must getattr it. This change moves that field +home to `OpenTelemetryConfig` (de-duplicating three declarations into one and making the +read typed), and adds a regression test pinning the *genuine* cross-instrument reads +(`prometheus_metrics_path` / `health_checks_path`) so a future rename breaks loudly +instead of silently dropping URLs from the trace-exclusion set. + +## Motivation + +`_build_excluded_urls` (`instruments/opentelemetry_instrument.py:144-153`) makes four +stringly-typed cross-config reads: + +```python +excluded_urls = set(getattr(self.bootstrap_config, "opentelemetry_excluded_urls", [])) # OTel's OWN field +prometheus_path = getattr(self.bootstrap_config, "prometheus_metrics_path", None) # PrometheusConfig +if not self.bootstrap_config.opentelemetry_generate_health_check_spans: + health_path = getattr(self.bootstrap_config, "health_checks_path", None) # HealthChecksConfig +``` + +These are two different kinds of read, and conflating them hides the real issue: + +- **`opentelemetry_excluded_urls` is OTel's own field, misplaced.** It is declared on + `FastAPIConfig:53`, `LitestarConfig:120`, and `FastStreamConfig:69` — three identical + copies — and not on `OpenTelemetryConfig`. The getattr exists only because the field + isn't where it belongs. The method is defined on the base `OpenTelemetryInstrument`, + whose `bootstrap_config` is typed `OpenTelemetryConfig`; if the field lived there, the + read would be typed. +- **`prometheus_metrics_path` / `health_checks_path` are genuinely other instruments' + fields.** OTel runs in `FreeConfig`, which composes neither Prometheus nor + HealthChecks, so these fields may legitimately be absent. The defensive + `getattr(..., None)` is the correct expression of "optional sibling" and should stay. + +The risk in the genuine-sibling reads is silent breakage: rename `prometheus_metrics_path` +on `PrometheusConfig` and the getattr returns `None`, the metrics endpoint silently stops +being excluded from traces, and **no test catches it** — the only direct +`_build_excluded_urls` test (`test_faststream_bootstrap.py:225`) covers just the +`opentelemetry_excluded_urls` passthrough. Same shape as the candidate-1 drift bug, lower +impact (extra spans, not lost data). (Surfaced as candidate 4 of the 2026-06-23 +architecture review; rated Speculative — this is the proportionate slice of it.) + +## Non-goals + +- **A "contribution" mechanism** (each instrument/config declares its excluded paths, + OTel unions them). Rejected: today `_build_excluded_urls` concentrates the entire + exclusion policy in one readable method (good locality); a contribution mechanism would + *spread* that policy across `PrometheusConfig`, `HealthChecksConfig`, and OTel, and the + health-check case still needs OTel's `opentelemetry_generate_health_check_spans` flag, + so the cross-coupling would not even disappear. It moves complexity and worsens + locality — fails the deletion test. +- **Removing the genuine-sibling getattrs.** `prometheus_metrics_path` / + `health_checks_path` / `pyroscope_endpoint` belong to other instruments and are + optional; the defensive read is correct. We pin them with a test, not a refactor. +- **Touching the `pyroscope_endpoint` getattr (line 174).** It is a span-processor + decision, not part of `_build_excluded_urls`; same intentional optional-sibling + pattern, different concern. + +## Design + +### 1. Move `opentelemetry_excluded_urls` onto `OpenTelemetryConfig` + +Add the field to `OpenTelemetryConfig` and delete the three framework-config copies: + +```python +# instruments/opentelemetry_instrument.py — OpenTelemetryConfig +opentelemetry_excluded_urls: list[str] = dataclasses.field(default_factory=list) +``` + +Users still set it exactly as before (`FastAPIConfig(opentelemetry_excluded_urls=[...])`) +— it is inherited. The read in `_build_excluded_urls` becomes typed: + +```python +excluded_urls: set[str] = set(self.bootstrap_config.opentelemetry_excluded_urls) +``` + +The `prometheus_metrics_path` / `health_checks_path` reads stay as `getattr(..., default)`. + +### 2. Accepted trade: the field is now inherited by `FreeConfig` + +`FreeConfig` composes `OpenTelemetryConfig`, so it gains `opentelemetry_excluded_urls` +(inert there — `_build_excluded_urls` is only called by the HTTP framework OTel +subclasses, never in Free's path). `FastMcpConfig` is unaffected (it composes no OTel +config). `FreeConfig.from_dict({"opentelemetry_excluded_urls": ...})` now accepts the key +instead of filtering it. This cosmetic widening is accepted: Free already inherits other +OTel fields that only matter in some setups, and the field belongs on `OpenTelemetryConfig`. + +### 3. Pin the genuine-sibling exclusions with one regression test + +One representative test (the method is a single shared base method; the fields live on the +shared `PrometheusConfig` / `HealthChecksConfig`, so a rename breaks every framework at +once — one test suffices). Placed beside the existing `_build_excluded_urls` test in +`test_faststream_bootstrap.py`. It asserts the full policy: + +- the prometheus metrics path is **always** in the excluded set; +- the health-checks path **is** excluded when `opentelemetry_generate_health_check_spans=False`; +- the health-checks path **is not** excluded when it is `True`. + +A rename of `prometheus_metrics_path` / `health_checks_path`, or a regression in the +conditional, then fails this test loudly. + +## Operations + +None. + +## Out of scope + +Covered under Non-goals. + +## Testing + +- **Existing guard (A):** `test_faststream_opentelemetry_excluded_urls_in_built_set` + (`test_faststream_bootstrap.py:225`) already pins the `opentelemetry_excluded_urls` + passthrough and keeps passing after the field moves (it is inherited), so A is a + refactor under a stable test. +- **New regression (B):** the cross-config exclusion test above. + +`just test` green at 100%, `just lint-ci` clean (`ty` included). + +## Risk + +- **Behaviour change from the field move (low likelihood × low impact).** Moving the + field is additive — every current call site keeps working via inheritance; the only + observable change is `FreeConfig` accepting a previously-rejected key. Mitigation: the + existing passthrough test guards the read. +- **`from_dict` semantics on Free (low × low).** Free now accepts `opentelemetry_excluded_urls` + in `from_dict`/`from_object`. Accepted per design §2. +- **Residual silent-rename on `pyroscope_endpoint` (low × low).** Out of scope; it is not + part of `_build_excluded_urls`. Noted so a future pass knows it was deliberately left. diff --git a/planning/changes/2026-06-24.02-otel-excluded-urls-home/plan.md b/planning/changes/2026-06-24.02-otel-excluded-urls-home/plan.md new file mode 100644 index 0000000..071530e --- /dev/null +++ b/planning/changes/2026-06-24.02-otel-excluded-urls-home/plan.md @@ -0,0 +1,137 @@ +--- +status: shipped +date: 2026-06-24 +slug: otel-excluded-urls-home +spec: otel-excluded-urls-home +pr: 132 +--- + +# otel-excluded-urls-home — implementation plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use +> superpowers:subagent-driven-development (recommended) or +> superpowers:executing-plans to implement this plan task-by-task. Steps +> use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Move `opentelemetry_excluded_urls` onto `OpenTelemetryConfig` (typed +access, one declaration), and pin the genuine cross-instrument exclusion reads +(`prometheus_metrics_path` / `health_checks_path`) with a regression test. + +**Spec:** [`design.md`](./design.md) + +**Branch:** `feat/otel-excluded-urls-home` + +**Commit strategy:** Per-task commits (each task leaves the suite green). + +--- + +### Task 1: Pin the genuine-sibling exclusions (B) — regression net first + +**Files:** +- Modify: `tests/test_faststream_bootstrap.py` + +Lock the cross-config exclusion behaviour that is currently untested, before +touching any config. This test passes against the current code (it characterises +existing behaviour) and guards the policy thereafter. + +- [ ] **Step 1: Add the cross-config exclusion test.** + + Beside `test_faststream_opentelemetry_excluded_urls_in_built_set` + (`test_faststream_bootstrap.py:225`), add a test that builds a config composing + Prometheus + HealthChecks (via `build_faststream_config` + `dataclasses.replace`), + constructs `FastStreamOpenTelemetryInstrument`, and asserts on `_build_excluded_urls()`: + - prometheus metrics path is always in the set; + - health-checks path is in the set when `opentelemetry_generate_health_check_spans=False`; + - health-checks path is **not** in the set when it is `True`. + + Run `just test -- -k excluded` → green (behaviour already exists). + +- [ ] **Step 2: Commit.** + + ```bash + git add tests/test_faststream_bootstrap.py + git commit -m "test: pin OTel cross-config metrics/health URL exclusion + + Co-Authored-By: Claude Opus 4.8 (1M context) " + ``` + +--- + +### Task 2: Move `opentelemetry_excluded_urls` onto `OpenTelemetryConfig` (A) + +**Files:** +- Modify: `lite_bootstrap/instruments/opentelemetry_instrument.py` +- Modify: `lite_bootstrap/bootstrappers/fastapi_bootstrapper.py` +- Modify: `lite_bootstrap/bootstrappers/litestar_bootstrapper.py` +- Modify: `lite_bootstrap/bootstrappers/faststream_bootstrapper.py` + +Field goes home; the read becomes typed. Guarded by the existing passthrough test +(`test_faststream_opentelemetry_excluded_urls_in_built_set`) and Task 1's net. + +- [ ] **Step 1: Add the field to `OpenTelemetryConfig`.** + + `opentelemetry_excluded_urls: list[str] = dataclasses.field(default_factory=list)` + on `OpenTelemetryConfig` (the main config, alongside the other `opentelemetry_*` + fields). + +- [ ] **Step 2: Remove the three framework-config declarations.** + + Delete the `opentelemetry_excluded_urls` line from `FastAPIConfig` (`:53`), + `LitestarConfig` (`:120`), `FastStreamConfig` (`:69`). They now inherit it. + +- [ ] **Step 3: Make the read typed.** + + In `_build_excluded_urls`, replace + `set(getattr(self.bootstrap_config, "opentelemetry_excluded_urls", []))` with + `set(self.bootstrap_config.opentelemetry_excluded_urls)`. Leave the + `prometheus_metrics_path` / `health_checks_path` getattrs unchanged. + + Run `just test` (full suite) → green. Confirm + `test_faststream_opentelemetry_excluded_urls_in_built_set` and Task 1's test pass. + +- [ ] **Step 4: Commit.** + + ```bash + git add lite_bootstrap/instruments/opentelemetry_instrument.py \ + lite_bootstrap/bootstrappers/fastapi_bootstrapper.py \ + lite_bootstrap/bootstrappers/litestar_bootstrapper.py \ + lite_bootstrap/bootstrappers/faststream_bootstrapper.py + git commit -m "refactor: move opentelemetry_excluded_urls onto OpenTelemetryConfig + + Co-Authored-By: Claude Opus 4.8 (1M context) " + ``` + +--- + +### Task 3: Docs + frontmatter + index + +**Files:** +- Modify: `docs/introduction/configuration.md` +- Modify: `planning/changes/2026-06-24.02-otel-excluded-urls-home/design.md` (frontmatter) + +- [ ] **Step 1: Update `docs/introduction/configuration.md`.** + + Read the `opentelemetry_excluded_urls` reference; if it is documented under a + framework-specific section, move/note it as an `OpenTelemetryConfig` field (it now + applies wherever OTel is configured). Keep it minimal — usage is unchanged. + +- [ ] **Step 2: Fill design frontmatter + regenerate index.** + + Set `summary`; set `status` per merge state (candidate-1/2 precedent: `approved` + until a PR exists, `shipped` + `pr` at PR time). Run `just index`. + +- [ ] **Step 3: Final verification.** + + `just lint-ci` clean (incl. `ty`), `just test` green at 100%. Spot-check: + `FreeConfig(opentelemetry_excluded_urls=["/x"])` constructs without error (field + now inherited). + +- [ ] **Step 4: Commit.** + + ```bash + git add docs/introduction/configuration.md \ + planning/changes/2026-06-24.02-otel-excluded-urls-home/ planning/ + git commit -m "docs: note opentelemetry_excluded_urls moved to OpenTelemetryConfig + + Co-Authored-By: Claude Opus 4.8 (1M context) " + ``` diff --git a/tests/test_faststream_bootstrap.py b/tests/test_faststream_bootstrap.py index 22d9a41..29cb29b 100644 --- a/tests/test_faststream_bootstrap.py +++ b/tests/test_faststream_bootstrap.py @@ -233,6 +233,24 @@ def test_faststream_opentelemetry_excluded_urls_in_built_set(broker: RedisBroker assert "/bar" in excluded +def test_faststream_build_excluded_urls_covers_prometheus_and_health_paths(broker: RedisBroker) -> None: + # Pins the genuine cross-instrument reads in _build_excluded_urls: a rename of + # prometheus_metrics_path / health_checks_path (or a regression in the health + # conditional) breaks this loudly instead of silently dropping the exclusion. + config_no_health_spans = dataclasses.replace( + build_faststream_config(broker=broker), + opentelemetry_generate_health_check_spans=False, + ) + excluded = FastStreamOpenTelemetryInstrument(bootstrap_config=config_no_health_spans)._build_excluded_urls() # noqa: SLF001 + assert config_no_health_spans.prometheus_metrics_path in excluded # always excluded + assert config_no_health_spans.health_checks_path in excluded # excluded when health spans are off + + config_with_health_spans = build_faststream_config(broker=broker) # generate_health_check_spans defaults True + excluded_with = FastStreamOpenTelemetryInstrument(bootstrap_config=config_with_health_spans)._build_excluded_urls() # noqa: SLF001 + assert config_with_health_spans.prometheus_metrics_path in excluded_with + assert config_with_health_spans.health_checks_path not in excluded_with # kept when health spans are on + + async def test_faststream_prometheus_uses_injected_registry(broker: RedisBroker) -> None: custom_registry = prometheus_client.CollectorRegistry() counter_name = f"injected_counter_{uuid.uuid4().hex}_total"