diff --git a/CHANGELOG.md b/CHANGELOG.md index acd7011af..daf3cc8ec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `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)) +- 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 diff --git a/instrumentation/README.md b/instrumentation/README.md index 6cd49a5df..deeb3693f 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 4b8dec4e6..3e5015abd 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 @@ from opentelemetry.instrumentation.django.middleware.otel_middleware import ( 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,29 @@ class DjangoInstrumentor(BaseInstrumentor): 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 660be75cf..42cdf4081 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 @@ -41,7 +42,12 @@ from opentelemetry.instrumentation.wsgi import ( 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 @@ -139,10 +145,19 @@ 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[ @@ -171,6 +186,7 @@ class _DjangoMiddleware(MiddlewareMixin): 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: @@ -185,7 +201,6 @@ class _DjangoMiddleware(MiddlewareMixin): # pylint:disable=W0212 request._otel_start_time = time() - request_meta = request.META if is_asgi_request: @@ -208,7 +223,16 @@ class _DjangoMiddleware(MiddlewareMixin): ) attributes = collect_request_attributes(carrier) + active_requests_count_attrs = _parse_active_request_count_attrs( + attributes + ) + duration_attrs = _parse_duration_attrs(attributes) + request.META[ + self._environ_active_request_attr_key + ] = active_requests_count_attrs + request.META[self._environ_duration_attr_key] = duration_attrs + 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 @@ -242,7 +266,8 @@ class _DjangoMiddleware(MiddlewareMixin): 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: @@ -281,6 +306,7 @@ class _DjangoMiddleware(MiddlewareMixin): 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 @@ -291,6 +317,17 @@ class _DjangoMiddleware(MiddlewareMixin): 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, None + ) + duration_attrs = request.META.pop( + self._environ_duration_attr_key, None + ) + 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: if is_asgi_request: @@ -341,6 +378,12 @@ class _DjangoMiddleware(MiddlewareMixin): else: activation.__exit__(None, None, None) + if request_start_time is not 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 5b3ba0810..290061a36 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 05457de43..8b584bfc3 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 @@ -32,6 +33,10 @@ from opentelemetry.instrumentation.propagators import ( set_global_response_propagator, ) 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 @@ -45,6 +50,8 @@ from opentelemetry.trace import ( 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, ) @@ -406,6 +413,64 @@ class TestMiddleware(WsgiTestBase): ) self.memory_exporter.clear() + # pylint: disable=too-many-locals + 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, + } + 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 + + 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 + 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] + ) + 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() + 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: + 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