Metrics instrumentation flask (#1186)

TheAnshul756 · web-flow · commit ebe6d1804bcc · 2022-08-05T07:47:14.000Z
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -19,6 +19,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - `opentelemetry-instrumentation-redis` add support to instrument RedisCluster clients
   ([#1177](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1177))
 - `opentelemetry-instrumentation-sqlalchemy` Added span for the connection phase ([#1133](https://github.com/open-telemetry/opentelemetry-python-contrib/issues/1133))
+- Add metric instumentation for flask
+  ([#1186](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1186))
 
 ## [1.12.0rc2-0.32b0](https://github.com/open-telemetry/opentelemetry-python/releases/tag/v1.12.0rc2-0.32b0) - 2022-07-01
 
diff --git a/instrumentation/README.md b/instrumentation/README.md
@@ -17,7 +17,7 @@
 | [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
-| [opentelemetry-instrumentation-flask](./opentelemetry-instrumentation-flask) | flask >= 1.0, < 3.0 | No
+| [opentelemetry-instrumentation-flask](./opentelemetry-instrumentation-flask) | flask >= 1.0, < 3.0 | Yes
 | [opentelemetry-instrumentation-grpc](./opentelemetry-instrumentation-grpc) | grpcio ~= 1.27 | No
 | [opentelemetry-instrumentation-httpx](./opentelemetry-instrumentation-httpx) | httpx >= 0.18.0 | No
 | [opentelemetry-instrumentation-jinja2](./opentelemetry-instrumentation-jinja2) | jinja2 >= 2.7, < 4.0 | No
diff --git a/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py b/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py
@@ -141,6 +141,7 @@ def response_hook(span: Span, status: str, response_headers: List):
 """
 
 from logging import getLogger
+from timeit import default_timer
 from typing import Collection
 
 import flask
@@ -154,6 +155,7 @@ def response_hook(span: Span, status: str, response_headers: List):
     get_global_response_propagator,
 )
 from opentelemetry.instrumentation.utils import _start_internal_or_server_span
+from opentelemetry.metrics import get_meter
 from opentelemetry.semconv.trace import SpanAttributes
 from opentelemetry.util._time import _time_ns
 from opentelemetry.util.http import get_excluded_urls, parse_excluded_urls
@@ -165,7 +167,6 @@ def response_hook(span: Span, status: str, response_headers: List):
 _ENVIRON_ACTIVATION_KEY = "opentelemetry-flask.activation_key"
 _ENVIRON_TOKEN = "opentelemetry-flask.token"
 
-
 _excluded_urls_from_env = get_excluded_urls("FLASK")
 
 
@@ -178,13 +179,26 @@ def get_default_span_name():
     return span_name
 
 
-def _rewrapped_app(wsgi_app, response_hook=None, excluded_urls=None):
+def _rewrapped_app(
+    wsgi_app,
+    active_requests_counter,
+    duration_histogram,
+    response_hook=None,
+    excluded_urls=None,
+):
     def _wrapped_app(wrapped_app_environ, start_response):
         # We want to measure the time for route matching, etc.
         # In theory, we could start the span here and use
         # update_name later but that API is "highly discouraged" so
         # we better avoid it.
         wrapped_app_environ[_ENVIRON_STARTTIME_KEY] = _time_ns()
+        start = default_timer()
+        attributes = otel_wsgi.collect_request_attributes(wrapped_app_environ)
+        active_requests_count_attrs = (
+            otel_wsgi._parse_active_request_count_attrs(attributes)
+        )
+        duration_attrs = otel_wsgi._parse_duration_attrs(attributes)
+        active_requests_counter.add(1, active_requests_count_attrs)
 
         def _start_response(status, response_headers, *args, **kwargs):
             if flask.request and (
@@ -204,6 +218,11 @@ def _start_response(status, response_headers, *args, **kwargs):
                     otel_wsgi.add_response_attributes(
                         span, status, response_headers
                     )
+                    status_code = otel_wsgi._parse_status_code(status)
+                    if status_code is not None:
+                        duration_attrs[
+                            SpanAttributes.HTTP_STATUS_CODE
+                        ] = status_code
                     if (
                         span.is_recording()
                         and span.kind == trace.SpanKind.SERVER
@@ -223,13 +242,19 @@ def _start_response(status, response_headers, *args, **kwargs):
                     response_hook(span, status, response_headers)
             return start_response(status, response_headers, *args, **kwargs)
 
-        return wsgi_app(wrapped_app_environ, _start_response)
+        result = wsgi_app(wrapped_app_environ, _start_response)
+        duration = max(round((default_timer() - start) * 1000), 0)
+        duration_histogram.record(duration, duration_attrs)
+        active_requests_counter.add(-1, active_requests_count_attrs)
+        return result
 
     return _wrapped_app
 
 
 def _wrapped_before_request(
-    request_hook=None, tracer=None, excluded_urls=None
+    request_hook=None,
+    tracer=None,
+    excluded_urls=None,
 ):
     def _before_request():
         if excluded_urls and excluded_urls.url_disabled(flask.request.url):
@@ -278,7 +303,9 @@ def _before_request():
     return _before_request
 
 
-def _wrapped_teardown_request(excluded_urls=None):
+def _wrapped_teardown_request(
+    excluded_urls=None,
+):
     def _teardown_request(exc):
         # pylint: disable=E1101
         if excluded_urls and excluded_urls.url_disabled(flask.request.url):
@@ -290,7 +317,6 @@ def _teardown_request(exc):
             # a way that doesn't run `before_request`, like when it is created
             # with `app.test_request_context`.
             return
-
         if exc is None:
             activation.__exit__(None, None, None)
         else:
@@ -310,15 +336,32 @@ class _InstrumentedFlask(flask.Flask):
     _tracer_provider = None
     _request_hook = None
     _response_hook = None
+    _meter_provider = None
 
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
 
         self._original_wsgi_app = self.wsgi_app
         self._is_instrumented_by_opentelemetry = True
 
+        meter = get_meter(
+            __name__, __version__, _InstrumentedFlask._meter_provider
+        )
+        duration_histogram = meter.create_histogram(
+            name="http.server.duration",
+            unit="ms",
+            description="measures the duration of the inbound HTTP request",
+        )
+        active_requests_counter = meter.create_up_down_counter(
+            name="http.server.active_requests",
+            unit="requests",
+            description="measures the number of concurrent HTTP requests that are currently in-flight",
+        )
+
         self.wsgi_app = _rewrapped_app(
             self.wsgi_app,
+            active_requests_counter,
+            duration_histogram,
             _InstrumentedFlask._response_hook,
             excluded_urls=_InstrumentedFlask._excluded_urls,
         )
@@ -367,6 +410,8 @@ def _instrument(self, **kwargs):
             if excluded_urls is None
             else parse_excluded_urls(excluded_urls)
         )
+        meter_provider = kwargs.get("meter_provider")
+        _InstrumentedFlask._meter_provider = meter_provider
         flask.Flask = _InstrumentedFlask
 
     def _uninstrument(self, **kwargs):
@@ -379,6 +424,7 @@ def instrument_app(
         response_hook=None,
         tracer_provider=None,
         excluded_urls=None,
+        meter_provider=None,
     ):
         if not hasattr(app, "_is_instrumented_by_opentelemetry"):
             app._is_instrumented_by_opentelemetry = False
@@ -389,9 +435,25 @@ def instrument_app(
                 if excluded_urls is not None
                 else _excluded_urls_from_env
             )
+            meter = get_meter(__name__, __version__, meter_provider)
+            duration_histogram = meter.create_histogram(
+                name="http.server.duration",
+                unit="ms",
+                description="measures the duration of the inbound HTTP request",
+            )
+            active_requests_counter = meter.create_up_down_counter(
+                name="http.server.active_requests",
+                unit="requests",
+                description="measures the number of concurrent HTTP requests that are currently in-flight",
+            )
+
             app._original_wsgi_app = app.wsgi_app
             app.wsgi_app = _rewrapped_app(
-                app.wsgi_app, response_hook, excluded_urls=excluded_urls
+                app.wsgi_app,
+                active_requests_counter,
+                duration_histogram,
+                response_hook,
+                excluded_urls=excluded_urls,
             )
 
             tracer = trace.get_tracer(__name__, __version__, tracer_provider)
diff --git a/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/package.py b/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/package.py
@@ -14,3 +14,5 @@
 
 
 _instruments = ("flask >= 1.0, < 3.0",)
+
+_supports_metrics = True
diff --git a/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py b/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py
@@ -12,6 +12,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+from timeit import default_timer
 from unittest.mock import Mock, patch
 
 from flask import Flask, request
@@ -23,7 +24,15 @@
     get_global_response_propagator,
     set_global_response_propagator,
 )
-from opentelemetry.instrumentation.wsgi import OpenTelemetryMiddleware
+from opentelemetry.instrumentation.wsgi import (
+    OpenTelemetryMiddleware,
+    _active_requests_count_attrs,
+    _duration_attrs,
+)
+from opentelemetry.sdk.metrics.export import (
+    HistogramDataPoint,
+    NumberDataPoint,
+)
 from opentelemetry.sdk.resources import Resource
 from opentelemetry.semconv.trace import SpanAttributes
 from opentelemetry.test.wsgitestutil import WsgiTestBase
@@ -49,6 +58,16 @@ def expected_attributes(override_attributes):
     return default_attributes
 
 
+_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,
+}
+
+
 class TestProgrammatic(InstrumentationTest, WsgiTestBase):
     def setUp(self):
         super().setUp()
@@ -250,6 +269,106 @@ def test_exclude_lists_from_explicit(self):
         span_list = self.memory_exporter.get_finished_spans()
         self.assertEqual(len(span_list), 1)
 
+    def test_flask_metrics(self):
+        start = default_timer()
+        self.client.get("/hello/123")
+        self.client.get("/hello/321")
+        self.client.get("/hello/756")
+        duration = max(round((default_timer() - start) * 1000), 0)
+        metrics_list = self.memory_metrics_reader.get_metrics_data()
+        number_data_point_seen = False
+        histogram_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)
+                            self.assertAlmostEqual(
+                                duration, point.sum, delta=10
+                            )
+                            histogram_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(number_data_point_seen and histogram_data_point_seen)
+
+    def test_flask_metric_values(self):
+        start = default_timer()
+        self.client.post("/hello/756")
+        self.client.post("/hello/756")
+        self.client.post("/hello/756")
+        duration = max(round((default_timer() - start) * 1000), 0)
+        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(point.count, 3)
+                            self.assertAlmostEqual(
+                                duration, point.sum, delta=10
+                            )
+                        if isinstance(point, NumberDataPoint):
+                            self.assertEqual(point.value, 0)
+
+    def test_basic_metric_success(self):
+        self.client.get("/hello/756")
+        expected_duration_attributes = {
+            "http.method": "GET",
+            "http.host": "localhost",
+            "http.scheme": "http",
+            "http.flavor": "1.1",
+            "http.server_name": "localhost",
+            "net.host.port": 80,
+            "http.status_code": 200,
+        }
+        expected_requests_count_attributes = {
+            "http.method": "GET",
+            "http.host": "localhost",
+            "http.scheme": "http",
+            "http.flavor": "1.1",
+            "http.server_name": "localhost",
+        }
+        metrics_list = self.memory_metrics_reader.get_metrics_data()
+        for resource_metric in metrics_list.resource_metrics:
+            for scope_metrics in resource_metric.scope_metrics:
+                for metric in scope_metrics.metrics:
+                    for point in list(metric.data.data_points):
+                        if isinstance(point, HistogramDataPoint):
+                            self.assertDictEqual(
+                                expected_duration_attributes,
+                                dict(point.attributes),
+                            )
+                            self.assertEqual(point.count, 1)
+                        elif isinstance(point, NumberDataPoint):
+                            self.assertDictEqual(
+                                expected_requests_count_attributes,
+                                dict(point.attributes),
+                            )
+                            self.assertEqual(point.value, 0)
+
+    def test_metric_uninstrument(self):
+        self.client.delete("/hello/756")
+        FlaskInstrumentor().uninstrument_app(self.app)
+        self.client.delete("/hello/756")
+        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(point.count, 1)
+
 
 class TestProgrammaticHooks(InstrumentationTest, WsgiTestBase):
     def setUp(self):
diff --git a/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py b/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py
@@ -334,6 +334,22 @@ def _parse_status_code(resp_status):
         return None
 
 
+def _parse_active_request_count_attrs(req_attrs):
+    active_requests_count_attrs = {}
+    for attr_key in _active_requests_count_attrs:
+        if req_attrs.get(attr_key) is not None:
+            active_requests_count_attrs[attr_key] = req_attrs[attr_key]
+    return active_requests_count_attrs
+
+
+def _parse_duration_attrs(req_attrs):
+    duration_attrs = {}
+    for attr_key in _duration_attrs:
+        if req_attrs.get(attr_key) is not None:
+            duration_attrs[attr_key] = req_attrs[attr_key]
+    return duration_attrs
+
+
 def add_response_attributes(
     span, start_response_status, response_headers
 ):  # pylint: disable=unused-argument
@@ -436,15 +452,10 @@ def __call__(self, environ, start_response):
             start_response: The WSGI start_response callable.
         """
         req_attrs = collect_request_attributes(environ)
-        active_requests_count_attrs = {}
-        for attr_key in _active_requests_count_attrs:
-            if req_attrs.get(attr_key) is not None:
-                active_requests_count_attrs[attr_key] = req_attrs[attr_key]
-
-        duration_attrs = {}
-        for attr_key in _duration_attrs:
-            if req_attrs.get(attr_key) is not None:
-                duration_attrs[attr_key] = req_attrs[attr_key]
+        active_requests_count_attrs = _parse_active_request_count_attrs(
+            req_attrs
+        )
+        duration_attrs = _parse_duration_attrs(req_attrs)
 
         span, token = _start_internal_or_server_span(
             tracer=self.tracer,

Original file line number	Diff line number	Diff line change
`@@ -14,3 +14,5 @@`
`14`	`14`
`15`	`15`
`16`	`16`	`_instruments = ("flask >= 1.0, < 3.0",)`
	`17`	`+`
	`18`	`+_supports_metrics = True`