From 743ac64661897087f2dca7f696e481648eb4d0fe Mon Sep 17 00:00:00 2001 From: Maciej Nachtygal <82878433+macieyng@users.noreply.github.com> Date: Fri, 16 Jun 2023 00:21:05 +0200 Subject: [PATCH] Issue #1757 - Update HTTP server/client instrumentation span names (#1759) Co-authored-by: Shalev Roda <65566801+shalevr@users.noreply.github.com> Co-authored-by: Srikanth Chekuri --- CHANGELOG.md | 4 ++ .../aiohttp_client/__init__.py | 2 +- .../tests/test_aiohttp_client_integration.py | 13 ++++--- .../instrumentation/asgi/__init__.py | 19 ++++++---- .../tests/test_asgi_middleware.py | 14 +++---- .../django/middleware/otel_middleware.py | 12 ++---- .../tests/test_middleware.py | 26 ++++--------- .../tests/test_middleware_asgi.py | 14 +++---- .../tests/test_falcon.py | 2 +- .../instrumentation/fastapi/__init__.py | 37 +++++++++++++++---- .../tests/test_fastapi_instrumentation.py | 6 +-- .../tests/test_programmatic.py | 2 +- .../instrumentation/httpx/__init__.py | 2 +- .../tests/test_httpx_integration.py | 8 ++-- .../tests/test_programmatic.py | 2 +- .../instrumentation/requests/__init__.py | 12 +++++- .../tests/test_requests_integration.py | 4 +- .../tests/test_requests_ip_support.py | 2 +- .../instrumentation/starlette/__init__.py | 37 +++++++++++++++---- .../tests/test_starlette_instrumentation.py | 4 +- .../instrumentation/tornado/__init__.py | 23 +++++++++--- .../tests/test_instrumentation.py | 23 ++++++------ .../instrumentation/urllib/__init__.py | 2 +- .../tests/test_urllib_integration.py | 4 +- .../instrumentation/urllib3/__init__.py | 2 +- .../tests/test_urllib3_integration.py | 2 +- .../tests/test_urllib3_ip_support.py | 2 +- .../instrumentation/wsgi/__init__.py | 17 ++++++++- .../tests/test_wsgi_middleware.py | 13 ++++--- .../opentelemetry/instrumentation/utils.py | 2 +- 30 files changed, 194 insertions(+), 118 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 777b5a3530..9959a49d01 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -51,6 +51,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Fix Flask instrumentation to only close the span if it was created by the same request context. ([#1692](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1692)) +### Changed +- Update HTTP server/client instrumentation span names to comply with spec + ([#1759](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1759) + ## Version 1.17.0/0.38b0 (2023-03-22) ### Added diff --git a/instrumentation/opentelemetry-instrumentation-aiohttp-client/src/opentelemetry/instrumentation/aiohttp_client/__init__.py b/instrumentation/opentelemetry-instrumentation-aiohttp-client/src/opentelemetry/instrumentation/aiohttp_client/__init__.py index 101e67f2ad..65e1601f34 100644 --- a/instrumentation/opentelemetry-instrumentation-aiohttp-client/src/opentelemetry/instrumentation/aiohttp_client/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-aiohttp-client/src/opentelemetry/instrumentation/aiohttp_client/__init__.py @@ -179,7 +179,7 @@ async def on_request_start( return http_method = params.method.upper() - request_span_name = f"HTTP {http_method}" + request_span_name = f"{http_method}" request_url = ( remove_url_credentials(trace_config_ctx.url_filter(params.url)) if callable(trace_config_ctx.url_filter) diff --git a/instrumentation/opentelemetry-instrumentation-aiohttp-client/tests/test_aiohttp_client_integration.py b/instrumentation/opentelemetry-instrumentation-aiohttp-client/tests/test_aiohttp_client_integration.py index 7c3d7b634d..6af9d41900 100644 --- a/instrumentation/opentelemetry-instrumentation-aiohttp-client/tests/test_aiohttp_client_integration.py +++ b/instrumentation/opentelemetry-instrumentation-aiohttp-client/tests/test_aiohttp_client_integration.py @@ -119,7 +119,7 @@ def test_status_codes(self): self.assert_spans( [ ( - "HTTP GET", + "GET", (span_status, None), { SpanAttributes.HTTP_METHOD: "GET", @@ -213,7 +213,7 @@ def strip_query_params(url: yarl.URL) -> str: self.assert_spans( [ ( - "HTTP GET", + "GET", (StatusCode.UNSET, None), { SpanAttributes.HTTP_METHOD: "GET", @@ -247,7 +247,7 @@ async def do_request(url): self.assert_spans( [ ( - "HTTP GET", + "GET", (expected_status, None), { SpanAttributes.HTTP_METHOD: "GET", @@ -274,7 +274,7 @@ async def request_handler(request): self.assert_spans( [ ( - "HTTP GET", + "GET", (StatusCode.ERROR, None), { SpanAttributes.HTTP_METHOD: "GET", @@ -301,7 +301,7 @@ async def request_handler(request): self.assert_spans( [ ( - "HTTP GET", + "GET", (StatusCode.ERROR, None), { SpanAttributes.HTTP_METHOD: "GET", @@ -338,7 +338,7 @@ async def do_request(url): self.assert_spans( [ ( - "HTTP GET", + "GET", (StatusCode.UNSET, None), { SpanAttributes.HTTP_METHOD: "GET", @@ -391,6 +391,7 @@ def test_instrument(self): self.get_default_request(), self.URL, self.default_handler ) span = self.assert_spans(1) + self.assertEqual("GET", span.name) self.assertEqual("GET", span.attributes[SpanAttributes.HTTP_METHOD]) self.assertEqual( f"http://{host}:{port}/test-path", 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 6fc88d3eeb..010c6accde 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py @@ -415,18 +415,23 @@ def set_status_code(span, status_code): def get_default_span_details(scope: dict) -> Tuple[str, dict]: - """Default implementation for get_default_span_details + """ + Default span name is the HTTP method and URL path, or just the method. + https://github.com/open-telemetry/opentelemetry-specification/pull/3165 + https://opentelemetry.io/docs/reference/specification/trace/semantic_conventions/http/#name + Args: scope: the ASGI scope dictionary Returns: a tuple of the span name, and any attributes to attach to the span. """ - span_name = ( - scope.get("path", "").strip() - or f"HTTP {scope.get('method', '').strip()}" - ) - - return span_name, {} + path = scope.get("path", "").strip() + method = scope.get("method", "").strip() + if method and path: # http + return f"{method} {path}", {} + if path: # websocket + return path, {} + return method, {} # http with no path def _collect_target_attribute( diff --git a/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py b/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py index 7cbae79c6d..f9a5731fd3 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py @@ -142,12 +142,12 @@ def validate_outputs(self, outputs, error=None, modifiers=None): self.assertEqual(len(span_list), 4) expected = [ { - "name": "/ http receive", + "name": "GET / http receive", "kind": trace_api.SpanKind.INTERNAL, "attributes": {"type": "http.request"}, }, { - "name": "/ http send", + "name": "GET / http send", "kind": trace_api.SpanKind.INTERNAL, "attributes": { SpanAttributes.HTTP_STATUS_CODE: 200, @@ -155,12 +155,12 @@ def validate_outputs(self, outputs, error=None, modifiers=None): }, }, { - "name": "/ http send", + "name": "GET / http send", "kind": trace_api.SpanKind.INTERNAL, "attributes": {"type": "http.response.body"}, }, { - "name": "/", + "name": "GET /", "kind": trace_api.SpanKind.SERVER, "attributes": { SpanAttributes.HTTP_METHOD: "GET", @@ -231,7 +231,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(" ")[2:] ) return expected @@ -493,9 +493,9 @@ def update_expected_hook_results(expected): for entry in expected: if entry["kind"] == trace_api.SpanKind.SERVER: entry["name"] = "name from server hook" - elif entry["name"] == "/ http receive": + elif entry["name"] == "GET / http receive": entry["name"] = "name from client request hook" - elif entry["name"] == "/ http send": + elif entry["name"] == "GET / http send": entry["attributes"].update({"attr-from-hook": "value"}) return expected diff --git a/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware/otel_middleware.py b/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware/otel_middleware.py index 1baa05eca9..02313a48ee 100644 --- a/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware/otel_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware/otel_middleware.py @@ -173,18 +173,12 @@ def _get_span_name(request): match = resolve(request.path) if hasattr(match, "route"): - return match.route + return f"{request.method} {match.route}" - # Instead of using `view_name`, better to use `_func_name` as some applications can use similar - # view names in different modules - if hasattr(match, "_func_name"): - return match._func_name # pylint: disable=protected-access - - # Fallback for safety as `_func_name` private field - return match.view_name + return request.method except Resolver404: - return f"HTTP {request.method}" + return request.method # pylint: disable=too-many-locals def process_request(self, request): diff --git a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py index e235c8840e..1f28819df0 100644 --- a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py @@ -150,9 +150,9 @@ def test_templated_route_get(self): self.assertEqual( span.name, - "^route/(?P[0-9]{4})/template/$" + "GET ^route/(?P[0-9]{4})/template/$" if DJANGO_2_2 - else "tests.views.traced_template", + else "GET", ) self.assertEqual(span.kind, SpanKind.SERVER) self.assertEqual(span.status.status_code, StatusCode.UNSET) @@ -177,9 +177,7 @@ def test_traced_get(self): span = spans[0] - self.assertEqual( - span.name, "^traced/" if DJANGO_2_2 else "tests.views.traced" - ) + self.assertEqual(span.name, "GET ^traced/" if DJANGO_2_2 else "GET") self.assertEqual(span.kind, SpanKind.SERVER) self.assertEqual(span.status.status_code, StatusCode.UNSET) self.assertEqual(span.attributes[SpanAttributes.HTTP_METHOD], "GET") @@ -215,9 +213,7 @@ def test_traced_post(self): span = spans[0] - self.assertEqual( - span.name, "^traced/" if DJANGO_2_2 else "tests.views.traced" - ) + self.assertEqual(span.name, "POST ^traced/" if DJANGO_2_2 else "POST") self.assertEqual(span.kind, SpanKind.SERVER) self.assertEqual(span.status.status_code, StatusCode.UNSET) self.assertEqual(span.attributes[SpanAttributes.HTTP_METHOD], "POST") @@ -241,9 +237,7 @@ def test_error(self): span = spans[0] - self.assertEqual( - span.name, "^error/" if DJANGO_2_2 else "tests.views.error" - ) + self.assertEqual(span.name, "GET ^error/" if DJANGO_2_2 else "GET") self.assertEqual(span.kind, SpanKind.SERVER) self.assertEqual(span.status.status_code, StatusCode.ERROR) self.assertEqual(span.attributes[SpanAttributes.HTTP_METHOD], "GET") @@ -307,9 +301,7 @@ def test_span_name(self): span = span_list[0] self.assertEqual( span.name, - "^span_name/([0-9]{4})/$" - if DJANGO_2_2 - else "tests.views.route_span_name", + "GET ^span_name/([0-9]{4})/$" if DJANGO_2_2 else "GET", ) def test_span_name_for_query_string(self): @@ -323,9 +315,7 @@ def test_span_name_for_query_string(self): span = span_list[0] self.assertEqual( span.name, - "^span_name/([0-9]{4})/$" - if DJANGO_2_2 - else "tests.views.route_span_name", + "GET ^span_name/([0-9]{4})/$" if DJANGO_2_2 else "GET", ) def test_span_name_404(self): @@ -334,7 +324,7 @@ def test_span_name_404(self): self.assertEqual(len(span_list), 1) span = span_list[0] - self.assertEqual(span.name, "HTTP GET") + self.assertEqual(span.name, "GET") def test_traced_request_attrs(self): Client().get("/span_name/1234/", CONTENT_TYPE="test/ct") diff --git a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware_asgi.py b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware_asgi.py index a78501bcb8..0e2472d15e 100644 --- a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware_asgi.py +++ b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware_asgi.py @@ -137,7 +137,7 @@ async def test_templated_route_get(self): span = spans[0] - self.assertEqual(span.name, "^route/(?P[0-9]{4})/template/$") + self.assertEqual(span.name, "GET ^route/(?P[0-9]{4})/template/$") self.assertEqual(span.kind, SpanKind.SERVER) self.assertEqual(span.status.status_code, StatusCode.UNSET) self.assertEqual(span.attributes[SpanAttributes.HTTP_METHOD], "GET") @@ -160,7 +160,7 @@ async def test_traced_get(self): span = spans[0] - self.assertEqual(span.name, "^traced/") + self.assertEqual(span.name, "GET ^traced/") self.assertEqual(span.kind, SpanKind.SERVER) self.assertEqual(span.status.status_code, StatusCode.UNSET) self.assertEqual(span.attributes[SpanAttributes.HTTP_METHOD], "GET") @@ -195,7 +195,7 @@ async def test_traced_post(self): span = spans[0] - self.assertEqual(span.name, "^traced/") + self.assertEqual(span.name, "POST ^traced/") self.assertEqual(span.kind, SpanKind.SERVER) self.assertEqual(span.status.status_code, StatusCode.UNSET) self.assertEqual(span.attributes[SpanAttributes.HTTP_METHOD], "POST") @@ -218,7 +218,7 @@ async def test_error(self): span = spans[0] - self.assertEqual(span.name, "^error/") + self.assertEqual(span.name, "GET ^error/") self.assertEqual(span.kind, SpanKind.SERVER) self.assertEqual(span.status.status_code, StatusCode.ERROR) self.assertEqual(span.attributes[SpanAttributes.HTTP_METHOD], "GET") @@ -264,7 +264,7 @@ async def test_span_name(self): self.assertEqual(len(span_list), 1) span = span_list[0] - self.assertEqual(span.name, "^span_name/([0-9]{4})/$") + self.assertEqual(span.name, "GET ^span_name/([0-9]{4})/$") async def test_span_name_for_query_string(self): """ @@ -275,7 +275,7 @@ async def test_span_name_for_query_string(self): self.assertEqual(len(span_list), 1) span = span_list[0] - self.assertEqual(span.name, "^span_name/([0-9]{4})/$") + self.assertEqual(span.name, "GET ^span_name/([0-9]{4})/$") async def test_span_name_404(self): await self.async_client.get("/span_name/1234567890/") @@ -283,7 +283,7 @@ async def test_span_name_404(self): self.assertEqual(len(span_list), 1) span = span_list[0] - self.assertEqual(span.name, "HTTP GET") + self.assertEqual(span.name, "GET") async def test_traced_request_attrs(self): await self.async_client.get("/span_name/1234/", CONTENT_TYPE="test/ct") diff --git a/instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py b/instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py index aeba57a9b5..cf61a9fd3c 100644 --- a/instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py +++ b/instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py @@ -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 /does-not-exist") self.assertEqual(span.status.status_code, StatusCode.UNSET) self.assertSpanHasAttributes( span, diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py index 934c11e110..e99c8be6ed 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py @@ -227,7 +227,7 @@ def instrument_app( app.add_middleware( OpenTelemetryMiddleware, excluded_urls=excluded_urls, - default_span_details=_get_route_details, + default_span_details=_get_default_span_details, server_request_hook=server_request_hook, client_request_hook=client_request_hook, client_response_hook=client_response_hook, @@ -300,7 +300,7 @@ def __init__(self, *args, **kwargs): self.add_middleware( OpenTelemetryMiddleware, excluded_urls=_InstrumentedFastAPI._excluded_urls, - default_span_details=_get_route_details, + default_span_details=_get_default_span_details, server_request_hook=_InstrumentedFastAPI._server_request_hook, client_request_hook=_InstrumentedFastAPI._client_request_hook, client_response_hook=_InstrumentedFastAPI._client_response_hook, @@ -316,15 +316,21 @@ def __del__(self): def _get_route_details(scope): - """Callback to retrieve the fastapi route being served. + """ + Function to retrieve Starlette route from scope. TODO: there is currently no way to retrieve http.route from a starlette application from scope. - See: https://github.com/encode/starlette/pull/804 + + Args: + scope: A Starlette scope + Returns: + A string containing the route or None """ app = scope["app"] route = None + for starlette_route in app.routes: match, _ = starlette_route.matches(scope) if match == Match.FULL: @@ -332,10 +338,27 @@ def _get_route_details(scope): break if match == Match.PARTIAL: route = starlette_route.path - # method only exists for http, if websocket - # leave it blank. - span_name = route or scope.get("method", "") + return route + + +def _get_default_span_details(scope): + """ + Callback to retrieve span name and attributes from scope. + + Args: + scope: A Starlette scope + Returns: + A tuple of span name and attributes + """ + route = _get_route_details(scope) + method = scope.get("method", "") attributes = {} if route: attributes[SpanAttributes.HTTP_ROUTE] = route + if method and route: # http + span_name = f"{method} {route}" + elif route: # websocket + span_name = route + else: # fallback + span_name = method return span_name, attributes diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py index 261b2e025f..9420ba2c0e 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py @@ -106,7 +106,7 @@ def test_instrument_app_with_instrument(self): spans = self.memory_exporter.get_finished_spans() self.assertEqual(len(spans), 3) for span in spans: - self.assertIn("/foobar", span.name) + self.assertIn("GET /foobar", span.name) def test_uninstrument_app(self): self._client.get("/foobar") @@ -138,7 +138,7 @@ def test_basic_fastapi_call(self): spans = self.memory_exporter.get_finished_spans() self.assertEqual(len(spans), 3) for span in spans: - self.assertIn("/foobar", span.name) + self.assertIn("GET /foobar", span.name) def test_fastapi_route_attribute_added(self): """Ensure that fastapi routes are used as the span name.""" @@ -146,7 +146,7 @@ def test_fastapi_route_attribute_added(self): spans = self.memory_exporter.get_finished_spans() self.assertEqual(len(spans), 3) for span in spans: - self.assertIn("/user/{username}", span.name) + self.assertIn("GET /user/{username}", span.name) self.assertEqual( spans[-1].attributes[SpanAttributes.HTTP_ROUTE], "/user/{username}" ) diff --git a/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py b/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py index 8c231b1d08..6393b927b8 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py +++ b/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py @@ -214,7 +214,7 @@ def test_404(self): resp.close() span_list = self.memory_exporter.get_finished_spans() self.assertEqual(len(span_list), 1) - self.assertEqual(span_list[0].name, "HTTP POST") + self.assertEqual(span_list[0].name, "POST /bye") self.assertEqual(span_list[0].kind, trace.SpanKind.SERVER) self.assertEqual(span_list[0].attributes, expected_attrs) diff --git a/instrumentation/opentelemetry-instrumentation-httpx/src/opentelemetry/instrumentation/httpx/__init__.py b/instrumentation/opentelemetry-instrumentation-httpx/src/opentelemetry/instrumentation/httpx/__init__.py index 736e6c3d32..bb40adbc26 100644 --- a/instrumentation/opentelemetry-instrumentation-httpx/src/opentelemetry/instrumentation/httpx/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-httpx/src/opentelemetry/instrumentation/httpx/__init__.py @@ -209,7 +209,7 @@ class ResponseInfo(typing.NamedTuple): def _get_default_span_name(method: str) -> str: - return f"HTTP {method.strip()}" + return method.strip() def _apply_status_code(span: Span, status_code: int) -> None: diff --git a/instrumentation/opentelemetry-instrumentation-httpx/tests/test_httpx_integration.py b/instrumentation/opentelemetry-instrumentation-httpx/tests/test_httpx_integration.py index 86c9a56ab2..daddaad306 100644 --- a/instrumentation/opentelemetry-instrumentation-httpx/tests/test_httpx_integration.py +++ b/instrumentation/opentelemetry-instrumentation-httpx/tests/test_httpx_integration.py @@ -142,7 +142,7 @@ def test_basic(self): span = self.assert_span() self.assertIs(span.kind, trace.SpanKind.CLIENT) - self.assertEqual(span.name, "HTTP GET") + self.assertEqual(span.name, "GET") self.assertEqual( span.attributes, @@ -258,7 +258,7 @@ def test_invalid_url(self): span = self.assert_span() - self.assertEqual(span.name, "HTTP POST") + self.assertEqual(span.name, "POST") self.assertEqual( span.attributes[SpanAttributes.HTTP_METHOD], "POST" ) @@ -350,7 +350,7 @@ def test_request_hook_no_span_change(self): self.assertEqual(result.text, "Hello!") span = self.assert_span() - self.assertEqual(span.name, "HTTP GET") + self.assertEqual(span.name, "GET") def test_not_recording(self): with mock.patch("opentelemetry.trace.INVALID_SPAN") as mock_span: @@ -444,7 +444,7 @@ def test_request_hook_no_span_update(self): self.assertEqual(result.text, "Hello!") span = self.assert_span() - self.assertEqual(span.name, "HTTP GET") + self.assertEqual(span.name, "GET") HTTPXClientInstrumentor().uninstrument() def test_not_recording(self): diff --git a/instrumentation/opentelemetry-instrumentation-pyramid/tests/test_programmatic.py b/instrumentation/opentelemetry-instrumentation-pyramid/tests/test_programmatic.py index d3a4fa91db..478eab1937 100644 --- a/instrumentation/opentelemetry-instrumentation-pyramid/tests/test_programmatic.py +++ b/instrumentation/opentelemetry-instrumentation-pyramid/tests/test_programmatic.py @@ -145,7 +145,7 @@ def test_404(self): resp.close() span_list = self.memory_exporter.get_finished_spans() self.assertEqual(len(span_list), 1) - self.assertEqual(span_list[0].name, "HTTP POST") + self.assertEqual(span_list[0].name, "POST /bye") self.assertEqual(span_list[0].kind, trace.SpanKind.SERVER) self.assertEqual(span_list[0].attributes, expected_attrs) diff --git a/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py b/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py index e5bb24223c..c3dabf05a5 100644 --- a/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py @@ -245,8 +245,16 @@ def _uninstrument_from(instr_root, restore_as_bound_func=False): def get_default_span_name(method): - """Default implementation for name_callback, returns HTTP {method_name}.""" - return f"HTTP {method.strip()}" + """ + Default implementation for name_callback, returns HTTP {method_name}. + https://opentelemetry.io/docs/reference/specification/trace/semantic_conventions/http/#name + + Args: + method: string representing HTTP method + Returns: + span name + """ + return method.strip() class RequestsInstrumentor(BaseInstrumentor): diff --git a/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py b/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py index 6e6a68cb8f..3bd76a6995 100644 --- a/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py +++ b/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py @@ -116,7 +116,7 @@ def test_basic(self): span = self.assert_span() self.assertIs(span.kind, trace.SpanKind.CLIENT) - self.assertEqual(span.name, "HTTP GET") + self.assertEqual(span.name, "GET") self.assertEqual( span.attributes, @@ -191,7 +191,7 @@ def name_callback(method, url): self.assertEqual(result.text, "Hello!") span = self.assert_span() - self.assertEqual(span.name, "HTTP GET") + self.assertEqual(span.name, "GET") def test_not_foundbasic(self): url_404 = "http://mock/status/404" diff --git a/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_ip_support.py b/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_ip_support.py index 593ed92fe9..cf2e7fb4dd 100644 --- a/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_ip_support.py +++ b/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_ip_support.py @@ -71,7 +71,7 @@ def assert_success_span(self, response: requests.Response): span = self.assert_span() self.assertIs(trace.SpanKind.CLIENT, span.kind) - self.assertEqual("HTTP GET", span.name) + self.assertEqual("GET", span.name) attributes = { "http.status_code": 200, diff --git a/instrumentation/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/starlette/__init__.py b/instrumentation/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/starlette/__init__.py index 30f24cc65c..2d123aa70e 100644 --- a/instrumentation/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/starlette/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/starlette/__init__.py @@ -212,7 +212,7 @@ def instrument_app( app.add_middleware( OpenTelemetryMiddleware, excluded_urls=_excluded_urls, - default_span_details=_get_route_details, + default_span_details=_get_default_span_details, server_request_hook=server_request_hook, client_request_hook=client_request_hook, client_response_hook=client_response_hook, @@ -278,7 +278,7 @@ def __init__(self, *args, **kwargs): self.add_middleware( OpenTelemetryMiddleware, excluded_urls=_excluded_urls, - default_span_details=_get_route_details, + default_span_details=_get_default_span_details, server_request_hook=_InstrumentedStarlette._server_request_hook, client_request_hook=_InstrumentedStarlette._client_request_hook, client_response_hook=_InstrumentedStarlette._client_response_hook, @@ -294,15 +294,21 @@ def __del__(self): def _get_route_details(scope): - """Callback to retrieve the starlette route being served. + """ + Function to retrieve Starlette route from scope. TODO: there is currently no way to retrieve http.route from a starlette application from scope. - See: https://github.com/encode/starlette/pull/804 + + Args: + scope: A Starlette scope + Returns: + A string containing the route or None """ app = scope["app"] route = None + for starlette_route in app.routes: match, _ = starlette_route.matches(scope) if match == Match.FULL: @@ -310,10 +316,27 @@ def _get_route_details(scope): break if match == Match.PARTIAL: route = starlette_route.path - # method only exists for http, if websocket - # leave it blank. - span_name = route or scope.get("method", "") + return route + + +def _get_default_span_details(scope): + """ + Callback to retrieve span name and attributes from scope. + + Args: + scope: A Starlette scope + Returns: + A tuple of span name and attributes + """ + route = _get_route_details(scope) + method = scope.get("method", "") attributes = {} if route: attributes[SpanAttributes.HTTP_ROUTE] = route + if method and route: # http + span_name = f"{method} {route}" + elif route: # websocket + span_name = route + else: # fallback + span_name = method return span_name, attributes diff --git a/instrumentation/opentelemetry-instrumentation-starlette/tests/test_starlette_instrumentation.py b/instrumentation/opentelemetry-instrumentation-starlette/tests/test_starlette_instrumentation.py index 9c658e0092..1f4570d293 100644 --- a/instrumentation/opentelemetry-instrumentation-starlette/tests/test_starlette_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-starlette/tests/test_starlette_instrumentation.py @@ -93,7 +93,7 @@ def test_basic_starlette_call(self): spans = self.memory_exporter.get_finished_spans() self.assertEqual(len(spans), 3) for span in spans: - self.assertIn("/foobar", span.name) + self.assertIn("GET /foobar", span.name) def test_starlette_route_attribute_added(self): """Ensure that starlette routes are used as the span name.""" @@ -101,7 +101,7 @@ def test_starlette_route_attribute_added(self): spans = self.memory_exporter.get_finished_spans() self.assertEqual(len(spans), 3) for span in spans: - self.assertIn("/user/{username}", span.name) + self.assertIn("GET /user/{username}", span.name) self.assertEqual( spans[-1].attributes[SpanAttributes.HTTP_ROUTE], "/user/{username}" ) diff --git a/instrumentation/opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/__init__.py b/instrumentation/opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/__init__.py index dd8da74f3f..1e2f0e5162 100644 --- a/instrumentation/opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/__init__.py @@ -454,10 +454,23 @@ def _get_attributes_from_request(request): ) -def _get_operation_name(handler, request): - full_class_name = type(handler).__name__ - class_name = full_class_name.rsplit(".")[-1] - return f"{class_name}.{request.method.lower()}" +def _get_default_span_name(request): + """ + Default span name is the HTTP method and URL path, or just the method. + https://github.com/open-telemetry/opentelemetry-specification/pull/3165 + https://opentelemetry.io/docs/reference/specification/trace/semantic_conventions/http/#name + + Args: + request: Tornado request object. + Returns: + Default span name. + """ + + path = request.path + method = request.method + if method and path: + return f"{method} {path}" + return f"{method}" def _get_full_handler_name(handler): @@ -468,7 +481,7 @@ def _get_full_handler_name(handler): def _start_span(tracer, handler) -> _TraceContext: span, token = _start_internal_or_server_span( tracer=tracer, - span_name=_get_operation_name(handler, handler.request), + span_name=_get_default_span_name(handler.request), start_time=time_ns(), context_carrier=handler.request.headers, context_getter=textmap.default_getter, diff --git a/instrumentation/opentelemetry-instrumentation-tornado/tests/test_instrumentation.py b/instrumentation/opentelemetry-instrumentation-tornado/tests/test_instrumentation.py index c875a331ef..0baaa348ab 100644 --- a/instrumentation/opentelemetry-instrumentation-tornado/tests/test_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-tornado/tests/test_instrumentation.py @@ -136,7 +136,7 @@ def _test_http_method_call(self, method): self.assertEqual(manual.parent, server.context) self.assertEqual(manual.context.trace_id, client.context.trace_id) - self.assertEqual(server.name, "MainHandler." + method.lower()) + self.assertEqual(server.name, f"{method} /") self.assertTrue(server.parent.is_remote) self.assertNotEqual(server.parent, client.context) self.assertEqual(server.parent.span_id, client.context.span_id) @@ -197,7 +197,7 @@ def _test_async_handler(self, url, handler_name): self.assertEqual(len(spans), 5) client = spans.by_name("GET") - server = spans.by_name(handler_name + ".get") + server = spans.by_name(f"GET {url}") sub_wrapper = spans.by_name("sub-task-wrapper") sub2 = spans.by_name("sub-task-2") @@ -214,7 +214,7 @@ def _test_async_handler(self, url, handler_name): self.assertEqual(sub_wrapper.parent, server.context) self.assertEqual(sub_wrapper.context.trace_id, client.context.trace_id) - self.assertEqual(server.name, handler_name + ".get") + self.assertEqual(server.name, f"GET {url}") self.assertTrue(server.parent.is_remote) self.assertNotEqual(server.parent, client.context) self.assertEqual(server.parent.span_id, client.context.span_id) @@ -230,6 +230,7 @@ def _test_async_handler(self, url, handler_name): SpanAttributes.HTTP_TARGET: url, SpanAttributes.HTTP_CLIENT_IP: "127.0.0.1", SpanAttributes.HTTP_STATUS_CODE: 201, + "tornado.handler": f"tests.tornado_test_app.{handler_name}", }, ) @@ -254,9 +255,9 @@ def test_500(self): self.assertEqual(len(spans), 2) client = spans.by_name("GET") - server = spans.by_name("BadHandler.get") + server = spans.by_name("GET /error") - self.assertEqual(server.name, "BadHandler.get") + self.assertEqual(server.name, "GET /error") self.assertEqual(server.kind, SpanKind.SERVER) self.assertSpanHasAttributes( server, @@ -291,7 +292,7 @@ def test_404(self): self.assertEqual(len(spans), 2) server, client = spans - self.assertEqual(server.name, "ErrorHandler.get") + self.assertEqual(server.name, "GET /missing-url") self.assertEqual(server.kind, SpanKind.SERVER) self.assertSpanHasAttributes( server, @@ -326,7 +327,7 @@ def test_http_error(self): self.assertEqual(len(spans), 2) server, client = spans - self.assertEqual(server.name, "RaiseHTTPErrorHandler.get") + self.assertEqual(server.name, "GET /raise_403") self.assertEqual(server.kind, SpanKind.SERVER) self.assertSpanHasAttributes( server, @@ -367,7 +368,7 @@ def test_dynamic_handler(self): self.assertEqual(len(spans), 2) server, client = spans - self.assertEqual(server.name, "DynamicHandler.get") + self.assertEqual(server.name, "GET /dyna") self.assertTrue(server.parent.is_remote) self.assertNotEqual(server.parent, client.context) self.assertEqual(server.parent.span_id, client.context.span_id) @@ -408,7 +409,7 @@ def test_handler_on_finish(self): self.assertEqual(len(spans), 3) auditor, server, client = spans - self.assertEqual(server.name, "FinishedHandler.get") + self.assertEqual(server.name, "GET /on_finish") self.assertTrue(server.parent.is_remote) self.assertNotEqual(server.parent, client.context) self.assertEqual(server.parent.span_id, client.context.span_id) @@ -535,7 +536,7 @@ def test_xheaders(self): self.assertEqual(response.code, 201) spans = self.get_finished_spans() self.assertSpanHasAttributes( - spans.by_name("MainHandler.get"), + spans.by_name("GET /"), { SpanAttributes.HTTP_METHOD: "GET", SpanAttributes.HTTP_SCHEME: "http", @@ -609,7 +610,7 @@ def test_uninstrument(self): self.assertEqual(len(spans), 3) manual, server, client = self.sorted_spans(spans) self.assertEqual(manual.name, "manual") - self.assertEqual(server.name, "MainHandler.get") + self.assertEqual(server.name, "GET /") self.assertEqual(client.name, "GET") self.memory_exporter.clear() diff --git a/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py b/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py index 091ccf99b1..cdd35a0bad 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py @@ -207,7 +207,7 @@ def _instrumented_open_call( method = request.get_method().upper() - span_name = f"HTTP {method}".strip() + span_name = method.strip() url = remove_url_credentials(url) diff --git a/instrumentation/opentelemetry-instrumentation-urllib/tests/test_urllib_integration.py b/instrumentation/opentelemetry-instrumentation-urllib/tests/test_urllib_integration.py index 35e9e1f1c7..f27f594a30 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib/tests/test_urllib_integration.py +++ b/instrumentation/opentelemetry-instrumentation-urllib/tests/test_urllib_integration.py @@ -124,7 +124,7 @@ def test_basic(self): span = self.assert_span() self.assertIs(span.kind, trace.SpanKind.CLIENT) - self.assertEqual(span.name, "HTTP GET") + self.assertEqual(span.name, "GET") self.assertEqual( span.attributes, @@ -209,7 +209,7 @@ def test_response_code_none(self): span = self.assert_span() self.assertIs(span.kind, trace.SpanKind.CLIENT) - self.assertEqual(span.name, "HTTP GET") + self.assertEqual(span.name, "GET") self.assertEqual( span.attributes, diff --git a/instrumentation/opentelemetry-instrumentation-urllib3/src/opentelemetry/instrumentation/urllib3/__init__.py b/instrumentation/opentelemetry-instrumentation-urllib3/src/opentelemetry/instrumentation/urllib3/__init__.py index 91d5576fc0..809f9b595f 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib3/src/opentelemetry/instrumentation/urllib3/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-urllib3/src/opentelemetry/instrumentation/urllib3/__init__.py @@ -225,7 +225,7 @@ def instrumented_urlopen(wrapped, instance, args, kwargs): headers = _prepare_headers(kwargs) body = _get_url_open_arg("body", args, kwargs) - span_name = f"HTTP {method.strip()}" + span_name = method.strip() span_attributes = { SpanAttributes.HTTP_METHOD: method, SpanAttributes.HTTP_URL: url, diff --git a/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_integration.py b/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_integration.py index 315cec1112..1082776f9a 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_integration.py +++ b/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_integration.py @@ -87,7 +87,7 @@ def assert_success_span( span = self.assert_span() self.assertIs(trace.SpanKind.CLIENT, span.kind) - self.assertEqual("HTTP GET", span.name) + self.assertEqual("GET", span.name) attributes = { SpanAttributes.HTTP_METHOD: "GET", diff --git a/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_ip_support.py b/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_ip_support.py index 5baddee516..1199ad3d5b 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_ip_support.py +++ b/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_ip_support.py @@ -77,7 +77,7 @@ def assert_success_span( span = self.assert_span() self.assertIs(trace.SpanKind.CLIENT, span.kind) - self.assertEqual("HTTP GET", span.name) + self.assertEqual("GET", span.name) attributes = { "http.status_code": 200, diff --git a/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py b/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py index b4d53f9a8b..f4012d7904 100644 --- a/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py @@ -440,8 +440,21 @@ def add_response_attributes( def get_default_span_name(environ): - """Default implementation for name_callback, returns HTTP {METHOD_NAME}.""" - return f"HTTP {environ.get('REQUEST_METHOD', '')}".strip() + """ + Default span name is the HTTP method and URL path, or just the method. + https://github.com/open-telemetry/opentelemetry-specification/pull/3165 + https://opentelemetry.io/docs/reference/specification/trace/semantic_conventions/http/#name + + Args: + environ: The WSGI environ object. + Returns: + The span name. + """ + method = environ.get("REQUEST_METHOD", "").strip() + path = environ.get("PATH_INFO", "").strip() + if method and path: + return f"{method} {path}" + return method class OpenTelemetryMiddleware: diff --git a/instrumentation/opentelemetry-instrumentation-wsgi/tests/test_wsgi_middleware.py b/instrumentation/opentelemetry-instrumentation-wsgi/tests/test_wsgi_middleware.py index 926124caf3..c2aaf3820d 100644 --- a/instrumentation/opentelemetry-instrumentation-wsgi/tests/test_wsgi_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-wsgi/tests/test_wsgi_middleware.py @@ -128,7 +128,7 @@ def validate_response( self, response, error=None, - span_name="HTTP GET", + span_name="GET /", http_method="GET", span_attributes=None, response_headers=None, @@ -284,12 +284,13 @@ def test_wsgi_metrics(self): ) self.assertTrue(number_data_point_seen and histogram_data_point_seen) - def test_default_span_name_missing_request_method(self): - """Test that default span_names with missing request method.""" - self.environ.pop("REQUEST_METHOD") + def test_default_span_name_missing_path_info(self): + """Test that default span_names with missing path info.""" + self.environ.pop("PATH_INFO") + method = self.environ.get("REQUEST_METHOD", "").strip() app = otel_wsgi.OpenTelemetryMiddleware(simple_wsgi) response = app(self.environ, self.start_response) - self.validate_response(response, span_name="HTTP", http_method=None) + self.validate_response(response, span_name=method) class TestWsgiAttributes(unittest.TestCase): @@ -455,7 +456,7 @@ def validate_response( response, exporter, error=None, - span_name="HTTP GET", + span_name="GET /", http_method="GET", ): while True: diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py index 3022e6ddd0..35a55a1279 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py @@ -95,7 +95,7 @@ def _start_internal_or_server_span( Args: tracer : tracer in use by given instrumentation library - name (string): name of the span + span_name (string): name of the span start_time : start time of the span context_carrier : object which contains values that are used to construct a Context. This object