From fe9405730f8d18fe70308db52093b7b5b1e74e0c Mon Sep 17 00:00:00 2001 From: Michael Date: Tue, 20 Jun 2023 14:09:53 +0300 Subject: [PATCH] fix: Update falcon instrumentation to follow semantic conventions (#1824) * fix: Update falcon instrumentation to follow semantic conventions * docs: Update changelog * fix linter errors * Disable falcon.HTTP_200 pylint checck --------- Co-authored-by: Shalev Roda <65566801+shalevr@users.noreply.github.com> Co-authored-by: Srikanth Chekuri --- CHANGELOG.md | 5 +++ .../instrumentation/falcon/__init__.py | 7 +++- .../tests/app.py | 9 +++++ .../tests/test_falcon.py | 33 +++++++++++++++++-- 4 files changed, 50 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index de2ed9475e..c83443dc02 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased +### Fixed + +- Update falcon instrumentation to follow semantic conventions + ([#1824](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1824)) + ### Added - Make Flask request span attributes available for `start_span`. 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 73c005fa17..669f41b0ab 100644 --- a/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py @@ -428,7 +428,6 @@ def process_resource(self, req, resp, resource, params): resource_name = resource.__class__.__name__ span.set_attribute("falcon.resource", resource_name) - span.update_name(f"{resource_name}.on_{req.method.lower()}") def process_response( self, req, resp, resource, req_succeeded=None @@ -483,6 +482,12 @@ def process_response( response_headers = resp.headers if span.is_recording() and span.kind == trace.SpanKind.SERVER: + # Check if low-cardinality route is available as per semantic-conventions + if req.uri_template: + span.update_name(f"{req.method} {req.uri_template}") + else: + span.update_name(f"{req.method}") + custom_attributes = ( otel_wsgi.collect_custom_response_headers_attributes( response_headers.items() diff --git a/instrumentation/opentelemetry-instrumentation-falcon/tests/app.py b/instrumentation/opentelemetry-instrumentation-falcon/tests/app.py index 3e4c62ec3e..a4d279149d 100644 --- a/instrumentation/opentelemetry-instrumentation-falcon/tests/app.py +++ b/instrumentation/opentelemetry-instrumentation-falcon/tests/app.py @@ -61,6 +61,13 @@ def on_get(self, _, resp): resp.set_header("my-secret-header", "my-secret-value") +class UserResource: + def on_get(self, req, resp, user_id): + # pylint: disable=no-member + resp.status = falcon.HTTP_200 + resp.body = f"Hello user {user_id}" + + def make_app(): _parsed_falcon_version = package_version.parse(falcon.__version__) if _parsed_falcon_version < package_version.parse("3.0.0"): @@ -76,4 +83,6 @@ def make_app(): app.add_route( "/test_custom_response_headers", CustomResponseHeaderResource() ) + app.add_route("/user/{user_id}", UserResource()) + return app diff --git a/instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py b/instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py index cf61a9fd3c..2245dbfd80 100644 --- a/instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py +++ b/instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py @@ -110,7 +110,7 @@ def _test_method(self, method): spans = self.memory_exporter.get_finished_spans() self.assertEqual(len(spans), 1) span = spans[0] - self.assertEqual(span.name, f"HelloWorldResource.on_{method.lower()}") + self.assertEqual(span.name, f"{method} /hello") self.assertEqual(span.status.status_code, StatusCode.UNSET) self.assertEqual( span.status.description, @@ -145,7 +145,7 @@ def test_404(self): spans = self.memory_exporter.get_finished_spans() self.assertEqual(len(spans), 1) span = spans[0] - self.assertEqual(span.name, "GET /does-not-exist") + self.assertEqual(span.name, "GET") self.assertEqual(span.status.status_code, StatusCode.UNSET) self.assertSpanHasAttributes( span, @@ -177,7 +177,7 @@ def test_500(self): spans = self.memory_exporter.get_finished_spans() self.assertEqual(len(spans), 1) span = spans[0] - self.assertEqual(span.name, "ErrorResource.on_get") + self.assertEqual(span.name, "GET /error") self.assertFalse(span.status.is_ok) self.assertEqual(span.status.status_code, StatusCode.ERROR) self.assertEqual( @@ -206,6 +206,33 @@ def test_500(self): span.attributes[SpanAttributes.NET_PEER_IP], "127.0.0.1" ) + def test_url_template(self): + self.client().simulate_get("/user/123") + spans = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans), 1) + 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_METHOD: "GET", + SpanAttributes.HTTP_SERVER_NAME: "falconframework.org", + SpanAttributes.HTTP_SCHEME: "http", + SpanAttributes.NET_HOST_PORT: 80, + SpanAttributes.HTTP_HOST: "falconframework.org", + SpanAttributes.HTTP_TARGET: "/", + SpanAttributes.NET_PEER_PORT: "65133", + SpanAttributes.HTTP_FLAVOR: "1.1", + "falcon.resource": "UserResource", + SpanAttributes.HTTP_STATUS_CODE: 200, + }, + ) + def test_uninstrument(self): self.client().simulate_get(path="/hello") spans = self.memory_exporter.get_finished_spans()