diff --git a/instrumentation/opentelemetry-instrumentation-django/CHANGELOG.md b/instrumentation/opentelemetry-instrumentation-django/CHANGELOG.md index ea9b3f56a7..d3de446730 100644 --- a/instrumentation/opentelemetry-instrumentation-django/CHANGELOG.md +++ b/instrumentation/opentelemetry-instrumentation-django/CHANGELOG.md @@ -2,6 +2,8 @@ ## Unreleased +- Changed span name extraction from request to comply semantic convention ([#992](https://github.com/open-telemetry/opentelemetry-python/pull/992)) + ## Version 0.13b0 Released 2020-09-17 diff --git a/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware.py b/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware.py index 2c155b1be1..59f7e6e622 100644 --- a/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware.py +++ b/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware.py @@ -26,6 +26,14 @@ from opentelemetry.trace import SpanKind, get_tracer from opentelemetry.util import ExcludeList +try: + from django.core.urlresolvers import ( # pylint: disable=no-name-in-module + resolve, + Resolver404, + ) +except ImportError: + from django.urls import resolve, Resolver404 + try: from django.utils.deprecation import MiddlewareMixin except ImportError: @@ -50,17 +58,33 @@ class _DjangoMiddleware(MiddlewareMixin): else: _excluded_urls = ExcludeList(_excluded_urls) - def process_view( - self, request, view_func, view_args, view_kwargs - ): # pylint: disable=unused-argument + @staticmethod + def _get_span_name(request): + try: + if getattr(request, "resolver_match"): + match = request.resolver_match + else: + match = resolve(request.get_full_path()) + + if hasattr(match, "route"): + return match.route + + # Instead of using `view_name`, better to use `_func_name` as some applications can use similar + # view names in different modules + if hasattr(match, "_func_name"): + return match._func_name # pylint: disable=protected-access + + # Fallback for safety as `_func_name` private field + return match.view_name + + except Resolver404: + return "HTTP {}".format(request.method) + + def process_request(self, request): # request.META is a dictionary containing all available HTTP headers # Read more about request.META here: # https://docs.djangoproject.com/en/3.0/ref/request-response/#django.http.HttpRequest.META - # environ = { - # key.lower().replace('_', '-').replace("http-", "", 1): value - # for key, value in request.META.items() - # } if self._excluded_urls.url_disabled(request.build_absolute_uri("?")): return @@ -73,7 +97,7 @@ def process_view( attributes = collect_request_attributes(environ) span = tracer.start_span( - view_func.__name__, + self._get_span_name(request), kind=SpanKind.SERVER, attributes=attributes, start_time=environ.get( diff --git a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py index 3250ac0c1c..ee82c5d7d9 100644 --- a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py @@ -15,6 +15,7 @@ from sys import modules from unittest.mock import patch +from django import VERSION from django.conf import settings from django.conf.urls import url from django.test import Client @@ -28,7 +29,16 @@ from opentelemetry.util import ExcludeList # pylint: disable=import-error -from .views import error, excluded, excluded_noarg, excluded_noarg2, traced +from .views import ( + error, + excluded, + excluded_noarg, + excluded_noarg2, + route_span_name, + traced, +) + +DJANGO_2_2 = VERSION >= (2, 2) urlpatterns = [ url(r"^traced/", traced), @@ -36,6 +46,7 @@ url(r"^excluded_arg/", excluded), url(r"^excluded_noarg/", excluded_noarg), url(r"^excluded_noarg2/", excluded_noarg2), + url(r"^span_name/([0-9]{4})/$", route_span_name), ] _django_instrumentor = DjangoInstrumentor() @@ -65,7 +76,9 @@ def test_traced_get(self): span = spans[0] - self.assertEqual(span.name, "traced") + self.assertEqual( + span.name, "^traced/" if DJANGO_2_2 else "tests.views.traced" + ) self.assertEqual(span.kind, SpanKind.SERVER) self.assertEqual(span.status.canonical_code, StatusCanonicalCode.OK) self.assertEqual(span.attributes["http.method"], "GET") @@ -84,7 +97,9 @@ def test_traced_post(self): span = spans[0] - self.assertEqual(span.name, "traced") + self.assertEqual( + span.name, "^traced/" if DJANGO_2_2 else "tests.views.traced" + ) self.assertEqual(span.kind, SpanKind.SERVER) self.assertEqual(span.status.canonical_code, StatusCanonicalCode.OK) self.assertEqual(span.attributes["http.method"], "POST") @@ -104,7 +119,9 @@ def test_error(self): span = spans[0] - self.assertEqual(span.name, "error") + self.assertEqual( + span.name, "^error/" if DJANGO_2_2 else "tests.views.error" + ) self.assertEqual(span.kind, SpanKind.SERVER) self.assertEqual( span.status.canonical_code, StatusCanonicalCode.UNKNOWN @@ -136,3 +153,24 @@ def test_exclude_lists(self): client.get("/excluded_noarg2/") span_list = self.memory_exporter.get_finished_spans() self.assertEqual(len(span_list), 1) + + def test_span_name(self): + Client().get("/span_name/1234/") + span_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(span_list), 1) + + span = span_list[0] + self.assertEqual( + span.name, + "^span_name/([0-9]{4})/$" + if DJANGO_2_2 + else "tests.views.route_span_name", + ) + + def test_span_name_404(self): + Client().get("/span_name/1234567890/") + span_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(span_list), 1) + + span = span_list[0] + self.assertEqual(span.name, "HTTP GET") diff --git a/instrumentation/opentelemetry-instrumentation-django/tests/views.py b/instrumentation/opentelemetry-instrumentation-django/tests/views.py index b5a2930404..e286841011 100644 --- a/instrumentation/opentelemetry-instrumentation-django/tests/views.py +++ b/instrumentation/opentelemetry-instrumentation-django/tests/views.py @@ -19,3 +19,9 @@ def excluded_noarg(request): # pylint: disable=unused-argument def excluded_noarg2(request): # pylint: disable=unused-argument return HttpResponse() + + +def route_span_name( + request, *args, **kwargs +): # pylint: disable=unused-argument + return HttpResponse()