From 8254ece5e0bc082f00d56ca652c5ec2ee00b93d1 Mon Sep 17 00:00:00 2001 From: Marc Dougherty Date: Mon, 8 May 2023 10:51:34 -0700 Subject: [PATCH 1/8] fix(django): avoid empty span name on empty path when using `path('',...)`, previous code produces spans with an empty string name. This change corrects this fallback behavior and adds a test. --- .../django/middleware/otel_middleware.py | 2 +- .../tests/test_middleware.py | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware/otel_middleware.py b/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware/otel_middleware.py index 1baa05eca9..9ae02a3108 100644 --- a/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware/otel_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware/otel_middleware.py @@ -172,7 +172,7 @@ def _get_span_name(request): else: match = resolve(request.path) - if hasattr(match, "route"): + if hasattr(match, "route") and match.route: return match.route # Instead of using `view_name`, better to use `_func_name` as some applications can use similar diff --git a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py index e235c8840e..693e267486 100644 --- a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py @@ -75,6 +75,7 @@ if DJANGO_2_0: from django.urls import re_path + from django.urls import path else: from django.conf.urls import url as re_path @@ -87,6 +88,7 @@ re_path(r"^excluded_noarg/", excluded_noarg), re_path(r"^excluded_noarg2/", excluded_noarg2), re_path(r"^span_name/([0-9]{4})/$", route_span_name), + path("", traced, name="empty"), ] _django_instrumentor = DjangoInstrumentor() @@ -207,6 +209,18 @@ def test_not_recording(self): self.assertFalse(mock_span.set_attribute.called) self.assertFalse(mock_span.set_status.called) + def test_empty_path(self): + Client().get("/") + + spans = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans), 1) + + span = spans[0] + + self.assertEqual( + span.name, "empty" + ) + def test_traced_post(self): Client().post("/traced/") From 18bd6bf17c99dc7d0040b97bcba5c4b3d2d789f8 Mon Sep 17 00:00:00 2001 From: Marc Dougherty Date: Mon, 8 May 2023 16:53:23 -0700 Subject: [PATCH 2/8] black format --- .../tests/test_middleware.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py index 693e267486..89c1a18944 100644 --- a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py @@ -217,9 +217,7 @@ def test_empty_path(self): span = spans[0] - self.assertEqual( - span.name, "empty" - ) + self.assertEqual(span.name, "empty") def test_traced_post(self): Client().post("/traced/") From 67cdd0d547c299a2e36cee320f65a2d8d01bb615 Mon Sep 17 00:00:00 2001 From: Marc Dougherty Date: Tue, 6 Jun 2023 09:19:13 -0700 Subject: [PATCH 3/8] fix tests for django1 by making a path() equivalent --- .../tests/test_middleware.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py index 89c1a18944..2cc1e56f3b 100644 --- a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py @@ -78,6 +78,8 @@ from django.urls import path else: from django.conf.urls import url as re_path + def path(p, *args, **kwargs): + return re_path(r"^%s$" % p, *args, **kwargs) urlpatterns = [ re_path(r"^traced/", traced), From c28c67238abe137c4f7328777849a67932d9686e Mon Sep 17 00:00:00 2001 From: Marc Dougherty Date: Tue, 6 Jun 2023 09:19:58 -0700 Subject: [PATCH 4/8] changelog entry --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ee92bdab7a..016cc115ee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased +### Fixed + +- `opentelemetry-instrumentation-django` Fix empty span name when using + `path("", ...)` ([#1788](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1788) + ## Version 1.18.0/0.39b0 (2023-05-10) - `opentelemetry-instrumentation-system-metrics` Add `process.` prefix to `runtime.memory`, `runtime.cpu.time`, and `runtime.gc_count`. Change `runtime.memory` from count to UpDownCounter. ([#1735](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1735)) From 0ea524b82c23c379a5ba501d7bf001dc5e83556e Mon Sep 17 00:00:00 2001 From: Marc Dougherty Date: Wed, 21 Jun 2023 09:41:28 -0700 Subject: [PATCH 5/8] re-add a name fallback This was removed in #1759. --- .../instrumentation/django/middleware/otel_middleware.py | 3 +++ .../tests/test_middleware.py | 6 ++++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware/otel_middleware.py b/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware/otel_middleware.py index bebf6fb42b..18c97b75fa 100644 --- a/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware/otel_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware/otel_middleware.py @@ -174,6 +174,9 @@ def _get_span_name(request): if hasattr(match, "route") and match.route: return f"{request.method} {match.route}" + + if hasattr(match, "url_name") and match.url_name: + return f"{request.method} {match.url_name}" return request.method diff --git a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py index 6129d0176e..6f219d1cba 100644 --- a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py @@ -78,8 +78,10 @@ from django.urls import path else: from django.conf.urls import url as re_path + def path(p, *args, **kwargs): - return re_path(r"^%s$" % p, *args, **kwargs) + return re_path(r"^%s$" % p, *args, **kwargs) + urlpatterns = [ re_path(r"^traced/", traced), @@ -217,7 +219,7 @@ def test_empty_path(self): span = spans[0] - self.assertEqual(span.name, "empty") + self.assertEqual(span.name, "GET empty") def test_traced_post(self): Client().post("/traced/") From 308dab0a40fc072605daaf65b622847151177e74 Mon Sep 17 00:00:00 2001 From: Marc Dougherty Date: Thu, 22 Jun 2023 10:37:05 -0700 Subject: [PATCH 6/8] oops trailing whitespace --- .../instrumentation/django/middleware/otel_middleware.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware/otel_middleware.py b/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware/otel_middleware.py index 18c97b75fa..491e78cab5 100644 --- a/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware/otel_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware/otel_middleware.py @@ -174,7 +174,7 @@ def _get_span_name(request): if hasattr(match, "route") and match.route: return f"{request.method} {match.route}" - + if hasattr(match, "url_name") and match.url_name: return f"{request.method} {match.url_name}" From 17f2cceae0f4c003c3f06aecadd538a4999fd0a0 Mon Sep 17 00:00:00 2001 From: Marc Dougherty Date: Mon, 26 Jun 2023 11:08:36 -0700 Subject: [PATCH 7/8] lint: isort --- .../tests/test_middleware.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py index 6f219d1cba..1271cfaae7 100644 --- a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py @@ -74,8 +74,7 @@ DJANGO_3_0 = VERSION >= (3, 0) if DJANGO_2_0: - from django.urls import re_path - from django.urls import path + from django.urls import path, re_path else: from django.conf.urls import url as re_path From 95fdc1d55d135cd1a79fae46406d4494e29368a2 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Wed, 12 Jul 2023 19:17:41 +0200 Subject: [PATCH 8/8] Fix lint --- .../tests/test_middleware.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py index 1271cfaae7..d7bb1e544f 100644 --- a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py @@ -78,8 +78,8 @@ else: from django.conf.urls import url as re_path - def path(p, *args, **kwargs): - return re_path(r"^%s$" % p, *args, **kwargs) + def path(path_argument, *args, **kwargs): + return re_path(rf"^{path_argument}$", *args, **kwargs) urlpatterns = [