feat(django): Pick custom urlconf up from request if any (getsentry#1308)

sl0thentr0py · web-flow · commit 20f0a76e680c · 2022-01-19T20:34:24.000+01:00
Django middlewares sometimes can override `request.urlconf` which we also need to respect in our transaction name resolving.
This fixes an issue (WEB-530) with a customer using `django-tenants` where all their transactions were named `Generic WSGI request` due to the default url resolution failing.
diff --git a/sentry_sdk/integrations/django/__init__.py b/sentry_sdk/integrations/django/__init__.py
@@ -346,6 +346,31 @@ def _before_get_response(request):
         )
 
 
+def _after_get_response(request):
+    # type: (WSGIRequest) -> None
+    """
+    Some django middlewares overwrite request.urlconf
+    so we need to respect that contract,
+    so we try to resolve the url again.
+    """
+    if not hasattr(request, "urlconf"):
+        return
+
+    hub = Hub.current
+    integration = hub.get_integration(DjangoIntegration)
+    if integration is None or integration.transaction_style != "url":
+        return
+
+    with hub.configure_scope() as scope:
+        try:
+            scope.transaction = LEGACY_RESOLVER.resolve(
+                request.path_info,
+                urlconf=request.urlconf,
+            )
+        except Exception:
+            pass
+
+
 def _patch_get_response():
     # type: () -> None
     """
@@ -358,7 +383,9 @@ def _patch_get_response():
     def sentry_patched_get_response(self, request):
         # type: (Any, WSGIRequest) -> Union[HttpResponse, BaseException]
         _before_get_response(request)
-        return old_get_response(self, request)
+        rv = old_get_response(self, request)
+        _after_get_response(request)
+        return rv
 
     BaseHandler.get_response = sentry_patched_get_response
 
diff --git a/tests/integrations/django/myapp/custom_urls.py b/tests/integrations/django/myapp/custom_urls.py
@@ -0,0 +1,31 @@
+"""myapp URL Configuration
+
+The `urlpatterns` list routes URLs to views. For more information please see:
+    https://docs.djangoproject.com/en/2.0/topics/http/urls/
+Examples:
+Function views
+    1. Add an import:  from my_app import views
+    2. Add a URL to urlpatterns:  path('', views.home, name='home')
+Class-based views
+    1. Add an import:  from other_app.views import Home
+    2. Add a URL to urlpatterns:  path('', Home.as_view(), name='home')
+Including another URLconf
+    1. Import the include() function: from django.urls import include, path
+    2. Add a URL to urlpatterns:  path('blog/', include('blog.urls'))
+"""
+from __future__ import absolute_import
+
+try:
+    from django.urls import path
+except ImportError:
+    from django.conf.urls import url
+
+    def path(path, *args, **kwargs):
+        return url("^{}$".format(path), *args, **kwargs)
+
+
+from . import views
+
+urlpatterns = [
+    path("custom/ok", views.custom_ok, name="custom_ok"),
+]
diff --git a/tests/integrations/django/myapp/middleware.py b/tests/integrations/django/myapp/middleware.py
@@ -1,19 +1,30 @@
-import asyncio
-from django.utils.decorators import sync_and_async_middleware
+import django
 
+if django.VERSION >= (3, 1):
+    import asyncio
+    from django.utils.decorators import sync_and_async_middleware
 
-@sync_and_async_middleware
-def simple_middleware(get_response):
-    if asyncio.iscoroutinefunction(get_response):
+    @sync_and_async_middleware
+    def simple_middleware(get_response):
+        if asyncio.iscoroutinefunction(get_response):
 
-        async def middleware(request):
-            response = await get_response(request)
-            return response
+            async def middleware(request):
+                response = await get_response(request)
+                return response
 
-    else:
+        else:
 
-        def middleware(request):
-            response = get_response(request)
-            return response
+            def middleware(request):
+                response = get_response(request)
+                return response
+
+        return middleware
+
+
+def custom_urlconf_middleware(get_response):
+    def middleware(request):
+        request.urlconf = "tests.integrations.django.myapp.custom_urls"
+        response = get_response(request)
+        return response
 
     return middleware
diff --git a/tests/integrations/django/myapp/views.py b/tests/integrations/django/myapp/views.py
@@ -120,6 +120,11 @@ def template_test(request, *args, **kwargs):
     return render(request, "user_name.html", {"user_age": 20})
 
 
+@csrf_exempt
+def custom_ok(request, *args, **kwargs):
+    return HttpResponse("custom ok")
+
+
 @csrf_exempt
 def template_test2(request, *args, **kwargs):
     return TemplateResponse(
diff --git a/tests/integrations/django/test_basic.py b/tests/integrations/django/test_basic.py
@@ -755,3 +755,30 @@ def test_csrf(sentry_init, client):
     content, status, _headers = client.post(reverse("message"))
     assert status.lower() == "200 ok"
     assert b"".join(content) == b"ok"
+
+
+@pytest.mark.skipif(DJANGO_VERSION < (2, 0), reason="Requires Django > 2.0")
+def test_custom_urlconf_middleware(
+    settings, sentry_init, client, capture_events, render_span_tree
+):
+    """
+    Some middlewares (for instance in django-tenants) overwrite request.urlconf.
+    Test that the resolver picks up the correct urlconf for transaction naming.
+    """
+    urlconf = "tests.integrations.django.myapp.middleware.custom_urlconf_middleware"
+    settings.ROOT_URLCONF = ""
+    settings.MIDDLEWARE.insert(0, urlconf)
+    client.application.load_middleware()
+
+    sentry_init(integrations=[DjangoIntegration()], traces_sample_rate=1.0)
+    events = capture_events()
+
+    content, status, _headers = client.get("/custom/ok")
+    assert status.lower() == "200 ok"
+    assert b"".join(content) == b"custom ok"
+
+    (event,) = events
+    assert event["transaction"] == "/custom/ok"
+    assert "custom_urlconf_middleware" in render_span_tree(event)
+
+    settings.MIDDLEWARE.pop(0)
