diff --git a/sentry_sdk/integrations/asgi.py b/sentry_sdk/integrations/asgi.py index b5170d3ab7..2cecdf9a81 100644 --- a/sentry_sdk/integrations/asgi.py +++ b/sentry_sdk/integrations/asgi.py @@ -17,12 +17,15 @@ from sentry_sdk.integrations._asgi_common import ( _get_headers, _get_request_data, + _get_url, ) from sentry_sdk.integrations.modules import _get_installed_modules from sentry_sdk.sessions import auto_session_tracking from sentry_sdk.tracing import ( SOURCE_FOR_STYLE, TRANSACTION_SOURCE_ROUTE, + TRANSACTION_SOURCE_URL, + TRANSACTION_SOURCE_COMPONENT, ) from sentry_sdk.utils import ( ContextVar, @@ -35,10 +38,11 @@ from sentry_sdk.tracing import Transaction if TYPE_CHECKING: - from typing import Dict from typing import Any - from typing import Optional from typing import Callable + from typing import Dict + from typing import Optional + from typing import Tuple from sentry_sdk._types import Event, Hint @@ -144,7 +148,8 @@ async def _run_asgi3(self, scope, receive, send): async def _run_app(self, scope, receive, send, asgi_version): # type: (Any, Any, Any, Any, int) -> Any is_recursive_asgi_middleware = _asgi_middleware_applied.get(False) - if is_recursive_asgi_middleware: + is_lifespan = scope["type"] == "lifespan" + if is_recursive_asgi_middleware or is_lifespan: try: if asgi_version == 2: return await self.app(scope)(receive, send) @@ -167,24 +172,35 @@ async def _run_app(self, scope, receive, send, asgi_version): sentry_scope.add_event_processor(processor) ty = scope["type"] + ( + transaction_name, + transaction_source, + ) = self._get_transaction_name_and_source( + self.transaction_style, + scope, + ) if ty in ("http", "websocket"): transaction = continue_trace( _get_headers(scope), op="{}.server".format(ty), + name=transaction_name, + source=transaction_source, ) logger.debug( "[ASGI] Created transaction (continuing trace): %s", transaction, ) else: - transaction = Transaction(op=OP.HTTP_SERVER) + transaction = Transaction( + op=OP.HTTP_SERVER, + name=transaction_name, + source=transaction_source, + ) logger.debug( "[ASGI] Created transaction (new): %s", transaction ) - transaction.name = _DEFAULT_TRANSACTION_NAME - transaction.source = TRANSACTION_SOURCE_ROUTE transaction.set_tag("asgi.type", ty) logger.debug( "[ASGI] Set transaction name and source on transaction: '%s' / '%s'", @@ -232,7 +248,25 @@ def event_processor(self, event, hint, asgi_scope): request_data.update(_get_request_data(asgi_scope)) event["request"] = deepcopy(request_data) - self._set_transaction_name_and_source(event, self.transaction_style, asgi_scope) + # Only set transaction name if not already set by Starlette or FastAPI (or other frameworks) + already_set = event["transaction"] != _DEFAULT_TRANSACTION_NAME and event[ + "transaction_info" + ].get("source") in [ + TRANSACTION_SOURCE_COMPONENT, + TRANSACTION_SOURCE_ROUTE, + ] + if not already_set: + name, source = self._get_transaction_name_and_source( + self.transaction_style, asgi_scope + ) + event["transaction"] = name + event["transaction_info"] = {"source": source} + + logger.debug( + "[ASGI] Set transaction name and source in event_processor: '%s' / '%s'", + event["transaction"], + event["transaction_info"]["source"], + ) return event @@ -242,16 +276,11 @@ def event_processor(self, event, hint, asgi_scope): # data to your liking it's recommended to use the `before_send` callback # for that. - def _set_transaction_name_and_source(self, event, transaction_style, asgi_scope): - # type: (Event, str, Any) -> None - transaction_name_already_set = ( - event.get("transaction", _DEFAULT_TRANSACTION_NAME) - != _DEFAULT_TRANSACTION_NAME - ) - if transaction_name_already_set: - return - - name = "" + def _get_transaction_name_and_source(self, transaction_style, asgi_scope): + # type: (SentryAsgiMiddleware, str, Any) -> Tuple[str, str] + name = None + source = SOURCE_FOR_STYLE[transaction_style] + ty = asgi_scope.get("type") if transaction_style == "endpoint": endpoint = asgi_scope.get("endpoint") @@ -260,6 +289,9 @@ def _set_transaction_name_and_source(self, event, transaction_style, asgi_scope) # an endpoint, overwrite our generic transaction name. if endpoint: name = transaction_from_function(endpoint) or "" + else: + name = _get_url(asgi_scope, "http" if ty == "http" else "ws", host=None) + source = TRANSACTION_SOURCE_URL elif transaction_style == "url": # FastAPI includes the route object in the scope to let Sentry extract the @@ -269,21 +301,13 @@ def _set_transaction_name_and_source(self, event, transaction_style, asgi_scope) 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 = TRANSACTION_SOURCE_URL - if not name: - event["transaction"] = _DEFAULT_TRANSACTION_NAME - event["transaction_info"] = {"source": TRANSACTION_SOURCE_ROUTE} - logger.debug( - "[ASGI] Set default transaction name and source on event: '%s' / '%s'", - event["transaction"], - event["transaction_info"]["source"], - ) - return - - event["transaction"] = name - event["transaction_info"] = {"source": SOURCE_FOR_STYLE[transaction_style]} - logger.debug( - "[ASGI] Set transaction name and source on event: '%s' / '%s'", - event["transaction"], - event["transaction_info"]["source"], - ) + if name is None: + name = _DEFAULT_TRANSACTION_NAME + source = TRANSACTION_SOURCE_ROUTE + return name, source + + return name, source diff --git a/sentry_sdk/integrations/starlette.py b/sentry_sdk/integrations/starlette.py index 1e3944aff3..ed95c757f1 100644 --- a/sentry_sdk/integrations/starlette.py +++ b/sentry_sdk/integrations/starlette.py @@ -14,7 +14,11 @@ request_body_within_bounds, ) from sentry_sdk.integrations.asgi import SentryAsgiMiddleware -from sentry_sdk.tracing import SOURCE_FOR_STYLE, TRANSACTION_SOURCE_ROUTE +from sentry_sdk.tracing import ( + SOURCE_FOR_STYLE, + TRANSACTION_SOURCE_COMPONENT, + TRANSACTION_SOURCE_ROUTE, +) from sentry_sdk.utils import ( AnnotatedValue, capture_internal_exceptions, @@ -25,7 +29,7 @@ ) if TYPE_CHECKING: - from typing import Any, Awaitable, Callable, Dict, Optional + from typing import Any, Awaitable, Callable, Dict, Optional, Tuple from sentry_sdk.scope import Scope as SentryScope @@ -106,6 +110,15 @@ async def _create_span_call(app, scope, receive, send, **kwargs): if integration is not None: middleware_name = app.__class__.__name__ + # Update transaction name with middleware name + with hub.configure_scope() as sentry_scope: + name, source = _get_transaction_from_middleware(app, scope, integration) + if name is not None: + sentry_scope.set_transaction_name( + name, + source=source, + ) + with hub.start_span( op=OP.MIDDLEWARE_STARLETTE, description=middleware_name ) as middleware_span: @@ -337,12 +350,14 @@ def patch_asgi_app(): async def _sentry_patched_asgi_app(self, scope, receive, send): # type: (Starlette, StarletteScope, Receive, Send) -> None - if Hub.current.get_integration(StarletteIntegration) is None: + integration = Hub.current.get_integration(StarletteIntegration) + if integration is None: return await old_app(self, scope, receive, send) middleware = SentryAsgiMiddleware( lambda *a, **kw: old_app(self, *a, **kw), mechanism_type=StarletteIntegration.identifier, + transaction_style=integration.transaction_style, ) middleware.__call__ = middleware._run_asgi3 @@ -620,35 +635,53 @@ async def json(self): return await self.request.json() +def _transaction_name_from_router(scope): + # type: (StarletteScope) -> Optional[str] + router = scope.get("router") + if not router: + return None + + for route in router.routes: + match = route.matches(scope) + if match[0] == Match.FULL: + return route.path + + return None + + def _set_transaction_name_and_source(scope, transaction_style, request): # type: (SentryScope, str, Any) -> None - name = "" + name = None + source = SOURCE_FOR_STYLE[transaction_style] if transaction_style == "endpoint": endpoint = request.scope.get("endpoint") if endpoint: - name = transaction_from_function(endpoint) or "" + name = transaction_from_function(endpoint) or None elif transaction_style == "url": - router = request.scope["router"] - for route in router.routes: - match = route.matches(request.scope) - - if match[0] == Match.FULL: - if transaction_style == "endpoint": - name = transaction_from_function(match[1]["endpoint"]) or "" - break - elif transaction_style == "url": - name = route.path - break - - if not name: + name = _transaction_name_from_router(request.scope) + + if name is None: name = _DEFAULT_TRANSACTION_NAME source = TRANSACTION_SOURCE_ROUTE - else: - source = SOURCE_FOR_STYLE[transaction_style] scope.set_transaction_name(name, source=source) logger.debug( "[Starlette] Set transaction name and source on scope: %s / %s", name, source ) + + +def _get_transaction_from_middleware(app, asgi_scope, integration): + # type: (Any, Dict[str, Any], StarletteIntegration) -> Tuple[Optional[str], Optional[str]] + name = None + source = None + + if integration.transaction_style == "endpoint": + name = transaction_from_function(app.__class__) + source = TRANSACTION_SOURCE_COMPONENT + elif integration.transaction_style == "url": + name = _transaction_name_from_router(asgi_scope) + source = TRANSACTION_SOURCE_ROUTE + + return name, source diff --git a/tests/integrations/asgi/test_asgi.py b/tests/integrations/asgi/test_asgi.py index 29aab5783a..f79b35db9a 100644 --- a/tests/integrations/asgi/test_asgi.py +++ b/tests/integrations/asgi/test_asgi.py @@ -157,13 +157,13 @@ async def test_capture_transaction( async with TestClient(app) as client: events = capture_events() - await client.get("/?somevalue=123") + await client.get("/some_url?somevalue=123") - (transaction_event, lifespan_transaction_event) = events + (transaction_event,) = events assert transaction_event["type"] == "transaction" - assert transaction_event["transaction"] == "generic ASGI request" - assert transaction_event["transaction_info"] == {"source": "route"} + assert transaction_event["transaction"] == "/some_url" + assert transaction_event["transaction_info"] == {"source": "url"} assert transaction_event["contexts"]["trace"]["op"] == "http.server" assert transaction_event["request"] == { "headers": { @@ -173,7 +173,7 @@ async def test_capture_transaction( }, "method": "GET", "query_string": "somevalue=123", - "url": "http://localhost/", + "url": "http://localhost/some_url", } @@ -191,12 +191,15 @@ async def test_capture_transaction_with_error( events = capture_events() with pytest.raises(ZeroDivisionError): async with TestClient(app) as client: - await client.get("/") + await client.get("/some_url") - (error_event, transaction_event, lifespan_transaction_event) = events + ( + error_event, + transaction_event, + ) = events - assert error_event["transaction"] == "generic ASGI request" - assert error_event["transaction_info"] == {"source": "route"} + assert error_event["transaction"] == "/some_url" + assert error_event["transaction_info"] == {"source": "url"} assert error_event["contexts"]["trace"]["op"] == "http.server" assert error_event["exception"]["values"][0]["type"] == "ZeroDivisionError" assert error_event["exception"]["values"][0]["value"] == "division by zero" @@ -393,13 +396,13 @@ async def test_auto_session_tracking_with_aggregates( for envelope in envelopes: count_item_types[envelope.items[0].type] += 1 - assert count_item_types["transaction"] == 4 + assert count_item_types["transaction"] == 3 assert count_item_types["event"] == 1 assert count_item_types["sessions"] == 1 - assert len(envelopes) == 6 + assert len(envelopes) == 5 session_aggregates = envelopes[-1].items[0].payload.json["aggregates"] - assert session_aggregates[0]["exited"] == 3 + assert session_aggregates[0]["exited"] == 2 assert session_aggregates[0]["crashed"] == 1 assert len(session_aggregates) == 1 @@ -445,7 +448,7 @@ async def test_transaction_style( events = capture_events() await client.get(url) - (transaction_event, lifespan_transaction_event) = events + (transaction_event,) = events assert transaction_event["transaction"] == expected_transaction assert transaction_event["transaction_info"] == {"source": expected_source} @@ -598,3 +601,108 @@ def test_get_headers(): "x-real-ip": "10.10.10.10", "some_header": "123, abc", } + + +@minimum_python_36 +@pytest.mark.asyncio +@pytest.mark.parametrize( + "request_url,transaction_style,expected_transaction_name,expected_transaction_source", + [ + ( + "/message/123456", + "endpoint", + "/message/123456", + "url", + ), + ( + "/message/123456", + "url", + "/message/123456", + "url", + ), + ], +) +async def test_transaction_name( + sentry_init, + request_url, + transaction_style, + expected_transaction_name, + expected_transaction_source, + asgi3_app, + capture_envelopes, +): + """ + Tests that the transaction name is something meaningful. + """ + sentry_init( + traces_sample_rate=1.0, + debug=True, + ) + + envelopes = capture_envelopes() + + app = SentryAsgiMiddleware(asgi3_app, transaction_style=transaction_style) + + async with TestClient(app) as client: + await client.get(request_url) + + (transaction_envelope,) = envelopes + transaction_event = transaction_envelope.get_transaction_event() + + assert transaction_event["transaction"] == expected_transaction_name + assert ( + transaction_event["transaction_info"]["source"] == expected_transaction_source + ) + + +@minimum_python_36 +@pytest.mark.asyncio +@pytest.mark.parametrize( + "request_url, transaction_style,expected_transaction_name,expected_transaction_source", + [ + ( + "/message/123456", + "endpoint", + "/message/123456", + "url", + ), + ( + "/message/123456", + "url", + "/message/123456", + "url", + ), + ], +) +async def test_transaction_name_in_traces_sampler( + sentry_init, + request_url, + transaction_style, + expected_transaction_name, + expected_transaction_source, + asgi3_app, +): + """ + Tests that a custom traces_sampler has a meaningful transaction name. + In this case the URL or endpoint, because we do not have the route yet. + """ + + def dummy_traces_sampler(sampling_context): + assert ( + sampling_context["transaction_context"]["name"] == expected_transaction_name + ) + assert ( + sampling_context["transaction_context"]["source"] + == expected_transaction_source + ) + + sentry_init( + traces_sampler=dummy_traces_sampler, + traces_sample_rate=1.0, + debug=True, + ) + + app = SentryAsgiMiddleware(asgi3_app, transaction_style=transaction_style) + + async with TestClient(app) as client: + await client.get(request_url) diff --git a/tests/integrations/fastapi/test_fastapi.py b/tests/integrations/fastapi/test_fastapi.py index 5a770a70af..26659c0a50 100644 --- a/tests/integrations/fastapi/test_fastapi.py +++ b/tests/integrations/fastapi/test_fastapi.py @@ -9,6 +9,7 @@ from fastapi import FastAPI, Request from fastapi.testclient import TestClient +from fastapi.middleware.trustedhost import TrustedHostMiddleware from sentry_sdk import capture_message from sentry_sdk.integrations.starlette import StarletteIntegration from sentry_sdk.integrations.asgi import SentryAsgiMiddleware @@ -322,3 +323,171 @@ def test_response_status_code_not_found_in_transaction_context( "response" in transaction["contexts"].keys() ), "Response context not found in transaction" assert transaction["contexts"]["response"]["status_code"] == 404 + + +@pytest.mark.parametrize( + "request_url,transaction_style,expected_transaction_name,expected_transaction_source", + [ + ( + "/message/123456", + "endpoint", + "tests.integrations.fastapi.test_fastapi.fastapi_app_factory.._message_with_id", + "component", + ), + ( + "/message/123456", + "url", + "/message/{message_id}", + "route", + ), + ], +) +def test_transaction_name( + sentry_init, + request_url, + transaction_style, + expected_transaction_name, + expected_transaction_source, + capture_envelopes, +): + """ + Tests that the transaction name is something meaningful. + """ + sentry_init( + auto_enabling_integrations=False, # Make sure that httpx integration is not added, because it adds tracing information to the starlette test clients request. + integrations=[ + StarletteIntegration(transaction_style=transaction_style), + FastApiIntegration(transaction_style=transaction_style), + ], + traces_sample_rate=1.0, + debug=True, + ) + + envelopes = capture_envelopes() + + app = fastapi_app_factory() + + client = TestClient(app) + client.get(request_url) + + (_, transaction_envelope) = envelopes + transaction_event = transaction_envelope.get_transaction_event() + + assert transaction_event["transaction"] == expected_transaction_name + assert ( + transaction_event["transaction_info"]["source"] == expected_transaction_source + ) + + +@pytest.mark.parametrize( + "request_url,transaction_style,expected_transaction_name,expected_transaction_source", + [ + ( + "/message/123456", + "endpoint", + "http://testserver/message/123456", + "url", + ), + ( + "/message/123456", + "url", + "http://testserver/message/123456", + "url", + ), + ], +) +def test_transaction_name_in_traces_sampler( + sentry_init, + request_url, + transaction_style, + expected_transaction_name, + expected_transaction_source, +): + """ + Tests that a custom traces_sampler retrieves a meaningful transaction name. + In this case the URL or endpoint, because we do not have the route yet. + """ + + def dummy_traces_sampler(sampling_context): + assert ( + sampling_context["transaction_context"]["name"] == expected_transaction_name + ) + assert ( + sampling_context["transaction_context"]["source"] + == expected_transaction_source + ) + + sentry_init( + auto_enabling_integrations=False, # Make sure that httpx integration is not added, because it adds tracing information to the starlette test clients request. + integrations=[StarletteIntegration(transaction_style=transaction_style)], + traces_sampler=dummy_traces_sampler, + traces_sample_rate=1.0, + debug=True, + ) + + app = fastapi_app_factory() + + client = TestClient(app) + client.get(request_url) + + +@pytest.mark.parametrize( + "request_url,transaction_style,expected_transaction_name,expected_transaction_source", + [ + ( + "/message/123456", + "endpoint", + "starlette.middleware.trustedhost.TrustedHostMiddleware", + "component", + ), + ( + "/message/123456", + "url", + "http://testserver/message/123456", + "url", + ), + ], +) +def test_transaction_name_in_middleware( + sentry_init, + request_url, + transaction_style, + expected_transaction_name, + expected_transaction_source, + capture_envelopes, +): + """ + Tests that the transaction name is something meaningful. + """ + sentry_init( + auto_enabling_integrations=False, # Make sure that httpx integration is not added, because it adds tracing information to the starlette test clients request. + integrations=[ + StarletteIntegration(transaction_style=transaction_style), + FastApiIntegration(transaction_style=transaction_style), + ], + traces_sample_rate=1.0, + debug=True, + ) + + envelopes = capture_envelopes() + + app = fastapi_app_factory() + + app.add_middleware( + TrustedHostMiddleware, + allowed_hosts=[ + "example.com", + ], + ) + + client = TestClient(app) + client.get(request_url) + + (transaction_envelope,) = envelopes + transaction_event = transaction_envelope.get_transaction_event() + + assert transaction_event["contexts"]["response"]["status_code"] == 400 + assert transaction_event["transaction"] == expected_transaction_name + assert ( + transaction_event["transaction_info"]["source"] == expected_transaction_source + ) diff --git a/tests/integrations/starlette/test_starlette.py b/tests/integrations/starlette/test_starlette.py index cc4d8cf3ba..22074f4710 100644 --- a/tests/integrations/starlette/test_starlette.py +++ b/tests/integrations/starlette/test_starlette.py @@ -33,8 +33,10 @@ ) from starlette.middleware import Middleware from starlette.middleware.authentication import AuthenticationMiddleware +from starlette.middleware.trustedhost import TrustedHostMiddleware from starlette.testclient import TestClient + STARLETTE_VERSION = parse_version(starlette.__version__) PICTURE = os.path.join(os.path.dirname(os.path.abspath(__file__)), "photo.jpg") @@ -949,3 +951,164 @@ def test_template_tracing_meta(sentry_init, capture_events): # Python 2 does not preserve sort order rendered_baggage = match.group(2) assert sorted(rendered_baggage.split(",")) == sorted(baggage.split(",")) + + +@pytest.mark.parametrize( + "request_url,transaction_style,expected_transaction_name,expected_transaction_source", + [ + ( + "/message/123456", + "endpoint", + "tests.integrations.starlette.test_starlette.starlette_app_factory.._message_with_id", + "component", + ), + ( + "/message/123456", + "url", + "/message/{message_id}", + "route", + ), + ], +) +def test_transaction_name( + sentry_init, + request_url, + transaction_style, + expected_transaction_name, + expected_transaction_source, + capture_envelopes, +): + """ + Tests that the transaction name is something meaningful. + """ + sentry_init( + auto_enabling_integrations=False, # Make sure that httpx integration is not added, because it adds tracing information to the starlette test clients request. + integrations=[StarletteIntegration(transaction_style=transaction_style)], + traces_sample_rate=1.0, + debug=True, + ) + + envelopes = capture_envelopes() + + app = starlette_app_factory() + client = TestClient(app) + client.get(request_url) + + (_, transaction_envelope) = envelopes + transaction_event = transaction_envelope.get_transaction_event() + + assert transaction_event["transaction"] == expected_transaction_name + assert ( + transaction_event["transaction_info"]["source"] == expected_transaction_source + ) + + +@pytest.mark.parametrize( + "request_url,transaction_style,expected_transaction_name,expected_transaction_source", + [ + ( + "/message/123456", + "endpoint", + "http://testserver/message/123456", + "url", + ), + ( + "/message/123456", + "url", + "http://testserver/message/123456", + "url", + ), + ], +) +def test_transaction_name_in_traces_sampler( + sentry_init, + request_url, + transaction_style, + expected_transaction_name, + expected_transaction_source, +): + """ + Tests that a custom traces_sampler has a meaningful transaction name. + In this case the URL or endpoint, because we do not have the route yet. + """ + + def dummy_traces_sampler(sampling_context): + assert ( + sampling_context["transaction_context"]["name"] == expected_transaction_name + ) + assert ( + sampling_context["transaction_context"]["source"] + == expected_transaction_source + ) + + sentry_init( + auto_enabling_integrations=False, # Make sure that httpx integration is not added, because it adds tracing information to the starlette test clients request. + integrations=[StarletteIntegration(transaction_style=transaction_style)], + traces_sampler=dummy_traces_sampler, + traces_sample_rate=1.0, + debug=True, + ) + + app = starlette_app_factory() + client = TestClient(app) + client.get(request_url) + + +@pytest.mark.parametrize( + "request_url,transaction_style,expected_transaction_name,expected_transaction_source", + [ + ( + "/message/123456", + "endpoint", + "starlette.middleware.trustedhost.TrustedHostMiddleware", + "component", + ), + ( + "/message/123456", + "url", + "http://testserver/message/123456", + "url", + ), + ], +) +def test_transaction_name_in_middleware( + sentry_init, + request_url, + transaction_style, + expected_transaction_name, + expected_transaction_source, + capture_envelopes, +): + """ + Tests that the transaction name is something meaningful. + """ + sentry_init( + auto_enabling_integrations=False, # Make sure that httpx integration is not added, because it adds tracing information to the starlette test clients request. + integrations=[ + StarletteIntegration(transaction_style=transaction_style), + ], + traces_sample_rate=1.0, + debug=True, + ) + + envelopes = capture_envelopes() + + middleware = [ + Middleware( + TrustedHostMiddleware, + allowed_hosts=["example.com", "*.example.com"], + ), + ] + + app = starlette_app_factory(middleware=middleware) + client = TestClient(app) + client.get(request_url) + + (transaction_envelope,) = envelopes + transaction_event = transaction_envelope.get_transaction_event() + + assert transaction_event["contexts"]["response"]["status_code"] == 400 + assert transaction_event["transaction"] == expected_transaction_name + assert ( + transaction_event["transaction_info"]["source"] == expected_transaction_source + )