Adding metric collection as part of instrumentations - Django

docs/examples/django/manage.py

@@ -21,13 +21,13 @@
 
 
 def main():
+    os.environ.setdefault(
+        "DJANGO_SETTINGS_MODULE", "instrumentation_example.settings"
+    )
 
     # This call is what makes the Django application be instrumented
     DjangoInstrumentor().instrument()
 
-    os.environ.setdefault(
-        "DJANGO_SETTINGS_MODULE", "instrumentation_example.settings"
-    )
     try:
         from django.core.management import execute_from_command_line
     except ImportError as exc:

instrumentation/opentelemetry-instrumentation-django/CHANGELOG.md

@@ -4,6 +4,8 @@
 
 - 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))
+- Add support for tracking http metrics
+  ([#1230](https://github.com/open-telemetry/opentelemetry-python/pull/1230))
 
 ## Version 0.13b0
 

instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/__init__.py

@@ -18,12 +18,18 @@
 
 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`
@@ -34,16 +40,6 @@ class DjangoInstrumentor(BaseInstrumentor):
     )
 
     def _instrument(self, **kwargs):
-
-        # FIXME this is probably a pattern that will show up in the rest of the
-        # ext. Find a better way of implementing this.
-        # FIXME Probably the evaluation of strings into boolean values can be
-        # built inside the Configuration class itself with the magic method
-        # __bool__
-
-        if not Configuration().DJANGO_INSTRUMENT:
-            return
-
         # 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:
@@ -57,6 +53,11 @@ def _instrument(self, **kwargs):
             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):

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__
@@ -44,8 +47,7 @@
 
 
 class _DjangoMiddleware(MiddlewareMixin):
-    """Django Middleware for OpenTelemetry
-    """
+    """Django Middleware for OpenTelemetry"""
 
     _environ_activation_key = (
         "opentelemetry-instrumentor-django.activation_key"
@@ -88,6 +90,32 @@ def _get_span_name(request):
         except Resolver404:
             return "HTTP {}".format(request.method)
 
+    @staticmethod
+    def _get_metric_labels_from_attributes(attributes):
+        labels = {}
+        labels["http.method"] = attributes.get("http.method", "")
+        if attributes.get("http.url"):
+            labels["http.url"] = attributes.get("http.url")
+        elif attributes.get("http.scheme"):
+            labels["http.scheme"] = attributes.get("http.scheme")
+            if attributes.get("http.target"):
+                labels["http.target"] = attributes.get("http.target")
+                if attributes.get("http.host"):
+                    labels["http.host"] = attributes.get("http.host")
+                elif attributes.get("net.host.port"):
+                    labels["net.host.port"] = attributes.get("net.host.port")
+                    if attributes.get("http.server_name"):
+                        labels["http.server_name"] = attributes.get(
+                            "http.server_name"
+                        )
+                    elif attributes.get("http.host.name"):
+                        labels["http.host.name"] = attributes.get(
+                            "http.host.name"
+                        )
+        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 +124,8 @@ def process_request(self, request):
         if self._excluded_urls.url_disabled(request.build_absolute_uri("?")):
             return
 
+        request.start_time = time.time()
+
         environ = request.META
 
         token = attach(extract(get_header_from_environ, environ))
@@ -110,8 +140,10 @@ def process_request(self, request):
             ),
         )
 
+        attributes = collect_request_attributes(environ)
+        request.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
             )
@@ -156,6 +188,7 @@ def process_response(self, request, response):
                 "{} {}".format(response.status_code, response.reason_phrase),
                 response,
             )
+            request.labels["http.status_code"] = str(response.status_code)
             request.META.pop(self._environ_span_key)
 
             request.META[self._environ_activation_key].__exit__(
@@ -167,4 +200,13 @@ def process_response(self, request, response):
             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:
+                metric_recorder.record_server_duration_range(
+                    request.start_time, time.time(), request.labels
+                )
+        except Exception as ex:  # pylint: disable=W0703
+            _logger.warning("Error recording duration metrics: %s", ex)
+
         return response

instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py

@@ -23,6 +23,8 @@
 
 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
@@ -51,7 +53,7 @@
 _django_instrumentor = DjangoInstrumentor()
 
 
-class TestMiddleware(WsgiTestBase):
+class TestMiddleware(TestBase, WsgiTestBase):
     @classmethod
     def setUpClass(cls):
         super().setUpClass()
@@ -89,6 +91,26 @@ def test_traced_get(self):
         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()
@@ -146,6 +168,23 @@ def test_error(self):
             span.attributes["http.url"], "http://testserver/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",

instrumentation/opentelemetry-instrumentation-requests/CHANGELOG.md

@@ -10,7 +10,7 @@ Released 2020-09-17
   ([#1040](https://github.com/open-telemetry/opentelemetry-python/pull/1040))
 - Drop support for Python 3.4
   ([#1099](https://github.com/open-telemetry/opentelemetry-python/pull/1099))
-- Add support for http metrics
+- Add support for tracking http metrics
   ([#1116](https://github.com/open-telemetry/opentelemetry-python/pull/1116))
 
 ## Version 0.12b0

instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py

@@ -45,6 +45,7 @@
 from opentelemetry.instrumentation.instrumentor import BaseInstrumentor
 from opentelemetry.instrumentation.metric import (
     HTTPMetricRecorder,
+    HTTPMetricType,
     MetricMixin,
 )
 from opentelemetry.instrumentation.requests.version import __version__
@@ -135,7 +136,7 @@ def _instrumented_requests_call(
             __name__, __version__, tracer_provider
         ).start_as_current_span(span_name, kind=SpanKind.CLIENT) as span:
             exception = None
-            with recorder.record_duration(labels):
+            with recorder.record_client_duration(labels):
                 if span.is_recording():
                     span.set_attribute("component", "http")
                     span.set_attribute("http.method", method)
@@ -176,7 +177,6 @@ def _instrumented_requests_call(
                             )
                         )
                     labels["http.status_code"] = str(result.status_code)
-                    labels["http.status_text"] = result.reason
                     if result.raw and result.raw.version:
                         labels["http.flavor"] = (
                             str(result.raw.version)[:1]
@@ -253,7 +253,9 @@ def _instrument(self, **kwargs):
             __name__, __version__,
         )
         # pylint: disable=W0201
-        self.metric_recorder = HTTPMetricRecorder(self.meter, SpanKind.CLIENT)
+        self.metric_recorder = HTTPMetricRecorder(
+            self.meter, HTTPMetricType.CLIENT
+        )
 
     def _uninstrument(self, **kwargs):
         _uninstrument()

instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py

@@ -97,7 +97,6 @@ def test_basic(self):
                 "http.flavor": "1.1",
                 "http.method": "GET",
                 "http.status_code": "200",
-                "http.status_text": "OK",
                 "http.url": "http://httpbin.org/status/200",
             }
         )
@@ -108,7 +107,7 @@ def test_basic(self):
             for view_data in bound.view_datas:
                 self.assertEqual(view_data.labels, key)
                 self.assertEqual(view_data.aggregator.current.count, 1)
-                self.assertGreater(view_data.aggregator.current.sum, 0)
+                self.assertGreaterEqual(view_data.aggregator.current.sum, 0)
 
     def test_not_foundbasic(self):
         url_404 = "http://httpbin.org/status/404"
@@ -318,7 +317,6 @@ def test_requests_exception_with_response(self, *_, **__):
             {
                 "http.method": "GET",
                 "http.status_code": "500",
-                "http.status_text": "Internal Server Error",
                 "http.url": "http://httpbin.org/status/200",
             }
         )

opentelemetry-instrumentation/src/opentelemetry/instrumentation/metric.py

@@ -28,7 +28,7 @@
 class HTTPMetricType(enum.Enum):
     CLIENT = 0
     SERVER = 1
-    # TODO: Add both
+    BOTH = 2
 
 
 class MetricMixin:
@@ -57,29 +57,54 @@ def __init__(
     ):
         super().__init__(meter)
         self._http_type = http_type
+        self._client_duration = None
+        self._server_duration = None
         if self._meter:
-            self._duration = self._meter.create_metric(
-                name="{}.{}.duration".format(
-                    "http", self._http_type.name.lower()
-                ),
-                description="measures the duration of the {} HTTP request".format(
-                    "inbound"
-                    if self._http_type is HTTPMetricType.SERVER
-                    else "outbound"
-                ),
-                unit="ms",
-                value_type=float,
-                metric_type=ValueRecorder,
-            )
+            if http_type in (HTTPMetricType.CLIENT, HTTPMetricType.BOTH):
+                self._client_duration = self._meter.create_metric(
+                    name="{}.{}.duration".format("http", "client"),
+                    description="measures the duration of the outbound HTTP request",
+                    unit="ms",
+                    value_type=float,
+                    metric_type=ValueRecorder,
+                )
+            if http_type is not HTTPMetricType.CLIENT:
+                self._server_duration = self._meter.create_metric(
+                    name="{}.{}.duration".format("http", "server"),
+                    description="measures the duration of the inbound HTTP request",
+                    unit="ms",
+                    value_type=float,
+                    metric_type=ValueRecorder,
+                )
 
     # Conventions for recording duration can be found at:
     # https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/metrics/semantic_conventions/http-metrics.md
     @contextmanager
-    def record_duration(self, labels: Dict[str, str]):
+    def record_client_duration(self, labels: Dict[str, str]):
         start_time = time()
         try:
             yield start_time
         finally:
-            if self._meter:
-                elapsed_time = (time() - start_time) * 1000
-                self._duration.record(elapsed_time, labels)
+            self.record_client_duration_range(start_time, time(), labels)
+
+    def record_client_duration_range(
+        self, start_time, end_time, labels: Dict[str, str]
+    ):
+        if self._client_duration:
+            elapsed_time = (end_time - start_time) * 1000
+            self._client_duration.record(elapsed_time, labels)
+
+    @contextmanager
+    def record_server_duration(self, labels: Dict[str, str]):
+        start_time = time()
+        try:
+            yield start_time
+        finally:
+            self.record_server_duration_range(start_time, time(), labels)
+
+    def record_server_duration_range(
+        self, start_time, end_time, labels: Dict[str, str]
+    ):
+        if self._server_duration:
+            elapsed_time = (end_time - start_time) * 1000
+            self._server_duration.record(elapsed_time, labels)