Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Jan 19, 2022
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
29 changes: 28 additions & 1 deletion sentry_sdk/integrations/django/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
"""
Expand All @@ -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

Expand Down
31 changes: 31 additions & 0 deletions tests/integrations/django/myapp/custom_urls.py
Original file line number Diff line number Diff line change
@@ -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"),
]
35 changes: 23 additions & 12 deletions tests/integrations/django/myapp/middleware.py
Original file line number Diff line number Diff line change
@@ -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
5 changes: 5 additions & 0 deletions tests/integrations/django/myapp/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
27 changes: 27 additions & 0 deletions tests/integrations/django/test_basic.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Copy link
Member

Choose a reason for hiding this comment

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

why do we need the bytes literal here?

Copy link
Member Author

Choose a reason for hiding this comment

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

it's a byte stream or something from werkzeug, not a string. I just followed the other tests.


(event,) = events
assert event["transaction"] == "/custom/ok"
assert "custom_urlconf_middleware" in render_span_tree(event)

settings.MIDDLEWARE.pop(0)