opentelemetry-instrumentation-aiohttp-server: fix span name and http.route attribute according to semconv (open-telemetry#4768)

* opentelemetry-instrumentation-aiohttp-server: fix span and http.route according to semconv

Do the method sanitization in the span name and use the canonical path
also for http.route instead of the handler name.

* Add changelog

* Use PURGE instead of QUERY

Author: xrmx

.changelog/4768.fixed

@@ -0,0 +1 @@
+`opentelemetry-instrumentation-aiohttp-server`: fix span name and `http.route` attribute according to http semantic conventions

instrumentation/opentelemetry-instrumentation-aiohttp-server/src/opentelemetry/instrumentation/aiohttp_server/__init__.py

@@ -247,28 +247,29 @@ def get_default_span_name(request: web.Request) -> str:
     Returns:
         The span name as "{method} {canonical_name}" of a resource if possible or just "{method}".
     """
-    try:
-        resource = request.match_info.route.resource
-        path = resource.canonical
-    except AttributeError:
-        path = ""
-
+    path = _get_canonical_path(request)
+    method = sanitize_method(request.method)
+    if method == "_OTHER":
+        method = "HTTP"
     if path:
-        return f"{request.method} {path}"
-    return f"{request.method}"
+        return f"{method} {path}"
+    return f"{method}"
 
 
-def _get_view_func(request: web.Request) -> str:
-    """Returns the name of the request handler.
+def _get_canonical_path(request: web.Request) -> str:
+    """Returns the canonical path from the request handler.
     Args:
         request: the request object itself.
     Returns:
-        a string containing the name of the handler function
+        a string containing the canonical path
     """
+
     try:
-        return request.match_info.handler.__name__
+        resource = request.match_info.route.resource
+        path = resource.canonical
     except AttributeError:
-        return "unknown"
+        path = ""
+    return path
 
 
 def collect_request_attributes(
@@ -323,7 +324,7 @@ def collect_request_attributes(
     _set_http_flavor_version(result, flavor, sem_conv_opt_in_mode)
 
     # http.route for both old and new
-    result[HTTP_ROUTE] = _get_view_func(request)
+    result[HTTP_ROUTE] = _get_canonical_path(request)
 
     if _report_old(sem_conv_opt_in_mode):
         http_host_value_list = (

instrumentation/opentelemetry-instrumentation-aiohttp-server/tests/test_aiohttp_server_integration.py

@@ -249,6 +249,30 @@ async def test_url_params_instrumentation(
         assert full_url == span.attributes[HTTP_URL]
 
 
+@pytest.mark.asyncio
+async def test_span_name_handler_unknown_method(
+    tracer,
+    server_fixture,
+):
+    _, memory_exporter = tracer
+    server, _ = server_fixture
+
+    assert len(memory_exporter.get_finished_spans()) == 0
+
+    async with aiohttp.ClientSession(
+        base_url=f"http://{server.host}:{server.port}"
+    ) as session:
+        # PURGE is accepted by aiohttp but not recognized by sanitize_method.
+        async with session.request("PURGE", "/test-path") as response:
+            assert response.status == HTTPStatus.METHOD_NOT_ALLOWED
+
+    spans = memory_exporter.get_finished_spans()
+    assert len(spans) == 1
+
+    assert "_OTHER" == spans[0].attributes[HTTP_METHOD]
+    assert "HTTP" == spans[0].name
+
+
 @pytest.mark.asyncio
 @pytest.mark.parametrize("suppress", [True])
 async def test_suppress_instrumentation(
@@ -563,11 +587,11 @@ async def test_semantic_conventions_metrics_old_default(
 
     AioHttpServerInstrumentor().instrument()
     app = aiohttp.web.Application()
-    app.router.add_get("/test-path", default_handler)
+    app.router.add_get("/test-path/{param}", default_handler)
     server = await aiohttp_server(app)
     client_session = aiohttp.ClientSession()
     try:
-        url = f"http://{server.host}:{server.port}/test-path?query=test"
+        url = f"http://{server.host}:{server.port}/test-path/test-param?query=test"
         async with client_session.get(  # pylint: disable=not-async-context-manager
             url, headers={"User-Agent": "test-agent"}
         ) as response:
@@ -587,11 +611,14 @@ async def test_semantic_conventions_metrics_old_default(
         assert span.attributes.get(HTTP_SCHEME) == "http"
         assert span.attributes.get(NET_HOST_NAME) == server.host
         assert span.attributes.get(NET_HOST_PORT) == server.port
-        assert span.attributes.get(HTTP_TARGET) == "/test-path?query=test"
+        assert (
+            span.attributes.get(HTTP_TARGET)
+            == "/test-path/test-param?query=test"
+        )
         assert span.attributes.get(HTTP_USER_AGENT) == "test-agent"
         assert span.attributes.get(HTTP_FLAVOR) == "1.1"
         assert span.attributes.get(HTTP_STATUS_CODE) == 200
-        assert span.attributes.get(HTTP_ROUTE) == "default_handler"
+        assert span.attributes.get(HTTP_ROUTE) == "/test-path/{param}"
         # New semconv span attributes NOT present
         assert HTTP_REQUEST_METHOD not in span.attributes
         assert URL_SCHEME not in span.attributes
@@ -636,11 +663,11 @@ async def test_semantic_conventions_metrics_new(
 
     AioHttpServerInstrumentor().instrument()
     app = aiohttp.web.Application()
-    app.router.add_get("/test-path", default_handler)
+    app.router.add_get("/test-path/{param}", default_handler)
     server = await aiohttp_server(app)
     client_session = aiohttp.ClientSession()
     try:
-        url = f"http://{server.host}:{server.port}/test-path?query=test"
+        url = f"http://{server.host}:{server.port}/test-path/test-param?query=test"
         async with client_session.get(  # pylint: disable=not-async-context-manager
             url, headers={"User-Agent": "test-agent"}
         ) as response:
@@ -660,12 +687,12 @@ async def test_semantic_conventions_metrics_new(
         assert span.attributes.get(URL_SCHEME) == "http"
         assert span.attributes.get(SERVER_ADDRESS) == server.host
         assert span.attributes.get(SERVER_PORT) == server.port
-        assert span.attributes.get(URL_PATH) == "/test-path"
+        assert span.attributes.get(URL_PATH) == "/test-path/test-param"
         assert span.attributes.get(URL_QUERY) == "query=test"
         assert span.attributes.get(USER_AGENT_ORIGINAL) == "test-agent"
         assert span.attributes.get(NETWORK_PROTOCOL_VERSION) == "1.1"
         assert span.attributes.get(HTTP_RESPONSE_STATUS_CODE) == 200
-        assert span.attributes.get(HTTP_ROUTE) == "default_handler"
+        assert span.attributes.get(HTTP_ROUTE) == "/test-path/{param}"
         # Old semconv span attributes NOT present
         assert HTTP_METHOD not in span.attributes
         assert HTTP_SCHEME not in span.attributes
@@ -755,7 +782,7 @@ async def test_semantic_conventions_metrics_both(
         assert span.attributes.get(NETWORK_PROTOCOL_VERSION) == "1.1"
         assert span.attributes.get(HTTP_STATUS_CODE) == 200
         assert span.attributes.get(HTTP_RESPONSE_STATUS_CODE) == 200
-        assert span.attributes.get(HTTP_ROUTE) == "default_handler"
+        assert span.attributes.get(HTTP_ROUTE) == "/test-path"
 
         metrics = test_base.get_sorted_metrics(SCOPE)
         assert len(metrics) == 3  # Both duration metrics + active requests