From a62d9d9375dfcd7284618a660614203972504fd8 Mon Sep 17 00:00:00 2001 From: "anshul.asawa" Date: Sun, 6 Aug 2023 18:49:26 +0530 Subject: [PATCH 1/9] 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/9] 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/9] 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/9] 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/9] 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 0c6cd3b9dd80e5a6e4a17f2ee32d31f4b2664bd2 Mon Sep 17 00:00:00 2001 From: Sai Chander Date: Thu, 17 Oct 2024 14:40:41 +0800 Subject: [PATCH 6/9] Addressed PR comments from 1903 and 2908 --- CHANGELOG.md | 2 +- .../instrumentation/django/__init__.py | 18 ++++++++++++++---- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ff49f5fa6c..eef4f681a0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -352,7 +352,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - 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) + ([#2912]https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2912) ### Added - `opentelemetry-instrumentation-kafka-python` Add instrumentation to `consume` method 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..55de539b44 100644 --- a/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/__init__.py @@ -388,15 +388,25 @@ 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") + try: + middleware_position = int(otel_position) + except (ValueError, TypeError): + _logger.debug( + "The middleware_position you provided (%s) is not an integer. Defaulting to 0.", + otel_position, + ) + 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.", + "The middleware_position you provided (%s) is greater than the number of middlewares (%s). Defaulting " + "the middleware_position to 0.", middleware_position, len(settings_middleware), ) - middleware_position = len(settings_middleware) + middleware_position = 0 + if is_sql_commentor_enabled: settings_middleware.insert( middleware_position, self._sql_commenter_middleware From 84992350a012432c87c4cd3329621e80f8a0b1fe Mon Sep 17 00:00:00 2001 From: Sai Chander Date: Thu, 17 Oct 2024 15:41:35 +0800 Subject: [PATCH 7/9] Fixed test case to put OOB at zero position --- .../tests/test_middleware.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py index e55ce1b430..1c85935892 100644 --- a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py @@ -193,7 +193,7 @@ def test_middleware_added_at_position_if_wrong_position(self): middleware_position=middleware_position ) self.assertEqual( - middleware[len(middleware) - 1], + middleware[0], "opentelemetry.instrumentation.django.middleware.otel_middleware._DjangoMiddleware", ) From f7ebc49d929d3fe89593783cbe9a4fcd28d39f4f Mon Sep 17 00:00:00 2001 From: Sai Chander Date: Tue, 22 Oct 2024 22:36:15 +0800 Subject: [PATCH 8/9] Refactored Django otel position to a separate function --- .../instrumentation/django/__init__.py | 45 +++++++++++-------- 1 file changed, 27 insertions(+), 18 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 a31f63e39e..e5851a17c2 100644 --- a/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/__init__.py @@ -285,6 +285,30 @@ def _get_django_middleware_setting() -> str: return "MIDDLEWARE" +def _get_django_otel_middleware_position( + middleware_length, default_middleware_position=0 +): + otel_position = environ.get("OTEL_PYTHON_DJANGO_MIDDLEWARE_POSITION") + try: + middleware_position = int(otel_position) + except (ValueError, TypeError): + _logger.debug( + "Invalid OTEL_PYTHON_DJANGO_MIDDLEWARE_POSITION value: (%s). Using default position: %d.", + otel_position, + default_middleware_position, + ) + middleware_position = default_middleware_position + + if middleware_position < 0 or middleware_position > middleware_length: + _logger.debug( + "Middleware position %d is out of range (0-%d). Using 0 as the position", + middleware_position, + middleware_length, + ) + middleware_position = 0 + return middleware_position + + class DjangoInstrumentor(BaseInstrumentor): """An instrumentor for Django @@ -388,24 +412,9 @@ def _instrument(self, **kwargs): is_sql_commentor_enabled = kwargs.pop("is_sql_commentor_enabled", None) - otel_position = environ.get("OTEL_PYTHON_DJANGO_MIDDLEWARE_POSITION") - try: - middleware_position = int(otel_position) - except (ValueError, TypeError): - _logger.debug( - "The middleware_position you provided (%s) is not an integer. Defaulting to 0.", - otel_position, - ) - middleware_position = kwargs.pop("middleware_position", 0) - - if len(settings_middleware) < middleware_position: - _logger.debug( - "The middleware_position you provided (%s) is greater than the number of middlewares (%s). Defaulting " - "the middleware_position to 0.", - middleware_position, - len(settings_middleware), - ) - middleware_position = 0 + middleware_position = _get_django_otel_middleware_position( + len(settings_middleware), kwargs.pop("middleware_position", 0) + ) if is_sql_commentor_enabled: settings_middleware.insert( From f5061ffb0d6b8f313436067c446cfa7b870af69f Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Thu, 24 Oct 2024 13:07:03 -0700 Subject: [PATCH 9/9] Update CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 43b385b81d..ee582205e6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -412,7 +412,7 @@ 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 +- `opentelemetry-instrumentation-django` - Add option to add Opentelemetry middleware at specific position in middleware chain ([#2912]https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2912) ### Added