Conversation
|
🤖 platform-health-agent · CI green, awaiting human review · 2026-10-31 details
History:
|
There was a problem hiding this comment.
No issues found across 3 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
You’re at about 92% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Shadow auto-approve: would require human review. Upgrades FastAPI to 0.143.0 and pins one server span per API request with a test; however the upgrade introduces new built-in tracing spans on Prefect worker health checks and leaves open whether to disable that tracing. Human sign-off is needed for that operational telemetry decision.
View guided diff | Re-trigger cubic
There was a problem hiding this comment.
0 issues found across 4 files (changes from recent commits).
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
You’re at about 93% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Shadow auto-approve: would auto-approve. Upgrades FastAPI to 0.143.0 while preserving prior tracing: a new sampler drops built-in spans on task-worker /health checks, and tests pin one contrib server span per API request. Bounded, restores old span behavior, with unit tests.
View guided diff | Re-trigger cubic
This comment was marked as low quality.
This comment was marked as low quality.
cffb692 to
6123b56
Compare
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
FastAPI 0.142.0 added built-in OpenTelemetry tracing. Infrahub instruments its API with the contrib FastAPIInstrumentor, and FastAPI skips its own tracing when it finds that middleware in the built stack. Pin that with a test that counts the server spans for one request, plus a control showing FastAPI's built-in tracing is active in the setup. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
FastAPI 0.142.0 traces any FastAPI app once a global tracer provider is set. The task worker sets one when tracing is enabled, and Prefect's health-check server (`--with-healthcheck`) runs in the same process, so every GET /health would record a server span plus three internal spans. Add ExcludeServerPathsSampler, which drops server spans for given paths and leaves every other decision to the sampler the SDK resolved from OTEL_TRACES_SAMPLER. Spans inside a dropped request follow their parent. Only the task worker passes a path (/health). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ests FastAPI's built-in tracing also records internal spans (dependencies, endpoint, serialization). Counting server spans alone would miss them, so also assert that every recorded span comes from the contrib instrumentation. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…set to The sampler dropped the request span of each GET /health, but the three spans FastAPI records inside the request were only dropped when the configured sampler follows the parent's decision. With always_on or traceidratio, each health check still recorded those three spans. The sampler now also drops a span whose parent in the same process was not sampled. With the built-in samplers this changes no other trace: always_on samples every root span, and traceidratio gives every span of a trace the same decision. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The API is traced by the OpenTelemetry contrib instrumentation. FastAPI skips its own tracing when it finds the contrib middleware in the middleware stack, so the API relied on that detection to avoid tracing every request twice. The API now passes telemetry with tracing, metrics and logs off, so it no longer depends on the detection. The new test removes the contrib middleware from the stack and checks that FastAPI still records no span; it fails without the setting. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Adds a knowledge page on how the API server and task workers produce traces, metrics and logs: which process sets up tracing, what records spans, why FastAPI's built-in telemetry stays off, how the health-check sampler works, and how the local observability stack collects the signals. Indexed from backend/AGENTS.md and dev/README.md. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
a4a5780 to
a0d768b
Compare
|
This PR has been inactive for 2 weeks and has been marked as stale. If you're still working on this, please:
The stale label will be removed automatically when there's new activity. |
FastAPI 0.143.0 traces requests on its own, and Infrahub must not record extra spans
This PR upgrades FastAPI from 0.141.1 to 0.143.0. Operators who turn tracing on in Infrahub receive the same spans from the API server and the task workers as before the upgrade.
Jira: INBOX-364
How tracing works in Infrahub, and what FastAPI 0.142.0 changes
When an operator turns tracing on, Infrahub records a span for each step of its work and sends the spans to a trace collector. A span is a timed record of one step: what ran, when it started, how long it took and whether it failed. For example, each API request records a request span.
opentelemetry-instrumentation-fastapi, whichserver.pyadds to the API when it starts.The Release Radar bot reported that the upgrade would double every request's spans in Infrahub. The effect differs for each Infrahub process:
server.pyadds that package before the API handles its first request. This PR also turns FastAPI's own telemetry off inserver.py, so the API no longer depends on FastAPI finding the package. Each API request still records one request span, and FastAPI records none. The new tests intest_server.pycheck this, also with the separate package removed from the app.--with-healthcheck, each task worker also runs a small web server from Prefect that answersGET /health(prefect/workers/server.py). Prefect creates that server as a plain FastAPI app, so Infrahub cannot add the separate package to it or turn FastAPI's tracing off on it. When tracing is on, the worker sets it up for its whole process, so on 0.143.0 every health check would record three spans: the request, resolving dependencies and running the endpoint.Changes
fastapi0.141.1 → 0.143.0 inpyproject.tomlanduv.lock. No package is added or removed.server.pypassestelemetry={"tracing": False, "metrics": False, "logs": False}toFastAPI(...), with a comment saying why.ExcludeServerPathsSamplerinbackend/infrahub/trace.py, drops the request span of everyGET /health.OTEL_TRACES_SAMPLERis set to.OTEL_TRACES_SAMPLER, so operators who set that variable keep their sampling.dev/knowledge/backend/observability.md: how the API server and the task workers produce traces, metrics and logs, why FastAPI's built-in telemetry stays off, and how the local observability stack collects them. It is linked frombackend/AGENTS.mdanddev/README.md.backend/tests/unit/test_server.pychecks that one API request records one request span from the separate OpenTelemetry package and no span from FastAPI's own tracing, and that FastAPI records no span even when the separate package is removed from the app.backend/tests/unit/test_trace.pychecks that a/healthrequest records no spans and that other requests record all their spans, with the default sampler,always_onandtraceidratio.No changelog fragment: traces contain the same spans as on 0.141.1.
How this was verified
instrument_appcall inserver.pyis removed.telemetrysetting is removed fromserver.py.always_onandtraceidratiocases also fail without the check on the parent span.FASTAPI_OTEL_AUTO_CONFIGUREis set, and Infrahub does not set it.fastapi>=0.139.0,<1.0.0.stable-based branch: ruff, mypy, the generated-file checks and 2782 backend unit tests.Spans recorded in a local stack, before and after the upgrade
The development stack was run with tracing turned on and the spans sent to Tempo, with an image built from this branch. Only two things changed between runs: the FastAPI version, switched with
uv pip installinside the server and task-worker containers, and the Infrahub code, either this branch or the commit it was then based on. The runs were done before this branch moved fromdeveloptostable, on top ofdevelopcommit62b99b2507, and beforeserver.pyturned FastAPI's telemetry off.stableanddeveloppin the same FastAPI, Prefect and OpenTelemetry versions, and the branch's commits apply tostablewithout changes. That setting changes no span, because FastAPI already skipped its tracing in the API. Each run sent 5GET /api/configrequests and 5 GraphQL queries (BuiltinTag { count }) to the API, and 5 health checks to each of the 2 task workers. The spans were then read back from Tempo.GET /api/configThe API recorded the same spans in all three runs, and every span came from the separate OpenTelemetry package or from Infrahub's own tracing. For
GET /api/config, these are the request span and threehttp sendspans. FastAPI recorded no span of its own in the API.Without this PR's code, every health check on a task worker recorded three spans from FastAPI. Prefect's health check returns its response directly, so FastAPI records no serialization span for it.
With
OTEL_TRACES_SAMPLER=always_onon the task workers and FastAPI 0.143.0, this PR before the check on the parent span (commit981a11f4a2) still recordedfastapi.dependenciesandfastapi.endpointfor every health check, without their request span. With the check (commit73555e5a48), health checks record no span. In that run, the worker's background tasks, such as "Sync Git Repositories" and "Detect failed merges", were recorded with the same spans as before the upgrade.Follow-up, not in this PR
A longer-term option, moving the API to FastAPI's built-in tracing and removing the separate package, is described in this comment.
🤖 Generated with Claude Code