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 e681fc346d..3f12f9d05f 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 @@ -95,7 +95,7 @@ def response_hook(span: Span, params: typing.Union[ _HTTPStabilityMode, _OpenTelemetrySemanticConventionStability, _OpenTelemetryStabilitySignalType, - _set_error_type, + _report_new, _set_http_method, _set_http_url, _set_status, @@ -108,7 +108,9 @@ def response_hook(span: Span, params: typing.Union[ unwrap, ) from opentelemetry.propagate import inject +from opentelemetry.semconv.attributes.error_attributes import ERROR_TYPE from opentelemetry.trace import Span, SpanKind, TracerProvider, get_tracer +from opentelemetry.trace.status import Status, StatusCode from opentelemetry.util.http import remove_url_credentials, sanitize_method _UrlFilterT = typing.Optional[typing.Callable[[yarl.URL], str]] @@ -129,7 +131,7 @@ def response_hook(span: Span, params: typing.Union[ ] -def _get_default_span_name(method: str) -> str: +def _get_span_name(method: str) -> str: method = sanitize_method(method.upper().strip()) if method == "_OTHER": method = "HTTP" @@ -188,7 +190,6 @@ def create_trace_config( # TODO: Use this when we have durations for aiohttp-client metrics_attributes = {} - server_span = False def _end_trace(trace_config_ctx: types.SimpleNamespace): context_api.detach(trace_config_ctx.token) @@ -204,7 +205,7 @@ async def on_request_start( return http_method = params.method - request_span_name = _get_default_span_name(http_method) + request_span_name = _get_span_name(http_method) request_url = ( remove_url_credentials(trace_config_ctx.url_filter(params.url)) if callable(trace_config_ctx.url_filter) @@ -245,11 +246,12 @@ async def on_request_end( response_hook(trace_config_ctx.span, params) if trace_config_ctx.span.is_recording(): - status_code_str = str(params.response.status) try: - status_code = int(status_code_str) + status_code = int(params.response.status) except ValueError: status_code = -1 + status_code_str = str(params.response.status) + server_span = False _set_status( trace_config_ctx.span, metrics_attributes, @@ -269,11 +271,12 @@ async def on_request_exception( return if trace_config_ctx.span.is_recording() and params.exception: - _set_error_type( - trace_config_ctx.span, - metrics_attributes, - type(params.exception).__qualname__, - sem_conv_opt_in_mode, + exc_type = type(params.exception).__qualname__ + if _report_new(sem_conv_opt_in_mode): + trace_config_ctx.span.set_attribute(ERROR_TYPE, exc_type) + + trace_config_ctx.span.set_status( + Status(StatusCode.ERROR, exc_type) ) trace_config_ctx.span.record_exception(params.exception) 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 686394741a..cdae4f7da0 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 @@ -21,17 +21,32 @@ from unittest import mock import aiohttp +import aiohttp.client_exceptions +import aiohttp.http_exceptions import aiohttp.test_utils +import aiohttp.web +import aiohttp.web_exceptions import yarl from http_server_mock import HttpServerMock from pkg_resources import iter_entry_points from opentelemetry import trace as trace_api from opentelemetry.instrumentation import aiohttp_client +from opentelemetry.instrumentation._semconv import ( + OTEL_SEMCONV_STABILITY_OPT_IN, + _HTTPStabilityMode, + _OpenTelemetrySemanticConventionStability, +) from opentelemetry.instrumentation.aiohttp_client import ( AioHttpClientInstrumentor, ) 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_RESPONSE_STATUS_CODE, +) +from opentelemetry.semconv.attributes.url_attributes import URL_FULL from opentelemetry.semconv.trace import SpanAttributes from opentelemetry.test.test_base import TestBase from opentelemetry.trace import Span, StatusCode @@ -59,7 +74,23 @@ async def do_request(): class TestAioHttpIntegration(TestBase): - def assert_spans(self, spans): + + _test_status_codes = ( + (HTTPStatus.OK, StatusCode.UNSET), + (HTTPStatus.TEMPORARY_REDIRECT, StatusCode.UNSET), + (HTTPStatus.NOT_FOUND, StatusCode.ERROR), + (HTTPStatus.BAD_REQUEST, StatusCode.ERROR), + (HTTPStatus.SERVICE_UNAVAILABLE, StatusCode.ERROR), + (HTTPStatus.GATEWAY_TIMEOUT, StatusCode.ERROR), + ) + + def setUp(self): + super().setUp() + _OpenTelemetrySemanticConventionStability._initialized = False + + def assert_spans(self, spans, num_spans=1): + finished_spans = self.memory_exporter.get_finished_spans() + self.assertEqual(num_spans, len(finished_spans)) self.assertEqual( [ ( @@ -67,7 +98,7 @@ def assert_spans(self, spans): (span.status.status_code, span.status.description), dict(span.attributes), ) - for span in self.memory_exporter.get_finished_spans() + for span in finished_spans ], spans, ) @@ -99,39 +130,72 @@ async def client_request(server: aiohttp.test_utils.TestServer): return run_with_test_server(client_request, url, handler) def test_status_codes(self): - for status_code, span_status in ( - (HTTPStatus.OK, StatusCode.UNSET), - (HTTPStatus.TEMPORARY_REDIRECT, StatusCode.UNSET), - (HTTPStatus.SERVICE_UNAVAILABLE, StatusCode.ERROR), - ( - HTTPStatus.GATEWAY_TIMEOUT, - StatusCode.ERROR, - ), - ): + for status_code, span_status in self._test_status_codes: with self.subTest(status_code=status_code): + path = "test-path?query=param#foobar" host, port = self._http_request( trace_config=aiohttp_client.create_trace_config(), - url="/test-path?query=param#foobar", + url=f"/{path}", status_code=status_code, ) + url = f"http://{host}:{port}/{path}" + attributes = { + SpanAttributes.HTTP_METHOD: "GET", + SpanAttributes.HTTP_URL: url, + SpanAttributes.HTTP_STATUS_CODE: int(status_code), + } + spans = [("GET", (span_status, None), attributes)] + self.assert_spans(spans) + self.memory_exporter.clear() - url = f"http://{host}:{port}/test-path?query=param#foobar" - self.assert_spans( - [ - ( - "GET", - (span_status, None), - { - SpanAttributes.HTTP_METHOD: "GET", - SpanAttributes.HTTP_URL: url, - SpanAttributes.HTTP_STATUS_CODE: int( - status_code - ), - }, - ) - ] + def test_status_codes_new_semconv(self): + for status_code, span_status in self._test_status_codes: + with self.subTest(status_code=status_code): + path = "test-path?query=param#foobar" + host, port = self._http_request( + trace_config=aiohttp_client.create_trace_config( + sem_conv_opt_in_mode=_HTTPStabilityMode.HTTP + ), + url=f"/{path}", + status_code=status_code, ) + url = f"http://{host}:{port}/{path}" + attributes = { + HTTP_REQUEST_METHOD: "GET", + URL_FULL: url, + HTTP_RESPONSE_STATUS_CODE: int(status_code), + } + if status_code >= 400: + attributes[ERROR_TYPE] = str(status_code) + spans = [("GET", (span_status, None), attributes)] + self.assert_spans(spans) + self.memory_exporter.clear() + def test_status_codes_both_semconv(self): + for status_code, span_status in self._test_status_codes: + with self.subTest(status_code=status_code): + path = "test-path?query=param#foobar" + host, port = self._http_request( + trace_config=aiohttp_client.create_trace_config( + sem_conv_opt_in_mode=_HTTPStabilityMode.HTTP_DUP + ), + url=f"/{path}", + status_code=status_code, + ) + url = f"http://{host}:{port}/{path}" + attributes = { + HTTP_REQUEST_METHOD: "GET", + SpanAttributes.HTTP_METHOD: "GET", + URL_FULL: url, + SpanAttributes.HTTP_URL: url, + HTTP_RESPONSE_STATUS_CODE: int(status_code), + SpanAttributes.HTTP_STATUS_CODE: int(status_code), + } + if status_code >= 400: + attributes[ERROR_TYPE] = str(status_code) + + spans = [("GET", (span_status, None), attributes)] + self.assert_spans(spans, 1) self.memory_exporter.clear() def test_schema_url(self): @@ -149,6 +213,40 @@ def test_schema_url(self): ) self.memory_exporter.clear() + def test_schema_url_new_semconv(self): + with self.subTest(status_code=200): + self._http_request( + trace_config=aiohttp_client.create_trace_config( + sem_conv_opt_in_mode=_HTTPStabilityMode.HTTP + ), + url="/test-path?query=param#foobar", + status_code=200, + ) + + span = self.memory_exporter.get_finished_spans()[0] + self.assertEqual( + span.instrumentation_info.schema_url, + "https://opentelemetry.io/schemas/v1.21.0", + ) + self.memory_exporter.clear() + + def test_schema_url_both_semconv(self): + with self.subTest(status_code=200): + self._http_request( + trace_config=aiohttp_client.create_trace_config( + sem_conv_opt_in_mode=_HTTPStabilityMode.HTTP_DUP + ), + url="/test-path?query=param#foobar", + status_code=200, + ) + + span = self.memory_exporter.get_finished_spans()[0] + self.assertEqual( + span.instrumentation_info.schema_url, + "https://opentelemetry.io/schemas/v1.21.0", + ) + self.memory_exporter.clear() + def test_not_recording(self): mock_tracer = mock.Mock() mock_span = mock.Mock() @@ -273,6 +371,89 @@ async def do_request(url): ) self.memory_exporter.clear() + def test_basic_exception(self): + async def request_handler(request): + assert "traceparent" in request.headers + + host, port = self._http_request( + trace_config=aiohttp_client.create_trace_config(), + url="/test", + request_handler=request_handler, + ) + span = self.memory_exporter.get_finished_spans()[0] + self.assertEqual(len(span.events), 1) + self.assertEqual(span.events[0].name, "exception") + self.assert_spans( + [ + ( + "GET", + (StatusCode.ERROR, "ServerDisconnectedError"), + { + SpanAttributes.HTTP_METHOD: "GET", + SpanAttributes.HTTP_URL: f"http://{host}:{port}/test", + }, + ) + ] + ) + + def test_basic_exception_new_semconv(self): + async def request_handler(request): + assert "traceparent" in request.headers + + host, port = self._http_request( + trace_config=aiohttp_client.create_trace_config( + sem_conv_opt_in_mode=_HTTPStabilityMode.HTTP + ), + url="/test", + request_handler=request_handler, + ) + span = self.memory_exporter.get_finished_spans()[0] + self.assertEqual(len(span.events), 1) + self.assertEqual(span.events[0].name, "exception") + self.assert_spans( + [ + ( + "GET", + (StatusCode.ERROR, "ServerDisconnectedError"), + { + HTTP_REQUEST_METHOD: "GET", + URL_FULL: f"http://{host}:{port}/test", + ERROR_TYPE: "ServerDisconnectedError", + }, + ) + ] + ) + + def test_basic_exception_both_semconv(self): + async def request_handler(request): + assert "traceparent" in request.headers + + host, port = self._http_request( + trace_config=aiohttp_client.create_trace_config( + sem_conv_opt_in_mode=_HTTPStabilityMode.HTTP_DUP + ), + url="/test", + request_handler=request_handler, + ) + span = self.memory_exporter.get_finished_spans()[0] + self.assertEqual(len(span.events), 1) + self.assertEqual(span.events[0].name, "exception") + self.assert_spans( + [ + ( + "GET", + (StatusCode.ERROR, "ServerDisconnectedError"), + { + HTTP_REQUEST_METHOD: "GET", + URL_FULL: f"http://{host}:{port}/test", + ERROR_TYPE: "ServerDisconnectedError", + SpanAttributes.HTTP_METHOD: "GET", + SpanAttributes.HTTP_URL: f"http://{host}:{port}/test", + }, + ) + ] + ) + def test_timeout(self): async def request_handler(request): await asyncio.sleep(1) @@ -374,6 +555,7 @@ class TestAioHttpClientInstrumentor(TestBase): def setUp(self): super().setUp() AioHttpClientInstrumentor().instrument() + _OpenTelemetrySemanticConventionStability._initialized = False def tearDown(self): super().tearDown() @@ -414,6 +596,46 @@ def test_instrument(self): ) self.assertEqual(200, span.attributes[SpanAttributes.HTTP_STATUS_CODE]) + def test_instrument_new_semconv(self): + AioHttpClientInstrumentor().uninstrument() + with mock.patch.dict( + "os.environ", {OTEL_SEMCONV_STABILITY_OPT_IN: "http"} + ): + AioHttpClientInstrumentor().instrument() + host, port = run_with_test_server( + self.get_default_request(), self.URL, self.default_handler + ) + span = self.assert_spans(1) + self.assertEqual("GET", span.name) + self.assertEqual("GET", span.attributes[HTTP_REQUEST_METHOD]) + self.assertEqual( + f"http://{host}:{port}/test-path", + span.attributes[URL_FULL], + ) + self.assertEqual(200, span.attributes[HTTP_RESPONSE_STATUS_CODE]) + + def test_instrument_both_semconv(self): + AioHttpClientInstrumentor().uninstrument() + with mock.patch.dict( + "os.environ", {OTEL_SEMCONV_STABILITY_OPT_IN: "http/dup"} + ): + AioHttpClientInstrumentor().instrument() + host, port = run_with_test_server( + self.get_default_request(), self.URL, self.default_handler + ) + url = f"http://{host}:{port}/test-path" + attributes = { + HTTP_REQUEST_METHOD: "GET", + SpanAttributes.HTTP_METHOD: "GET", + URL_FULL: url, + SpanAttributes.HTTP_URL: url, + HTTP_RESPONSE_STATUS_CODE: 200, + SpanAttributes.HTTP_STATUS_CODE: 200, + } + span = self.assert_spans(1) + self.assertEqual("GET", span.name) + self.assertEqual(span.attributes, attributes) + def test_instrument_with_custom_trace_config(self): trace_config = aiohttp.TraceConfig() diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py index e8e9819a23..1b0923fdd3 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py @@ -347,19 +347,6 @@ def _set_http_flavor_version(result, version, sem_conv_opt_in_mode): set_string_attribute(result, NETWORK_PROTOCOL_VERSION, version) -def _set_error_type( - span, - metrics_attributes: dict, - error_type: str, - sem_conv_opt_in_mode: _HTTPStabilityMode, -): - if _report_new(sem_conv_opt_in_mode): - span.set_attribute(ERROR_TYPE, error_type) - metrics_attributes[ERROR_TYPE] = error_type - - span.set_status(Status(StatusCode.ERROR, error_type)) - - def _set_status( span, metrics_attributes: dict,