From eaa2618896bad1000eb2f093f386a9c89724a1fb Mon Sep 17 00:00:00 2001 From: Michael Manganiello Date: Fri, 4 Jun 2021 20:50:45 -0300 Subject: [PATCH] Migrate Django middleware to new-style New-style middlewares were [introduced][django_1_10_changelog] in Django `1.10`, and also `settings.MIDDLEWARE_CLASSES` was [removed][django_2_0_changelog] in Django 2.0. This change migrates the Django middleware to conform with the new style. This is useful because it will help us solve the pending issue in https://github.com/open-telemetry/opentelemetry-python-contrib/pull/391. By having a single entrypoint to the middleware, `__call__`, which is [wrapped with `sync_to_async` just once][call_wrapped] for async requests, we avoid the [issue][asgiref_issue] where a `ContextVar` cannot be reset from a different context. With the current deprecated `MiddlewareMixin` way, both `process_request` and `process_response` were being [wrapped separately with `sync_to_async`][mixin_wrapped], which was the source of the mentioned issue. [django_1_10_changelog]: https://docs.djangoproject.com/en/3.2/releases/1.10/#new-style-middleware [django_2_0_changelog]: https://docs.djangoproject.com/en/3.2/releases/2.0/#features-removed-in-2-0 [call_wrapped]: https://github.com/django/django/blob/213850b4b9641bdcb714172999725ec9aa9c9e84/django/core/handlers/base.py#L54-L57 [mixin_wrapped]: https://github.com/django/django/blob/213850b4b9641bdcb714172999725ec9aa9c9e84/django/utils/deprecation.py#L137-L147 [asgiref_issue]: https://github.com/django/asgiref/issues/267 --- CHANGELOG.md | 2 ++ .../instrumentation/django/middleware.py | 15 +++++++++------ 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 66d75fa655..b5be15a96f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `opentelemetry-instrumentation` Fixed cases where trying to use an instrumentation package without the target library was crashing auto instrumentation agent. ([#530](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/530)) +- `opentelemetry-instrumentation-django` Migrated Django middleware to new-style. + ([#533](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/533)) ## [0.22b0](https://github.com/open-telemetry/opentelemetry-python/releases/tag/v1.3.0-0.22b0) - 2021-06-01 diff --git a/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware.py b/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware.py index f37372d721..6846aa9fa8 100644 --- a/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware.py +++ b/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware.py @@ -42,11 +42,6 @@ except ImportError: from django.urls import Resolver404, resolve -try: - from django.utils.deprecation import MiddlewareMixin -except ImportError: - MiddlewareMixin = object - _logger = getLogger(__name__) _attributes_by_preference = [ [ @@ -70,7 +65,7 @@ ] -class _DjangoMiddleware(MiddlewareMixin): +class _DjangoMiddleware: """Django Middleware for OpenTelemetry""" _environ_activation_key = ( @@ -89,6 +84,9 @@ class _DjangoMiddleware(MiddlewareMixin): [Span, HttpRequest, HttpResponse], None ] = None + def __init__(self, get_response): + self.get_response = get_response + @staticmethod def _get_span_name(request): try: @@ -111,6 +109,11 @@ def _get_span_name(request): except Resolver404: return "HTTP {}".format(request.method) + def __call__(self, request): + self.process_request(request) + response = self.get_response(request) + return self.process_response(request, response) + def process_request(self, request): # request.META is a dictionary containing all available HTTP headers # Read more about request.META here: