From 09e0f47500f67414981515f26a749f0f867737c4 Mon Sep 17 00:00:00 2001 From: Michael Manganiello Date: Tue, 6 Apr 2021 10:41:38 -0300 Subject: [PATCH 1/4] Fix ASGI instrumentation default span name Fixes ASGI default span name for SERVER spans, to be `HTTP `. Fixes #146. --- CHANGELOG.md | 8 +++--- .../instrumentation/asgi/__init__.py | 8 +++--- .../tests/test_asgi_middleware.py | 27 ++++++++++--------- 3 files changed, 24 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 50e0efb246..11bc269669 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,12 +12,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `opentelemetry-instrumentation-tornado` Fixed cases where description was used with non- error status code when creating Status objects. ([#504](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/504)) +- Fix ASGI instrumentation default span name. + ([#418](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/418)) ### Added - `opentelemetry-instrumentation-botocore` now supports - context propagation for lambda invoke via Payload embedded headers. + context propagation for lambda invoke via Payload embedded headers. ([#458](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/458)) - + ## [0.21b0](https://github.com/open-telemetry/opentelemetry-python/releases/tag/v1.2.0-0.21b0) - 2021-05-11 ### Changed @@ -81,7 +83,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#436](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/436)) - `opentelemetry-instrumentation-grpc` Keep client interceptor in sync with grpc client interceptors. ([#442](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/442)) - + ### Removed - Remove `http.status_text` from span attributes ([#406](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/406)) diff --git a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py index c02ae50287..4848c86622 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py @@ -147,12 +147,12 @@ def get_default_span_details(scope: dict) -> Tuple[str, dict]: scope: the asgi scope dictionary Returns: - a tuple of the span, and any attributes to attach to the - span. + a tuple of the span name, and any attributes to attach to the span. """ method_or_path = scope.get("method") or scope.get("path") + span_name = "HTTP {}".format(method_or_path.strip()) - return method_or_path, {} + return span_name, {} class OpenTelemetryMiddleware: @@ -205,7 +205,7 @@ async def __call__(self, scope, receive, send): try: with self.tracer.start_as_current_span( - span_name + " asgi", kind=trace.SpanKind.SERVER, + span_name, kind=trace.SpanKind.SERVER, ) as span: if span.is_recording(): attributes = collect_request_attributes(scope) diff --git a/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py b/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py index 0a3b451b9c..5898e928e2 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py @@ -116,12 +116,12 @@ def validate_outputs(self, outputs, error=None, modifiers=None): self.assertEqual(len(span_list), 4) expected = [ { - "name": "GET asgi.http.receive", + "name": "HTTP GET asgi.http.receive", "kind": trace_api.SpanKind.INTERNAL, "attributes": {"type": "http.request"}, }, { - "name": "GET asgi.http.send", + "name": "HTTP GET asgi.http.send", "kind": trace_api.SpanKind.INTERNAL, "attributes": { SpanAttributes.HTTP_STATUS_CODE: 200, @@ -129,12 +129,12 @@ def validate_outputs(self, outputs, error=None, modifiers=None): }, }, { - "name": "GET asgi.http.send", + "name": "HTTP GET asgi.http.send", "kind": trace_api.SpanKind.INTERNAL, "attributes": {"type": "http.response.body"}, }, { - "name": "GET asgi", + "name": "HTTP GET", "kind": trace_api.SpanKind.SERVER, "attributes": { SpanAttributes.HTTP_METHOD: "GET", @@ -201,9 +201,12 @@ def get_predefined_span_details(_): def update_expected_span_name(expected): for entry in expected: - entry["name"] = " ".join( - [span_name] + entry["name"].split(" ")[-1:] - ) + if entry["kind"] == trace_api.SpanKind.SERVER: + entry["name"] = span_name + else: + entry["name"] = " ".join( + [span_name] + entry["name"].split(" ")[-1:] + ) return expected app = otel_asgi.OpenTelemetryMiddleware( @@ -305,17 +308,17 @@ def test_websocket(self): self.assertEqual(len(span_list), 6) expected = [ { - "name": "/ asgi.websocket.receive", + "name": "/ asgi.websocket receive", "kind": trace_api.SpanKind.INTERNAL, "attributes": {"type": "websocket.connect"}, }, { - "name": "/ asgi.websocket.send", + "name": "/ asgi.websocket send", "kind": trace_api.SpanKind.INTERNAL, "attributes": {"type": "websocket.accept"}, }, { - "name": "/ asgi.websocket.receive", + "name": "/ asgi.websocket receive", "kind": trace_api.SpanKind.INTERNAL, "attributes": { "type": "websocket.receive", @@ -323,7 +326,7 @@ def test_websocket(self): }, }, { - "name": "/ asgi.websocket.send", + "name": "/ asgi.websocket send", "kind": trace_api.SpanKind.INTERNAL, "attributes": { "type": "websocket.send", @@ -331,7 +334,7 @@ def test_websocket(self): }, }, { - "name": "/ asgi.websocket.receive", + "name": "/ asgi.websocket receive", "kind": trace_api.SpanKind.INTERNAL, "attributes": {"type": "websocket.disconnect"}, }, From 2fcc2b571071cb6b031c43b707ecb3186c591c5c Mon Sep 17 00:00:00 2001 From: Michael Manganiello Date: Tue, 6 Apr 2021 20:21:00 -0300 Subject: [PATCH 2/4] Apply review comments --- .../instrumentation/asgi/__init__.py | 7 +++---- .../tests/test_asgi_middleware.py | 20 +++++++++---------- 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py index 4848c86622..1e1b9dcf5a 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py @@ -149,8 +149,7 @@ def get_default_span_details(scope: dict) -> Tuple[str, dict]: Returns: a tuple of the span name, and any attributes to attach to the span. """ - method_or_path = scope.get("method") or scope.get("path") - span_name = "HTTP {}".format(method_or_path.strip()) + span_name = scope.get("path") or "HTTP {}".format(scope.get("method", "").strip()) return span_name, {} @@ -216,7 +215,7 @@ async def __call__(self, scope, receive, send): @wraps(receive) async def wrapped_receive(): with self.tracer.start_as_current_span( - span_name + " asgi." + scope["type"] + ".receive" + span_name + " " + scope["type"] + ".receive" ) as receive_span: message = await receive() if receive_span.is_recording(): @@ -228,7 +227,7 @@ async def wrapped_receive(): @wraps(send) async def wrapped_send(message): with self.tracer.start_as_current_span( - span_name + " asgi." + scope["type"] + ".send" + span_name + " " + scope["type"] + ".send" ) as send_span: if send_span.is_recording(): if message["type"] == "http.response.start": diff --git a/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py b/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py index 5898e928e2..3f5f44ddf6 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py @@ -116,12 +116,12 @@ def validate_outputs(self, outputs, error=None, modifiers=None): self.assertEqual(len(span_list), 4) expected = [ { - "name": "HTTP GET asgi.http.receive", + "name": "/ http.receive", "kind": trace_api.SpanKind.INTERNAL, "attributes": {"type": "http.request"}, }, { - "name": "HTTP GET asgi.http.send", + "name": "/ http.send", "kind": trace_api.SpanKind.INTERNAL, "attributes": { SpanAttributes.HTTP_STATUS_CODE: 200, @@ -129,12 +129,12 @@ def validate_outputs(self, outputs, error=None, modifiers=None): }, }, { - "name": "HTTP GET asgi.http.send", + "name": "/ http.send", "kind": trace_api.SpanKind.INTERNAL, "attributes": {"type": "http.response.body"}, }, { - "name": "HTTP GET", + "name": "/", "kind": trace_api.SpanKind.SERVER, "attributes": { SpanAttributes.HTTP_METHOD: "GET", @@ -308,17 +308,17 @@ def test_websocket(self): self.assertEqual(len(span_list), 6) expected = [ { - "name": "/ asgi.websocket receive", + "name": "/ websocket receive", "kind": trace_api.SpanKind.INTERNAL, "attributes": {"type": "websocket.connect"}, }, { - "name": "/ asgi.websocket send", + "name": "/ websocket send", "kind": trace_api.SpanKind.INTERNAL, "attributes": {"type": "websocket.accept"}, }, { - "name": "/ asgi.websocket receive", + "name": "/ websocket receive", "kind": trace_api.SpanKind.INTERNAL, "attributes": { "type": "websocket.receive", @@ -326,7 +326,7 @@ def test_websocket(self): }, }, { - "name": "/ asgi.websocket send", + "name": "/ websocket send", "kind": trace_api.SpanKind.INTERNAL, "attributes": { "type": "websocket.send", @@ -334,12 +334,12 @@ def test_websocket(self): }, }, { - "name": "/ asgi.websocket receive", + "name": "/ websocket receive", "kind": trace_api.SpanKind.INTERNAL, "attributes": {"type": "websocket.disconnect"}, }, { - "name": "/ asgi", + "name": "/", "kind": trace_api.SpanKind.SERVER, "attributes": { SpanAttributes.HTTP_SCHEME: self.scope["scheme"], From 4ee5d3fc0adf452687e79a239359654467d4dc4e Mon Sep 17 00:00:00 2001 From: Michael Manganiello Date: Tue, 6 Apr 2021 20:39:19 -0300 Subject: [PATCH 3/4] Fix linting issues --- .../src/opentelemetry/instrumentation/asgi/__init__.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py index 1e1b9dcf5a..bc32a09794 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py @@ -149,7 +149,9 @@ def get_default_span_details(scope: dict) -> Tuple[str, dict]: Returns: a tuple of the span name, and any attributes to attach to the span. """ - span_name = scope.get("path") or "HTTP {}".format(scope.get("method", "").strip()) + span_name = scope.get("path", "").strip() or "HTTP {}".format( + scope.get("method", "").strip() + ) return span_name, {} From c977e2f456b6e2a1b47326ee5c88c2f1217bdfa1 Mon Sep 17 00:00:00 2001 From: Michael Manganiello Date: Mon, 24 May 2021 22:23:52 -0300 Subject: [PATCH 4/4] Replace dots with spaces to separate destination and operation names --- CHANGELOG.md | 2 +- .../src/opentelemetry/instrumentation/asgi/__init__.py | 4 ++-- .../tests/test_asgi_middleware.py | 8 ++++---- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 11bc269669..70bf019fd1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,7 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `opentelemetry-instrumentation-tornado` Fixed cases where description was used with non- error status code when creating Status objects. ([#504](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/504)) -- Fix ASGI instrumentation default span name. +- `opentelemetry-instrumentation-asgi` Fix instrumentation default span name. ([#418](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/418)) ### Added diff --git a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py index bc32a09794..97fb4e1bb4 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py @@ -217,7 +217,7 @@ async def __call__(self, scope, receive, send): @wraps(receive) async def wrapped_receive(): with self.tracer.start_as_current_span( - span_name + " " + scope["type"] + ".receive" + " ".join((span_name, scope["type"], "receive")) ) as receive_span: message = await receive() if receive_span.is_recording(): @@ -229,7 +229,7 @@ async def wrapped_receive(): @wraps(send) async def wrapped_send(message): with self.tracer.start_as_current_span( - span_name + " " + scope["type"] + ".send" + " ".join((span_name, scope["type"], "send")) ) as send_span: if send_span.is_recording(): if message["type"] == "http.response.start": diff --git a/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py b/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py index 3f5f44ddf6..3ed438aa02 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py @@ -116,12 +116,12 @@ def validate_outputs(self, outputs, error=None, modifiers=None): self.assertEqual(len(span_list), 4) expected = [ { - "name": "/ http.receive", + "name": "/ http receive", "kind": trace_api.SpanKind.INTERNAL, "attributes": {"type": "http.request"}, }, { - "name": "/ http.send", + "name": "/ http send", "kind": trace_api.SpanKind.INTERNAL, "attributes": { SpanAttributes.HTTP_STATUS_CODE: 200, @@ -129,7 +129,7 @@ def validate_outputs(self, outputs, error=None, modifiers=None): }, }, { - "name": "/ http.send", + "name": "/ http send", "kind": trace_api.SpanKind.INTERNAL, "attributes": {"type": "http.response.body"}, }, @@ -205,7 +205,7 @@ def update_expected_span_name(expected): entry["name"] = span_name else: entry["name"] = " ".join( - [span_name] + entry["name"].split(" ")[-1:] + [span_name] + entry["name"].split(" ")[1:] ) return expected