Repository navigation
Default opentelemetry_generate_health_check_spans to False - #366
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #311.
opentelemetry_generate_health_check_spansnow defaults toFalse, so a Kubernetes probe sends no trace. The access log has always left the health path out, so tracing and logging now agree. The change is the default atopentelemetry_instrument.py:86._build_infrastructure_excluded_pathsalready adds the health path while the flag is off, so FastAPI, Litestar and FastMCP need nothing else. FastStream's health handler skips its own span wrapper on the same flag.Tests
Written first. These failed on main:
test_faststream_health_check_records_no_span_by_default: one health request with the default config produced aGET /custom-health/span.test_fastapi_otel_leaves_the_health_check_untraced_by_default: a defaultFastAPIConfigtracedGET /custom-health/. This covers the issue's check, a default FastAPI app withopentelemetry_log_traces=Trueprinting no span for the health path. That test uses its own config because the shared fixture sets the flag explicitly.These kept the old default and now set
opentelemetry_generate_health_check_spans=Trueexplicitly:test_faststream_bootstrap_without_opentelemetry, so it still covers the unboundtracerbug from FastStream bootstrapper NameError whenprometheus/opentelemetryextras are not installed #87, as the issue asked.test_faststream_build_excluded_urls_covers_prometheus_and_health_paths, for the "kept when health spans are on" half.test_faststream_health_check_span_is_recorded_when_turned_onis new, and it passes on main too. No FastStream test had checked that the span is actually recorded. Its helper patches the module'stracerwith a fresh proxy tracer. The module-leveltrace.get_tracer(__name__)binds to the first real provider it sees, and the suite installs a new provider for every test. In production there is one provider per process, so this only affects tests.The full suite passes, 577 tests at 100% coverage (
just test-ci).just lint(includingty),just adr-checkandjust docs-buildpass too. No floor changes.Not in the issue
While the flag is off, OpenTelemetry's HTTP metrics also skip health-check requests, because both instrumentations skip an excluded URL completely. I checked this on FastAPI and Litestar with an in-memory metric reader: with the flag off, a health request records no
http.server.*metric, and with it on, it records one. Since #365, a service withopentelemetry_metrics_endpointset has no Prometheus HTTP metrics either, so its probe requests are now counted nowhere. Theconfiguration.mdbullet says so. Probes usually stay out of request metrics anyway, so I kept the issue's behaviour and didn't add a separate switch.Measurement
In-process on the M2 host (macOS, CPython 3.14.7, fastapi 0.162.0, litestar 2.25.0, opentelemetry-sdk 1.45.1), at 100% sampling. Figures are µs per
/health/request over the health check without OpenTelemetry:health_otel: the 2.0 defaulthealth_otel_spans: the 1.x defaultLitestar's rounds differed by up to 24%. As the issue says, the gain is trace volume, one trace per probe per pod, not CPU.
health_otel_spansis a new scenario in the FastAPI and Litestar suites.health_otel_nospannow has the same config ashealth_otel, and I kept it so the existing README command still runs.Docs
configuration.md: the flag's bullet gives the new default, says it now matches the access log, explains the metrics side effect and says how to turn spans back on. The FastStream paragraph says the span is recorded when the flag is True.fastmcp.md: "unless ... is True".performance.md: the "savings applied for you" paragraph now has eight savings.benchmarks/README.md: the two health rows are relabelled "1.x default", §4d gets the rerun, and §9 gets the FastAPI command.Breaking changes
Health-check requests are no longer traced, and while the flag is off they no longer appear in OpenTelemetry's HTTP metrics. Set
opentelemetry_generate_health_check_spans=Trueto keep both, for example to watch probe latency.