Skip to content

Commit

Permalink
django: reintroduce wrapt for view method patching (#1622)
Browse files Browse the repository at this point in the history
Since GrahamDumpleton/wrapt#153 was fixed, we
can use wrapt again for the view method patching.
  • Loading branch information
Kyle-Verhoog authored and majorgreys committed Aug 24, 2020
1 parent c7ec459 commit dcf18b4
Show file tree
Hide file tree
Showing 7 changed files with 76 additions and 105 deletions.
98 changes: 0 additions & 98 deletions ddtrace/compat.py
Original file line number Diff line number Diff line change
Expand Up @@ -147,104 +147,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. """
Expand Down
9 changes: 2 additions & 7 deletions ddtrace/contrib/django/patch.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,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
Expand Down Expand Up @@ -446,17 +445,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)

Expand Down
2 changes: 2 additions & 0 deletions tests/contrib/django/django1_app/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
]
2 changes: 2 additions & 0 deletions tests/contrib/django/django_app/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,4 +46,6 @@ def authenticated_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"),
]
1 change: 1 addition & 0 deletions tests/contrib/django/templates/custom_dispatch.html
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
custom dispatch {{ dispatch_call_counter }}
33 changes: 33 additions & 0 deletions tests/contrib/django/test_django.py
Original file line number Diff line number Diff line change
Expand Up @@ -1259,3 +1259,36 @@ def test_user_name_excluded(client, test_spans):
root = test_spans.get_root_span()
assert "django.user.name" not in root.meta
assert root.meta.get("django.user.is_authenticated") == "True"


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",
]
36 changes: 36 additions & 0 deletions tests/contrib/django/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.")

0 comments on commit dcf18b4

Please sign in to comment.