From 23a670ca656a77bc104c7fff0fc0cf37a156fc0e Mon Sep 17 00:00:00 2001 From: Sebastian Kreft Date: Sat, 19 Nov 2022 14:25:20 -0300 Subject: [PATCH 1/6] fix(asgi-instrumentation): extract target after running the framework There was a mistake in the original strategy to extract the feature, as before running the framework the scope does not have all the information necessary to deduct the route. We move the target extraction after running the framewrork function, but that means that we won't have those attributes for the span or for the active_requests metrics. The target will be available only for the duration metric. The test was also updated to reflect that the scope changes only after running the framework function. --- .../opentelemetry/instrumentation/asgi/__init__.py | 8 +++----- .../tests/test_asgi_middleware.py | 11 +++++++++-- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py index 608aeade7f..55bb418647 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py @@ -542,11 +542,6 @@ async def __call__(self, scope, receive, send): ) duration_attrs = _parse_duration_attrs(attributes) - target = _collect_target_attribute(scope) - if target: - active_requests_count_attrs[SpanAttributes.HTTP_TARGET] = target - duration_attrs[SpanAttributes.HTTP_TARGET] = target - if scope["type"] == "http": self.active_requests_counter.add(1, active_requests_count_attrs) try: @@ -581,6 +576,9 @@ async def __call__(self, scope, receive, send): await self.app(scope, otel_receive, otel_send) finally: if scope["type"] == "http": + target = _collect_target_attribute(scope) + if target: + duration_attrs[SpanAttributes.HTTP_TARGET] = target duration = max(round((default_timer() - start) * 1000), 0) self.duration_histogram.record(duration, duration_attrs) self.active_requests_counter.add( diff --git a/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py b/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py index 7582ffb998..f1812891f0 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py @@ -591,8 +591,15 @@ def test_metric_target_attribute(self): class TestRoute: path_format = expected_target - self.scope["route"] = TestRoute() - app = otel_asgi.OpenTelemetryMiddleware(simple_asgi) + async def target_asgi(scope, receive, send): + assert isinstance(scope, dict) + if scope["type"] == "http": + await http_app(scope, receive, send) + scope["route"] = TestRoute() + else: + raise ValueError("websockets not supported") + + app = otel_asgi.OpenTelemetryMiddleware(target_asgi) self.seed_app(app) self.send_default_request() From 8c41392614c34afd0037317e482aed24dcac4616 Mon Sep 17 00:00:00 2001 From: Sebastian Kreft Date: Sat, 19 Nov 2022 16:30:36 -0300 Subject: [PATCH 2/6] Add changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b95e366603..ccd3e17633 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#1424](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1424)) - Remove db.name attribute from Redis instrumentation ([#1427](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1427)) +- `opentelemetry-instrumentation-asgi` Fix target extraction for duration metric + ([#1461](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1461)) ## Version 1.14.0/0.35b0 (2022-11-03) From 0e802ac6e34a9203a66af033ce23cfd6cde6348c Mon Sep 17 00:00:00 2001 From: Sebastian Kreft Date: Sat, 19 Nov 2022 16:41:20 -0300 Subject: [PATCH 3/6] fix fastapi tests --- .../tests/test_fastapi_instrumentation.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py index 14c3164029..bb1a4bc01c 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py @@ -46,7 +46,7 @@ ] _recommended_attrs = { "http.server.active_requests": _active_requests_count_attrs, - "http.server.duration": _duration_attrs, + "http.server.duration": _duration_attrs + [SpanAttributes.HTTP_TARGET], } @@ -218,6 +218,7 @@ def test_basic_metric_success(self): "http.server_name": "testserver", "net.host.port": 80, "http.status_code": 200, + "http.target": "/foobar", } expected_requests_count_attributes = { "http.method": "GET", From 6d0168999e47a641112bd357587b39f534860363 Mon Sep 17 00:00:00 2001 From: Sebastian Kreft Date: Sat, 3 Dec 2022 08:45:19 -0300 Subject: [PATCH 4/6] fix asgi metric test Now the target attribute is only present in the duration metric --- .../tests/test_asgi_middleware.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py b/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py index f1812891f0..633e2a51bd 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py @@ -608,6 +608,8 @@ async def target_asgi(scope, receive, send): for resource_metric in metrics_list.resource_metrics: for scope_metrics in resource_metric.scope_metrics: for metric in scope_metrics.metrics: + if metric.name != "http.server.duration": + continue for point in metric.data.data_points: if isinstance(point, HistogramDataPoint): self.assertEqual( @@ -615,12 +617,6 @@ async def target_asgi(scope, receive, send): expected_target, ) assertions += 1 - elif isinstance(point, NumberDataPoint): - self.assertEqual( - point.attributes["http.target"], - expected_target, - ) - assertions += 1 self.assertEqual(assertions, 2) def test_no_metric_for_websockets(self): From 29bc7c0e99a9af5578ce93256782d58e19241680 Mon Sep 17 00:00:00 2001 From: Sebastian Kreft Date: Sat, 3 Dec 2022 09:11:00 -0300 Subject: [PATCH 5/6] fix test --- .../tests/test_asgi_middleware.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py b/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py index 633e2a51bd..bfa5720f99 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py @@ -617,7 +617,7 @@ async def target_asgi(scope, receive, send): expected_target, ) assertions += 1 - self.assertEqual(assertions, 2) + self.assertEqual(assertions, 1) def test_no_metric_for_websockets(self): self.scope = { From 75d19b1b062581c1dc1a8cacad87854a46ffe51d Mon Sep 17 00:00:00 2001 From: Sebastian Kreft Date: Sat, 3 Dec 2022 20:11:50 -0300 Subject: [PATCH 6/6] fix fastapi test --- .../tests/test_fastapi_instrumentation.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py index bb1a4bc01c..c006b5c4fe 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py @@ -46,7 +46,7 @@ ] _recommended_attrs = { "http.server.active_requests": _active_requests_count_attrs, - "http.server.duration": _duration_attrs + [SpanAttributes.HTTP_TARGET], + "http.server.duration": {*_duration_attrs, SpanAttributes.HTTP_TARGET}, }