From 9c886925038eff8b61463cc14afa4847fdc25030 Mon Sep 17 00:00:00 2001 From: sanket Mehta Date: Sat, 30 Jul 2022 01:15:13 +0530 Subject: [PATCH 01/19] #1043: restore metrics in django --- .../instrumentation/django/__init__.py | 21 +++++++-- .../django/middleware/otel_middleware.py | 39 ++++++++++++++-- .../instrumentation/django/package.py | 1 + .../tests/test_middleware.py | 46 +++++++++++++++++++ 4 files changed, 100 insertions(+), 7 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 4b8dec4e64..f5773cea07 100644 --- a/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/__init__.py @@ -205,6 +205,7 @@ def response_hook(span, request, response): from opentelemetry.instrumentation.django.package import _instruments from opentelemetry.instrumentation.django.version import __version__ from opentelemetry.instrumentation.instrumentor import BaseInstrumentor +from opentelemetry.metrics import get_meter from opentelemetry.trace import get_tracer DJANGO_2_0 = django_version >= (2, 0) @@ -244,19 +245,33 @@ def _instrument(self, **kwargs): return tracer_provider = kwargs.get("tracer_provider") + meter_provider = kwargs.get("meter_provider") tracer = get_tracer( __name__, __version__, tracer_provider=tracer_provider, ) - + meter = get_meter( + __name__, + __version__, + meter_provider=meter_provider + ) _DjangoMiddleware._tracer = tracer - + _DjangoMiddleware._meter = meter _DjangoMiddleware._otel_request_hook = kwargs.pop("request_hook", None) _DjangoMiddleware._otel_response_hook = kwargs.pop( "response_hook", None ) - + _DjangoMiddleware._duration_histogram = meter.create_histogram( + name="http.server.duration", + unit="ms", + description="measures the duration of the inbound http request" + ) + _DjangoMiddleware._active_request_counter = meter.create_up_down_counter( + name="http.server.active_requests", + unit="requests", + description="measures the number of concurent HTTP requests those are currently in flight" + ) # This can not be solved, but is an inherent problem of this approach: # the order of middleware entries matters, and here you have no control # on that: 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 660be75cf7..daf88c6c69 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 @@ -15,6 +15,7 @@ import types from logging import getLogger from time import time +from timeit import default_timer from typing import Callable from django import VERSION as django_version @@ -28,7 +29,12 @@ _start_internal_or_server_span, extract_attributes_from_object, ) -from opentelemetry.instrumentation.wsgi import add_response_attributes + +from opentelemetry.instrumentation.wsgi import ( + _active_requests_count_attrs, + _duration_attrs, + add_response_attributes, +) from opentelemetry.instrumentation.wsgi import ( collect_custom_request_headers_attributes as wsgi_collect_custom_request_headers_attributes, ) @@ -139,10 +145,15 @@ class _DjangoMiddleware(MiddlewareMixin): _environ_token = "opentelemetry-instrumentor-django.token" _environ_span_key = "opentelemetry-instrumentor-django.span_key" _environ_exception_key = "opentelemetry-instrumentor-django.exception_key" - + _environ_active_request_attr_key = "opentelemetry-instrumentor-django.active_request_attr_key" + _environ_duration_attr_key = "opentelemetry-instrumentor-django.duration_attr_key" + _environ_timer_key = "opentelemetry-instrumentor-django.timer_key" _traced_request_attrs = get_traced_request_attrs("DJANGO") _excluded_urls = get_excluded_urls("DJANGO") _tracer = None + _meter = None + _duration_histogram = None + _active_request_counter = None _otel_request_hook: Callable[[Span, HttpRequest], None] = None _otel_response_hook: Callable[ @@ -185,7 +196,6 @@ def process_request(self, request): # pylint:disable=W0212 request._otel_start_time = time() - request_meta = request.META if is_asgi_request: @@ -208,7 +218,22 @@ def process_request(self, request): ) attributes = collect_request_attributes(carrier) - + active_requests_count_attrs = { + attr_key: attributes[attr_key] + for attr_key in _active_requests_count_attrs + if attributes.get(attr_key) is not None + } + duration_attrs = { + attr_key: attributes[attr_key] + for attr_key in _duration_attrs + if attributes.get(attr_key) is not None + } + request_start_time = default_timer() + request.META[self._environ_active_request_attr_key] = active_requests_count_attrs + request.META[self._environ_duration_attr_key] = duration_attrs + request.META[self._environ_timer_key] = request_start_time + + self._active_request_counter.add(1, active_requests_count_attrs) if span.is_recording(): attributes = extract_attributes_from_object( request, self._traced_request_attrs, attributes @@ -291,6 +316,9 @@ def process_response(self, request, response): activation = request.META.pop(self._environ_activation_key, None) span = request.META.pop(self._environ_span_key, None) + active_requests_count_attrs = request.META.pop(self._environ_active_request_attr_key) + duration_attrs = request.META.pop(self._environ_duration_attr_key) + request_start_time = request.META.pop(self._environ_timer_key) if activation and span: if is_asgi_request: @@ -341,6 +369,9 @@ def process_response(self, request, response): else: activation.__exit__(None, None, None) + duration = max(round((default_timer() - request_start_time) * 1000), 0) + self._duration_histogram.record(duration, duration_attrs) + self._active_request_counter.add(-1, active_requests_count_attrs) if request.META.get(self._environ_token, None) is not None: detach(request.META.get(self._environ_token)) request.META.pop(self._environ_token) diff --git a/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/package.py b/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/package.py index 5b3ba08106..290061a36f 100644 --- a/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/package.py +++ b/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/package.py @@ -14,3 +14,4 @@ _instruments = ("django >= 1.10",) +_supports_metrics = True diff --git a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py index 05457de43d..e3ef0669aa 100644 --- a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py @@ -21,6 +21,7 @@ from django.http import HttpRequest, HttpResponse from django.test.client import Client from django.test.utils import setup_test_environment, teardown_test_environment +from numpy import histogram from opentelemetry import trace from opentelemetry.instrumentation.django import ( @@ -31,7 +32,15 @@ TraceResponsePropagator, set_global_response_propagator, ) +from opentelemetry.instrumentation.wsgi import ( + _active_requests_count_attrs, + _duration_attrs, +) from opentelemetry.sdk import resources +from opentelemetry.sdk.metrics.export import ( + HistogramDataPoint, + NumberDataPoint, +) from opentelemetry.sdk.trace import Span from opentelemetry.sdk.trace.id_generator import RandomIdGenerator from opentelemetry.semconv.trace import SpanAttributes @@ -406,6 +415,43 @@ def test_trace_response_headers(self): ) self.memory_exporter.clear() + def test_wsgi_metrics(self): + _expected_metric_names = [ + "http.server.active_requests", + "http.server.duration", + ] + _recommended_attrs = { + "http.server.active_requests": _active_requests_count_attrs, + "http.server.duration": _duration_attrs, + } + for req_index in range(3): + response = Client().get("/span_name/1234/") + self.assertEqual(response.status_code, 200) + metrics_list = self.memory_metrics_reader.get_metrics_data() + number_data_point_seen = False + histrogram_data_point_seen = False + + self.assertTrue(len(metrics_list.resource_metrics) != 0) + for resource_metric in metrics_list.resource_metrics: + self.assertTrue(len(resource_metric.scope_metrics) != 0) + for scope_metric in resource_metric.scope_metrics: + self.assertTrue(len(scope_metric.metrics) != 0) + for metric in scope_metric.metrics: + self.assertIn(metric.name, _expected_metric_names) + data_points = list(metric.data.data_points) + self.assertEqual(len(data_points), 1) + for point in data_points: + if isinstance(point, HistogramDataPoint): + self.assertEqual(point.count, 3) + histrogram_data_point_seen = True + if isinstance(point, NumberDataPoint): + number_data_point_seen = True + for attr in point.attributes: + self.assertIn( + attr, _recommended_attrs[metric.name] + ) + self.assertTrue(histrogram_data_point_seen and number_data_point_seen) + class TestMiddlewareWithTracerProvider(WsgiTestBase): @classmethod From 4457dc6228d371474204773b90557847cb4a9eab Mon Sep 17 00:00:00 2001 From: sanket Mehta Date: Sat, 30 Jul 2022 01:20:20 +0530 Subject: [PATCH 02/19] adding eentry in changelog.md --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 793568e433..2d71f8a6a9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased](https://github.com/open-telemetry/opentelemetry-python/compare/v1.12.0rc2-0.32b0...HEAD) +- Fixed restoring metrics in django framewok +- ([#1208](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1208)) - Adding multiple db connections support for django-instrumentation's sqlcommenter ([#1187](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1187)) - SQLCommenter semicolon bug fix From 137919d3159829055eed11187476e687c9273acd Mon Sep 17 00:00:00 2001 From: sanket Mehta Date: Sat, 30 Jul 2022 01:28:35 +0530 Subject: [PATCH 03/19] resolving generate command errors --- instrumentation/README.md | 2 +- .../instrumentation/django/__init__.py | 10 +++------- .../django/middleware/otel_middleware.py | 17 ++++++++++++----- 3 files changed, 16 insertions(+), 13 deletions(-) diff --git a/instrumentation/README.md b/instrumentation/README.md index 71d79ea258..82355ee36b 100644 --- a/instrumentation/README.md +++ b/instrumentation/README.md @@ -13,7 +13,7 @@ | [opentelemetry-instrumentation-celery](./opentelemetry-instrumentation-celery) | celery >= 4.0, < 6.0 | No | [opentelemetry-instrumentation-confluent-kafka](./opentelemetry-instrumentation-confluent-kafka) | confluent-kafka ~= 1.8.2 | No | [opentelemetry-instrumentation-dbapi](./opentelemetry-instrumentation-dbapi) | dbapi | No -| [opentelemetry-instrumentation-django](./opentelemetry-instrumentation-django) | django >= 1.10 | No +| [opentelemetry-instrumentation-django](./opentelemetry-instrumentation-django) | django >= 1.10 | Yes | [opentelemetry-instrumentation-elasticsearch](./opentelemetry-instrumentation-elasticsearch) | elasticsearch >= 2.0 | No | [opentelemetry-instrumentation-falcon](./opentelemetry-instrumentation-falcon) | falcon >= 1.4.1, < 4.0.0 | No | [opentelemetry-instrumentation-fastapi](./opentelemetry-instrumentation-fastapi) | fastapi ~= 0.58 | No 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 f5773cea07..3e5015abd6 100644 --- a/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/__init__.py @@ -251,11 +251,7 @@ def _instrument(self, **kwargs): __version__, tracer_provider=tracer_provider, ) - meter = get_meter( - __name__, - __version__, - meter_provider=meter_provider - ) + meter = get_meter(__name__, __version__, meter_provider=meter_provider) _DjangoMiddleware._tracer = tracer _DjangoMiddleware._meter = meter _DjangoMiddleware._otel_request_hook = kwargs.pop("request_hook", None) @@ -265,12 +261,12 @@ def _instrument(self, **kwargs): _DjangoMiddleware._duration_histogram = meter.create_histogram( name="http.server.duration", unit="ms", - description="measures the duration of the inbound http request" + description="measures the duration of the inbound http request", ) _DjangoMiddleware._active_request_counter = meter.create_up_down_counter( name="http.server.active_requests", unit="requests", - description="measures the number of concurent HTTP requests those are currently in flight" + description="measures the number of concurent HTTP requests those are currently in flight", ) # This can not be solved, but is an inherent problem of this approach: # the order of middleware entries matters, and here you have no control 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 daf88c6c69..46cb352ee2 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 @@ -29,7 +29,6 @@ _start_internal_or_server_span, extract_attributes_from_object, ) - from opentelemetry.instrumentation.wsgi import ( _active_requests_count_attrs, _duration_attrs, @@ -145,8 +144,12 @@ class _DjangoMiddleware(MiddlewareMixin): _environ_token = "opentelemetry-instrumentor-django.token" _environ_span_key = "opentelemetry-instrumentor-django.span_key" _environ_exception_key = "opentelemetry-instrumentor-django.exception_key" - _environ_active_request_attr_key = "opentelemetry-instrumentor-django.active_request_attr_key" - _environ_duration_attr_key = "opentelemetry-instrumentor-django.duration_attr_key" + _environ_active_request_attr_key = ( + "opentelemetry-instrumentor-django.active_request_attr_key" + ) + _environ_duration_attr_key = ( + "opentelemetry-instrumentor-django.duration_attr_key" + ) _environ_timer_key = "opentelemetry-instrumentor-django.timer_key" _traced_request_attrs = get_traced_request_attrs("DJANGO") _excluded_urls = get_excluded_urls("DJANGO") @@ -229,7 +232,9 @@ def process_request(self, request): if attributes.get(attr_key) is not None } request_start_time = default_timer() - request.META[self._environ_active_request_attr_key] = active_requests_count_attrs + request.META[ + self._environ_active_request_attr_key + ] = active_requests_count_attrs request.META[self._environ_duration_attr_key] = duration_attrs request.META[self._environ_timer_key] = request_start_time @@ -316,7 +321,9 @@ def process_response(self, request, response): activation = request.META.pop(self._environ_activation_key, None) span = request.META.pop(self._environ_span_key, None) - active_requests_count_attrs = request.META.pop(self._environ_active_request_attr_key) + active_requests_count_attrs = request.META.pop( + self._environ_active_request_attr_key + ) duration_attrs = request.META.pop(self._environ_duration_attr_key) request_start_time = request.META.pop(self._environ_timer_key) From 0caaab4d7fd00f0aef4e55d4d4c29f2251b02a4a Mon Sep 17 00:00:00 2001 From: sanket Mehta Date: Thu, 4 Aug 2022 21:50:54 +0530 Subject: [PATCH 04/19] removing numpy as it was unnecessary --- .../tests/test_middleware.py | 1 - 1 file changed, 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py index e3ef0669aa..b495744884 100644 --- a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py @@ -21,7 +21,6 @@ from django.http import HttpRequest, HttpResponse from django.test.client import Client from django.test.utils import setup_test_environment, teardown_test_environment -from numpy import histogram from opentelemetry import trace from opentelemetry.instrumentation.django import ( From bf61f1e8018a0b2d122bb315bab29ae30616264a Mon Sep 17 00:00:00 2001 From: sanket Mehta Date: Thu, 4 Aug 2022 23:10:00 +0530 Subject: [PATCH 05/19] resolving linting error --- .../instrumentation/django/middleware/otel_middleware.py | 2 ++ .../tests/test_middleware.py | 2 +- 2 files changed, 3 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 46cb352ee2..f12521d2c9 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 @@ -185,6 +185,7 @@ def _get_span_name(request): except Resolver404: return f"HTTP {request.method}" + # pylint: disable=too-many-locals def process_request(self, request): # request.META is a dictionary containing all available HTTP headers # Read more about request.META here: @@ -311,6 +312,7 @@ def process_exception(self, request, exception): request.META[self._environ_exception_key] = exception # pylint: disable=too-many-branches + # pylint: disable=too-many-locals def process_response(self, request, response): if self._excluded_urls.url_disabled(request.build_absolute_uri("?")): return response diff --git a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py index b495744884..675e10f938 100644 --- a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py @@ -423,7 +423,7 @@ def test_wsgi_metrics(self): "http.server.active_requests": _active_requests_count_attrs, "http.server.duration": _duration_attrs, } - for req_index in range(3): + for _ in range(3): response = Client().get("/span_name/1234/") self.assertEqual(response.status_code, 200) metrics_list = self.memory_metrics_reader.get_metrics_data() From dca6b048ba1d8d6eaf74549104bdaf126a3db548 Mon Sep 17 00:00:00 2001 From: sanket Mehta Date: Thu, 11 Aug 2022 15:25:23 +0530 Subject: [PATCH 06/19] added check for duration and point.values --- CHANGELOG.md | 2 +- .../tests/test_middleware.py | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 09cb496694..e612f703a1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased](https://github.com/open-telemetry/opentelemetry-python/compare/v1.12.0rc2-0.32b0...HEAD) -- Fixed restoring metrics in django framewok +- Fixed restoring metrics in django framework - ([#1208](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1208)) - Adding multiple db connections support for django-instrumentation's sqlcommenter ([#1187](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1187)) diff --git a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py index 675e10f938..c8d46243e6 100644 --- a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py @@ -15,6 +15,7 @@ # pylint: disable=E0611 from sys import modules +from timeit import default_timer from unittest.mock import Mock, patch from django import VERSION, conf @@ -423,9 +424,11 @@ def test_wsgi_metrics(self): "http.server.active_requests": _active_requests_count_attrs, "http.server.duration": _duration_attrs, } + start = default_timer() for _ in range(3): response = Client().get("/span_name/1234/") self.assertEqual(response.status_code, 200) + duration = max(round((default_timer() - start) * 1000), 0) metrics_list = self.memory_metrics_reader.get_metrics_data() number_data_point_seen = False histrogram_data_point_seen = False @@ -443,8 +446,12 @@ def test_wsgi_metrics(self): if isinstance(point, HistogramDataPoint): self.assertEqual(point.count, 3) histrogram_data_point_seen = True + self.assertAlmostEqual( + duration, point.sum, delta=100 + ) if isinstance(point, NumberDataPoint): number_data_point_seen = True + self.assertEqual(point.value, 0) for attr in point.attributes: self.assertIn( attr, _recommended_attrs[metric.name] From d0dc0a4cd01c4fff06aee3a42943a584294a2521 Mon Sep 17 00:00:00 2001 From: sanket Mehta Date: Thu, 11 Aug 2022 21:34:42 +0530 Subject: [PATCH 07/19] resolving linting issues --- .../tests/test_middleware.py | 1 + 1 file changed, 1 insertion(+) diff --git a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py index c8d46243e6..f117cd04b9 100644 --- a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py @@ -415,6 +415,7 @@ def test_trace_response_headers(self): ) self.memory_exporter.clear() + # pylint: disable=too-many-locals def test_wsgi_metrics(self): _expected_metric_names = [ "http.server.active_requests", From 396df469b8eb64dbb074a11e652982fdcb460361 Mon Sep 17 00:00:00 2001 From: sanket Mehta Date: Wed, 17 Aug 2022 19:22:31 +0530 Subject: [PATCH 08/19] Adding None Checks before adding values to metrics. Adding test for unintrument. replacing some calculations with utility functions from util package. --- .../django/middleware/otel_middleware.py | 39 ++++++++----------- .../tests/test_middleware.py | 13 +++++++ 2 files changed, 30 insertions(+), 22 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 f12521d2c9..92f9ebde74 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 @@ -29,11 +29,7 @@ _start_internal_or_server_span, extract_attributes_from_object, ) -from opentelemetry.instrumentation.wsgi import ( - _active_requests_count_attrs, - _duration_attrs, - add_response_attributes, -) +from opentelemetry.instrumentation.wsgi import add_response_attributes from opentelemetry.instrumentation.wsgi import ( collect_custom_request_headers_attributes as wsgi_collect_custom_request_headers_attributes, ) @@ -46,7 +42,12 @@ from opentelemetry.instrumentation.wsgi import wsgi_getter from opentelemetry.semconv.trace import SpanAttributes from opentelemetry.trace import Span, SpanKind, use_span -from opentelemetry.util.http import get_excluded_urls, get_traced_request_attrs +from opentelemetry.util.http import ( + _parse_active_request_count_attrs, + _parse_duration_attrs, + get_excluded_urls, + get_traced_request_attrs, +) try: from django.core.urlresolvers import ( # pylint: disable=no-name-in-module @@ -222,16 +223,8 @@ def process_request(self, request): ) attributes = collect_request_attributes(carrier) - active_requests_count_attrs = { - attr_key: attributes[attr_key] - for attr_key in _active_requests_count_attrs - if attributes.get(attr_key) is not None - } - duration_attrs = { - attr_key: attributes[attr_key] - for attr_key in _duration_attrs - if attributes.get(attr_key) is not None - } + active_requests_count_attrs = _parse_active_request_count_attrs(attributes) + duration_attrs = _parse_duration_attrs(attributes) request_start_time = default_timer() request.META[ self._environ_active_request_attr_key @@ -324,10 +317,10 @@ def process_response(self, request, response): activation = request.META.pop(self._environ_activation_key, None) span = request.META.pop(self._environ_span_key, None) active_requests_count_attrs = request.META.pop( - self._environ_active_request_attr_key + self._environ_active_request_attr_key, None ) - duration_attrs = request.META.pop(self._environ_duration_attr_key) - request_start_time = request.META.pop(self._environ_timer_key) + duration_attrs = request.META.pop(self._environ_duration_attr_key, None) + request_start_time = request.META.pop(self._environ_timer_key, None) if activation and span: if is_asgi_request: @@ -378,9 +371,11 @@ def process_response(self, request, response): else: activation.__exit__(None, None, None) - duration = max(round((default_timer() - request_start_time) * 1000), 0) - self._duration_histogram.record(duration, duration_attrs) - self._active_request_counter.add(-1, active_requests_count_attrs) + if request_start_time and duration_attrs: + duration = max(round((default_timer() - request_start_time) * 1000), 0) + self._duration_histogram.record(duration, duration_attrs) + if active_requests_count_attrs: + self._active_request_counter.add(-1, active_requests_count_attrs) if request.META.get(self._environ_token, None) is not None: detach(request.META.get(self._environ_token)) request.META.pop(self._environ_token) diff --git a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py index f117cd04b9..67e36fb423 100644 --- a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py @@ -459,6 +459,19 @@ def test_wsgi_metrics(self): ) self.assertTrue(histrogram_data_point_seen and number_data_point_seen) + def test_wsgi_metrics_unistrument(self): + Client().get("/span_name/1234/") + _django_instrumentor.uninstrument() + metrics_list = self.memory_metrics_reader.get_metrics_data() + for resource_metric in metrics_list.resource_metrics: + for scope_metric in resource_metric.scope_metrics: + for metric in scope_metric.metrics: + for point in list(metric.data.data_points): + if isinstance(point, HistogramDataPoint): + self.assertEqual(1, point.count) + if isinstance(point, NumberDataPoint): + self.assertEqual(0, point.value) + class TestMiddlewareWithTracerProvider(WsgiTestBase): @classmethod From f3523c2914afbb781fa8fd17df32b7f9eb6bae84 Mon Sep 17 00:00:00 2001 From: sanket Mehta Date: Thu, 18 Aug 2022 09:11:33 +0530 Subject: [PATCH 09/19] Resolving Lint errors --- .../django/middleware/otel_middleware.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 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 92f9ebde74..01c390c5f0 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 @@ -223,7 +223,9 @@ def process_request(self, request): ) attributes = collect_request_attributes(carrier) - active_requests_count_attrs = _parse_active_request_count_attrs(attributes) + active_requests_count_attrs = _parse_active_request_count_attrs( + attributes + ) duration_attrs = _parse_duration_attrs(attributes) request_start_time = default_timer() request.META[ @@ -319,7 +321,9 @@ def process_response(self, request, response): active_requests_count_attrs = request.META.pop( self._environ_active_request_attr_key, None ) - duration_attrs = request.META.pop(self._environ_duration_attr_key, None) + duration_attrs = request.META.pop( + self._environ_duration_attr_key, None + ) request_start_time = request.META.pop(self._environ_timer_key, None) if activation and span: @@ -372,7 +376,9 @@ def process_response(self, request, response): activation.__exit__(None, None, None) if request_start_time and duration_attrs: - duration = max(round((default_timer() - request_start_time) * 1000), 0) + duration = max( + round((default_timer() - request_start_time) * 1000), 0 + ) self._duration_histogram.record(duration, duration_attrs) if active_requests_count_attrs: self._active_request_counter.add(-1, active_requests_count_attrs) From f4ffef0761d7137ba6aebc4faf0d4650ae231a0e Mon Sep 17 00:00:00 2001 From: sanket Mehta Date: Thu, 18 Aug 2022 16:38:24 +0530 Subject: [PATCH 10/19] resolving review comments --- CHANGELOG.md | 5 +++-- .../instrumentation/django/middleware/otel_middleware.py | 5 ++--- .../tests/test_middleware.py | 1 + 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 443eb2b62e..824af834ef 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,10 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] -## [1.12.0-0.33b0](https://github.com/open-telemetry/opentelemetry-python/releases/tag/v1.12.0-0.33b0) - 2022-08-08 - - Fixed restoring metrics in django framework - ([#1208](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1208)) + +## [1.12.0-0.33b0](https://github.com/open-telemetry/opentelemetry-python/releases/tag/v1.12.0-0.33b0) - 2022-08-08 + - Adding multiple db connections support for django-instrumentation's sqlcommenter ([#1187](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1187)) - SQLCommenter semicolon bug fix 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 01c390c5f0..30ac696d19 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 @@ -375,13 +375,12 @@ def process_response(self, request, response): else: activation.__exit__(None, None, None) - if request_start_time and duration_attrs: + if request_start_time is not None: duration = max( round((default_timer() - request_start_time) * 1000), 0 ) self._duration_histogram.record(duration, duration_attrs) - if active_requests_count_attrs: - self._active_request_counter.add(-1, active_requests_count_attrs) + self._active_request_counter.add(-1, active_requests_count_attrs) if request.META.get(self._environ_token, None) is not None: detach(request.META.get(self._environ_token)) request.META.pop(self._environ_token) diff --git a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py index 67e36fb423..c3b3b06e05 100644 --- a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py @@ -462,6 +462,7 @@ def test_wsgi_metrics(self): def test_wsgi_metrics_unistrument(self): Client().get("/span_name/1234/") _django_instrumentor.uninstrument() + Client().get("/span_name/1234/") metrics_list = self.memory_metrics_reader.get_metrics_data() for resource_metric in metrics_list.resource_metrics: for scope_metric in resource_metric.scope_metrics: From 4ad66b6ca8e934af09d3d4331c9f2ac0444acdbc Mon Sep 17 00:00:00 2001 From: sanket Mehta Date: Thu, 18 Aug 2022 16:39:35 +0530 Subject: [PATCH 11/19] changing import paths for metrics attributes --- .../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 c3b3b06e05..9cf98f7a71 100644 --- a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py @@ -32,7 +32,7 @@ TraceResponsePropagator, set_global_response_propagator, ) -from opentelemetry.instrumentation.wsgi import ( +from opentelemetry.util.http import ( _active_requests_count_attrs, _duration_attrs, ) From 3f905e84ce96e8035addc2a90744784cf33661cb Mon Sep 17 00:00:00 2001 From: sanket Mehta Date: Thu, 18 Aug 2022 16:59:01 +0530 Subject: [PATCH 12/19] Resolving linting errors --- .../tests/test_middleware.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py index 9cf98f7a71..8b584bfc3b 100644 --- a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py @@ -32,10 +32,6 @@ TraceResponsePropagator, set_global_response_propagator, ) -from opentelemetry.util.http import ( - _active_requests_count_attrs, - _duration_attrs, -) from opentelemetry.sdk import resources from opentelemetry.sdk.metrics.export import ( HistogramDataPoint, @@ -54,6 +50,8 @@ from opentelemetry.util.http import ( OTEL_INSTRUMENTATION_HTTP_CAPTURE_HEADERS_SERVER_REQUEST, OTEL_INSTRUMENTATION_HTTP_CAPTURE_HEADERS_SERVER_RESPONSE, + _active_requests_count_attrs, + _duration_attrs, get_excluded_urls, get_traced_request_attrs, ) From 804e42beed147d84464420f7a2d285f2e561f2b5 Mon Sep 17 00:00:00 2001 From: sanket Mehta Date: Mon, 22 Aug 2022 14:29:06 +0530 Subject: [PATCH 13/19] added response status code to duration_attrs --- CHANGELOG.md | 2 +- .../instrumentation/django/middleware/otel_middleware.py | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 824af834ef..6186360bf4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] - Fixed restoring metrics in django framework -- ([#1208](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1208)) + ([#1208](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1208)) ## [1.12.0-0.33b0](https://github.com/open-telemetry/opentelemetry-python/releases/tag/v1.12.0-0.33b0) - 2022-08-08 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 30ac696d19..c92eee7ac2 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 @@ -324,6 +324,7 @@ def process_response(self, request, response): duration_attrs = request.META.pop( self._environ_duration_attr_key, None ) + duration_attrs[SpanAttributes.HTTP_STATUS_CODE] = response.status_code request_start_time = request.META.pop(self._environ_timer_key, None) if activation and span: From 0ded68ebb27933ed17cb37ce6db010a8fc91c3d4 Mon Sep 17 00:00:00 2001 From: sanket Mehta Date: Tue, 23 Aug 2022 16:13:11 +0530 Subject: [PATCH 14/19] checking for duration_attr for none and start the timer after span is recorded to get more accurate start_time for duration --- .../django/middleware/otel_middleware.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 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 c92eee7ac2..5c2ee137d8 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 @@ -227,13 +227,11 @@ def process_request(self, request): attributes ) duration_attrs = _parse_duration_attrs(attributes) - request_start_time = default_timer() + request.META[ self._environ_active_request_attr_key ] = active_requests_count_attrs request.META[self._environ_duration_attr_key] = duration_attrs - request.META[self._environ_timer_key] = request_start_time - self._active_request_counter.add(1, active_requests_count_attrs) if span.is_recording(): attributes = extract_attributes_from_object( @@ -268,7 +266,8 @@ def process_request(self, request): activation = use_span(span, end_on_exit=True) activation.__enter__() # pylint: disable=E1101 - + request_start_time = default_timer() + request.META[self._environ_timer_key] = request_start_time request.META[self._environ_activation_key] = activation request.META[self._environ_span_key] = span if token: @@ -324,7 +323,8 @@ def process_response(self, request, response): duration_attrs = request.META.pop( self._environ_duration_attr_key, None ) - duration_attrs[SpanAttributes.HTTP_STATUS_CODE] = response.status_code + if duration_attrs: + duration_attrs[SpanAttributes.HTTP_STATUS_CODE] = response.status_code request_start_time = request.META.pop(self._environ_timer_key, None) if activation and span: From 91c1d2a9818cd8836c4204ee12955b3ea1314a17 Mon Sep 17 00:00:00 2001 From: sanket Mehta Date: Tue, 23 Aug 2022 16:34:53 +0530 Subject: [PATCH 15/19] resolving merge conflicts in changelog.md --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c9b4c7bba7..e730864d37 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +- Fixed restoring metrics in django framework + ([#1208](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1208)) + ## [1.12.0-0.33b0](https://github.com/open-telemetry/opentelemetry-python/releases/tag/v1.12.0-0.33b0) - 2022-08-08 - Adding multiple db connections support for django-instrumentation's sqlcommenter From 73a81618bb4fceb22f7f9ec44221548e64e20210 Mon Sep 17 00:00:00 2001 From: sanket Mehta Date: Tue, 23 Aug 2022 16:50:52 +0530 Subject: [PATCH 16/19] resolving linting issues --- .../instrumentation/django/middleware/otel_middleware.py | 6 ++++-- 1 file changed, 4 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 5c2ee137d8..42cdf40812 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 @@ -227,7 +227,7 @@ def process_request(self, request): attributes ) duration_attrs = _parse_duration_attrs(attributes) - + request.META[ self._environ_active_request_attr_key ] = active_requests_count_attrs @@ -324,7 +324,9 @@ def process_response(self, request, response): self._environ_duration_attr_key, None ) if duration_attrs: - duration_attrs[SpanAttributes.HTTP_STATUS_CODE] = response.status_code + duration_attrs[ + SpanAttributes.HTTP_STATUS_CODE + ] = response.status_code request_start_time = request.META.pop(self._environ_timer_key, None) if activation and span: From 55d572685ab3a8085aed9c432a9b3d4ac62839cb Mon Sep 17 00:00:00 2001 From: sanket Mehta Date: Wed, 24 Aug 2022 10:22:28 +0530 Subject: [PATCH 17/19] restoring deleted entries --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e730864d37..16bbfb3e0c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,12 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +### Fixed + +- `opentelemetry-instrumentation-boto3sqs` Make propagation compatible with other SQS + instrumentations, add 'messaging.url' span attribute, and fix missing package dependencies. + ([#1234](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1234)) + ## [Unreleased] - Fixed restoring metrics in django framework From 7b544772077593081a9191f8d28d971ae2ae63f0 Mon Sep 17 00:00:00 2001 From: sanket Mehta Date: Wed, 24 Aug 2022 12:28:39 +0530 Subject: [PATCH 18/19] Resolved merge conflicts --- CHANGELOG.md | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 16bbfb3e0c..eb16a48ab0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,14 +5,13 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). -### Fixed -- `opentelemetry-instrumentation-boto3sqs` Make propagation compatible with other SQS +## [Unreleased] + +- Fixed `opentelemetry-instrumentation-boto3sqs` Make propagation compatible with other SQS instrumentations, add 'messaging.url' span attribute, and fix missing package dependencies. ([#1234](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1234)) -## [Unreleased] - - Fixed restoring metrics in django framework ([#1208](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1208)) From 4ed54e3adc63281e29e719bde594354f6e092208 Mon Sep 17 00:00:00 2001 From: sanket Mehta Date: Wed, 24 Aug 2022 19:31:27 +0530 Subject: [PATCH 19/19] Resolving merging conflicts in changelog.md file --- CHANGELOG.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index eb16a48ab0..daf3cc8eca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,14 +5,14 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - ## [Unreleased] -- Fixed `opentelemetry-instrumentation-boto3sqs` Make propagation compatible with other SQS - instrumentations, add 'messaging.url' span attribute, and fix missing package dependencies. +### Fixed + +- `opentelemetry-instrumentation-boto3sqs` Make propagation compatible with other SQS instrumentations, add 'messaging.url' span attribute, and fix missing package dependencies. ([#1234](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1234)) -- Fixed restoring metrics in django framework +- restoring metrics in django framework ([#1208](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1208)) ## [1.12.0-0.33b0](https://github.com/open-telemetry/opentelemetry-python/releases/tag/v1.12.0-0.33b0) - 2022-08-08