diff --git a/CHANGELOG.md b/CHANGELOG.md index c0e2bf7696..4dfee88d7b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#1242](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1242)) - `opentelemetry-util-http` Add support for sanitizing HTTP header values. ([#1253](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1253)) +- Add metric instrumentation in starlette + ([#1327](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1327)) + ### Fixed diff --git a/instrumentation/README.md b/instrumentation/README.md index 725c717468..973adeee85 100644 --- a/instrumentation/README.md +++ b/instrumentation/README.md @@ -36,7 +36,7 @@ | [opentelemetry-instrumentation-sklearn](./opentelemetry-instrumentation-sklearn) | scikit-learn ~= 0.24.0 | No | [opentelemetry-instrumentation-sqlalchemy](./opentelemetry-instrumentation-sqlalchemy) | sqlalchemy | No | [opentelemetry-instrumentation-sqlite3](./opentelemetry-instrumentation-sqlite3) | sqlite3 | No -| [opentelemetry-instrumentation-starlette](./opentelemetry-instrumentation-starlette) | starlette ~= 0.13.0 | No +| [opentelemetry-instrumentation-starlette](./opentelemetry-instrumentation-starlette) | starlette ~= 0.13.0 | Yes | [opentelemetry-instrumentation-system-metrics](./opentelemetry-instrumentation-system-metrics) | psutil >= 5 | No | [opentelemetry-instrumentation-tornado](./opentelemetry-instrumentation-tornado) | tornado >= 5.1.1 | No | [opentelemetry-instrumentation-urllib](./opentelemetry-instrumentation-urllib) | urllib | No diff --git a/instrumentation/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/starlette/__init__.py b/instrumentation/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/starlette/__init__.py index 110644cfb3..65e97e9a65 100644 --- a/instrumentation/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/starlette/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/starlette/__init__.py @@ -131,6 +131,8 @@ def client_response_hook(span: Span, message: dict): from opentelemetry.instrumentation.asgi import OpenTelemetryMiddleware from opentelemetry.instrumentation.asgi.package import _instruments from opentelemetry.instrumentation.instrumentor import BaseInstrumentor +from opentelemetry.instrumentation.starlette.version import __version__ +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 @@ -156,9 +158,11 @@ def instrument_app( server_request_hook: _ServerRequestHookT = None, client_request_hook: _ClientRequestHookT = None, client_response_hook: _ClientResponseHookT = None, + meter_provider=None, tracer_provider=None, ): """Instrument an uninstrumented Starlette application.""" + meter = get_meter(__name__, __version__, meter_provider) if not getattr(app, "is_instrumented_by_opentelemetry", False): app.add_middleware( OpenTelemetryMiddleware, @@ -168,9 +172,24 @@ def instrument_app( client_request_hook=client_request_hook, client_response_hook=client_response_hook, tracer_provider=tracer_provider, + meter=meter, ) app.is_instrumented_by_opentelemetry = True + # adding apps to set for uninstrumenting + if app not in _InstrumentedStarlette._instrumented_starlette_apps: + _InstrumentedStarlette._instrumented_starlette_apps.add(app) + + @staticmethod + def uninstrument_app(app: applications.Starlette): + app.user_middleware = [ + x + for x in app.user_middleware + if x.cls is not OpenTelemetryMiddleware + ] + app.middleware_stack = app.build_middleware_stack() + app._is_instrumented_by_opentelemetry = False + def instrumentation_dependencies(self) -> Collection[str]: return _instruments @@ -186,20 +205,32 @@ def _instrument(self, **kwargs): _InstrumentedStarlette._client_response_hook = kwargs.get( "client_response_hook" ) + _InstrumentedStarlette._meter_provider = kwargs.get("_meter_provider") + applications.Starlette = _InstrumentedStarlette def _uninstrument(self, **kwargs): + + """uninstrumenting all created apps by user""" + for instance in _InstrumentedStarlette._instrumented_starlette_apps: + self.uninstrument_app(instance) + _InstrumentedStarlette._instrumented_starlette_apps.clear() applications.Starlette = self._original_starlette class _InstrumentedStarlette(applications.Starlette): _tracer_provider = None + _meter_provider = None _server_request_hook: _ServerRequestHookT = None _client_request_hook: _ClientRequestHookT = None _client_response_hook: _ClientResponseHookT = None + _instrumented_starlette_apps = set() def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) + meter = get_meter( + __name__, __version__, _InstrumentedStarlette._meter_provider + ) self.add_middleware( OpenTelemetryMiddleware, excluded_urls=_excluded_urls, @@ -208,7 +239,14 @@ def __init__(self, *args, **kwargs): client_request_hook=_InstrumentedStarlette._client_request_hook, client_response_hook=_InstrumentedStarlette._client_response_hook, tracer_provider=_InstrumentedStarlette._tracer_provider, + meter=meter, ) + self._is_instrumented_by_opentelemetry = True + # adding apps to set for uninstrumenting + _InstrumentedStarlette._instrumented_starlette_apps.add(self) + + def __del__(self): + _InstrumentedStarlette._instrumented_starlette_apps.remove(self) def _get_route_details(scope): diff --git a/instrumentation/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/starlette/package.py b/instrumentation/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/starlette/package.py index 3a9b5948d2..11c3ed2e7b 100644 --- a/instrumentation/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/starlette/package.py +++ b/instrumentation/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/starlette/package.py @@ -14,3 +14,5 @@ _instruments = ("starlette ~= 0.13.0",) + +_supports_metrics = True diff --git a/instrumentation/opentelemetry-instrumentation-starlette/tests/test_starlette_instrumentation.py b/instrumentation/opentelemetry-instrumentation-starlette/tests/test_starlette_instrumentation.py index 6f9a509450..b168643211 100644 --- a/instrumentation/opentelemetry-instrumentation-starlette/tests/test_starlette_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-starlette/tests/test_starlette_instrumentation.py @@ -13,6 +13,7 @@ # limitations under the License. import unittest +from timeit import default_timer from unittest.mock import patch from starlette import applications @@ -22,6 +23,10 @@ from starlette.websockets import WebSocket import opentelemetry.instrumentation.starlette as otel_starlette +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 @@ -35,9 +40,20 @@ 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, ) +_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 TestStarletteManualInstrumentation(TestBase): def _create_app(self): @@ -100,6 +116,109 @@ def test_starlette_excluded_urls(self): spans = self.memory_exporter.get_finished_spans() self.assertEqual(len(spans), 0) + def test_starlette_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) == 1) + 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) + 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) + + def test_basic_post_request_metric_success(self): + start = default_timer() + expected_duration_attributes = { + "http.flavor": "1.1", + "http.host": "testserver", + "http.method": "POST", + "http.scheme": "http", + "http.server_name": "testserver", + "http.status_code": 405, + "net.host.port": 80, + } + expected_requests_count_attributes = { + "http.flavor": "1.1", + "http.host": "testserver", + "http.method": "POST", + "http.scheme": "http", + "http.server_name": "testserver", + } + self._client.post("/foobar") + duration = max(round((default_timer() - start) * 1000), 0) + 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) + self.assertAlmostEqual(duration, point.sum, delta=30) + self.assertDictEqual( + dict(point.attributes), expected_duration_attributes + ) + if isinstance(point, NumberDataPoint): + self.assertDictEqual( + expected_requests_count_attributes, + dict(point.attributes), + ) + self.assertEqual(point.value, 0) + + def test_metric_for_uninstrment_app_method(self): + self._client.get("/foobar") + # uninstrumenting the existing client app + self._instrumentor.uninstrument_app(self._app) + self._client.get("/foobar") + self._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) + + def test_metric_uninstrument_inherited_by_base(self): + # instrumenting class and creating app to send request + self._instrumentor.instrument() + app = self._create_starlette_app() + client = TestClient(app) + client.get("/foobar") + # calling uninstrument and checking for telemetry data + self._instrumentor.uninstrument() + client.get("/foobar") + client.get("/foobar") + 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_starlette_app(): def home(_):