From b94156968f2158358aaa3eeedf22f2d9bfa79afa Mon Sep 17 00:00:00 2001 From: Filip Nikolovski Date: Tue, 10 Dec 2024 12:41:03 +0100 Subject: [PATCH] set request attributes in env, set status code in metric attributes independent from tracing decisions; update tests --- .../instrumentation/falcon/__init__.py | 57 +++++++++++++---- .../tests/test_falcon.py | 64 ++++++++++++++++++- 2 files changed, 108 insertions(+), 13 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py b/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py index 41a73aa7b0..429db76d9f 100644 --- a/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py +++ b/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}") diff --git a/instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py b/instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py index 305d4822a0..f5f82216e9 100644 --- a/instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py +++ b/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],