diff --git a/CHANGELOG.md b/CHANGELOG.md index a320a1d4f..4228bf125 100644 --- a/CHANGELOG.md +++ b/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)) 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 459e3121b..de60fa637 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 @@ _ResponseHookT = typing.Optional[ 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 @@ def create_trace_config( 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 @@ def create_trace_config( 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 538421dd6..3873a6f09 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.instrumentation.utils import suppress_instrumentation 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 @@ class TestAioHttpIntegration(TestBase): ] ) + 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 e3ce383d7..f2a18a277 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 @@ class SyncOpenTelemetryTransport(httpx.BaseTransport): span_attributes, url, method_original, - span_name, self._sem_conv_opt_in_mode, ) @@ -572,7 +575,6 @@ class AsyncOpenTelemetryTransport(httpx.AsyncBaseTransport): 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 03141e61b..011b5e57d 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.sdk import resources 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 @@ class BaseTestCases: 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 0a5556438..3aa1b476f 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 _instrument( 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 diff --git a/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py b/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py index 75518fc8d..a5cb8927a 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.sdk import resources 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 @@ class RequestsIntegrationTestBase(abc.ABC): 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")