From 3ff4a78b6993fa101ab14d9247781273209563aa Mon Sep 17 00:00:00 2001 From: emdneto <9735060+emdneto@users.noreply.github.com> Date: Sun, 21 Jul 2024 19:22:34 -0300 Subject: [PATCH] fix http method attribute in case of nonstandard http method --- .../aiohttp_client/__init__.py | 11 ++- .../tests/test_aiohttp_client_integration.py | 89 ++++++++++++++++++- .../instrumentation/httpx/__init__.py | 12 +-- .../tests/test_httpx_integration.py | 54 +++++++++++ .../instrumentation/requests/__init__.py | 10 ++- .../tests/test_requests_integration.py | 43 +++++++++ 6 files changed, 205 insertions(+), 14 deletions(-) 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 459e3121b9..de60fa6379 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 @@ -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) 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 538421dd6a..fc8fb1eff5 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 @@ -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 @@ -57,7 +58,7 @@ async def do_request(): app.add_routes([aiohttp.web.get(parsed_url.path, handler)]) app.add_routes([aiohttp.web.post(parsed_url.path, handler)]) app.add_routes([aiohttp.web.patch(parsed_url.path, handler)]) - + app.add_routes([aiohttp.web.route("*", "/nonstandard", handler)]) with contextlib.suppress(aiohttp.ClientError): async with aiohttp.test_utils.TestServer(app) as server: netloc = (server.host, server.port) @@ -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()] 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 e3ce383d7e..f2a18a2770 100644 --- a/instrumentation/opentelemetry-instrumentation-httpx/src/opentelemetry/instrumentation/httpx/__init__.py +++ b/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, ) diff --git a/instrumentation/opentelemetry-instrumentation-httpx/tests/test_httpx_integration.py b/instrumentation/opentelemetry-instrumentation-httpx/tests/test_httpx_integration.py index 03141e61b5..011b5e57d2 100644 --- a/instrumentation/opentelemetry-instrumentation-httpx/tests/test_httpx_integration.py +++ b/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( 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 0a55564386..81978f817b 100644 --- a/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py +++ b/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: diff --git a/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py b/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py index 75518fc8d3..a5cb8927ae 100644 --- a/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py +++ b/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")