From a60c3335f532960a312bac6188632a549b5e0a66 Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Tue, 29 Sep 2020 13:48:27 +0200 Subject: [PATCH 1/3] fix(django): Do not patch resolver_match --- sentry_sdk/integrations/django/__init__.py | 4 +- sentry_sdk/integrations/django/views.py | 65 ++++++--------------- tests/integrations/django/myapp/settings.py | 5 ++ 3 files changed, 26 insertions(+), 48 deletions(-) diff --git a/sentry_sdk/integrations/django/__init__.py b/sentry_sdk/integrations/django/__init__.py index 60fa874f18..008dc386bb 100644 --- a/sentry_sdk/integrations/django/__init__.py +++ b/sentry_sdk/integrations/django/__init__.py @@ -39,7 +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 +from sentry_sdk.integrations.django.views import patch_views if MYPY: @@ -200,7 +200,7 @@ def _django_queryset_repr(value, hint): _patch_channels() patch_django_middlewares() - patch_resolver() + patch_views() _DRF_PATCHED = False diff --git a/sentry_sdk/integrations/django/views.py b/sentry_sdk/integrations/django/views.py index 24cfb73282..7384bc5a58 100644 --- a/sentry_sdk/integrations/django/views.py +++ b/sentry_sdk/integrations/django/views.py @@ -8,60 +8,33 @@ from django.urls.resolvers import ResolverMatch -def patch_resolver(): +def patch_views(): # 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 django.core.handlers.base import BaseHandler from sentry_sdk.integrations.django import DjangoIntegration - old_resolve = URLResolver.resolve + old_make_view_atomic = BaseHandler.make_view_atomic - 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 + @_functools.wraps(old_make_view_atomic) + def sentry_patched_make_view_atomic(self, *args, **kwargs): + callback = old_make_view_atomic(self, *args, **kwargs) + # XXX: The wrapper function is created for every request. Find more + # efficient way to wrap views (or build a cache?) -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 + hub = Hub.current + integration = hub.get_integration(DjangoIntegration) - # Explicitly forward `csrf_exempt` in case it is not an attribute in - # callback.__dict__, but rather a class attribute (on a class - # implementing __call__) such as this: - # - # class Foo(object): - # csrf_exempt = True - # - # def __call__(self, request): ... - # - # We have had this in the Sentry codebase (for no good reason, but - # nevertheless we broke user code) - assigned = _functools.WRAPPER_ASSIGNMENTS + ("csrf_exempt",) + if integration is not None and integration.middleware_spans: + @_functools.wraps(callback) + def sentry_wrapped_callback(request, *args, **kwargs): + with hub.start_span(op="django.view", description=request.resolver_match.view_name): + return callback(request, *args, **kwargs) - @_functools.wraps(old_callback, assigned=assigned) - 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) + else: + sentry_wrapped_callback = callback - resolver_match.func = callback + return sentry_wrapped_callback - return resolver_match + BaseHandler.make_view_atomic = sentry_patched_make_view_atomic diff --git a/tests/integrations/django/myapp/settings.py b/tests/integrations/django/myapp/settings.py index 235df5c8bd..adbf5d94fa 100644 --- a/tests/integrations/django/myapp/settings.py +++ b/tests/integrations/django/myapp/settings.py @@ -59,6 +59,11 @@ class TestMiddleware(MiddlewareMixin): def process_request(self, request): + # https://github.com/getsentry/sentry-python/issues/837 -- We should + # not touch the resolver_match because apparently people rely on it. + if request.resolver_match: + assert not getattr(request.resolver_match.callback, "__wrapped__", None) + if "middleware-exc" in request.path: 1 / 0 From 71329ca42ccffb0fb9966ca7e86a0ba4f4d26fad Mon Sep 17 00:00:00 2001 From: sentry-bot Date: Tue, 29 Sep 2020 11:53:34 +0000 Subject: [PATCH 2/3] fix: Formatting --- sentry_sdk/integrations/django/views.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/sentry_sdk/integrations/django/views.py b/sentry_sdk/integrations/django/views.py index 7384bc5a58..a92608a30c 100644 --- a/sentry_sdk/integrations/django/views.py +++ b/sentry_sdk/integrations/django/views.py @@ -27,9 +27,12 @@ def sentry_patched_make_view_atomic(self, *args, **kwargs): integration = hub.get_integration(DjangoIntegration) if integration is not None and integration.middleware_spans: + @_functools.wraps(callback) def sentry_wrapped_callback(request, *args, **kwargs): - with hub.start_span(op="django.view", description=request.resolver_match.view_name): + with hub.start_span( + op="django.view", description=request.resolver_match.view_name + ): return callback(request, *args, **kwargs) else: From 8f160f293b40f79bf7e8460017737f83372f6da6 Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Tue, 29 Sep 2020 16:28:50 +0200 Subject: [PATCH 3/3] fix linters --- sentry_sdk/integrations/django/views.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sentry_sdk/integrations/django/views.py b/sentry_sdk/integrations/django/views.py index a92608a30c..b73ebf29ea 100644 --- a/sentry_sdk/integrations/django/views.py +++ b/sentry_sdk/integrations/django/views.py @@ -5,8 +5,6 @@ if MYPY: from typing import Any - from django.urls.resolvers import ResolverMatch - def patch_views(): # type: () -> None @@ -18,6 +16,7 @@ def patch_views(): @_functools.wraps(old_make_view_atomic) def sentry_patched_make_view_atomic(self, *args, **kwargs): + # type: (Any, *Any, **Any) -> Any callback = old_make_view_atomic(self, *args, **kwargs) # XXX: The wrapper function is created for every request. Find more @@ -30,6 +29,7 @@ def sentry_patched_make_view_atomic(self, *args, **kwargs): @_functools.wraps(callback) def sentry_wrapped_callback(request, *args, **kwargs): + # type: (Any, *Any, **Any) -> Any with hub.start_span( op="django.view", description=request.resolver_match.view_name ):