diff --git a/.changelog/5029.fixed b/.changelog/5029.fixed new file mode 100644 index 0000000000..a6d22ae86e --- /dev/null +++ b/.changelog/5029.fixed @@ -0,0 +1 @@ +`opentelemetry-instrumentation-aiohttp-client`: create the duration histograms and the tracer once instead of per `ClientSession`. `instrument()` now gets its tracer when it is called, matching `requests` and `httpx`. With `OTEL_PYTHON_TRACER_PROVIDER` set, that installs the configured provider at `instrument()` time, so a later `set_tracer_provider()` call is refused 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 0ac64d67c9..e626950c82 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 @@ -235,7 +235,7 @@ def response_hook( is_http_instrumentation_enabled, unwrap, ) -from opentelemetry.metrics import MeterProvider, get_meter +from opentelemetry.metrics import Histogram, MeterProvider, get_meter from opentelemetry.propagate import inject from opentelemetry.semconv.attributes.error_attributes import ERROR_TYPE from opentelemetry.semconv.metrics import ( @@ -244,7 +244,7 @@ def response_hook( from opentelemetry.semconv.metrics.http_metrics import ( HTTP_CLIENT_REQUEST_DURATION, ) -from opentelemetry.trace import Span, SpanKind, TracerProvider, get_tracer +from opentelemetry.trace import Span, SpanKind, Tracer, TracerProvider, get_tracer from opentelemetry.trace.status import Status, StatusCode from opentelemetry.util.http import ( OTEL_INSTRUMENTATION_HTTP_CAPTURE_HEADERS_CLIENT_REQUEST, @@ -323,78 +323,27 @@ def _set_http_status_code_attribute( ) -# pylint: disable=too-many-locals -# pylint: disable=too-many-statements -def create_trace_config( - url_filter: UrlFilterT = None, - request_hook: RequestHookT = None, - response_hook: ResponseHookT = None, - tracer_provider: TracerProvider | None = None, - meter_provider: MeterProvider | None = None, - sem_conv_opt_in_mode: _StabilityMode = _StabilityMode.DEFAULT, - captured_request_headers: list[str] | None = None, - captured_response_headers: list[str] | None = None, - sensitive_headers: list[str] | None = None, -) -> aiohttp.TraceConfig: - """Create an aiohttp-compatible trace configuration. - - One span is created for the entire HTTP request, including initial - TCP/TLS setup if the connection doesn't exist. - - By default the span name is set to the HTTP request method. - - Example usage: - - .. code:: python - - import aiohttp - from opentelemetry.instrumentation.aiohttp_client import create_trace_config - - async with aiohttp.ClientSession(trace_configs=[create_trace_config()]) as session: - async with session.get(url) as response: - await response.text() - - - :param url_filter: A callback to process the requested URL prior to adding - it as a span attribute. This can be useful to remove sensitive data - such as API keys or user personal information. - - :param Callable request_hook: Optional callback that can modify span name and request params. - :param Callable response_hook: Optional callback that can modify span name and response params. - :param tracer_provider: optional TracerProvider from which to get a Tracer - :param meter_provider: optional Meter provider to use - :param captured_request_headers: List of HTTP request header regexes to capture as - span attributes. Header names matching these patterns will be added as span - attributes with the format ``http.request.header.``. - :param captured_response_headers: List of HTTP response header regexes to capture as - span attributes. Header names matching these patterns will be added as span - attributes with the format ``http.response.header.``. - :param sensitive_headers: List of HTTP header regexes whose values should be - sanitized (redacted) when captured. Header values matching these patterns - will be replaced with ``[REDACTED]``. - - :return: An object suitable for use with :py:class:`aiohttp.ClientSession`. - :rtype: :py:class:`aiohttp.TraceConfig` - """ - # `aiohttp.TraceRequestStartParams` resolves to `aiohttp.tracing.TraceRequestStartParams` - # which doesn't exist in the aiohttp intersphinx inventory. - # Explicitly specify the type for the `request_hook` and `response_hook` param and rtype to work - # around this issue. - - schema_url = _get_schema_url(sem_conv_opt_in_mode) - - tracer = get_tracer( +def _create_tracer( + tracer_provider: TracerProvider | None, + sem_conv_opt_in_mode: _StabilityMode, +) -> Tracer: + return get_tracer( __name__, __version__, tracer_provider, - schema_url=schema_url, + schema_url=_get_schema_url(sem_conv_opt_in_mode), ) + +def _create_duration_histograms( + meter_provider: MeterProvider | None, + sem_conv_opt_in_mode: _StabilityMode, +) -> tuple[Histogram | None, Histogram | None]: meter = get_meter( __name__, __version__, meter_provider, - schema_url, + _get_schema_url(sem_conv_opt_in_mode), ) duration_histogram_old = None @@ -413,7 +362,23 @@ def create_trace_config( description="Duration of HTTP client requests.", explicit_bucket_boundaries_advisory=HTTP_DURATION_HISTOGRAM_BUCKETS_NEW, ) + return duration_histogram_old, duration_histogram_new + +# pylint: disable=too-many-locals +# pylint: disable=too-many-statements +def _create_trace_config( + tracer: Tracer, + url_filter: UrlFilterT = None, + request_hook: RequestHookT = None, + response_hook: ResponseHookT = None, + sem_conv_opt_in_mode: _StabilityMode = _StabilityMode.DEFAULT, + captured_request_headers: list[str] | None = None, + captured_response_headers: list[str] | None = None, + sensitive_headers: list[str] | None = None, + duration_histogram_old: Histogram | None = None, + duration_histogram_new: Histogram | None = None, +) -> aiohttp.TraceConfig: excluded_urls = get_excluded_urls("AIOHTTP_CLIENT") def _end_trace(trace_config_ctx: types.SimpleNamespace): @@ -607,6 +572,76 @@ def _trace_config_ctx_factory(**kwargs: Any) -> types.SimpleNamespace: return trace_config +def create_trace_config( + url_filter: UrlFilterT = None, + request_hook: RequestHookT = None, + response_hook: ResponseHookT = None, + tracer_provider: TracerProvider | None = None, + meter_provider: MeterProvider | None = None, + sem_conv_opt_in_mode: _StabilityMode = _StabilityMode.DEFAULT, + captured_request_headers: list[str] | None = None, + captured_response_headers: list[str] | None = None, + sensitive_headers: list[str] | None = None, +) -> aiohttp.TraceConfig: + """Create an aiohttp-compatible trace configuration. + + One span is created for the entire HTTP request, including initial + TCP/TLS setup if the connection doesn't exist. + + By default the span name is set to the HTTP request method. + + Example usage: + + .. code:: python + + import aiohttp + from opentelemetry.instrumentation.aiohttp_client import create_trace_config + + async with aiohttp.ClientSession(trace_configs=[create_trace_config()]) as session: + async with session.get(url) as response: + await response.text() + + + :param url_filter: A callback to process the requested URL prior to adding + it as a span attribute. This can be useful to remove sensitive data + such as API keys or user personal information. + + :param Callable request_hook: Optional callback that can modify span name and request params. + :param Callable response_hook: Optional callback that can modify span name and response params. + :param tracer_provider: optional TracerProvider from which to get a Tracer + :param meter_provider: optional Meter provider to use + :param captured_request_headers: List of HTTP request header regexes to capture as + span attributes. Header names matching these patterns will be added as span + attributes with the format ``http.request.header.``. + :param captured_response_headers: List of HTTP response header regexes to capture as + span attributes. Header names matching these patterns will be added as span + attributes with the format ``http.response.header.``. + :param sensitive_headers: List of HTTP header regexes whose values should be + sanitized (redacted) when captured. Header values matching these patterns + will be replaced with ``[REDACTED]``. + + :return: An object suitable for use with :py:class:`aiohttp.ClientSession`. + :rtype: :py:class:`aiohttp.TraceConfig` + """ + # `aiohttp.TraceRequestStartParams` resolves to `aiohttp.tracing.TraceRequestStartParams` + # which doesn't exist in the aiohttp intersphinx inventory. + # Explicitly specify the type for the `request_hook` and `response_hook` param and rtype to work + # around this issue. + duration_histogram_old, duration_histogram_new = _create_duration_histograms(meter_provider, sem_conv_opt_in_mode) + return _create_trace_config( + _create_tracer(tracer_provider, sem_conv_opt_in_mode), + url_filter=url_filter, + request_hook=request_hook, + response_hook=response_hook, + sem_conv_opt_in_mode=sem_conv_opt_in_mode, + captured_request_headers=captured_request_headers, + captured_response_headers=captured_response_headers, + sensitive_headers=sensitive_headers, + duration_histogram_old=duration_histogram_old, + duration_histogram_new=duration_histogram_new, + ) + + def _instrument( tracer_provider: TracerProvider | None = None, meter_provider: MeterProvider | None = None, @@ -627,6 +662,13 @@ def _instrument( trace_configs = trace_configs or () + # The tracer and the instruments are created once here rather than per session: the + # default proxy meter provider keeps every meter it hands out, so building one in + # instrumented_init would accumulate one per ClientSession. This matches how + # requests and httpx resolve their providers at instrument() time. + tracer = _create_tracer(tracer_provider, sem_conv_opt_in_mode) + duration_histogram_old, duration_histogram_new = _create_duration_histograms(meter_provider, sem_conv_opt_in_mode) + # pylint:disable=unused-argument def instrumented_init( wrapped: Callable[..., None], @@ -637,16 +679,17 @@ def instrumented_init( client_trace_configs = list(kwargs.get("trace_configs") or []) client_trace_configs.extend(trace_configs) - trace_config = create_trace_config( + trace_config = _create_trace_config( + tracer, url_filter=url_filter, request_hook=request_hook, response_hook=response_hook, - tracer_provider=tracer_provider, - meter_provider=meter_provider, sem_conv_opt_in_mode=sem_conv_opt_in_mode, captured_request_headers=captured_request_headers, captured_response_headers=captured_response_headers, sensitive_headers=sensitive_headers, + duration_histogram_old=duration_histogram_old, + duration_histogram_new=duration_histogram_new, ) setattr(trace_config, "_is_instrumented_by_opentelemetry", True) client_trace_configs.append(trace_config) 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 96795bcc77..03ee2b2adf 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 @@ -17,6 +17,7 @@ import yarl from http_server_mock import HttpServerMock +from opentelemetry import metrics as metrics_api from opentelemetry import trace as trace_api from opentelemetry.instrumentation import aiohttp_client from opentelemetry.instrumentation._semconv import ( @@ -54,6 +55,10 @@ HTTP_RESPONSE_STATUS_CODE, ) from opentelemetry.semconv.attributes.url_attributes import URL_FULL +from opentelemetry.test.globals_test import ( + reset_metrics_globals, + reset_trace_globals, +) from opentelemetry.test.test_base import TestBase from opentelemetry.trace import Span, StatusCode from opentelemetry.util._importlib_metadata import entry_points @@ -1522,6 +1527,42 @@ def test_ignores_excluded_urls(self): self._assert_spans(0) self._assert_metrics(0) + def test_instrument_creates_one_meter_and_tracer_for_all_sessions(self): + AioHttpClientInstrumentor().uninstrument() + + async def open_and_close_sessions(count: int): + for _ in range(count): + await aiohttp.ClientSession().close() + + with ( + mock.patch( + "opentelemetry.instrumentation.aiohttp_client.get_meter", + wraps=aiohttp_client.get_meter, + ) as patched_get_meter, + mock.patch( + "opentelemetry.instrumentation.aiohttp_client.get_tracer", + wraps=aiohttp_client.get_tracer, + ) as patched_get_tracer, + ): + AioHttpClientInstrumentor().instrument() + asyncio.run(open_and_close_sessions(5)) + + self.assertEqual(1, patched_get_meter.call_count) + self.assertEqual(1, patched_get_tracer.call_count) + + def test_providers_set_after_instrument_receive_telemetry(self): + AioHttpClientInstrumentor().uninstrument() + reset_trace_globals() + reset_metrics_globals() + + AioHttpClientInstrumentor().instrument() + trace_api.set_tracer_provider(self.tracer_provider) + metrics_api.set_meter_provider(self.meter_provider) + run_with_test_server(self.get_default_request(), self.URL, self.default_handler) + + self._assert_spans(1) + self._assert_metrics(1) + class TestLoadingAioHttpInstrumentor(unittest.TestCase): def test_loading_instrumentor(self):