aiohttp-client: Fix producing additional spans with each newly create…

…d ClientSession (#1246)
open-telemetry · Aug 27, 2022 · 934af7e · 934af7e
1 parent 03d97ff
commit 934af7e
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 17 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -11,9 +11,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - `opentelemetry-instrumentation-boto3sqs` Make propagation compatible with other SQS instrumentations, add 'messaging.url' span attribute, and fix missing package dependencies.
   ([#1234](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1234))
-
 - restoring metrics in django framework
   ([#1208](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1208))
+- `opentelemetry-instrumentation-aiohttp-client` Fix producing additional spans with each newly created ClientSession
+- ([#1246](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1246))
 
 ## [1.12.0-0.33b0](https://github.com/open-telemetry/opentelemetry-python/releases/tag/v1.12.0-0.33b0) - 2022-08-08
 

diff --git a/...strumentation-aiohttp-client/src/opentelemetry/instrumentation/aiohttp_client/__init__.py b/...strumentation-aiohttp-client/src/opentelemetry/instrumentation/aiohttp_client/__init__.py
@@ -262,24 +262,25 @@ def _instrument(
     url_filter: _UrlFilterT = None,
     request_hook: _RequestHookT = None,
     response_hook: _ResponseHookT = None,
-    trace_configs: typing.Optional[aiohttp.TraceConfig] = None,
+    trace_configs: typing.Optional[
+        typing.Sequence[aiohttp.TraceConfig]
+    ] = None,
 ):
     """Enables tracing of all ClientSessions
 
     When a ClientSession gets created a TraceConfig is automatically added to
     the session's trace_configs.
     """
 
-    if trace_configs is None:
-        trace_configs = []
+    trace_configs = trace_configs or ()
 
     # pylint:disable=unused-argument
     def instrumented_init(wrapped, instance, args, kwargs):
         if context_api.get_value(_SUPPRESS_INSTRUMENTATION_KEY):
             return wrapped(*args, **kwargs)
 
-        if kwargs.get("trace_configs"):
-            trace_configs.extend(kwargs.get("trace_configs"))
+        client_trace_configs = list(kwargs.get("trace_configs", ()))
+        client_trace_configs.extend(trace_configs)
 
         trace_config = create_trace_config(
             url_filter=url_filter,
@@ -288,9 +289,9 @@ def instrumented_init(wrapped, instance, args, kwargs):
             tracer_provider=tracer_provider,
         )
         trace_config._is_instrumented_by_opentelemetry = True
-        trace_configs.append(trace_config)
+        client_trace_configs.append(trace_config)
 
-        kwargs["trace_configs"] = trace_configs
+        kwargs["trace_configs"] = client_trace_configs
         return wrapped(*args, **kwargs)
 
     wrapt.wrap_function_wrapper(

diff --git a/...ion/opentelemetry-instrumentation-aiohttp-client/tests/test_aiohttp_client_integration.py b/...ion/opentelemetry-instrumentation-aiohttp-client/tests/test_aiohttp_client_integration.py
@@ -387,18 +387,35 @@ def test_instrument(self):
         self.assertEqual(200, span.attributes[SpanAttributes.HTTP_STATUS_CODE])
 
     def test_instrument_with_custom_trace_config(self):
+        trace_config = aiohttp.TraceConfig()
+
         AioHttpClientInstrumentor().uninstrument()
-        AioHttpClientInstrumentor().instrument(
-            trace_configs=[aiohttp_client.create_trace_config()]
-        )
+        AioHttpClientInstrumentor().instrument(trace_configs=[trace_config])
 
-        self.assert_spans(0)
+        async def make_request(server: aiohttp.test_utils.TestServer):
+            async with aiohttp.test_utils.TestClient(server) as client:
+                trace_configs = client.session._trace_configs
+                self.assertEqual(2, len(trace_configs))
+                self.assertTrue(trace_config in trace_configs)
+                async with client as session:
+                    await session.get(TestAioHttpClientInstrumentor.URL)
 
-        run_with_test_server(
-            self.get_default_request(), self.URL, self.default_handler
-        )
+        run_with_test_server(make_request, self.URL, self.default_handler)
+        self.assert_spans(1)
+
+    def test_every_request_by_new_session_creates_one_span(self):
+        async def make_request(server: aiohttp.test_utils.TestServer):
+            async with aiohttp.test_utils.TestClient(server) as client:
+                async with client as session:
+                    await session.get(TestAioHttpClientInstrumentor.URL)
 
-        self.assert_spans(2)
+        for request_no in range(3):
+            self.memory_exporter.clear()
+            with self.subTest(request_no=request_no):
+                run_with_test_server(
+                    make_request, self.URL, self.default_handler
+                )
+                self.assert_spans(1)
 
     def test_instrument_with_existing_trace_config(self):
         trace_config = aiohttp.TraceConfig()
@@ -446,7 +463,7 @@ async def uninstrument_request(server: aiohttp.test_utils.TestServer):
         run_with_test_server(
             self.get_default_request(), self.URL, self.default_handler
         )
-        self.assert_spans(2)
+        self.assert_spans(1)
 
     def test_suppress_instrumentation(self):
         token = context.attach(