Skip to content

Commit 123f7af

Browse files
ahmedetefysentry-bot
ahmedetefy
and
sentry-bot
authored
fix(django) - Fix Django async views not behaving asyncronuously (getsentry#992)
* Refactored middlware span creation logic for middleware functions * Added async instrumentation for django middlewares * Added conditional that checks if async * fix: Formatting * Inherit from MiddlewareMixin for async behavior * Refactored __call__ to be like __acall__ for better readability * fix: Formatting * Removed baseclass MiddlewareMixin for unecpected behavior * fix: Formatting * Added async_capable attribute to SentryWrappingMiddleware * Added types to function signatures * Refactored py3 logic to asgi module for py2 compat * fix: Formatting * Fixed function signature error * fix: Formatting * Refactored code to support both versions prior to Django 3.1 and after * fix: Formatting * Refactor middleware arg from asgi mixin factory * fix: Formatting * Added Types and documentation * fix: Formatting * Fixed py2 asgi mixin signature * Added my_async_viewto myapp.views * Added test to ensure concurrent behaviour in both ASGI and Django Channels * Added urlpattern for my_async_view * fix: Formatting * Added test that ensures Performance timing spans are done correctly for async views * Removed print statement * Modified async_route_check function * Added check for forwarding the async calls * fix: Formatting * Fixed django compat asgi_application import issue * Fixed type import issues * Linting changes * fix: Formatting * Fixed failing test by adding safeguard for middleware invocation for older django versions * Removed unused import * Removed redundant ASGI_APP global variable * Added better documentation and modified method name for asgi middleware mixin factory * Removed concurrency test for channels * fix: Formatting * Fixed typing and lint issues Co-authored-by: sentry-bot <markus+ghbot@sentry.io>
1 parent 2df9e1a commit 123f7af

5 files changed

Lines changed: 202 additions & 21 deletions

File tree

‎sentry_sdk/integrations/django/asgi.py‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
`django.core.handlers.asgi`.
77
"""
88

9+
import asyncio
10+
911
from sentry_sdk import Hub, _functools
1012
from sentry_sdk._types import MYPY
1113

@@ -14,6 +16,7 @@
1416
if MYPY:
1517
from typing import Any
1618
from typing import Union
19+
from typing import Callable
1720

1821
from django.http.response import HttpResponse
1922

@@ -91,3 +94,52 @@ async def sentry_wrapped_callback(request, *args, **kwargs):
9194
return await callback(request, *args, **kwargs)
9295

9396
return sentry_wrapped_callback
97+
98+
99+
def _asgi_middleware_mixin_factory(_check_middleware_span):
100+
# type: (Callable[..., Any]) -> Any
101+
"""
102+
Mixin class factory that generates a middleware mixin for handling requests
103+
in async mode.
104+
"""
105+
106+
class SentryASGIMixin:
107+
def __init__(self, get_response):
108+
# type: (Callable[..., Any]) -> None
109+
self.get_response = get_response
110+
self._acall_method = None
111+
self._async_check()
112+
113+
def _async_check(self):
114+
# type: () -> None
115+
"""
116+
If get_response is a coroutine function, turns us into async mode so
117+
a thread is not consumed during a whole request.
118+
Taken from django.utils.deprecation::MiddlewareMixin._async_check
119+
"""
120+
if asyncio.iscoroutinefunction(self.get_response):
121+
self._is_coroutine = asyncio.coroutines._is_coroutine # type: ignore
122+
123+
def async_route_check(self):
124+
# type: () -> bool
125+
"""
126+
Function that checks if we are in async mode,
127+
and if we are forwards the handling of requests to __acall__
128+
"""
129+
return asyncio.iscoroutinefunction(self.get_response)
130+
131+
async def __acall__(self, *args, **kwargs):
132+
# type: (*Any, **Any) -> Any
133+
f = self._acall_method
134+
if f is None:
135+
self._acall_method = f = self._inner.__acall__ # type: ignore
136+
137+
middleware_span = _check_middleware_span(old_method=f)
138+
139+
if middleware_span is None:
140+
return await f(*args, **kwargs)
141+
142+
with middleware_span:
143+
return await f(*args, **kwargs)
144+
145+
return SentryASGIMixin

‎sentry_sdk/integrations/django/middleware.py‎

Lines changed: 62 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,11 @@
1616
if MYPY:
1717
from typing import Any
1818
from typing import Callable
19+
from typing import Optional
1920
from typing import TypeVar
2021

22+
from sentry_sdk.tracing import Span
23+
2124
F = TypeVar("F", bound=Callable[..., Any])
2225

2326
_import_string_should_wrap_middleware = ContextVar(
@@ -30,6 +33,12 @@
3033
import_string_name = "import_string"
3134

3235

36+
if DJANGO_VERSION < (3, 1):
37+
_asgi_middleware_mixin_factory = lambda _: object
38+
else:
39+
from .asgi import _asgi_middleware_mixin_factory
40+
41+
3342
def patch_django_middlewares():
3443
# type: () -> None
3544
from django.core.handlers import base
@@ -64,29 +73,40 @@ def _wrap_middleware(middleware, middleware_name):
6473
# type: (Any, str) -> Any
6574
from sentry_sdk.integrations.django import DjangoIntegration
6675

76+
def _check_middleware_span(old_method):
77+
# type: (Callable[..., Any]) -> Optional[Span]
78+
hub = Hub.current
79+
integration = hub.get_integration(DjangoIntegration)
80+
if integration is None or not integration.middleware_spans:
81+
return None
82+
83+
function_name = transaction_from_function(old_method)
84+
85+
description = middleware_name
86+
function_basename = getattr(old_method, "__name__", None)
87+
if function_basename:
88+
description = "{}.{}".format(description, function_basename)
89+
90+
middleware_span = hub.start_span(
91+
op="django.middleware", description=description
92+
)
93+
middleware_span.set_tag("django.function_name", function_name)
94+
middleware_span.set_tag("django.middleware_name", middleware_name)
95+
96+
return middleware_span
97+
6798
def _get_wrapped_method(old_method):
6899
# type: (F) -> F
69100
with capture_internal_exceptions():
70101

71102
def sentry_wrapped_method(*args, **kwargs):
72103
# type: (*Any, **Any) -> Any
73-
hub = Hub.current
74-
integration = hub.get_integration(DjangoIntegration)
75-
if integration is None or not integration.middleware_spans:
76-
return old_method(*args, **kwargs)
77-
78-
function_name = transaction_from_function(old_method)
104+
middleware_span = _check_middleware_span(old_method)
79105

80-
description = middleware_name
81-
function_basename = getattr(old_method, "__name__", None)
82-
if function_basename:
83-
description = "{}.{}".format(description, function_basename)
106+
if middleware_span is None:
107+
return old_method(*args, **kwargs)
84108

85-
with hub.start_span(
86-
op="django.middleware", description=description
87-
) as span:
88-
span.set_tag("django.function_name", function_name)
89-
span.set_tag("django.middleware_name", middleware_name)
109+
with middleware_span:
90110
return old_method(*args, **kwargs)
91111

92112
try:
@@ -102,11 +122,22 @@ def sentry_wrapped_method(*args, **kwargs):
102122

103123
return old_method
104124

105-
class SentryWrappingMiddleware(object):
106-
def __init__(self, *args, **kwargs):
107-
# type: (*Any, **Any) -> None
108-
self._inner = middleware(*args, **kwargs)
125+
class SentryWrappingMiddleware(
126+
_asgi_middleware_mixin_factory(_check_middleware_span) # type: ignore
127+
):
128+
129+
async_capable = getattr(middleware, "async_capable", False)
130+
131+
def __init__(self, get_response=None, *args, **kwargs):
132+
# type: (Optional[Callable[..., Any]], *Any, **Any) -> None
133+
if get_response:
134+
self._inner = middleware(get_response, *args, **kwargs)
135+
else:
136+
self._inner = middleware(*args, **kwargs)
137+
self.get_response = get_response
109138
self._call_method = None
139+
if self.async_capable:
140+
super(SentryWrappingMiddleware, self).__init__(get_response)
110141

111142
# We need correct behavior for `hasattr()`, which we can only determine
112143
# when we have an instance of the middleware we're wrapping.
@@ -128,10 +159,20 @@ def __getattr__(self, method_name):
128159

129160
def __call__(self, *args, **kwargs):
130161
# type: (*Any, **Any) -> Any
162+
if hasattr(self, "async_route_check") and self.async_route_check():
163+
return self.__acall__(*args, **kwargs)
164+
131165
f = self._call_method
132166
if f is None:
133-
self._call_method = f = _get_wrapped_method(self._inner.__call__)
134-
return f(*args, **kwargs)
167+
self._call_method = f = self._inner.__call__
168+
169+
middleware_span = _check_middleware_span(old_method=f)
170+
171+
if middleware_span is None:
172+
return f(*args, **kwargs)
173+
174+
with middleware_span:
175+
return f(*args, **kwargs)
135176

136177
if hasattr(middleware, "__name__"):
137178
SentryWrappingMiddleware.__name__ = middleware.__name__

‎tests/integrations/django/asgi/test_asgi.py‎

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,3 +68,80 @@ async def test_async_views(sentry_init, capture_events, application):
6868
"query_string": None,
6969
"url": "/async_message",
7070
}
71+
72+
73+
@pytest.mark.asyncio
74+
@pytest.mark.skipif(
75+
django.VERSION < (3, 1), reason="async views have been introduced in Django 3.1"
76+
)
77+
async def test_async_views_concurrent_execution(sentry_init, capture_events, settings):
78+
import asyncio
79+
import time
80+
81+
settings.MIDDLEWARE = []
82+
asgi_application.load_middleware(is_async=True)
83+
84+
sentry_init(integrations=[DjangoIntegration()], send_default_pii=True)
85+
86+
comm = HttpCommunicator(asgi_application, "GET", "/my_async_view")
87+
comm2 = HttpCommunicator(asgi_application, "GET", "/my_async_view")
88+
89+
loop = asyncio.get_event_loop()
90+
91+
start = time.time()
92+
93+
r1 = loop.create_task(comm.get_response(timeout=5))
94+
r2 = loop.create_task(comm2.get_response(timeout=5))
95+
96+
(resp1, resp2), _ = await asyncio.wait({r1, r2})
97+
98+
end = time.time()
99+
100+
assert resp1.result()["status"] == 200
101+
assert resp2.result()["status"] == 200
102+
103+
assert end - start < 1.5
104+
105+
106+
@pytest.mark.asyncio
107+
@pytest.mark.skipif(
108+
django.VERSION < (3, 1), reason="async views have been introduced in Django 3.1"
109+
)
110+
async def test_async_middleware_spans(
111+
sentry_init, render_span_tree, capture_events, settings
112+
):
113+
settings.MIDDLEWARE = [
114+
"django.contrib.sessions.middleware.SessionMiddleware",
115+
"django.contrib.auth.middleware.AuthenticationMiddleware",
116+
"django.middleware.csrf.CsrfViewMiddleware",
117+
"tests.integrations.django.myapp.settings.TestMiddleware",
118+
]
119+
asgi_application.load_middleware(is_async=True)
120+
121+
sentry_init(
122+
integrations=[DjangoIntegration(middleware_spans=True)],
123+
traces_sample_rate=1.0,
124+
_experiments={"record_sql_params": True},
125+
)
126+
127+
events = capture_events()
128+
129+
comm = HttpCommunicator(asgi_application, "GET", "/async_message")
130+
response = await comm.get_response()
131+
assert response["status"] == 200
132+
133+
await comm.wait()
134+
135+
message, transaction = events
136+
137+
assert (
138+
render_span_tree(transaction)
139+
== """\
140+
- op="http.server": description=null
141+
- op="django.middleware": description="django.contrib.sessions.middleware.SessionMiddleware.__acall__"
142+
- op="django.middleware": description="django.contrib.auth.middleware.AuthenticationMiddleware.__acall__"
143+
- op="django.middleware": description="django.middleware.csrf.CsrfViewMiddleware.__acall__"
144+
- op="django.middleware": description="tests.integrations.django.myapp.settings.TestMiddleware.__acall__"
145+
- op="django.middleware": description="django.middleware.csrf.CsrfViewMiddleware.process_view"
146+
- op="django.view": description="async_message\""""
147+
)

‎tests/integrations/django/myapp/urls.py‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,9 @@ def path(path, *args, **kwargs):
6363
if views.async_message is not None:
6464
urlpatterns.append(path("async_message", views.async_message, name="async_message"))
6565

66+
if views.my_async_view is not None:
67+
urlpatterns.append(path("my_async_view", views.my_async_view, name="my_async_view"))
68+
6669
# rest framework
6770
try:
6871
urlpatterns.append(

‎tests/integrations/django/myapp/views.py‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,5 +141,13 @@ def csrf_hello_not_exempt(*args, **kwargs):
141141
sentry_sdk.capture_message("hi")
142142
return HttpResponse("ok")"""
143143
)
144+
145+
exec(
146+
"""async def my_async_view(request):
147+
import asyncio
148+
await asyncio.sleep(1)
149+
return HttpResponse('Hello World')"""
150+
)
144151
else:
145152
async_message = None
153+
my_async_view = None

0 commit comments

Comments
 (0)