fix: anchor litestar trace-exclusion patterns to the normalized path - #250
Merged
Merged
Conversation
Litestar strips the trailing slash out of scope["path"], so an excluded entry carrying one never matched the URL OpenTelemetryMiddleware builds. The default health_checks_path is "/health/", so opentelemetry_generate_health_check_spans=False silently did nothing. Closes #248
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 #248.
The defect
opentelemetry_generate_health_check_spans=Falsesilently did nothing on Litestar. Litestarnormalizes the trailing slash out of
scope["path"], soOpenTelemetryMiddlewarebuilthttp://host/custom-healthwhile the excluded entry was/custom-health/.ExcludeListregex-searches, missed, and the health check was traced anyway.
The default
health_checks_pathis/health/, so this is the default configuration. FastAPI isunaffected because Starlette does not normalize the slash away.
Confirmed before the fix with an in-memory exporter: 3 spans for
/custom-health/and 3 for/custom-metrics/, each carryinghttp.url = http://testserver.local/custom-health.The fix
Stripping the slash alone would have been wrong.
ExcludeList.url_disabledisbool(search(regex, url))over a|alternation, so entries are unanchored substrings, and a bare/custom-healthwould have started excluding/custom-healthyfrom tracing instead. So the derivedpaths are rendered as anchored patterns:
rf"^\w+://[^/]*{re.escape(path.rstrip('/'))}(?:/|$)"[^/]*cannot cross a slash, which pins the path to the start of the URL's path component, so/api/custom-healthdoes not match either.(?:/|$)admits sub-paths but not the lookalike. Themiddleware's URL carries no query string (
get_host_port_url_tuplebuildsscheme://host+scope["path"]), so$is safe.Caller-supplied entries pass through verbatim
opentelemetry_excluded_urlsentries are OpenTelemetry regexes, not paths.re.escapewould havedestroyed an entry like
client/.*/info, and anchoring it to the scheme and host would have brokenit a second way — on Litestar only, while FastAPI kept honoring it. They are therefore left
untouched, with a test pinning that.
That required separating the two kinds of entry, so
_build_excluded_urlsnow composes_build_infrastructure_excluded_paths()with the caller's list. FastAPI and FastStream consume themerged set exactly as before. The exclusion policy — which paths, and the
getattrsibling reads —still lives in one method, so ADR-0002 holds; only the rendering differs per framework.
Scope
Litestar only, as the issue specified. FastAPI has the mirror-image defect from the same unanchored
search (
prometheus_metrics_path="/metrics"also excludes/metrics-internal), but that is a falseexclusion rather than a false trace, nobody has hit it, and fixing it would change released behaviour
on a framework that currently works.
Verification
Span counts through an in-memory exporter, not
url_disabledon a hand-written URL — the test addedin #247 asserted
url_disabled("http://test/custom-health/"), a URL form the middleware neverreceives, and passed while the behaviour it described was false. That stale comment is removed here.
test_litestar_otel_excludes_infrastructure_paths_normalized_by_litestar— health and metricspaths emit no spans;
/custom-healthystill does.test_litestar_otel_keeps_caller_supplied_excluded_urls_as_regexes—/items/\d+$still excludes/items/42, which only passes if the regex survived unescaped.Full suite green: 327 passed, three consecutive randomized runs.
ruff format,ruff check --no-fixand
ty checkclean.