From 91d863e76e51a3917baebc0f97e4d785ad499998 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 11 Oct 2022 15:19:59 +0200 Subject: [PATCH 1/8] Have instrumentation for ASGI middleware receive/send callbacks. --- sentry_sdk/consts.py | 2 ++ sentry_sdk/integrations/starlette.py | 30 +++++++++++++++++++++++++++- 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/sentry_sdk/consts.py b/sentry_sdk/consts.py index b6e546e336..3be5fe6779 100644 --- a/sentry_sdk/consts.py +++ b/sentry_sdk/consts.py @@ -118,6 +118,8 @@ class OP: HTTP_SERVER = "http.server" MIDDLEWARE_DJANGO = "middleware.django" MIDDLEWARE_STARLETTE = "middleware.starlette" + MIDDLEWARE_STARLETTE_RECEIVE = "middleware.starlette.receive" + MIDDLEWARE_STARLETTE_SEND = "middleware.starlette.send" QUEUE_SUBMIT_CELERY = "queue.submit.celery" QUEUE_TASK_CELERY = "queue.task.celery" QUEUE_TASK_RQ = "queue.task.rq" diff --git a/sentry_sdk/integrations/starlette.py b/sentry_sdk/integrations/starlette.py index dffba5afd5..c38a007270 100644 --- a/sentry_sdk/integrations/starlette.py +++ b/sentry_sdk/integrations/starlette.py @@ -91,12 +91,40 @@ async def _create_span_call(*args, **kwargs): integration = hub.get_integration(StarletteIntegration) if integration is not None: middleware_name = args[0].__class__.__name__ + with hub.start_span( op=OP.MIDDLEWARE_STARLETTE, description=middleware_name ) as middleware_span: middleware_span.set_tag("starlette.middleware_name", middleware_name) - await old_call(*args, **kwargs) + app, scope, receive, send = args + + # Creating spans for the "receive" callback + async def _sentry_receive(*args, **kwargs): + hub = Hub.current + with hub.start_span( + op=OP.MIDDLEWARE_STARLETTE_RECEIVE, + description=receive.__qualname__, + ) as span: + span.set_tag("starlette.middleware_name", middleware_name) + await receive(*args, **kwargs) + + receive_patched = receive.__name__ == "_sentry_receive" + new_receive = _sentry_receive if not receive_patched else receive + + # Creating spans for the "send" callback + async def _sentry_send(*args, **kwargs): + hub = Hub.current + with hub.start_span( + op=OP.MIDDLEWARE_STARLETTE_SEND, description=send.__qualname__ + ) as span: + span.set_tag("starlette.middleware_name", middleware_name) + await send(*args, **kwargs) + + send_patched = send.__name__ == "_sentry_send" + new_send = _sentry_send if not send_patched else send + + await old_call(app, scope, new_receive, new_send, **kwargs) else: await old_call(*args, **kwargs) From 2c4065222fe23ddbb9887b0e898d8ac6f962085b Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 11 Oct 2022 15:28:41 +0200 Subject: [PATCH 2/8] Fixed type annotations --- sentry_sdk/integrations/starlette.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/sentry_sdk/integrations/starlette.py b/sentry_sdk/integrations/starlette.py index c38a007270..429e57460a 100644 --- a/sentry_sdk/integrations/starlette.py +++ b/sentry_sdk/integrations/starlette.py @@ -101,6 +101,7 @@ async def _create_span_call(*args, **kwargs): # Creating spans for the "receive" callback async def _sentry_receive(*args, **kwargs): + # type: (*Any, **Any) -> Any hub = Hub.current with hub.start_span( op=OP.MIDDLEWARE_STARLETTE_RECEIVE, @@ -114,6 +115,7 @@ async def _sentry_receive(*args, **kwargs): # Creating spans for the "send" callback async def _sentry_send(*args, **kwargs): + # type: (*Any, **Any) -> Any hub = Hub.current with hub.start_span( op=OP.MIDDLEWARE_STARLETTE_SEND, description=send.__qualname__ From 119796a575ba9bd0050b066ee6f00f832a32e409 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 11 Oct 2022 16:06:10 +0200 Subject: [PATCH 3/8] Added tests for new callback spans. --- .../integrations/starlette/test_starlette.py | 92 +++++++++++++++++++ 1 file changed, 92 insertions(+) diff --git a/tests/integrations/starlette/test_starlette.py b/tests/integrations/starlette/test_starlette.py index 24254b69ef..023fdb0d8f 100644 --- a/tests/integrations/starlette/test_starlette.py +++ b/tests/integrations/starlette/test_starlette.py @@ -152,6 +152,26 @@ async def __anext__(self): raise StopAsyncIteration +class TestMiddleware: + def __init__(self, app): + self.app = app + + async def __call__(self, scope, receive, send): + # only handle http requests + if scope["type"] != "http": + await self.app(scope, receive, send) + return + + async def do_stuff(message): + if message["type"] == "http.response.start": + # do something here. + pass + + await send(message) + + await self.app(scope, receive, do_stuff) + + @pytest.mark.asyncio async def test_starlettrequestextractor_content_length(sentry_init): with mock.patch( @@ -546,6 +566,78 @@ def test_middleware_spans(sentry_init, capture_events): idx += 1 +def test_middleware_callback_spans(sentry_init, capture_events): + sentry_init( + traces_sample_rate=1.0, + integrations=[StarletteIntegration()], + ) + starlette_app = starlette_app_factory(middleware=[Middleware(TestMiddleware)]) + events = capture_events() + + client = TestClient(starlette_app, raise_server_exceptions=False) + try: + client.get("/message", auth=("Gabriela", "hello123")) + except Exception: + pass + + (_, transaction_event) = events + + expected = [ + { + "op": "middleware.starlette", + "description": "ServerErrorMiddleware", + "tags": {"starlette.middleware_name": "ServerErrorMiddleware"}, + }, + { + "op": "middleware.starlette", + "description": "TestMiddleware", + "tags": {"starlette.middleware_name": "TestMiddleware"}, + }, + { + "op": "middleware.starlette", + "description": "ExceptionMiddleware", + "tags": {"starlette.middleware_name": "ExceptionMiddleware"}, + }, + { + "op": "middleware.starlette.send", + "description": "TestMiddleware.__call__..do_stuff", + "tags": {"starlette.middleware_name": "ExceptionMiddleware"}, + }, + { + "op": "middleware.starlette.send", + "description": "ServerErrorMiddleware.__call__.._send", + "tags": {"starlette.middleware_name": "TestMiddleware"}, + }, + { + "op": "middleware.starlette.send", + "description": "_TestClientTransport.handle_request..send", + "tags": {"starlette.middleware_name": "ServerErrorMiddleware"}, + }, + { + "op": "middleware.starlette.send", + "description": "TestMiddleware.__call__..do_stuff", + "tags": {"starlette.middleware_name": "ExceptionMiddleware"}, + }, + { + "op": "middleware.starlette.send", + "description": "ServerErrorMiddleware.__call__.._send", + "tags": {"starlette.middleware_name": "TestMiddleware"}, + }, + { + "op": "middleware.starlette.send", + "description": "_TestClientTransport.handle_request..send", + "tags": {"starlette.middleware_name": "ServerErrorMiddleware"}, + }, + ] + + idx = 0 + for span in transaction_event["spans"]: + assert span["op"] == expected[idx]["op"] + assert span["description"] == expected[idx]["description"] + assert span["tags"] == expected[idx]["tags"] + idx += 1 + + def test_last_event_id(sentry_init, capture_events): sentry_init( integrations=[StarletteIntegration()], From 89ba74f2eed79d9d843ad9b6ebaa5623b14aaab5 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 12 Oct 2022 11:47:03 +0200 Subject: [PATCH 4/8] Fixed tests for Starlette --- tests/integrations/starlette/test_starlette.py | 6 +++++- tox.ini | 4 +++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/tests/integrations/starlette/test_starlette.py b/tests/integrations/starlette/test_starlette.py index 023fdb0d8f..ab810c6e09 100644 --- a/tests/integrations/starlette/test_starlette.py +++ b/tests/integrations/starlette/test_starlette.py @@ -31,6 +31,8 @@ from starlette.middleware.authentication import AuthenticationMiddleware from starlette.testclient import TestClient +STARLETTE_VERSION = tuple([int(x) for x in starlette.__version__.split(".")]) + PICTURE = os.path.join(os.path.dirname(os.path.abspath(__file__)), "photo.jpg") BODY_JSON = {"some": "json", "for": "testing", "nested": {"numbers": 123}} @@ -610,7 +612,9 @@ def test_middleware_callback_spans(sentry_init, capture_events): }, { "op": "middleware.starlette.send", - "description": "_TestClientTransport.handle_request..send", + "description": "_ASGIAdapter.send..send" + if STARLETTE_VERSION < (0, 21) + else "_TestClientTransport.handle_request..send", "tags": {"starlette.middleware_name": "ServerErrorMiddleware"}, }, { diff --git a/tox.ini b/tox.ini index 2b26d2f45a..5aa52644fb 100644 --- a/tox.ini +++ b/tox.ini @@ -36,7 +36,7 @@ envlist = {py3.7,py3.8,py3.9,py3.10}-asgi - {py3.7,py3.8,py3.9,py3.10}-starlette-{0.19.1,0.20} + {py3.7,py3.8,py3.9,py3.10}-starlette-{0.19.1,0.20,0.21} {py3.7,py3.8,py3.9,py3.10}-fastapi @@ -152,8 +152,10 @@ deps = starlette: pytest-asyncio starlette: python-multipart starlette: requests + starlette-0.21: httpx starlette-0.19.1: starlette==0.19.1 starlette-0.20: starlette>=0.20.0,<0.21.0 + starlette-0.21: starlette>=0.21.0,<0.22.0 fastapi: fastapi fastapi: pytest-asyncio From d51c1dbabef9f29c72543c8eba014ff37582f132 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 12 Oct 2022 13:07:18 +0200 Subject: [PATCH 5/8] Fixed tests again. --- tests/integrations/starlette/test_starlette.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/integrations/starlette/test_starlette.py b/tests/integrations/starlette/test_starlette.py index ab810c6e09..d41893dbda 100644 --- a/tests/integrations/starlette/test_starlette.py +++ b/tests/integrations/starlette/test_starlette.py @@ -629,7 +629,9 @@ def test_middleware_callback_spans(sentry_init, capture_events): }, { "op": "middleware.starlette.send", - "description": "_TestClientTransport.handle_request..send", + "description": "_ASGIAdapter.send..send" + if STARLETTE_VERSION < (0, 21) + else "_TestClientTransport.handle_request..send", "tags": {"starlette.middleware_name": "ServerErrorMiddleware"}, }, ] From f4b42732c35ac44b501925edee9f5dd1db4b3d2d Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 12 Oct 2022 13:21:10 +0200 Subject: [PATCH 6/8] Better naming --- tests/integrations/starlette/test_starlette.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/integrations/starlette/test_starlette.py b/tests/integrations/starlette/test_starlette.py index d41893dbda..29e5916adb 100644 --- a/tests/integrations/starlette/test_starlette.py +++ b/tests/integrations/starlette/test_starlette.py @@ -154,7 +154,7 @@ async def __anext__(self): raise StopAsyncIteration -class TestMiddleware: +class SampleMiddleware: def __init__(self, app): self.app = app @@ -573,7 +573,7 @@ def test_middleware_callback_spans(sentry_init, capture_events): traces_sample_rate=1.0, integrations=[StarletteIntegration()], ) - starlette_app = starlette_app_factory(middleware=[Middleware(TestMiddleware)]) + starlette_app = starlette_app_factory(middleware=[Middleware(SampleMiddleware)]) events = capture_events() client = TestClient(starlette_app, raise_server_exceptions=False) @@ -592,8 +592,8 @@ def test_middleware_callback_spans(sentry_init, capture_events): }, { "op": "middleware.starlette", - "description": "TestMiddleware", - "tags": {"starlette.middleware_name": "TestMiddleware"}, + "description": "SampleMiddleware", + "tags": {"starlette.middleware_name": "SampleMiddleware"}, }, { "op": "middleware.starlette", @@ -602,13 +602,13 @@ def test_middleware_callback_spans(sentry_init, capture_events): }, { "op": "middleware.starlette.send", - "description": "TestMiddleware.__call__..do_stuff", + "description": "SampleMiddleware.__call__..do_stuff", "tags": {"starlette.middleware_name": "ExceptionMiddleware"}, }, { "op": "middleware.starlette.send", "description": "ServerErrorMiddleware.__call__.._send", - "tags": {"starlette.middleware_name": "TestMiddleware"}, + "tags": {"starlette.middleware_name": "SampleMiddleware"}, }, { "op": "middleware.starlette.send", @@ -619,13 +619,13 @@ def test_middleware_callback_spans(sentry_init, capture_events): }, { "op": "middleware.starlette.send", - "description": "TestMiddleware.__call__..do_stuff", + "description": "SampleMiddleware.__call__..do_stuff", "tags": {"starlette.middleware_name": "ExceptionMiddleware"}, }, { "op": "middleware.starlette.send", "description": "ServerErrorMiddleware.__call__.._send", - "tags": {"starlette.middleware_name": "TestMiddleware"}, + "tags": {"starlette.middleware_name": "SampleMiddleware"}, }, { "op": "middleware.starlette.send", From dc82cea05800b149f62c1b29a8932c5b4572a823 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Fri, 14 Oct 2022 15:45:23 +0200 Subject: [PATCH 7/8] Have explicit parameters to make it better readable. --- sentry_sdk/integrations/starlette.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/sentry_sdk/integrations/starlette.py b/sentry_sdk/integrations/starlette.py index 429e57460a..8e82ff9b35 100644 --- a/sentry_sdk/integrations/starlette.py +++ b/sentry_sdk/integrations/starlette.py @@ -85,20 +85,18 @@ def _enable_span_for_middleware(middleware_class): # type: (Any) -> type old_call = middleware_class.__call__ - async def _create_span_call(*args, **kwargs): - # type: (Any, Any) -> None + async def _create_span_call(app, scope, receive, send, **kwargs): + # type: (Any, Dict, Callable, Callable, Any) -> None hub = Hub.current integration = hub.get_integration(StarletteIntegration) if integration is not None: - middleware_name = args[0].__class__.__name__ + middleware_name = app.__class__.__name__ with hub.start_span( op=OP.MIDDLEWARE_STARLETTE, description=middleware_name ) as middleware_span: middleware_span.set_tag("starlette.middleware_name", middleware_name) - app, scope, receive, send = args - # Creating spans for the "receive" callback async def _sentry_receive(*args, **kwargs): # type: (*Any, **Any) -> Any @@ -129,7 +127,7 @@ async def _sentry_send(*args, **kwargs): await old_call(app, scope, new_receive, new_send, **kwargs) else: - await old_call(*args, **kwargs) + await old_call(app, scope, receive, send, **kwargs) not_yet_patched = old_call.__name__ not in [ "_create_span_call", From aadc37cde9a5916964e9e89152de9f95dd60059b Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Fri, 14 Oct 2022 16:18:41 +0200 Subject: [PATCH 8/8] Fixed typing --- sentry_sdk/integrations/starlette.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry_sdk/integrations/starlette.py b/sentry_sdk/integrations/starlette.py index 8e82ff9b35..aaf7fb3dc4 100644 --- a/sentry_sdk/integrations/starlette.py +++ b/sentry_sdk/integrations/starlette.py @@ -86,7 +86,7 @@ def _enable_span_for_middleware(middleware_class): old_call = middleware_class.__call__ async def _create_span_call(app, scope, receive, send, **kwargs): - # type: (Any, Dict, Callable, Callable, Any) -> None + # type: (Any, Dict[str, Any], Callable[[], Awaitable[Dict[str, Any]]], Callable[[Dict[str, Any]], Awaitable[None]], Any) -> None hub = Hub.current integration = hub.get_integration(StarletteIntegration) if integration is not None: