From d3056342db2560c6a085928190b20cc943302473 Mon Sep 17 00:00:00 2001 From: Radhika Gupta Date: Mon, 29 Sep 2025 14:22:48 -0700 Subject: [PATCH 01/11] Fixed an issue #2352 where http_server_request_duration metrics was being recorded for excluded urls. --- CHANGELOG.md | 1 + .../instrumentation/flask/__init__.py | 46 ++++++++++--------- 2 files changed, 26 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9814367242..289099b59e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased ### Fixed +- `opentelemetry-instrumentation-flask` Fixed an issue where http_server_request_duration metrics was being recorded for excluded urls. - `opentelemetry-instrumentation-botocore`: migrate off the deprecated events API to use the logs API ([#3624](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3624)) 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 9a22383e1b..b390ed0247 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py @@ -395,31 +395,35 @@ def _start_response(status, response_headers, *args, **kwargs): return start_response(status, response_headers, *args, **kwargs) result = wsgi_app(wrapped_app_environ, _start_response) - duration_s = default_timer() - start - if duration_histogram_old: - duration_attrs_old = otel_wsgi._parse_duration_attrs( - attributes, _StabilityMode.DEFAULT - ) + if flask.request and ( + excluded_urls is None + or not excluded_urls.url_disabled(flask.request.url) + ): + duration_s = default_timer() - start + if duration_histogram_old: + duration_attrs_old = otel_wsgi._parse_duration_attrs( + attributes, _StabilityMode.DEFAULT + ) - if request_route: - # http.target to be included in old semantic conventions - duration_attrs_old[HTTP_TARGET] = str(request_route) + if request_route: + # http.target to be included in old semantic conventions + duration_attrs_old[HTTP_TARGET] = str(request_route) - duration_histogram_old.record( - max(round(duration_s * 1000), 0), duration_attrs_old - ) - if duration_histogram_new: - duration_attrs_new = otel_wsgi._parse_duration_attrs( - attributes, _StabilityMode.HTTP - ) + duration_histogram_old.record( + max(round(duration_s * 1000), 0), duration_attrs_old + ) + if duration_histogram_new: + duration_attrs_new = otel_wsgi._parse_duration_attrs( + attributes, _StabilityMode.HTTP + ) - if request_route: - duration_attrs_new[HTTP_ROUTE] = str(request_route) + if request_route: + duration_attrs_new[HTTP_ROUTE] = str(request_route) - duration_histogram_new.record( - max(duration_s, 0), duration_attrs_new - ) - active_requests_counter.add(-1, active_requests_count_attrs) + duration_histogram_new.record( + max(duration_s, 0), duration_attrs_new + ) + active_requests_counter.add(-1, active_requests_count_attrs) return result return _wrapped_app From 304652c526c9e68509af2ed43371319b1ac7cbb5 Mon Sep 17 00:00:00 2001 From: Radhika Gupta Date: Mon, 29 Sep 2025 14:27:53 -0700 Subject: [PATCH 02/11] Updated CHANGELOG --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 289099b59e..8020012137 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - `opentelemetry-instrumentation-flask` Fixed an issue where http_server_request_duration metrics was being recorded for excluded urls. + ([#3794](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3794)) - `opentelemetry-instrumentation-botocore`: migrate off the deprecated events API to use the logs API ([#3624](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3624)) From 964f6ed0e63e998dc955e1d670681c696ba73557 Mon Sep 17 00:00:00 2001 From: Radhika Gupta Date: Mon, 29 Sep 2025 14:56:11 -0700 Subject: [PATCH 03/11] Fix indentation --- .../src/opentelemetry/instrumentation/flask/__init__.py | 6 +++--- 1 file changed, 3 insertions(+), 3 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 b390ed0247..310d11b322 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py @@ -396,9 +396,9 @@ def _start_response(status, response_headers, *args, **kwargs): result = wsgi_app(wrapped_app_environ, _start_response) if flask.request and ( - excluded_urls is None - or not excluded_urls.url_disabled(flask.request.url) - ): + excluded_urls is None + or not excluded_urls.url_disabled(flask.request.url) + ): duration_s = default_timer() - start if duration_histogram_old: duration_attrs_old = otel_wsgi._parse_duration_attrs( From c23fd0ff2c4fb9746076fa6dd6c79604ea94e580 Mon Sep 17 00:00:00 2001 From: Radhika Gupta Date: Mon, 29 Sep 2025 16:18:33 -0700 Subject: [PATCH 04/11] Fix tests --- .../src/opentelemetry/instrumentation/flask/__init__.py | 2 +- .../tests/test_programmatic.py | 4 ++-- 2 files changed, 3 insertions(+), 3 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 310d11b322..2dfe135ff2 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py @@ -423,7 +423,7 @@ def _start_response(status, response_headers, *args, **kwargs): duration_histogram_new.record( max(duration_s, 0), duration_attrs_new ) - active_requests_counter.add(-1, active_requests_count_attrs) + active_requests_counter.add(-1, active_requests_count_attrs) return result return _wrapped_app diff --git a/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py b/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py index 60f08c7cd2..92cdff1a73 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py +++ b/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py @@ -523,7 +523,7 @@ def test_flask_metrics(self): attr, _recommended_metrics_attrs_old[metric.name], ) - self.assertTrue(number_data_point_seen and histogram_data_point_seen) + self.assertFalse(number_data_point_seen and histogram_data_point_seen) def test_flask_metrics_new_semconv(self): start = default_timer() @@ -561,7 +561,7 @@ def test_flask_metrics_new_semconv(self): attr, _recommended_metrics_attrs_new[metric.name], ) - self.assertTrue(number_data_point_seen and histogram_data_point_seen) + self.assertFalse(number_data_point_seen and histogram_data_point_seen) def test_flask_metric_values(self): start = default_timer() From f914ef7d35ce0d78a39ad6534c2f021dd112ac29 Mon Sep 17 00:00:00 2001 From: Radhika Gupta Date: Mon, 29 Sep 2025 16:29:36 -0700 Subject: [PATCH 05/11] Fix assert condition --- .../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 92cdff1a73..3ff2345297 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py +++ b/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py @@ -523,7 +523,8 @@ def test_flask_metrics(self): attr, _recommended_metrics_attrs_old[metric.name], ) - self.assertFalse(number_data_point_seen and histogram_data_point_seen) + self.assertTrue(number_data_point_seen) + self.assertFalse(histogram_data_point_seen) def test_flask_metrics_new_semconv(self): start = default_timer() @@ -561,7 +562,8 @@ def test_flask_metrics_new_semconv(self): attr, _recommended_metrics_attrs_new[metric.name], ) - self.assertFalse(number_data_point_seen and histogram_data_point_seen) + self.assertTrue(number_data_point_seen) + self.assertFalse(histogram_data_point_seen) def test_flask_metric_values(self): start = default_timer() From 87a8cee942c6c95c03101616cf652a2550297372 Mon Sep 17 00:00:00 2001 From: rads-1996 Date: Mon, 13 Oct 2025 07:21:56 -0700 Subject: [PATCH 06/11] Update CHANGELOG.md Co-authored-by: Riccardo Magliocchetti --- CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8020012137..e77a7c1ba2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,9 +12,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased ### Fixed -- `opentelemetry-instrumentation-flask` Fixed an issue where http_server_request_duration metrics was being recorded for excluded urls. - ([#3794](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3794)) +- `opentelemetry-instrumentation-flask`: Do not record `http.server.duration` metrics for excluded URLs. + ([#3794](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3794)) - `opentelemetry-instrumentation-botocore`: migrate off the deprecated events API to use the logs API ([#3624](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3624)) - `opentelemetry-instrumentation-dbapi`: fix crash retrieving libpq version when enabling commenter with psycopg From 834241628e5ab720e4db9bb5bf004085b08de472 Mon Sep 17 00:00:00 2001 From: Radhika Gupta Date: Mon, 13 Oct 2025 13:21:46 -0700 Subject: [PATCH 07/11] Addressed comments --- .../instrumentation/flask/__init__.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 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 2dfe135ff2..0731edcf9c 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py @@ -351,10 +351,7 @@ def _wrapped_app(wrapped_app_environ, start_response): request_route = None def _start_response(status, response_headers, *args, **kwargs): - if flask.request and ( - excluded_urls is None - or not excluded_urls.url_disabled(flask.request.url) - ): + if _should_exclude_request(excluded_urls): nonlocal request_route request_route = flask.request.url_rule @@ -395,10 +392,7 @@ def _start_response(status, response_headers, *args, **kwargs): return start_response(status, response_headers, *args, **kwargs) result = wsgi_app(wrapped_app_environ, _start_response) - if flask.request and ( - excluded_urls is None - or not excluded_urls.url_disabled(flask.request.url) - ): + if _should_exclude_request(excluded_urls): duration_s = default_timer() - start if duration_histogram_old: duration_attrs_old = otel_wsgi._parse_duration_attrs( @@ -426,6 +420,13 @@ def _start_response(status, response_headers, *args, **kwargs): active_requests_counter.add(-1, active_requests_count_attrs) return result + def _should_exclude_request(excluded_urls): + if flask.request: + return excluded_urls is None or not excluded_urls.url_disabled( + flask.request.url + ) + return False + return _wrapped_app From 68983d3d757380bee73171b5f89890906fedc5e8 Mon Sep 17 00:00:00 2001 From: Radhika Gupta Date: Tue, 14 Oct 2025 13:02:32 -0700 Subject: [PATCH 08/11] Modified logic for excluded urls --- .../instrumentation/flask/__init__.py | 20 +++-- .../tests/test_programmatic.py | 81 +++++++++++++++++-- 2 files changed, 89 insertions(+), 12 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 0731edcf9c..ed64960b62 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py @@ -350,8 +350,12 @@ def _wrapped_app(wrapped_app_environ, start_response): active_requests_counter.add(1, active_requests_count_attrs) request_route = None + should_trace = False + def _start_response(status, response_headers, *args, **kwargs): - if _should_exclude_request(excluded_urls): + nonlocal should_trace + should_trace = _should_trace(excluded_urls) + if should_trace: nonlocal request_route request_route = flask.request.url_rule @@ -392,7 +396,7 @@ def _start_response(status, response_headers, *args, **kwargs): return start_response(status, response_headers, *args, **kwargs) result = wsgi_app(wrapped_app_environ, _start_response) - if _should_exclude_request(excluded_urls): + if should_trace: duration_s = default_timer() - start if duration_histogram_old: duration_attrs_old = otel_wsgi._parse_duration_attrs( @@ -420,12 +424,14 @@ def _start_response(status, response_headers, *args, **kwargs): active_requests_counter.add(-1, active_requests_count_attrs) return result - def _should_exclude_request(excluded_urls): - if flask.request: - return excluded_urls is None or not excluded_urls.url_disabled( - flask.request.url + def _should_trace(excluded_urls) -> bool: + return bool( + flask.request + and ( + excluded_urls is None + or not excluded_urls.url_disabled(flask.request.url) ) - return False + ) return _wrapped_app diff --git a/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py b/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py index 3ff2345297..4813d7da4f 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py +++ b/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py @@ -164,7 +164,7 @@ def setUp(self): self.env_patch = patch.dict( "os.environ", { - "OTEL_PYTHON_FLASK_EXCLUDED_URLS": "http://localhost/env_excluded_arg/123,env_excluded_noarg", + "OTEL_PYTHON_FLASK_EXCLUDED_URLS": "http://localhost/env_excluded_arg/123,env_excluded_noarg,env_excluded_arg/789", OTEL_SEMCONV_STABILITY_OPT_IN: sem_conv_mode, }, ) @@ -523,8 +523,7 @@ def test_flask_metrics(self): attr, _recommended_metrics_attrs_old[metric.name], ) - self.assertTrue(number_data_point_seen) - self.assertFalse(histogram_data_point_seen) + self.assertTrue(number_data_point_seen and histogram_data_point_seen) def test_flask_metrics_new_semconv(self): start = default_timer() @@ -562,8 +561,7 @@ def test_flask_metrics_new_semconv(self): attr, _recommended_metrics_attrs_new[metric.name], ) - self.assertTrue(number_data_point_seen) - self.assertFalse(histogram_data_point_seen) + self.assertTrue(number_data_point_seen and histogram_data_point_seen) def test_flask_metric_values(self): start = default_timer() @@ -738,6 +736,79 @@ def test_metric_uninstrument(self): if isinstance(point, HistogramDataPoint): self.assertEqual(point.count, 1) + def test_flask_metrics_excluded_urls(self): + start = default_timer() + self.client.get("/env_excluded_arg/123") + self.client.get("/env_excluded_noarg") + 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_old) + 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, 1) + 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_metrics_attrs_old[metric.name], + ) + self.assertTrue(number_data_point_seen and histogram_data_point_seen) + + def test_flask_metrics_new_semconv_excluded_urls(self): + start = default_timer() + self.client.get("/env_excluded_arg/123") + self.client.get("/env_excluded_noarg") + self.client.get("/env_excluded_arg/789") + duration_s = max(default_timer() - start, 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_new) + 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, 0) + self.assertAlmostEqual( + duration_s, point.sum, places=1 + ) + self.assertEqual( + point.explicit_bounds, + HTTP_DURATION_HISTOGRAM_BUCKETS_NEW, + ) + histogram_data_point_seen = True + if isinstance(point, NumberDataPoint): + number_data_point_seen = True + for attr in point.attributes: + self.assertIn( + attr, + _recommended_metrics_attrs_new[metric.name], + ) + self.assertTrue(number_data_point_seen) + self.assertFalse(histogram_data_point_seen) + class TestProgrammaticHooks(InstrumentationTest, WsgiTestBase): def setUp(self): From e8d3cc05674de09eb31ea5c0bae28f6e6e1690b4 Mon Sep 17 00:00:00 2001 From: Radhika Gupta Date: Wed, 15 Oct 2025 08:41:49 -0700 Subject: [PATCH 09/11] Fix test name --- .../tests/test_programmatic.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py b/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py index 4813d7da4f..4a70871a7f 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py +++ b/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py @@ -770,7 +770,7 @@ def test_flask_metrics_excluded_urls(self): ) self.assertTrue(number_data_point_seen and histogram_data_point_seen) - def test_flask_metrics_new_semconv_excluded_urls(self): + def test_flask_metrics_excluded_urls_new_semconv(self): start = default_timer() self.client.get("/env_excluded_arg/123") self.client.get("/env_excluded_noarg") From abbff1a14f8e8bccf2c6b8d8c7c3cf7ec3b1b011 Mon Sep 17 00:00:00 2001 From: Radhika Gupta Date: Wed, 15 Oct 2025 10:19:59 -0700 Subject: [PATCH 10/11] Align tests with the same assertion --- .../tests/test_programmatic.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py b/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py index 4a70871a7f..2da579adda 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py +++ b/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py @@ -740,7 +740,7 @@ def test_flask_metrics_excluded_urls(self): start = default_timer() self.client.get("/env_excluded_arg/123") self.client.get("/env_excluded_noarg") - self.client.get("/hello/756") + self.client.get("/env_excluded_arg/789") duration = max(round((default_timer() - start) * 1000), 0) metrics_list = self.memory_metrics_reader.get_metrics_data() number_data_point_seen = False @@ -756,7 +756,7 @@ def test_flask_metrics_excluded_urls(self): self.assertEqual(len(data_points), 1) for point in data_points: if isinstance(point, HistogramDataPoint): - self.assertEqual(point.count, 1) + self.assertEqual(point.count, 0) self.assertAlmostEqual( duration, point.sum, delta=10 ) @@ -768,7 +768,8 @@ def test_flask_metrics_excluded_urls(self): attr, _recommended_metrics_attrs_old[metric.name], ) - self.assertTrue(number_data_point_seen and histogram_data_point_seen) + self.assertTrue(number_data_point_seen) + self.assertFalse(histogram_data_point_seen) def test_flask_metrics_excluded_urls_new_semconv(self): start = default_timer() From 4148dad4b38347b7f79daa98bf8ccc90574dbaf7 Mon Sep 17 00:00:00 2001 From: Riccardo Magliocchetti Date: Thu, 16 Oct 2025 09:50:12 +0200 Subject: [PATCH 11/11] Update instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py --- .../src/opentelemetry/instrumentation/flask/__init__.py | 2 +- 1 file changed, 1 insertion(+), 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 ed64960b62..57f0f75ca3 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py @@ -350,7 +350,7 @@ def _wrapped_app(wrapped_app_environ, start_response): active_requests_counter.add(1, active_requests_count_attrs) request_route = None - should_trace = False + should_trace = True def _start_response(status, response_headers, *args, **kwargs): nonlocal should_trace