From fed3fe25ba2b2fc34deb7331266a24208a5c54d9 Mon Sep 17 00:00:00 2001 From: Iman Shafiei Date: Mon, 8 May 2023 15:56:35 -0700 Subject: [PATCH 01/10] Add http.server.response.size metric to ASGI implementation. Add new unit tests. --- .../instrumentation/asgi/__init__.py | 17 +++++++++++ .../tests/test_asgi_middleware.py | 30 +++++++++++++------ 2 files changed, 38 insertions(+), 9 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 6fc88d3eeb..70035fa5e9 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py @@ -501,6 +501,11 @@ def __init__( unit="ms", description="measures the duration of the inbound HTTP request", ) + self.server_response_size_histogram = self.meter.create_histogram( + name=MetricInstruments.HTTP_SERVER_RESPONSE_SIZE, + unit="By", + description="measures the size of HTTP response messages (compressed).", + ) self.active_requests_counter = self.meter.create_up_down_counter( name=MetricInstruments.HTTP_SERVER_ACTIVE_REQUESTS, unit="requests", @@ -615,6 +620,18 @@ def _get_otel_send( ): @wraps(send) async def otel_send(message): + if message.get("headers"): + headers = { + _key.decode("utf8"): _value.decode("utf8") + for (_key, _value) in message.get("headers") + } + if headers.get("content-length"): + target = _collect_target_attribute(scope) + if target: + duration_attrs[SpanAttributes.HTTP_TARGET] = target + self.server_response_size_histogram.record( + int(headers.get("content-length")), duration_attrs + ) with self.tracer.start_as_current_span( " ".join((server_span_name, scope["type"], "send")) ) as send_span: diff --git a/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py b/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py index bfa5720f99..b9627d196a 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py @@ -46,10 +46,12 @@ _expected_metric_names = [ "http.server.active_requests", "http.server.duration", + "http.server.response.size", ] _recommended_attrs = { "http.server.active_requests": _active_requests_count_attrs, "http.server.duration": _duration_attrs, + "http.server.response.size": _duration_attrs, } @@ -61,7 +63,10 @@ async def http_app(scope, receive, send): { "type": "http.response.start", "status": 200, - "headers": [[b"Content-Type", b"text/plain"]], + "headers": [ + [b"Content-Type", b"text/plain"], + [b"content-length", b"1024"], + ], } ) await send({"type": "http.response.body", "body": b"*"}) @@ -103,7 +108,10 @@ async def error_asgi(scope, receive, send): { "type": "http.response.start", "status": 200, - "headers": [[b"Content-Type", b"text/plain"]], + "headers": [ + [b"Content-Type", b"text/plain"], + [b"content-length", b"1024"], + ], } ) await send({"type": "http.response.body", "body": b"*"}) @@ -126,7 +134,8 @@ def validate_outputs(self, outputs, error=None, modifiers=None): # Check http response start self.assertEqual(response_start["status"], 200) self.assertEqual( - response_start["headers"], [[b"Content-Type", b"text/plain"]] + response_start["headers"], + [[b"Content-Type", b"text/plain"], [b"content-length", b"1024"]], ) exc_info = self.scope.get("hack_exc_info") @@ -352,6 +361,7 @@ def test_traceresponse_header(self): response_start["headers"], [ [b"Content-Type", b"text/plain"], + [b"content-length", b"1024"], [b"traceresponse", f"{traceresponse}".encode()], [b"access-control-expose-headers", b"traceresponse"], ], @@ -575,9 +585,12 @@ def test_basic_metric_success(self): dict(point.attributes), ) self.assertEqual(point.count, 1) - self.assertAlmostEqual( - duration, point.sum, delta=5 - ) + if metric.name == "http.server.duration": + self.assertAlmostEqual( + duration, point.sum, delta=5 + ) + elif metric.name == "http.server.response.size": + self.assertEqual(1024, point.sum) elif isinstance(point, NumberDataPoint): self.assertDictEqual( expected_requests_count_attributes, @@ -602,13 +615,12 @@ async def target_asgi(scope, receive, send): app = otel_asgi.OpenTelemetryMiddleware(target_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: - if metric.name != "http.server.duration": + if metric.name == "http.server.active_requests": continue for point in metric.data.data_points: if isinstance(point, HistogramDataPoint): @@ -617,7 +629,7 @@ async def target_asgi(scope, receive, send): expected_target, ) assertions += 1 - self.assertEqual(assertions, 1) + self.assertEqual(assertions, 2) def test_no_metric_for_websockets(self): self.scope = { From 54f9cbee40fb51f1afb30d6cc1c76d12e8ed3455 Mon Sep 17 00:00:00 2001 From: Iman Shafiei Date: Mon, 8 May 2023 16:09:51 -0700 Subject: [PATCH 02/10] Update changelog. --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 15e7f953c2..f0a914f991 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#1407](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1407)) - `opentelemetry-instrumentation-logging` Add `otelTraceSampled` to instrumetation-logging ([#1773](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1773)) +- `opentelemetry-instrumentation-asgi` Add `http.server.response.size` metric + ([#1789](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1789)) ### Fixed From 8dd938146150fcb4d7028fb438422a9465002a2e Mon Sep 17 00:00:00 2001 From: Iman Shafiei Date: Tue, 9 May 2023 12:21:47 -0700 Subject: [PATCH 03/10] Fix linting by disabling too-many-nested-blocks --- .../tests/test_asgi_middleware.py | 1 + 1 file changed, 1 insertion(+) diff --git a/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py b/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py index b9627d196a..095efe7fcf 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py @@ -575,6 +575,7 @@ def test_basic_metric_success(self): "http.flavor": "1.0", } metrics_list = self.memory_metrics_reader.get_metrics_data() + # pylint: disable=too-many-nested-blocks for resource_metric in metrics_list.resource_metrics: for scope_metrics in resource_metric.scope_metrics: for metric in scope_metrics.metrics: From fb3c287c175781eb5df7d807a6ac034316f36876 Mon Sep 17 00:00:00 2001 From: Iman Shafiei Date: Tue, 9 May 2023 13:52:58 -0700 Subject: [PATCH 04/10] Put new logic in a new method --- .../instrumentation/asgi/__init__.py | 27 ++++++++++--------- 1 file changed, 15 insertions(+), 12 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 70035fa5e9..5ba3d35def 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py @@ -519,6 +519,20 @@ def __init__( self.client_request_hook = client_request_hook self.client_response_hook = client_response_hook + def send_response_size_metric(self, message, scope, duration_attrs): + if message.get("headers"): + headers = { + _key.decode("utf8"): _value.decode("utf8") + for (_key, _value) in message.get("headers") + } + if headers.get("content-length"): + target = _collect_target_attribute(scope) + if target: + duration_attrs[SpanAttributes.HTTP_TARGET] = target + self.server_response_size_histogram.record( + int(headers.get("content-length")), duration_attrs + ) + async def __call__(self, scope, receive, send): """The ASGI application @@ -620,18 +634,7 @@ def _get_otel_send( ): @wraps(send) async def otel_send(message): - if message.get("headers"): - headers = { - _key.decode("utf8"): _value.decode("utf8") - for (_key, _value) in message.get("headers") - } - if headers.get("content-length"): - target = _collect_target_attribute(scope) - if target: - duration_attrs[SpanAttributes.HTTP_TARGET] = target - self.server_response_size_histogram.record( - int(headers.get("content-length")), duration_attrs - ) + self.send_response_size_metric(message, scope, duration_attrs) with self.tracer.start_as_current_span( " ".join((server_span_name, scope["type"], "send")) ) as send_span: From c9579b7f09954d782602770ad99722a7feff2c0a Mon Sep 17 00:00:00 2001 From: Iman Shafiei Date: Tue, 9 May 2023 14:57:55 -0700 Subject: [PATCH 05/10] Refactor the placement of new logic. --- .../instrumentation/asgi/__init__.py | 33 ++++++++++--------- 1 file changed, 18 insertions(+), 15 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 5ba3d35def..ee556ce6e0 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py @@ -518,20 +518,7 @@ def __init__( self.server_request_hook = server_request_hook self.client_request_hook = client_request_hook self.client_response_hook = client_response_hook - - def send_response_size_metric(self, message, scope, duration_attrs): - if message.get("headers"): - headers = { - _key.decode("utf8"): _value.decode("utf8") - for (_key, _value) in message.get("headers") - } - if headers.get("content-length"): - target = _collect_target_attribute(scope) - if target: - duration_attrs[SpanAttributes.HTTP_TARGET] = target - self.server_response_size_histogram.record( - int(headers.get("content-length")), duration_attrs - ) + self.content_length_header = None async def __call__(self, scope, receive, send): """The ASGI application @@ -607,6 +594,10 @@ async def __call__(self, scope, receive, send): self.active_requests_counter.add( -1, active_requests_count_attrs ) + if self.content_length_header: + self.server_response_size_histogram.record( + self.content_length_header, duration_attrs + ) if token: context.detach(token) @@ -634,7 +625,6 @@ def _get_otel_send( ): @wraps(send) async def otel_send(message): - self.send_response_size_metric(message, scope, duration_attrs) with self.tracer.start_as_current_span( " ".join((server_span_name, scope["type"], "send")) ) as send_span: @@ -675,6 +665,19 @@ async def otel_send(message): setter=asgi_setter, ) + if message.get("headers"): + headers = { + _key.decode("utf8"): _value.decode("utf8") + for (_key, _value) in message.get("headers") + } + if headers.get("content-length"): + try: + self.content_length_header = int( + headers["content-length"] + ) + except TypeError: + pass + await send(message) return otel_send From ba4a2443fe2733f495a2ad9c82d207988618dfe1 Mon Sep 17 00:00:00 2001 From: Iman Shafiei Date: Wed, 17 May 2023 09:55:40 -0700 Subject: [PATCH 06/10] Fixed the unit tests in FastAPI and Starlette --- .../tests/test_fastapi_instrumentation.py | 4 +++- .../tests/test_starlette_instrumentation.py | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py index 261b2e025f..b5eeddbbb0 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py @@ -44,10 +44,12 @@ _expected_metric_names = [ "http.server.active_requests", "http.server.duration", + "http.server.response.size", ] _recommended_attrs = { "http.server.active_requests": _active_requests_count_attrs, "http.server.duration": {*_duration_attrs, SpanAttributes.HTTP_TARGET}, + "http.server.response.size": {*_duration_attrs, SpanAttributes.HTTP_TARGET}, } @@ -187,7 +189,7 @@ def test_fastapi_metrics(self): for resource_metric in metrics_list.resource_metrics: self.assertTrue(len(resource_metric.scope_metrics) == 1) for scope_metric in resource_metric.scope_metrics: - self.assertTrue(len(scope_metric.metrics) == 2) + self.assertTrue(len(scope_metric.metrics) == 3) for metric in scope_metric.metrics: self.assertIn(metric.name, _expected_metric_names) data_points = list(metric.data.data_points) diff --git a/instrumentation/opentelemetry-instrumentation-starlette/tests/test_starlette_instrumentation.py b/instrumentation/opentelemetry-instrumentation-starlette/tests/test_starlette_instrumentation.py index 9c658e0092..5b2bc20cd9 100644 --- a/instrumentation/opentelemetry-instrumentation-starlette/tests/test_starlette_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-starlette/tests/test_starlette_instrumentation.py @@ -49,10 +49,12 @@ _expected_metric_names = [ "http.server.active_requests", "http.server.duration", + "http.server.response.size", ] _recommended_attrs = { "http.server.active_requests": _active_requests_count_attrs, "http.server.duration": _duration_attrs, + "http.server.response.size": _duration_attrs, } @@ -128,7 +130,7 @@ def test_starlette_metrics(self): for resource_metric in metrics_list.resource_metrics: self.assertTrue(len(resource_metric.scope_metrics) == 1) for scope_metric in resource_metric.scope_metrics: - self.assertTrue(len(scope_metric.metrics) == 2) + self.assertTrue(len(scope_metric.metrics) == 3) for metric in scope_metric.metrics: self.assertIn(metric.name, _expected_metric_names) data_points = list(metric.data.data_points) From 1a50c1511b90b988ddcedb0647e9434e97548e39 Mon Sep 17 00:00:00 2001 From: Iman Shafiei Date: Wed, 17 May 2023 13:00:06 -0700 Subject: [PATCH 07/10] Update changelog. --- CHANGELOG.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index da488f1164..1cc5a47785 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased +- `opentelemetry-instrumentation-asgi` Add `http.server.response.size` metric + ([#1789](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1789)) + ## Version 1.18.0/0.39b0 (2023-05-10) - `opentelemetry-instrumentation-system-metrics` Add `process.` prefix to `runtime.memory`, `runtime.cpu.time`, and `runtime.gc_count`. Change `runtime.memory` from count to UpDownCounter. ([#1735](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1735)) @@ -29,8 +32,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#1407](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1407)) - `opentelemetry-instrumentation-logging` Add `otelTraceSampled` to instrumetation-logging ([#1773](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1773)) -- `opentelemetry-instrumentation-asgi` Add `http.server.response.size` metric - ([#1789](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1789)) ### Fixed From 050e69c9629c2276e03bcb36eacd92e8c2c7b8bf Mon Sep 17 00:00:00 2001 From: Iman Shafiei Date: Tue, 23 May 2023 08:44:06 -0700 Subject: [PATCH 08/10] FIx lint errors. --- .../tests/test_fastapi_instrumentation.py | 5 ++++- 1 file changed, 4 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 b5eeddbbb0..97b7500051 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py @@ -49,7 +49,10 @@ _recommended_attrs = { "http.server.active_requests": _active_requests_count_attrs, "http.server.duration": {*_duration_attrs, SpanAttributes.HTTP_TARGET}, - "http.server.response.size": {*_duration_attrs, SpanAttributes.HTTP_TARGET}, + "http.server.response.size": { + *_duration_attrs, + SpanAttributes.HTTP_TARGET, + }, } From be70c8ba3e4f84529d41819d3b121585daa3e39c Mon Sep 17 00:00:00 2001 From: Iman Shafiei Date: Mon, 19 Jun 2023 10:35:10 -0700 Subject: [PATCH 09/10] Refactor getting content-length header --- .../instrumentation/asgi/__init__.py | 15 +++------------ 1 file changed, 3 insertions(+), 12 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 e0a8efcec9..0d8a9f5c71 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py @@ -670,18 +670,9 @@ async def otel_send(message): setter=asgi_setter, ) - if message.get("headers"): - headers = { - _key.decode("utf8"): _value.decode("utf8") - for (_key, _value) in message.get("headers") - } - if headers.get("content-length"): - try: - self.content_length_header = int( - headers["content-length"] - ) - except TypeError: - pass + content_length = asgi_getter.get(message, "content-length") + if content_length: + self.content_length_header = int(content_length[0]) await send(message) From edf0f92bc9a35ef34c174d5bcd6e45b842041133 Mon Sep 17 00:00:00 2001 From: Iman Shafiei Date: Mon, 19 Jun 2023 11:07:54 -0700 Subject: [PATCH 10/10] Refactor getting content-length header --- .../src/opentelemetry/instrumentation/asgi/__init__.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) 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 0d8a9f5c71..1ee25ae7d9 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py @@ -672,7 +672,10 @@ async def otel_send(message): content_length = asgi_getter.get(message, "content-length") if content_length: - self.content_length_header = int(content_length[0]) + try: + self.content_length_header = int(content_length[0]) + except ValueError: + pass await send(message)