fix(django): add check for instrument view mro

.circleci/config.yml

@@ -444,15 +444,15 @@ jobs:
           pattern: '^falcon_contrib'
 
   django:
-    executor: ddtrace_dev
-    docker:
-      - image: *ddtrace_dev_image
-      - image: *redis_image
-      - image: *memcached_image
-      - *datadog_agent
+    <<: *machine_executor
     steps:
-      - run_tox_scenario:
-          pattern: '^django_contrib'
+      - checkout
+      - run: SNAPSHOT_CI=1 docker-compose up -d memcached redis testagent
+      - run:
+          command: docker-compose logs -f
+          background: true
+      - run:
+          command: ./scripts/ddtest scripts/run-tox-scenario '^django_contrib-'
 
   djangorestframework:
     executor: ddtrace_dev

ddtrace/contrib/django/__init__.py

@@ -252,9 +252,10 @@
 with require_modules(required_modules) as missing_modules:
     if not missing_modules:
         from .middleware import TraceMiddleware
+        from . import patch as _patch
         from .patch import patch, unpatch
 
-        __all__ = ["patch", "unpatch", "TraceMiddleware"]
+        __all__ = ["patch", "unpatch", "TraceMiddleware", "_patch"]
 
 
 # define the Django app configuration

ddtrace/contrib/django/patch.py

@@ -465,13 +465,11 @@ def instrument_view(django, view):
 
     We want to wrap all lifecycle/http method functions for every class in the MRO for this view
     """
-    if isfunction(view):
-        return _instrument_view(django, view)
+    if hasattr(view, "__mro__"):
+        for cls in reversed(getmro(view)):
+            _instrument_view(django, cls)
 
-    for cls in reversed(getmro(view)):
-        _instrument_view(django, cls)
-
-    return view
+    return _instrument_view(django, view)
 
 
 def _instrument_view(django, view):

releasenotes/notes/django-view-mro-check-782374fb9c427305.yaml

@@ -0,0 +1,4 @@
+---
+fixes:
+  - |
+    Check Django view before instrumenting MRO

tests/__init__.py

@@ -9,7 +9,7 @@
 
 import ddtrace
 from ddtrace import Tracer, Span
-from ddtrace.compat import httplib
+from ddtrace.compat import httplib, to_unicode
 from ddtrace.constants import SPAN_MEASURED_KEY
 from ddtrace.encoding import JSONEncoder
 from ddtrace.ext import http
@@ -844,7 +844,7 @@ def wrapper(wrapped, instance, args, kwargs):
             r = conn.getresponse()
             if r.status != 200:
                 # The test agent returns nice error messages we can forward to the user.
-                raise SnapshotFailed(r.read().decode())
+                raise SnapshotFailed(r.read())
 
             # Run the test.
             ret = wrapped(*args, **kwargs)
@@ -855,14 +855,14 @@ def wrapper(wrapped, instance, args, kwargs):
             # Query for the results of the test.
             conn = httplib.HTTPConnection(tracer.writer.api.hostname, tracer.writer.api.port)
             conn.request("GET", "/test/snapshot?ignores=%s&token=%s" % (",".join(ignores), token))
-            response = conn.getresponse()
-            if response.status != 200:
-                raise SnapshotFailed(response.read().decode())
+            r = conn.getresponse()
+            if r.status != 200:
+                raise SnapshotFailed(r.read())
             return ret
         except SnapshotFailed as e:
             # Fail the test if a failure has occurred and print out the
             # message we got from the test agent.
-            pytest.fail(str(e), pytrace=False)
+            pytest.fail(to_unicode(e.args[0]), pytrace=False)
         finally:
             conn.close()
 

tests/contrib/django/conftest.py

@@ -24,29 +24,24 @@ def pytest_configure():
     django.setup()
 
 
-@pytest.fixture(autouse=True)
-def patch_django(tracer):
+@pytest.fixture
+def tracer():
+    tracer = DummyTracer()
     # Patch Django and override tracer to be our test tracer
     pin = Pin.get_from(django)
     original_tracer = pin.tracer
     Pin.override(django, tracer=tracer)
 
     # Yield to our test
-    yield
+    yield tracer
+    tracer.writer.pop()
 
     # Reset the tracer pinned to Django and unpatch
     # DEV: unable to properly unpatch and reload django app with each test
     # unpatch()
     Pin.override(django, tracer=original_tracer)
 
 
-@pytest.fixture
-def tracer():
-    tracer = DummyTracer()
-    yield tracer
-    tracer.writer.pop()
-
-
 @pytest.fixture
 def test_spans(tracer):
     container = TracerSpanContainer(tracer)

tests/contrib/django/test_django.py

@@ -500,19 +500,6 @@ def test_middleware_trace_function_based_view(client, test_spans):
         assert span.resource == "GET tests.contrib.django.views.function_view"
 
 
-def test_middleware_trace_callable_view(client, test_spans):
-    # ensures that the internals are properly traced when using callable views
-    assert client.get("/feed-view/").status_code == 200
-
-    span = test_spans.get_root_span()
-    assert span.get_tag("http.status_code") == "200"
-    assert span.get_tag(http.URL) == "http://testserver/feed-view/"
-    if django.VERSION >= (2, 2, 0):
-        assert span.resource == "GET ^feed-view/$"
-    else:
-        assert span.resource == "GET tests.contrib.django.views.FeedView"
-
-
 def test_middleware_trace_errors(client, test_spans):
     # ensures that the internals are properly traced
     assert client.get("/fail-view/").status_code == 403
@@ -539,19 +526,6 @@ def test_middleware_trace_staticmethod(client, test_spans):
         assert span.resource == "GET tests.contrib.django.views.StaticMethodView"
 
 
-def test_middleware_trace_partial_based_view(client, test_spans):
-    # ensures that the internals are properly traced when using a function views
-    assert client.get("/partial-view/").status_code == 200
-
-    span = test_spans.get_root_span()
-    assert span.get_tag("http.status_code") == "200"
-    assert span.get_tag(http.URL) == "http://testserver/partial-view/"
-    if django.VERSION >= (2, 2, 0):
-        assert span.resource == "GET ^partial-view/$"
-    else:
-        assert span.resource == "GET partial"
-
-
 def test_simple_view_get(client, test_spans):
     # The `get` method of a view should be traced
     assert client.get("/simple/").status_code == 200
@@ -1352,18 +1326,6 @@ def test_urlpatterns_path(client, test_spans):
     assert len(list(test_spans.filter_spans(name="django.view"))) == 1
 
 
-@pytest.mark.skipif(django.VERSION < (2, 0, 0), reason="include only exists in >=2.0.0")
-def test_urlpatterns_include(client, test_spans):
-    """
-    When a view is specified using `django.urls.include`
-        The view is traced
-    """
-    assert client.get("/include/test/").status_code == 200
-
-    # Ensure the view was traced
-    assert len(list(test_spans.filter_spans(name="django.view"))) == 1
-
-
 @pytest.mark.skipif(django.VERSION < (2, 0, 0), reason="repath only exists in >=2.0.0")
 def test_urlpatterns_repath(client, test_spans):
     """

tests/contrib/django/test_django_snapshots.py

@@ -0,0 +1,80 @@
+import django
+import pytest
+
+from tests import snapshot
+
+
+@pytest.mark.skipif(not (django.VERSION > (2, 0) and django.VERSION < (2, 2)), reason="")
+@snapshot()
+def test_urlpatterns_include_21x(client):
+    """
+    When a view is specified using `django.urls.include`
+        The view is traced
+    """
+    assert client.get("/include/test/").status_code == 200
+
+
+@pytest.mark.skipif(django.VERSION < (2, 2), reason="")
+@snapshot()
+def test_urlpatterns_include(client):
+    """
+    When a view is specified using `django.urls.include`
+        The view is traced
+    """
+    assert client.get("/include/test/").status_code == 200
+
+
+@pytest.mark.skipif(django.VERSION >= (1, 9), reason="")
+@snapshot()
+def test_middleware_trace_callable_view_18x(client):
+    # ensures that the internals are properly traced when using callable views
+    assert client.get("/feed-view/").status_code == 200
+
+
+@pytest.mark.skipif(not (django.VERSION >= (1, 9) and django.VERSION < (1, 12)), reason="")
+@snapshot()
+def test_middleware_trace_callable_view_111x(client):
+    # ensures that the internals are properly traced when using callable views
+    assert client.get("/feed-view/").status_code == 200
+
+
+@pytest.mark.skipif(not (django.VERSION > (1, 12) and django.VERSION < (2, 2)), reason="")
+@snapshot()
+def test_middleware_trace_callable_view_21x(client):
+    # ensures that the internals are properly traced when using callable views
+    assert client.get("/feed-view/").status_code == 200
+
+
+@pytest.mark.skipif(django.VERSION < (2, 2), reason="")
+@snapshot()
+def test_middleware_trace_callable_view(client):
+    # ensures that the internals are properly traced when using callable views
+    assert client.get("/feed-view/").status_code == 200
+
+
+@pytest.mark.skipif(django.VERSION >= (1, 9), reason="")
+@snapshot()
+def test_middleware_trace_partial_based_view_18x(client):
+    # ensures that the internals are properly traced when using a function views
+    assert client.get("/partial-view/").status_code == 200
+
+
+@pytest.mark.skipif(not (django.VERSION >= (1, 9) and django.VERSION < (1, 12)), reason="")
+@snapshot()
+def test_middleware_trace_partial_based_view_111x(client):
+    # ensures that the internals are properly traced when using a function views
+    assert client.get("/partial-view/").status_code == 200
+
+
+@pytest.mark.skipif(not (django.VERSION > (1, 12) and django.VERSION < (2, 2)), reason="")
+@snapshot()
+def test_middleware_trace_partial_based_view_21x(client):
+    # ensures that the internals are properly traced when using a function views
+    assert client.get("/partial-view/").status_code == 200
+
+
+@pytest.mark.skipif(django.VERSION < (2, 2), reason="")
+@snapshot()
+def test_middleware_trace_partial_based_view(client):
+    # ensures that the internals are properly traced when using a function views
+    assert client.get("/partial-view/").status_code == 200

tests/contrib/djangorestframework/conftest.py

@@ -1,11 +1,11 @@
-__all__ = ["pytest_configure", "test_spans", "tracer", "patch_django"]
+__all__ = ["pytest_configure", "test_spans", "tracer"]
 
 import os
 import django
 from django.conf import settings
 
 from ddtrace.contrib.django import patch
-from ..django.conftest import test_spans, tracer, patch_django
+from ..django.conftest import test_spans, tracer
 
 # We manually designate which settings we will be using in an environment variable
 # This is similar to what occurs in the `manage.py`