fix(django): patch staticmethods in views (#1246)

Co-authored-by: Brett Langdon <[email protected]>

Author: majorgreys

ddtrace/compat.py

@@ -112,6 +112,100 @@ def iscoroutinefunction(fn):
     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 and # noqa: E721,E261,W504
+                        class_dict.__name__ == "__dict__" and # noqa: E261,W504
+                        class_dict.__objclass__ is entry):
+                    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 # noqa: E261,E721,W504
+                type(dict_attr) is types.MemberDescriptorType):
+                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 # noqa: W504,E261,E721
+                _check_class(type(klass_result), '__set__') is not _sentinel):
+                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):

ddtrace/contrib/django/patch.py

@@ -13,6 +13,7 @@
 
 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
@@ -433,7 +434,6 @@ def traced_template_render(django, pin, wrapped, instance, args, kwargs):
 
 def instrument_view(django, view):
     """Helper to wrap Django views."""
-
     # All views should be callable, double check before doing anything
     if not callable(view) or isinstance(view, wrapt.ObjectProxy):
         return view
@@ -443,13 +443,18 @@ def instrument_view(django, view):
     lifecycle_methods = ("setup", "dispatch", "http_method_not_allowed")
     for name in list(http_method_names) + list(lifecycle_methods):
         try:
-            func = getattr(view, name, None)
+            # View methods can be staticmethods
+            func = getattr_static(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)
-            wrap(view, name, traced_func(django, name=op_name, resource=resource))
+
+            # 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)
         except Exception:
             log.debug("Failed to instrument Django view %r function %s", view, name, exc_info=True)
 

tests/contrib/django/django1_app/urls.py

@@ -11,6 +11,7 @@
     url(r"^cached-template/$", views.TemplateCachedUserList.as_view(), name="cached-template-list"),
     url(r"^cached-users/$", cache_page(60)(views.UserList.as_view()), name="cached-users-list"),
     url(r"^fail-view/$", views.ForbiddenView.as_view(), name="forbidden-view"),
+    url(r"^static-method-view/$", views.StaticMethodView.as_view(), name="static-method-view"),
     url(r"^fn-view/$", views.function_view, name="fn-view"),
     url(r"^feed-view/$", views.FeedView(), name="feed-view"),
     url(r"^partial-view/$", views.partial_view, name="partial-view"),

tests/contrib/django/django_app/urls.py

@@ -21,6 +21,7 @@ def path_view(request):
     url(r"^cached-template/$", views.TemplateCachedUserList.as_view(), name="cached-template-list"),
     url(r"^cached-users/$", cache_page(60)(views.UserList.as_view()), name="cached-users-list"),
     url(r"^fail-view/$", views.ForbiddenView.as_view(), name="forbidden-view"),
+    url(r"^static-method-view/$", views.StaticMethodView.as_view(), name="static-method-view"),
     url(r"^fn-view/$", views.function_view, name="fn-view"),
     url(r"^feed-view/$", views.FeedView(), name="feed-view"),
     url(r"^partial-view/$", views.partial_view, name="partial-view"),

tests/contrib/django/test_django.py

@@ -416,6 +416,19 @@ def test_middleware_trace_errors(client, test_spans):
         assert span.resource == "GET tests.contrib.django.views.ForbiddenView"
 
 
+def test_middleware_trace_staticmethod(client, test_spans):
+    # ensures that the internals are properly traced
+    assert client.get("/static-method-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/static-method-view/"
+    if django.VERSION >= (2, 2, 0):
+        assert span.resource == "GET ^static-method-view/$"
+    else:
+        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

tests/contrib/django/views.py

@@ -41,6 +41,12 @@ def get(self, request, *args, **kwargs):
         return HttpResponse(status=403)
 
 
+class StaticMethodView(View):
+    @staticmethod
+    def get(request):
+        return HttpResponse("")
+
+
 def function_view(request):
     return HttpResponse(status=200)