Let SentryAsgiMiddleware work with Starlette and FastAPI integrations (#1594)

People where complaining (rightly so) that just raising an error when SentryAsgiMiddleware and Starlette/Fastapi is used is not a nice thing to do.

So we tried again to make this work together. To not break our users code.
The plan was to make SentryASGIMiddleware no-op when there is already one there. Turns out this works already on Starlette but on FastAPI it broke. (This was because of how FastAPI deals with middlewares)

We debugged the whole thing and it turns out that we where patching our own SentryAsgiMiddleware (like the FastAPI internal ones) to create spans when they are executed. This and the fact that we use __slots__ extensively made the integration break.

We found out, that if we are not patching our own middleware this fixes the problem when initializing the middleware twice (once by our users and once by our auto-enabled FastAPI integration).

Fixes #1592

Author: antonpirker

sentry_sdk/integrations/asgi.py

@@ -1,7 +1,7 @@
 """
 An ASGI middleware.
 
-Based on Tom Christie's `sentry-asgi <https://github.com/encode/sentry-asgi>`_.
+Based on Tom Christie's `sentry-asgi <https://github.com/encode/sentry-asgi>`.
 """
 
 import asyncio
@@ -23,6 +23,7 @@
     event_from_exception,
     HAS_REAL_CONTEXTVARS,
     CONTEXTVARS_ERROR_MESSAGE,
+    logger,
     transaction_from_function,
 )
 from sentry_sdk.tracing import Transaction
@@ -104,20 +105,21 @@ def __init__(
                 "Invalid value for transaction_style: %s (must be in %s)"
                 % (transaction_style, TRANSACTION_STYLE_VALUES)
             )
-        self.transaction_style = transaction_style
-        self.mechanism_type = mechanism_type
-        self.app = app
 
         asgi_middleware_while_using_starlette_or_fastapi = (
-            "starlette" in _get_installed_modules() and self.mechanism_type == "asgi"
+            "starlette" in _get_installed_modules() and mechanism_type == "asgi"
         )
         if asgi_middleware_while_using_starlette_or_fastapi:
-            raise RuntimeError(
+            logger.warning(
                 "The Sentry Python SDK can now automatically support ASGI frameworks like Starlette and FastAPI. "
                 "Please remove 'SentryAsgiMiddleware' from your project. "
                 "See https://docs.sentry.io/platforms/python/guides/asgi/ for more information."
             )
 
+        self.transaction_style = transaction_style
+        self.mechanism_type = mechanism_type
+        self.app = app
+
         if _looks_like_asgi3(app):
             self.__call__ = self._run_asgi3  # type: Callable[..., Any]
         else:
@@ -138,7 +140,6 @@ async def _run_asgi3(self, scope, receive, send):
     async def _run_app(self, scope, callback):
         # type: (Any, Any) -> Any
         is_recursive_asgi_middleware = _asgi_middleware_applied.get(False)
-
         if is_recursive_asgi_middleware:
             try:
                 return await callback()

sentry_sdk/integrations/starlette.py

@@ -257,6 +257,9 @@ 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)
+
             span_enabled_cls = _enable_span_for_middleware(cls)
             old_middleware_init(self, span_enabled_cls, **options)
 
@@ -285,6 +288,7 @@ async def _sentry_patched_asgi_app(self, scope, receive, send):
             lambda *a, **kw: old_app(self, *a, **kw),
             mechanism_type=StarletteIntegration.identifier,
         )
+
         middleware.__call__ = middleware._run_asgi3
         return await middleware(scope, receive, send)
 

tests/integrations/fastapi/test_fastapi.py

@@ -117,17 +117,21 @@ def test_transaction_style(
     assert "transaction" not in event
 
 
-def test_legacy_setup(sentry_init):
-    # Check for error message if the user
-    # updates and the integrations are auto enabled
-    # and the SentryAsgiMiddleware is still there
+def test_legacy_setup(
+    sentry_init,
+    capture_events,
+):
+    # Check that behaviour does not change
+    # if the user just adds the new Integrations
+    # and forgets to remove SentryAsgiMiddleware
     sentry_init()
+    app = fastapi_app_factory()
+    asgi_app = SentryAsgiMiddleware(app)
 
-    with pytest.raises(RuntimeError) as exc:
-        app = fastapi_app_factory()
-        app = SentryAsgiMiddleware(app)
+    events = capture_events()
 
-    assert (
-        "The Sentry Python SDK can now automatically support ASGI frameworks like Starlette and FastAPI."
-        in str(exc)
-    )
+    client = TestClient(asgi_app)
+    client.get("/message/123456")
+
+    (event,) = events
+    assert event["transaction"] == "/message/{message_id}"

tests/integrations/starlette/test_starlette.py

@@ -543,17 +543,21 @@ def test_middleware_spans(sentry_init, capture_events):
             idx += 1
 
 
-def test_legacy_setup(sentry_init):
-    # Check for error message if the user
-    # updates and the integration is auto enabled
-    # and the SentryAsgiMiddleware is still there
+def test_legacy_setup(
+    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)
 
-    with pytest.raises(RuntimeError) as exc:
-        app = starlette_app_factory()
-        app = SentryAsgiMiddleware(app)
+    events = capture_events()
 
-    assert (
-        "The Sentry Python SDK can now automatically support ASGI frameworks like Starlette and FastAPI."
-        in str(exc)
-    )
+    client = TestClient(asgi_app)
+    client.get("/message/123456")
+
+    (event,) = events
+    assert event["transaction"] == "/message/{message_id}"