django: reintroduce wrapt for view method patching (#1622)

Since GrahamDumpleton/wrapt#153 was fixed, we
can use wrapt again for the view method patching.

Author: Kyle-Verhoog

ddtrace/compat.py

@@ -117,104 +117,6 @@ def make_async_decorator(tracer, fn, *params, **kw_params):
         return fn
 
 
-# static version of getattr backported from Python 3.7
-try:
-    from inspect import getattr_static
-except ImportError:
-    import types
-
-    _sentinel = object()
-
-    def _static_getmro(klass):
-        return type.__dict__["__mro__"].__get__(klass)
-
-    def _check_instance(obj, attr):
-        instance_dict = {}
-        try:
-            instance_dict = object.__getattribute__(obj, "__dict__")
-        except AttributeError:
-            pass
-        return dict.get(instance_dict, attr, _sentinel)
-
-    def _check_class(klass, attr):
-        for entry in _static_getmro(klass):
-            if _shadowed_dict(type(entry)) is _sentinel:
-                try:
-                    return entry.__dict__[attr]
-                except KeyError:
-                    pass
-        return _sentinel
-
-    def _is_type(obj):
-        try:
-            _static_getmro(obj)
-        except TypeError:
-            return False
-        return True
-
-    def _shadowed_dict(klass):
-        dict_attr = type.__dict__["__dict__"]
-        for entry in _static_getmro(klass):
-            try:
-                class_dict = dict_attr.__get__(entry)["__dict__"]
-            except KeyError:
-                pass
-            else:
-                if not (
-                    type(class_dict) is types.GetSetDescriptorType  # noqa: E721
-                    and class_dict.__name__ == "__dict__"  # noqa: E721,E261,W504
-                    and class_dict.__objclass__ is entry  # noqa: E261,W504
-                ):
-                    return class_dict
-        return _sentinel
-
-    def getattr_static(obj, attr, default=_sentinel):
-        """Retrieve attributes without triggering dynamic lookup via the
-        descriptor protocol,  __getattr__ or __getattribute__.
-
-        Note: this function may not be able to retrieve all attributes
-        that getattr can fetch (like dynamically created attributes)
-        and may find attributes that getattr can't (like descriptors
-        that raise AttributeError). It can also return descriptor objects
-        instead of instance members in some cases. See the
-        documentation for details.
-        """
-        instance_result = _sentinel
-        if not _is_type(obj):
-            klass = type(obj)
-            dict_attr = _shadowed_dict(klass)
-            if dict_attr is _sentinel or type(dict_attr) is types.MemberDescriptorType:  # noqa: E261,E721,W504
-                instance_result = _check_instance(obj, attr)
-        else:
-            klass = obj
-
-        klass_result = _check_class(klass, attr)
-
-        if instance_result is not _sentinel and klass_result is not _sentinel:
-            if (
-                _check_class(type(klass_result), "__get__") is not _sentinel
-                and _check_class(type(klass_result), "__set__") is not _sentinel  # noqa: W504,E261,E721
-            ):
-                return klass_result
-
-        if instance_result is not _sentinel:
-            return instance_result
-        if klass_result is not _sentinel:
-            return klass_result
-
-        if obj is klass:
-            # for types we check the metaclass too
-            for entry in _static_getmro(type(klass)):
-                if _shadowed_dict(type(entry)) is _sentinel:
-                    try:
-                        return entry.__dict__[attr]
-                    except KeyError:
-                        pass
-        if default is not _sentinel:
-            return default
-        raise AttributeError(attr)
-
-
 # DEV: There is `six.u()` which does something similar, but doesn't have the guard around `hasattr(s, 'decode')`
 def to_unicode(s):
     """ Return a unicode string for the given bytes or string instance. """

ddtrace/contrib/django/patch.py

@@ -13,7 +13,6 @@
 
 from ddtrace import config, Pin
 from ddtrace.vendor import debtcollector, wrapt
-from ddtrace.compat import getattr_static
 from ddtrace.constants import ANALYTICS_SAMPLE_RATE_KEY
 from ddtrace.contrib import func_name, dbapi
 from ddtrace.ext import http, sql as sqlx, SpanTypes
@@ -444,17 +443,13 @@ def instrument_view(django, view):
     for name in list(http_method_names) + list(lifecycle_methods):
         try:
             # View methods can be staticmethods
-            func = getattr_static(view, name, None)
+            func = getattr(view, name, None)
             if not func or isinstance(func, wrapt.ObjectProxy):
                 continue
 
             resource = "{0}.{1}".format(func_name(view), name)
             op_name = "django.view.{0}".format(name)
-
-            # Set attribute here rather than using wrapt.wrappers.wrap_function_wrapper
-            # since it will not resolve attribute to staticmethods
-            wrapper = wrapt.FunctionWrapper(func, traced_func(django, name=op_name, resource=resource))
-            setattr(view, name, wrapper)
+            wrap(view, name, traced_func(django, name=op_name, resource=resource))
         except Exception:
             log.debug("Failed to instrument Django view %r function %s", view, name, exc_info=True)
 

tests/contrib/django/django1_app/urls.py

@@ -17,4 +17,6 @@
     url(r"^partial-view/$", views.partial_view, name="partial-view"),
     url(r"^lambda-view/$", views.lambda_view, name="lambda-view"),
     url(r"^error-500/$", views.error_500, name="error-500"),
+    url(r"^composed-template-view/$", views.ComposedTemplateView.as_view(), name="composed-template-view"),
+    url(r"^composed-get-view/$", views.ComposedGetView.as_view(), name="composed-get-view"),
 ]

tests/contrib/django/django_app/urls.py

@@ -30,4 +30,6 @@ def path_view(request):
     re_path(r"re-path.*/", repath_view),
     path("path/", path_view),
     path("include/", include("tests.contrib.django.django_app.extra_urls")),
+    url(r"^composed-template-view/$", views.ComposedTemplateView.as_view(), name="composed-template-view"),
+    url(r"^composed-get-view/$", views.ComposedGetView.as_view(), name="composed-get-view"),
 ]

tests/contrib/django/templates/custom_dispatch.html

@@ -0,0 +1 @@
+custom dispatch {{ dispatch_call_counter }}

tests/contrib/django/test_django.py

@@ -1226,3 +1226,36 @@ def test_urlpatterns_repath(client, test_spans):
 
     # Ensure the view was traced
     assert len(list(test_spans.filter_spans(name="django.view"))) == 1
+
+
+def test_custom_dispatch_template_view(client, test_spans):
+    """
+    Test that a template view with a custom dispatch method inherited from a
+    mixin is called.
+    """
+    resp = client.get("/composed-template-view/")
+    assert resp.status_code == 200
+    assert resp.content.strip() == b"custom dispatch 2"
+
+    spans = test_spans.get_spans()
+    assert [s.resource for s in spans if s.resource.endswith("dispatch")] == [
+        "tests.contrib.django.views.ComposedTemplateView.dispatch",
+    ]
+
+
+def test_custom_dispatch_get_view(client, test_spans):
+    """
+    Test that a get method on a view with a custom dispatch method inherited
+    from a mixin is called.
+    """
+    resp = client.get("/composed-get-view/")
+    assert resp.status_code == 200
+    assert resp.content.strip() == b"custom get"
+
+    spans = test_spans.get_spans()
+    assert [s.resource for s in spans if s.resource.endswith("dispatch")] == [
+        "tests.contrib.django.views.ComposedGetView.dispatch",
+    ]
+    assert [s.resource for s in spans if s.resource.endswith("get")] == [
+        "tests.contrib.django.views.ComposedGetView.get",
+    ]

tests/contrib/django/views.py

@@ -82,3 +82,39 @@ def item_description(self, item):
 
 def index(request):
     return HttpResponse("Hello, test app.")
+
+
+class CustomDispatchMixin(View):
+    def dispatch(self, request):
+        self.dispatch_call_counter += 1
+        return super(CustomDispatchMixin, self).dispatch(request)
+
+
+class AnotherCustomDispatchMixin(View):
+    def dispatch(self, request):
+        self.dispatch_call_counter += 1
+        return super(AnotherCustomDispatchMixin, self).dispatch(request)
+
+
+class ComposedTemplateView(TemplateView, CustomDispatchMixin, AnotherCustomDispatchMixin):
+    template_name = "custom_dispatch.html"
+    dispatch_call_counter = 0
+
+    def get_context_data(self, **kwargs):
+        context = super(ComposedTemplateView, self).get_context_data(**kwargs)
+        context["dispatch_call_counter"] = self.dispatch_call_counter
+        return context
+
+
+class CustomGetView(View):
+    def get(self, request):
+        return HttpResponse("custom get")
+
+
+class ComposedGetView(CustomGetView, CustomDispatchMixin):
+    dispatch_call_counter = 0
+
+    def get(self, request):
+        if self.dispatch_call_counter == 1:
+            return super(ComposedGetView, self).get(request)
+        raise Exception("Custom dispatch not called.")