From ad1ed83571578d09bcbd468052fa148d92b4668d Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Thu, 15 Oct 2020 20:24:29 -0400 Subject: [PATCH] Adding metric collection as part of instrumentations - Django (#1230) --- .../CHANGELOG.md | 2 + .../instrumentation/django/__init__.py | 13 ++++- .../instrumentation/django/middleware.py | 51 +++++++++++++++++-- .../tests/test_middleware.py | 41 ++++++++++++++- 4 files changed, 102 insertions(+), 5 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-django/CHANGELOG.md b/instrumentation/opentelemetry-instrumentation-django/CHANGELOG.md index 439b65b17..36962fbdc 100644 --- a/instrumentation/opentelemetry-instrumentation-django/CHANGELOG.md +++ b/instrumentation/opentelemetry-instrumentation-django/CHANGELOG.md @@ -11,6 +11,8 @@ Released 2020-10-13 - Changed span name extraction from request to comply semantic convention ([#992](https://github.com/open-telemetry/opentelemetry-python/pull/992)) - Added support for `OTEL_PYTHON_DJANGO_TRACED_REQUEST_ATTRS` ([#1154](https://github.com/open-telemetry/opentelemetry-python/pull/1154)) - Added capture of http.route ([#1226](https://github.com/open-telemetry/opentelemetry-python/issues/1226)) +- Add support for tracking http metrics + ([#1230](https://github.com/open-telemetry/opentelemetry-python/pull/1230)) ## Version 0.13b0 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 9ba3bbb91..26e21a1f7 100644 --- a/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/__init__.py @@ -18,12 +18,18 @@ from django.conf import settings from opentelemetry.configuration import Configuration from opentelemetry.instrumentation.django.middleware import _DjangoMiddleware +from opentelemetry.instrumentation.django.version import __version__ from opentelemetry.instrumentation.instrumentor import BaseInstrumentor +from opentelemetry.instrumentation.metric import ( + HTTPMetricRecorder, + HTTPMetricType, + MetricMixin, +) _logger = getLogger(__name__) -class DjangoInstrumentor(BaseInstrumentor): +class DjangoInstrumentor(BaseInstrumentor, MetricMixin): """An instrumentor for Django See `BaseInstrumentor` @@ -57,6 +63,11 @@ class DjangoInstrumentor(BaseInstrumentor): settings_middleware = list(settings_middleware) settings_middleware.insert(0, self._opentelemetry_middleware) + self.init_metrics( + __name__, __version__, + ) + metric_recorder = HTTPMetricRecorder(self.meter, HTTPMetricType.SERVER) + setattr(settings, "OTEL_METRIC_RECORDER", metric_recorder) setattr(settings, "MIDDLEWARE", settings_middleware) def _uninstrument(self, **kwargs): diff --git a/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware.py b/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware.py index e3cb78dbd..0503d7bbc 100644 --- a/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware.py +++ b/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware.py @@ -12,8 +12,11 @@ # See the License for the specific language governing permissions and # limitations under the License. +import time from logging import getLogger +from django.conf import settings + from opentelemetry.configuration import Configuration from opentelemetry.context import attach, detach from opentelemetry.instrumentation.django.version import __version__ @@ -41,11 +44,16 @@ except ImportError: MiddlewareMixin = object _logger = getLogger(__name__) +_attributes_by_preference = [ + ["http.scheme", "http.host", "http.target"], + ["http.scheme", "http.server_name", "net.host.port", "http.target"], + ["http.scheme", "net.host.name", "net.host.port", "http.target"], + ["http.url"], +] class _DjangoMiddleware(MiddlewareMixin): - """Django Middleware for OpenTelemetry - """ + """Django Middleware for OpenTelemetry""" _environ_activation_key = ( "opentelemetry-instrumentor-django.activation_key" @@ -88,6 +96,21 @@ class _DjangoMiddleware(MiddlewareMixin): except Resolver404: return "HTTP {}".format(request.method) + @staticmethod + def _get_metric_labels_from_attributes(attributes): + labels = {} + labels["http.method"] = attributes.get("http.method", "") + for attrs in _attributes_by_preference: + labels_from_attributes = { + attr: attributes.get(attr, None) for attr in attrs + } + if set(attrs).issubset(attributes.keys()): + labels.update(labels_from_attributes) + break + if attributes.get("http.flavor"): + labels["http.flavor"] = attributes.get("http.flavor") + return labels + def process_request(self, request): # request.META is a dictionary containing all available HTTP headers # Read more about request.META here: @@ -96,6 +119,9 @@ class _DjangoMiddleware(MiddlewareMixin): if self._excluded_urls.url_disabled(request.build_absolute_uri("?")): return + # pylint:disable=W0212 + request._otel_start_time = time.time() + environ = request.META token = attach(extract(get_header_from_environ, environ)) @@ -110,8 +136,13 @@ class _DjangoMiddleware(MiddlewareMixin): ), ) + attributes = collect_request_attributes(environ) + # pylint:disable=W0212 + request._otel_labels = self._get_metric_labels_from_attributes( + attributes + ) + if span.is_recording(): - attributes = collect_request_attributes(environ) attributes = extract_attributes_from_object( request, self._traced_request_attrs, attributes ) @@ -176,6 +207,10 @@ class _DjangoMiddleware(MiddlewareMixin): "{} {}".format(response.status_code, response.reason_phrase), response, ) + # pylint:disable=W0212 + request._otel_labels["http.status_code"] = str( + response.status_code + ) request.META.pop(self._environ_span_key) request.META[self._environ_activation_key].__exit__( @@ -187,4 +222,14 @@ class _DjangoMiddleware(MiddlewareMixin): detach(request.environ.get(self._environ_token)) request.META.pop(self._environ_token) + try: + metric_recorder = getattr(settings, "OTEL_METRIC_RECORDER", None) + if metric_recorder is not None: + # pylint:disable=W0212 + metric_recorder.record_server_duration_range( + request._otel_start_time, time.time(), request._otel_labels + ) + except Exception as ex: # pylint: disable=W0703 + _logger.warning("Error recording duration metrics: %s", ex) + return response diff --git a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py index 6e3196e3e..5c034a23a 100644 --- a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py @@ -23,6 +23,8 @@ from django.test.utils import setup_test_environment, teardown_test_environment from opentelemetry.configuration import Configuration from opentelemetry.instrumentation.django import DjangoInstrumentor +from opentelemetry.sdk.util import get_dict_as_key +from opentelemetry.test.test_base import TestBase from opentelemetry.test.wsgitestutil import WsgiTestBase from opentelemetry.trace import SpanKind from opentelemetry.trace.status import StatusCanonicalCode @@ -53,7 +55,7 @@ urlpatterns = [ _django_instrumentor = DjangoInstrumentor() -class TestMiddleware(WsgiTestBase): +class TestMiddleware(TestBase, WsgiTestBase): @classmethod def setUpClass(cls): super().setUpClass() @@ -121,6 +123,26 @@ class TestMiddleware(WsgiTestBase): self.assertEqual(span.attributes["http.status_code"], 200) self.assertEqual(span.attributes["http.status_text"], "OK") + self.assertIsNotNone(_django_instrumentor.meter) + self.assertEqual(len(_django_instrumentor.meter.metrics), 1) + recorder = _django_instrumentor.meter.metrics.pop() + match_key = get_dict_as_key( + { + "http.flavor": "1.1", + "http.method": "GET", + "http.status_code": "200", + "http.url": "http://testserver/traced/", + } + ) + for key in recorder.bound_instruments.keys(): + self.assertEqual(key, match_key) + # pylint: disable=protected-access + bound = recorder.bound_instruments.get(key) + for view_data in bound.view_datas: + self.assertEqual(view_data.labels, key) + self.assertEqual(view_data.aggregator.current.count, 1) + self.assertGreaterEqual(view_data.aggregator.current.sum, 0) + def test_not_recording(self): mock_tracer = Mock() mock_span = Mock() @@ -180,6 +202,23 @@ class TestMiddleware(WsgiTestBase): ) self.assertEqual(span.attributes["http.route"], "^error/") self.assertEqual(span.attributes["http.scheme"], "http") + self.assertIsNotNone(_django_instrumentor.meter) + self.assertEqual(len(_django_instrumentor.meter.metrics), 1) + recorder = _django_instrumentor.meter.metrics.pop() + match_key = get_dict_as_key( + { + "http.flavor": "1.1", + "http.method": "GET", + "http.url": "http://testserver/error/", + } + ) + for key in recorder.bound_instruments.keys(): + self.assertEqual(key, match_key) + # pylint: disable=protected-access + bound = recorder.bound_instruments.get(key) + for view_data in bound.view_datas: + self.assertEqual(view_data.labels, key) + self.assertEqual(view_data.aggregator.current.count, 1) @patch( "opentelemetry.instrumentation.django.middleware._DjangoMiddleware._excluded_urls",