set request attributes in env, set status code in metric attributes independent from tracing decisions; update tests

Author: Filip Nikolovski

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

@@ -214,17 +214,20 @@ def response_hook(span, req, resp):
     extract_attributes_from_object,
 )
 from opentelemetry.metrics import get_meter
+from opentelemetry.semconv.attributes.http_attributes import (
+    HTTP_ROUTE,
+)
 from opentelemetry.semconv.metrics import MetricInstruments
 from opentelemetry.semconv.metrics.http_metrics import (
     HTTP_SERVER_REQUEST_DURATION,
 )
-from opentelemetry.semconv.trace import SpanAttributes
 from opentelemetry.util.http import get_excluded_urls, get_traced_request_attrs
 
 _logger = getLogger(__name__)
 
 _ENVIRON_STARTTIME_KEY = "opentelemetry-falcon.starttime_key"
 _ENVIRON_SPAN_KEY = "opentelemetry-falcon.span_key"
+_ENVIRON_REQ_ATTRS = "opentelemetry-falcon.req_attrs"
 _ENVIRON_ACTIVATION_KEY = "opentelemetry-falcon.activation_key"
 _ENVIRON_TOKEN = "opentelemetry-falcon.token"
 _ENVIRON_EXC = "opentelemetry-falcon.exc"
@@ -247,6 +250,31 @@ def response_hook(span, req, resp):
     _falcon_version = 1
 
 
+def set_status_code(
+    span,
+    status_code,
+    metric_attributes=None,
+    sem_conv_opt_in_mode=_HTTPStabilityMode.DEFAULT,
+):
+    """Adds HTTP response attributes to span using the status_code argument."""
+    status_code_str = str(status_code)
+
+    try:
+        status_code = int(status_code)
+    except ValueError:
+        status_code = -1
+    if metric_attributes is None:
+        metric_attributes = {}
+    _set_status(
+        span,
+        metric_attributes,
+        status_code,
+        status_code_str,
+        server_span=True,
+        sem_conv_opt_in_mode=sem_conv_opt_in_mode,
+    )
+
+
 class _InstrumentedFalconAPI(getattr(falcon, _instrument_app)):
     _instrumented_falcon_apps = set()
 
@@ -393,6 +421,7 @@ def __call__(self, env, start_response):
         activation.__enter__()
         env[_ENVIRON_SPAN_KEY] = span
         env[_ENVIRON_ACTIVATION_KEY] = activation
+        env[_ENVIRON_REQ_ATTRS] = attributes
         exception = None
 
         def _start_response(status, response_headers, *args, **kwargs):
@@ -478,8 +507,9 @@ def process_resource(self, req, resp, resource, params):
 
     def process_response(self, req, resp, resource, req_succeeded=None):  # pylint:disable=R0201,R0912
         span = req.env.get(_ENVIRON_SPAN_KEY)
+        req_attrs = req.env.get(_ENVIRON_REQ_ATTRS)
 
-        if not span or not span.is_recording():
+        if not span:
             return
 
         status = resp.status
@@ -497,17 +527,22 @@ def process_response(self, req, resp, resource, req_succeeded=None):  # pylint:d
                 else:
                     status = "500"
 
-        status = status.split(" ")[0]
+        status_code = status.split(" ")[0]
         try:
-            _set_status(
+            set_status_code(
                 span,
-                {},
-                int(status),
-                resp.status,
-                span.kind == trace.SpanKind.SERVER,
-                self._sem_conv_opt_in_mode,
+                status_code,
+                req_attrs,
+                sem_conv_opt_in_mode=self._sem_conv_opt_in_mode,
             )
 
+            if (
+                _report_new(self._sem_conv_opt_in_mode)
+                and req.uri_template
+                and req_attrs is not None
+            ):
+                req_attrs[HTTP_ROUTE] = req.uri_template
+
             # Falcon 1 does not support response headers. So
             # send an empty dict.
             response_headers = {}
@@ -518,9 +553,7 @@ def process_response(self, req, resp, resource, req_succeeded=None):  # pylint:d
                 # Check if low-cardinality route is available as per semantic-conventions
                 if req.uri_template:
                     span.update_name(f"{req.method} {req.uri_template}")
-                    span.set_attribute(
-                        SpanAttributes.HTTP_ROUTE, req.uri_template
-                    )
+                    span.set_attribute(HTTP_ROUTE, req.uri_template)
                 else:
                     span.update_name(f"{req.method}")
 

instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py

@@ -308,6 +308,47 @@ def test_500(self):
                 span.attributes[SpanAttributes.NET_PEER_IP], "127.0.0.1"
             )
 
+    def test_url_template_new_semconv(self):
+        self.client().simulate_get("/user/123")
+        spans = self.memory_exporter.get_finished_spans()
+        metrics_list = self.memory_metrics_reader.get_metrics_data()
+
+        self.assertEqual(len(spans), 1)
+        self.assertTrue(len(metrics_list.resource_metrics) != 0)
+        span = spans[0]
+        self.assertEqual(span.name, "GET /user/{user_id}")
+        self.assertEqual(span.status.status_code, StatusCode.UNSET)
+        self.assertEqual(
+            span.status.description,
+            None,
+        )
+        self.assertSpanHasAttributes(
+            span,
+            {
+                SpanAttributes.HTTP_REQUEST_METHOD: "GET",
+                SpanAttributes.SERVER_ADDRESS: "falconframework.org",
+                SpanAttributes.URL_SCHEME: "http",
+                SpanAttributes.SERVER_PORT: 80,
+                SpanAttributes.URL_PATH: "/",
+                SpanAttributes.CLIENT_PORT: 65133,
+                SpanAttributes.NETWORK_PROTOCOL_VERSION: "1.1",
+                "falcon.resource": "UserResource",
+                SpanAttributes.HTTP_RESPONSE_STATUS_CODE: 200,
+                SpanAttributes.HTTP_ROUTE: "/user/{user_id}",
+            },
+        )
+
+        for resource_metric in metrics_list.resource_metrics:
+            for scope_metric in resource_metric.scope_metrics:
+                for metric in scope_metric.metrics:
+                    if metric.name == "http.server.request.duration":
+                        data_points = list(metric.data.data_points)
+                        for point in data_points:
+                            self.assertIn(
+                                "http.route",
+                                point.attributes,
+                            )
+
     def test_url_template(self):
         self.client().simulate_get("/user/123")
         spans = self.memory_exporter.get_finished_spans()
@@ -391,6 +432,28 @@ def test_traced_not_recording(self):
             self.assertFalse(mock_span.set_attribute.called)
             self.assertFalse(mock_span.set_status.called)
 
+            metrics_list = self.memory_metrics_reader.get_metrics_data()
+            self.assertTrue(len(metrics_list.resource_metrics) != 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:
+                        data_points = list(metric.data.data_points)
+                        self.assertEqual(len(data_points), 1)
+                        for point in list(metric.data.data_points):
+                            if isinstance(point, HistogramDataPoint):
+                                self.assertEqual(point.count, 1)
+                            if isinstance(point, NumberDataPoint):
+                                self.assertEqual(point.value, 0)
+                            for attr in point.attributes:
+                                self.assertIn(
+                                    attr,
+                                    _recommended_metrics_attrs_old[
+                                        metric.name
+                                    ],
+                                )
+
     def test_uninstrument_after_instrument(self):
         self.client().simulate_get(path="/hello")
         spans = self.memory_exporter.get_finished_spans()
@@ -486,7 +549,6 @@ def test_falcon_metric_values_both_semconv(self):
                             self.assertEqual(point.value, 0)
                             number_data_point_seen = True
                         for attr in point.attributes:
-                            print(metric.name)
                             self.assertIn(
                                 attr,
                                 _recommended_metrics_attrs_both[metric.name],