feat(django): Instrument views as spans (getsentry#787)

untitaker · web-flow · commit 193f591b34b9 · 2020-08-13T16:49:57.000+02:00
diff --git a/sentry_sdk/integrations/django/__init__.py b/sentry_sdk/integrations/django/__init__.py
@@ -39,6 +39,7 @@
 from sentry_sdk.integrations.django.transactions import LEGACY_RESOLVER
 from sentry_sdk.integrations.django.templates import get_template_frame_from_exception
 from sentry_sdk.integrations.django.middleware import patch_django_middlewares
+from sentry_sdk.integrations.django.views import patch_resolver
 
 
 if MYPY:
@@ -199,6 +200,7 @@ def _django_queryset_repr(value, hint):
 
         _patch_channels()
         patch_django_middlewares()
+        patch_resolver()
 
 
 _DRF_PATCHED = False
diff --git a/sentry_sdk/integrations/django/views.py b/sentry_sdk/integrations/django/views.py
@@ -0,0 +1,55 @@
+import functools
+
+from sentry_sdk.hub import Hub
+from sentry_sdk._types import MYPY
+
+if MYPY:
+    from typing import Any
+
+    from django.urls.resolvers import ResolverMatch
+
+
+def patch_resolver():
+    # type: () -> None
+    try:
+        from django.urls.resolvers import URLResolver
+    except ImportError:
+        try:
+            from django.urls.resolvers import RegexURLResolver as URLResolver
+        except ImportError:
+            from django.core.urlresolvers import RegexURLResolver as URLResolver
+
+    from sentry_sdk.integrations.django import DjangoIntegration
+
+    old_resolve = URLResolver.resolve
+
+    def resolve(self, path):
+        # type: (URLResolver, Any) -> ResolverMatch
+        hub = Hub.current
+        integration = hub.get_integration(DjangoIntegration)
+
+        if integration is None or not integration.middleware_spans:
+            return old_resolve(self, path)
+
+        return _wrap_resolver_match(hub, old_resolve(self, path))
+
+    URLResolver.resolve = resolve
+
+
+def _wrap_resolver_match(hub, resolver_match):
+    # type: (Hub, ResolverMatch) -> ResolverMatch
+
+    # XXX: The wrapper function is created for every request. Find more
+    # efficient way to wrap views (or build a cache?)
+
+    old_callback = resolver_match.func
+
+    @functools.wraps(old_callback)
+    def callback(*args, **kwargs):
+        # type: (*Any, **Any) -> Any
+        with hub.start_span(op="django.view", description=resolver_match.view_name):
+            return old_callback(*args, **kwargs)
+
+    resolver_match.func = callback
+
+    return resolver_match
diff --git a/tests/conftest.py b/tests/conftest.py
@@ -334,8 +334,8 @@ def inner(event):
             by_parent.setdefault(span["parent_span_id"], []).append(span)
 
         def render_span(span):
-            yield "- op={!r}: description={!r}".format(
-                span.get("op"), span.get("description")
+            yield "- op={}: description={}".format(
+                json.dumps(span.get("op")), json.dumps(span.get("description"))
             )
             for subspan in by_parent.get(span["span_id"]) or ():
                 for line in render_span(subspan):
diff --git a/tests/integrations/django/test_basic.py b/tests/integrations/django/test_basic.py
@@ -511,7 +511,7 @@ def test_does_not_capture_403(sentry_init, client, capture_events, endpoint):
     assert not events
 
 
-def test_middleware_spans(sentry_init, client, capture_events):
+def test_middleware_spans(sentry_init, client, capture_events, render_span_tree):
     sentry_init(
         integrations=[DjangoIntegration()],
         traces_sample_rate=1.0,
@@ -525,26 +525,32 @@ def test_middleware_spans(sentry_init, client, capture_events):
 
     assert message["message"] == "hi"
 
-    for middleware in transaction["spans"]:
-        assert middleware["op"] == "django.middleware"
-
     if DJANGO_VERSION >= (1, 10):
-        reference_value = [
-            "django.contrib.sessions.middleware.SessionMiddleware.__call__",
-            "django.contrib.auth.middleware.AuthenticationMiddleware.__call__",
-            "tests.integrations.django.myapp.settings.TestMiddleware.__call__",
-            "tests.integrations.django.myapp.settings.TestFunctionMiddleware.__call__",
-        ]
-    else:
-        reference_value = [
-            "django.contrib.sessions.middleware.SessionMiddleware.process_request",
-            "django.contrib.auth.middleware.AuthenticationMiddleware.process_request",
-            "tests.integrations.django.myapp.settings.TestMiddleware.process_request",
-            "tests.integrations.django.myapp.settings.TestMiddleware.process_response",
-            "django.contrib.sessions.middleware.SessionMiddleware.process_response",
-        ]
+        assert (
+            render_span_tree(transaction)
+            == """\
+- op="http.server": description=null
+  - op="django.middleware": description="django.contrib.sessions.middleware.SessionMiddleware.__call__"
+    - op="django.middleware": description="django.contrib.auth.middleware.AuthenticationMiddleware.__call__"
+      - op="django.middleware": description="tests.integrations.django.myapp.settings.TestMiddleware.__call__"
+        - op="django.middleware": description="tests.integrations.django.myapp.settings.TestFunctionMiddleware.__call__"
+          - op="django.view": description="message"\
+"""
+        )
 
-    assert [t["description"] for t in transaction["spans"]] == reference_value
+    else:
+        assert (
+            render_span_tree(transaction)
+            == """\
+- op="http.server": description=null
+  - op="django.middleware": description="django.contrib.sessions.middleware.SessionMiddleware.process_request"
+  - op="django.middleware": description="django.contrib.auth.middleware.AuthenticationMiddleware.process_request"
+  - op="django.middleware": description="tests.integrations.django.myapp.settings.TestMiddleware.process_request"
+  - op="django.view": description="message"
+  - op="django.middleware": description="tests.integrations.django.myapp.settings.TestMiddleware.process_response"
+  - op="django.middleware": description="django.contrib.sessions.middleware.SessionMiddleware.process_response"\
+"""
+        )
 
 
 def test_middleware_spans_disabled(sentry_init, client, capture_events):
diff --git a/tests/integrations/sqlalchemy/test_sqlalchemy.py b/tests/integrations/sqlalchemy/test_sqlalchemy.py
@@ -120,19 +120,19 @@ class Address(Base):
     assert (
         render_span_tree(event)
         == """\
-- op=None: description=None
-  - op='db': description='SAVEPOINT sa_savepoint_1'
-  - op='db': description='SELECT person.id AS person_id, person.name AS person_name \\nFROM person\\n LIMIT ? OFFSET ?'
-  - op='db': description='RELEASE SAVEPOINT sa_savepoint_1'
-  - op='db': description='SAVEPOINT sa_savepoint_2'
-  - op='db': description='INSERT INTO person (id, name) VALUES (?, ?)'
-  - op='db': description='ROLLBACK TO SAVEPOINT sa_savepoint_2'
-  - op='db': description='SAVEPOINT sa_savepoint_3'
-  - op='db': description='INSERT INTO person (id, name) VALUES (?, ?)'
-  - op='db': description='ROLLBACK TO SAVEPOINT sa_savepoint_3'
-  - op='db': description='SAVEPOINT sa_savepoint_4'
-  - op='db': description='SELECT person.id AS person_id, person.name AS person_name \\nFROM person\\n LIMIT ? OFFSET ?'
-  - op='db': description='RELEASE SAVEPOINT sa_savepoint_4'\
+- op=null: description=null
+  - op="db": description="SAVEPOINT sa_savepoint_1"
+  - op="db": description="SELECT person.id AS person_id, person.name AS person_name \\nFROM person\\n LIMIT ? OFFSET ?"
+  - op="db": description="RELEASE SAVEPOINT sa_savepoint_1"
+  - op="db": description="SAVEPOINT sa_savepoint_2"
+  - op="db": description="INSERT INTO person (id, name) VALUES (?, ?)"
+  - op="db": description="ROLLBACK TO SAVEPOINT sa_savepoint_2"
+  - op="db": description="SAVEPOINT sa_savepoint_3"
+  - op="db": description="INSERT INTO person (id, name) VALUES (?, ?)"
+  - op="db": description="ROLLBACK TO SAVEPOINT sa_savepoint_3"
+  - op="db": description="SAVEPOINT sa_savepoint_4"
+  - op="db": description="SELECT person.id AS person_id, person.name AS person_name \\nFROM person\\n LIMIT ? OFFSET ?"
+  - op="db": description="RELEASE SAVEPOINT sa_savepoint_4"\
 """
     )
 
