Modified logic for excluded urls

Author: rads-1996

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
 

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):