From e9f7d276b16669708b0be03ca0b50e513a614998 Mon Sep 17 00:00:00 2001 From: Nek0trkstr Date: Wed, 24 May 2023 22:20:27 +0300 Subject: [PATCH 1/4] fix: Update falcon instrumentation to follow semantic conventions --- .../instrumentation/falcon/__init__.py | 7 +++- .../tests/app.py | 7 ++++ .../tests/test_falcon.py | 33 +++++++++++++++++-- 3 files changed, 43 insertions(+), 4 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 3a6a86e4fb..738dd77189 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 @@ -477,6 +476,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..d40f462469 100644 --- a/instrumentation/opentelemetry-instrumentation-falcon/tests/app.py +++ b/instrumentation/opentelemetry-instrumentation-falcon/tests/app.py @@ -60,6 +60,11 @@ def on_get(self, _, resp): ) resp.set_header("my-secret-header", "my-secret-value") +class UserResource: + def on_get(self, req, resp, user_id): + resp.status = falcon.HTTP_200 + resp.body = f'Hello user {user_id}' + def make_app(): _parsed_falcon_version = package_version.parse(falcon.__version__) @@ -76,4 +81,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 aeba57a9b5..5ce630e2e2 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, "HTTP GET") + 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( @@ -205,6 +205,33 @@ def test_500(self): self.assertEqual( 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") From 6da964db85c8ed64bd8d20d2dc7153038f011456 Mon Sep 17 00:00:00 2001 From: Nek0trkstr Date: Wed, 24 May 2023 22:30:46 +0300 Subject: [PATCH 2/4] docs: Update changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ee92bdab7a..a2cd6ddb50 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,8 @@ 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)) ## Version 1.18.0/0.39b0 (2023-05-10) - `opentelemetry-instrumentation-system-metrics` Add `process.` prefix to `runtime.memory`, `runtime.cpu.time`, and `runtime.gc_count`. Change `runtime.memory` from count to UpDownCounter. ([#1735](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1735)) From 1cd45be75c578e31c3e90a27f03bf95939799a28 Mon Sep 17 00:00:00 2001 From: Nek0trkstr Date: Thu, 25 May 2023 08:33:10 +0300 Subject: [PATCH 3/4] fix linter errors --- .../src/opentelemetry/instrumentation/falcon/__init__.py | 4 ++-- .../opentelemetry-instrumentation-falcon/tests/app.py | 5 +++-- .../tests/test_falcon.py | 2 +- 3 files changed, 6 insertions(+), 5 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 738dd77189..81cd7c98cf 100644 --- a/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py @@ -478,9 +478,9 @@ def process_response( 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}') + span.update_name(f"{req.method} {req.uri_template}") else: - span.update_name(f'{req.method}') + span.update_name(f"{req.method}") custom_attributes = ( otel_wsgi.collect_custom_response_headers_attributes( diff --git a/instrumentation/opentelemetry-instrumentation-falcon/tests/app.py b/instrumentation/opentelemetry-instrumentation-falcon/tests/app.py index d40f462469..66475db396 100644 --- a/instrumentation/opentelemetry-instrumentation-falcon/tests/app.py +++ b/instrumentation/opentelemetry-instrumentation-falcon/tests/app.py @@ -60,10 +60,11 @@ def on_get(self, _, resp): ) resp.set_header("my-secret-header", "my-secret-value") + class UserResource: def on_get(self, req, resp, user_id): resp.status = falcon.HTTP_200 - resp.body = f'Hello user {user_id}' + resp.body = f"Hello user {user_id}" def make_app(): @@ -81,6 +82,6 @@ def make_app(): app.add_route( "/test_custom_response_headers", CustomResponseHeaderResource() ) - app.add_route("/user/{user_id}", UserResource()) + 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 5ce630e2e2..2245dbfd80 100644 --- a/instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py +++ b/instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py @@ -205,7 +205,7 @@ def test_500(self): self.assertEqual( 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() From f16434545757303233eac282adfad3f4ead06004 Mon Sep 17 00:00:00 2001 From: Nek0trkstr Date: Thu, 25 May 2023 14:16:34 +0300 Subject: [PATCH 4/4] Disable falcon.HTTP_200 pylint checck --- .../opentelemetry-instrumentation-falcon/tests/app.py | 1 + 1 file changed, 1 insertion(+) diff --git a/instrumentation/opentelemetry-instrumentation-falcon/tests/app.py b/instrumentation/opentelemetry-instrumentation-falcon/tests/app.py index 66475db396..a4d279149d 100644 --- a/instrumentation/opentelemetry-instrumentation-falcon/tests/app.py +++ b/instrumentation/opentelemetry-instrumentation-falcon/tests/app.py @@ -63,6 +63,7 @@ def on_get(self, _, resp): 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}"