From 71c7700e27c3974f74d1be973d845ee07beda361 Mon Sep 17 00:00:00 2001 From: Sebastian Kreft Date: Sat, 22 Oct 2022 10:29:53 -0300 Subject: [PATCH] feat(instrumentation/asgi): add target to metrics (#1323) --- CHANGELOG.md | 2 + .../instrumentation/asgi/__init__.py | 35 ++++++++++ .../tests/test_asgi_middleware.py | 64 +++++++++++++++++++ 3 files changed, 101 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 58c9c75d01..7f0c50b8e9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#1369](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1369)) - `opentelemetry-instrumentation-system-metrics` add supports to collect system thread count. ([#1339](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1339)) - `opentelemetry-exporter-richconsole` Fixing RichConsoleExpoter to allow multiple traces, fixing duplicate spans and include resources ([#1336](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1336)) +- `opentelemetry-instrumentation-asgi` metrics record target attribute (FastAPI only) + ([#1323](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1323)) ## [1.13.0-0.34b0](https://github.com/open-telemetry/opentelemetry-python/releases/tag/v1.13.0-0.34b0) - 2022-09-26 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 80e0e60eb2..dade87c4f4 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py @@ -366,6 +366,32 @@ def get_default_span_details(scope: dict) -> Tuple[str, dict]: return span_name, {} +def _collect_target_attribute( + scope: typing.Dict[str, typing.Any] +) -> typing.Optional[str]: + """ + Returns the target path as defined by the Semantic Conventions. + + This value is suitable to use in metrics as it should replace concrete + values with a parameterized name. Example: /api/users/{user_id} + + Refer to the specification + https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/semantic_conventions/http-metrics.md#parameterized-attributes + + Note: this function requires specific code for each framework, as there's no + standard attribute to use. + """ + # FastAPI + root_path = scope.get("root_path", "") + + route = scope.get("route") + path_format = getattr(route, "path_format", None) + if path_format: + return f"{root_path}{path_format}" + + return None + + class OpenTelemetryMiddleware: """The ASGI application middleware. @@ -387,6 +413,7 @@ class OpenTelemetryMiddleware: the current globally configured one is used. """ + # pylint: disable=too-many-branches def __init__( self, app, @@ -454,6 +481,12 @@ async def __call__(self, scope, receive, send): attributes ) 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: @@ -496,6 +529,8 @@ async def __call__(self, scope, receive, send): if token: context.detach(token) + # pylint: enable=too-many-branches + def _get_otel_receive(self, server_span_name, scope, receive): @wraps(receive) async def otel_receive(): diff --git a/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py b/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py index 1b00ee1279..a74e658438 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py @@ -12,6 +12,8 @@ # See the License for the specific language governing permissions and # limitations under the License. +# pylint: disable=too-many-lines + import sys import unittest from timeit import default_timer @@ -626,6 +628,37 @@ def test_basic_metric_success(self): ) self.assertEqual(point.value, 0) + def test_metric_target_attribute(self): + expected_target = "/api/user/{id}" + + class TestRoute: + path_format = expected_target + + self.scope["route"] = TestRoute() + app = otel_asgi.OpenTelemetryMiddleware(simple_asgi) + self.seed_app(app) + self.send_default_request() + + metrics_list = self.memory_metrics_reader.get_metrics_data() + assertions = 0 + for resource_metric in metrics_list.resource_metrics: + for scope_metrics in resource_metric.scope_metrics: + for metric in scope_metrics.metrics: + for point in metric.data.data_points: + if isinstance(point, HistogramDataPoint): + self.assertEqual( + point.attributes["http.target"], + 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): self.scope = { "type": "websocket", @@ -719,6 +752,37 @@ def test_credential_removal(self): attrs[SpanAttributes.HTTP_URL], "http://httpbin.org/status/200" ) + def test_collect_target_attribute_missing(self): + self.assertIsNone(otel_asgi._collect_target_attribute(self.scope)) + + def test_collect_target_attribute_fastapi(self): + class TestRoute: + path_format = "/api/users/{user_id}" + + self.scope["route"] = TestRoute() + self.assertEqual( + otel_asgi._collect_target_attribute(self.scope), + "/api/users/{user_id}", + ) + + def test_collect_target_attribute_fastapi_mounted(self): + class TestRoute: + path_format = "/users/{user_id}" + + self.scope["route"] = TestRoute() + self.scope["root_path"] = "/api/v2" + self.assertEqual( + otel_asgi._collect_target_attribute(self.scope), + "/api/v2/users/{user_id}", + ) + + def test_collect_target_attribute_fastapi_starlette_invalid(self): + self.scope["route"] = object() + self.assertIsNone( + otel_asgi._collect_target_attribute(self.scope), + "HTTP_TARGET values is not None", + ) + class TestWrappedApplication(AsgiTestBase): def test_mark_span_internal_in_presence_of_span_from_other_framework(self):