From 8684258cbffa9672f4ae157ffd0b229400367720 Mon Sep 17 00:00:00 2001 From: "anshul.asawa" Date: Tue, 12 Jul 2022 12:25:37 +0530 Subject: [PATCH 01/18] add metric instumentation --- .../src/opentelemetry/instrumentation/fastapi/__init__.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py index 5cf17d36f0..cf347cb88e 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py @@ -165,6 +165,7 @@ def instrument_app( client_request_hook: _ClientRequestHookT = None, client_response_hook: _ClientResponseHookT = None, tracer_provider=None, + meter_provider=None, excluded_urls=None, ): """Instrument an uninstrumented FastAPI application.""" @@ -185,6 +186,7 @@ def instrument_app( client_request_hook=client_request_hook, client_response_hook=client_response_hook, tracer_provider=tracer_provider, + meter_provider=meter_provider, ) app._is_instrumented_by_opentelemetry = True else: @@ -223,6 +225,7 @@ def _instrument(self, **kwargs): if _excluded_urls is None else parse_excluded_urls(_excluded_urls) ) + _InstrumentedFastAPI._meter_provider = kwargs.get("meter_provider") fastapi.FastAPI = _InstrumentedFastAPI def _uninstrument(self, **kwargs): @@ -231,6 +234,7 @@ def _uninstrument(self, **kwargs): class _InstrumentedFastAPI(fastapi.FastAPI): _tracer_provider = None + _meter_provider = None _excluded_urls = None _server_request_hook: _ServerRequestHookT = None _client_request_hook: _ClientRequestHookT = None @@ -246,6 +250,7 @@ def __init__(self, *args, **kwargs): client_request_hook=_InstrumentedFastAPI._client_request_hook, client_response_hook=_InstrumentedFastAPI._client_response_hook, tracer_provider=_InstrumentedFastAPI._tracer_provider, + meter_provider=_InstrumentedFastAPI._meter_provider, ) From a01f115365638da65669e9e8d68e60c982ea83ef Mon Sep 17 00:00:00 2001 From: "anshul.asawa" Date: Mon, 18 Jul 2022 14:36:38 +0530 Subject: [PATCH 02/18] add metric instrumentation changes I --- .../instrumentation/asgi/__init__.py | 51 ++++++++++++++++++- 1 file changed, 50 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 d2e42450a0..f788fc2950 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py @@ -150,10 +150,12 @@ def client_response_hook(span: Span, message: dict): import urllib from functools import wraps from typing import Tuple +from timeit import default_timer from asgiref.compatibility import guarantee_single_callable from opentelemetry import context, trace +from opentelemetry.metrics import get_meter from opentelemetry.instrumentation.asgi.version import __version__ # noqa from opentelemetry.instrumentation.propagators import ( get_global_response_propagator, @@ -179,6 +181,25 @@ def client_response_hook(span: Span, message: dict): _ClientRequestHookT = typing.Optional[typing.Callable[[Span, dict], None]] _ClientResponseHookT = typing.Optional[typing.Callable[[Span, dict], None]] +# List of recommended attributes +_duration_attrs = [ + SpanAttributes.HTTP_METHOD, + SpanAttributes.HTTP_HOST, + SpanAttributes.HTTP_SCHEME, + SpanAttributes.HTTP_STATUS_CODE, + SpanAttributes.HTTP_FLAVOR, + SpanAttributes.HTTP_SERVER_NAME, + SpanAttributes.NET_HOST_NAME, + SpanAttributes.NET_HOST_PORT, +] + +_active_requests_count_attrs = [ + SpanAttributes.HTTP_METHOD, + SpanAttributes.HTTP_HOST, + SpanAttributes.HTTP_SCHEME, + SpanAttributes.HTTP_FLAVOR, + SpanAttributes.HTTP_SERVER_NAME, +] class ASGIGetter(Getter[dict]): def get( @@ -360,6 +381,20 @@ def get_default_span_details(scope: dict) -> Tuple[str, dict]: return span_name, {} +def parse_active_request_count_attrs(req_attrs): + active_requests_count_attrs = {} + for attr_key in _active_requests_count_attrs: + if req_attrs.get(attr_key) is not None: + active_requests_count_attrs[attr_key] = req_attrs[attr_key] + return active_requests_count_attrs + +def parse_duration_attrs(req_attrs): + duration_attrs = {} + for attr_key in _duration_attrs: + if req_attrs.get(attr_key) is not None: + duration_attrs[attr_key] = req_attrs[attr_key] + return duration_attrs + class OpenTelemetryMiddleware: """The ASGI application middleware. @@ -391,9 +426,21 @@ def __init__( client_request_hook: _ClientRequestHookT = None, client_response_hook: _ClientResponseHookT = None, tracer_provider=None, + meter_provider=None, ): self.app = guarantee_single_callable(app) self.tracer = trace.get_tracer(__name__, __version__, tracer_provider) + self.meter = get_meter(__name__, __version__, meter_provider) + self.duration_histogram = self.meter.create_histogram( + name="http.server.duration", + unit="ms", + description="measures the duration of the inbound HTTP request", + ) + self.active_requests_counter = self.meter.create_up_down_counter( + name="http.server.active_requests", + unit="requests", + description="measures the number of concurrent HTTP requests that are currently in-flight", + ) self.excluded_urls = excluded_urls self.default_span_details = ( default_span_details or get_default_span_details @@ -426,7 +473,7 @@ async def __call__(self, scope, receive, send): context_carrier=scope, context_getter=asgi_getter, ) - + start = default_timer() try: with trace.use_span(span, end_on_exit=True) as current_span: if current_span.is_recording(): @@ -434,6 +481,8 @@ async def __call__(self, scope, receive, send): attributes.update(additional_attributes) for key, value in attributes.items(): current_span.set_attribute(key, value) + active_requests_count_attrs = parse_active_request_count_attrs(attributes) + duration_attrs = parse_duration_attrs(attributes) if current_span.kind == trace.SpanKind.SERVER: custom_attributes = ( From 1cbf9ebd3f4a32248ccb7ce43dcfcf7dd874f2ad Mon Sep 17 00:00:00 2001 From: "anshul.asawa" Date: Tue, 19 Jul 2022 16:30:14 +0530 Subject: [PATCH 03/18] add metric instrumentation in asgi --- .../instrumentation/asgi/__init__.py | 20 ++++++--- .../tests/test_asgi_middleware.py | 44 +++++++++++++++++++ 2 files changed, 57 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 f788fc2950..ffa4e632ec 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py @@ -381,14 +381,14 @@ def get_default_span_details(scope: dict) -> Tuple[str, dict]: return span_name, {} -def parse_active_request_count_attrs(req_attrs): +def _parse_active_request_count_attrs(req_attrs): active_requests_count_attrs = {} for attr_key in _active_requests_count_attrs: if req_attrs.get(attr_key) is not None: active_requests_count_attrs[attr_key] = req_attrs[attr_key] return active_requests_count_attrs -def parse_duration_attrs(req_attrs): +def _parse_duration_attrs(req_attrs): duration_attrs = {} for attr_key in _duration_attrs: if req_attrs.get(attr_key) is not None: @@ -473,16 +473,17 @@ async def __call__(self, scope, receive, send): context_carrier=scope, context_getter=asgi_getter, ) + attributes = collect_request_attributes(scope) + attributes.update(additional_attributes) + active_requests_count_attrs = _parse_active_request_count_attrs(attributes) + duration_attrs = _parse_duration_attrs(attributes) start = default_timer() + self.active_requests_counter.add(1, active_requests_count_attrs) try: with trace.use_span(span, end_on_exit=True) as current_span: if current_span.is_recording(): - attributes = collect_request_attributes(scope) - attributes.update(additional_attributes) for key, value in attributes.items(): current_span.set_attribute(key, value) - active_requests_count_attrs = parse_active_request_count_attrs(attributes) - duration_attrs = parse_duration_attrs(attributes) if current_span.kind == trace.SpanKind.SERVER: custom_attributes = ( @@ -503,10 +504,14 @@ async def __call__(self, scope, receive, send): span_name, scope, send, + duration_attrs, ) await self.app(scope, otel_receive, otel_send) finally: + duration = max(round((default_timer() - start) * 1000), 0) + self.duration_histogram.record(duration, duration_attrs) + self.active_requests_counter.add(-1, active_requests_count_attrs) if token: context.detach(token) @@ -527,7 +532,7 @@ async def otel_receive(): return otel_receive - def _get_otel_send(self, server_span, server_span_name, scope, send): + def _get_otel_send(self, server_span, server_span_name, scope, send, duration_attrs): @wraps(send) async def otel_send(message): with self.tracer.start_as_current_span( @@ -538,6 +543,7 @@ async def otel_send(message): if send_span.is_recording(): if message["type"] == "http.response.start": status_code = message["status"] + duration_attrs[SpanAttributes.HTTP_STATUS_CODE] = status_code set_status_code(server_span, status_code) set_status_code(send_span, status_code) elif message["type"] == "websocket.send": diff --git a/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py b/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py index 3a1e8424a8..df6b413445 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py @@ -35,7 +35,19 @@ OTEL_INSTRUMENTATION_HTTP_CAPTURE_HEADERS_SERVER_REQUEST, OTEL_INSTRUMENTATION_HTTP_CAPTURE_HEADERS_SERVER_RESPONSE, ) +from opentelemetry.sdk.metrics.export import ( + HistogramDataPoint, + NumberDataPoint, +) +_expected_metric_names = [ + "http.server.active_requests", + "http.server.duration", +] +_recommended_attrs = { + "http.server.active_requests": otel_asgi._active_requests_count_attrs, + "http.server.duration": otel_asgi._duration_attrs, +} async def http_app(scope, receive, send): message = await receive() @@ -523,6 +535,38 @@ def update_expected_hook_results(expected): outputs, modifiers=[update_expected_hook_results] ) + def test_asgi_metrics(self): + app = otel_asgi.OpenTelemetryMiddleware(simple_asgi) + self.seed_app(app) + self.send_default_request() + self.seed_app(app) + self.send_default_request() + self.seed_app(app) + self.send_default_request() + metrics_list = self.memory_metrics_reader.get_metrics_data() + number_data_point_seen = False + histogram_data_point_seen = False + self.assertTrue(len(metrics_list.resource_metrics) != 0) + for resource_metric in metrics_list.resource_metrics: + self.assertTrue(len(resource_metric.scope_metrics) != 0) + for scope_metric in resource_metric.scope_metrics: + self.assertTrue(len(scope_metric.metrics) != 0) + for metric in scope_metric.metrics: + self.assertIn(metric.name, _expected_metric_names) + data_points = list(metric.data.data_points) + self.assertEqual(len(data_points), 1) + for point in data_points: + if isinstance(point, HistogramDataPoint): + self.assertEqual(point.count, 3) + histogram_data_point_seen = True + if isinstance(point, NumberDataPoint): + number_data_point_seen = True + for attr in point.attributes: + self.assertIn( + attr, _recommended_attrs[metric.name] + ) + self.assertTrue(number_data_point_seen and histogram_data_point_seen) + class TestAsgiAttributes(unittest.TestCase): def setUp(self): From 8a452c0eb297a618b90aa31f61174be1fbfc7cda Mon Sep 17 00:00:00 2001 From: "anshul.asawa" Date: Tue, 19 Jul 2022 16:38:45 +0530 Subject: [PATCH 04/18] add change log --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 20cdb6cb5f..c1350b27fd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `opentelemetry-instrumentation-redis` add support to instrument RedisCluster clients ([#1177](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1177)) - `opentelemetry-instrumentation-sqlalchemy` Added span for the connection phase ([#1133](https://github.com/open-telemetry/opentelemetry-python-contrib/issues/1133)) +- Add metric instrumentation in asgi + ([#1197](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1197)) ## [1.12.0rc2-0.32b0](https://github.com/open-telemetry/opentelemetry-python/releases/tag/v1.12.0rc2-0.32b0) - 2022-07-01 From 5bf7fb9e54473130b338fccbe912408aa9ff3b26 Mon Sep 17 00:00:00 2001 From: "anshul.asawa" Date: Tue, 19 Jul 2022 16:43:20 +0530 Subject: [PATCH 05/18] fix lint --- .../instrumentation/asgi/__init__.py | 19 ++++++++++++++----- .../tests/test_asgi_middleware.py | 9 +++++---- 2 files changed, 19 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 ffa4e632ec..28f5b642bd 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py @@ -149,13 +149,12 @@ def client_response_hook(span: Span, message: dict): import typing import urllib from functools import wraps -from typing import Tuple from timeit import default_timer +from typing import Tuple from asgiref.compatibility import guarantee_single_callable from opentelemetry import context, trace -from opentelemetry.metrics import get_meter from opentelemetry.instrumentation.asgi.version import __version__ # noqa from opentelemetry.instrumentation.propagators import ( get_global_response_propagator, @@ -164,6 +163,7 @@ def client_response_hook(span: Span, message: dict): _start_internal_or_server_span, http_status_to_status_code, ) +from opentelemetry.metrics import get_meter from opentelemetry.propagators.textmap import Getter, Setter from opentelemetry.semconv.trace import SpanAttributes from opentelemetry.trace import Span, set_span_in_context @@ -201,6 +201,7 @@ def client_response_hook(span: Span, message: dict): SpanAttributes.HTTP_SERVER_NAME, ] + class ASGIGetter(Getter[dict]): def get( self, carrier: dict, key: str @@ -381,6 +382,7 @@ def get_default_span_details(scope: dict) -> Tuple[str, dict]: return span_name, {} + def _parse_active_request_count_attrs(req_attrs): active_requests_count_attrs = {} for attr_key in _active_requests_count_attrs: @@ -388,6 +390,7 @@ def _parse_active_request_count_attrs(req_attrs): active_requests_count_attrs[attr_key] = req_attrs[attr_key] return active_requests_count_attrs + def _parse_duration_attrs(req_attrs): duration_attrs = {} for attr_key in _duration_attrs: @@ -475,7 +478,9 @@ async def __call__(self, scope, receive, send): ) attributes = collect_request_attributes(scope) attributes.update(additional_attributes) - active_requests_count_attrs = _parse_active_request_count_attrs(attributes) + active_requests_count_attrs = _parse_active_request_count_attrs( + attributes + ) duration_attrs = _parse_duration_attrs(attributes) start = default_timer() self.active_requests_counter.add(1, active_requests_count_attrs) @@ -532,7 +537,9 @@ async def otel_receive(): return otel_receive - def _get_otel_send(self, server_span, server_span_name, scope, send, duration_attrs): + def _get_otel_send( + self, server_span, server_span_name, scope, send, duration_attrs + ): @wraps(send) async def otel_send(message): with self.tracer.start_as_current_span( @@ -543,7 +550,9 @@ async def otel_send(message): if send_span.is_recording(): if message["type"] == "http.response.start": status_code = message["status"] - duration_attrs[SpanAttributes.HTTP_STATUS_CODE] = status_code + duration_attrs[ + SpanAttributes.HTTP_STATUS_CODE + ] = status_code set_status_code(server_span, status_code) set_status_code(send_span, status_code) elif message["type"] == "websocket.send": diff --git a/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py b/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py index df6b413445..2c82ba545c 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py @@ -24,6 +24,10 @@ set_global_response_propagator, ) from opentelemetry.sdk import resources +from opentelemetry.sdk.metrics.export import ( + HistogramDataPoint, + NumberDataPoint, +) from opentelemetry.semconv.trace import SpanAttributes from opentelemetry.test.asgitestutil import ( AsgiTestBase, @@ -35,10 +39,6 @@ OTEL_INSTRUMENTATION_HTTP_CAPTURE_HEADERS_SERVER_REQUEST, OTEL_INSTRUMENTATION_HTTP_CAPTURE_HEADERS_SERVER_RESPONSE, ) -from opentelemetry.sdk.metrics.export import ( - HistogramDataPoint, - NumberDataPoint, -) _expected_metric_names = [ "http.server.active_requests", @@ -49,6 +49,7 @@ "http.server.duration": otel_asgi._duration_attrs, } + async def http_app(scope, receive, send): message = await receive() assert scope["type"] == "http" From 8f6a5a9dcdac78c5d9bb34bf307ab22b7a8ed480 Mon Sep 17 00:00:00 2001 From: "anshul.asawa" Date: Tue, 19 Jul 2022 17:07:48 +0530 Subject: [PATCH 06/18] add test for fastapi metrics --- .../tests/test_fastapi_instrumentation.py | 47 ++++++++++++++++++- 1 file changed, 46 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 e4a0960a26..8913e26df9 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py @@ -21,7 +21,15 @@ import opentelemetry.instrumentation.fastapi as otel_fastapi from opentelemetry import trace -from opentelemetry.instrumentation.asgi import OpenTelemetryMiddleware +from opentelemetry.instrumentation.asgi import ( + OpenTelemetryMiddleware, + _active_requests_count_attrs, + _duration_attrs, +) +from opentelemetry.sdk.metrics.export import ( + HistogramDataPoint, + NumberDataPoint, +) from opentelemetry.sdk.resources import Resource from opentelemetry.semconv.trace import SpanAttributes from opentelemetry.test.globals_test import reset_trace_globals @@ -32,6 +40,15 @@ get_excluded_urls, ) +_expected_metric_names = [ + "http.server.active_requests", + "http.server.duration", +] +_recommended_attrs = { + "http.server.active_requests": _active_requests_count_attrs, + "http.server.duration": _duration_attrs, +} + class TestFastAPIManualInstrumentation(TestBase): def _create_app(self): @@ -161,6 +178,34 @@ def test_fastapi_excluded_urls_not_env(self): spans = self.memory_exporter.get_finished_spans() self.assertEqual(len(spans), 0) + def test_fastapi_metrics(self): + self._client.get("/foobar") + self._client.get("/foobar") + self._client.get("/foobar") + metrics_list = self.memory_metrics_reader.get_metrics_data() + number_data_point_seen = False + histogram_data_point_seen = False + self.assertTrue(len(metrics_list.resource_metrics) != 0) + for resource_metric in metrics_list.resource_metrics: + self.assertTrue(len(resource_metric.scope_metrics) != 0) + for scope_metric in resource_metric.scope_metrics: + self.assertTrue(len(scope_metric.metrics) != 0) + for metric in scope_metric.metrics: + self.assertIn(metric.name, _expected_metric_names) + data_points = list(metric.data.data_points) + self.assertEqual(len(data_points), 1) + for point in data_points: + if isinstance(point, HistogramDataPoint): + self.assertEqual(point.count, 3) + histogram_data_point_seen = True + if isinstance(point, NumberDataPoint): + number_data_point_seen = True + for attr in point.attributes: + self.assertIn( + attr, _recommended_attrs[metric.name] + ) + self.assertTrue(number_data_point_seen and histogram_data_point_seen) + @staticmethod def _create_fastapi_app(): app = fastapi.FastAPI() From b7e1b3338d4156a7e3f862cfe2c55376ba6b0cfd Mon Sep 17 00:00:00 2001 From: "anshul.asawa" Date: Wed, 20 Jul 2022 11:34:57 +0530 Subject: [PATCH 07/18] add change log --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c1350b27fd..454ec797b8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `opentelemetry-instrumentation-sqlalchemy` Added span for the connection phase ([#1133](https://github.com/open-telemetry/opentelemetry-python-contrib/issues/1133)) - Add metric instrumentation in asgi ([#1197](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1197)) +- Add metric instrumentation in fastapi + ([#1199](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1199)) ## [1.12.0rc2-0.32b0](https://github.com/open-telemetry/opentelemetry-python/releases/tag/v1.12.0rc2-0.32b0) - 2022-07-01 From d2df8b179835e2153d444048056d15d49ef4bba5 Mon Sep 17 00:00:00 2001 From: "anshul.asawa" Date: Wed, 27 Jul 2022 12:58:10 +0530 Subject: [PATCH 08/18] change time duration start time position for metric --- .../src/opentelemetry/instrumentation/asgi/__init__.py | 2 +- 1 file changed, 1 insertion(+), 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 28f5b642bd..3e41f739f1 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py @@ -482,7 +482,6 @@ async def __call__(self, scope, receive, send): attributes ) duration_attrs = _parse_duration_attrs(attributes) - start = default_timer() self.active_requests_counter.add(1, active_requests_count_attrs) try: with trace.use_span(span, end_on_exit=True) as current_span: @@ -511,6 +510,7 @@ async def __call__(self, scope, receive, send): send, duration_attrs, ) + start = default_timer() await self.app(scope, otel_receive, otel_send) finally: From f4b89621738c09c3d57e4ec9f6c405c34837bb51 Mon Sep 17 00:00:00 2001 From: "anshul.asawa" Date: Thu, 28 Jul 2022 14:06:20 +0530 Subject: [PATCH 09/18] add basic success test --- .../tests/test_asgi_middleware.py | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py b/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py index 2c82ba545c..4965394772 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py @@ -568,6 +568,42 @@ def test_asgi_metrics(self): ) self.assertTrue(number_data_point_seen and histogram_data_point_seen) + def test_basic_metric_success(self): + app = otel_asgi.OpenTelemetryMiddleware(simple_asgi) + self.seed_app(app) + self.send_default_request() + expected_duration_attributes = { + "http.method": "GET", + "http.host": "127.0.0.1", + "http.scheme": "http", + "http.flavor": "1.0", + "net.host.port": 80, + "http.status_code": 200, + } + expected_requests_count_attributes = { + "http.method": "GET", + "http.host": "127.0.0.1", + "http.scheme": "http", + "http.flavor": "1.0", + } + metrics_list = self.memory_metrics_reader.get_metrics_data() + 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 list(metric.data.data_points): + if isinstance(point, HistogramDataPoint): + self.assertDictEqual( + expected_duration_attributes, + dict(point.attributes), + ) + self.assertEqual(point.count, 1) + elif isinstance(point, NumberDataPoint): + self.assertDictEqual( + expected_requests_count_attributes, + dict(point.attributes), + ) + self.assertEqual(point.value, 0) + class TestAsgiAttributes(unittest.TestCase): def setUp(self): From 0874f2365431a6d44f27cb00137e5fd6b7fd8a68 Mon Sep 17 00:00:00 2001 From: "anshul.asawa" Date: Fri, 29 Jul 2022 14:52:46 +0530 Subject: [PATCH 10/18] remove metric instrumentation for websockets --- .../opentelemetry/instrumentation/asgi/__init__.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 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 3e41f739f1..b737dad2c8 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py @@ -482,7 +482,8 @@ async def __call__(self, scope, receive, send): attributes ) duration_attrs = _parse_duration_attrs(attributes) - self.active_requests_counter.add(1, active_requests_count_attrs) + if scope["type"] == "http": + self.active_requests_counter.add(1, active_requests_count_attrs) try: with trace.use_span(span, end_on_exit=True) as current_span: if current_span.is_recording(): @@ -514,9 +515,12 @@ async def __call__(self, scope, receive, send): await self.app(scope, otel_receive, otel_send) finally: - duration = max(round((default_timer() - start) * 1000), 0) - self.duration_histogram.record(duration, duration_attrs) - self.active_requests_counter.add(-1, active_requests_count_attrs) + if scope["type"] == "http": + duration = max(round((default_timer() - start) * 1000), 0) + self.duration_histogram.record(duration, duration_attrs) + self.active_requests_counter.add( + -1, active_requests_count_attrs + ) if token: context.detach(token) From 1aef36e13995549bb2ff6bcbd52bbc6c85b1952a Mon Sep 17 00:00:00 2001 From: "anshul.asawa" Date: Tue, 2 Aug 2022 13:28:50 +0530 Subject: [PATCH 11/18] add value test and websocket test for metric --- .../tests/test_asgi_middleware.py | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py b/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py index 4965394772..0f9669ee57 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py @@ -14,6 +14,7 @@ import sys import unittest +from timeit import default_timer from unittest import mock import opentelemetry.instrumentation.asgi as otel_asgi @@ -571,7 +572,9 @@ def test_asgi_metrics(self): def test_basic_metric_success(self): app = otel_asgi.OpenTelemetryMiddleware(simple_asgi) self.seed_app(app) + start = default_timer() self.send_default_request() + duration = max(round((default_timer() - start) * 1000), 0) expected_duration_attributes = { "http.method": "GET", "http.host": "127.0.0.1", @@ -597,6 +600,9 @@ def test_basic_metric_success(self): dict(point.attributes), ) self.assertEqual(point.count, 1) + self.assertAlmostEqual( + duration, point.sum, delta=5 + ) elif isinstance(point, NumberDataPoint): self.assertDictEqual( expected_requests_count_attributes, @@ -604,6 +610,28 @@ def test_basic_metric_success(self): ) self.assertEqual(point.value, 0) + def test_no_metric_for_websockets(self): + self.scope = { + "type": "websocket", + "http_version": "1.1", + "scheme": "ws", + "path": "/", + "query_string": b"", + "headers": [], + "client": ("127.0.0.1", 32767), + "server": ("127.0.0.1", 80), + } + app = otel_asgi.OpenTelemetryMiddleware(simple_asgi) + self.seed_app(app) + self.send_input({"type": "websocket.connect"}) + self.send_input({"type": "websocket.receive", "text": "ping"}) + self.send_input({"type": "websocket.disconnect"}) + self.get_all_output() + metrics_list = self.memory_metrics_reader.get_metrics_data() + self.assertEqual( + len(metrics_list.resource_metrics[0].scope_metrics), 0 + ) + class TestAsgiAttributes(unittest.TestCase): def setUp(self): From 4f8dfe36a8fe68268a43c8aa95d4eeaa87d7828d Mon Sep 17 00:00:00 2001 From: "anshul.asawa" Date: Tue, 2 Aug 2022 14:15:51 +0530 Subject: [PATCH 12/18] add metric tests for fastapi --- .../tests/test_fastapi_instrumentation.py | 73 +++++++++++++++++++ 1 file changed, 73 insertions(+) diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py index 8913e26df9..752c258510 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py @@ -13,6 +13,7 @@ # limitations under the License. import unittest +from timeit import default_timer from unittest.mock import patch import fastapi @@ -206,6 +207,78 @@ def test_fastapi_metrics(self): ) self.assertTrue(number_data_point_seen and histogram_data_point_seen) + def test_basic_metric_success(self): + start = default_timer() + self._client.get("/foobar") + duration = max(round((default_timer() - start) * 1000), 0) + expected_duration_attributes = { + "http.method": "GET", + "http.host": "testserver", + "http.scheme": "http", + "http.flavor": "1.1", + "http.server_name": "testserver", + "net.host.port": 80, + "http.status_code": 200, + } + expected_requests_count_attributes = { + "http.method": "GET", + "http.host": "testserver", + "http.scheme": "http", + "http.flavor": "1.1", + "http.server_name": "testserver", + } + metrics_list = self.memory_metrics_reader.get_metrics_data() + for resource_metric in metrics_list.resource_metrics: + for scope_metric in resource_metric.scope_metrics: + for metric in scope_metric.metrics: + for point in list(metric.data.data_points): + if isinstance(point, HistogramDataPoint): + self.assertDictEqual( + expected_duration_attributes, + dict(point.attributes), + ) + self.assertEqual(point.count, 1) + self.assertAlmostEqual( + duration, point.sum, delta=10 + ) + if isinstance(point, NumberDataPoint): + self.assertDictEqual( + expected_requests_count_attributes, + dict(point.attributes), + ) + self.assertEqual(point.value, 0) + + def test_basic_post_request_metric_success(self): + start = default_timer() + self._client.post("/foobar") + duration = max(round((default_timer() - start) * 1000), 0) + metrics_list = self.memory_metrics_reader.get_metrics_data() + for resource_metric in metrics_list.resource_metrics: + for scope_metric in resource_metric.scope_metrics: + for metric in scope_metric.metrics: + for point in list(metric.data.data_points): + if isinstance(point, HistogramDataPoint): + self.assertEqual(point.count, 1) + self.assertAlmostEqual( + duration, point.sum, delta=10 + ) + if isinstance(point, NumberDataPoint): + self.assertEqual(point.value, 0) + + def test_metric_uninstruemnt(self): + self._client.get("/foobar") + self._instrumentor.uninstrument_app(self._app) + self._client.get("/foobar") + metrics_list = self.memory_metrics_reader.get_metrics_data() + for resource_metric in metrics_list.resource_metrics: + for scope_metric in resource_metric.scope_metrics: + for metric in scope_metric.metrics: + for point in list(metric.data.data_points): + if isinstance(point, HistogramDataPoint): + self.assertEqual(point.count, 1) + if isinstance(point, NumberDataPoint): + self.assertEqual(point.value, 0) + @staticmethod def _create_fastapi_app(): app = fastapi.FastAPI() From e724a982b0430614334fd432174431d40d40730a Mon Sep 17 00:00:00 2001 From: "anshul.asawa" Date: Tue, 9 Aug 2022 13:52:56 +0530 Subject: [PATCH 13/18] fix error --- .../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 752c258510..8d53838cba 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py @@ -239,7 +239,7 @@ def test_basic_metric_success(self): ) self.assertEqual(point.count, 1) self.assertAlmostEqual( - duration, point.sum, delta=10 + duration, point.sum, delta=20 ) if isinstance(point, NumberDataPoint): self.assertDictEqual( From 48dbe5f00653bddb76c9a0bf8a4874be188391c3 Mon Sep 17 00:00:00 2001 From: "anshul.asawa" Date: Tue, 16 Aug 2022 15:31:18 +0530 Subject: [PATCH 14/18] add meter in asgi instead of just passing meter provider --- .../src/opentelemetry/instrumentation/asgi/__init__.py | 7 ++++++- .../src/opentelemetry/instrumentation/fastapi/__init__.py | 5 ++++- 2 files changed, 10 insertions(+), 2 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 14b6427348..69973c382f 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py @@ -432,10 +432,15 @@ def __init__( client_response_hook: _ClientResponseHookT = None, tracer_provider=None, meter_provider=None, + meter=None, ): self.app = guarantee_single_callable(app) self.tracer = trace.get_tracer(__name__, __version__, tracer_provider) - self.meter = get_meter(__name__, __version__, meter_provider) + self.meter = ( + get_meter(__name__, __version__, meter_provider) + if meter is None + else meter + ) self.duration_histogram = self.meter.create_histogram( name="http.server.duration", unit="ms", diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py index cf347cb88e..8e72ec1e2e 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py @@ -137,7 +137,9 @@ def client_response_hook(span: Span, message: dict): from opentelemetry.instrumentation.asgi import OpenTelemetryMiddleware from opentelemetry.instrumentation.asgi.package import _instruments +from opentelemetry.instrumentation.fastapi.version import __version__ from opentelemetry.instrumentation.instrumentor import BaseInstrumentor +from opentelemetry.metrics import get_meter from opentelemetry.semconv.trace import SpanAttributes from opentelemetry.trace import Span from opentelemetry.util.http import get_excluded_urls, parse_excluded_urls @@ -177,6 +179,7 @@ def instrument_app( excluded_urls = _excluded_urls_from_env else: excluded_urls = parse_excluded_urls(excluded_urls) + meter = get_meter(__name__, __version__, meter_provider) app.add_middleware( OpenTelemetryMiddleware, @@ -186,7 +189,7 @@ def instrument_app( client_request_hook=client_request_hook, client_response_hook=client_response_hook, tracer_provider=tracer_provider, - meter_provider=meter_provider, + meter=meter, ) app._is_instrumented_by_opentelemetry = True else: From 975dde685e5a682bab804cdd47cd94f7e7372181 Mon Sep 17 00:00:00 2001 From: "anshul.asawa" Date: Thu, 18 Aug 2022 15:20:04 +0530 Subject: [PATCH 15/18] fix asgi duplicate changes --- CHANGELOG.md | 6 +- .../instrumentation/asgi/__init__.py | 36 -------- .../instrumentation/fastapi/__init__.py | 5 +- .../tests/test_fastapi_instrumentation.py | 87 +++++++++---------- 4 files changed, 49 insertions(+), 85 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5a58bd8e39..e09d9e2591 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added +- Add metric instrumentation in fastapi + ([#1199](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1199)) + ## [1.12.0-0.33b0](https://github.com/open-telemetry/opentelemetry-python/releases/tag/v1.12.0-0.33b0) - 2022-08-08 - Adding multiple db connections support for django-instrumentation's sqlcommenter @@ -24,8 +28,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `opentelemetry-instrumentation-sqlalchemy` Added span for the connection phase ([#1133](https://github.com/open-telemetry/opentelemetry-python-contrib/issues/1133)) - Add metric instrumentation in asgi ([#1197](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1197)) -- Add metric instrumentation in fastapi - ([#1199](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1199)) - Add metric instumentation for flask ([#1186](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1186)) 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 69973c382f..305ad08d15 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py @@ -183,26 +183,6 @@ def client_response_hook(span: Span, message: dict): _ClientRequestHookT = typing.Optional[typing.Callable[[Span, dict], None]] _ClientResponseHookT = typing.Optional[typing.Callable[[Span, dict], None]] -# List of recommended attributes -_duration_attrs = [ - SpanAttributes.HTTP_METHOD, - SpanAttributes.HTTP_HOST, - SpanAttributes.HTTP_SCHEME, - SpanAttributes.HTTP_STATUS_CODE, - SpanAttributes.HTTP_FLAVOR, - SpanAttributes.HTTP_SERVER_NAME, - SpanAttributes.NET_HOST_NAME, - SpanAttributes.NET_HOST_PORT, -] - -_active_requests_count_attrs = [ - SpanAttributes.HTTP_METHOD, - SpanAttributes.HTTP_HOST, - SpanAttributes.HTTP_SCHEME, - SpanAttributes.HTTP_FLAVOR, - SpanAttributes.HTTP_SERVER_NAME, -] - class ASGIGetter(Getter[dict]): def get( @@ -385,22 +365,6 @@ def get_default_span_details(scope: dict) -> Tuple[str, dict]: return span_name, {} -def _parse_active_request_count_attrs(req_attrs): - active_requests_count_attrs = {} - for attr_key in _active_requests_count_attrs: - if req_attrs.get(attr_key) is not None: - active_requests_count_attrs[attr_key] = req_attrs[attr_key] - return active_requests_count_attrs - - -def _parse_duration_attrs(req_attrs): - duration_attrs = {} - for attr_key in _duration_attrs: - if req_attrs.get(attr_key) is not None: - duration_attrs[attr_key] = req_attrs[attr_key] - return duration_attrs - - class OpenTelemetryMiddleware: """The ASGI application middleware. diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py index 8e72ec1e2e..821e4234dd 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py @@ -245,6 +245,9 @@ class _InstrumentedFastAPI(fastapi.FastAPI): def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) + meter = get_meter( + __name__, __version__, _InstrumentedFastAPI._meter_provider + ) self.add_middleware( OpenTelemetryMiddleware, excluded_urls=_InstrumentedFastAPI._excluded_urls, @@ -253,7 +256,7 @@ def __init__(self, *args, **kwargs): client_request_hook=_InstrumentedFastAPI._client_request_hook, client_response_hook=_InstrumentedFastAPI._client_response_hook, tracer_provider=_InstrumentedFastAPI._tracer_provider, - meter_provider=_InstrumentedFastAPI._meter_provider, + meter=meter, ) diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py index 8d53838cba..282e197a07 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py @@ -22,11 +22,7 @@ import opentelemetry.instrumentation.fastapi as otel_fastapi from opentelemetry import trace -from opentelemetry.instrumentation.asgi import ( - OpenTelemetryMiddleware, - _active_requests_count_attrs, - _duration_attrs, -) +from opentelemetry.instrumentation.asgi import OpenTelemetryMiddleware from opentelemetry.sdk.metrics.export import ( HistogramDataPoint, NumberDataPoint, @@ -38,6 +34,8 @@ from opentelemetry.util.http import ( OTEL_INSTRUMENTATION_HTTP_CAPTURE_HEADERS_SERVER_REQUEST, OTEL_INSTRUMENTATION_HTTP_CAPTURE_HEADERS_SERVER_RESPONSE, + _active_requests_count_attrs, + _duration_attrs, get_excluded_urls, ) @@ -186,11 +184,12 @@ def test_fastapi_metrics(self): metrics_list = self.memory_metrics_reader.get_metrics_data() number_data_point_seen = False histogram_data_point_seen = False - self.assertTrue(len(metrics_list.resource_metrics) != 0) + self.assertTrue(len(metrics_list.resource_metrics) == 1) for resource_metric in metrics_list.resource_metrics: - self.assertTrue(len(resource_metric.scope_metrics) != 0) + self.assertTrue(len(resource_metric.scope_metrics) == 1) for scope_metric in resource_metric.scope_metrics: - self.assertTrue(len(scope_metric.metrics) != 0) + self.assertTrue(len(scope_metric.metrics) == 2) + print(scope_metric.metrics) for metric in scope_metric.metrics: self.assertIn(metric.name, _expected_metric_names) data_points = list(metric.data.data_points) @@ -228,56 +227,52 @@ def test_basic_metric_success(self): "http.server_name": "testserver", } metrics_list = self.memory_metrics_reader.get_metrics_data() - for resource_metric in metrics_list.resource_metrics: - for scope_metric in resource_metric.scope_metrics: - for metric in scope_metric.metrics: - for point in list(metric.data.data_points): - if isinstance(point, HistogramDataPoint): - self.assertDictEqual( - expected_duration_attributes, - dict(point.attributes), - ) - self.assertEqual(point.count, 1) - self.assertAlmostEqual( - duration, point.sum, delta=20 - ) - if isinstance(point, NumberDataPoint): - self.assertDictEqual( - expected_requests_count_attributes, - dict(point.attributes), - ) - self.assertEqual(point.value, 0) + for metric in ( + metrics_list.resource_metrics[0].scope_metrics[0].metrics + ): + for point in list(metric.data.data_points): + if isinstance(point, HistogramDataPoint): + self.assertDictEqual( + expected_duration_attributes, + dict(point.attributes), + ) + self.assertEqual(point.count, 1) + self.assertAlmostEqual(duration, point.sum, delta=20) + if isinstance(point, NumberDataPoint): + self.assertDictEqual( + expected_requests_count_attributes, + dict(point.attributes), + ) + self.assertEqual(point.value, 0) def test_basic_post_request_metric_success(self): start = default_timer() self._client.post("/foobar") duration = max(round((default_timer() - start) * 1000), 0) metrics_list = self.memory_metrics_reader.get_metrics_data() - for resource_metric in metrics_list.resource_metrics: - for scope_metric in resource_metric.scope_metrics: - for metric in scope_metric.metrics: - for point in list(metric.data.data_points): - if isinstance(point, HistogramDataPoint): - self.assertEqual(point.count, 1) - self.assertAlmostEqual( - duration, point.sum, delta=10 - ) - if isinstance(point, NumberDataPoint): - self.assertEqual(point.value, 0) + for metric in ( + metrics_list.resource_metrics[0].scope_metrics[0].metrics + ): + for point in list(metric.data.data_points): + if isinstance(point, HistogramDataPoint): + self.assertEqual(point.count, 1) + self.assertAlmostEqual(duration, point.sum, delta=30) + if isinstance(point, NumberDataPoint): + self.assertEqual(point.value, 0) def test_metric_uninstruemnt(self): self._client.get("/foobar") self._instrumentor.uninstrument_app(self._app) self._client.get("/foobar") metrics_list = self.memory_metrics_reader.get_metrics_data() - for resource_metric in metrics_list.resource_metrics: - for scope_metric in resource_metric.scope_metrics: - for metric in scope_metric.metrics: - for point in list(metric.data.data_points): - if isinstance(point, HistogramDataPoint): - self.assertEqual(point.count, 1) - if isinstance(point, NumberDataPoint): - self.assertEqual(point.value, 0) + for metric in ( + metrics_list.resource_metrics[0].scope_metrics[0].metrics + ): + for point in list(metric.data.data_points): + if isinstance(point, HistogramDataPoint): + self.assertEqual(point.count, 1) + if isinstance(point, NumberDataPoint): + self.assertEqual(point.value, 0) @staticmethod def _create_fastapi_app(): From 65507521ebea8c4016c3a1d28290f381f3cee7ea Mon Sep 17 00:00:00 2001 From: "anshul.asawa" Date: Tue, 23 Aug 2022 14:57:28 +0530 Subject: [PATCH 16/18] add uninstrument app test --- .../tests/test_fastapi_instrumentation.py | 24 ++++++++++++++++++- .../src/opentelemetry/util/http/__init__.py | 24 +++++++++---------- 2 files changed, 35 insertions(+), 13 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py index 282e197a07..a2e5371001 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py @@ -260,7 +260,7 @@ def test_basic_post_request_metric_success(self): if isinstance(point, NumberDataPoint): self.assertEqual(point.value, 0) - def test_metric_uninstruemnt(self): + def test_metric_uninstruemnt_app(self): self._client.get("/foobar") self._instrumentor.uninstrument_app(self._app) self._client.get("/foobar") @@ -274,6 +274,28 @@ def test_metric_uninstruemnt(self): if isinstance(point, NumberDataPoint): self.assertEqual(point.value, 0) + def test_metric_uninstrument(self): + # instrumenting class and creating app to send request + self._instrumentor.instrument() + app = self._create_fastapi_app() + client = TestClient(app) + client.get("/foobar") + # uninstrumenting class and creating the app again + self._instrumentor.uninstrument() + app = self._create_fastapi_app() + client = TestClient(app) + client.get("/foobar") + + metrics_list = self.memory_metrics_reader.get_metrics_data() + for metric in ( + metrics_list.resource_metrics[0].scope_metrics[0].metrics + ): + for point in list(metric.data.data_points): + if isinstance(point, HistogramDataPoint): + self.assertEqual(point.count, 1) + if isinstance(point, NumberDataPoint): + self.assertEqual(point.value, 0) + @staticmethod def _create_fastapi_app(): app = fastapi.FastAPI() diff --git a/util/opentelemetry-util-http/src/opentelemetry/util/http/__init__.py b/util/opentelemetry-util-http/src/opentelemetry/util/http/__init__.py index 60eafdcd5b..fa559af19e 100644 --- a/util/opentelemetry-util-http/src/opentelemetry/util/http/__init__.py +++ b/util/opentelemetry-util-http/src/opentelemetry/util/http/__init__.py @@ -28,7 +28,7 @@ ) # List of recommended metrics attributes -_duration_attrs = [ +_duration_attrs = { SpanAttributes.HTTP_METHOD, SpanAttributes.HTTP_HOST, SpanAttributes.HTTP_SCHEME, @@ -37,15 +37,15 @@ SpanAttributes.HTTP_SERVER_NAME, SpanAttributes.NET_HOST_NAME, SpanAttributes.NET_HOST_PORT, -] +} -_active_requests_count_attrs = [ +_active_requests_count_attrs = { SpanAttributes.HTTP_METHOD, SpanAttributes.HTTP_HOST, SpanAttributes.HTTP_SCHEME, SpanAttributes.HTTP_FLAVOR, SpanAttributes.HTTP_SERVER_NAME, -] +} class ExcludeList: @@ -150,16 +150,16 @@ def get_custom_headers(env_var: str) -> List[str]: def _parse_active_request_count_attrs(req_attrs): - active_requests_count_attrs = {} - for attr_key in _active_requests_count_attrs: - if req_attrs.get(attr_key) is not None: - active_requests_count_attrs[attr_key] = req_attrs[attr_key] + active_requests_count_attrs = { + key: req_attrs[key] + for key in _active_requests_count_attrs.intersection(req_attrs.keys()) + } return active_requests_count_attrs def _parse_duration_attrs(req_attrs): - duration_attrs = {} - for attr_key in _duration_attrs: - if req_attrs.get(attr_key) is not None: - duration_attrs[attr_key] = req_attrs[attr_key] + duration_attrs = { + key: req_attrs[key] + for key in _duration_attrs.intersection(req_attrs.keys()) + } return duration_attrs From 34ca9d6ebf54499e7e25fa3d852b8659ff0ee7a8 Mon Sep 17 00:00:00 2001 From: Anshul Asawa <35421635+TheAnshul756@users.noreply.github.com> Date: Fri, 26 Aug 2022 10:00:26 +0530 Subject: [PATCH 17/18] Update instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py Co-authored-by: Srikanth Chekuri --- .../tests/test_fastapi_instrumentation.py | 1 - 1 file changed, 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 a2e5371001..6abb6380af 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py @@ -189,7 +189,6 @@ def test_fastapi_metrics(self): self.assertTrue(len(resource_metric.scope_metrics) == 1) for scope_metric in resource_metric.scope_metrics: self.assertTrue(len(scope_metric.metrics) == 2) - print(scope_metric.metrics) for metric in scope_metric.metrics: self.assertIn(metric.name, _expected_metric_names) data_points = list(metric.data.data_points) From ac47ef07a44f2b1157704d47dac52bdd176bd8c3 Mon Sep 17 00:00:00 2001 From: "anshul.asawa" Date: Tue, 6 Sep 2022 19:17:57 +0530 Subject: [PATCH 18/18] add readme --- instrumentation/README.md | 2 +- .../src/opentelemetry/instrumentation/fastapi/package.py | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/instrumentation/README.md b/instrumentation/README.md index deeb3693f0..d7dd93c3a3 100644 --- a/instrumentation/README.md +++ b/instrumentation/README.md @@ -16,7 +16,7 @@ | [opentelemetry-instrumentation-django](./opentelemetry-instrumentation-django) | django >= 1.10 | Yes | [opentelemetry-instrumentation-elasticsearch](./opentelemetry-instrumentation-elasticsearch) | elasticsearch >= 2.0 | No | [opentelemetry-instrumentation-falcon](./opentelemetry-instrumentation-falcon) | falcon >= 1.4.1, < 4.0.0 | No -| [opentelemetry-instrumentation-fastapi](./opentelemetry-instrumentation-fastapi) | fastapi ~= 0.58 | No +| [opentelemetry-instrumentation-fastapi](./opentelemetry-instrumentation-fastapi) | fastapi ~= 0.58 | Yes | [opentelemetry-instrumentation-flask](./opentelemetry-instrumentation-flask) | flask >= 1.0, < 3.0 | Yes | [opentelemetry-instrumentation-grpc](./opentelemetry-instrumentation-grpc) | grpcio ~= 1.27 | No | [opentelemetry-instrumentation-httpx](./opentelemetry-instrumentation-httpx) | httpx >= 0.18.0 | No diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/package.py b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/package.py index cccaa51a8c..8df84fc931 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/package.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/package.py @@ -14,3 +14,5 @@ _instruments = ("fastapi ~= 0.58",) + +_supports_metrics = True