fix(tracing): capture request paths for unmatched routes
Build and Push Docker Container / build-and-push (push) Successful in 4m43s
Build and Push Docker Container / build-and-push (push) Successful in 4m43s
- Record url.path before routing so 404s and early responses retain their path. - Keep matched route templates as span names to limit cardinality. - Exclude query strings, full URLs, cookies and authorization headers. - Test decoded paths on 404 responses and path attributes on matched routes. - Document that paths may expose filenames, IDs or share tokens. - Validate NanoShare with 294 passing tests.
This commit is contained in:
@@ -44,8 +44,11 @@ worker/thread boundaries, propagates W3C context, and sends native
|
||||
`convex-request-id: otel-<trace-id>-<span-id>` headers without changing function
|
||||
arguments. Retries get distinct span/request IDs.
|
||||
|
||||
Spans do **not** capture payloads, query strings, raw URLs, cookies, credentials,
|
||||
or exception text. WebSockets and ASGI lifespan messages pass through unchanged.
|
||||
HTTP spans include `url.path` for every request, including unmatched 404s.
|
||||
Paths can contain filenames, IDs or share tokens; treat stored traces accordingly.
|
||||
Span names still use route templates when available to avoid high cardinality.
|
||||
Spans do **not** capture payloads, query strings, full URLs, cookies, authorization
|
||||
headers or exception text. WebSockets and ASGI lifespan messages pass through unchanged.
|
||||
This integration exports **traces only**, not OTel metrics or logs; existing
|
||||
wide-event logging remains independent. The SDK shuts down its exporter off the
|
||||
event loop during application shutdown. Configure the collector/network outside
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
"""HTTP request -> Convex client spans. No payload/header/URL capture."""
|
||||
"""HTTP and Convex spans with request paths, but no query/header/payload capture."""
|
||||
import asyncio
|
||||
import os
|
||||
|
||||
@@ -32,7 +32,12 @@ class RequestTracingMiddleware:
|
||||
f"HTTP {method}",
|
||||
context=_PROPAGATOR.extract(carrier),
|
||||
kind=SpanKind.SERVER,
|
||||
attributes={"http.request.method": method},
|
||||
attributes={
|
||||
"http.request.method": method,
|
||||
# ASGI path excludes the query string. Capture it before routing so
|
||||
# unmatched requests and early middleware responses remain identifiable.
|
||||
"url.path": scope.get("path", ""),
|
||||
},
|
||||
record_exception=False,
|
||||
set_status_on_exception=False,
|
||||
) as span:
|
||||
@@ -93,7 +98,7 @@ def register_request_tracing(app, *, provider=None):
|
||||
request_id = event.get("request_id")
|
||||
if request_id:
|
||||
span.set_attribute("request.id", request_id)
|
||||
# Only the route template, not the raw path, query string or user values.
|
||||
# Keep span names low-cardinality; the actual path is in url.path.
|
||||
if request.url_rule is not None:
|
||||
route = request.url_rule.rule
|
||||
span.update_name(f"{request.method} {route}")
|
||||
|
||||
@@ -88,7 +88,7 @@ def test_request_to_worker_to_convex_keeps_trace_and_payload_private(monkeypatch
|
||||
assert events[0]["trace_id"] == "1" * 32
|
||||
exported = " ".join(str((s.name, dict(s.attributes), s.events, s.status.description)) for s in spans)
|
||||
assert "never-export" not in exported
|
||||
assert "private-value" not in exported
|
||||
assert server.attributes["url.path"] == "/items/private-value"
|
||||
assert "token=" not in exported
|
||||
provider.shutdown()
|
||||
|
||||
@@ -163,6 +163,31 @@ def test_retry_attempts_get_unique_ids_and_do_not_export_exception_text(monkeypa
|
||||
assert clients[0].status.description is None
|
||||
provider.shutdown()
|
||||
|
||||
def test_unmatched_request_records_path_without_query_or_headers(monkeypatch):
|
||||
provider, exporter = tracing_provider(monkeypatch)
|
||||
|
||||
async def run():
|
||||
app = Quart(__name__)
|
||||
register_request_tracing(app, provider=provider)
|
||||
result = await app.test_client().get(
|
||||
"/missing/%E2%9C%93?token=never-export",
|
||||
headers={"Authorization": "Bearer never-export", "Cookie": "secret=never-export"},
|
||||
)
|
||||
assert result.status_code == 404
|
||||
|
||||
try:
|
||||
asyncio.run(run())
|
||||
span = exporter.get_finished_spans()[0]
|
||||
assert span.attributes["url.path"] == "/missing/✓"
|
||||
assert span.attributes["http.response.status_code"] == 404
|
||||
assert "http.route" not in span.attributes
|
||||
assert span.name == "HTTP GET"
|
||||
assert "never-export" not in str(dict(span.attributes))
|
||||
assert "url.full" not in span.attributes
|
||||
assert "url.query" not in span.attributes
|
||||
finally:
|
||||
provider.shutdown()
|
||||
|
||||
def test_invalid_incoming_traceparent_is_not_used(monkeypatch):
|
||||
provider, exporter = tracing_provider(monkeypatch)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user