Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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
9 changes: 5 additions & 4 deletions sentry_sdk/integrations/_asgi_common.py
Original file line number Diff line number Diff line change
Expand Up @@ -121,17 +121,18 @@
for header, value in headers.items():
attributes[f"http.request.header.{header.lower()}"] = value

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

attributes["url.full"] = _get_url(

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

View check run for this annotation

@sentry/warden / warden: find-bugs

`url.full` accidentally gated behind `should_send_default_pii()`

`url.full` is now only collected when PII is enabled, but `_get_url` explicitly does not include the query string (per its docstring: "without also including the querystring"), so it is not PII and should always be collected like other standard span attributes.
asgi_scope, "http" if ty == "http" else "ws", headers.get("host")

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

View check run for this annotation

@sentry/warden / warden: code-review

`url.full` accidentally gated behind `should_send_default_pii()`, missing in default config

`url.full` is not PII (it's the URL path, not the query string), but it's now inside the `if should_send_default_pii():` block, so it will never be set when PII is disabled — breaking span URL attribution in the default configuration.
)

client = asgi_scope.get("client")
if client and should_send_default_pii():
ip = _get_ip(asgi_scope)
attributes["client.address"] = ip
client = asgi_scope.get("client")

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

View check run for this annotation

@sentry/warden / warden: code-review

`NameError` when non-http/websocket ASGI scope is processed with PII enabled

When `asgi_scope['type']` is not `'http'` or `'websocket'` (e.g. `'lifespan'`) and `should_send_default_pii()` is `True`, `headers` is undefined at `headers.get('host')` because it is only assigned inside the `if ty in ('http', 'websocket'):` block.
Comment thread
ericapisani marked this conversation as resolved.
Outdated
if client:
ip = _get_ip(asgi_scope)
attributes["client.address"] = ip

return attributes
Comment thread
ericapisani marked this conversation as resolved.
Outdated
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