Skip to content
Draft
Show file tree
Hide file tree
Changes from 8 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
29 changes: 29 additions & 0 deletions sentry_sdk/integrations/_asgi_common.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
from typing import Union
from typing_extensions import Literal

from sentry_sdk._types import Attributes
from sentry_sdk.utils import AnnotatedValue


Expand Down Expand Up @@ -105,3 +106,31 @@
request_data["env"] = {"REMOTE_ADDR": _get_ip(asgi_scope)}

return request_data


def _get_request_attributes(asgi_scope: "Any") -> "dict[str, Any]":
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an attributes based copy of _get_request_data just above

"""
Return attributes related to the HTTP request from the ASGI scope.
"""
attributes: "Attributes" = {}

ty = asgi_scope["type"]
if ty in ("http", "websocket"):
if asgi_scope.get("method"):
attributes["http.request.method"] = asgi_scope["method"].upper()

headers = _filter_headers(_get_headers(asgi_scope))
for header, value in headers.items():
attributes[f"http.request.header.{header.lower()}"] = value

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

View check run for this annotation

@sentry/warden / warden: code-review

AnnotatedValue objects from sensitive headers not properly handled in attributes

When `should_send_default_pii()` returns False, `_filter_headers()` returns `AnnotatedValue` objects for sensitive headers (like `authorization`, `cookie`, etc.). The new `_get_request_attributes()` function directly assigns these values to attributes. When `set_attribute()` processes these, `format_attribute()` will fall back to `safe_repr()`, producing unhelpful string representations like `<AnnotatedValue object at 0x...>` instead of properly filtered values. While no actual PII leaks (the original sensitive data is already stripped), the resulting attribute values are not meaningful.

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

View check run for this annotation

@sentry/warden / warden: find-bugs

Sensitive headers are improperly serialized as AnnotatedValue string representation instead of filtered values

When `should_send_default_pii()` returns False, `_filter_headers()` replaces sensitive headers (like authorization, cookies, x-forwarded-for) with `AnnotatedValue` objects. In `_get_request_attributes()`, these `AnnotatedValue` objects are passed directly to attributes. When `scope.set_attribute()` processes them via `format_attribute()`, they fall through to `safe_repr()`, resulting in string representations like "{'value': '[Filtered]', 'metadata': ...}" instead of the intended filtered value. This causes ugly, confusing attribute values and may leak metadata information.

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

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

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

return attributes
60 changes: 58 additions & 2 deletions sentry_sdk/integrations/asgi.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
from sentry_sdk.consts import OP
from sentry_sdk.integrations._asgi_common import (
_get_headers,
_get_request_attributes,
_get_request_data,
_get_url,
)
Expand All @@ -23,7 +24,11 @@
nullcontext,
)
from sentry_sdk.sessions import track_session
from sentry_sdk.traces import StreamedSpan
from sentry_sdk.traces import (
StreamedSpan,
SegmentSource,
SOURCE_FOR_STYLE as SEGMENT_SOURCE_FOR_STYLE,
)
from sentry_sdk.tracing import (
SOURCE_FOR_STYLE,
Transaction,
Expand All @@ -40,6 +45,7 @@
_get_installed_modules,
reraise,
capture_internal_exceptions,
qualname_from_function,
)

from typing import TYPE_CHECKING
Expand Down Expand Up @@ -235,7 +241,7 @@ async def _run_app(
transaction_source, "value", transaction_source
),
"sentry.origin": self.span_origin,
"asgi.type": ty,
"network.protocol.name": ty,
}

if ty in ("http", "websocket"):
Expand Down Expand Up @@ -301,6 +307,9 @@ async def _run_app(
else nullcontext()
)

for attribute, value in _get_request_attributes(scope).items():
sentry_scope.set_attribute(attribute, value)

with span_ctx as span:
try:

Expand Down Expand Up @@ -336,6 +345,7 @@ async def _sentry_wrapped_send(
return await self.app(
scope, receive, _sentry_wrapped_send
)

except Exception as exc:
suppress_chained_exceptions = (
sentry_sdk.get_client()
Expand All @@ -350,6 +360,15 @@ async def _sentry_wrapped_send(
with capture_internal_exceptions():
self._capture_request_exception(exc)
reraise(*exc_info)

finally:
with capture_internal_exceptions():
name, source = self._get_segment_name_and_source(
self.transaction_style, scope
)
if isinstance(span, StreamedSpan):
span.name = name
span.set_attribute("sentry.span.source", source)
finally:
_asgi_middleware_applied.set(False)

Expand Down Expand Up @@ -424,3 +443,40 @@ def _get_transaction_name_and_source(
return name, source

return name, source

def _get_segment_name_and_source(
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a copy of _get_transaction_name_and_source above, just adapted for segments

self: "SentryAsgiMiddleware", segment_style: str, asgi_scope: "Any"
) -> "Tuple[str, str]":
name = None
source = SEGMENT_SOURCE_FOR_STYLE[segment_style].value
ty = asgi_scope.get("type")

if segment_style == "endpoint":
endpoint = asgi_scope.get("endpoint")
# Webframeworks like Starlette mutate the ASGI env once routing is
# done, which is sometime after the request has started. If we have
# an endpoint, overwrite our generic transaction name.
if endpoint:
name = qualname_from_function(endpoint) or ""
else:
name = _get_url(asgi_scope, "http" if ty == "http" else "ws", host=None)
source = SegmentSource.URL.value

elif segment_style == "url":
# FastAPI includes the route object in the scope to let Sentry extract the
# path from it for the transaction name
route = asgi_scope.get("route")
if route:
path = getattr(route, "path", None)
if path is not None:
name = path
else:
name = _get_url(asgi_scope, "http" if ty == "http" else "ws", host=None)
source = SegmentSource.URL.value

if name is None:
name = _DEFAULT_TRANSACTION_NAME
source = SegmentSource.ROUTE.value
return name, source

return name, source
65 changes: 65 additions & 0 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import brotli
import gzip
import io
from dataclasses import dataclass
from threading import Thread
from contextlib import contextmanager
from http.server import BaseHTTPRequestHandler, HTTPServer
Expand Down Expand Up @@ -320,6 +321,47 @@
return inner


@dataclass
class UnwrappedItem:
type: str
payload: dict


@pytest.fixture
def capture_items(monkeypatch):
"""Capture envelope payload, unfurling individual items."""

def inner(types=None):
telemetry = []
test_client = sentry_sdk.get_client()
old_capture_envelope = test_client.transport.capture_envelope

def append_envelope(envelope):
for item in envelope:
if types is None or item.type not in types:
continue

Check failure on line 342 in tests/conftest.py

View check run for this annotation

@sentry/warden / warden: code-review

Inverted filter logic causes all items to be skipped when types=None

The condition `if types is None or item.type not in types: continue` has inverted logic. When `types` is `None` (intended to mean 'capture all types'), the first condition `types is None` evaluates to `True`, causing `continue` to execute and skip ALL items. This defeats the purpose of having a default behavior that captures everything.

Check warning on line 342 in tests/conftest.py

View check run for this annotation

@sentry/warden / warden: find-bugs

Filter logic in capture_items fixture skips all items when types is None

The condition `if types is None or item.type not in types` is inverted. When `types=None`, all items should be captured (no filtering), but the current code skips ALL items because `types is None` evaluates to `True`. The correct logic should be `if types is not None and item.type not in types`. While the current tests always pass a `types` argument, any future test calling `capture_items()` without arguments will get an empty list.

if item.type in ("metric", "log", "span"):
for json in item.payload.json["items"]:
t = {k: v for k, v in json.items() if k != "attributes"}
t["attributes"] = {
k: v["value"] for k, v in json["attributes"].items()
}
telemetry.append(UnwrappedItem(type=item.type, payload=t))
else:
telemetry.append(
UnwrappedItem(type=item.type, payload=item.payload.json)
)

return old_capture_envelope(envelope)

monkeypatch.setattr(test_client.transport, "capture_envelope", append_envelope)

return telemetry

return inner


@pytest.fixture
def capture_record_lost_event_calls(monkeypatch):
def inner():
Expand Down Expand Up @@ -1212,6 +1254,29 @@
client.set_cookie(key, value)


def envelopes_to_spans(envelopes):
res: "list[dict[str, Any]]" = []
for envelope in envelopes:
for item in envelope.items:
if item.type == "span":
for span_json in item.payload.json["items"]:
span = {
"start_timestamp": span_json["start_timestamp"],
"end_timestamp": span_json.get("end_timestamp"),
"trace_id": span_json["trace_id"],
"span_id": span_json["span_id"],
"name": span_json["name"],
"status": span_json["status"],
"is_segment": span_json["is_segment"],
"parent_span_id": span_json.get("parent_span_id"),
"attributes": {
k: v["value"] for (k, v) in span_json["attributes"].items()
},

Check warning on line 1274 in tests/conftest.py

View check run for this annotation

@sentry/warden / warden: code-review

KeyError when span has no attributes

The `envelopes_to_spans` helper accesses `span_json["attributes"]` directly, but in `_span_batcher.py:116-119`, the `attributes` key is only added to the span JSON when `item._attributes` is truthy. If a span has no attributes, this will raise a `KeyError`. This would cause tests to fail when processing spans without attributes.
}
res.append(span)
return res


@contextmanager
def patch_start_tracing_child(
fake_transaction_is_none: bool = False,
Expand Down
Loading
Loading