Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions sentry_sdk/integrations/django/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -200,7 +200,7 @@ def _django_queryset_repr(value, hint):

_patch_channels()
patch_django_middlewares()
patch_resolver()
patch_views()


_DRF_PATCHED = False
Expand Down
70 changes: 23 additions & 47 deletions sentry_sdk/integrations/django/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,63 +5,39 @@
if MYPY:
from typing import Any

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

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)
old_make_view_atomic = BaseHandler.make_view_atomic
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is make_view_atomic available in every version of Django we support?

Had quick look at
https://github.com/django/django/blob/c1442e1192057a3bf14aecbaa1b713eee139eaff/django/core/handlers/base.py#L320

It is not so obvious that this is called for every view once.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make_view_atomic is called in get_response_async and get_response which is where all requests go through. It is the only thing called directly before accessing the resolver match, so it should be pretty close with regard to how timings are done.

For example called here: https://github.com/django/django/blob/c1442e1192057a3bf14aecbaa1b713eee139eaff/django/core/handlers/base.py#L174

The impact on timing I could see is that the function wrapper is created at a different point in time, so it could shift instrumentation overhead to a different point in time and therefore move a small gap in teh trace somewhere else.


return _wrap_resolver_match(hub, old_resolve(self, path))
@_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)

URLResolver.resolve = resolve
# XXX: The wrapper function is created for every request. Find more
# efficient way to wrap views (or build a cache?)

hub = Hub.current
integration = hub.get_integration(DjangoIntegration)

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
if integration is not None and integration.middleware_spans:

# 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",)
@_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
):
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
5 changes: 5 additions & 0 deletions tests/integrations/django/myapp/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down