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
25 changes: 20 additions & 5 deletions sentry_sdk/integrations/_asgi_common.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,19 @@ def _get_url(
asgi_scope: "Dict[str, Any]",
default_scheme: "Literal['ws', 'http']",
host: "Optional[Union[AnnotatedValue, str]]",
path_includes_root_path: "bool" = True,
) -> str:
"""
Extract URL from the ASGI scope, without also including the querystring.
"""
scheme = asgi_scope.get("scheme", default_scheme)

server = asgi_scope.get("server", None)
path = asgi_scope.get("root_path", "") + asgi_scope.get("path", "")
path = (
asgi_scope.get("path", "")
if path_includes_root_path
else asgi_scope.get("root_path", "") + asgi_scope.get("path", "")
Comment thread
sentry-warden[bot] marked this conversation as resolved.
)

if host:
return "%s://%s%s" % (scheme, host, path)
Expand Down Expand Up @@ -81,7 +86,9 @@ def _get_ip(asgi_scope: "Any") -> str:
return asgi_scope.get("client")[0]


def _get_request_data(asgi_scope: "Any") -> "Dict[str, Any]":
def _get_request_data(
asgi_scope: "Any", path_includes_root_path: "bool" = True
) -> "Dict[str, Any]":
"""
Returns data related to the HTTP request from the ASGI scope.
"""
Expand All @@ -96,7 +103,10 @@ def _get_request_data(asgi_scope: "Any") -> "Dict[str, Any]":
request_data["query_string"] = _get_query(asgi_scope)

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

client = asgi_scope.get("client")
Expand All @@ -106,7 +116,9 @@ def _get_request_data(asgi_scope: "Any") -> "Dict[str, Any]":
return request_data


def _get_request_attributes(asgi_scope: "Any") -> "dict[str, Any]":
def _get_request_attributes(
asgi_scope: "Any", path_includes_root_path: "bool" = True
) -> "dict[str, Any]":
"""
Return attributes related to the HTTP request from the ASGI scope.
"""
Expand All @@ -127,7 +139,10 @@ def _get_request_attributes(asgi_scope: "Any") -> "dict[str, Any]":
attributes["http.query"] = query

url_without_query_string = _get_url(
asgi_scope, "http" if ty == "http" else "ws", headers.get("host")
asgi_scope,
"http" if ty == "http" else "ws",
headers.get("host"),
path_includes_root_path=path_includes_root_path,
)
query_string = _get_query(asgi_scope)
attributes["url.full"] = (
Expand Down
40 changes: 34 additions & 6 deletions sentry_sdk/integrations/asgi.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ class SentryAsgiMiddleware:
"mechanism_type",
"span_origin",
"http_methods_to_capture",
"path_includes_root_path",
)

def __init__(
Expand All @@ -116,6 +117,7 @@ def __init__(
span_origin: str = "manual",
http_methods_to_capture: "Tuple[str, ...]" = DEFAULT_HTTP_METHODS_TO_CAPTURE,
asgi_version: "Optional[int]" = None,
path_includes_root_path: bool = True,
Comment thread
sentry-warden[bot] marked this conversation as resolved.
Comment thread
cursor[bot] marked this conversation as resolved.
Comment thread
alexander-alderman-webb marked this conversation as resolved.
) -> None:
"""
Instrument an ASGI application with Sentry. Provides HTTP/websocket
Expand Down Expand Up @@ -152,6 +154,7 @@ def __init__(
self.span_origin = span_origin
self.app = app
self.http_methods_to_capture = http_methods_to_capture
self.path_includes_root_path = path_includes_root_path

if asgi_version is None:
if _looks_like_asgi3(app):
Expand Down Expand Up @@ -319,7 +322,8 @@ async def _run_app(
with span_ctx as span:
if isinstance(span, StreamedSpan):
for attribute, value in _get_request_attributes(
scope
scope,
path_includes_root_path=self.path_includes_root_path,
).items():
span.set_attribute(attribute, value)

Expand Down Expand Up @@ -401,7 +405,11 @@ def event_processor(
self, event: "Event", hint: "Hint", asgi_scope: "Any"
) -> "Optional[Event]":
request_data = event.get("request", {})
request_data.update(_get_request_data(asgi_scope))
request_data.update(
_get_request_data(
asgi_scope, path_includes_root_path=self.path_includes_root_path
)
)
event["request"] = deepcopy(request_data)

# Only set transaction name if not already set by Starlette or FastAPI (or other frameworks)
Expand Down Expand Up @@ -447,7 +455,12 @@ def _get_transaction_name_and_source(
if endpoint:
name = transaction_from_function(endpoint) or ""
else:
name = _get_url(asgi_scope, "http" if ty == "http" else "ws", host=None)
name = _get_url(
asgi_scope,
"http" if ty == "http" else "ws",
host=None,
path_includes_root_path=self.path_includes_root_path,
)
source = TransactionSource.URL

elif transaction_style == "url":
Expand All @@ -459,7 +472,12 @@ def _get_transaction_name_and_source(
if path is not None:
name = path
else:
name = _get_url(asgi_scope, "http" if ty == "http" else "ws", host=None)
name = _get_url(
asgi_scope,
"http" if ty == "http" else "ws",
host=None,
path_includes_root_path=self.path_includes_root_path,
)
source = TransactionSource.URL

if name is None:
Expand All @@ -484,7 +502,12 @@ def _get_segment_name_and_source(
if endpoint:
name = qualname_from_function(endpoint) or ""
else:
name = _get_url(asgi_scope, "http" if ty == "http" else "ws", host=None)
name = _get_url(
asgi_scope,
"http" if ty == "http" else "ws",
host=None,
path_includes_root_path=self.path_includes_root_path,
)
source = SegmentSource.URL.value

elif segment_style == "url":
Expand All @@ -496,7 +519,12 @@ def _get_segment_name_and_source(
if path is not None:
name = path
else:
name = _get_url(asgi_scope, "http" if ty == "http" else "ws", host=None)
name = _get_url(
asgi_scope,
"http" if ty == "http" else "ws",
host=None,
path_includes_root_path=self.path_includes_root_path,
)
source = SegmentSource.URL.value

if name is None:
Expand Down
5 changes: 5 additions & 0 deletions sentry_sdk/integrations/django/asgi.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import inspect
from typing import TYPE_CHECKING

from django import VERSION as DJANGO_VERSION
from django.core.handlers.wsgi import WSGIRequest

import sentry_sdk
Expand Down Expand Up @@ -96,6 +97,10 @@
unsafe_context_data=True,
span_origin=DjangoIntegration.origin,
http_methods_to_capture=integration.http_methods_to_capture,
# From Django 5.1 onwards, ASGI request.path is taken directly from scope["path"] without prepending scope["root_path"].
# Assume that scope["path"] includes scope["root_path"] for these versions, as otherwise request.path is also incorrect.
# https://github.com/django/django/commit/bcd255cd5ca0a1e686d276cca71f45ec400d84a2
path_includes_root_path=DJANGO_VERSION >= (5, 1),

Check warning on line 103 in sentry_sdk/integrations/django/asgi.py

View check run for this annotation

@sentry/warden / warden: code-review

[8KA-TNT] test_request_url fails on Django 3.0–5.0 due to version-unaware path assertion (additional location)

The test passes `scope["path"] = "/root/nomessage"` together with `scope["root_path"] = "/root"`, but for Django < 5.1 the SDK uses `path_includes_root_path=False`, which prepends `root_path` to `path`, yielding `"/root/root/nomessage"` — not the asserted `"/root/nomessage"`. Either restrict the test to `django.VERSION >= (5, 1)` or use path `"/nomessage"` (without the root prefix) so that `root_path + path` gives the expected URL on all versions.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Channels middleware missing path flag

Medium Severity

The Channels (<3) SentryAsgiMiddleware still relies on the new default path_includes_root_path=True, while the main Django ASGI handler sets it from DJANGO_VERSION. On Django before 5.1, mounted root_path can be omitted from reported request URLs and span attributes.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit a96d284. Configure here.

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.

test_request_url fails on Django < 5.1 due to incorrect root_path setup

The new test_request_url test sets scope["path"] = "/root/nomessage" with scope["root_path"] = "/root" (path already includes root_path), but only skips for Django < 3.0. On Django < 5.1, path_includes_root_path=False causes the URL to be constructed as root_path + path = "/root/root/nomessage", which will fail the assertion == "/root/nomessage". Consider adding @pytest.mark.skipif(django.VERSION < (5, 1), ...) or adding a separate test case for the pre-5.1 path format.

Evidence
  • Line 103 sets path_includes_root_path=DJANGO_VERSION >= (5, 1), so Django < 5.1 gets False.
  • _get_url with path_includes_root_path=False constructs URL as root_path + path (/root + /root/nomessage = /root/root/nomessage).
  • tests/integrations/django/asgi/test_asgi.py:1078 sets comm.scope["root_path"] = "/root" with request path /root/nomessage — a Django 5.1+ style where path already contains root_path.
  • The test asserts url.full == "/root/nomessage" but has no skipif django.VERSION < (5, 1) guard, only skipif django.VERSION < (3, 0).
  • Test would fail on Django 3.x–4.x where the code path produces /root/root/nomessage.
Also found at 1 additional location
  • tests/integrations/django/asgi/test_asgi.py:1093-1101

Identified by Warden code-review · GNZ-9Q4

)._run_asgi3

return await middleware(scope, receive, send)
Expand Down
5 changes: 5 additions & 0 deletions sentry_sdk/integrations/litestar.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,11 @@ def __init__(
mechanism_type="asgi",
span_origin=span_origin,
asgi_version=3,
# Unlike Starlette, LiteStar does not extend scope["root_path"] with the mount path.
# Since LiteStar handles servers that include and do not include scope["root_path"] in scope["path"]
# with the commit below, keep the existing behavior for compatibility.
# https://github.com/litestar-org/litestar/commit/72dda171768bd470adc065c47c1ecf1d80b5e749
path_includes_root_path=False,
)

def _capture_request_exception(self, exc: Exception) -> None:
Expand Down
12 changes: 11 additions & 1 deletion sentry_sdk/integrations/quart.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
capture_internal_exceptions,
ensure_integration_enabled,
event_from_exception,
package_version,
)

if TYPE_CHECKING:
Expand Down Expand Up @@ -86,16 +87,25 @@
def patch_asgi_app() -> None:
old_app = Quart.__call__

version = package_version("quart")

async def sentry_patched_asgi_app(
self: "Any", scope: "Any", receive: "Any", send: "Any"
) -> "Any":
if sentry_sdk.get_client().get_integration(QuartIntegration) is None:
if (
sentry_sdk.get_client().get_integration(QuartIntegration) is None
or version is None
):

Check warning on line 98 in sentry_sdk/integrations/quart.py

View check run for this annotation

@sentry/warden / warden: code-review

Quart integration silently drops all monitoring when `package_version()` returns None

If `package_version("quart")` returns `None` (e.g. package metadata is unavailable in a bundled/frozen app), the new `or version is None` guard causes the patched ASGI app to silently bypass the Sentry middleware entirely, losing all request monitoring. Consider falling back to a safe default (e.g. `path_includes_root_path=False`) instead of skipping the middleware.
Comment thread
alexander-alderman-webb marked this conversation as resolved.
Comment thread
alexander-alderman-webb marked this conversation as resolved.
Comment thread
alexander-alderman-webb marked this conversation as resolved.
return await old_app(self, scope, receive, send)

middleware = SentryAsgiMiddleware(
lambda *a, **kw: old_app(self, *a, **kw),
span_origin=QuartIntegration.origin,
asgi_version=3,
# Starting with the commit below, Quart treats any scope["path"]
# that does not include scope["root_path"] as invalid.
# https://github.com/pallets/quart/commit/7be545c
path_includes_root_path=version >= (0, 19),
)
return await middleware(scope, receive, send)

Expand Down
8 changes: 6 additions & 2 deletions sentry_sdk/integrations/starlette.py
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,10 @@ def setup_once() -> None:
)

patch_middlewares()
patch_asgi_app()
# Starlette's Mount includes scope["root_path"] in scope["path"] starting with:
# https://github.com/Kludex/starlette/commit/e8f0dcd54e4ceec47e02c45f5275374e292339ad.
path_includes_root_path = version >= (0, 33)
patch_asgi_app(path_includes_root_path=path_includes_root_path)
patch_request_response()

if version >= (0, 24):
Expand Down Expand Up @@ -427,7 +430,7 @@ def _sentry_middleware_init(
Middleware.__init__ = _sentry_middleware_init


def patch_asgi_app() -> None:
def patch_asgi_app(path_includes_root_path: "bool") -> None:
"""
Instrument Starlette ASGI app using the SentryAsgiMiddleware.
"""
Expand All @@ -451,6 +454,7 @@ async def _sentry_patched_asgi_app(
else DEFAULT_HTTP_METHODS_TO_CAPTURE
),
asgi_version=3,
path_includes_root_path=path_includes_root_path,
)

return await middleware(scope, receive, send)
Expand Down
1 change: 1 addition & 0 deletions sentry_sdk/integrations/starlite.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ def __init__(
mechanism_type="asgi",
span_origin=span_origin,
asgi_version=3,
path_includes_root_path=False,
)


Expand Down
50 changes: 50 additions & 0 deletions tests/integrations/django/asgi/test_asgi.py
Original file line number Diff line number Diff line change
Expand Up @@ -1049,3 +1049,53 @@
(event1, event2) = events
assert event1["request"]["method"] == "OPTIONS"
assert event2["request"]["method"] == "HEAD"


@pytest.mark.parametrize("application", APPS)
@pytest.mark.asyncio
@pytest.mark.skipif(
django.VERSION < (3, 0), reason="Django ASGI support shipped in 3.0"
)
@pytest.mark.parametrize("span_streaming", [True, False])
async def test_request_url(
sentry_init,
capture_events,
capture_items,
application,
span_streaming,
):
sentry_init(
integrations=[DjangoIntegration()],
traces_sample_rate=1.0,
send_default_pii=True,
_experiments={"trace_lifecycle": "stream" if span_streaming else "static"},
)
comm = HttpCommunicator(
application,
"GET",
"/root/nomessage",
)
comm.scope["root_path"] = "/root"

if span_streaming:
items = capture_items("span")
await comm.get_response()
await comm.wait()

sentry_sdk.flush()
spans = [item.payload for item in items]

(server_span,) = (
span
for span in spans
if span["attributes"].get("sentry.op") == "http.server"
)
assert server_span["attributes"]["url.full"] == ("/root/nomessage")
else:
events = capture_events()

await comm.get_response()
await comm.wait()

(event,) = events
assert event["request"]["url"] == "/root/nomessage"

Check warning on line 1101 in tests/integrations/django/asgi/test_asgi.py

View check run for this annotation

@sentry/warden / warden: code-review

test_request_url fails on Django 3.0–5.0 due to version-unaware path assertion

The test passes `scope["path"] = "/root/nomessage"` together with `scope["root_path"] = "/root"`, but for Django &lt; 5.1 the SDK uses `path_includes_root_path=False`, which prepends `root_path` to `path`, yielding `"/root/root/nomessage"` — not the asserted `"/root/nomessage"`. Either restrict the test to `django.VERSION >= (5, 1)` or use path `"/nomessage"` (without the root prefix) so that `root_path + path` gives the expected URL on all versions.
Comment thread
alexander-alderman-webb marked this conversation as resolved.
45 changes: 45 additions & 0 deletions tests/integrations/fastapi/test_fastapi.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@

def fastapi_app_factory():
app = FastAPI()
mounted_app = FastAPI()

@app.get("/error")
async def _error():
Expand All @@ -74,6 +75,7 @@ async def _message():
capture_message("Hi")
return {"message": "Hi"}

@mounted_app.get("/nomessage")
@app.delete("/nomessage")
@app.get("/nomessage")
@app.head("/nomessage")
Expand Down Expand Up @@ -118,6 +120,8 @@ async def body_form(
capture_message("hi")
return {"status": "ok"}

app.mount("/root", mounted_app)

return app


Expand Down Expand Up @@ -1043,6 +1047,47 @@ def test_transaction_http_method_custom(sentry_init, capture_events):
assert event2["request"]["method"] == "HEAD"


@pytest.mark.parametrize("span_streaming", [True, False])
def test_request_url(sentry_init, capture_events, capture_items, span_streaming):
sentry_init(
traces_sample_rate=1.0,
send_default_pii=True,
integrations=[
StarletteIntegration(),
],
_experiments={
"trace_lifecycle": "stream" if span_streaming else "static",
},
)

starlette_app = fastapi_app_factory()

client = TestClient(starlette_app)

if span_streaming:
items = capture_items("span")

client.get("/root/nomessage")
sentry_sdk.flush()
spans = [item.payload for item in items]

(server_span,) = (
span
for span in spans
if span["attributes"].get("sentry.op") == "http.server"
)
assert server_span["attributes"]["url.full"] == (
"http://testserver/root/nomessage"
)
else:
events = capture_events()

client.get("/root/nomessage")

(event,) = events
assert event["request"]["url"] == "http://testserver/root/nomessage"


@parametrize_test_configurable_status_codes
def test_configurable_status_codes(
sentry_init,
Expand Down
Loading
Loading