diff --git a/AGENTS.md b/AGENTS.md index 8190a88..2e8a7db 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -24,11 +24,19 @@ Every link in `README.md` must be absolute: `https://github.com/modern-python/` for a directory. Never a relative path: `README.md` is also the PyPI long description, and PyPI does not rewrite relative links, so a relative one 404s on the package page. -## Agent docs +## Agent skills -- Issue tracker: GitHub issues on `modern-python/lite-bootstrap` via `gh`. `docs/agents/issue-tracker.md`. -- Triage labels: five canonical roles, each label string equal to its name. `docs/agents/triage-labels.md`. -- Domain docs: single-context, `CONTEXT.md` and `docs/adr/` at the repo root. `docs/agents/domain.md`. +### Issue tracker + +GitHub issues on `modern-python/lite-bootstrap`, via `gh`. See `docs/agents/issue-tracker.md`. + +### Triage labels + +The five canonical roles, each label string equal to its name. See `docs/agents/triage-labels.md`. + +### Domain docs + +Single-context: `CONTEXT.md` and `docs/adr/` at the repo root. See `docs/agents/domain.md`. ## Code style diff --git a/docs/adr/0001-fastmcp-teardown-via-provider-lifespan.md b/docs/adr/0001-fastmcp-teardown-via-provider-lifespan.md index 051b001..a889a83 100644 --- a/docs/adr/0001-fastmcp-teardown-via-provider-lifespan.md +++ b/docs/adr/0001-fastmcp-teardown-via-provider-lifespan.md @@ -1,30 +1,15 @@ # FastMCP teardown attaches through a Provider lifespan -**Decision:** `FastMcpBootstrapper` wires its `teardown` into FastMCP's shutdown by registering a -`_TeardownProvider` via the public `FastMCP.add_provider()`, whose `async def lifespan(self)` runs -teardown on the exit branch. We will not reach into `FastMCP._lifespan`, rebuild the user's -`FastMCP`, or leave teardown manual. - -FastMCP is the one supported framework with no `on_shutdown`-shaped hook. `FastMCP.lifespan` is a -bound method on the `AggregateProvider` mixin, not a settable attribute: assigning `app.lifespan` -succeeds and has zero runtime effect, because the transport runners read the private `_lifespan` -attribute that is only set at construction time. `add_provider()` is the sole public, -post-construction hook whose callback is invoked by the server's ASGI lifespan. - -Rejected, with the reasoning that would otherwise be re-litigated: - -- **Mutate `app._lifespan` directly.** Works today, but it is private API on a fast-moving - dependency, and a rename ships as a silent no-teardown rather than an error. -- **Rebuild the user's `FastMCP` with a composed `lifespan=`.** Breaks the contract every other - bootstrapper keeps — the user owns the application object they passed in, and gets the same - object back. -- **No automatic wiring; document a manual `teardown()` call.** Adopted briefly and reverted once - `add_provider` was found. It makes FastMCP the only framework where shutdown is the user's job. - -The accepted cost is semantic: `Provider` is FastMCP's general extension abstraction for tools, -resources and prompts, and using one purely for a shutdown callback is thin. A one-line comment at -the registration site says so. - -**Revisit trigger:** FastMCP grows a first-class shutdown hook (an `on_shutdown` API, or a -documented public way to compose a lifespan post-construction). At that point the provider is the -indirect route and should be replaced by the direct one. +FastMCP is the one supported framework with no `on_shutdown`-shaped hook: `FastMCP.lifespan` is a +bound method on the `AggregateProvider` mixin, so assigning to it succeeds and does nothing — the +transport runners read the private `_lifespan`, set only at construction — which leaves +`add_provider()` as the single public post-construction hook the server's ASGI lifespan actually +invokes. `FastMcpBootstrapper` therefore registers a `_TeardownProvider` whose `lifespan` runs +teardown on the exit branch, accepting that `Provider` is FastMCP's extension abstraction for tools, +resources and prompts and that using one for a shutdown callback is thin. + +Rejected: mutating `app._lifespan` (private API on a fast-moving dependency, where a rename ships as +silent no-teardown rather than an error); rebuilding the user's `FastMCP` with a composed `lifespan=` +(every other bootstrapper hands back the object it was given); and documenting a manual `teardown()` +call (adopted briefly, reverted once `add_provider` was found). If FastMCP grows a real shutdown +hook, the provider becomes the indirect route and should be replaced. diff --git a/docs/adr/0002-cross-cutting-logic-stays-put.md b/docs/adr/0002-cross-cutting-logic-stays-put.md new file mode 100644 index 0000000..e92a502 --- /dev/null +++ b/docs/adr/0002-cross-cutting-logic-stays-put.md @@ -0,0 +1,23 @@ +# Cross-cutting logic stays where it is: per-instrument, and in one method + +Two re-organizations have been proposed and rejected for the same reason — each moves complexity +rather than concentrating it, and the matrix is inherently O(instruments × frameworks) whichever axis +it is cut along. + +The instrument × framework matrix keeps its **per-instrument axis**: base instruments own the hoisted +logic and each framework is a thin `bootstrap()` subclass, rather than per-framework adapters driving +generic instruments. The bindings genuinely differ — FastAPI imperative (`add_middleware`, +`include_router`), Litestar declarative before the app is built, FastStream attaching to a broker, +FastMCP via `custom_route` — so a uniform `add_route`/`add_middleware` interface would leak; and +framework-locality already exists, because all of a framework's subclasses live in its one +bootstrapper file. Reconsider if the per-cell bindings start converging, or if a new framework shares +an existing one's attach mechanism exactly. + +`_build_excluded_urls` likewise keeps the whole trace-exclusion policy in **one method** that reads +its siblings' paths through `getattr(..., None)` — those sibling configs are genuinely optional, so +the defensive read is correct rather than a smell. A per-instrument contribution mechanism would +spread the policy across `PrometheusConfig`, `HealthChecksConfig` and OpenTelemetry while still +needing `opentelemetry_generate_health_check_spans`, so the coupling would not even disappear. The +real risk — renaming `prometheus_metrics_path` and silently stopping the exclusion — is answered by a +test pinning each sibling path in the built set. Reconsider when a user-supplied instrument needs +paths the policy cannot know about. diff --git a/docs/adr/0002-keep-per-instrument-axis.md b/docs/adr/0002-keep-per-instrument-axis.md deleted file mode 100644 index 8813a4d..0000000 --- a/docs/adr/0002-keep-per-instrument-axis.md +++ /dev/null @@ -1,49 +0,0 @@ -# Keep the per-instrument axis for the instrument × framework matrix - -**Decision:** The framework-binding code stays organized around the *instrument* — base instrument -classes own the shared logic, and each framework is a thin subclass overriding only its `bootstrap()` -binding. We reject inverting to a per-framework adapter axis (a -`FastAPIAdapter`/`LitestarAdapter`/… that knows how to attach any instrument, driven by generic -instruments). - -## Context - -The codebase has an instrument × framework matrix: every filled cell is "how instrument *I* binds to -framework *F*" (e.g. `FastAPIHealthChecksInstrument`, `LitestarPrometheusInstrument`). The -2026-06-23 architecture review raised the matrix as candidate 3 — "no framework-locality; ~28 shallow -per-cell subclasses" — and proposed inverting the axis so a per-framework adapter owns the binding -and instruments become generic. - -## Decision & rationale - -The candidate's premise and payoff do not hold up: - -- **Framework-locality already exists at the file level.** All of a framework's instrument - subclasses live in its one bootstrapper file, so "what does lite-bootstrap do to my FastAPI app" - is already answered by reading one file. The review's "five scattered classes" are co-located, - not scattered. -- **The shared depth is already hoisted.** `render_health_check_data()` lives in the base - `HealthChecksInstrument`; provider setup in base `OpenTelemetryInstrument`; config validation in - the base configs. The per-framework subclasses contain *only* the genuinely-different binding, - which is what you want — the thinness is the result of correct hoisting, not shallowness to fix. -- **The N×M bindings differ genuinely.** FastAPI is imperative (`app.add_middleware`, - `include_router`), Litestar is declarative *before the app is built* (`application_config.cors_config = …`, - append to `middleware`/`route_handlers`), FastStream attaches middleware to a *broker* not the - app, FastMCP uses `custom_route`. A uniform adapter interface (`add_route`/`add_middleware`) would - have to paper over imperative-vs-declarative-vs-broker and normalize differing handler return - types — it would leak. -- **Deletion test fails for the inversion.** Inverting relocates the same N×M genuinely-different - bindings into framework-grouped adapters; the complexity *moves*, it does not *concentrate*. The - matrix is inherently O(instruments × frameworks); no axis choice removes a cell. Adding an - instrument touches every framework either way; adding a framework touches every instrument either - way. - -No friction worth a refactor was identified: navigation is satisfied by the file layout, the -cross-cutting change cost is inherent to the matrix, and the per-cell "shallowness" is hoisted-out -depth rather than duplication. - -**Revisit trigger:** the per-cell bindings start genuinely converging — the shared part outgrows the -base instrument and the same binding code appears across framework subclasses; then hoist the -convergent part and reconsider an adapter for that specific shared mechanism. Or a new framework -arrives that shares an existing framework's attach mechanism (another ASGI app driven exactly like -FastAPI), turning the hypothetical seam into a real one for that pair. diff --git a/docs/adr/0003-teardown-marker-accepted-limits.md b/docs/adr/0003-teardown-marker-accepted-limits.md index e3d374c..05cedf2 100644 --- a/docs/adr/0003-teardown-marker-accepted-limits.md +++ b/docs/adr/0003-teardown-marker-accepted-limits.md @@ -1,52 +1,20 @@ # The teardown-attach guard is an attribute marker, and its two limits are accepted -**Decision:** `BaseBootstrapper._attach_teardown_once` detects a second bootstrapper by tagging the -attach target with the `_lite_bootstrap_teardown_attached` attribute (#130). Two consequences of -that choice — FastMCP detecting via the attribute rather than a provider-list scan, and Litestar -tagging the shared `AppConfig` rather than a built app — are accepted rather than designed around. - -## Why an attribute on the target - -Rejected: **a class-level registry or `WeakSet` of already-attached applications.** It would -contradict the `_lite_bootstrap_*` app-tagging convention the codebase already follows, and it -introduces process-global mutable state with the test-isolation hazards that come with it. The -marker keeps the "attached" bit local to the application's own lifetime, which is exactly the -lifetime the fact is true for. - -Rejected: **a free-function helper module.** The guard is bootstrapper-lifecycle logic and belongs on -`BaseBootstrapper` next to `teardown()`, where `type(self).__name__` is available for the warning. - -## The two accepted limits - -1. **FastMCP detects via the marker, not structurally.** FastMCP previously detected double-attach - by scanning `any(isinstance(p, _TeardownProvider) for p in app.providers)`, which reads the actual - attach state. If the providers list is cleared after the first bootstrap while the marker - survives, the second bootstrapper is refused rather than re-attaching. Accepted: uniformity across - all four app-bearing bootstrappers is worth more than the sliver of state-accuracy the scan gave, - the only scenario the guard exists for is detected identically either way, and the regression - requires user or framework code to mutate `app.providers` after bootstrap, which no supported flow - does. Keeping FastMCP on a bespoke structural check would re-fragment detection and defeat the - seam's whole point. - -2. **Litestar tags the `AppConfig`.** The Litestar app does not exist at `__init__` time — it is - built later by `Litestar.from_config()` — so the attach, and therefore the marker, lands on - `application_config`. Two `LitestarConfig` instances that *share* one `AppConfig` but intend two - distinct apps will collide. Accepted: config-level is the only option while the app is built - lazily, and sharing one mutable `AppConfig` across two intended apps is already broken - independently of teardown — instrument bootstrap mutates the shared config's `cors_config`, - `route_handlers` and `openapi_config`. The marker collision is one symptom of an - already-unsupported pattern, not a new hazard. - -The genuinely actionable finding from the same review — the marker being set before a fallible -`attach()` — was *fixed*, not accepted: the target is tagged only after `attach()` returns. - -Since #167 the consequence of hitting the marker is louder in both cases: the losing bootstrapper's -`bootstrap()` raises `ConfigurationError` before any instrument is applied, rather than warning and -leaving a half-wired application whose teardown never runs. The marker and these two limits are -unchanged. - -**Revisit trigger:** for FastMCP, a supported flow starts mutating `FastMCP.providers` after -bootstrap (a documented hot-reload or provider-swap API), making the attribute diverge from real -attach state — then move back to structural detection or reconcile the two. For Litestar, sharing one -`AppConfig` across multiple apps becomes a supported pattern, or the attach is restructured to run at -`bootstrap()` time when the app exists — then tag the built `Litestar` app instead. +`BaseBootstrapper._attach_teardown_once` detects a second bootstrapper by tagging the attach target +with `_lite_bootstrap_teardown_attached`, so the "attached" bit lives exactly as long as the +application it describes — rather than in a class-level registry or `WeakSet`, which would mean +process-global mutable state and would contradict the `_lite_bootstrap_*` tagging convention the +codebase already follows. The target is tagged only after `attach()` returns, and since #167 hitting +the marker raises `ConfigurationError` from `bootstrap()` rather than warning and leaving a half-wired +application. + +Two consequences are accepted rather than designed around: + +- **FastMCP detects via the marker, not by scanning `app.providers`.** If that list is cleared after + the first bootstrap, the second bootstrapper is refused instead of re-attaching. No supported flow + mutates it, and uniformity across the four app-bearing bootstrappers is worth more than the sliver + of state-accuracy the structural check gave. +- **Litestar tags the shared `AppConfig`**, because the app does not exist until + `Litestar.from_config()`. Two configs sharing one `AppConfig` collide — but that pattern is already + broken independently, since instrument bootstrap mutates the shared config's `cors_config`, + `route_handlers` and `openapi_config`. diff --git a/docs/adr/0004-excluded-urls-stay-one-method.md b/docs/adr/0004-excluded-urls-stay-one-method.md deleted file mode 100644 index 282f917..0000000 --- a/docs/adr/0004-excluded-urls-stay-one-method.md +++ /dev/null @@ -1,25 +0,0 @@ -# Trace-exclusion policy stays in one method, not contributed per instrument - -**Decision:** `_build_excluded_urls` keeps the whole OpenTelemetry URL-exclusion policy in one -method that reads its sibling instruments' paths. We reject a contribution mechanism in which each -instrument or config declares the paths it wants excluded and OpenTelemetry unions them (#132). - -The method reads `prometheus_metrics_path` and `health_checks_path` off the config through defensive -`getattr(..., None)`, because a given framework config need not compose `PrometheusConfig` or -`HealthChecksConfig` at all — those are genuinely optional siblings, and the defensive read is the -correct expression of that, not a smell to refactor away. - -A contribution mechanism was the obvious alternative and fails the deletion test. Today the entire -exclusion policy is readable in one place; contributing would *spread* it across `PrometheusConfig`, -`HealthChecksConfig` and OpenTelemetry, and the health-check case would still need OpenTelemetry's -own `opentelemetry_generate_health_check_spans` flag, so the cross-coupling would not even -disappear. It moves complexity and worsens locality. - -The real risk in the sibling reads is silent breakage: rename `prometheus_metrics_path` and the -`getattr` returns `None`, the metrics endpoint quietly stops being excluded from traces, and nothing -fails. That is answered with a test that pins each sibling path in the built set, not with a -refactor. - -**Revisit trigger:** a third-party or user-supplied instrument needs its own paths excluded. The -policy would then have to name paths it cannot know about, which is precisely the case a contribution -mechanism exists for. diff --git a/docs/adr/0004-native-dependency-extras-are-opt-in.md b/docs/adr/0004-native-dependency-extras-are-opt-in.md new file mode 100644 index 0000000..66b40e1 --- /dev/null +++ b/docs/adr/0004-native-dependency-extras-are-opt-in.md @@ -0,0 +1,20 @@ +# Native-dependency extras are opt-in, and never per-framework variants + +Free-threaded CPython has wheels for neither `orjson` nor `grpcio`, and PEP 508 has no environment +marker for "GIL enabled", so neither can be required conditionally. Both are therefore opt-in: +`orjson` is its own extra (`logging` is `structlog` only) with the logging serializer falling back to +the stdlib `json` accelerator, and OTLP over HTTP is a sibling `otl-http` extra beside a gRPC-only +`otl`. The fallback is a documented performance change, not a correctness one — roughly 2-5x slower, +with non-JSON-native values in log `extra` rendering via `repr`; the GIL fast path is byte-for-byte +unchanged when `[orjson]` is installed. + +Both refuse the combinatorial alternative: `*-ft` twins of every logging-bearing and `*-all` extra, +or `fastapi-otl-http`-style framework variants. A free-threaded service composes `[fastapi, otl-http]` +itself. Replacing `orjson` with msgspec, ujson or rapidjson was also rejected — a permanent mandatory +dependency, with a different encoder API and output shape, to paper over a temporary gap +([ijl/orjson#530](https://github.com/ijl/orjson/issues/530) tracks ft wheels); if that resolves, +`orjson` could return to core and the fallback branch retire. + +The HTTP exporter deliberately carries no insecure-endpoint warning mirroring the gRPC one: it has no +`insecure` flag to inspect, only a full URL whose scheme is a stronger signal than anything +`__post_init__` could re-derive, and the user typed it explicitly. diff --git a/docs/adr/0005-core-declares-typing-extensions.md b/docs/adr/0005-core-declares-typing-extensions.md new file mode 100644 index 0000000..3429c66 --- /dev/null +++ b/docs/adr/0005-core-declares-typing-extensions.md @@ -0,0 +1,12 @@ +# Core declares `typing-extensions` + +A genuinely zero-dependency core was implemented, tested and abandoned. `HealthCheckTypedDict` is a +FastAPI response model, and pydantic refuses a stdlib `typing.TypedDict` model on Python < 3.12 +(`PydanticUserError`) — a constraint that lives in pydantic's behaviour on an old interpreter, not in +a `TypedDict` you can build in isolation, so the attempt passed locally on 3.12 and failed the CI +matrix on 3.10 and 3.11. `1.3.0` had already shipped claiming a zero-dependency core and raised +`ModuleNotFoundError` on a bare install, masked everywhere because every extra pulls +`typing_extensions` transitively. + +It is pure Python, so core stays free-threading-friendly, and the dependency can be dropped once the +floor reaches 3.12 — where `typing.TypedDict` is acceptable to pydantic and `Self` is in `typing`. diff --git a/docs/adr/0005-orjson-is-opt-in.md b/docs/adr/0005-orjson-is-opt-in.md deleted file mode 100644 index cf06369..0000000 --- a/docs/adr/0005-orjson-is-opt-in.md +++ /dev/null @@ -1,36 +0,0 @@ -# `orjson` is an opt-in extra with a stdlib-`json` fallback - -**Decision:** `orjson` is its own extra (`lite-bootstrap[orjson]`), the `logging` extra is -`structlog` only, and the logging serializer falls back to the stdlib `json` accelerator when -`orjson` is absent. It is not bundled with `logging`/`*-all`, and it is not replaced by a -free-threading-ready native encoder. - -## Context - -`orjson` used to be a mandatory core dependency, used only by the logging serializer, and it -hard-blocks free-threaded installs: no ft wheels, and the build refuses to compile on ft (verified -on 3.14t). PEP 508 has **no environment marker for "GIL enabled"**, so `orjson` cannot be required -conditionally on GIL builds only. That missing marker is what forces the choice. - -## Rejected alternatives - -**Keep `orjson` reachable by default and add parallel `*-ft` extras that omit it.** Zero behaviour -change for GIL users, at the cost of an ft twin for every logging-bearing and `*-all` extra — -`fastapi-logging-ft`, `free-all-ft`, and so on. That is exactly the combinatorial extras sprawl this -project refuses; the same argument later rejected `*-otl-http` framework variants (ADR-0006). - -**Replace `orjson` with an ft-ready native encoder — msgspec, ujson, rapidjson.** A permanent new -mandatory dependency to paper over a *temporary* gap ([ijl/orjson#530](https://github.com/ijl/orjson/issues/530) -tracks ft wheels). msgspec's encoder API differs (`enc_hook`, not orjson's `default=`) and so does -its output shape, forcing a serializer rewrite and a test re-baseline — for a dependency that would -outlive the problem. - -The chosen shape keeps one coherent rule: `orjson` is a per-build opt-in speedup, and no extra drags -it in. It also fixed a standing hygiene defect — a JSON encoder had no business being a mandatory -core dependency. The GIL fast path is byte-for-byte unchanged when `[orjson]` is present; the cost -is a documented, opt-in performance change (stdlib `json`, roughly 2-5x slower, same correctness, -and non-JSON-native values in log `extra` render via `repr` rather than orjson's native encoding). - -**Revisit trigger:** `orjson` ships free-threaded wheels (#530 resolves). At that point it could -return to `logging`/core as a hard dependency and the fallback branch retire — reopen then to decide -whether that simplification is worth removing the opt-in extra. diff --git a/docs/adr/0006-otlp-http-exporter-shape.md b/docs/adr/0006-otlp-http-exporter-shape.md deleted file mode 100644 index fd62561..0000000 --- a/docs/adr/0006-otlp-http-exporter-shape.md +++ /dev/null @@ -1,33 +0,0 @@ -# OTLP over HTTP is a sibling extra, and HTTP carries no insecure warning - -**Decision:** `otl` points at `opentelemetry-exporter-otlp-proto-grpc`; a sibling `otl-http` carries -`opentelemetry-exporter-otlp-proto-http` (no `grpcio`). There are no framework `*-otl-http` variants. -The `__post_init__` insecure-endpoint warning stays tied to the gRPC `insecure` flag; for HTTP the -endpoint URL's scheme is the security signal, and it is documented rather than warned about. - -## Context - -`otl` used to pull `opentelemetry-exporter-otlp`, a meta package that drags in the gRPC exporter and -therefore `grpcio`. `grpcio` has no free-threaded wheels, so OTLP export on ft needs the HTTP -exporter to be installable on its own. - -## Rejected alternatives - -**Keep `otl` as the meta package and layer `otl-http` on top.** Leaves `otl` `grpcio`-bound and -redundant, since the meta package already ships the HTTP exporter. Repointing `otl` at the gRPC -exporter package is functionally identical for existing users — the exporter already defaulted to -gRPC and the HTTP package was never used — and drops only an unused transitive dependency. - -**Framework `*-otl-http` variants (`fastapi-otl-http`, `litestar-otl-http`, …).** The same -combinatorial sprawl rejected for `orjson` in ADR-0005. A free-threaded framework service composes -`[fastapi, otl-http]` and adds its own instrumentation package directly. - -**An `http://`-non-local warning mirroring the gRPC one.** The gRPC exporter has an `insecure` bool -that the config can inspect; the HTTP exporter has no such flag, only a full endpoint URL. Re-deriving -an "insecure" state would mean parsing `http://` vs `https://` in `__post_init__` to nudge the user -about a signal they already typed explicitly into the URL — more code for a weaker signal than the -URL itself gives. - -**Revisit trigger:** a user asks for a framework-specific free-threaded OTLP-HTTP convenience extra, -or for the HTTP endpoint to accept a bare `host:port` and auto-build the URL. Either reopens the -extras shape or the URL handling. diff --git a/docs/adr/0006-structlog-sentry-seam-stays-a-heuristic.md b/docs/adr/0006-structlog-sentry-seam-stays-a-heuristic.md new file mode 100644 index 0000000..91e8e41 --- /dev/null +++ b/docs/adr/0006-structlog-sentry-seam-stays-a-heuristic.md @@ -0,0 +1,22 @@ +# The structlog→Sentry seam is a value object, not a marker key + +`StructuredLogPayload` in `logging_factory.py` owns both the parse of a rendered log line and the +meta-key vocabulary (`STRUCTLOG_META_KEYS`), leaving `sentry_instrument.py` only the orchestration. +The problem solved was ownership, not the heuristic: the meta-key set was duplicated across the +producer's processor chain and the consumer's strip list and owned by neither, so renaming a meta-key +silently degraded Sentry enrichment. The producer still emits flat JSON and the consumer still +recognises it by `startswith("{")` — an explicit marker key would pollute user-visible stdout for +every logging user to serve one consumer, and a neutral third module both instruments import would +abstract a sharing that does not exist. + +The trap worth knowing before "fixing" it: structlog's own idiomatic +`ProcessorFormatter.wrap_for_formatter` is incompatible with this seam. It defers rendering to +handler-flush time, so `record.msg` is an `EventDict`, `sentry_sdk` builds `logentry.formatted` from +`record.getMessage()`, and the resulting Python repr opens with `{` — which the parser accepts, fails +to decode, and returns `None` from, silently dropping both `skip_sentry` and `contexts.structlog`. +The seam requires the line to be rendered before it becomes a `LogRecord`. + +A known accepted limit: a custom top-level meta-processor whose key is not added to +`STRUCTLOG_META_KEYS` still leaks into the payload. Nesting user kwargs under one `extra` key would +make that impossible, at the cost of changing the emitted log shape for every user; a round-trip test +is the trade taken instead. diff --git a/docs/adr/0007-core-declares-typing-extensions.md b/docs/adr/0007-core-declares-typing-extensions.md deleted file mode 100644 index b917086..0000000 --- a/docs/adr/0007-core-declares-typing-extensions.md +++ /dev/null @@ -1,37 +0,0 @@ -# Core declares `typing-extensions`; a genuinely zero-dependency core was tried and failed - -**Decision:** `lite-bootstrap`'s core declares exactly one runtime dependency, `typing-extensions`. -The alternative — removing the two runtime uses so a bare install has no dependencies at all — was -implemented, tested, and abandoned. - -`1.3.0` shipped claiming a zero-dependency core once `orjson` became opt-in (ADR-0005), but -`import lite_bootstrap` on a bare install raised `ModuleNotFoundError: No module named -'typing_extensions'`. Every install with any extra masked it, because every extra pulls -`typing_extensions` transitively. - -## Rejected alternative: remove the usage - -There are two runtime uses: `typing_extensions.Self` return annotations on `BaseConfig`'s -constructors, and `class HealthCheckTypedDict(typing_extensions.TypedDict, ...)` in the health-checks -instrument. The first is easy to drop (`from __future__ import annotations` plus `TYPE_CHECKING`). -The second is not, and that is what settles it: `HealthCheckTypedDict` is a FastAPI response model, -and pydantic refuses a stdlib `typing.TypedDict` model on Python < 3.12 — - -``` -PydanticUserError: Please use `typing_extensions.TypedDict` instead of -`typing.TypedDict` on Python < 3.12. -``` - -The attempt passed locally on 3.12 and against an isolated 3.10 `TypedDict` construction, and failed -the CI matrix on 3.10 and 3.11 — which is the shape of this whole class of mistake: the constraint -lives in pydantic's behaviour on an old interpreter, not in a `TypedDict` you can build in isolation. -So for as long as 3.10 and 3.11 are supported, core genuinely needs `typing_extensions` at runtime, -and declaring it is the honest fix. Dropping 3.10/3.11 to reclaim zero-dep was considered and is not -worth it on its own. - -`typing-extensions` is pure Python, so core stays free-threading-friendly; this is the leanest core -available rather than a compromise on the ft story. - -**Revisit trigger:** Python 3.11 goes out of support and the floor rises to 3.12, at which point -`typing.TypedDict` is acceptable to pydantic, `Self` is in `typing`, and the dependency can be -dropped for a genuinely zero-dependency core. diff --git a/docs/adr/0007-no-django-bootstrapper.md b/docs/adr/0007-no-django-bootstrapper.md new file mode 100644 index 0000000..e25c677 --- /dev/null +++ b/docs/adr/0007-no-django-bootstrapper.md @@ -0,0 +1,9 @@ +# No Django bootstrapper + +Every bootstrapper keeps one contract — the user constructs the application, passes it in, and gets +the same object back — and Django's observability is conventionally owned by `settings.py` +(`MIDDLEWARE` ordering, installed apps), which runs before any object a bootstrapper could be handed. +ADR-0001 is the calibration: FastMCP was the hardest framework to fit and still had an application +object to attach to. Attaching to a constructed `ASGIHandler` is the shape that would fit, and today +that means middleware outside `MIDDLEWARE`, diverging from every Django deployment guide and from +what a Django user would debug against. diff --git a/docs/adr/0008-otel-sampling-is-a-sampler-object.md b/docs/adr/0008-otel-sampling-is-a-sampler-object.md new file mode 100644 index 0000000..1ce2ef1 --- /dev/null +++ b/docs/adr/0008-otel-sampling-is-a-sampler-object.md @@ -0,0 +1,15 @@ +# OpenTelemetry sampling takes a `Sampler`, not a sample-rate float + +`opentelemetry_sampler` is handed straight to `TracerProvider(sampler=...)`. Left `None` — the +default — the SDK picks its own sampler, which honours `OTEL_TRACES_SAMPLER` / `OTEL_TRACES_SAMPLER_ARG`, +so both the default behaviour and the existing environment-variable path are unchanged; the +pass-through needs no conditional, because `TracerProvider(sampler=None)` is identical to omitting +the argument at the declared 1.28 floor and at 1.44 alike. + +A `float` sample rate mirroring `sentry_traces_sample_rate` was rejected. It buys no dependency +independence — `opentelemetry_instrumentors` already carries `BaseInstrumentor`, and +`sentry_integrations` / `sentry_before_send` carry Sentry types, all as quoted annotations over +`TYPE_CHECKING` imports that never run — and it closes only part of the gap, leaving `ALWAYS_OFF`, a +bare `TraceIdRatioBased` and vendor samplers unreachable behind an instrument that owns provider +construction and offers no `*_additional_params` escape hatch. A float can still be layered on top +later, which is what a settings object mapping one env var would want. diff --git a/docs/adr/0008-structlog-sentry-seam-stays-a-heuristic.md b/docs/adr/0008-structlog-sentry-seam-stays-a-heuristic.md deleted file mode 100644 index 97cf24f..0000000 --- a/docs/adr/0008-structlog-sentry-seam-stays-a-heuristic.md +++ /dev/null @@ -1,49 +0,0 @@ -# The structlog→Sentry seam is a value object, not a marker key or a shared module - -**Decision:** `StructuredLogPayload` in `logging_factory.py` owns the parse of a rendered log line -and the meta-key vocabulary (`STRUCTLOG_META_KEYS`); `sentry_instrument.py` keeps only the -orchestration — drop on `skip_sentry`, lift `message`, attach `extra` under `contexts.structlog`. -The producer still emits a plain flat JSON object and the consumer still recognises one by -`startswith("{")`. - -The problem being solved was ownership, not the heuristic: the meta-key set was duplicated across -the producer's processor chain and the consumer's strip list and owned by neither, so renaming a -meta-key silently degraded Sentry enrichment with nothing failing. - -## Rejected alternatives - -**Stamp an explicit sentinel/marker key on every log line** instead of sniffing `startswith("{")`. -This pollutes the stdout JSON shape for every logging user in order to serve one consumer. The -heuristic is cheap and adequate, and the log shape is user-visible output. - -**A neutral third module both instruments import.** There is exactly one consumer. A -shared-for-sharing's-sake module abstracts a sharing that does not exist and pulls the vocabulary -away from the chain that generates it — the value object belongs next to the serializer that -produces what it parses. - -**A symmetric `serialize()` on the value object.** The producer never constructs a -`StructuredLogPayload`; it hands structlog's full `event_dict` to the existing serializer. A -`serialize()` nobody calls would be dead surface. - -**An absolute, compile-time drift fix.** Nesting user kwargs under a single `extra` key would make -drift impossible, and would change the emitted log shape for every user. The deliberate trade is -"unlikely and caught by a round-trip test" over "impossible and a breaking change": a custom -top-level meta-processor whose key is not added to `STRUCTLOG_META_KEYS` still leaks, and that is a -known, accepted limit. - -**Deferring the render to `ProcessorFormatter.wrap_for_formatter`.** This is structlog's own -idiomatic stdlib integration, it is already what `_configure_foreign_loggers` uses ten lines away, -and it was the obvious fix for #193's double-rendered traceback. It is incompatible with this seam. -`wrap_for_formatter` moves rendering to handler-flush time, so `record.msg` is the `EventDict` -rather than a rendered line; `sentry_sdk` builds `logentry.formatted` from `record.getMessage()`, -which for a dict `msg` is a Python repr. That repr opens with `{`, so `StructuredLogPayload.parse` -accepts it, fails to decode it, and returns `None` — `skip_sentry` stops being honoured and -`contexts.structlog` stops being attached, with nothing failing. The seam requires that the line -be rendered before it becomes a `LogRecord`; anything downstream of the chain is transport only. - -`IGNORED_STRUCTLOG_ATTRIBUTES` survives in `sentry_instrument.py` as a silent alias of -`STRUCTLOG_META_KEYS` for external importers of the old name. - -**Revisit trigger:** a second consumer honours `skip_sentry` or needs the parsed payload. At that -point the value object has a real audience, `skip_sentry` should be renamed to something reporter- -neutral, and a marker key stops being a cost paid for one consumer. diff --git a/docs/adr/0009-no-django-bootstrapper.md b/docs/adr/0009-no-django-bootstrapper.md deleted file mode 100644 index 25b3b28..0000000 --- a/docs/adr/0009-no-django-bootstrapper.md +++ /dev/null @@ -1,17 +0,0 @@ -# No Django bootstrapper - -**Decision:** `lite-bootstrap` will not ship a Django bootstrapper. Django's size makes it the -framework most likely to be proposed by someone who has not hit the contract below. - -Every bootstrapper keeps one contract: the user constructs the application, passes it in, and gets -the same object back. Django's observability is conventionally owned by `settings.py` — `MIDDLEWARE` -ordering, installed apps — which runs before any object a bootstrapper could be handed, so the -contract has no natural expression there. ADR-0001 is the calibration: FastMCP was the hardest -framework to fit and still had an application object to attach to. - -Rejected: **attach to a constructed `ASGIHandler` instead.** This is the shape that would fit, and -today it means putting middleware outside `MIDDLEWARE`, diverging from every Django deployment guide -and from what a Django user would debug against. - -**Revisit trigger:** a released, maintained path that attaches instrumentation to a constructed -`ASGIHandler` (or equivalent) without going through `settings.py`. diff --git a/docs/agents/domain.md b/docs/agents/domain.md index 77cb857..c5d8994 100644 --- a/docs/agents/domain.md +++ b/docs/agents/domain.md @@ -2,31 +2,26 @@ How the engineering skills should consume this repo's domain documentation when exploring the codebase. -## Layout +## Before exploring, read these + +- **`CONTEXT.md`** at the repo root. +- **`docs/adr/`**: read ADRs that touch the area you're about to work in. + +If any of these files don't exist, **proceed silently**. Don't flag their absence; don't suggest creating them upfront. The `/domain-modeling` skill (reached via `/grill-with-docs` and `/improve-codebase-architecture`) creates them lazily when terms or decisions actually get resolved. + +## File structure -Single-context, and flat: +Single-context repo: ``` / ├── CONTEXT.md ├── docs/adr/ │ ├── 0001-fastmcp-teardown-via-provider-lifespan.md -│ └── ... +│ └── 0002-cross-cutting-logic-stays-put.md └── lite_bootstrap/ ``` -There is no `CONTEXT-MAP.md` and no per-package `CONTEXT.md` or `docs/adr/`. If you are looking for -either, the answer is that this repo does not have them. - -## Before exploring, read these - -- **`CONTEXT.md`** at the repo root. It owns the vocabulary, and `AGENTS.md` requires reading it - before naming a concept in code, a test name, or an issue title. The **configured** / **ready** - split and the two kinds of **skip** are defined there and are load-bearing throughout. -- **`docs/adr/`**: read the ADRs that touch the area you are about to work in. They are an internal - decision record, excluded from the published docs site, so an ADR is written for contributors - rather than users. - ## Use the glossary's vocabulary When your output names a domain concept (in an issue title, a refactor proposal, a hypothesis, a test name), use the term as defined in `CONTEXT.md`. Don't drift to synonyms the glossary explicitly avoids. @@ -37,4 +32,4 @@ If the concept you need isn't in the glossary yet, that's a signal: either you'r If your output contradicts an existing ADR, surface it explicitly rather than silently overriding: -> _Contradicts ADR-0002 (keep the per-instrument axis), but worth reopening because…_ +> _Contradicts ADR-0002 (cross-cutting logic stays put), but worth reopening because…_ diff --git a/docs/introduction/configuration.md b/docs/introduction/configuration.md index f8fb32c..0e35807 100644 --- a/docs/introduction/configuration.md +++ b/docs/introduction/configuration.md @@ -78,9 +78,23 @@ Additional parameters: - `opentelemetry_insecure` - whether the gRPC OTLP connection is insecure (gRPC only; for `http` the endpoint URL scheme carries security). - `opentelemetry_instrumentors` - a list of extra instrumentors. - `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. +Sampling is the cheapest way to cut what tracing costs: on a benchmark endpoint returning a constant, +`ParentBased(TraceIdRatioBased(0.01))` saved ~55 µs per request against the always-on default. + +```python +from opentelemetry.sdk.trace.sampling import ParentBased, TraceIdRatioBased + +config = FastAPIConfig( + service_name="my-service", + opentelemetry_endpoint="otl", + opentelemetry_sampler=ParentBased(TraceIdRatioBased(0.01)), +) +``` + For FastStream you must provide additionally: - `opentelemetry_middleware_cls` diff --git a/lite_bootstrap/instruments/logging_factory.py b/lite_bootstrap/instruments/logging_factory.py index b6c6e0e..ba293d6 100644 --- a/lite_bootstrap/instruments/logging_factory.py +++ b/lite_bootstrap/instruments/logging_factory.py @@ -46,7 +46,7 @@ def _dumps_stdlib(value: typing.Any, **kwargs: typing.Any) -> str: # noqa: ANN4 # orjson has no free-threaded wheels and refuses to build on ft; fall back to the -# stdlib json accelerator (always ft-native) when it is absent. See ADR-0005. +# stdlib json accelerator (always ft-native) when it is absent. See ADR-0004. _serialize_log_to_string = _dumps_orjson if import_checker.is_orjson_installed else _dumps_stdlib _json_loads = orjson.loads if import_checker.is_orjson_installed else json.loads diff --git a/lite_bootstrap/instruments/opentelemetry_instrument.py b/lite_bootstrap/instruments/opentelemetry_instrument.py index d30d03f..1f709c6 100644 --- a/lite_bootstrap/instruments/opentelemetry_instrument.py +++ b/lite_bootstrap/instruments/opentelemetry_instrument.py @@ -12,6 +12,7 @@ if typing.TYPE_CHECKING: from opentelemetry.instrumentation.instrumentor import BaseInstrumentor + from opentelemetry.sdk.trace.sampling import Sampler if import_checker.is_opentelemetry_sdk_installed: from opentelemetry.context import Context @@ -69,6 +70,9 @@ class OpenTelemetryConfig(OpenTelemetryServiceFieldsConfig): default_factory=list ) opentelemetry_log_traces: bool = False + # Left None, the SDK picks its own default, which reads OTEL_TRACES_SAMPLER; see ADR-0008 + # (docs/adr/0008-otel-sampling-is-a-sampler-object.md). + opentelemetry_sampler: "Sampler | None" = None opentelemetry_generate_health_check_spans: bool = True opentelemetry_excluded_urls: list[str] = dataclasses.field(default_factory=list) @@ -228,7 +232,7 @@ def _apply_instrumentors(self, tracer_provider: "TracerProvider") -> None: def bootstrap(self) -> None: config = self.bootstrap_config self._silence_otel_loggers() - tracer_provider = TracerProvider(resource=self._build_resource()) + tracer_provider = TracerProvider(resource=self._build_resource(), sampler=config.opentelemetry_sampler) set_tracer_provider(tracer_provider) self._tracer_provider = tracer_provider if import_checker.is_pyroscope_installed and getattr(config, "pyroscope_endpoint", None): diff --git a/scripts/floor_smoke.py b/scripts/floor_smoke.py index 717fe68..b6f81de 100644 --- a/scripts/floor_smoke.py +++ b/scripts/floor_smoke.py @@ -132,7 +132,7 @@ def _fastapi() -> None: ) application = bootstrapper.bootstrap() # Generating the schema puts HealthCheckTypedDict through pydantic as a response model, - # which is the typing-extensions use ADR-0007 records. + # which is the typing-extensions use ADR-0005 records. assert application.openapi()["paths"] assert application.url_path_for("health_check_handler") == HEALTH_PATH _emit_span() diff --git a/tests/instruments/test_opentelemetry_instrument.py b/tests/instruments/test_opentelemetry_instrument.py index 0ab0c78..1a37cd4 100644 --- a/tests/instruments/test_opentelemetry_instrument.py +++ b/tests/instruments/test_opentelemetry_instrument.py @@ -6,6 +6,7 @@ import pytest from opentelemetry.instrumentation.instrumentor import BaseInstrumentor +from opentelemetry.sdk.trace import sampling import lite_bootstrap.instruments.opentelemetry_instrument as otel_module from lite_bootstrap import import_checker @@ -46,6 +47,73 @@ def test_opentelemetry_instrument_empty_instruments() -> None: opentelemetry_instrument.teardown() +_SAMPLER_ENV_VARS: typing.Final = ("OTEL_TRACES_SAMPLER", "OTEL_TRACES_SAMPLER_ARG") + + +@pytest.fixture +def without_sampler_env(monkeypatch: pytest.MonkeyPatch) -> None: + for env_var in _SAMPLER_ENV_VARS: + monkeypatch.delenv(env_var, raising=False) + + +@pytest.mark.usefixtures("without_sampler_env") +def test_opentelemetry_sampler_reaches_tracer_provider() -> None: + sample_rate = 0.01 + sampler = sampling.ParentBased(sampling.TraceIdRatioBased(sample_rate)) + instrument = OpenTelemetryInstrument( + bootstrap_config=OpenTelemetryConfig(opentelemetry_log_traces=True, opentelemetry_sampler=sampler), + ) + try: + instrument.bootstrap() + assert instrument._tracer_provider is not None # noqa: SLF001 + assert instrument._tracer_provider.sampler is sampler # noqa: SLF001 + finally: + instrument.teardown() + + +@pytest.mark.usefixtures("without_sampler_env") +def test_opentelemetry_sampler_unset_keeps_sdk_default() -> None: + instrument = OpenTelemetryInstrument(bootstrap_config=OpenTelemetryConfig(opentelemetry_log_traces=True)) + try: + instrument.bootstrap() + assert instrument._tracer_provider is not None # noqa: SLF001 + assert instrument._tracer_provider.sampler is sampling.DEFAULT_ON # noqa: SLF001 + finally: + instrument.teardown() + + +def test_opentelemetry_sampler_unset_honours_sampler_env_vars(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("OTEL_TRACES_SAMPLER", "traceidratio") + monkeypatch.setenv("OTEL_TRACES_SAMPLER_ARG", "0.25") + instrument = OpenTelemetryInstrument(bootstrap_config=OpenTelemetryConfig(opentelemetry_log_traces=True)) + try: + instrument.bootstrap() + assert instrument._tracer_provider is not None # noqa: SLF001 + sampler = instrument._tracer_provider.sampler # noqa: SLF001 + assert isinstance(sampler, sampling.TraceIdRatioBased) + env_rate = 0.25 + assert sampler.rate == env_rate + finally: + instrument.teardown() + + +def test_opentelemetry_sampler_wins_over_sampler_env_vars(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("OTEL_TRACES_SAMPLER", "traceidratio") + monkeypatch.setenv("OTEL_TRACES_SAMPLER_ARG", "0.25") + instrument = OpenTelemetryInstrument( + bootstrap_config=OpenTelemetryConfig( + opentelemetry_log_traces=True, + opentelemetry_sampler=sampling.ALWAYS_OFF, + ), + ) + try: + instrument.bootstrap() + assert instrument._tracer_provider is not None # noqa: SLF001 + assert instrument._tracer_provider.sampler is sampling.ALWAYS_OFF # noqa: SLF001 + finally: + instrument.teardown() + + def test_opentelemetry_instrument_teardown_shuts_down_tracer_provider() -> None: instrument = OpenTelemetryInstrument( bootstrap_config=OpenTelemetryConfig(opentelemetry_log_traces=True), diff --git a/tests/test_adr_citations.py b/tests/test_adr_citations.py index 760f6f1..292686b 100644 --- a/tests/test_adr_citations.py +++ b/tests/test_adr_citations.py @@ -8,43 +8,63 @@ _REPO_ROOT: typing.Final = pathlib.Path(__file__).resolve().parent.parent _ADR_DIR: typing.Final = "docs/adr/" _CITATION: typing.Final = re.compile(r"docs/adr/\d{4}-[a-z0-9-]+\.md") +_NUMBER_PREFIX: typing.Final = "ADR-" +_NUMBER_CITATION: typing.Final = re.compile(_NUMBER_PREFIX + r"\d{4}") _UNWALKED_DIR: typing.Final = "node_modules" +# mkdocs build output: a second copy of docs/, whose citations are the originals'. +_GENERATED_DIR: typing.Final = "site" +_SCANNED_SUFFIXES: typing.Final = (".py", ".md", ".toml") -def _python_files(root: pathlib.Path) -> list[pathlib.Path]: +def _scanned_files(root: pathlib.Path) -> list[pathlib.Path]: found: list[pathlib.Path] = [] for dirpath, dirnames, filenames in os.walk(root): - dirnames[:] = sorted(name for name in dirnames if not name.startswith(".") and name != _UNWALKED_DIR) - found.extend(pathlib.Path(dirpath, name) for name in sorted(filenames) if name.endswith(".py")) + dirnames[:] = sorted( + name for name in dirnames if not name.startswith(".") and name not in (_UNWALKED_DIR, _GENERATED_DIR) + ) + found.extend(pathlib.Path(dirpath, name) for name in sorted(filenames) if name.endswith(_SCANNED_SUFFIXES)) return found -def _citations(source: str) -> set[str]: +def _citations(file: pathlib.Path, source: str) -> set[str]: texts = [source] - texts.extend( - node.value - for node in ast.walk(ast.parse(source)) - if isinstance(node, ast.Constant) and isinstance(node.value, str) - ) - return {cited for text in texts for cited in _CITATION.findall(text)} + if file.suffix == ".py": + # Adjacent literals are joined at parse time, so a path split across them is one string + # in the AST and two fragments in the raw text. + texts.extend( + node.value + for node in ast.walk(ast.parse(source)) + if isinstance(node, ast.Constant) and isinstance(node.value, str) + ) + return {cited for text in texts for pattern in (_CITATION, _NUMBER_CITATION) for cited in pattern.findall(text)} + + +def _resolves(root: pathlib.Path, cited: str) -> bool: + if cited.startswith(_ADR_DIR): + return (root / cited).is_file() + return any((root / _ADR_DIR).glob(f"{cited.removeprefix(_NUMBER_PREFIX)}-*.md")) def unresolved_citations(root: pathlib.Path) -> list[tuple[str, str]]: return sorted( (file.relative_to(root).as_posix(), cited) - for file in _python_files(root) - for cited in _citations(file.read_text(encoding="utf-8")) - if not (root / cited).is_file() + for file in _scanned_files(root) + for cited in _citations(file, file.read_text(encoding="utf-8")) + if not _resolves(root, cited) ) -def test_every_adr_path_cited_from_python_resolves() -> None: - """INVARIANT: a `docs/adr/NNNN-.md` path named anywhere in this repo's Python exists. +def test_every_adr_citation_in_the_repo_resolves() -> None: + """INVARIANT: every ADR named in this repo resolves, by full path or by bare `ADR-NNNN` number. + + Broken by renaming, renumbering or pruning an ADR without following its citations. The offline + link gate reads Markdown links only, so a path in a docstring, a comment, a guard message or a + `pyproject.toml` dependency rationale is otherwise checked by nothing, and neither is the bare + number form, which is how most of them are written. A user who trips a guard is handed a link + to follow. - Broken by renaming, renumbering or pruning an ADR without following its citations. The - offline link gate reads Markdown only, so a path in a docstring, a comment or a guard message - is otherwise checked by nothing, and an `INVARIANT:` docstring that names its ADR silently - loses the rationale the test depends on. A user who trips a guard is handed a link to follow. + The number form is checked for existence only: a citation renumbered onto a *different* live + ADR still resolves, and nothing here can know it now names the wrong decision. """ unresolved = unresolved_citations(_REPO_ROOT) @@ -81,6 +101,29 @@ def test_a_citation_inside_a_hash_comment_is_found(tmp_path: pathlib.Path) -> No assert unresolved_citations(tmp_path) == [("graph.py", f"{_ADR_DIR}0009-comment.md")] +def test_a_bare_adr_number_naming_no_file_is_reported(tmp_path: pathlib.Path) -> None: + """A renumber leaves `ADR-NNNN` prose behind; only the path form was ever checked.""" + (tmp_path / _ADR_DIR).mkdir(parents=True) + (tmp_path / _ADR_DIR / "0001-kept.md").write_text("# kept\n", encoding="utf-8") + (tmp_path / "smoke.py").write_text( + f"# the constraint {_NUMBER_PREFIX}0001 records, unlike {_NUMBER_PREFIX}9999\n", encoding="utf-8" + ) + + assert unresolved_citations(tmp_path) == [("smoke.py", f"{_NUMBER_PREFIX}9999")] + + +def test_a_citation_outside_python_is_found(tmp_path: pathlib.Path) -> None: + """`pyproject.toml` explains dependency floors by citing ADRs, and Markdown cites them in prose.""" + (tmp_path / _ADR_DIR).mkdir(parents=True) + (tmp_path / "pyproject.toml").write_text(f"# see {_ADR_DIR}0002-floor.md\ndeps = []\n", encoding="utf-8") + (tmp_path / "AGENTS.md").write_text(f"Read {_NUMBER_PREFIX}0004 before editing.\n", encoding="utf-8") + + assert unresolved_citations(tmp_path) == [ + ("AGENTS.md", f"{_NUMBER_PREFIX}0004"), + ("pyproject.toml", f"{_ADR_DIR}0002-floor.md"), + ] + + def test_a_tree_with_no_citations_and_no_adr_directory_reports_nothing(tmp_path: pathlib.Path) -> None: (tmp_path / "plain.py").write_text("X = 1\n", encoding="utf-8") diff --git a/tests/test_core_import_surface.py b/tests/test_core_import_surface.py index 183dbe6..f16b828 100644 --- a/tests/test_core_import_surface.py +++ b/tests/test_core_import_surface.py @@ -36,7 +36,7 @@ def test_importing_lite_bootstrap_needs_only_the_stdlib_and_typing_extensions() The rule is also what makes the free-threaded story work: nothing native is reachable from a bare import, so core installs and imports on any interpreter and each unavailable extra degrades to a skipped instrument rather than an ImportError. Adding a genuinely mandatory dependency is - allowed — declare it in `[project.dependencies]` and add it here; ADR-0007 records why + allowed — declare it in `[project.dependencies]` and add it here; ADR-0005 records why `typing-extensions` is the only one. """ offenders = { diff --git a/tests/test_free_bootstrap.py b/tests/test_free_bootstrap.py index 8b37243..d8d1732 100644 --- a/tests/test_free_bootstrap.py +++ b/tests/test_free_bootstrap.py @@ -5,6 +5,7 @@ import pytest import structlog +from opentelemetry.sdk.trace import sampling from lite_bootstrap import ( FreeBootstrapper, @@ -14,6 +15,7 @@ ) from lite_bootstrap.bootstrappers.base import BaseBootstrapper from lite_bootstrap.instruments.logging_instrument import LoggingInstrument +from lite_bootstrap.instruments.opentelemetry_instrument import OpenTelemetryInstrument from lite_bootstrap.instruments.pyroscope_instrument import PyroscopeInstrument from lite_bootstrap.instruments.sentry_instrument import SentryInstrument from tests.conftest import CustomInstrumentor, SentryTestTransport, emulate_package_missing @@ -42,6 +44,25 @@ def test_free_bootstrap(free_bootstrapper_config: FreeConfig) -> None: bootstrapper.teardown() +def test_free_bootstrap_passes_sampler_to_tracer_provider() -> None: + sampler = sampling.ParentBased(sampling.TraceIdRatioBased(0.01)) + bootstrapper = FreeBootstrapper( + bootstrap_config=FreeConfig( + opentelemetry_log_traces=True, + opentelemetry_sampler=sampler, + logging_buffer_capacity=0, + ), + ) + bootstrapper.bootstrap() + try: + instruments = [one for one in bootstrapper.instruments if isinstance(one, OpenTelemetryInstrument)] + assert len(instruments) == 1 + assert instruments[0]._tracer_provider is not None # noqa: SLF001 + assert instruments[0]._tracer_provider.sampler is sampler # noqa: SLF001 + finally: + bootstrapper.teardown() + + def test_free_bootstrap_logging_disabled() -> None: bootstrapper = FreeBootstrapper( bootstrap_config=FreeConfig(