fix(starlette): Stop duplicating scope["root_path"] in URLs (#6579)

Add the `root_path_in_path` parameter to functions used for setting the `url.path` and `url.full` attributes.

When `root_path_in_path` is `_RootPathInPath.EXCLUDED`, preserve the existing behavior of prepending `scope["root_path"]` when building the URL.

The other `_RootPathInPath.EITHER` option is used by the Starlette integration. It preserves Starlette's behavior of accepting ASGI scopes where `scope["path"]` either includes or excludes `scope["root_path"]`. The detection is based on Starlette's route resolution.

Closes #6577

Author: alexander-alderman-webb

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.

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,18 +34,34 @@ 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.
     """
     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,18 +157,21 @@ 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"] = (
                 f"{url_without_query_string}?{query_string}"
                 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():

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:

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)

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",

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,