From a62d9d9375dfcd7284618a660614203972504fd8 Mon Sep 17 00:00:00 2001 From: "anshul.asawa" Date: Sun, 6 Aug 2023 18:49:26 +0530 Subject: [PATCH 1/6] add middleware_position feature in django --- .../opentelemetry/instrumentation/django/__init__.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/__init__.py b/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/__init__.py index d545a1950b..79f452d087 100644 --- a/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/__init__.py @@ -355,10 +355,18 @@ def _instrument(self, **kwargs): is_sql_commentor_enabled = kwargs.pop("is_sql_commentor_enabled", None) + middleware_position = kwargs.pop("middleware_position", 0) + if len(settings_middleware) < middleware_position: + _logger.debug( + "The middleware_position you provided (%d) is less than the current number of middlewares (%d). \ + Since the number of middlewares is less than the total, the Otel middleware will be appended at the end of the middleware chain.", + middleware_position, len(settings_middleware) + ) + middleware_position = len(settings_middleware) if is_sql_commentor_enabled: - settings_middleware.insert(0, self._sql_commenter_middleware) + settings_middleware.insert(middleware_position, self._sql_commenter_middleware) - settings_middleware.insert(0, self._opentelemetry_middleware) + settings_middleware.insert(middleware_position, self._opentelemetry_middleware) setattr(settings, _middleware_setting, settings_middleware) From a586d927d70f72846388d4f601fb3c640271eb74 Mon Sep 17 00:00:00 2001 From: "anshul.asawa" Date: Mon, 7 Aug 2023 00:54:47 +0530 Subject: [PATCH 2/6] add tests --- .../tests/test_middleware.py | 37 +++++++++++++++++++ .../tests/test_sqlcommenter.py | 28 ++++++++++++++ .../tests/views.py | 7 ++++ 3 files changed, 72 insertions(+) diff --git a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py index d7bb1e544f..d6e924961d 100644 --- a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py @@ -67,6 +67,7 @@ route_span_name, traced, traced_template, + DummyMiddleware, ) DJANGO_2_0 = VERSION >= (2, 0) @@ -144,6 +145,42 @@ def tearDown(self): def tearDownClass(cls): super().tearDownClass() conf.settings = conf.LazySettings() + + def test_middleware_added_at_position(self): + _django_instrumentor.uninstrument() + if DJANGO_2_0: + middleware = conf.settings.MIDDLEWARE + else: + middleware = conf.settings.MIDDLEWARE_CLASSES + # adding two dummy middlewares + temprory_middelware = "django.utils.deprecation.MiddlewareMixin" + middleware.append(temprory_middelware) + middleware.append(temprory_middelware) + + middleware_position = 1 + _django_instrumentor.instrument(middleware_position=middleware_position) + self.assertEqual( + middleware[middleware_position], + "opentelemetry.instrumentation.django.middleware.otel_middleware._DjangoMiddleware" + ) + + + def test_middleware_added_at_position_if_wrong_position(self): + _django_instrumentor.uninstrument() + if DJANGO_2_0: + middleware = conf.settings.MIDDLEWARE + else: + middleware = conf.settings.MIDDLEWARE_CLASSES + # adding middleware + temprory_middelware = "django.utils.deprecation.MiddlewareMixin" + middleware.append(temprory_middelware) + middleware_position = 756 # wrong position out of bound of middleware length + _django_instrumentor.instrument(middleware_position=middleware_position) + self.assertEqual( + middleware[len(middleware) - 1], + "opentelemetry.instrumentation.django.middleware.otel_middleware._DjangoMiddleware" + ) + def test_templated_route_get(self): Client().get("/route/2020/template/") diff --git a/instrumentation/opentelemetry-instrumentation-django/tests/test_sqlcommenter.py b/instrumentation/opentelemetry-instrumentation-django/tests/test_sqlcommenter.py index f9b8ed5233..f0acbc293e 100644 --- a/instrumentation/opentelemetry-instrumentation-django/tests/test_sqlcommenter.py +++ b/instrumentation/opentelemetry-instrumentation-django/tests/test_sqlcommenter.py @@ -71,6 +71,34 @@ def test_middleware_added(self, sqlcommenter_middleware): "opentelemetry.instrumentation.django.middleware.sqlcommenter_middleware.SqlCommenter" in middleware ) + + @patch( + "opentelemetry.instrumentation.django.middleware.sqlcommenter_middleware.SqlCommenter" + ) + def test_middleware_added_at_position(self, sqlcommenter_middleware): + _django_instrumentor.uninstrument() + if DJANGO_2_0: + middleware = conf.settings.MIDDLEWARE + else: + middleware = conf.settings.MIDDLEWARE_CLASSES + + # adding two dummy middlewares + temprory_middelware = "django.utils.deprecation.MiddlewareMixin" + middleware.append(temprory_middelware) + middleware.append(temprory_middelware) + + middleware_position = 1 + _django_instrumentor.instrument(is_sql_commentor_enabled=True, middleware_position=middleware_position) + instance = sqlcommenter_middleware.return_value + instance.get_response = HttpResponse() + self.assertEqual( + middleware[middleware_position], + "opentelemetry.instrumentation.django.middleware.otel_middleware._DjangoMiddleware" + ) + self.assertEqual( + middleware[middleware_position + 1], + "opentelemetry.instrumentation.django.middleware.sqlcommenter_middleware.SqlCommenter" + ) @patch( "opentelemetry.instrumentation.django.middleware.sqlcommenter_middleware._get_opentelemetry_values" diff --git a/instrumentation/opentelemetry-instrumentation-django/tests/views.py b/instrumentation/opentelemetry-instrumentation-django/tests/views.py index 452a7c0fdd..1b98c27c1c 100644 --- a/instrumentation/opentelemetry-instrumentation-django/tests/views.py +++ b/instrumentation/opentelemetry-instrumentation-django/tests/views.py @@ -82,3 +82,10 @@ async def async_with_custom_header(request): response.headers["custom-test-header-1"] = "test-header-value-1" response.headers["custom-test-header-2"] = "test-header-value-2" return response + +class DummyMiddleware: + def __init__(self, get_response): + self.get_response = get_response + + def __call__(self, request): + return self.get_response(request) \ No newline at end of file From 67aa9ac3b41ed65c4bc8407f44e61cfc1dc16232 Mon Sep 17 00:00:00 2001 From: "anshul.asawa" Date: Mon, 7 Aug 2023 02:17:47 +0530 Subject: [PATCH 3/6] add changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 909eea507d..b48df8ccbe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -48,6 +48,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#1879](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1879)) - Add optional distro and configurator selection for auto-instrumentation ([#1823](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1823)) +- Add option to add Opentelemetry middleware at specific position in middleware chain + ([#1903]https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1903) ## Version 1.18.0/0.39b0 (2023-05-10) From af1776c9127045ac19e589e528acf0e4343bef3e Mon Sep 17 00:00:00 2001 From: "anshul.asawa" Date: Mon, 7 Aug 2023 02:26:17 +0530 Subject: [PATCH 4/6] remove unwanted code --- .../opentelemetry-instrumentation-django/tests/views.py | 7 ------- 1 file changed, 7 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-django/tests/views.py b/instrumentation/opentelemetry-instrumentation-django/tests/views.py index 1b98c27c1c..452a7c0fdd 100644 --- a/instrumentation/opentelemetry-instrumentation-django/tests/views.py +++ b/instrumentation/opentelemetry-instrumentation-django/tests/views.py @@ -82,10 +82,3 @@ async def async_with_custom_header(request): response.headers["custom-test-header-1"] = "test-header-value-1" response.headers["custom-test-header-2"] = "test-header-value-2" return response - -class DummyMiddleware: - def __init__(self, get_response): - self.get_response = get_response - - def __call__(self, request): - return self.get_response(request) \ No newline at end of file From cff7e9e3d0e907280b7d0dd046ddf8d2e8a803fb Mon Sep 17 00:00:00 2001 From: "anshul.asawa" Date: Mon, 7 Aug 2023 02:44:03 +0530 Subject: [PATCH 5/6] fix lint --- .../instrumentation/django/__init__.py | 11 +++++--- .../tests/test_middleware.py | 25 +++++++++++-------- .../tests/test_sqlcommenter.py | 15 ++++++----- 3 files changed, 31 insertions(+), 20 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/__init__.py b/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/__init__.py index 79f452d087..bfc2294081 100644 --- a/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/__init__.py @@ -360,13 +360,18 @@ def _instrument(self, **kwargs): _logger.debug( "The middleware_position you provided (%d) is less than the current number of middlewares (%d). \ Since the number of middlewares is less than the total, the Otel middleware will be appended at the end of the middleware chain.", - middleware_position, len(settings_middleware) + middleware_position, + len(settings_middleware), ) middleware_position = len(settings_middleware) if is_sql_commentor_enabled: - settings_middleware.insert(middleware_position, self._sql_commenter_middleware) + settings_middleware.insert( + middleware_position, self._sql_commenter_middleware + ) - settings_middleware.insert(middleware_position, self._opentelemetry_middleware) + settings_middleware.insert( + middleware_position, self._opentelemetry_middleware + ) setattr(settings, _middleware_setting, settings_middleware) diff --git a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py index d6e924961d..d6428a8c75 100644 --- a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py @@ -67,7 +67,6 @@ route_span_name, traced, traced_template, - DummyMiddleware, ) DJANGO_2_0 = VERSION >= (2, 0) @@ -145,7 +144,7 @@ def tearDown(self): def tearDownClass(cls): super().tearDownClass() conf.settings = conf.LazySettings() - + def test_middleware_added_at_position(self): _django_instrumentor.uninstrument() if DJANGO_2_0: @@ -156,15 +155,16 @@ def test_middleware_added_at_position(self): temprory_middelware = "django.utils.deprecation.MiddlewareMixin" middleware.append(temprory_middelware) middleware.append(temprory_middelware) - + middleware_position = 1 - _django_instrumentor.instrument(middleware_position=middleware_position) + _django_instrumentor.instrument( + middleware_position=middleware_position + ) self.assertEqual( middleware[middleware_position], - "opentelemetry.instrumentation.django.middleware.otel_middleware._DjangoMiddleware" + "opentelemetry.instrumentation.django.middleware.otel_middleware._DjangoMiddleware", ) - - + def test_middleware_added_at_position_if_wrong_position(self): _django_instrumentor.uninstrument() if DJANGO_2_0: @@ -174,14 +174,17 @@ def test_middleware_added_at_position_if_wrong_position(self): # adding middleware temprory_middelware = "django.utils.deprecation.MiddlewareMixin" middleware.append(temprory_middelware) - middleware_position = 756 # wrong position out of bound of middleware length - _django_instrumentor.instrument(middleware_position=middleware_position) + middleware_position = ( + 756 # wrong position out of bound of middleware length + ) + _django_instrumentor.instrument( + middleware_position=middleware_position + ) self.assertEqual( middleware[len(middleware) - 1], - "opentelemetry.instrumentation.django.middleware.otel_middleware._DjangoMiddleware" + "opentelemetry.instrumentation.django.middleware.otel_middleware._DjangoMiddleware", ) - def test_templated_route_get(self): Client().get("/route/2020/template/") diff --git a/instrumentation/opentelemetry-instrumentation-django/tests/test_sqlcommenter.py b/instrumentation/opentelemetry-instrumentation-django/tests/test_sqlcommenter.py index f0acbc293e..eec02d7a54 100644 --- a/instrumentation/opentelemetry-instrumentation-django/tests/test_sqlcommenter.py +++ b/instrumentation/opentelemetry-instrumentation-django/tests/test_sqlcommenter.py @@ -71,7 +71,7 @@ def test_middleware_added(self, sqlcommenter_middleware): "opentelemetry.instrumentation.django.middleware.sqlcommenter_middleware.SqlCommenter" in middleware ) - + @patch( "opentelemetry.instrumentation.django.middleware.sqlcommenter_middleware.SqlCommenter" ) @@ -81,23 +81,26 @@ def test_middleware_added_at_position(self, sqlcommenter_middleware): middleware = conf.settings.MIDDLEWARE else: middleware = conf.settings.MIDDLEWARE_CLASSES - + # adding two dummy middlewares temprory_middelware = "django.utils.deprecation.MiddlewareMixin" middleware.append(temprory_middelware) middleware.append(temprory_middelware) - + middleware_position = 1 - _django_instrumentor.instrument(is_sql_commentor_enabled=True, middleware_position=middleware_position) + _django_instrumentor.instrument( + is_sql_commentor_enabled=True, + middleware_position=middleware_position, + ) instance = sqlcommenter_middleware.return_value instance.get_response = HttpResponse() self.assertEqual( middleware[middleware_position], - "opentelemetry.instrumentation.django.middleware.otel_middleware._DjangoMiddleware" + "opentelemetry.instrumentation.django.middleware.otel_middleware._DjangoMiddleware", ) self.assertEqual( middleware[middleware_position + 1], - "opentelemetry.instrumentation.django.middleware.sqlcommenter_middleware.SqlCommenter" + "opentelemetry.instrumentation.django.middleware.sqlcommenter_middleware.SqlCommenter", ) @patch( From ca9e744ce7a96d7ac34a926239c8c0f0cff88511 Mon Sep 17 00:00:00 2001 From: Puneet Singhwaiya <@appdynamics.com> Date: Wed, 28 Aug 2024 14:24:08 +0530 Subject: [PATCH 6/6] Adding Environment Variable for Auto Instrumentation --- .../src/opentelemetry/instrumentation/django/__init__.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/__init__.py b/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/__init__.py index 5e5f5fb5eb..e49f5eacfd 100644 --- a/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/__init__.py @@ -388,11 +388,13 @@ def _instrument(self, **kwargs): is_sql_commentor_enabled = kwargs.pop("is_sql_commentor_enabled", None) - middleware_position = kwargs.pop("middleware_position", 0) + otel_position = environ.get("OTEL_PYTHON_DJANGO_MIDDLEWARE_POSITION") + middleware_position = int(otel_position) if otel_position is not None else kwargs.pop("middleware_position", 0) + if len(settings_middleware) < middleware_position: _logger.debug( - "The middleware_position you provided (%d) is less than the current number of middlewares (%d). \ - Since the number of middlewares is less than the total, the Otel middleware will be appended at the end of the middleware chain.", + "The middleware_position you provided (%d) is greater than the current number of middlewares (%d). \ + Since the number of middlewares is less than the total number of middlewares, the Otel middleware will be appended at the end of the middleware chain.", middleware_position, len(settings_middleware), )