fix(aiohttp-client): create the duration histograms once, not per ClientSession

.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

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.<header_name>``.
-    :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.<header_name>``.
-    :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.<header_name>``.
+    :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.<header_name>``.
+    :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)

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):