From 4e9748688b9e460aa1825a237fabeb3ffe540367 Mon Sep 17 00:00:00 2001 From: Bergutov Ruslan Date: Mon, 17 Aug 2020 13:45:38 +0500 Subject: [PATCH 1/7] span name --- .../instrumentation/django/middleware.py | 33 +++++++++++++++++-- .../tests/test_middleware.py | 25 +++++++++++--- .../tests/views.py | 4 +++ 3 files changed, 55 insertions(+), 7 deletions(-) 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 2c155b1be14..6629c7e9a6c 100644 --- a/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware.py +++ b/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware.py @@ -14,6 +14,11 @@ from logging import getLogger +try: + from django.urls import resolve, Resolver404 +except ImportError: + from django.core.urlresolvers import resolve + from opentelemetry.configuration import Configuration from opentelemetry.context import attach, detach from opentelemetry.instrumentation.django.version import __version__ @@ -50,8 +55,30 @@ class _DjangoMiddleware(MiddlewareMixin): else: _excluded_urls = ExcludeList(_excluded_urls) - def process_view( - self, request, view_func, view_args, view_kwargs + @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 + else: + # Instead of using `view_name`, better to use `_func_path` as some applications can use similar + # view names in different modules + if hasattr(match, "_func_name"): + return match._func_name + else: + # Fallback for safety as `_func_name` private field + return match.view_name + + except Resolver404: + return "HTTP {}".format(request.method) + + def process_request( + self, request ): # pylint: disable=unused-argument # request.META is a dictionary containing all available HTTP headers # Read more about request.META here: @@ -73,7 +100,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 3250ac0c1c3..2d1680de174 100644 --- a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py @@ -28,7 +28,7 @@ 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, traced, route_span_name urlpatterns = [ url(r"^traced/", traced), @@ -36,6 +36,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 +66,7 @@ def test_traced_get(self): span = spans[0] - self.assertEqual(span.name, "traced") + self.assertEqual(span.name, "^traced/") self.assertEqual(span.kind, SpanKind.SERVER) self.assertEqual(span.status.canonical_code, StatusCanonicalCode.OK) self.assertEqual(span.attributes["http.method"], "GET") @@ -84,7 +85,7 @@ def test_traced_post(self): span = spans[0] - self.assertEqual(span.name, "traced") + self.assertEqual(span.name, "^traced/") self.assertEqual(span.kind, SpanKind.SERVER) self.assertEqual(span.status.canonical_code, StatusCanonicalCode.OK) self.assertEqual(span.attributes["http.method"], "POST") @@ -104,7 +105,7 @@ def test_error(self): span = spans[0] - self.assertEqual(span.name, "error") + self.assertEqual(span.name, "^error/") self.assertEqual(span.kind, SpanKind.SERVER) self.assertEqual( span.status.canonical_code, StatusCanonicalCode.UNKNOWN @@ -136,3 +137,19 @@ 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})/$") + + 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 b5a29304042..5d8f9db31c3 100644 --- a/instrumentation/opentelemetry-instrumentation-django/tests/views.py +++ b/instrumentation/opentelemetry-instrumentation-django/tests/views.py @@ -19,3 +19,7 @@ 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() From 252fa3ff5f0a2b229ce2bb38b3889a0daae7fbf3 Mon Sep 17 00:00:00 2001 From: Bergutov Ruslan Date: Mon, 17 Aug 2020 14:00:53 +0500 Subject: [PATCH 2/7] lints --- .../instrumentation/django/middleware.py | 37 +++++++++---------- .../tests/test_middleware.py | 9 ++++- .../tests/views.py | 4 +- 3 files changed, 28 insertions(+), 22 deletions(-) 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 6629c7e9a6c..a13bccd31ba 100644 --- a/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware.py +++ b/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware.py @@ -14,11 +14,6 @@ from logging import getLogger -try: - from django.urls import resolve, Resolver404 -except ImportError: - from django.core.urlresolvers import resolve - from opentelemetry.configuration import Configuration from opentelemetry.context import attach, detach from opentelemetry.instrumentation.django.version import __version__ @@ -31,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: @@ -65,29 +68,23 @@ def _get_span_name(request): if hasattr(match, "route"): return match.route - else: - # Instead of using `view_name`, better to use `_func_path` as some applications can use similar - # view names in different modules - if hasattr(match, "_func_name"): - return match._func_name - else: - # Fallback for safety as `_func_name` private field - return match.view_name + + # Instead of using `view_name`, better to use `_func_path` 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 - ): # pylint: disable=unused-argument + 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 diff --git a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py index 2d1680de174..8666014ad09 100644 --- a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py @@ -28,7 +28,14 @@ from opentelemetry.util import ExcludeList # pylint: disable=import-error -from .views import error, excluded, excluded_noarg, excluded_noarg2, traced, route_span_name +from .views import ( + error, + excluded, + excluded_noarg, + excluded_noarg2, + route_span_name, + traced, +) urlpatterns = [ url(r"^traced/", traced), diff --git a/instrumentation/opentelemetry-instrumentation-django/tests/views.py b/instrumentation/opentelemetry-instrumentation-django/tests/views.py index 5d8f9db31c3..e2868410111 100644 --- a/instrumentation/opentelemetry-instrumentation-django/tests/views.py +++ b/instrumentation/opentelemetry-instrumentation-django/tests/views.py @@ -21,5 +21,7 @@ def excluded_noarg2(request): # pylint: disable=unused-argument return HttpResponse() -def route_span_name(request, *args, **kwargs): # pylint: disable=unused-argument +def route_span_name( + request, *args, **kwargs +): # pylint: disable=unused-argument return HttpResponse() From 2b99266e25eae5a037529f481702fc170dda8c12 Mon Sep 17 00:00:00 2001 From: Bergutov Ruslan Date: Mon, 17 Aug 2020 14:19:09 +0500 Subject: [PATCH 3/7] changelog --- .../opentelemetry-instrumentation-django/CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/instrumentation/opentelemetry-instrumentation-django/CHANGELOG.md b/instrumentation/opentelemetry-instrumentation-django/CHANGELOG.md index 3e4e42e55bd..a5b64eb6d7c 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 + ## Version 0.12b0 Released 2020-08-14 From 0801a2883bdb394136b97462d4137a74a98aab9b Mon Sep 17 00:00:00 2001 From: Bergutov Ruslan Date: Mon, 17 Aug 2020 14:54:09 +0500 Subject: [PATCH 4/7] tests for django<2.2 --- .../tests/test_middleware.py | 22 +++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py index 8666014ad09..ee82c5d7d9f 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 @@ -37,6 +38,8 @@ traced, ) +DJANGO_2_2 = VERSION >= (2, 2) + urlpatterns = [ url(r"^traced/", traced), url(r"^error/", error), @@ -73,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") @@ -92,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") @@ -112,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 @@ -151,7 +160,12 @@ def test_span_name(self): self.assertEqual(len(span_list), 1) span = span_list[0] - self.assertEqual(span.name, "^span_name/([0-9]{4})/$") + 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/") From 2e8262c258179fe8e79005bef1752f29456850df Mon Sep 17 00:00:00 2001 From: Bergutov Ruslan Date: Tue, 18 Aug 2020 08:04:40 +0500 Subject: [PATCH 5/7] PR link in changelog, comment fix --- .../opentelemetry-instrumentation-django/CHANGELOG.md | 2 +- .../src/opentelemetry/instrumentation/django/middleware.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-django/CHANGELOG.md b/instrumentation/opentelemetry-instrumentation-django/CHANGELOG.md index a5b64eb6d7c..481b31db98d 100644 --- a/instrumentation/opentelemetry-instrumentation-django/CHANGELOG.md +++ b/instrumentation/opentelemetry-instrumentation-django/CHANGELOG.md @@ -2,7 +2,7 @@ ## Unreleased -- Changed span name extraction from request to comply semantic convention +- Changed span name extraction from request to comply semantic convention ([#992] (https://github.com/open-telemetry/opentelemetry-python/pull/992)) ## Version 0.12b0 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 a13bccd31ba..59f7e6e6229 100644 --- a/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware.py +++ b/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware.py @@ -69,7 +69,7 @@ def _get_span_name(request): if hasattr(match, "route"): return match.route - # Instead of using `view_name`, better to use `_func_path` as some applications can use similar + # 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 From 2a069ca5c9a6a5b4cf5c5b3a21a92ed58f481ff5 Mon Sep 17 00:00:00 2001 From: Bergutov Ruslan Date: Tue, 18 Aug 2020 08:08:23 +0500 Subject: [PATCH 6/7] fix --- .../opentelemetry-instrumentation-django/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-django/CHANGELOG.md b/instrumentation/opentelemetry-instrumentation-django/CHANGELOG.md index 481b31db98d..5a1e4c343ee 100644 --- a/instrumentation/opentelemetry-instrumentation-django/CHANGELOG.md +++ b/instrumentation/opentelemetry-instrumentation-django/CHANGELOG.md @@ -2,7 +2,7 @@ ## Unreleased -- Changed span name extraction from request to comply semantic convention ([#992] (https://github.com/open-telemetry/opentelemetry-python/pull/992)) +- Changed span name extraction from request to comply semantic convention ([#992](https://github.com/open-telemetry/opentelemetry-python/pull/992)) ## Version 0.12b0 From fbffab33362e03d7d55ba015f6020ea818af6c33 Mon Sep 17 00:00:00 2001 From: Bergutov Ruslan Date: Tue, 18 Aug 2020 23:33:42 +0500 Subject: [PATCH 7/7] fix --- .../opentelemetry-instrumentation-django/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-django/CHANGELOG.md b/instrumentation/opentelemetry-instrumentation-django/CHANGELOG.md index 5a1e4c343ee..6402fc79c32 100644 --- a/instrumentation/opentelemetry-instrumentation-django/CHANGELOG.md +++ b/instrumentation/opentelemetry-instrumentation-django/CHANGELOG.md @@ -2,7 +2,7 @@ ## Unreleased -- Changed span name extraction from request to comply semantic convention ([#992](https://github.com/open-telemetry/opentelemetry-python/pull/992)) +- Changed span name extraction from request to comply semantic convention ([#992](https://github.com/open-telemetry/opentelemetry-python/pull/992)) ## Version 0.12b0