diff --git a/sentry_sdk/consts.py b/sentry_sdk/consts.py index b85b179223..8306d01626 100644 --- a/sentry_sdk/consts.py +++ b/sentry_sdk/consts.py @@ -1020,6 +1020,12 @@ class SPANDATA: Example: "details" """ + URL_PATH = "url.path" + """ + The URI path component. + Example: "/foo" + """ + URL_QUERY = "url.query" """ The query string present in the URL. Note that this does not contain the leading ? character, while the `http.query` attribute does. diff --git a/sentry_sdk/integrations/_asgi_common.py b/sentry_sdk/integrations/_asgi_common.py index eda75a2926..85f8f11d6d 100644 --- a/sentry_sdk/integrations/_asgi_common.py +++ b/sentry_sdk/integrations/_asgi_common.py @@ -1,4 +1,5 @@ import urllib +from enum import Enum from typing import TYPE_CHECKING from sentry_sdk.integrations._wsgi_common import _filter_headers @@ -12,6 +13,11 @@ from sentry_sdk.utils import AnnotatedValue +class _RootPathInPath(Enum): + EXCLUDED = "excluded" + EITHER = "either" + + def _get_headers(asgi_scope: "Any") -> "Dict[str, str]": """ Extract headers from the ASGI scope, in the format that the Sentry protocol expects. @@ -28,10 +34,27 @@ def _get_headers(asgi_scope: "Any") -> "Dict[str, str]": return headers +def _get_path( + asgi_scope: "Dict[str, Any]", root_path_in_path: "_RootPathInPath" +) -> "str": + if root_path_in_path is _RootPathInPath.EXCLUDED: + return asgi_scope.get("root_path", "") + asgi_scope.get("path", "") + + # Inverse of https://github.com/Kludex/starlette/blob/de970d7b3facb853eb7ad077decbf3d94f2aab6c/starlette/_utils.py#L96 + path = asgi_scope["path"] + root_path = asgi_scope.get("root_path", "") + + if not root_path or path == root_path or path.startswith(root_path + "/"): + return path + + return root_path + path + + def _get_url( asgi_scope: "Dict[str, Any]", default_scheme: "Literal['ws', 'http']", host: "Optional[Union[AnnotatedValue, str]]", + path: str, ) -> str: """ Extract URL from the ASGI scope, without also including the querystring. @@ -39,7 +62,6 @@ def _get_url( scheme = asgi_scope.get("scheme", default_scheme) server = asgi_scope.get("server", None) - path = asgi_scope.get("root_path", "") + asgi_scope.get("path", "") if host: return "%s://%s%s" % (scheme, host, path) @@ -81,7 +103,10 @@ 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", + root_path_in_path: "_RootPathInPath", +) -> "Dict[str, Any]": """ Returns data related to the HTTP request from the ASGI scope. """ @@ -96,7 +121,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=_get_path(asgi_scope=asgi_scope, root_path_in_path=root_path_in_path), ) client = asgi_scope.get("client") @@ -106,7 +134,10 @@ 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", + root_path_in_path: "_RootPathInPath", +) -> "dict[str, Any]": """ Return attributes related to the HTTP request from the ASGI scope. """ @@ -126,8 +157,14 @@ def _get_request_attributes(asgi_scope: "Any") -> "dict[str, Any]": if query: attributes["http.query"] = query + path = _get_path(asgi_scope=asgi_scope, root_path_in_path=root_path_in_path) + attributes["url.path"] = path + 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=path, ) query_string = _get_query(asgi_scope) attributes["url.full"] = ( @@ -135,9 +172,6 @@ def _get_request_attributes(asgi_scope: "Any") -> "dict[str, Any]": if query_string is not None else url_without_query_string ) - attributes["url.path"] = asgi_scope.get("root_path", "") + asgi_scope.get( - "path", "" - ) client = asgi_scope.get("client") if client and should_send_default_pii(): diff --git a/sentry_sdk/integrations/asgi.py b/sentry_sdk/integrations/asgi.py index f0470e33fc..8b1ff5e2a3 100644 --- a/sentry_sdk/integrations/asgi.py +++ b/sentry_sdk/integrations/asgi.py @@ -16,9 +16,11 @@ from sentry_sdk.integrations._asgi_common import ( _get_headers, _get_ip, + _get_path, _get_request_attributes, _get_request_data, _get_url, + _RootPathInPath, ) from sentry_sdk.integrations._wsgi_common import ( DEFAULT_HTTP_METHODS_TO_CAPTURE, @@ -105,6 +107,7 @@ class SentryAsgiMiddleware: "mechanism_type", "span_origin", "http_methods_to_capture", + "root_path_in_path", ) def __init__( @@ -116,6 +119,7 @@ def __init__( span_origin: str = "manual", http_methods_to_capture: "Tuple[str, ...]" = DEFAULT_HTTP_METHODS_TO_CAPTURE, asgi_version: "Optional[int]" = None, + root_path_in_path: "_RootPathInPath" = _RootPathInPath.EXCLUDED, ) -> None: """ Instrument an ASGI application with Sentry. Provides HTTP/websocket @@ -152,6 +156,7 @@ def __init__( self.span_origin = span_origin self.app = app self.http_methods_to_capture = http_methods_to_capture + self.root_path_in_path = root_path_in_path if asgi_version is None: if _looks_like_asgi3(app): @@ -319,7 +324,8 @@ async def _run_app( with span_ctx as span: if isinstance(span, StreamedSpan): for attribute, value in _get_request_attributes( - scope + scope, + root_path_in_path=self.root_path_in_path, ).items(): span.set_attribute(attribute, value) @@ -401,7 +407,9 @@ 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, root_path_in_path=self.root_path_in_path) + ) event["request"] = deepcopy(request_data) # Only set transaction name if not already set by Starlette or FastAPI (or other frameworks) @@ -447,7 +455,14 @@ 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=_get_path( + asgi_scope=asgi_scope, root_path_in_path=self.root_path_in_path + ), + ) source = TransactionSource.URL elif transaction_style == "url": @@ -459,7 +474,14 @@ 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=_get_path( + asgi_scope=asgi_scope, root_path_in_path=self.root_path_in_path + ), + ) source = TransactionSource.URL if name is None: @@ -484,7 +506,14 @@ 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=_get_path( + asgi_scope=asgi_scope, root_path_in_path=self.root_path_in_path + ), + ) source = SegmentSource.URL.value elif segment_style == "url": @@ -496,7 +525,14 @@ 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=_get_path( + asgi_scope=asgi_scope, root_path_in_path=self.root_path_in_path + ), + ) source = SegmentSource.URL.value if name is None: diff --git a/sentry_sdk/integrations/starlette.py b/sentry_sdk/integrations/starlette.py index 1482efc25b..eccb4f780f 100644 --- a/sentry_sdk/integrations/starlette.py +++ b/sentry_sdk/integrations/starlette.py @@ -15,6 +15,7 @@ DidNotEnable, Integration, ) +from sentry_sdk.integrations._asgi_common import _RootPathInPath from sentry_sdk.integrations._wsgi_common import ( DEFAULT_HTTP_METHODS_TO_CAPTURE, HttpCodeRangeContainer, @@ -143,7 +144,12 @@ def setup_once() -> None: ) patch_middlewares() - patch_asgi_app() + # Starlette tolerates both starting with: + # https://github.com/Kludex/starlette/commit/e8f0dcd54e4ceec47e02c45f5275374e292339ad. + root_path_in_path = ( + _RootPathInPath.EITHER if version >= (0, 33) else _RootPathInPath.EXCLUDED + ) + patch_asgi_app(root_path_in_path=root_path_in_path) patch_request_response() if version >= (0, 24): @@ -427,7 +433,7 @@ def _sentry_middleware_init( Middleware.__init__ = _sentry_middleware_init -def patch_asgi_app() -> None: +def patch_asgi_app(root_path_in_path: "_RootPathInPath") -> None: """ Instrument Starlette ASGI app using the SentryAsgiMiddleware. """ @@ -451,6 +457,7 @@ async def _sentry_patched_asgi_app( else DEFAULT_HTTP_METHODS_TO_CAPTURE ), asgi_version=3, + root_path_in_path=root_path_in_path, ) return await middleware(scope, receive, send) diff --git a/tests/integrations/asgi/test_asgi.py b/tests/integrations/asgi/test_asgi.py index 60ccd88bd1..3bce0d1e10 100644 --- a/tests/integrations/asgi/test_asgi.py +++ b/tests/integrations/asgi/test_asgi.py @@ -243,30 +243,6 @@ async def test_capture_transaction( } -@pytest.mark.asyncio -async def test_capture_transaction_with_root_path( - sentry_init, - asgi3_app, - capture_items, -): - sentry_init( - send_default_pii=True, - traces_sample_rate=1.0, - _experiments={"trace_lifecycle": "stream"}, - ) - app = SentryAsgiMiddleware(asgi3_app) - - async with TestClient(app, scope={"root_path": "/api"}) as client: - items = capture_items("span") - await client.get("/some_url") - - sentry_sdk.flush() - - assert len(items) == 1 - span = items[0].payload - assert span["attributes"]["url.path"] == "/api/some_url" - - @pytest.mark.asyncio @pytest.mark.parametrize( "span_streaming", diff --git a/tests/integrations/fastapi/test_fastapi.py b/tests/integrations/fastapi/test_fastapi.py index dc6bde89c8..d91927a644 100644 --- a/tests/integrations/fastapi/test_fastapi.py +++ b/tests/integrations/fastapi/test_fastapi.py @@ -62,6 +62,7 @@ def fastapi_app_factory(): app = FastAPI() + mounted_app = FastAPI() @app.get("/error") async def _error(): @@ -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") @@ -118,6 +120,8 @@ async def body_form( capture_message("hi") return {"status": "ok"} + app.mount("/root", mounted_app) + return app @@ -1043,6 +1047,48 @@ 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"][SPANDATA.URL_FULL] == ( + "http://testserver/root/nomessage" + ) + assert server_span["attributes"][SPANDATA.URL_PATH] == "/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, diff --git a/tests/integrations/starlette/test_starlette.py b/tests/integrations/starlette/test_starlette.py index 22ae7c55a4..fb458a84d1 100644 --- a/tests/integrations/starlette/test_starlette.py +++ b/tests/integrations/starlette/test_starlette.py @@ -146,6 +146,12 @@ async def _body_raw(request): "TRACE", ] + mounted_app = starlette.applications.Starlette( + routes=[ + starlette.routing.Route("/nomessage", _nomessage, methods=all_methods), + ], + ) + app = starlette.applications.Starlette( debug=debug, routes=[ @@ -160,6 +166,7 @@ async def _body_raw(request): starlette.routing.Route("/body/json", _body_json, methods=["POST"]), starlette.routing.Route("/body/form", _body_form, methods=["POST"]), starlette.routing.Route("/body/raw", _body_raw, methods=["POST"]), + starlette.routing.Mount("/root", app=mounted_app), ], middleware=middleware, ) @@ -1477,6 +1484,48 @@ def test_transaction_http_method_default(sentry_init, capture_events): assert event["request"]["method"] == "GET" +@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 = starlette_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"][SPANDATA.URL_FULL] == ( + "http://testserver/root/nomessage" + ) + assert server_span["attributes"][SPANDATA.URL_PATH] == "/root/nomessage" + else: + events = capture_events() + + client.get("/root/nomessage") + + (event,) = events + assert event["request"]["url"] == "http://testserver/root/nomessage" + + @pytest.mark.skipif( STARLETTE_VERSION < (0, 21), reason="Requires Starlette >= 0.21, because earlier versions do not support HTTP 'HEAD' requests",