Skip to content
Draft
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
6 changes: 6 additions & 0 deletions sentry_sdk/consts.py
Original file line number Diff line number Diff line change
Expand Up @@ -878,6 +878,12 @@ class SPANDATA:
Example: GET
"""

HTTP_ROUTE = "http.route"
"""
The matched route, that is, the path template used to match the request.
Example: /users/{id}
"""

HTTP_QUERY = "http.query"
"""
The Query string present in the URL.
Expand Down
19 changes: 16 additions & 3 deletions sentry_sdk/integrations/asgi.py
Original file line number Diff line number Diff line change
Expand Up @@ -246,13 +246,19 @@ async def _run_app(
span_ctx: "ContextManager[Union[Span, StreamedSpan, None]]"
if span_streaming:
segment: "Optional[StreamedSpan]" = None
segment_source = getattr(
transaction_source, "value", transaction_source
)
attributes: "Attributes" = {
"sentry.segment.name.source": getattr(
transaction_source, "value", transaction_source
),
"sentry.segment.name.source": segment_source,
"sentry.origin": self.span_origin,
"network.protocol.name": ty,
}
if (
segment_source == SegmentNameSource.ROUTE.value
and transaction_name != _DEFAULT_TRANSACTION_NAME
):
attributes[SPANDATA.HTTP_ROUTE] = transaction_name

if scope.get("client"):
client_options = sentry_sdk.get_client().options
Expand Down Expand Up @@ -412,6 +418,13 @@ async def _sentry_wrapped_send(
span.set_attribute(
"sentry.segment.name.source", source
)
if (
source == SegmentNameSource.ROUTE.value
and name != _DEFAULT_TRANSACTION_NAME

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

scope.py sets http.route on default transaction names without matching guard

The ASGI middleware correctly avoids writing http.route for fallback transaction names, but scope.set_transaction_name() in the same PR lacks the same guard. When FastAPI or Starlette hit a 404 and call scope.set_transaction_name(default, ROUTE), http.route is set to a generic string rather than a route template.

Evidence
  • scope.py set_transaction_name unconditionally sets SPANDATA.HTTP_ROUTE whenever source_value == SegmentNameSource.ROUTE.value, without checking name != _DEFAULT_TRANSACTION_NAME.
  • Both FastAPI and Starlette call scope.set_transaction_name(_DEFAULT_TRANSACTION_NAME, TransactionSource.ROUTE) when no route matches the request.
  • For streaming spans, that causes http.route to be set to values like "generic FastAPI request" in attributes, while this hunk explicitly avoids writing the attribute in the same circumstance.

Identified by Warden · code-review · DS5-DYC

):
span.set_attribute(
SPANDATA.HTTP_ROUTE, name
)
finally:
_asgi_middleware_applied.set(False)

Expand Down
2 changes: 2 additions & 0 deletions sentry_sdk/integrations/django/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -448,6 +448,8 @@ def _set_transaction_name_and_source(
source = TransactionSource.URL
else:
source = SOURCE_FOR_STYLE[transaction_style]
if source == TransactionSource.ROUTE:
scope.set_segment_attribute(SPANDATA.HTTP_ROUTE, transaction_name)

scope.set_transaction_name(
transaction_name,
Expand Down
2 changes: 2 additions & 0 deletions sentry_sdk/integrations/fastapi.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,8 @@ def _set_transaction_name_and_source(
source = TransactionSource.ROUTE
else:
source = SOURCE_FOR_STYLE[transaction_style]
if source == TransactionSource.ROUTE:
scope.set_segment_attribute(SPANDATA.HTTP_ROUTE, name)

scope.set_transaction_name(name, source=source)

Expand Down
12 changes: 7 additions & 5 deletions sentry_sdk/integrations/flask.py
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
from typing import TYPE_CHECKING

import sentry_sdk
from sentry_sdk.consts import SPANDATA
from sentry_sdk.integrations import DidNotEnable, Integration, _check_minimum_version
from sentry_sdk.integrations._wsgi_common import (
DEFAULT_HTTP_METHODS_TO_CAPTURE,
RequestExtractor,
)
from sentry_sdk.integrations.wsgi import SentryWsgiMiddleware
from sentry_sdk.scope import should_send_default_pii
from sentry_sdk.tracing import SOURCE_FOR_STYLE
from sentry_sdk.tracing import SOURCE_FOR_STYLE, TransactionSource
from sentry_sdk.utils import (
capture_internal_exceptions,
ensure_integration_enabled,
Expand Down Expand Up @@ -134,10 +135,11 @@ def _set_transaction_name_and_source(
"url": request.url_rule.rule,
"endpoint": request.url_rule.endpoint,
}
scope.set_transaction_name(
name_for_style[transaction_style],
source=SOURCE_FOR_STYLE[transaction_style],
)
name = name_for_style[transaction_style]
source = SOURCE_FOR_STYLE[transaction_style]
if source == TransactionSource.ROUTE:
scope.set_segment_attribute(SPANDATA.HTTP_ROUTE, name)
scope.set_transaction_name(name, source=source)
except Exception:
pass

Expand Down
2 changes: 2 additions & 0 deletions sentry_sdk/integrations/starlette.py
Original file line number Diff line number Diff line change
Expand Up @@ -861,6 +861,8 @@ def _set_transaction_name_and_source(
if name is None:
name = _DEFAULT_TRANSACTION_NAME
source = TransactionSource.ROUTE
elif source == TransactionSource.ROUTE:
scope.set_segment_attribute(SPANDATA.HTTP_ROUTE, name)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Starlette sync skips http.route

Medium Severity

set_segment_attribute only writes http.route when the given scope already holds a StreamedSpan. Starlette's sync handler calls _set_transaction_name_and_source with the isolation scope, whose _span is unset, so the attribute is silently dropped for sync endpoints. Async handlers use the current scope and are unaffected.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit b1363a5. Configure here.


scope.set_transaction_name(name, source=source)

Expand Down
11 changes: 11 additions & 0 deletions sentry_sdk/scope.py
Original file line number Diff line number Diff line change
Expand Up @@ -2053,6 +2053,17 @@ def set_attributes(self, attributes: "dict[str, AttributeValue]") -> None:
for attribute, value in attributes.items():
self.set_attribute(attribute, value)

def set_segment_attribute(self, key: str, value: "AttributeValue") -> None:
"""
Set an attribute on the active segment (the root span of the trace).

Unlike :py:meth:`set_attribute`, which applies to all telemetry captured
while the scope is active, this sets the attribute on the segment span
only. It has no effect outside of span streaming mode.
"""
if isinstance(self._span, StreamedSpan):
self._span._segment.set_attribute(key, value)

def remove_attribute(self, attribute: str) -> None:
"""Remove an attribute if set on the scope. No-op if there is no such attribute."""
try:
Expand Down
35 changes: 35 additions & 0 deletions tests/integrations/asgi/test_asgi.py
Original file line number Diff line number Diff line change
Expand Up @@ -670,6 +670,7 @@ async def test_transaction_style(

assert span["name"] == expected_transaction
assert span["attributes"]["sentry.segment.name.source"] == expected_source
assert "http.route" not in span["attributes"]

else:
(transaction_event,) = events
Expand All @@ -678,6 +679,40 @@ async def test_transaction_style(
assert transaction_event["transaction_info"] == {"source": expected_source}


@pytest.mark.asyncio
async def test_http_route_set_for_route_segment_name(
sentry_init,
asgi3_app,
capture_items,
):
sentry_init(
traces_sample_rate=1.0,
trace_lifecycle="stream",
)
app = SentryAsgiMiddleware(asgi3_app, transaction_style="url")

class Route:
path = "/message/{message_id}"

scope = {
"endpoint": asgi3_app,
"route": Route(),
"client": ("127.0.0.1", 60457),
}

async with TestClient(app, scope=scope) as client:
items = capture_items("span")
await client.get("/message/123456")

sentry_sdk.flush()

assert len(items) == 1
span = items[0].payload
assert span["name"] == "/message/{message_id}"
assert span["attributes"]["sentry.segment.name.source"] == "route"
assert span["attributes"]["http.route"] == "/message/{message_id}"


def mock_asgi2_app():
pass

Expand Down
4 changes: 4 additions & 0 deletions tests/integrations/django/test_basic.py
Original file line number Diff line number Diff line change
Expand Up @@ -1276,6 +1276,10 @@ def test_transaction_style(

assert spans[2]["is_segment"] is True
assert spans[2]["attributes"]["sentry.segment.name.source"] == expected_source
if expected_source == "route":
assert spans[2]["attributes"]["http.route"] == expected_transaction
else:
assert "http.route" not in spans[2]["attributes"]

(event,) = (item.payload for item in items if item.type == "event")
else:
Expand Down
1 change: 1 addition & 0 deletions tests/integrations/fastapi/test_fastapi.py
Original file line number Diff line number Diff line change
Expand Up @@ -770,6 +770,7 @@ async def get_user(user_id: int):
segment = segments[0]
assert segment["name"] == "/api/users/{user_id}"
assert segment["attributes"]["sentry.segment.name.source"] == "route"
assert segment["attributes"]["http.route"] == "/api/users/{user_id}"
else:
(transaction_envelope,) = envelopes
transaction_event = transaction_envelope.get_transaction_event()
Expand Down
4 changes: 4 additions & 0 deletions tests/integrations/flask/test_flask.py
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,10 @@ def test_transaction_or_segment_style(
(segment,) = spans
assert segment["name"] == expected_transaction
assert segment["attributes"]["sentry.segment.name.source"] == expected_source
if expected_source == "route":
assert segment["attributes"]["http.route"] == expected_transaction
else:
assert "http.route" not in segment["attributes"]
else:
(_, event) = events
assert event["transaction"] == expected_transaction
Expand Down
24 changes: 24 additions & 0 deletions tests/tracing/test_span_streaming.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,30 @@ def test_start_span(sentry_init, capture_items):
assert segment["status"] == "ok"


def test_set_segment_attribute(sentry_init, capture_items):
sentry_init(
traces_sample_rate=1.0,
trace_lifecycle="stream",
)

items = capture_items("span")

with sentry_sdk.traces.start_span(name="segment"):
with sentry_sdk.traces.start_span(name="child"):
# Set from within a child span to prove it targets the segment,
# not the active span.
sentry_sdk.get_current_scope().set_segment_attribute(
"http.route", "/users/{id}"
)

sentry_sdk.get_client().flush()
spans = {item.payload["name"]: item.payload for item in items}

assert spans["segment"]["attributes"]["http.route"] == "/users/{id}"
# Unlike scope-wide set_attribute, the child span must not inherit it.
assert "http.route" not in spans["child"]["attributes"]


def test_start_span_no_context_manager(sentry_init, capture_items):
sentry_init(
traces_sample_rate=1.0,
Expand Down
Loading