From e91b396ad262163e1589d0776d643086fc669515 Mon Sep 17 00:00:00 2001 From: "anshul.asawa" Date: Tue, 5 Jul 2022 14:17:35 +0530 Subject: [PATCH 01/18] add metric instrumentation --- .../instrumentation/flask/__init__.py | 107 ++++++++++++++++-- 1 file changed, 100 insertions(+), 7 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py b/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py index cca8743556..839b3a35c0 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py @@ -142,6 +142,7 @@ def response_hook(span: Span, status: str, response_headers: List): from logging import getLogger from typing import Collection +from timeit import default_timer import flask @@ -149,13 +150,13 @@ def response_hook(span: Span, status: str, response_headers: List): from opentelemetry import context, trace from opentelemetry.instrumentation.flask.package import _instruments from opentelemetry.instrumentation.flask.version import __version__ +from opentelemetry.metrics import get_meter from opentelemetry.instrumentation.instrumentor import BaseInstrumentor from opentelemetry.instrumentation.propagators import ( get_global_response_propagator, ) from opentelemetry.instrumentation.utils import _start_internal_or_server_span from opentelemetry.semconv.trace import SpanAttributes -from opentelemetry.util._time import _time_ns from opentelemetry.util.http import get_excluded_urls, parse_excluded_urls _logger = getLogger(__name__) @@ -164,7 +165,27 @@ def response_hook(span: Span, status: str, response_headers: List): _ENVIRON_SPAN_KEY = "opentelemetry-flask.span_key" _ENVIRON_ACTIVATION_KEY = "opentelemetry-flask.activation_key" _ENVIRON_TOKEN = "opentelemetry-flask.token" - +_ENVIRON_STATUS_CODE_KEY = "opentelemetry-flask.status_code" + +# List of recommended attributes +_duration_attrs = [ + SpanAttributes.HTTP_METHOD, + SpanAttributes.HTTP_HOST, + SpanAttributes.HTTP_SCHEME, + SpanAttributes.HTTP_STATUS_CODE, + SpanAttributes.HTTP_FLAVOR, + SpanAttributes.HTTP_SERVER_NAME, + SpanAttributes.NET_HOST_NAME, + SpanAttributes.NET_HOST_PORT, +] + +_active_requests_count_attrs = [ + SpanAttributes.HTTP_METHOD, + SpanAttributes.HTTP_HOST, + SpanAttributes.HTTP_SCHEME, + SpanAttributes.HTTP_FLAVOR, + SpanAttributes.HTTP_SERVER_NAME, +] _excluded_urls_from_env = get_excluded_urls("FLASK") @@ -177,6 +198,12 @@ def get_default_span_name(): span_name = otel_wsgi.get_default_span_name(flask.request.environ) return span_name +def _parse_status_code(resp_status): + status_code, _ = resp_status.split(" ", 1) + try: + return int(status_code) + except ValueError: + return None def _rewrapped_app(wsgi_app, response_hook=None, excluded_urls=None): def _wrapped_app(wrapped_app_environ, start_response): @@ -184,8 +211,7 @@ def _wrapped_app(wrapped_app_environ, start_response): # 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() - + wrapped_app_environ[_ENVIRON_STARTTIME_KEY] = default_timer() def _start_response(status, response_headers, *args, **kwargs): if flask.request and ( excluded_urls is None @@ -204,6 +230,9 @@ def _start_response(status, response_headers, *args, **kwargs): otel_wsgi.add_response_attributes( span, status, response_headers ) + status_code = _parse_status_code(status) + if status_code is not None: + flask.request.environ[_ENVIRON_STATUS_CODE_KEY] = status_code if ( span.is_recording() and span.kind == trace.SpanKind.SERVER @@ -229,7 +258,10 @@ def _start_response(status, response_headers, *args, **kwargs): def _wrapped_before_request( - request_hook=None, tracer=None, excluded_urls=None + active_requests_counter, + request_hook=None, + tracer=None, + excluded_urls=None, ): def _before_request(): if excluded_urls and excluded_urls.url_disabled(flask.request.url): @@ -252,6 +284,11 @@ def _before_request(): attributes = otel_wsgi.collect_request_attributes( flask_request_environ ) + active_requests_count_attrs = {} + for attr_key in _active_requests_count_attrs: + if attributes.get(attr_key) is not None: + active_requests_count_attrs[attr_key] = attributes[attr_key] + active_requests_counter.add(1, active_requests_count_attrs) if flask.request.url_rule: # For 404 that result from no route found, etc, we # don't have a url_rule. @@ -278,7 +315,11 @@ def _before_request(): return _before_request -def _wrapped_teardown_request(excluded_urls=None): +def _wrapped_teardown_request( + active_requests_counter, + duration_histogram, + excluded_urls=None, +): def _teardown_request(exc): # pylint: disable=E1101 if excluded_urls and excluded_urls.url_disabled(flask.request.url): @@ -290,7 +331,26 @@ def _teardown_request(exc): # a way that doesn't run `before_request`, like when it is created # with `app.test_request_context`. return - + start = flask.request.environ.get(_ENVIRON_STARTTIME_KEY) + duration = max(round((default_timer() - start) * 1000), 0) + attributes = otel_wsgi.collect_request_attributes( + flask.request.environ + ) + active_requests_count_attrs = {} + for attr_key in _active_requests_count_attrs: + if attributes.get(attr_key) is not None: + active_requests_count_attrs[attr_key] = attributes[attr_key] + + duration_attrs = {} + for attr_key in _duration_attrs: + if attributes.get(attr_key) is not None: + duration_attrs[attr_key] = attributes[attr_key] + status_code = flask.request.environ.get(_ENVIRON_STATUS_CODE_KEY, None) + if status_code: + duration_attrs[SpanAttributes.HTTP_STATUS_CODE] = status_code + + duration_histogram.record(duration, duration_attrs) + active_requests_counter.add(-1, active_requests_count_attrs) if exc is None: activation.__exit__(None, None, None) else: @@ -310,6 +370,7 @@ 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) @@ -327,16 +388,31 @@ def __init__(self, *args, **kwargs): __name__, __version__, _InstrumentedFlask._tracer_provider ) + 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", + ) + _before_request = _wrapped_before_request( _InstrumentedFlask._request_hook, tracer, excluded_urls=_InstrumentedFlask._excluded_urls, + active_requests_counter=active_requests_counter, ) self._before_request = _before_request self.before_request(_before_request) _teardown_request = _wrapped_teardown_request( excluded_urls=_InstrumentedFlask._excluded_urls, + active_requests_counter=active_requests_counter, + duration_histogram=duration_histogram, ) self.teardown_request(_teardown_request) @@ -367,6 +443,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 +457,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 @@ -395,8 +474,20 @@ def instrument_app( ) tracer = trace.get_tracer(__name__, __version__, tracer_provider) + 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", + ) _before_request = _wrapped_before_request( + active_requests_counter, request_hook, tracer, excluded_urls=excluded_urls, @@ -405,6 +496,8 @@ def instrument_app( app.before_request(_before_request) _teardown_request = _wrapped_teardown_request( + active_requests_counter, + duration_histogram, excluded_urls=excluded_urls, ) app._teardown_request = _teardown_request From 139a1cccfe171ed00b3ae7e5272b38693d6f04b7 Mon Sep 17 00:00:00 2001 From: "anshul.asawa" Date: Thu, 7 Jul 2022 14:51:44 +0530 Subject: [PATCH 02/18] add metrics tests --- .../instrumentation/flask/__init__.py | 8 ++-- .../tests/test_programmatic.py | 47 ++++++++++++++++++- 2 files changed, 50 insertions(+), 5 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py b/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py index 839b3a35c0..a31f554efe 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py @@ -401,18 +401,18 @@ def __init__(self, *args, **kwargs): ) _before_request = _wrapped_before_request( + active_requests_counter, _InstrumentedFlask._request_hook, tracer, excluded_urls=_InstrumentedFlask._excluded_urls, - active_requests_counter=active_requests_counter, ) self._before_request = _before_request self.before_request(_before_request) _teardown_request = _wrapped_teardown_request( - excluded_urls=_InstrumentedFlask._excluded_urls, - active_requests_counter=active_requests_counter, - duration_histogram=duration_histogram, + active_requests_counter, + duration_histogram, + excluded_urls=_InstrumentedFlask._excluded_urls ) self.teardown_request(_teardown_request) diff --git a/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py b/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py index 2bcb097c7b..8224a756fa 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py +++ b/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py @@ -17,12 +17,20 @@ from flask import Flask, request from opentelemetry import trace -from opentelemetry.instrumentation.flask import FlaskInstrumentor +from opentelemetry.instrumentation.flask import ( + FlaskInstrumentor, + _duration_attrs, + _active_requests_count_attrs, +) from opentelemetry.instrumentation.propagators import ( TraceResponsePropagator, get_global_response_propagator, set_global_response_propagator, ) +from opentelemetry.sdk.metrics.export import ( + HistogramDataPoint, + NumberDataPoint, +) from opentelemetry.instrumentation.wsgi import OpenTelemetryMiddleware from opentelemetry.sdk.resources import Resource from opentelemetry.semconv.trace import SpanAttributes @@ -48,6 +56,15 @@ def expected_attributes(override_attributes): default_attributes[key] = val 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): @@ -250,6 +267,34 @@ def test_exclude_lists_from_explicit(self): span_list = self.memory_exporter.get_finished_spans() self.assertEqual(len(span_list), 1) + def test_wsgi_metrics(self): + self.client.get('/hello/123') + self.client.get('/hello/321') + self.client.get('/hello/756') + 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) + 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) + class TestProgrammaticHooks(InstrumentationTest, WsgiTestBase): def setUp(self): From 528efb0f2012112c38ccec33634028105ef56622 Mon Sep 17 00:00:00 2001 From: "anshul.asawa" Date: Fri, 8 Jul 2022 12:32:20 +0530 Subject: [PATCH 03/18] add changlog and fix lint --- CHANGELOG.md | 4 ++++ .../instrumentation/flask/__init__.py | 23 +++++++++++++------ .../tests/test_programmatic.py | 11 +++++---- 3 files changed, 26 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a9ec0ed3d7..2a8a88a09b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased](https://github.com/open-telemetry/opentelemetry-python/compare/v1.12.0rc2-0.32b0...HEAD) +### Added +- 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/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py b/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py index a31f554efe..fff84f6143 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py @@ -141,8 +141,8 @@ def response_hook(span: Span, status: str, response_headers: List): """ from logging import getLogger -from typing import Collection from timeit import default_timer +from typing import Collection import flask @@ -150,12 +150,12 @@ def response_hook(span: Span, status: str, response_headers: List): from opentelemetry import context, trace from opentelemetry.instrumentation.flask.package import _instruments from opentelemetry.instrumentation.flask.version import __version__ -from opentelemetry.metrics import get_meter from opentelemetry.instrumentation.instrumentor import BaseInstrumentor from opentelemetry.instrumentation.propagators import ( 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.http import get_excluded_urls, parse_excluded_urls @@ -198,6 +198,7 @@ def get_default_span_name(): span_name = otel_wsgi.get_default_span_name(flask.request.environ) return span_name + def _parse_status_code(resp_status): status_code, _ = resp_status.split(" ", 1) try: @@ -205,6 +206,7 @@ def _parse_status_code(resp_status): except ValueError: return None + def _rewrapped_app(wsgi_app, response_hook=None, excluded_urls=None): def _wrapped_app(wrapped_app_environ, start_response): # We want to measure the time for route matching, etc. @@ -212,6 +214,7 @@ def _wrapped_app(wrapped_app_environ, start_response): # update_name later but that API is "highly discouraged" so # we better avoid it. wrapped_app_environ[_ENVIRON_STARTTIME_KEY] = default_timer() + def _start_response(status, response_headers, *args, **kwargs): if flask.request and ( excluded_urls is None @@ -232,7 +235,9 @@ def _start_response(status, response_headers, *args, **kwargs): ) status_code = _parse_status_code(status) if status_code is not None: - flask.request.environ[_ENVIRON_STATUS_CODE_KEY] = status_code + flask.request.environ[ + _ENVIRON_STATUS_CODE_KEY + ] = status_code if ( span.is_recording() and span.kind == trace.SpanKind.SERVER @@ -287,7 +292,9 @@ def _before_request(): active_requests_count_attrs = {} for attr_key in _active_requests_count_attrs: if attributes.get(attr_key) is not None: - active_requests_count_attrs[attr_key] = attributes[attr_key] + active_requests_count_attrs[attr_key] = attributes[ + attr_key + ] active_requests_counter.add(1, active_requests_count_attrs) if flask.request.url_rule: # For 404 that result from no route found, etc, we @@ -340,7 +347,7 @@ def _teardown_request(exc): for attr_key in _active_requests_count_attrs: if attributes.get(attr_key) is not None: active_requests_count_attrs[attr_key] = attributes[attr_key] - + duration_attrs = {} for attr_key in _duration_attrs: if attributes.get(attr_key) is not None: @@ -388,7 +395,9 @@ def __init__(self, *args, **kwargs): __name__, __version__, _InstrumentedFlask._tracer_provider ) - meter = get_meter(__name__, __version__, _InstrumentedFlask._meter_provider) + meter = get_meter( + __name__, __version__, _InstrumentedFlask._meter_provider + ) duration_histogram = meter.create_histogram( name="http.server.duration", unit="ms", @@ -412,7 +421,7 @@ def __init__(self, *args, **kwargs): _teardown_request = _wrapped_teardown_request( active_requests_counter, duration_histogram, - excluded_urls=_InstrumentedFlask._excluded_urls + excluded_urls=_InstrumentedFlask._excluded_urls, ) self.teardown_request(_teardown_request) diff --git a/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py b/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py index 8224a756fa..bc9a8c2ce3 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py +++ b/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py @@ -19,19 +19,19 @@ from opentelemetry import trace from opentelemetry.instrumentation.flask import ( FlaskInstrumentor, - _duration_attrs, _active_requests_count_attrs, + _duration_attrs, ) from opentelemetry.instrumentation.propagators import ( TraceResponsePropagator, get_global_response_propagator, set_global_response_propagator, ) +from opentelemetry.instrumentation.wsgi import OpenTelemetryMiddleware from opentelemetry.sdk.metrics.export import ( HistogramDataPoint, NumberDataPoint, ) -from opentelemetry.instrumentation.wsgi import OpenTelemetryMiddleware from opentelemetry.sdk.resources import Resource from opentelemetry.semconv.trace import SpanAttributes from opentelemetry.test.wsgitestutil import WsgiTestBase @@ -56,6 +56,7 @@ def expected_attributes(override_attributes): default_attributes[key] = val return default_attributes + _expected_metric_names = [ "http.server.active_requests", "http.server.duration", @@ -268,9 +269,9 @@ def test_exclude_lists_from_explicit(self): self.assertEqual(len(span_list), 1) def test_wsgi_metrics(self): - self.client.get('/hello/123') - self.client.get('/hello/321') - self.client.get('/hello/756') + self.client.get("/hello/123") + self.client.get("/hello/321") + self.client.get("/hello/756") metrics_list = self.memory_metrics_reader.get_metrics_data() number_data_point_seen = False histogram_data_point_seen = False From aea88d423d74f5db5cc4be3bc255630bee8a0f40 Mon Sep 17 00:00:00 2001 From: "anshul.asawa" Date: Wed, 13 Jul 2022 16:22:03 +0530 Subject: [PATCH 04/18] use wsgi functions instaed of flask's own --- .../instrumentation/flask/__init__.py | 48 ++----------------- .../tests/test_programmatic.py | 8 ++-- .../instrumentation/wsgi/__init__.py | 28 ++++++----- 3 files changed, 26 insertions(+), 58 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py b/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py index fff84f6143..d2a3b114c8 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py @@ -167,26 +167,6 @@ def response_hook(span: Span, status: str, response_headers: List): _ENVIRON_TOKEN = "opentelemetry-flask.token" _ENVIRON_STATUS_CODE_KEY = "opentelemetry-flask.status_code" -# List of recommended attributes -_duration_attrs = [ - SpanAttributes.HTTP_METHOD, - SpanAttributes.HTTP_HOST, - SpanAttributes.HTTP_SCHEME, - SpanAttributes.HTTP_STATUS_CODE, - SpanAttributes.HTTP_FLAVOR, - SpanAttributes.HTTP_SERVER_NAME, - SpanAttributes.NET_HOST_NAME, - SpanAttributes.NET_HOST_PORT, -] - -_active_requests_count_attrs = [ - SpanAttributes.HTTP_METHOD, - SpanAttributes.HTTP_HOST, - SpanAttributes.HTTP_SCHEME, - SpanAttributes.HTTP_FLAVOR, - SpanAttributes.HTTP_SERVER_NAME, -] - _excluded_urls_from_env = get_excluded_urls("FLASK") @@ -199,14 +179,6 @@ def get_default_span_name(): return span_name -def _parse_status_code(resp_status): - status_code, _ = resp_status.split(" ", 1) - try: - return int(status_code) - except ValueError: - return None - - def _rewrapped_app(wsgi_app, response_hook=None, excluded_urls=None): def _wrapped_app(wrapped_app_environ, start_response): # We want to measure the time for route matching, etc. @@ -233,7 +205,7 @@ def _start_response(status, response_headers, *args, **kwargs): otel_wsgi.add_response_attributes( span, status, response_headers ) - status_code = _parse_status_code(status) + status_code = otel_wsgi.parse_status_code(status) if status_code is not None: flask.request.environ[ _ENVIRON_STATUS_CODE_KEY @@ -289,12 +261,7 @@ def _before_request(): attributes = otel_wsgi.collect_request_attributes( flask_request_environ ) - active_requests_count_attrs = {} - for attr_key in _active_requests_count_attrs: - if attributes.get(attr_key) is not None: - active_requests_count_attrs[attr_key] = attributes[ - attr_key - ] + active_requests_count_attrs = otel_wsgi.parse_active_request_count_attrs(attributes) active_requests_counter.add(1, active_requests_count_attrs) if flask.request.url_rule: # For 404 that result from no route found, etc, we @@ -343,15 +310,8 @@ def _teardown_request(exc): attributes = otel_wsgi.collect_request_attributes( flask.request.environ ) - active_requests_count_attrs = {} - for attr_key in _active_requests_count_attrs: - if attributes.get(attr_key) is not None: - active_requests_count_attrs[attr_key] = attributes[attr_key] - - duration_attrs = {} - for attr_key in _duration_attrs: - if attributes.get(attr_key) is not None: - duration_attrs[attr_key] = attributes[attr_key] + active_requests_count_attrs = otel_wsgi.parse_active_request_count_attrs(attributes) + duration_attrs = otel_wsgi.parse_duration_attrs(attributes) status_code = flask.request.environ.get(_ENVIRON_STATUS_CODE_KEY, None) if status_code: duration_attrs[SpanAttributes.HTTP_STATUS_CODE] = status_code diff --git a/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py b/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py index bc9a8c2ce3..2ea6f2c41e 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py +++ b/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py @@ -19,15 +19,17 @@ from opentelemetry import trace from opentelemetry.instrumentation.flask import ( FlaskInstrumentor, - _active_requests_count_attrs, - _duration_attrs, ) from opentelemetry.instrumentation.propagators import ( TraceResponsePropagator, 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, diff --git a/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py b/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py index 11c5acf643..6e814f1537 100644 --- a/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py @@ -326,13 +326,26 @@ def collect_custom_response_headers_attributes(response_headers): return attributes -def _parse_status_code(resp_status): +def parse_status_code(resp_status): status_code, _ = resp_status.split(" ", 1) try: return int(status_code) except ValueError: 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 @@ -412,7 +425,7 @@ def _create_start_response( @functools.wraps(start_response) def _start_response(status, response_headers, *args, **kwargs): add_response_attributes(span, status, response_headers) - status_code = _parse_status_code(status) + status_code = 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: @@ -436,15 +449,8 @@ 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, From cade131c8e8200d8b015639544a622217dc79fa0 Mon Sep 17 00:00:00 2001 From: "anshul.asawa" Date: Wed, 13 Jul 2022 16:25:45 +0530 Subject: [PATCH 05/18] fix lint --- .../src/opentelemetry/instrumentation/flask/__init__.py | 8 ++++++-- .../tests/test_programmatic.py | 4 +--- .../src/opentelemetry/instrumentation/wsgi/__init__.py | 7 ++++++- 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py b/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py index d2a3b114c8..ab6ababa2e 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py @@ -261,7 +261,9 @@ def _before_request(): attributes = otel_wsgi.collect_request_attributes( flask_request_environ ) - active_requests_count_attrs = otel_wsgi.parse_active_request_count_attrs(attributes) + active_requests_count_attrs = ( + otel_wsgi.parse_active_request_count_attrs(attributes) + ) active_requests_counter.add(1, active_requests_count_attrs) if flask.request.url_rule: # For 404 that result from no route found, etc, we @@ -310,7 +312,9 @@ def _teardown_request(exc): attributes = otel_wsgi.collect_request_attributes( flask.request.environ ) - active_requests_count_attrs = otel_wsgi.parse_active_request_count_attrs(attributes) + active_requests_count_attrs = ( + otel_wsgi.parse_active_request_count_attrs(attributes) + ) duration_attrs = otel_wsgi.parse_duration_attrs(attributes) status_code = flask.request.environ.get(_ENVIRON_STATUS_CODE_KEY, None) if status_code: diff --git a/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py b/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py index 2ea6f2c41e..d0b4f20998 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py +++ b/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py @@ -17,9 +17,7 @@ from flask import Flask, request from opentelemetry import trace -from opentelemetry.instrumentation.flask import ( - FlaskInstrumentor, -) +from opentelemetry.instrumentation.flask import FlaskInstrumentor from opentelemetry.instrumentation.propagators import ( TraceResponsePropagator, get_global_response_propagator, diff --git a/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py b/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py index 6e814f1537..6aac913f66 100644 --- a/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py @@ -333,6 +333,7 @@ def parse_status_code(resp_status): except ValueError: return None + def parse_active_request_count_attrs(req_attrs): active_requests_count_attrs = {} for attr_key in _active_requests_count_attrs: @@ -340,6 +341,7 @@ def parse_active_request_count_attrs(req_attrs): 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: @@ -347,6 +349,7 @@ def parse_duration_attrs(req_attrs): 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 @@ -449,7 +452,9 @@ def __call__(self, environ, start_response): start_response: The WSGI start_response callable. """ req_attrs = collect_request_attributes(environ) - active_requests_count_attrs = parse_active_request_count_attrs(req_attrs) + 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( From d6026f875e30f7741e80ed2c36e6d094039e4c58 Mon Sep 17 00:00:00 2001 From: "anshul.asawa" Date: Thu, 14 Jul 2022 09:56:20 +0530 Subject: [PATCH 06/18] change starttime and duration time --- .../src/opentelemetry/instrumentation/flask/__init__.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py b/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py index ab6ababa2e..56ed17ce7e 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py @@ -157,11 +157,13 @@ def response_hook(span: Span, status: str, response_headers: List): 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 _logger = getLogger(__name__) _ENVIRON_STARTTIME_KEY = "opentelemetry-flask.starttime_key" +_ENVIRON_DURATION_STARTTIME_KEY = "opentelemetry-flask.duration_starttime_key" _ENVIRON_SPAN_KEY = "opentelemetry-flask.span_key" _ENVIRON_ACTIVATION_KEY = "opentelemetry-flask.activation_key" _ENVIRON_TOKEN = "opentelemetry-flask.token" @@ -185,7 +187,8 @@ def _wrapped_app(wrapped_app_environ, start_response): # 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] = default_timer() + wrapped_app_environ[_ENVIRON_STARTTIME_KEY] = _time_ns() + wrapped_app_environ[_ENVIRON_DURATION_STARTTIME_KEY] = default_timer() def _start_response(status, response_headers, *args, **kwargs): if flask.request and ( @@ -307,7 +310,7 @@ def _teardown_request(exc): # a way that doesn't run `before_request`, like when it is created # with `app.test_request_context`. return - start = flask.request.environ.get(_ENVIRON_STARTTIME_KEY) + start = flask.request.environ.get(_ENVIRON_DURATION_STARTTIME_KEY) duration = max(round((default_timer() - start) * 1000), 0) attributes = otel_wsgi.collect_request_attributes( flask.request.environ From ea828be92630e3fa8b0ffe975fa3ce441e50d45c Mon Sep 17 00:00:00 2001 From: "anshul.asawa" Date: Mon, 18 Jul 2022 15:16:35 +0530 Subject: [PATCH 07/18] add metric instrumentation in call instead of before request and teardown --- .../instrumentation/flask/__init__.py | 94 +++++++++---------- .../tests/test_programmatic.py | 2 +- 2 files changed, 46 insertions(+), 50 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py b/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py index 56ed17ce7e..b3d122adda 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py @@ -143,6 +143,7 @@ def response_hook(span: Span, status: str, response_headers: List): from logging import getLogger from timeit import default_timer from typing import Collection +from unittest import result import flask @@ -181,14 +182,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() - wrapped_app_environ[_ENVIRON_DURATION_STARTTIME_KEY] = default_timer() + 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 ( @@ -210,8 +223,8 @@ def _start_response(status, response_headers, *args, **kwargs): ) status_code = otel_wsgi.parse_status_code(status) if status_code is not None: - flask.request.environ[ - _ENVIRON_STATUS_CODE_KEY + duration_attrs[ + SpanAttributes.HTTP_STATUS_CODE ] = status_code if ( span.is_recording() @@ -232,13 +245,16 @@ 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( - active_requests_counter, request_hook=None, tracer=None, excluded_urls=None, @@ -264,10 +280,6 @@ def _before_request(): attributes = otel_wsgi.collect_request_attributes( flask_request_environ ) - active_requests_count_attrs = ( - otel_wsgi.parse_active_request_count_attrs(attributes) - ) - active_requests_counter.add(1, active_requests_count_attrs) if flask.request.url_rule: # For 404 that result from no route found, etc, we # don't have a url_rule. @@ -295,8 +307,6 @@ def _before_request(): def _wrapped_teardown_request( - active_requests_counter, - duration_histogram, excluded_urls=None, ): def _teardown_request(exc): @@ -310,21 +320,6 @@ def _teardown_request(exc): # a way that doesn't run `before_request`, like when it is created # with `app.test_request_context`. return - start = flask.request.environ.get(_ENVIRON_DURATION_STARTTIME_KEY) - duration = max(round((default_timer() - start) * 1000), 0) - attributes = otel_wsgi.collect_request_attributes( - flask.request.environ - ) - active_requests_count_attrs = ( - otel_wsgi.parse_active_request_count_attrs(attributes) - ) - duration_attrs = otel_wsgi.parse_duration_attrs(attributes) - status_code = flask.request.environ.get(_ENVIRON_STATUS_CODE_KEY, None) - if status_code: - duration_attrs[SpanAttributes.HTTP_STATUS_CODE] = status_code - - duration_histogram.record(duration, duration_attrs) - active_requests_counter.add(-1, active_requests_count_attrs) if exc is None: activation.__exit__(None, None, None) else: @@ -352,16 +347,6 @@ def __init__(self, *args, **kwargs): self._original_wsgi_app = self.wsgi_app self._is_instrumented_by_opentelemetry = True - self.wsgi_app = _rewrapped_app( - self.wsgi_app, - _InstrumentedFlask._response_hook, - excluded_urls=_InstrumentedFlask._excluded_urls, - ) - - tracer = trace.get_tracer( - __name__, __version__, _InstrumentedFlask._tracer_provider - ) - meter = get_meter( __name__, __version__, _InstrumentedFlask._meter_provider ) @@ -376,8 +361,19 @@ def __init__(self, *args, **kwargs): description="measures the number of concurrent HTTP requests that are currently in-flight", ) - _before_request = _wrapped_before_request( + self.wsgi_app = _rewrapped_app( + self.wsgi_app, active_requests_counter, + duration_histogram, + _InstrumentedFlask._response_hook, + excluded_urls=_InstrumentedFlask._excluded_urls, + ) + + tracer = trace.get_tracer( + __name__, __version__, _InstrumentedFlask._tracer_provider + ) + + _before_request = _wrapped_before_request( _InstrumentedFlask._request_hook, tracer, excluded_urls=_InstrumentedFlask._excluded_urls, @@ -386,8 +382,6 @@ def __init__(self, *args, **kwargs): self.before_request(_before_request) _teardown_request = _wrapped_teardown_request( - active_requests_counter, - duration_histogram, excluded_urls=_InstrumentedFlask._excluded_urls, ) self.teardown_request(_teardown_request) @@ -444,12 +438,6 @@ def instrument_app( if excluded_urls is not None else _excluded_urls_from_env ) - app._original_wsgi_app = app.wsgi_app - app.wsgi_app = _rewrapped_app( - app.wsgi_app, response_hook, excluded_urls=excluded_urls - ) - - tracer = trace.get_tracer(__name__, __version__, tracer_provider) meter = get_meter(__name__, __version__, meter_provider) duration_histogram = meter.create_histogram( name="http.server.duration", @@ -462,8 +450,18 @@ def instrument_app( description="measures the number of concurrent HTTP requests that are currently in-flight", ) - _before_request = _wrapped_before_request( + app._original_wsgi_app = app.wsgi_app + app.wsgi_app = _rewrapped_app( + app.wsgi_app, active_requests_counter, + duration_histogram, + response_hook, + excluded_urls=excluded_urls, + ) + + tracer = trace.get_tracer(__name__, __version__, tracer_provider) + + _before_request = _wrapped_before_request( request_hook, tracer, excluded_urls=excluded_urls, @@ -472,8 +470,6 @@ def instrument_app( app.before_request(_before_request) _teardown_request = _wrapped_teardown_request( - active_requests_counter, - duration_histogram, excluded_urls=excluded_urls, ) app._teardown_request = _teardown_request diff --git a/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py b/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py index d0b4f20998..7a0a5b29ff 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py +++ b/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py @@ -268,7 +268,7 @@ def test_exclude_lists_from_explicit(self): span_list = self.memory_exporter.get_finished_spans() self.assertEqual(len(span_list), 1) - def test_wsgi_metrics(self): + def test_flask_metrics(self): self.client.get("/hello/123") self.client.get("/hello/321") self.client.get("/hello/756") From 42731777a70eed4024e74eaf063bdd37e2feb741 Mon Sep 17 00:00:00 2001 From: "anshul.asawa" Date: Mon, 18 Jul 2022 15:23:22 +0530 Subject: [PATCH 08/18] change functions to private and remove unused vaiables --- .../opentelemetry/instrumentation/flask/__init__.py | 8 +++----- .../opentelemetry/instrumentation/wsgi/__init__.py | 12 ++++++------ 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py b/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py index b3d122adda..09c71c0291 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py @@ -164,11 +164,9 @@ def response_hook(span: Span, status: str, response_headers: List): _logger = getLogger(__name__) _ENVIRON_STARTTIME_KEY = "opentelemetry-flask.starttime_key" -_ENVIRON_DURATION_STARTTIME_KEY = "opentelemetry-flask.duration_starttime_key" _ENVIRON_SPAN_KEY = "opentelemetry-flask.span_key" _ENVIRON_ACTIVATION_KEY = "opentelemetry-flask.activation_key" _ENVIRON_TOKEN = "opentelemetry-flask.token" -_ENVIRON_STATUS_CODE_KEY = "opentelemetry-flask.status_code" _excluded_urls_from_env = get_excluded_urls("FLASK") @@ -198,9 +196,9 @@ def _wrapped_app(wrapped_app_environ, start_response): start = default_timer() attributes = otel_wsgi.collect_request_attributes(wrapped_app_environ) active_requests_count_attrs = ( - otel_wsgi.parse_active_request_count_attrs(attributes) + otel_wsgi._parse_active_request_count_attrs(attributes) ) - duration_attrs = otel_wsgi.parse_duration_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): @@ -221,7 +219,7 @@ 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) + status_code = otel_wsgi._parse_status_code(status) if status_code is not None: duration_attrs[ SpanAttributes.HTTP_STATUS_CODE diff --git a/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py b/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py index 6aac913f66..1d82b33037 100644 --- a/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py @@ -326,7 +326,7 @@ def collect_custom_response_headers_attributes(response_headers): return attributes -def parse_status_code(resp_status): +def _parse_status_code(resp_status): status_code, _ = resp_status.split(" ", 1) try: return int(status_code) @@ -334,7 +334,7 @@ def parse_status_code(resp_status): return None -def parse_active_request_count_attrs(req_attrs): +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: @@ -342,7 +342,7 @@ def parse_active_request_count_attrs(req_attrs): return active_requests_count_attrs -def parse_duration_attrs(req_attrs): +def _parse_duration_attrs(req_attrs): duration_attrs = {} for attr_key in _duration_attrs: if req_attrs.get(attr_key) is not None: @@ -428,7 +428,7 @@ def _create_start_response( @functools.wraps(start_response) def _start_response(status, response_headers, *args, **kwargs): add_response_attributes(span, status, response_headers) - status_code = parse_status_code(status) + status_code = _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: @@ -452,10 +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 = parse_active_request_count_attrs( + active_requests_count_attrs = _parse_active_request_count_attrs( req_attrs ) - duration_attrs = parse_duration_attrs(req_attrs) + duration_attrs = _parse_duration_attrs(req_attrs) span, token = _start_internal_or_server_span( tracer=self.tracer, From 6bbfea222d1c09c8ea8d7cdeb30ddd5ba4f47a6c Mon Sep 17 00:00:00 2001 From: "anshul.asawa" Date: Mon, 18 Jul 2022 15:25:09 +0530 Subject: [PATCH 09/18] fix typo --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 660f9bff26..85e4215e7c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Adding multiple db connections support for django-instrumentation's sqlcommenter ([#1187](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1187)) -### Addedgi +### Added - `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)) From ab37e053e18131113d1dcc87a2e41e6fec910d74 Mon Sep 17 00:00:00 2001 From: "anshul.asawa" Date: Mon, 18 Jul 2022 15:26:25 +0530 Subject: [PATCH 10/18] fix typo --- .../src/opentelemetry/instrumentation/flask/__init__.py | 1 - 1 file changed, 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py b/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py index 09c71c0291..db63c8c07c 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py @@ -143,7 +143,6 @@ def response_hook(span: Span, status: str, response_headers: List): from logging import getLogger from timeit import default_timer from typing import Collection -from unittest import result import flask From 9286231544d98ff689352f975d1b4b09e7208394 Mon Sep 17 00:00:00 2001 From: "anshul.asawa" Date: Wed, 27 Jul 2022 16:25:57 +0530 Subject: [PATCH 11/18] add duration test --- .../tests/test_programmatic.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py b/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py index 7a0a5b29ff..cb26888f3d 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py +++ b/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py @@ -17,6 +17,7 @@ from flask import Flask, request from opentelemetry import trace +from timeit import default_timer from opentelemetry.instrumentation.flask import FlaskInstrumentor from opentelemetry.instrumentation.propagators import ( TraceResponsePropagator, @@ -269,9 +270,11 @@ def test_exclude_lists_from_explicit(self): 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 @@ -287,6 +290,7 @@ def test_flask_metrics(self): 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 From 1a33b212ec0350f4082b76e489b7a9664a785aea Mon Sep 17 00:00:00 2001 From: "anshul.asawa" Date: Wed, 27 Jul 2022 17:29:35 +0530 Subject: [PATCH 12/18] fix lint --- .../tests/test_programmatic.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py b/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py index cb26888f3d..49f29f0f51 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py +++ b/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py @@ -12,12 +12,12 @@ # 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 from opentelemetry import trace -from timeit import default_timer from opentelemetry.instrumentation.flask import FlaskInstrumentor from opentelemetry.instrumentation.propagators import ( TraceResponsePropagator, @@ -290,7 +290,9 @@ def test_flask_metrics(self): for point in data_points: if isinstance(point, HistogramDataPoint): self.assertEqual(point.count, 3) - self.assertAlmostEqual(duration, point.sum, delta=10) + self.assertAlmostEqual( + duration, point.sum, delta=10 + ) histogram_data_point_seen = True if isinstance(point, NumberDataPoint): number_data_point_seen = True From dda754814d97e28fa33979e51627a07526ea23ff Mon Sep 17 00:00:00 2001 From: "anshul.asawa" Date: Wed, 27 Jul 2022 19:11:54 +0530 Subject: [PATCH 13/18] add attributes value test for metrics --- .../tests/test_programmatic.py | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py b/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py index 49f29f0f51..0b8b10dbfc 100644 --- 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 cmath import e from timeit import default_timer from unittest.mock import Mock, patch @@ -302,6 +303,42 @@ def test_flask_metrics(self): ) self.assertTrue(number_data_point_seen and histogram_data_point_seen) + def test_basic_metric_success(self): + self.client.get("/hello/123") + 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) + class TestProgrammaticHooks(InstrumentationTest, WsgiTestBase): def setUp(self): From 95570352044656dd1dd3036c08b9cbc505e256d2 Mon Sep 17 00:00:00 2001 From: "anshul.asawa" Date: Thu, 28 Jul 2022 12:05:16 +0530 Subject: [PATCH 14/18] remove unwanted import --- .../tests/test_programmatic.py | 1 - 1 file changed, 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py b/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py index 0b8b10dbfc..2998c0c630 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py +++ b/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py @@ -12,7 +12,6 @@ # See the License for the specific language governing permissions and # limitations under the License. -from cmath import e from timeit import default_timer from unittest.mock import Mock, patch From 27cdbc72f57fa93efa29061d13a9288cb8e317f4 Mon Sep 17 00:00:00 2001 From: "anshul.asawa" Date: Mon, 1 Aug 2022 16:47:34 +0530 Subject: [PATCH 15/18] add test for uninstrument --- .../tests/test_programmatic.py | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py b/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py index 2998c0c630..d0a6014fdb 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py +++ b/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py @@ -303,7 +303,7 @@ def test_flask_metrics(self): self.assertTrue(number_data_point_seen and histogram_data_point_seen) def test_basic_metric_success(self): - self.client.get("/hello/123") + self.client.get("/hello/756") expected_duration_attributes = { "http.method": "GET", "http.host": "localhost", @@ -338,6 +338,19 @@ def test_basic_metric_success(self): ) self.assertEqual(point.value, 0) + def test_metric_uninstrument(self): + self.client.get("/hello/756") + FlaskInstrumentor().uninstrument_app(self.app) + self.client.get("/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: + data_points = list(metric.data.data_points) + for point in data_points: + if isinstance(point, HistogramDataPoint): + self.assertEqual(point.count, 1) + class TestProgrammaticHooks(InstrumentationTest, WsgiTestBase): def setUp(self): From bc89aa0f4c1eaa6d32e2fe01ea3959a666c46f96 Mon Sep 17 00:00:00 2001 From: "anshul.asawa" Date: Mon, 1 Aug 2022 17:41:32 +0530 Subject: [PATCH 16/18] change request type to post for testing --- .../tests/test_programmatic.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py b/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py index d0a6014fdb..baff626f82 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py +++ b/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py @@ -339,9 +339,9 @@ def test_basic_metric_success(self): self.assertEqual(point.value, 0) def test_metric_uninstrument(self): - self.client.get("/hello/756") + self.client.post("/hello/756") FlaskInstrumentor().uninstrument_app(self.app) - self.client.get("/hello/756") + self.client.post("/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: From 0ba053fad4190ec46f135bccd3c7ce18ede135d1 Mon Sep 17 00:00:00 2001 From: "anshul.asawa" Date: Tue, 2 Aug 2022 12:10:50 +0530 Subject: [PATCH 17/18] add value test for post request --- .../tests/test_programmatic.py | 26 ++++++++++++++++--- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py b/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py index baff626f82..a64ca48d55 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py +++ b/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py @@ -302,6 +302,25 @@ def test_flask_metrics(self): ) 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 = { @@ -339,15 +358,14 @@ def test_basic_metric_success(self): self.assertEqual(point.value, 0) def test_metric_uninstrument(self): - self.client.post("/hello/756") + self.client.delete("/hello/756") FlaskInstrumentor().uninstrument_app(self.app) - self.client.post("/hello/756") + 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: - data_points = list(metric.data.data_points) - for point in data_points: + for point in list(metric.data.data_points): if isinstance(point, HistogramDataPoint): self.assertEqual(point.count, 1) From f45ca5109f7199d582cae886f01c4d2d90e8a33c Mon Sep 17 00:00:00 2001 From: "anshul.asawa" Date: Fri, 5 Aug 2022 12:11:59 +0530 Subject: [PATCH 18/18] add readme for metrics --- instrumentation/README.md | 2 +- .../src/opentelemetry/instrumentation/flask/package.py | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/instrumentation/README.md b/instrumentation/README.md index 71d79ea258..269010b54f 100644 --- 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/package.py b/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/package.py index b081564202..33bfe4ccba 100644 --- 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