Skip to content

Commit ccea42c

Browse files
herin049xrmx
andauthored
opentelemetry-instrumentation-aiohttp-client: fix metric attribute leakage (#3936)
* opentelemetry-instrumentation-aiohttp-client: fix metric attribute leakage * opentelemetry-instrumentation-aiohttp-client: add tests verifying isolation of metric attributes * update tests and CHANGELOG.md --------- Co-authored-by: Riccardo Magliocchetti <[email protected]>
1 parent 4421670 commit ccea42c

File tree

3 files changed

+63
-10
lines changed

3 files changed

+63
-10
lines changed

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
5555
([#3941](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3941))
5656
- `opentelemetry-instrumentation-pymongo`: Fix invalid mongodb collection attribute type
5757
([#3942](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3942))
58+
- `opentelemetry-instrumentation-aiohttp-client`: Fix metric attribute leakage
59+
([#3936](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3936))
5860
- `opentelemetry-instrumentation-aiohttp-client`: Update instrumentor to respect suppressing http instrumentation
5961
([#3957](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3957))
6062

instrumentation/opentelemetry-instrumentation-aiohttp-client/src/opentelemetry/instrumentation/aiohttp_client/__init__.py

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -287,8 +287,6 @@ def create_trace_config(
287287
explicit_bucket_boundaries_advisory=HTTP_DURATION_HISTOGRAM_BUCKETS_NEW,
288288
)
289289

290-
metric_attributes = {}
291-
292290
excluded_urls = get_excluded_urls("AIOHTTP_CLIENT")
293291

294292
def _end_trace(trace_config_ctx: types.SimpleNamespace):
@@ -299,7 +297,7 @@ def _end_trace(trace_config_ctx: types.SimpleNamespace):
299297

300298
if trace_config_ctx.duration_histogram_old is not None:
301299
duration_attrs_old = _filter_semconv_duration_attrs(
302-
metric_attributes,
300+
trace_config_ctx.metric_attributes,
303301
_client_duration_attrs_old,
304302
_client_duration_attrs_new,
305303
_StabilityMode.DEFAULT,
@@ -310,7 +308,7 @@ def _end_trace(trace_config_ctx: types.SimpleNamespace):
310308
)
311309
if trace_config_ctx.duration_histogram_new is not None:
312310
duration_attrs_new = _filter_semconv_duration_attrs(
313-
metric_attributes,
311+
trace_config_ctx.metric_attributes,
314312
_client_duration_attrs_old,
315313
_client_duration_attrs_new,
316314
_StabilityMode.HTTP,
@@ -348,7 +346,7 @@ async def on_request_start(
348346
sem_conv_opt_in_mode,
349347
)
350348
_set_http_method(
351-
metric_attributes,
349+
trace_config_ctx.metric_attributes,
352350
method,
353351
sanitize_method(method),
354352
sem_conv_opt_in_mode,
@@ -359,12 +357,12 @@ async def on_request_start(
359357
parsed_url = urlparse(request_url)
360358
if parsed_url.hostname:
361359
_set_http_host_client(
362-
metric_attributes,
360+
trace_config_ctx.metric_attributes,
363361
parsed_url.hostname,
364362
sem_conv_opt_in_mode,
365363
)
366364
_set_http_net_peer_name_client(
367-
metric_attributes,
365+
trace_config_ctx.metric_attributes,
368366
parsed_url.hostname,
369367
sem_conv_opt_in_mode,
370368
)
@@ -376,7 +374,9 @@ async def on_request_start(
376374
)
377375
if parsed_url.port:
378376
_set_http_peer_port_client(
379-
metric_attributes, parsed_url.port, sem_conv_opt_in_mode
377+
trace_config_ctx.metric_attributes,
378+
parsed_url.port,
379+
sem_conv_opt_in_mode,
380380
)
381381
if _report_new(sem_conv_opt_in_mode):
382382
_set_http_peer_port_client(
@@ -411,7 +411,7 @@ async def on_request_end(
411411
_set_http_status_code_attribute(
412412
trace_config_ctx.span,
413413
params.response.status,
414-
metric_attributes,
414+
trace_config_ctx.metric_attributes,
415415
sem_conv_opt_in_mode,
416416
)
417417

@@ -429,7 +429,7 @@ async def on_request_exception(
429429
exc_type = type(params.exception).__qualname__
430430
if _report_new(sem_conv_opt_in_mode):
431431
trace_config_ctx.span.set_attribute(ERROR_TYPE, exc_type)
432-
metric_attributes[ERROR_TYPE] = exc_type
432+
trace_config_ctx.metric_attributes[ERROR_TYPE] = exc_type
433433

434434
trace_config_ctx.span.set_status(
435435
Status(StatusCode.ERROR, exc_type)
@@ -450,6 +450,7 @@ def _trace_config_ctx_factory(**kwargs):
450450
duration_histogram_old=duration_histogram_old,
451451
duration_histogram_new=duration_histogram_new,
452452
excluded_urls=excluded_urls,
453+
metric_attributes={},
453454
**kwargs,
454455
)
455456

instrumentation/opentelemetry-instrumentation-aiohttp-client/tests/test_aiohttp_client_integration.py

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -831,6 +831,56 @@ async def request_handler(request):
831831
self._assert_spans([], 0)
832832
self._assert_metrics(0)
833833

834+
def test_metric_attributes_isolation(self):
835+
async def success_handler(request):
836+
assert "traceparent" in request.headers
837+
return aiohttp.web.Response(status=HTTPStatus.OK)
838+
839+
async def timeout_handler(request):
840+
await asyncio.sleep(60)
841+
assert "traceparent" in request.headers
842+
return aiohttp.web.Response(status=HTTPStatus.OK)
843+
844+
trace_config: aiohttp.TraceConfig = (
845+
aiohttp_client.create_trace_config()
846+
)
847+
848+
success_host, success_port = self._http_request(
849+
trace_config=trace_config,
850+
url="/success",
851+
request_handler=success_handler,
852+
)
853+
854+
timeout_host, timeout_port = self._http_request(
855+
trace_config=trace_config,
856+
url="/timeout",
857+
request_handler=timeout_handler,
858+
timeout=aiohttp.ClientTimeout(sock_read=0.01),
859+
)
860+
861+
metrics = self._assert_metrics(1)
862+
duration_dp_attributes = [
863+
dict(dp.attributes) for dp in metrics[0].data.data_points
864+
]
865+
self.assertEqual(
866+
[
867+
{
868+
HTTP_METHOD: "GET",
869+
HTTP_HOST: success_host,
870+
HTTP_STATUS_CODE: int(HTTPStatus.OK),
871+
NET_PEER_NAME: success_host,
872+
NET_PEER_PORT: success_port,
873+
},
874+
{
875+
HTTP_METHOD: "GET",
876+
HTTP_HOST: timeout_host,
877+
NET_PEER_NAME: timeout_host,
878+
NET_PEER_PORT: timeout_port,
879+
},
880+
],
881+
duration_dp_attributes,
882+
)
883+
834884

835885
class TestAioHttpClientInstrumentor(TestBase):
836886
URL = "/test-path"

0 commit comments

Comments
 (0)