django: Fix instrumentation and tests for all Django major versions (#780)

CHANGELOG.md

@@ -25,6 +25,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   ([#767](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/767))
 - Don't set Span Status on 4xx http status code for SpanKind.SERVER spans
   ([#776](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/776))
+- `opentelemetry-instrumentation-django` Fixed instrumentation and tests for all Django major versions.
+  ([#780](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/780))
 
 ## [1.6.2-0.25b2](https://github.com/open-telemetry/opentelemetry-python/releases/tag/v1.6.2-0.25b2) - 2021-10-19
 

instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/__init__.py

@@ -83,6 +83,7 @@ from logging import getLogger
 from os import environ
 from typing import Collection
 
+from django import VERSION as django_version
 from django.conf import settings
 
 from opentelemetry.instrumentation.django.environment_variables import (
@@ -94,9 +95,20 @@ from opentelemetry.instrumentation.django.version import __version__
 from opentelemetry.instrumentation.instrumentor import BaseInstrumentor
 from opentelemetry.trace import get_tracer
 
+DJANGO_2_0 = django_version >= (2, 0)
+
 _logger = getLogger(__name__)
 
 
+def _get_django_middleware_setting() -> str:
+    # In Django versions 1.x, setting MIDDLEWARE_CLASSES can be used as a legacy
+    # alternative to MIDDLEWARE. This is the case when `settings.MIDDLEWARE` has
+    # its default value (`None`).
+    if not DJANGO_2_0 and getattr(settings, "MIDDLEWARE", []) is None:
+        return "MIDDLEWARE_CLASSES"
+    return "MIDDLEWARE"
+
+
 class DjangoInstrumentor(BaseInstrumentor):
     """An instrumentor for Django
 
@@ -135,17 +147,20 @@ class DjangoInstrumentor(BaseInstrumentor):
         # https://docs.djangoproject.com/en/3.0/topics/http/middleware/#activating-middleware
         # https://docs.djangoproject.com/en/3.0/ref/middleware/#middleware-ordering
 
-        settings_middleware = getattr(settings, "MIDDLEWARE", [])
+        _middleware_setting = _get_django_middleware_setting()
+        settings_middleware = getattr(settings, _middleware_setting, [])
+
         # Django allows to specify middlewares as a tuple, so we convert this tuple to a
         # list, otherwise we wouldn't be able to call append/remove
         if isinstance(settings_middleware, tuple):
             settings_middleware = list(settings_middleware)
 
         settings_middleware.insert(0, self._opentelemetry_middleware)
-        setattr(settings, "MIDDLEWARE", settings_middleware)
+        setattr(settings, _middleware_setting, settings_middleware)
 
     def _uninstrument(self, **kwargs):
-        settings_middleware = getattr(settings, "MIDDLEWARE", None)
+        _middleware_setting = _get_django_middleware_setting()
+        settings_middleware = getattr(settings, _middleware_setting, None)
 
         # FIXME This is starting to smell like trouble. We have 2 mechanisms
         # that may make this condition be True, one implemented in
@@ -158,4 +173,4 @@ class DjangoInstrumentor(BaseInstrumentor):
             return
 
         settings_middleware.remove(self._opentelemetry_middleware)
-        setattr(settings, "MIDDLEWARE", settings_middleware)
+        setattr(settings, _middleware_setting, settings_middleware)

instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py

@@ -19,7 +19,6 @@ from django import VERSION, conf
 from django.http import HttpRequest, HttpResponse
 from django.test.client import Client
 from django.test.utils import setup_test_environment, teardown_test_environment
-from django.urls import re_path
 
 from opentelemetry.instrumentation.django import (
     DjangoInstrumentor,
@@ -54,7 +53,14 @@ from .views import (
     traced_template,
 )
 
+DJANGO_2_0 = VERSION >= (2, 0)
 DJANGO_2_2 = VERSION >= (2, 2)
+DJANGO_3_0 = VERSION >= (3, 0)
+
+if DJANGO_2_0:
+    from django.urls import re_path
+else:
+    from django.conf.urls import url as re_path
 
 urlpatterns = [
     re_path(r"^traced/", traced),
@@ -122,7 +128,7 @@ class TestMiddleware(TestBase, WsgiTestBase):
             span.name,
             "^route/(?P<year>[0-9]{4})/template/$"
             if DJANGO_2_2
-            else "tests.views.traced",
+            else "tests.views.traced_template",
         )
         self.assertEqual(span.kind, SpanKind.SERVER)
         self.assertEqual(span.status.status_code, StatusCode.UNSET)
@@ -131,6 +137,7 @@ class TestMiddleware(TestBase, WsgiTestBase):
             span.attributes[SpanAttributes.HTTP_URL],
             "http://testserver/route/2020/template/",
         )
+        if DJANGO_2_2:
             self.assertEqual(
                 span.attributes[SpanAttributes.HTTP_ROUTE],
                 "^route/(?P<year>[0-9]{4})/template/$",
@@ -156,6 +163,7 @@ class TestMiddleware(TestBase, WsgiTestBase):
             span.attributes[SpanAttributes.HTTP_URL],
             "http://testserver/traced/",
         )
+        if DJANGO_2_2:
             self.assertEqual(
                 span.attributes[SpanAttributes.HTTP_ROUTE], "^traced/"
             )
@@ -193,6 +201,7 @@ class TestMiddleware(TestBase, WsgiTestBase):
             span.attributes[SpanAttributes.HTTP_URL],
             "http://testserver/traced/",
         )
+        if DJANGO_2_2:
             self.assertEqual(
                 span.attributes[SpanAttributes.HTTP_ROUTE], "^traced/"
             )
@@ -218,7 +227,10 @@ class TestMiddleware(TestBase, WsgiTestBase):
             span.attributes[SpanAttributes.HTTP_URL],
             "http://testserver/error/",
         )
-        self.assertEqual(span.attributes[SpanAttributes.HTTP_ROUTE], "^error/")
+        if DJANGO_2_2:
+            self.assertEqual(
+                span.attributes[SpanAttributes.HTTP_ROUTE], "^error/"
+            )
         self.assertEqual(span.attributes[SpanAttributes.HTTP_SCHEME], "http")
         self.assertEqual(span.attributes[SpanAttributes.HTTP_STATUS_CODE], 500)
 
@@ -363,14 +375,14 @@ class TestMiddleware(TestBase, WsgiTestBase):
     def test_trace_response_headers(self):
         response = Client().get("/span_name/1234/")
 
-        self.assertNotIn("Server-Timing", response.headers)
+        self.assertFalse(response.has_header("Server-Timing"))
         self.memory_exporter.clear()
 
         set_global_response_propagator(TraceResponsePropagator())
 
         response = Client().get("/span_name/1234/")
         self.assertTraceResponseHeaderMatchesSpan(
-            response.headers, self.memory_exporter.get_finished_spans()[0]
+            response, self.memory_exporter.get_finished_spans()[0],
         )
         self.memory_exporter.clear()
 

instrumentation/opentelemetry-instrumentation-django/tests/test_middleware_asgi.py

@@ -20,7 +20,6 @@ from django import VERSION, conf
 from django.http import HttpRequest, HttpResponse
 from django.test import SimpleTestCase
 from django.test.utils import setup_test_environment, teardown_test_environment
-from django.urls import re_path
 
 from opentelemetry.instrumentation.django import (
     DjangoInstrumentor,
@@ -54,8 +53,14 @@ from .views import (
     async_traced_template,
 )
 
+DJANGO_2_0 = VERSION >= (2, 0)
 DJANGO_3_1 = VERSION >= (3, 1)
 
+if DJANGO_2_0:
+    from django.urls import re_path
+else:
+    from django.conf.urls import url as re_path
+
 urlpatterns = [
     re_path(r"^traced/", async_traced),
     re_path(r"^route/(?P<year>[0-9]{4})/template/$", async_traced_template),
@@ -344,7 +349,7 @@ class TestMiddlewareAsgi(SimpleTestCase, TestBase):
     async def test_trace_response_headers(self):
         response = await self.async_client.get("/span_name/1234/")
 
-        self.assertNotIn("Server-Timing", response.headers)
+        self.assertFalse(response.has_header("Server-Timing"))
         self.memory_exporter.clear()
 
         set_global_response_propagator(TraceResponsePropagator())
@@ -352,12 +357,12 @@ class TestMiddlewareAsgi(SimpleTestCase, TestBase):
         response = await self.async_client.get("/span_name/1234/")
         span = self.memory_exporter.get_finished_spans()[0]
 
-        self.assertIn("traceresponse", response.headers)
+        self.assertTrue(response.has_header("traceresponse"))
         self.assertEqual(
-            response.headers["Access-Control-Expose-Headers"], "traceresponse",
+            response["Access-Control-Expose-Headers"], "traceresponse",
         )
         self.assertEqual(
-            response.headers["traceresponse"],
+            response["traceresponse"],
             "00-{}-{}-01".format(
                 format_trace_id(span.get_span_context().trace_id),
                 format_span_id(span.get_span_context().span_id),

tox.ini

@@ -30,8 +30,14 @@ envlist =
     pypy3-test-instrumentation-botocore
 
     ; opentelemetry-instrumentation-django
-    py3{6,7,8,9,10}-test-instrumentation-django
+    ; Only officially supported Python versions are tested for each Django
-    pypy3-test-instrumentation-django
+    ; major release. Updated list can be found at:
+    ; https://docs.djangoproject.com/en/dev/faq/install/#what-python-version-can-i-use-with-django
+    py3{6,7}-test-instrumentation-django1
+    py3{6,7,8,9}-test-instrumentation-django2
+    py3{6,7,8,9,10}-test-instrumentation-django3
+    py3{8,9,10}-test-instrumentation-django4
+    pypy3-test-instrumentation-django{1,2,3}
 
     ; opentelemetry-instrumentation-dbapi
     py3{6,7,8,9,10}-test-instrumentation-dbapi
@@ -186,6 +192,10 @@ deps =
   test: pytest-benchmark
   coverage: pytest
   coverage: pytest-cov
+  django1: django~=1.0
+  django2: django~=2.0
+  django3: django~=3.0
+  django4: django>=4.0b1,<5.0
   elasticsearch2: elasticsearch-dsl>=2.0,<3.0
   elasticsearch2: elasticsearch>=2.0,<3.0
   elasticsearch5: elasticsearch-dsl>=5.0,<6.0
@@ -221,7 +231,7 @@ changedir =
   test-instrumentation-botocore: instrumentation/opentelemetry-instrumentation-botocore/tests
   test-instrumentation-celery: instrumentation/opentelemetry-instrumentation-celery/tests
   test-instrumentation-dbapi: instrumentation/opentelemetry-instrumentation-dbapi/tests
-  test-instrumentation-django: instrumentation/opentelemetry-instrumentation-django/tests
+  test-instrumentation-django{1,2,3,4}: instrumentation/opentelemetry-instrumentation-django/tests
   test-instrumentation-elasticsearch{2,5,6}: instrumentation/opentelemetry-instrumentation-elasticsearch/tests
   test-instrumentation-falcon{2,3}: instrumentation/opentelemetry-instrumentation-falcon/tests
   test-instrumentation-fastapi: instrumentation/opentelemetry-instrumentation-fastapi/tests
@@ -272,9 +282,9 @@ commands_pre =
 
   grpc: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-grpc[test]
 
-  falcon{2,3},flask,django,pyramid,tornado,starlette,fastapi,aiohttp,asgi,requests,urllib,urllib3,wsgi: pip install {toxinidir}/util/opentelemetry-util-http[test]
+  falcon{2,3},flask,django{1,2,3,4},pyramid,tornado,starlette,fastapi,aiohttp,asgi,requests,urllib,urllib3,wsgi: pip install {toxinidir}/util/opentelemetry-util-http[test]
-  wsgi,falcon{2,3},flask,django,pyramid: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-wsgi[test]
+  wsgi,falcon{2,3},flask,django{1,2,3,4},pyramid: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-wsgi[test]
-  asgi,django,starlette,fastapi: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-asgi[test]
+  asgi,django{3,4},starlette,fastapi: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-asgi[test]
 
   asyncpg: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-asyncpg[test]
 
@@ -293,7 +303,7 @@ commands_pre =
 
   dbapi: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-dbapi[test]
 
-  django: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-django[test]
+  django{1,2,3,4}: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-django[test]
 
   fastapi: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-fastapi[test]