Fix http clients method attribute in case of non standard http methods (open-telemetry#2726)

Author: emdneto

CHANGELOG.md

@@ -52,6 +52,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   ([#2682](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2682))
 - Populate `{method}` as `HTTP` on `_OTHER` methods from scope for `django` middleware
   ([#2714](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2714))
+- `opentelemetry-instrumentation-httpx`, `opentelemetry-instrumentation-aiohttp-client`,
+  `opentelemetry-instrumentation-requests` Populate `{method}` as `HTTP` on `_OTHER` methods
+  ([#2726](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2726))
 
 ### Fixed
 - Handle `redis.exceptions.WatchError` as a non-error event in redis instrumentation
@@ -79,7 +82,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   ([#2153](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2153))
 - `opentelemetry-instrumentation-asgi` Removed `NET_HOST_NAME` AND `NET_HOST_PORT` from active requests count attribute
   ([#2610](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2610))
-- `opentelemetry-instrumentation-asgi` Bugfix: Middleware did not set status code attribute on duration metrics for non-recording spans. 
+- `opentelemetry-instrumentation-asgi` Bugfix: Middleware did not set status code attribute on duration metrics for non-recording spans.
   ([#2627](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2627))
 
 

instrumentation/opentelemetry-instrumentation-aiohttp-client/src/opentelemetry/instrumentation/aiohttp_client/__init__.py

@@ -132,10 +132,9 @@ def response_hook(span: Span, params: typing.Union[
 
 
 def _get_span_name(method: str) -> str:
-    method = sanitize_method(method.upper().strip())
+    method = sanitize_method(method.strip())
     if method == "_OTHER":
         method = "HTTP"
-
     return method
 
 
@@ -230,8 +229,8 @@ async def on_request_start(
             trace_config_ctx.span = None
             return
 
-        http_method = params.method
-        request_span_name = _get_span_name(http_method)
+        method = params.method
+        request_span_name = _get_span_name(method)
         request_url = (
             remove_url_credentials(trace_config_ctx.url_filter(params.url))
             if callable(trace_config_ctx.url_filter)
@@ -241,8 +240,8 @@ async def on_request_start(
         span_attributes = {}
         _set_http_method(
             span_attributes,
-            http_method,
-            request_span_name,
+            method,
+            sanitize_method(method),
             sem_conv_opt_in_mode,
         )
         _set_http_url(span_attributes, request_url, sem_conv_opt_in_mode)

instrumentation/opentelemetry-instrumentation-aiohttp-client/tests/test_aiohttp_client_integration.py

@@ -40,6 +40,7 @@
 from opentelemetry.semconv.attributes.error_attributes import ERROR_TYPE
 from opentelemetry.semconv.attributes.http_attributes import (
     HTTP_REQUEST_METHOD,
+    HTTP_REQUEST_METHOD_ORIGINAL,
     HTTP_RESPONSE_STATUS_CODE,
 )
 from opentelemetry.semconv.attributes.url_attributes import URL_FULL
@@ -503,6 +504,92 @@ async def request_handler(request):
             ]
         )
 
+    def test_nonstandard_http_method(self):
+        trace_configs = [aiohttp_client.create_trace_config()]
+        app = HttpServerMock("nonstandard_method")
+
+        @app.route("/status/200", methods=["NONSTANDARD"])
+        def index():
+            return ("", 405, {})
+
+        url = "http://localhost:5000/status/200"
+
+        with app.run("localhost", 5000):
+            with self.subTest(url=url):
+
+                async def do_request(url):
+                    async with aiohttp.ClientSession(
+                        trace_configs=trace_configs,
+                    ) as session:
+                        async with session.request("NONSTANDARD", url):
+                            pass
+
+                loop = asyncio.get_event_loop()
+                loop.run_until_complete(do_request(url))
+
+        self.assert_spans(
+            [
+                (
+                    "HTTP",
+                    (StatusCode.ERROR, None),
+                    {
+                        SpanAttributes.HTTP_METHOD: "_OTHER",
+                        SpanAttributes.HTTP_URL: url,
+                        SpanAttributes.HTTP_STATUS_CODE: int(
+                            HTTPStatus.METHOD_NOT_ALLOWED
+                        ),
+                    },
+                )
+            ]
+        )
+        self.memory_exporter.clear()
+
+    def test_nonstandard_http_method_new_semconv(self):
+        trace_configs = [
+            aiohttp_client.create_trace_config(
+                sem_conv_opt_in_mode=_HTTPStabilityMode.HTTP
+            )
+        ]
+        app = HttpServerMock("nonstandard_method")
+
+        @app.route("/status/200", methods=["NONSTANDARD"])
+        def index():
+            return ("", 405, {})
+
+        url = "http://localhost:5000/status/200"
+
+        with app.run("localhost", 5000):
+            with self.subTest(url=url):
+
+                async def do_request(url):
+                    async with aiohttp.ClientSession(
+                        trace_configs=trace_configs,
+                    ) as session:
+                        async with session.request("NONSTANDARD", url):
+                            pass
+
+                loop = asyncio.get_event_loop()
+                loop.run_until_complete(do_request(url))
+
+        self.assert_spans(
+            [
+                (
+                    "HTTP",
+                    (StatusCode.ERROR, None),
+                    {
+                        HTTP_REQUEST_METHOD: "_OTHER",
+                        URL_FULL: url,
+                        HTTP_RESPONSE_STATUS_CODE: int(
+                            HTTPStatus.METHOD_NOT_ALLOWED
+                        ),
+                        HTTP_REQUEST_METHOD_ORIGINAL: "NONSTANDARD",
+                        ERROR_TYPE: "405",
+                    },
+                )
+            ]
+        )
+        self.memory_exporter.clear()
+
     def test_credential_removal(self):
         trace_configs = [aiohttp_client.create_trace_config()]
 

instrumentation/opentelemetry-instrumentation-httpx/src/opentelemetry/instrumentation/httpx/__init__.py

@@ -259,7 +259,7 @@ class ResponseInfo(typing.NamedTuple):
 
 
 def _get_default_span_name(method: str) -> str:
-    method = sanitize_method(method.upper().strip())
+    method = sanitize_method(method.strip())
     if method == "_OTHER":
         method = "HTTP"
 
@@ -326,12 +326,16 @@ def _apply_request_client_attributes_to_span(
     span_attributes: dict,
     url: typing.Union[str, URL, httpx.URL],
     method_original: str,
-    span_name: str,
     semconv: _HTTPStabilityMode,
 ):
     url = httpx.URL(url)
     # http semconv transition: http.method -> http.request.method
-    _set_http_method(span_attributes, method_original, span_name, semconv)
+    _set_http_method(
+        span_attributes,
+        method_original,
+        sanitize_method(method_original),
+        semconv,
+    )
     # http semconv transition: http.url -> url.full
     _set_http_url(span_attributes, str(url), semconv)
 
@@ -450,7 +454,6 @@ def handle_request(
             span_attributes,
             url,
             method_original,
-            span_name,
             self._sem_conv_opt_in_mode,
         )
 
@@ -572,7 +575,6 @@ async def handle_async_request(self, *args, **kwargs) -> typing.Union[
             span_attributes,
             url,
             method_original,
-            span_name,
             self._sem_conv_opt_in_mode,
         )
 

instrumentation/opentelemetry-instrumentation-httpx/tests/test_httpx_integration.py

@@ -39,6 +39,7 @@
 from opentelemetry.semconv.attributes.error_attributes import ERROR_TYPE
 from opentelemetry.semconv.attributes.http_attributes import (
     HTTP_REQUEST_METHOD,
+    HTTP_REQUEST_METHOD_ORIGINAL,
     HTTP_RESPONSE_STATUS_CODE,
 )
 from opentelemetry.semconv.attributes.network_attributes import (
@@ -217,6 +218,59 @@ def test_basic(self):
                 span, opentelemetry.instrumentation.httpx
             )
 
+        def test_nonstandard_http_method(self):
+            respx.route(method="NONSTANDARD").mock(
+                return_value=httpx.Response(405)
+            )
+            self.perform_request(self.URL, method="NONSTANDARD")
+            span = self.assert_span()
+
+            self.assertIs(span.kind, trace.SpanKind.CLIENT)
+            self.assertEqual(span.name, "HTTP")
+            self.assertEqual(
+                span.attributes,
+                {
+                    SpanAttributes.HTTP_METHOD: "_OTHER",
+                    SpanAttributes.HTTP_URL: self.URL,
+                    SpanAttributes.HTTP_STATUS_CODE: 405,
+                },
+            )
+
+            self.assertIs(span.status.status_code, trace.StatusCode.ERROR)
+
+            self.assertEqualSpanInstrumentationInfo(
+                span, opentelemetry.instrumentation.httpx
+            )
+
+        def test_nonstandard_http_method_new_semconv(self):
+            respx.route(method="NONSTANDARD").mock(
+                return_value=httpx.Response(405)
+            )
+            self.perform_request(self.URL, method="NONSTANDARD")
+            span = self.assert_span()
+
+            self.assertIs(span.kind, trace.SpanKind.CLIENT)
+            self.assertEqual(span.name, "HTTP")
+            self.assertEqual(
+                span.attributes,
+                {
+                    HTTP_REQUEST_METHOD: "_OTHER",
+                    URL_FULL: self.URL,
+                    SERVER_ADDRESS: "mock",
+                    NETWORK_PEER_ADDRESS: "mock",
+                    HTTP_RESPONSE_STATUS_CODE: 405,
+                    NETWORK_PROTOCOL_VERSION: "1.1",
+                    ERROR_TYPE: "405",
+                    HTTP_REQUEST_METHOD_ORIGINAL: "NONSTANDARD",
+                },
+            )
+
+            self.assertIs(span.status.status_code, trace.StatusCode.ERROR)
+
+            self.assertEqualSpanInstrumentationInfo(
+                span, opentelemetry.instrumentation.httpx
+            )
+
         def test_basic_new_semconv(self):
             url = "http://mock:8080/status/200"
             respx.get(url).mock(

instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py

@@ -188,13 +188,19 @@ def get_or_create_headers():
 
         span_attributes = {}
         _set_http_method(
-            span_attributes, method, span_name, sem_conv_opt_in_mode
+            span_attributes,
+            method,
+            sanitize_method(method),
+            sem_conv_opt_in_mode,
         )
         _set_http_url(span_attributes, url, sem_conv_opt_in_mode)
 
         metric_labels = {}
         _set_http_method(
-            metric_labels, method, span_name, sem_conv_opt_in_mode
+            metric_labels,
+            method,
+            sanitize_method(method),
+            sem_conv_opt_in_mode,
         )
 
         try:
@@ -365,7 +371,7 @@ def get_default_span_name(method):
     Returns:
         span name
     """
-    method = sanitize_method(method.upper().strip())
+    method = sanitize_method(method.strip())
     if method == "_OTHER":
         return "HTTP"
     return method

instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py

@@ -36,6 +36,7 @@
 from opentelemetry.semconv.attributes.error_attributes import ERROR_TYPE
 from opentelemetry.semconv.attributes.http_attributes import (
     HTTP_REQUEST_METHOD,
+    HTTP_REQUEST_METHOD_ORIGINAL,
     HTTP_RESPONSE_STATUS_CODE,
 )
 from opentelemetry.semconv.attributes.network_attributes import (
@@ -247,6 +248,48 @@ def test_basic_both_semconv(self):
             span, opentelemetry.instrumentation.requests
         )
 
+    @mock.patch("httpretty.http.HttpBaseClass.METHODS", ("NONSTANDARD",))
+    def test_nonstandard_http_method(self):
+        httpretty.register_uri("NONSTANDARD", self.URL, status=405)
+        session = requests.Session()
+        session.request("NONSTANDARD", self.URL)
+        span = self.assert_span()
+        self.assertIs(span.kind, trace.SpanKind.CLIENT)
+        self.assertEqual(span.name, "HTTP")
+        self.assertEqual(
+            span.attributes,
+            {
+                SpanAttributes.HTTP_METHOD: "_OTHER",
+                SpanAttributes.HTTP_URL: self.URL,
+                SpanAttributes.HTTP_STATUS_CODE: 405,
+            },
+        )
+
+        self.assertIs(span.status.status_code, trace.StatusCode.ERROR)
+
+    @mock.patch("httpretty.http.HttpBaseClass.METHODS", ("NONSTANDARD",))
+    def test_nonstandard_http_method_new_semconv(self):
+        httpretty.register_uri("NONSTANDARD", self.URL, status=405)
+        session = requests.Session()
+        session.request("NONSTANDARD", self.URL)
+        span = self.assert_span()
+        self.assertIs(span.kind, trace.SpanKind.CLIENT)
+        self.assertEqual(span.name, "HTTP")
+        self.assertEqual(
+            span.attributes,
+            {
+                HTTP_REQUEST_METHOD: "_OTHER",
+                URL_FULL: self.URL,
+                SERVER_ADDRESS: "mock",
+                NETWORK_PEER_ADDRESS: "mock",
+                HTTP_RESPONSE_STATUS_CODE: 405,
+                NETWORK_PROTOCOL_VERSION: "1.1",
+                ERROR_TYPE: "405",
+                HTTP_REQUEST_METHOD_ORIGINAL: "NONSTANDARD",
+            },
+        )
+        self.assertIs(span.status.status_code, trace.StatusCode.ERROR)
+
     def test_hooks(self):
         def request_hook(span, request_obj):
             span.update_name("name set from hook")