From ae27086b6c5432375592aa0ceeeafee7069e7ca3 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 19 Jul 2023 16:17:48 +0200 Subject: [PATCH 01/14] Set the url as a transaction name (instead of 'generic ASGI request' in the beginning, so has something to work with --- sentry_sdk/integrations/asgi.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/sentry_sdk/integrations/asgi.py b/sentry_sdk/integrations/asgi.py index dc63be9d7d..c48eb74b53 100644 --- a/sentry_sdk/integrations/asgi.py +++ b/sentry_sdk/integrations/asgi.py @@ -164,14 +164,18 @@ async def _run_app(self, scope, callback): ty = scope["type"] if ty in ("http", "websocket"): + headers = self._get_headers(scope) transaction = continue_trace( - self._get_headers(scope), + headers, op="{}.server".format(ty), ) else: + headers = {} transaction = Transaction(op=OP.HTTP_SERVER) - transaction.name = _DEFAULT_TRANSACTION_NAME + transaction.name = self._get_url( + scope, "http" if ty == "http" else "ws", headers.get("host") + ) transaction.source = TRANSACTION_SOURCE_ROUTE transaction.set_tag("asgi.type", ty) From 31543ece91a950b4b408b7f20f7c900af60028c4 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 19 Jul 2023 16:46:10 +0200 Subject: [PATCH 02/14] Committing this to check why it work in old version. --- sentry_sdk/integrations/asgi.py | 17 ++++++++++++----- tests/integrations/asgi/test_asgi.py | 6 +++--- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/sentry_sdk/integrations/asgi.py b/sentry_sdk/integrations/asgi.py index c48eb74b53..7282a4d22a 100644 --- a/sentry_sdk/integrations/asgi.py +++ b/sentry_sdk/integrations/asgi.py @@ -173,9 +173,16 @@ async def _run_app(self, scope, callback): headers = {} transaction = Transaction(op=OP.HTTP_SERVER) - transaction.name = self._get_url( - scope, "http" if ty == "http" else "ws", headers.get("host") - ) + # Set default transaction name + if self.transaction_style == "url": + transaction.name = self._get_url( + scope, "http" if ty == "http" else "ws", headers.get("host") + ) + else: + endpoint = scope.get("endpoint") + if endpoint: + transaction.name = transaction_from_function(endpoint) or "" + transaction.source = TRANSACTION_SOURCE_ROUTE transaction.set_tag("asgi.type", ty) @@ -233,8 +240,8 @@ def _set_transaction_name_and_source(self, event, transaction_style, asgi_scope) event.get("transaction", _DEFAULT_TRANSACTION_NAME) != _DEFAULT_TRANSACTION_NAME ) - if transaction_name_already_set: - return + # if transaction_name_already_set: + # return name = "" diff --git a/tests/integrations/asgi/test_asgi.py b/tests/integrations/asgi/test_asgi.py index d51293af75..3a4f7f5beb 100644 --- a/tests/integrations/asgi/test_asgi.py +++ b/tests/integrations/asgi/test_asgi.py @@ -142,7 +142,7 @@ async def test_capture_transaction( (transaction_event,) = events assert transaction_event["type"] == "transaction" - assert transaction_event["transaction"] == "generic ASGI request" + assert transaction_event["transaction"] == "http://localhost/" assert transaction_event["contexts"]["trace"]["op"] == "http.server" assert transaction_event["request"] == { "headers": { @@ -174,7 +174,7 @@ async def test_capture_transaction_with_error( (error_event, transaction_event) = events - assert error_event["transaction"] == "generic ASGI request" + assert error_event["transaction"] == "http://localhost/" 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" @@ -389,7 +389,7 @@ async def test_auto_session_tracking_with_aggregates( ( "/message", "url", - "generic ASGI request", + "http://localhost/message", "route", ), ( From ceb4b80a1d0053746fcbbd2637d9d906925639d0 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Thu, 20 Jul 2023 14:40:25 +0200 Subject: [PATCH 03/14] Committing everything. Waiting for others to come back from conference to discuss how to move forward --- sentry_sdk/integrations/asgi.py | 104 ++++++++++++++++++--------- sentry_sdk/integrations/fastapi.py | 3 + sentry_sdk/integrations/starlette.py | 9 +++ 3 files changed, 83 insertions(+), 33 deletions(-) diff --git a/sentry_sdk/integrations/asgi.py b/sentry_sdk/integrations/asgi.py index 7282a4d22a..4b66afdc47 100644 --- a/sentry_sdk/integrations/asgi.py +++ b/sentry_sdk/integrations/asgi.py @@ -20,6 +20,8 @@ from sentry_sdk.tracing import ( SOURCE_FOR_STYLE, TRANSACTION_SOURCE_ROUTE, + TRANSACTION_SOURCE_URL, + TRANSACTION_SOURCE_COMPONENT, ) from sentry_sdk.utils import ( ContextVar, @@ -79,7 +81,13 @@ def _looks_like_asgi3(app): class SentryAsgiMiddleware: - __slots__ = ("app", "__call__", "transaction_style", "mechanism_type") + __slots__ = ( + "app", + "__call__", + "transaction_style", + "mechanism_type", + "should_start_transaction", + ) def __init__( self, @@ -87,6 +95,7 @@ def __init__( unsafe_context_data=False, transaction_style="endpoint", mechanism_type="asgi", + should_start_transaction=True, ): # type: (Any, bool, str, str) -> None """ @@ -121,6 +130,7 @@ def __init__( self.transaction_style = transaction_style self.mechanism_type = mechanism_type + self.should_start_transaction = should_start_transaction self.app = app if _looks_like_asgi3(app): @@ -128,6 +138,24 @@ def __init__( else: self.__call__ = self._run_asgi2 + def _get_default_transaction_name_and_source(self, scope): + name = _DEFAULT_TRANSACTION_NAME + source = TRANSACTION_SOURCE_ROUTE + + if self.transaction_style == "url": + source = TRANSACTION_SOURCE_URL + headers = self._get_headers(scope) + name = self._get_url( + scope, "http" if scope["type"] == "http" else "ws", headers.get("host") + ) + else: + source = TRANSACTION_SOURCE_COMPONENT + endpoint = scope.get("endpoint") + if endpoint: + name = transaction_from_function(endpoint) or "" + + return name, source + def _run_asgi2(self, scope): # type: (Any) -> Any async def inner(receive, send): @@ -161,44 +189,48 @@ async def _run_app(self, scope, callback): processor = partial(self.event_processor, asgi_scope=scope) sentry_scope.add_event_processor(processor) - ty = scope["type"] + import ipdb + + ipdb.set_trace() + ( + default_transaction_name, + default_transaction_source, + ) = self._get_default_transaction_name_and_source(scope) + ty = scope["type"] if ty in ("http", "websocket"): - headers = self._get_headers(scope) transaction = continue_trace( - headers, + self._get_headers(scope), op="{}.server".format(ty), + name=default_transaction_name, + source=default_transaction_source, ) else: - headers = {} - transaction = Transaction(op=OP.HTTP_SERVER) - - # Set default transaction name - if self.transaction_style == "url": - transaction.name = self._get_url( - scope, "http" if ty == "http" else "ws", headers.get("host") + transaction = Transaction( + op=OP.HTTP_SERVER, + name=default_transaction_name, + source=default_transaction_source, ) - else: - endpoint = scope.get("endpoint") - if endpoint: - transaction.name = transaction_from_function(endpoint) or "" - transaction.source = TRANSACTION_SOURCE_ROUTE transaction.set_tag("asgi.type", ty) - with hub.start_transaction( - transaction, custom_sampling_context={"asgi_scope": scope} - ): - # XXX: Would be cool to have correct span status, but we - # would have to wrap send(). That is a bit hard to do with - # the current abstraction over ASGI 2/3. - try: - return await callback() - except Exception as exc: - _capture_exception( - hub, exc, mechanism_type=self.mechanism_type - ) - raise exc from None + if self.should_start_transaction: + transaction = hub.start_transaction( + transaction, custom_sampling_context={"asgi_scope": scope} + ) + + # XXX: Would be cool to have correct span status, but we + # would have to wrap send(). That is a bit hard to do with + # the current abstraction over ASGI 2/3. + try: + return await callback() + except Exception as exc: + _capture_exception(hub, exc, mechanism_type=self.mechanism_type) + raise exc from None + finally: + if self.should_start_transaction: + transaction.__exit__() + finally: _asgi_middleware_applied.set(False) @@ -236,12 +268,18 @@ def event_processor(self, event, hint, asgi_scope): def _set_transaction_name_and_source(self, event, transaction_style, asgi_scope): # type: (Event, str, Any) -> None + import ipdb + + ipdb.set_trace() + default_transaction_name, _ = self._get_default_transaction_name_and_source( + asgi_scope + ) transaction_name_already_set = ( - event.get("transaction", _DEFAULT_TRANSACTION_NAME) - != _DEFAULT_TRANSACTION_NAME + event.get("transaction", default_transaction_name) + != default_transaction_name ) - # if transaction_name_already_set: - # return + if transaction_name_already_set: + return name = "" diff --git a/sentry_sdk/integrations/fastapi.py b/sentry_sdk/integrations/fastapi.py index 17e0576c18..c52fe0fbd0 100644 --- a/sentry_sdk/integrations/fastapi.py +++ b/sentry_sdk/integrations/fastapi.py @@ -39,6 +39,9 @@ def setup_once(): def _set_transaction_name_and_source(scope, transaction_style, request): # type: (Scope, str, Any) -> None + import ipdb + + ipdb.set_trace() name = "" if transaction_style == "endpoint": diff --git a/sentry_sdk/integrations/starlette.py b/sentry_sdk/integrations/starlette.py index b44e8f10b7..09f90249e4 100644 --- a/sentry_sdk/integrations/starlette.py +++ b/sentry_sdk/integrations/starlette.py @@ -342,9 +342,15 @@ async def _sentry_patched_asgi_app(self, scope, receive, send): middleware = SentryAsgiMiddleware( lambda *a, **kw: old_app(self, *a, **kw), mechanism_type=StarletteIntegration.identifier, + should_start_transaction=False, ) middleware.__call__ = middleware._run_asgi3 + + import ipdb + + ipdb.set_trace() + return await middleware(scope, receive, send) Starlette.__call__ = _sentry_patched_asgi_app @@ -621,6 +627,9 @@ async def json(self): def _set_transaction_name_and_source(scope, transaction_style, request): # type: (SentryScope, str, Any) -> None + import ipdb + + ipdb.set_trace() name = "" if transaction_style == "endpoint": From e2b7e5b562b50d5aded5eb3119a7b22e6cc8ea58 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Fri, 21 Jul 2023 09:15:23 +0200 Subject: [PATCH 04/14] Restored asgi.py from master --- sentry_sdk/integrations/asgi.py | 89 ++++++++------------------------- 1 file changed, 20 insertions(+), 69 deletions(-) diff --git a/sentry_sdk/integrations/asgi.py b/sentry_sdk/integrations/asgi.py index 4b66afdc47..dc63be9d7d 100644 --- a/sentry_sdk/integrations/asgi.py +++ b/sentry_sdk/integrations/asgi.py @@ -20,8 +20,6 @@ from sentry_sdk.tracing import ( SOURCE_FOR_STYLE, TRANSACTION_SOURCE_ROUTE, - TRANSACTION_SOURCE_URL, - TRANSACTION_SOURCE_COMPONENT, ) from sentry_sdk.utils import ( ContextVar, @@ -81,13 +79,7 @@ def _looks_like_asgi3(app): class SentryAsgiMiddleware: - __slots__ = ( - "app", - "__call__", - "transaction_style", - "mechanism_type", - "should_start_transaction", - ) + __slots__ = ("app", "__call__", "transaction_style", "mechanism_type") def __init__( self, @@ -95,7 +87,6 @@ def __init__( unsafe_context_data=False, transaction_style="endpoint", mechanism_type="asgi", - should_start_transaction=True, ): # type: (Any, bool, str, str) -> None """ @@ -130,7 +121,6 @@ def __init__( self.transaction_style = transaction_style self.mechanism_type = mechanism_type - self.should_start_transaction = should_start_transaction self.app = app if _looks_like_asgi3(app): @@ -138,24 +128,6 @@ def __init__( else: self.__call__ = self._run_asgi2 - def _get_default_transaction_name_and_source(self, scope): - name = _DEFAULT_TRANSACTION_NAME - source = TRANSACTION_SOURCE_ROUTE - - if self.transaction_style == "url": - source = TRANSACTION_SOURCE_URL - headers = self._get_headers(scope) - name = self._get_url( - scope, "http" if scope["type"] == "http" else "ws", headers.get("host") - ) - else: - source = TRANSACTION_SOURCE_COMPONENT - endpoint = scope.get("endpoint") - if endpoint: - name = transaction_from_function(endpoint) or "" - - return name, source - def _run_asgi2(self, scope): # type: (Any) -> Any async def inner(receive, send): @@ -189,48 +161,33 @@ async def _run_app(self, scope, callback): processor = partial(self.event_processor, asgi_scope=scope) sentry_scope.add_event_processor(processor) - import ipdb - - ipdb.set_trace() - ( - default_transaction_name, - default_transaction_source, - ) = self._get_default_transaction_name_and_source(scope) - ty = scope["type"] + if ty in ("http", "websocket"): transaction = continue_trace( self._get_headers(scope), op="{}.server".format(ty), - name=default_transaction_name, - source=default_transaction_source, ) else: - transaction = Transaction( - op=OP.HTTP_SERVER, - name=default_transaction_name, - source=default_transaction_source, - ) + transaction = Transaction(op=OP.HTTP_SERVER) + transaction.name = _DEFAULT_TRANSACTION_NAME + transaction.source = TRANSACTION_SOURCE_ROUTE transaction.set_tag("asgi.type", ty) - if self.should_start_transaction: - transaction = hub.start_transaction( - transaction, custom_sampling_context={"asgi_scope": scope} - ) - - # XXX: Would be cool to have correct span status, but we - # would have to wrap send(). That is a bit hard to do with - # the current abstraction over ASGI 2/3. - try: - return await callback() - except Exception as exc: - _capture_exception(hub, exc, mechanism_type=self.mechanism_type) - raise exc from None - finally: - if self.should_start_transaction: - transaction.__exit__() - + with hub.start_transaction( + transaction, custom_sampling_context={"asgi_scope": scope} + ): + # XXX: Would be cool to have correct span status, but we + # would have to wrap send(). That is a bit hard to do with + # the current abstraction over ASGI 2/3. + try: + return await callback() + except Exception as exc: + _capture_exception( + hub, exc, mechanism_type=self.mechanism_type + ) + raise exc from None finally: _asgi_middleware_applied.set(False) @@ -268,15 +225,9 @@ def event_processor(self, event, hint, asgi_scope): def _set_transaction_name_and_source(self, event, transaction_style, asgi_scope): # type: (Event, str, Any) -> None - import ipdb - - ipdb.set_trace() - default_transaction_name, _ = self._get_default_transaction_name_and_source( - asgi_scope - ) transaction_name_already_set = ( - event.get("transaction", default_transaction_name) - != default_transaction_name + event.get("transaction", _DEFAULT_TRANSACTION_NAME) + != _DEFAULT_TRANSACTION_NAME ) if transaction_name_already_set: return From ba3b4d380bf432151a93ac3cb3a9cd3e94200e18 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Fri, 21 Jul 2023 09:16:04 +0200 Subject: [PATCH 05/14] Start transaction in FastAPI integration --- sentry_sdk/integrations/fastapi.py | 71 +++++++++++++++++------------- 1 file changed, 40 insertions(+), 31 deletions(-) diff --git a/sentry_sdk/integrations/fastapi.py b/sentry_sdk/integrations/fastapi.py index c52fe0fbd0..d69dba990f 100644 --- a/sentry_sdk/integrations/fastapi.py +++ b/sentry_sdk/integrations/fastapi.py @@ -2,6 +2,7 @@ from copy import deepcopy from sentry_sdk._types import TYPE_CHECKING +from sentry_sdk.api import continue_trace from sentry_sdk.hub import Hub, _should_send_default_pii from sentry_sdk.integrations import DidNotEnable from sentry_sdk.tracing import SOURCE_FOR_STYLE, TRANSACTION_SOURCE_ROUTE @@ -37,11 +38,8 @@ def setup_once(): patch_get_request_handler() -def _set_transaction_name_and_source(scope, transaction_style, request): +def _get_transaction_name_and_source(scope, transaction_style, request): # type: (Scope, str, Any) -> None - import ipdb - - ipdb.set_trace() name = "" if transaction_style == "endpoint": @@ -62,7 +60,7 @@ def _set_transaction_name_and_source(scope, transaction_style, request): else: source = SOURCE_FOR_STYLE[transaction_style] - scope.set_transaction_name(name, source=source) + return name, source def patch_get_request_handler(): @@ -100,37 +98,48 @@ async def _sentry_app(*args, **kwargs): with hub.configure_scope() as sentry_scope: request = args[0] + scope = request.scope + ty = scope["type"] - _set_transaction_name_and_source( + transaction_name, transaction_source = _get_transaction_name_and_source( sentry_scope, integration.transaction_style, request ) - - extractor = StarletteRequestExtractor(request) - info = await extractor.extract_request_info() - - def _make_request_event_processor(req, integration): - # type: (Any, Any) -> Callable[[Dict[str, Any], Dict[str, Any]], Dict[str, Any]] - def event_processor(event, hint): - # type: (Dict[str, Any], Dict[str, Any]) -> Dict[str, Any] - - # Extract information from request - request_info = event.get("request", {}) - if info: - if "cookies" in info and _should_send_default_pii(): - request_info["cookies"] = info["cookies"] - if "data" in info: - request_info["data"] = info["data"] - event["request"] = deepcopy(request_info) - - return event - - return event_processor - - sentry_scope._name = FastApiIntegration.identifier - sentry_scope.add_event_processor( - _make_request_event_processor(request, integration) + transaction = continue_trace( + integration._get_headers(scope), + op="{}.server".format(ty), + name=transaction_name, + source=transaction_source, ) + with hub.start_transaction( + transaction, custom_sampling_context={"asgi_scope": scope} + ): + extractor = StarletteRequestExtractor(request) + info = await extractor.extract_request_info() + + def _make_request_event_processor(req, integration): + # type: (Any, Any) -> Callable[[Dict[str, Any], Dict[str, Any]], Dict[str, Any]] + def event_processor(event, hint): + # type: (Dict[str, Any], Dict[str, Any]) -> Dict[str, Any] + + # Extract information from request + request_info = event.get("request", {}) + if info: + if "cookies" in info and _should_send_default_pii(): + request_info["cookies"] = info["cookies"] + if "data" in info: + request_info["data"] = info["data"] + event["request"] = deepcopy(request_info) + + return event + + return event_processor + + sentry_scope._name = FastApiIntegration.identifier + sentry_scope.add_event_processor( + _make_request_event_processor(request, integration) + ) + return await old_app(*args, **kwargs) return _sentry_app From b3c66a994a9e918eefd9ec660e9c34858886162c Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Fri, 21 Jul 2023 09:22:55 +0200 Subject: [PATCH 06/14] Cleanup (do not use asgi middleware, and add the get_headers helper) --- sentry_sdk/integrations/starlette.py | 55 +++++++++------------------- 1 file changed, 17 insertions(+), 38 deletions(-) diff --git a/sentry_sdk/integrations/starlette.py b/sentry_sdk/integrations/starlette.py index 09f90249e4..1960b6925c 100644 --- a/sentry_sdk/integrations/starlette.py +++ b/sentry_sdk/integrations/starlette.py @@ -13,7 +13,6 @@ _is_json_content_type, 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.utils import ( AnnotatedValue, @@ -31,7 +30,6 @@ try: import starlette # type: ignore from starlette import __version__ as STARLETTE_VERSION - from starlette.applications import Starlette # type: ignore from starlette.datastructures import UploadFile # type: ignore from starlette.middleware import Middleware # type: ignore from starlette.middleware.authentication import ( # type: ignore @@ -39,7 +37,7 @@ ) from starlette.requests import Request # type: ignore from starlette.routing import Match # type: ignore - from starlette.types import ASGIApp, Receive, Scope as StarletteScope, Send # type: ignore + from starlette.types import ASGIApp # type: ignore except ImportError: raise DidNotEnable("Starlette is not installed") @@ -87,12 +85,26 @@ def setup_once(): ) patch_middlewares() - patch_asgi_app() patch_request_response() if version >= (0, 24): patch_templates() + def _get_headers(self, scope): + # type: (Any) -> Dict[str, str] + """ + Extract headers from the ASGI scope, in the format that the Sentry protocol expects. + """ + headers = {} # type: Dict[str, str] + for raw_key, raw_value in scope["headers"]: + key = raw_key.decode("latin-1") + value = raw_value.decode("latin-1") + if key in headers: + headers[key] = headers[key] + ", " + value + else: + headers[key] = value + return headers + def _enable_span_for_middleware(middleware_class): # type: (Any) -> type @@ -312,9 +324,8 @@ def patch_middlewares(): def _sentry_middleware_init(self, cls, **options): # type: (Any, Any, Any) -> None - if cls == SentryAsgiMiddleware: - return old_middleware_init(self, cls, **options) + # TODO: check if we should return the return value of old_middleware_init at the end of this function? span_enabled_cls = _enable_span_for_middleware(cls) old_middleware_init(self, span_enabled_cls, **options) @@ -327,35 +338,6 @@ def _sentry_middleware_init(self, cls, **options): Middleware.__init__ = _sentry_middleware_init -def patch_asgi_app(): - # type: () -> None - """ - Instrument Starlette ASGI app using the SentryAsgiMiddleware. - """ - old_app = Starlette.__call__ - - async def _sentry_patched_asgi_app(self, scope, receive, send): - # type: (Starlette, StarletteScope, Receive, Send) -> None - if Hub.current.get_integration(StarletteIntegration) is None: - return await old_app(self, scope, receive, send) - - middleware = SentryAsgiMiddleware( - lambda *a, **kw: old_app(self, *a, **kw), - mechanism_type=StarletteIntegration.identifier, - should_start_transaction=False, - ) - - middleware.__call__ = middleware._run_asgi3 - - import ipdb - - ipdb.set_trace() - - return await middleware(scope, receive, send) - - Starlette.__call__ = _sentry_patched_asgi_app - - # This was vendored in from Starlette to support Starlette 0.19.1 because # this function was only introduced in 0.20.x def _is_async_callable(obj): @@ -627,9 +609,6 @@ async def json(self): def _set_transaction_name_and_source(scope, transaction_style, request): # type: (SentryScope, str, Any) -> None - import ipdb - - ipdb.set_trace() name = "" if transaction_style == "endpoint": From 7c97d354ed168b7f2696c7e0de994b7859214911 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 25 Jul 2023 14:36:08 +0200 Subject: [PATCH 07/14] Set transaction name to something useful in Starlette --- sentry_sdk/integrations/starlette.py | 91 ++++++++++++++++++++-------- 1 file changed, 67 insertions(+), 24 deletions(-) diff --git a/sentry_sdk/integrations/starlette.py b/sentry_sdk/integrations/starlette.py index 1960b6925c..d3a86d515e 100644 --- a/sentry_sdk/integrations/starlette.py +++ b/sentry_sdk/integrations/starlette.py @@ -6,6 +6,7 @@ from sentry_sdk._compat import iteritems from sentry_sdk._types import TYPE_CHECKING +from sentry_sdk.api import continue_trace from sentry_sdk.consts import OP from sentry_sdk.hub import Hub, _should_send_default_pii from sentry_sdk.integrations import DidNotEnable, Integration @@ -25,11 +26,10 @@ if TYPE_CHECKING: from typing import Any, Awaitable, Callable, Dict, Optional - from sentry_sdk.scope import Scope as SentryScope - try: import starlette # type: ignore from starlette import __version__ as STARLETTE_VERSION + from starlette.applications import Starlette # type: ignore from starlette.datastructures import UploadFile # type: ignore from starlette.middleware import Middleware # type: ignore from starlette.middleware.authentication import ( # type: ignore @@ -84,6 +84,7 @@ def setup_once(): "Unparsable Starlette version: {}".format(STARLETTE_VERSION) ) + patch_starlette_application() patch_middlewares() patch_request_response() @@ -106,6 +107,42 @@ def _get_headers(self, scope): return headers +def patch_starlette_application(): + old_call = Starlette.__call__ + + def _call(self, *outer_args, **outer_kwargs): + async def _sentry_starlette_call(*args, **kwargs): + hub = Hub.current + integration = hub.get_integration(StarletteIntegration) + if integration is None: + return await old_call(self, *args, **kwargs) + + scope = args[0] + receive = args[1] + request = Request(scope, receive) + + transaction_name, transaction_source = _get_transaction_name_and_source( + integration.transaction_style, request + ) + + ty = scope["type"] + transaction = continue_trace( + integration._get_headers(scope), + op="{}.server".format(ty), + name=transaction_name, + source=transaction_source, + ) + + with hub.start_transaction( + transaction, custom_sampling_context={"asgi_scope": scope} + ): + return await old_call(self, *args, **kwargs) + + return _sentry_starlette_call + + Starlette.__call__ = _call + + def _enable_span_for_middleware(middleware_class): # type: (Any) -> type old_call = middleware_class.__call__ @@ -370,9 +407,15 @@ async def _sentry_async_func(*args, **kwargs): with hub.configure_scope() as sentry_scope: request = args[0] - - _set_transaction_name_and_source( - sentry_scope, integration.transaction_style, request + ( + transaction_name, + transaction_source, + ) = _get_transaction_name_and_source( + integration.transaction_style, request + ) + sentry_scope.set_transaction_name( + name=transaction_name, + source=transaction_source, ) extractor = StarletteRequestExtractor(request) @@ -418,11 +461,6 @@ def _sentry_sync_func(*args, **kwargs): sentry_scope.profile.update_active_thread_id() request = args[0] - - _set_transaction_name_and_source( - sentry_scope, integration.transaction_style, request - ) - extractor = StarletteRequestExtractor(request) cookies = extractor.extract_cookies_from_request() @@ -607,27 +645,32 @@ async def json(self): return await self.request.json() -def _set_transaction_name_and_source(scope, transaction_style, request): - # type: (SentryScope, str, Any) -> None +def _get_transaction_name_and_source(transaction_style, request): + # type: (str, Any) -> None name = "" if transaction_style == "endpoint": endpoint = request.scope.get("endpoint") if endpoint: name = transaction_from_function(endpoint) or "" + else: + name = request.scope.get("raw_path") 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 + router = request.scope.get("router") + if 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 + else: + name = request.scope.get("raw_path") if not name: name = _DEFAULT_TRANSACTION_NAME @@ -635,4 +678,4 @@ def _set_transaction_name_and_source(scope, transaction_style, request): else: source = SOURCE_FOR_STYLE[transaction_style] - scope.set_transaction_name(name, source=source) + return name, source From cd9abc856fda67768f0b1984696646a433dbe12b Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Thu, 27 Jul 2023 09:31:48 +0200 Subject: [PATCH 08/14] Fix monkey patching of Starlette __call__ method. --- sentry_sdk/integrations/starlette.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/sentry_sdk/integrations/starlette.py b/sentry_sdk/integrations/starlette.py index d3a86d515e..78cd84bf0a 100644 --- a/sentry_sdk/integrations/starlette.py +++ b/sentry_sdk/integrations/starlette.py @@ -108,17 +108,17 @@ def _get_headers(self, scope): def patch_starlette_application(): - old_call = Starlette.__call__ - - def _call(self, *outer_args, **outer_kwargs): - async def _sentry_starlette_call(*args, **kwargs): + # type: () -> None + def wrap_call_function(func): + # type: (Callable[..., Any]) -> Callable[..., Any] + @functools.wraps(func) + async def __sentry_call__(self, scope, receive, send): + # type: (Starlette, Dict[str, Any], Callable[[], Awaitable[Dict[str, Any]]], Callable[[Dict[str, Any]], Awaitable[None]]) -> None hub = Hub.current integration = hub.get_integration(StarletteIntegration) if integration is None: - return await old_call(self, *args, **kwargs) + await func(self, scope, receive, send) - scope = args[0] - receive = args[1] request = Request(scope, receive) transaction_name, transaction_source = _get_transaction_name_and_source( @@ -136,11 +136,11 @@ async def _sentry_starlette_call(*args, **kwargs): with hub.start_transaction( transaction, custom_sampling_context={"asgi_scope": scope} ): - return await old_call(self, *args, **kwargs) + await func(self, scope, receive, send) - return _sentry_starlette_call + return __sentry_call__ - Starlette.__call__ = _call + Starlette.__call__ = wrap_call_function(Starlette.__call__) def _enable_span_for_middleware(middleware_class): From 7be2d1f0769b6a8bc2ec95866d31a01068da84f7 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Thu, 27 Jul 2023 09:54:47 +0200 Subject: [PATCH 09/14] Do not create transaction in FastAPI, becauese it is done in Starlette integration --- sentry_sdk/integrations/fastapi.py | 64 +++++++++++++----------------- 1 file changed, 27 insertions(+), 37 deletions(-) diff --git a/sentry_sdk/integrations/fastapi.py b/sentry_sdk/integrations/fastapi.py index d69dba990f..43e40aaa7f 100644 --- a/sentry_sdk/integrations/fastapi.py +++ b/sentry_sdk/integrations/fastapi.py @@ -2,7 +2,6 @@ from copy import deepcopy from sentry_sdk._types import TYPE_CHECKING -from sentry_sdk.api import continue_trace from sentry_sdk.hub import Hub, _should_send_default_pii from sentry_sdk.integrations import DidNotEnable from sentry_sdk.tracing import SOURCE_FOR_STYLE, TRANSACTION_SOURCE_ROUTE @@ -98,47 +97,38 @@ async def _sentry_app(*args, **kwargs): with hub.configure_scope() as sentry_scope: request = args[0] - scope = request.scope - ty = scope["type"] - transaction_name, transaction_source = _get_transaction_name_and_source( sentry_scope, integration.transaction_style, request ) - transaction = continue_trace( - integration._get_headers(scope), - op="{}.server".format(ty), - name=transaction_name, - source=transaction_source, + sentry_scope.set_transaction_name( + transaction_name, source=transaction_source ) - with hub.start_transaction( - transaction, custom_sampling_context={"asgi_scope": scope} - ): - extractor = StarletteRequestExtractor(request) - info = await extractor.extract_request_info() - - def _make_request_event_processor(req, integration): - # type: (Any, Any) -> Callable[[Dict[str, Any], Dict[str, Any]], Dict[str, Any]] - def event_processor(event, hint): - # type: (Dict[str, Any], Dict[str, Any]) -> Dict[str, Any] - - # Extract information from request - request_info = event.get("request", {}) - if info: - if "cookies" in info and _should_send_default_pii(): - request_info["cookies"] = info["cookies"] - if "data" in info: - request_info["data"] = info["data"] - event["request"] = deepcopy(request_info) - - return event - - return event_processor - - sentry_scope._name = FastApiIntegration.identifier - sentry_scope.add_event_processor( - _make_request_event_processor(request, integration) - ) + extractor = StarletteRequestExtractor(request) + info = await extractor.extract_request_info() + + def _make_request_event_processor(req, integration): + # type: (Any, Any) -> Callable[[Dict[str, Any], Dict[str, Any]], Dict[str, Any]] + def event_processor(event, hint): + # type: (Dict[str, Any], Dict[str, Any]) -> Dict[str, Any] + + # Extract information from request + request_info = event.get("request", {}) + if info: + if "cookies" in info and _should_send_default_pii(): + request_info["cookies"] = info["cookies"] + if "data" in info: + request_info["data"] = info["data"] + event["request"] = deepcopy(request_info) + + return event + + return event_processor + + sentry_scope._name = FastApiIntegration.identifier + sentry_scope.add_event_processor( + _make_request_event_processor(request, integration) + ) return await old_app(*args, **kwargs) From 1d7b2f440a06ed601f35e83ac6c265896102966a Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 29 Aug 2023 16:47:01 +0200 Subject: [PATCH 10/14] Some tests --- sentry_sdk/integrations/starlette.py | 18 +++- tests/integrations/fastapi/test_fastapi.py | 29 ++++++ .../integrations/starlette/test_starlette.py | 94 +++++++++++++++++++ 3 files changed, 136 insertions(+), 5 deletions(-) diff --git a/sentry_sdk/integrations/starlette.py b/sentry_sdk/integrations/starlette.py index 78cd84bf0a..27b2656aaa 100644 --- a/sentry_sdk/integrations/starlette.py +++ b/sentry_sdk/integrations/starlette.py @@ -14,7 +14,11 @@ _is_json_content_type, request_body_within_bounds, ) -from sentry_sdk.tracing import SOURCE_FOR_STYLE, TRANSACTION_SOURCE_ROUTE +from sentry_sdk.tracing import ( + SOURCE_FOR_STYLE, + TRANSACTION_SOURCE_ROUTE, + TRANSACTION_SOURCE_URL, +) from sentry_sdk.utils import ( AnnotatedValue, capture_internal_exceptions, @@ -647,14 +651,17 @@ async def json(self): def _get_transaction_name_and_source(transaction_style, request): # type: (str, Any) -> None - name = "" + name = None + source = None if transaction_style == "endpoint": endpoint = request.scope.get("endpoint") if endpoint: name = transaction_from_function(endpoint) or "" + source = SOURCE_FOR_STYLE[transaction_style] else: name = request.scope.get("raw_path") + source = TRANSACTION_SOURCE_URL elif transaction_style == "url": router = request.scope.get("router") @@ -665,17 +672,18 @@ def _get_transaction_name_and_source(transaction_style, request): if match[0] == Match.FULL: if transaction_style == "endpoint": name = transaction_from_function(match[1]["endpoint"]) or "" + source = SOURCE_FOR_STYLE[transaction_style] break elif transaction_style == "url": name = route.path + source = SOURCE_FOR_STYLE[transaction_style] break else: name = request.scope.get("raw_path") + source = TRANSACTION_SOURCE_URL - if not name: + if name is None: name = _DEFAULT_TRANSACTION_NAME source = TRANSACTION_SOURCE_ROUTE - else: - source = SOURCE_FOR_STYLE[transaction_style] return name, source diff --git a/tests/integrations/fastapi/test_fastapi.py b/tests/integrations/fastapi/test_fastapi.py index 5a770a70af..8967878899 100644 --- a/tests/integrations/fastapi/test_fastapi.py +++ b/tests/integrations/fastapi/test_fastapi.py @@ -322,3 +322,32 @@ 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 + + +def test_transaction_name(): + """ + Tests that the transaction name is something meaningful. + """ + # this: sampling_context["transaction_context"]["name"] + # for transaction_style "endpoint" and "url" + assert False + + +def test_transaction_name_in_traces_sampler(): + """ + Tests that a custom traces_sampler has a meaningful the transaction name + In this case the URL or endpoint, because we do not have the route yet. + """ + # this: sampling_context["transaction_context"]["name"] + # for transaction_style "endpoint" and "url" + assert False + + +def test_transaction_name_in_middleware(): + """ + Tests that the transaction name in the middleware + (like CORSMiddleware) is something meaningful. + In this case the URL or endpoint, because we do not have the route yet. + """ + # for transaction_style "endpoint" and "url" + assert False diff --git a/tests/integrations/starlette/test_starlette.py b/tests/integrations/starlette/test_starlette.py index cc4d8cf3ba..f1dcb8bbe6 100644 --- a/tests/integrations/starlette/test_starlette.py +++ b/tests/integrations/starlette/test_starlette.py @@ -949,3 +949,97 @@ 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( + "transaction_style,expected_transaction_name,expected_transaction_source", + [ + ( + "endpoint", + "tests.integrations.starlette.test_starlette.starlette_app_factory.._message_with_id", + "component", + ), + ( + "url", + "/message/{message_id}", + "route", + ), + ], +) +def test_transaction_name( + sentry_init, + transaction_style, + capture_envelopes, + expected_transaction_name, + expected_transaction_source, +): + """ + 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, + ) + + envelopes = capture_envelopes() + + app = starlette_app_factory() + client = TestClient(app) + client.get("/message/123456") + + (_, 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( + "transaction_style,expected_transaction_name,expected_transaction_source", + [ + ("endpoint", b"/message/123456", "url"), + ("url", b"/message/123456", "url"), + ], +) +def test_transaction_name_in_traces_sampler( + sentry_init, + 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, + ) + + app = starlette_app_factory() + client = TestClient(app) + client.get("/message/123456") + + +def test_transaction_name_in_middleware(): + """ + Tests that the transaction name in the middleware (like CORSMiddleware) is something meaningful. + In this case the URL or endpoint, because we do not have the route yet. + """ + # for transaction_style "endpoint" and "url" + assert False From a1252a8db7766f2f6ca3b72cf1a4a01671e3e5cc Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 30 Aug 2023 11:04:47 +0200 Subject: [PATCH 11/14] Fixed some tests --- sentry_sdk/integrations/asgi.py | 65 +++++++++++++++++--------- tests/integrations/asgi/test_asgi.py | 68 +++++++++++++++++++--------- 2 files changed, 90 insertions(+), 43 deletions(-) diff --git a/sentry_sdk/integrations/asgi.py b/sentry_sdk/integrations/asgi.py index 25846cfc6e..84cb2ff2cf 100644 --- a/sentry_sdk/integrations/asgi.py +++ b/sentry_sdk/integrations/asgi.py @@ -20,6 +20,7 @@ from sentry_sdk.tracing import ( SOURCE_FOR_STYLE, TRANSACTION_SOURCE_ROUTE, + TRANSACTION_SOURCE_URL, ) from sentry_sdk.utils import ( ContextVar, @@ -168,15 +169,21 @@ async def _run_app(self, scope, receive, send, asgi_version): ty = scope["type"] if ty in ("http", "websocket"): + ( + transaction_name, + transaction_source, + ) = self._get_transaction_name_and_source( + self.transaction_style, scope + ) transaction = continue_trace( self._get_headers(scope), op="{}.server".format(ty), + name=transaction_name, + source=transaction_source, ) else: transaction = Transaction(op=OP.HTTP_SERVER) - transaction.name = _DEFAULT_TRANSACTION_NAME - transaction.source = TRANSACTION_SOURCE_ROUTE transaction.set_tag("asgi.type", ty) with hub.start_transaction( @@ -232,7 +239,11 @@ def event_processor(self, event, hint, asgi_scope): if client and _should_send_default_pii(): request_info["env"] = {"REMOTE_ADDR": self._get_ip(asgi_scope)} - self._set_transaction_name_and_source(event, self.transaction_style, asgi_scope) + transaction_name, transaction_source = self._get_transaction_name_and_source( + self.transaction_style, asgi_scope + ) + event["transaction"] = transaction_name + event["transaction_info"] = {"source": transaction_source} event["request"] = deepcopy(request_info) @@ -244,16 +255,10 @@ 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): + def _get_transaction_name_and_source(self, 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 = "" + name = None + source = None if transaction_style == "endpoint": endpoint = asgi_scope.get("endpoint") @@ -262,23 +267,41 @@ 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 "" + source = SOURCE_FOR_STYLE[transaction_style] + else: + ty = asgi_scope.get("type") + if ty in ("http", "websocket"): + name = self._get_url( + asgi_scope, "http" if ty == "http" else "ws", host=None + ) + source = TRANSACTION_SOURCE_URL + else: + name = _DEFAULT_TRANSACTION_NAME + source = TRANSACTION_SOURCE_ROUTE elif transaction_style == "url": # FastAPI includes the route object in the scope to let Sentry extract the # path from it for the transaction name route = asgi_scope.get("route") if route: - path = getattr(route, "path", None) - if path is not None: - name = path + name = route + source = SOURCE_FOR_STYLE[transaction_style] + else: + ty = asgi_scope.get("type") + if ty in ("http", "websocket"): + name = self._get_url( + asgi_scope, "http" if ty == "http" else "ws", host=None + ) + source = TRANSACTION_SOURCE_URL + else: + name = _DEFAULT_TRANSACTION_NAME + source = TRANSACTION_SOURCE_ROUTE - if not name: - event["transaction"] = _DEFAULT_TRANSACTION_NAME - event["transaction_info"] = {"source": TRANSACTION_SOURCE_ROUTE} - return + if name is None: + name = _DEFAULT_TRANSACTION_NAME + source = TRANSACTION_SOURCE_ROUTE - event["transaction"] = name - event["transaction_info"] = {"source": SOURCE_FOR_STYLE[transaction_style]} + return name, source def _get_url(self, scope, default_scheme, host): # type: (Dict[str, Any], Literal["ws", "http"], Optional[str]) -> str diff --git a/tests/integrations/asgi/test_asgi.py b/tests/integrations/asgi/test_asgi.py index ec65467b34..35eeafb918 100644 --- a/tests/integrations/asgi/test_asgi.py +++ b/tests/integrations/asgi/test_asgi.py @@ -19,7 +19,17 @@ @pytest.fixture def asgi3_app(): async def app(scope, receive, send): - if ( + if scope["type"] == "lifespan": + while True: + message = await receive() + if message["type"] == "lifespan.startup": + ... # Do some startup here! + await send({"type": "lifespan.startup.complete"}) + elif message["type"] == "lifespan.shutdown": + ... # Do some shutdown here! + await send({"type": "lifespan.shutdown.complete"}) + return + elif ( scope["type"] == "http" and "route" in scope and scope["route"] == "/trigger/error" @@ -52,21 +62,32 @@ async def send_with_error(event): 1 / 0 async def app(scope, receive, send): - await send_with_error( - { - "type": "http.response.start", - "status": 200, - "headers": [ - [b"content-type", b"text/plain"], - ], - } - ) - await send_with_error( - { - "type": "http.response.body", - "body": b"Hello, world!", - } - ) + if scope["type"] == "lifespan": + while True: + message = await receive() + if message["type"] == "lifespan.startup": + ... # Do some startup here! + await send({"type": "lifespan.startup.complete"}) + elif message["type"] == "lifespan.shutdown": + ... # Do some shutdown here! + await send({"type": "lifespan.shutdown.complete"}) + return + else: + await send_with_error( + { + "type": "http.response.start", + "status": 200, + "headers": [ + [b"content-type", b"text/plain"], + ], + } + ) + await send_with_error( + { + "type": "http.response.body", + "body": b"Hello, world!", + } + ) return app @@ -139,10 +160,11 @@ async def test_capture_transaction( events = capture_events() await client.get("/?somevalue=123") - (transaction_event,) = events + (transaction_event, lifespan_transaction_event) = events assert transaction_event["type"] == "transaction" - assert transaction_event["transaction"] == "http://localhost/" + assert transaction_event["transaction"] == "/" + assert transaction_event["transaction_info"] == {"source": "url"} assert transaction_event["contexts"]["trace"]["op"] == "http.server" assert transaction_event["request"] == { "headers": { @@ -172,15 +194,17 @@ async def test_capture_transaction_with_error( async with TestClient(app) as client: await client.get("/") - (error_event, transaction_event) = events + (error_event, transaction_event, lifespan_transaction_event) = events - assert error_event["transaction"] == "http://localhost/" + assert error_event["transaction"] == "/" 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" assert error_event["exception"]["values"][0]["mechanism"]["handled"] is False assert error_event["exception"]["values"][0]["mechanism"]["type"] == "asgi" + assert transaction_event["transaction"] == "/" + assert transaction_event["transaction_info"] == {"source": "url"} assert transaction_event["type"] == "transaction" assert transaction_event["contexts"]["trace"] == DictionaryContaining( error_event["contexts"]["trace"] @@ -389,7 +413,7 @@ async def test_auto_session_tracking_with_aggregates( ( "/message", "url", - "http://localhost/message", + "/message", "route", ), ( @@ -423,7 +447,7 @@ async def test_transaction_style( events = capture_events() await client.get(url) - (transaction_event,) = events + (transaction_event, lifespan_transaction_event) = events assert transaction_event["transaction"] == expected_transaction assert transaction_event["transaction_info"] == {"source": expected_source} From c37ee12dee414d45ddef5a13869b95b061d3518d Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Thu, 31 Aug 2023 16:03:13 +0200 Subject: [PATCH 12/14] Moved things around to better have common funcitons used by asgi and starlette --- sentry_sdk/integrations/_asgi_common.py | 104 +++++++++++++++ sentry_sdk/integrations/asgi.py | 124 ++++-------------- sentry_sdk/integrations/starlette.py | 43 +++--- tests/integrations/asgi/test_asgi.py | 28 ++-- .../integrations/starlette/test_starlette.py | 44 ++++++- 5 files changed, 202 insertions(+), 141 deletions(-) create mode 100644 sentry_sdk/integrations/_asgi_common.py diff --git a/sentry_sdk/integrations/_asgi_common.py b/sentry_sdk/integrations/_asgi_common.py new file mode 100644 index 0000000000..3d14393b03 --- /dev/null +++ b/sentry_sdk/integrations/_asgi_common.py @@ -0,0 +1,104 @@ +import urllib + +from sentry_sdk.hub import _should_send_default_pii +from sentry_sdk.integrations._wsgi_common import _filter_headers +from sentry_sdk._types import TYPE_CHECKING + +if TYPE_CHECKING: + from typing import Any + from typing import Dict + from typing import Optional + from typing_extensions import Literal + + +def _get_headers(asgi_scope): + # type: (Any) -> Dict[str, str] + """ + Extract headers from the ASGI scope, in the format that the Sentry protocol expects. + """ + headers = {} # type: Dict[str, str] + for raw_key, raw_value in asgi_scope["headers"]: + key = raw_key.decode("latin-1") + value = raw_value.decode("latin-1") + if key in headers: + headers[key] = headers[key] + ", " + value + else: + headers[key] = value + + return headers + + +def _get_url(asgi_scope, default_scheme, host): + # type: (Dict[str, Any], Literal["ws", "http"], Optional[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) + + if server is not None: + host, port = server + default_port = {"http": 80, "https": 443, "ws": 80, "wss": 443}[scheme] + if port != default_port: + return "%s://%s:%s%s" % (scheme, host, port, path) + return "%s://%s%s" % (scheme, host, path) + return path + + +def _get_query(asgi_scope): + # type: (Any) -> Any + """ + Extract querystring from the ASGI scope, in the format that the Sentry protocol expects. + """ + qs = asgi_scope.get("query_string") + if not qs: + return None + return urllib.parse.unquote(qs.decode("latin-1")) + + +def _get_ip(asgi_scope): + # type: (Any) -> str + """ + Extract IP Address from the ASGI scope based on request headers with fallback to scope client. + """ + headers = _get_headers(asgi_scope) + try: + return headers["x-forwarded-for"].split(",")[0].strip() + except (KeyError, IndexError): + pass + + try: + return headers["x-real-ip"] + except KeyError: + pass + + return asgi_scope.get("client")[0] + + +def _get_request_data(asgi_scope): + # type: (Any) -> Dict[str, Any] + """ + Returns data related to the HTTP request from the ASGI scope. + """ + request_data = {} # type: Dict[str, Any] + ty = asgi_scope["type"] + if ty in ("http", "websocket"): + request_data["method"] = asgi_scope.get("method") + + request_data["headers"] = headers = _filter_headers(_get_headers(asgi_scope)) + request_data["query_string"] = _get_query(asgi_scope) + + request_data["url"] = _get_url( + asgi_scope, "http" if ty == "http" else "ws", headers.get("host") + ) + + client = asgi_scope.get("client") + if client and _should_send_default_pii(): + request_data["env"] = {"REMOTE_ADDR": _get_ip(asgi_scope)} + + return request_data diff --git a/sentry_sdk/integrations/asgi.py b/sentry_sdk/integrations/asgi.py index 84cb2ff2cf..dedebfcddb 100644 --- a/sentry_sdk/integrations/asgi.py +++ b/sentry_sdk/integrations/asgi.py @@ -6,15 +6,19 @@ import asyncio import inspect -import urllib +import logging from copy import deepcopy from sentry_sdk._functools import partial from sentry_sdk._types import TYPE_CHECKING from sentry_sdk.api import continue_trace from sentry_sdk.consts import OP -from sentry_sdk.hub import Hub, _should_send_default_pii -from sentry_sdk.integrations._wsgi_common import _filter_headers +from sentry_sdk.hub import Hub +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 ( @@ -38,8 +42,6 @@ from typing import Optional from typing import Callable - from typing_extensions import Literal - from sentry_sdk._types import Event, Hint @@ -176,16 +178,28 @@ async def _run_app(self, scope, receive, send, asgi_version): self.transaction_style, scope ) transaction = continue_trace( - self._get_headers(scope), + _get_headers(scope), op="{}.server".format(ty), name=transaction_name, source=transaction_source, ) + logging.warning( + "[ASGI] Created Transaction %s, %s, %s", + transaction, + transaction.sampled, + transaction_name, + ) else: transaction = Transaction(op=OP.HTTP_SERVER) transaction.set_tag("asgi.type", ty) + logging.warning( + "[ASGI] Starting Transaction %s, %s, %s", + transaction, + transaction.sampled, + transaction.name, + ) with hub.start_transaction( transaction, custom_sampling_context={"asgi_scope": scope} ): @@ -221,23 +235,9 @@ async def _sentry_wrapped_send(event): def event_processor(self, event, hint, asgi_scope): # type: (Event, Hint, Any) -> Optional[Event] - request_info = event.get("request", {}) - - ty = asgi_scope["type"] - if ty in ("http", "websocket"): - request_info["method"] = asgi_scope.get("method") - request_info["headers"] = headers = _filter_headers( - self._get_headers(asgi_scope) - ) - request_info["query_string"] = self._get_query(asgi_scope) - - request_info["url"] = self._get_url( - asgi_scope, "http" if ty == "http" else "ws", headers.get("host") - ) - - client = asgi_scope.get("client") - if client and _should_send_default_pii(): - request_info["env"] = {"REMOTE_ADDR": self._get_ip(asgi_scope)} + request_data = event.get("request", {}) + request_data.update(_get_request_data(asgi_scope)) + event["request"] = deepcopy(request_data) transaction_name, transaction_source = self._get_transaction_name_and_source( self.transaction_style, asgi_scope @@ -245,16 +245,8 @@ def event_processor(self, event, hint, asgi_scope): event["transaction"] = transaction_name event["transaction_info"] = {"source": transaction_source} - event["request"] = deepcopy(request_info) - return event - # Helper functions for extracting request data. - # - # Note: Those functions are not public API. If you want to mutate request - # data to your liking it's recommended to use the `before_send` callback - # for that. - def _get_transaction_name_and_source(self, transaction_style, asgi_scope): # type: (Event, str, Any) -> None name = None @@ -271,7 +263,7 @@ def _get_transaction_name_and_source(self, transaction_style, asgi_scope): else: ty = asgi_scope.get("type") if ty in ("http", "websocket"): - name = self._get_url( + name = _get_url( asgi_scope, "http" if ty == "http" else "ws", host=None ) source = TRANSACTION_SOURCE_URL @@ -280,8 +272,6 @@ def _get_transaction_name_and_source(self, transaction_style, asgi_scope): source = TRANSACTION_SOURCE_ROUTE elif transaction_style == "url": - # FastAPI includes the route object in the scope to let Sentry extract the - # path from it for the transaction name route = asgi_scope.get("route") if route: name = route @@ -289,7 +279,7 @@ def _get_transaction_name_and_source(self, transaction_style, asgi_scope): else: ty = asgi_scope.get("type") if ty in ("http", "websocket"): - name = self._get_url( + name = _get_url( asgi_scope, "http" if ty == "http" else "ws", host=None ) source = TRANSACTION_SOURCE_URL @@ -302,67 +292,3 @@ def _get_transaction_name_and_source(self, transaction_style, asgi_scope): source = TRANSACTION_SOURCE_ROUTE return name, source - - def _get_url(self, scope, default_scheme, host): - # type: (Dict[str, Any], Literal["ws", "http"], Optional[str]) -> str - """ - Extract URL from the ASGI scope, without also including the querystring. - """ - scheme = scope.get("scheme", default_scheme) - - server = scope.get("server", None) - path = scope.get("root_path", "") + scope.get("path", "") - - if host: - return "%s://%s%s" % (scheme, host, path) - - if server is not None: - host, port = server - default_port = {"http": 80, "https": 443, "ws": 80, "wss": 443}[scheme] - if port != default_port: - return "%s://%s:%s%s" % (scheme, host, port, path) - return "%s://%s%s" % (scheme, host, path) - return path - - def _get_query(self, scope): - # type: (Any) -> Any - """ - Extract querystring from the ASGI scope, in the format that the Sentry protocol expects. - """ - qs = scope.get("query_string") - if not qs: - return None - return urllib.parse.unquote(qs.decode("latin-1")) - - def _get_ip(self, scope): - # type: (Any) -> str - """ - Extract IP Address from the ASGI scope based on request headers with fallback to scope client. - """ - headers = self._get_headers(scope) - try: - return headers["x-forwarded-for"].split(",")[0].strip() - except (KeyError, IndexError): - pass - - try: - return headers["x-real-ip"] - except KeyError: - pass - - return scope.get("client")[0] - - def _get_headers(self, scope): - # type: (Any) -> Dict[str, str] - """ - Extract headers from the ASGI scope, in the format that the Sentry protocol expects. - """ - headers = {} # type: Dict[str, str] - for raw_key, raw_value in scope["headers"]: - key = raw_key.decode("latin-1") - value = raw_value.decode("latin-1") - if key in headers: - headers[key] = headers[key] + ", " + value - else: - headers[key] = value - return headers diff --git a/sentry_sdk/integrations/starlette.py b/sentry_sdk/integrations/starlette.py index 27b2656aaa..c37dcd52da 100644 --- a/sentry_sdk/integrations/starlette.py +++ b/sentry_sdk/integrations/starlette.py @@ -3,6 +3,7 @@ import asyncio import functools from copy import deepcopy +import logging from sentry_sdk._compat import iteritems from sentry_sdk._types import TYPE_CHECKING @@ -10,10 +11,12 @@ from sentry_sdk.consts import OP from sentry_sdk.hub import Hub, _should_send_default_pii from sentry_sdk.integrations import DidNotEnable, Integration +from sentry_sdk.integrations._asgi_common import _get_headers from sentry_sdk.integrations._wsgi_common import ( _is_json_content_type, request_body_within_bounds, ) + from sentry_sdk.tracing import ( SOURCE_FOR_STYLE, TRANSACTION_SOURCE_ROUTE, @@ -95,21 +98,6 @@ def setup_once(): if version >= (0, 24): patch_templates() - def _get_headers(self, scope): - # type: (Any) -> Dict[str, str] - """ - Extract headers from the ASGI scope, in the format that the Sentry protocol expects. - """ - headers = {} # type: Dict[str, str] - for raw_key, raw_value in scope["headers"]: - key = raw_key.decode("latin-1") - value = raw_value.decode("latin-1") - if key in headers: - headers[key] = headers[key] + ", " + value - else: - headers[key] = value - return headers - def patch_starlette_application(): # type: () -> None @@ -119,6 +107,7 @@ def wrap_call_function(func): async def __sentry_call__(self, scope, receive, send): # type: (Starlette, Dict[str, Any], Callable[[], Awaitable[Dict[str, Any]]], Callable[[Dict[str, Any]], Awaitable[None]]) -> None hub = Hub.current + integration = hub.get_integration(StarletteIntegration) if integration is None: await func(self, scope, receive, send) @@ -131,16 +120,32 @@ async def __sentry_call__(self, scope, receive, send): ty = scope["type"] transaction = continue_trace( - integration._get_headers(scope), + _get_headers(scope), op="{}.server".format(ty), name=transaction_name, source=transaction_source, ) + logging.warning( + "[Starlette] Created Transaction %s, %s, %s", + transaction, + transaction.sampled, + transaction_name, + ) + logging.warning( + "[Starlette] Starting Transaction %s, %s, %s", + transaction, + transaction.sampled, + transaction_name, + ) with hub.start_transaction( transaction, custom_sampling_context={"asgi_scope": scope} ): - await func(self, scope, receive, send) + try: + await func(self, scope, receive, send) + except Exception as exc: + _capture_exception(exc, handled=False) + raise exc from None return __sentry_call__ @@ -417,6 +422,10 @@ async def _sentry_async_func(*args, **kwargs): ) = _get_transaction_name_and_source( integration.transaction_style, request ) + logging.warning( + "[Starlette] Setting Transaction Name %s", transaction_name + ) + sentry_scope.set_transaction_name( name=transaction_name, source=transaction_source, diff --git a/tests/integrations/asgi/test_asgi.py b/tests/integrations/asgi/test_asgi.py index 35eeafb918..dca2f377ad 100644 --- a/tests/integrations/asgi/test_asgi.py +++ b/tests/integrations/asgi/test_asgi.py @@ -6,6 +6,7 @@ import sentry_sdk from sentry_sdk import capture_message from sentry_sdk.integrations.asgi import SentryAsgiMiddleware, _looks_like_asgi3 +from sentry_sdk.integrations.asgi import _get_ip, _get_headers async_asgi_testclient = pytest.importorskip("async_asgi_testclient") from async_asgi_testclient import TestClient @@ -496,8 +497,7 @@ def test_get_ip_x_forwarded_for(): "client": ("127.0.0.1", 60457), "headers": headers, } - middleware = SentryAsgiMiddleware({}) - ip = middleware._get_ip(scope) + ip = _get_ip(scope) assert ip == "8.8.8.8" # x-forwarded-for overrides x-real-ip @@ -509,8 +509,7 @@ def test_get_ip_x_forwarded_for(): "client": ("127.0.0.1", 60457), "headers": headers, } - middleware = SentryAsgiMiddleware({}) - ip = middleware._get_ip(scope) + ip = _get_ip(scope) assert ip == "8.8.8.8" # when multiple x-forwarded-for headers are, the first is taken @@ -523,8 +522,7 @@ def test_get_ip_x_forwarded_for(): "client": ("127.0.0.1", 60457), "headers": headers, } - middleware = SentryAsgiMiddleware({}) - ip = middleware._get_ip(scope) + ip = _get_ip(scope) assert ip == "5.5.5.5" @@ -537,8 +535,7 @@ def test_get_ip_x_real_ip(): "client": ("127.0.0.1", 60457), "headers": headers, } - middleware = SentryAsgiMiddleware({}) - ip = middleware._get_ip(scope) + ip = _get_ip(scope) assert ip == "10.10.10.10" # x-forwarded-for overrides x-real-ip @@ -550,8 +547,7 @@ def test_get_ip_x_real_ip(): "client": ("127.0.0.1", 60457), "headers": headers, } - middleware = SentryAsgiMiddleware({}) - ip = middleware._get_ip(scope) + ip = _get_ip(scope) assert ip == "8.8.8.8" @@ -563,8 +559,7 @@ def test_get_ip(): "client": ("127.0.0.1", 60457), "headers": headers, } - middleware = SentryAsgiMiddleware({}) - ip = middleware._get_ip(scope) + ip = _get_ip(scope) assert ip == "127.0.0.1" # x-forwarded-for header overides the ip from client @@ -575,8 +570,7 @@ def test_get_ip(): "client": ("127.0.0.1", 60457), "headers": headers, } - middleware = SentryAsgiMiddleware({}) - ip = middleware._get_ip(scope) + ip = _get_ip(scope) assert ip == "8.8.8.8" # x-real-for header overides the ip from client @@ -587,8 +581,7 @@ def test_get_ip(): "client": ("127.0.0.1", 60457), "headers": headers, } - middleware = SentryAsgiMiddleware({}) - ip = middleware._get_ip(scope) + ip = _get_ip(scope) assert ip == "10.10.10.10" @@ -603,8 +596,7 @@ def test_get_headers(): "client": ("127.0.0.1", 60457), "headers": headers, } - middleware = SentryAsgiMiddleware({}) - headers = middleware._get_headers(scope) + headers = _get_headers(scope) assert headers == { "x-real-ip": "10.10.10.10", "some_header": "123, abc", diff --git a/tests/integrations/starlette/test_starlette.py b/tests/integrations/starlette/test_starlette.py index f1dcb8bbe6..d9e8f8742c 100644 --- a/tests/integrations/starlette/test_starlette.py +++ b/tests/integrations/starlette/test_starlette.py @@ -700,7 +700,9 @@ def test_middleware_callback_spans(sentry_init, capture_events): }, { "op": "middleware.starlette.send", - "description": "SentryAsgiMiddleware._run_app.._sentry_wrapped_send", + "description": "_ASGIAdapter.send..send" + if STARLETTE_VERSION < (0, 21) + else "_TestClientTransport.handle_request..send", "tags": {"starlette.middleware_name": "ServerErrorMiddleware"}, }, { @@ -715,7 +717,9 @@ def test_middleware_callback_spans(sentry_init, capture_events): }, { "op": "middleware.starlette.send", - "description": "SentryAsgiMiddleware._run_app.._sentry_wrapped_send", + "description": "_ASGIAdapter.send..send" + if STARLETTE_VERSION < (0, 21) + else "_TestClientTransport.handle_request..send", "tags": {"starlette.middleware_name": "ServerErrorMiddleware"}, }, ] @@ -789,7 +793,9 @@ def test_middleware_partial_receive_send(sentry_init, capture_events): }, { "op": "middleware.starlette.send", - "description": "SentryAsgiMiddleware._run_app.._sentry_wrapped_send", + "description": "_ASGIAdapter.send..send" + if STARLETTE_VERSION < (0, 21) + else "_TestClientTransport.handle_request..send", "tags": {"starlette.middleware_name": "ServerErrorMiddleware"}, }, { @@ -830,7 +836,7 @@ def handler(request, exc): app = starlette_app_factory(debug=False) app.add_exception_handler(500, handler) - client = TestClient(SentryAsgiMiddleware(app), raise_server_exceptions=False) + client = TestClient(app, raise_server_exceptions=False) response = client.get("/custom_error") assert response.status_code == 500 @@ -858,7 +864,32 @@ def test_legacy_setup( client.get("/message/123456") (event,) = events - assert event["transaction"] == "/message/{message_id}" + assert ( + event["transaction"] + == "tests.integrations.starlette.test_starlette.starlette_app_factory.._message_with_id" + ) + + +def test_legacy_setup_transaction_style_url( + sentry_init, + capture_events, +): + # Check that behaviour does not change + # if the user just adds the new Integration + # and forgets to remove SentryAsgiMiddleware + sentry_init() + app = starlette_app_factory() + asgi_app = SentryAsgiMiddleware(app, transaction_style="url") + + events = capture_events() + + client = TestClient(asgi_app) + client.get("/message/123456") + + (event,) = events + assert ( + event["transaction"] == "http://testserver/message/123456" + ) # the url from AsgiMiddleware (because it does not know about routes) @pytest.mark.parametrize("endpoint", ["/sync/thread_ids", "/async/thread_ids"]) @@ -869,11 +900,10 @@ def test_active_thread_id(sentry_init, capture_envelopes, teardown_profiling, en _experiments={"profiles_sample_rate": 1.0}, ) app = starlette_app_factory() - asgi_app = SentryAsgiMiddleware(app) envelopes = capture_envelopes() - client = TestClient(asgi_app) + client = TestClient(app) response = client.get(endpoint) assert response.status_code == 200 From 13efe51a4ebf59d24660a5e9ea981f06ef85dc56 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Thu, 31 Aug 2023 16:21:20 +0200 Subject: [PATCH 13/14] Fixed import --- tests/integrations/asgi/test_asgi.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integrations/asgi/test_asgi.py b/tests/integrations/asgi/test_asgi.py index dca2f377ad..e4fc90cdc2 100644 --- a/tests/integrations/asgi/test_asgi.py +++ b/tests/integrations/asgi/test_asgi.py @@ -6,7 +6,7 @@ import sentry_sdk from sentry_sdk import capture_message from sentry_sdk.integrations.asgi import SentryAsgiMiddleware, _looks_like_asgi3 -from sentry_sdk.integrations.asgi import _get_ip, _get_headers +from sentry_sdk.integrations._asgi_common import _get_ip, _get_headers async_asgi_testclient = pytest.importorskip("async_asgi_testclient") from async_asgi_testclient import TestClient From 55f53dd687f7d5d76f7a7681022c39422d66b856 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Thu, 31 Aug 2023 16:37:55 +0200 Subject: [PATCH 14/14] Adding asgi request data to sentry event --- sentry_sdk/integrations/starlette.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/sentry_sdk/integrations/starlette.py b/sentry_sdk/integrations/starlette.py index c37dcd52da..23529e089f 100644 --- a/sentry_sdk/integrations/starlette.py +++ b/sentry_sdk/integrations/starlette.py @@ -11,7 +11,7 @@ from sentry_sdk.consts import OP from sentry_sdk.hub import Hub, _should_send_default_pii from sentry_sdk.integrations import DidNotEnable, Integration -from sentry_sdk.integrations._asgi_common import _get_headers +from sentry_sdk.integrations._asgi_common import _get_headers, _get_request_data from sentry_sdk.integrations._wsgi_common import ( _is_json_content_type, request_body_within_bounds, @@ -431,6 +431,7 @@ async def _sentry_async_func(*args, **kwargs): source=transaction_source, ) + asgi_request_data = _get_request_data(request.scope) extractor = StarletteRequestExtractor(request) info = await extractor.extract_request_info() @@ -441,6 +442,7 @@ def event_processor(event, hint): # Add info from request to event request_info = event.get("request", {}) + request_info.update(asgi_request_data) if info: if "cookies" in info: request_info["cookies"] = info["cookies"] @@ -474,6 +476,7 @@ def _sentry_sync_func(*args, **kwargs): sentry_scope.profile.update_active_thread_id() request = args[0] + asgi_request_data = _get_request_data(request.scope) extractor = StarletteRequestExtractor(request) cookies = extractor.extract_cookies_from_request() @@ -484,6 +487,7 @@ def event_processor(event, hint): # Extract information from request request_info = event.get("request", {}) + request_info.update(asgi_request_data) if cookies: request_info["cookies"] = cookies