Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 8 additions & 7 deletions sentry_sdk/integrations/_asgi_common.py
Original file line number Diff line number Diff line change
Expand Up @@ -121,13 +121,14 @@
for header, value in headers.items():
attributes[f"http.request.header.{header.lower()}"] = value

query = _get_query(asgi_scope)
if query:
attributes["http.query"] = query

attributes["url.full"] = _get_url(
asgi_scope, "http" if ty == "http" else "ws", headers.get("host")
)
if should_send_default_pii():
query = _get_query(asgi_scope)
if query:
attributes["http.query"] = query

attributes["url.full"] = _get_url(
asgi_scope, "http" if ty == "http" else "ws", headers.get("host")
)

Check failure on line 131 in sentry_sdk/integrations/_asgi_common.py

View check run for this annotation

@sentry/warden / warden: code-review

`url.full` incorrectly gated behind `should_send_default_pii()`, suppressing it for all default users

`url.full` is placed inside the `should_send_default_pii()` block alongside `http.query`, but `_get_url()` explicitly strips the query string (see its docstring: "without also including the querystring"), so the base URL is not PII and should always be captured. This causes `url.full` to be absent from all spans when PII is disabled, breaking standard OTel HTTP server span semantics for the majority of users.

Check warning on line 131 in sentry_sdk/integrations/_asgi_common.py

View check run for this annotation

@sentry/warden / warden: find-bugs

`url.full` (URL without query string) incorrectly gated behind `should_send_default_pii()`

`url.full` is the base URL path with no query string — `_get_url()` docstring says it builds the URL "without also including the querystring" — so gating it behind `should_send_default_pii()` silently drops the request URL from all ASGI traces when PII is disabled, breaking basic HTTP observability.

client = asgi_scope.get("client")
if client and should_send_default_pii():
Expand Down
13 changes: 10 additions & 3 deletions tests/integrations/asgi/test_asgi.py
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,10 @@ def test_invalid_transaction_style(asgi3_app):


@pytest.mark.asyncio
@pytest.mark.parametrize(
"should_send_pii",
[True, False],
)
@pytest.mark.parametrize(
"span_streaming",
[True, False],
Expand All @@ -174,9 +178,10 @@ async def test_capture_transaction(
capture_events,
capture_items,
span_streaming,
should_send_pii,
):
sentry_init(
send_default_pii=True,
send_default_pii=should_send_pii,
traces_sample_rate=1.0,
_experiments={
"trace_lifecycle": "stream" if span_streaming else "static",
Expand All @@ -203,16 +208,18 @@ async def test_capture_transaction(
assert span["attributes"]["sentry.span.source"] == "url"
assert span["attributes"]["sentry.op"] == "http.server"

assert span["attributes"]["url.full"] == "http://localhost/some_url"
assert span["attributes"]["network.protocol.name"] == "http"
assert span["attributes"]["http.request.method"] == "GET"
assert span["attributes"]["http.query"] == "somevalue=123"
assert span["attributes"]["http.request.header.host"] == "localhost"
assert span["attributes"]["http.request.header.remote-addr"] == "127.0.0.1"
assert (
span["attributes"]["http.request.header.user-agent"] == "ASGI-Test-Client"
)

if should_send_pii:
assert span["attributes"]["url.full"] == "http://localhost/some_url"
assert span["attributes"]["http.query"] == "somevalue=123"

else:
(transaction_event,) = events

Comment thread
ericapisani marked this conversation as resolved.
Expand Down
Loading