From 45989bb8fdcaac2867eb6949d9529fa92a4ee8ee Mon Sep 17 00:00:00 2001 From: Marcelo Trylesinski Date: Mon, 25 Nov 2024 11:16:23 +0100 Subject: [PATCH 1/5] Add type hints to Starlette instrumentation --- .../opentelemetry/instrumentation/py.typed | 0 .../instrumentation/starlette/__init__.py | 85 ++++++++++--------- 2 files changed, 45 insertions(+), 40 deletions(-) create mode 100644 instrumentation/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/py.typed diff --git a/instrumentation/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/py.typed b/instrumentation/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/py.typed new file mode 100644 index 0000000000..e69de29bb2 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 50d2fb03d8..a63c1a28a1 100644 --- a/instrumentation/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/starlette/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/starlette/__init__.py @@ -169,8 +169,11 @@ def client_response_hook(span: Span, scope: dict[str, Any], message: dict[str, A API --- """ +# pyright: reportPrivateUsage=false -from typing import Collection +from __future__ import annotations + +from typing import TYPE_CHECKING, Any, Collection, cast from starlette import applications from starlette.routing import Match @@ -184,18 +187,29 @@ def client_response_hook(span: Span, scope: dict[str, Any], message: dict[str, A from opentelemetry.instrumentation.instrumentor import BaseInstrumentor from opentelemetry.instrumentation.starlette.package import _instruments from opentelemetry.instrumentation.starlette.version import __version__ -from opentelemetry.metrics import get_meter +from opentelemetry.metrics import MeterProvider, get_meter from opentelemetry.semconv.trace import SpanAttributes -from opentelemetry.trace import get_tracer +from opentelemetry.trace import TracerProvider, get_tracer from opentelemetry.util.http import get_excluded_urls +if TYPE_CHECKING: + from typing import NotRequired, TypedDict, Unpack + + class InstrumentKwargs(TypedDict): + tracer_provider: NotRequired[TracerProvider] + meter_provider: NotRequired[MeterProvider] + server_request_hook: NotRequired[ServerRequestHook] + client_request_hook: NotRequired[ClientRequestHook] + client_response_hook: NotRequired[ClientResponseHook] + + _excluded_urls = get_excluded_urls("STARLETTE") class StarletteInstrumentor(BaseInstrumentor): - """An instrumentor for starlette + """An instrumentor for Starlette. - See `BaseInstrumentor` + See `BaseInstrumentor`. """ _original_starlette = None @@ -206,8 +220,8 @@ def instrument_app( server_request_hook: ServerRequestHook = None, client_request_hook: ClientRequestHook = None, client_response_hook: ClientResponseHook = None, - meter_provider=None, - tracer_provider=None, + meter_provider: MeterProvider | None = None, + tracer_provider: TracerProvider | None = None, ): """Instrument an uninstrumented Starlette application.""" tracer = get_tracer( @@ -242,34 +256,24 @@ def instrument_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.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 - def _instrument(self, **kwargs): + def _instrument(self, **kwargs: Unpack[InstrumentKwargs]): self._original_starlette = applications.Starlette _InstrumentedStarlette._tracer_provider = kwargs.get("tracer_provider") - _InstrumentedStarlette._server_request_hook = kwargs.get( - "server_request_hook" - ) - _InstrumentedStarlette._client_request_hook = kwargs.get( - "client_request_hook" - ) - _InstrumentedStarlette._client_response_hook = kwargs.get( - "client_response_hook" - ) + _InstrumentedStarlette._server_request_hook = kwargs.get("server_request_hook") + _InstrumentedStarlette._client_request_hook = kwargs.get("client_request_hook") + _InstrumentedStarlette._client_response_hook = kwargs.get("client_response_hook") _InstrumentedStarlette._meter_provider = kwargs.get("_meter_provider") applications.Starlette = _InstrumentedStarlette - def _uninstrument(self, **kwargs): + def _uninstrument(self, **kwargs: Any): """uninstrumenting all created apps by user""" for instance in _InstrumentedStarlette._instrumented_starlette_apps: self.uninstrument_app(instance) @@ -278,14 +282,14 @@ def _uninstrument(self, **kwargs): class _InstrumentedStarlette(applications.Starlette): - _tracer_provider = None - _meter_provider = None + _tracer_provider: TracerProvider | None = None + _meter_provider: MeterProvider | None = None _server_request_hook: ServerRequestHook = None _client_request_hook: ClientRequestHook = None _client_response_hook: ClientResponseHook = None - _instrumented_starlette_apps = set() + _instrumented_starlette_apps: set[applications.Starlette] = set() - def __init__(self, *args, **kwargs): + def __init__(self, *args: Any, **kwargs: Any): super().__init__(*args, **kwargs) tracer = get_tracer( __name__, @@ -318,21 +322,22 @@ def __del__(self): _InstrumentedStarlette._instrumented_starlette_apps.remove(self) -def _get_route_details(scope): +def _get_route_details(scope: dict[str, Any]) -> str | None: """ - Function to retrieve Starlette route from scope. + Function to retrieve Starlette route from ASGI scope. TODO: there is currently no way to retrieve http.route from a starlette application from scope. See: https://github.com/encode/starlette/pull/804 Args: - scope: A Starlette scope + scope: The ASGI scope that contains the Starlette application in the "app" key. + Returns: - A string containing the route or None + The path to the route if found, otherwise None. """ - app = scope["app"] - route = None + app = cast(applications.Starlette, scope["app"]) + route: str | None = None for starlette_route in app.routes: match, _ = starlette_route.matches(scope) @@ -344,18 +349,18 @@ def _get_route_details(scope): return route -def _get_default_span_details(scope): - """ - Callback to retrieve span name and attributes from scope. +def _get_default_span_details(scope: dict[str, Any]) -> tuple[str, dict[str, Any]]: + """Callback to retrieve span name and attributes from ASGI scope. Args: - scope: A Starlette scope + scope: The ASGI scope that contains the Starlette application in the "app" key. + Returns: - A tuple of span name and attributes + A tuple of span name and attributes. """ route = _get_route_details(scope) - method = scope.get("method", "") - attributes = {} + method: str = scope.get("method", "") + attributes: dict[str, Any] = {} if route: attributes[SpanAttributes.HTTP_ROUTE] = route if method and route: # http From 92d122a2aba5606644e3b85a4cac4146ffcb0c1b Mon Sep 17 00:00:00 2001 From: Marcelo Trylesinski Date: Mon, 25 Nov 2024 11:23:43 +0100 Subject: [PATCH 2/5] format --- .../instrumentation/starlette/__init__.py | 22 ++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) 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 a63c1a28a1..677d4d2144 100644 --- a/instrumentation/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/starlette/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/starlette/__init__.py @@ -256,7 +256,11 @@ def instrument_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.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 @@ -266,9 +270,15 @@ def instrumentation_dependencies(self) -> Collection[str]: def _instrument(self, **kwargs: Unpack[InstrumentKwargs]): self._original_starlette = applications.Starlette _InstrumentedStarlette._tracer_provider = kwargs.get("tracer_provider") - _InstrumentedStarlette._server_request_hook = kwargs.get("server_request_hook") - _InstrumentedStarlette._client_request_hook = kwargs.get("client_request_hook") - _InstrumentedStarlette._client_response_hook = kwargs.get("client_response_hook") + _InstrumentedStarlette._server_request_hook = kwargs.get( + "server_request_hook" + ) + _InstrumentedStarlette._client_request_hook = kwargs.get( + "client_request_hook" + ) + _InstrumentedStarlette._client_response_hook = kwargs.get( + "client_response_hook" + ) _InstrumentedStarlette._meter_provider = kwargs.get("_meter_provider") applications.Starlette = _InstrumentedStarlette @@ -349,7 +359,9 @@ def _get_route_details(scope: dict[str, Any]) -> str | None: return route -def _get_default_span_details(scope: dict[str, Any]) -> tuple[str, dict[str, Any]]: +def _get_default_span_details( + scope: dict[str, Any], +) -> tuple[str, dict[str, Any]]: """Callback to retrieve span name and attributes from ASGI scope. Args: From c88ac3f6bb3a6074b87e5d06f7a1e2d130693a82 Mon Sep 17 00:00:00 2001 From: Marcelo Trylesinski Date: Tue, 26 Nov 2024 08:07:35 +0100 Subject: [PATCH 3/5] Add changelog --- CHANGELOG.md | 2 ++ .../instrumentation/starlette/__init__.py | 32 +++++++------------ 2 files changed, 13 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 173abf88f4..04f802043c 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 ### Added +- `opentelemetry-instrumentation-starlette` Add type hints to the instrumentation + ([#3045](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3045)) - `opentelemetry-instrumentation-sqlalchemy` Update unit tests to run with SQLALchemy 2 ([#2976](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2976)) - Add `opentelemetry-instrumentation-openai-v2` to `opentelemetry-bootstrap` 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 677d4d2144..ca940f748c 100644 --- a/instrumentation/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/starlette/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/starlette/__init__.py @@ -193,14 +193,14 @@ def client_response_hook(span: Span, scope: dict[str, Any], message: dict[str, A from opentelemetry.util.http import get_excluded_urls if TYPE_CHECKING: - from typing import NotRequired, TypedDict, Unpack + from typing import TypedDict, Unpack - class InstrumentKwargs(TypedDict): - tracer_provider: NotRequired[TracerProvider] - meter_provider: NotRequired[MeterProvider] - server_request_hook: NotRequired[ServerRequestHook] - client_request_hook: NotRequired[ClientRequestHook] - client_response_hook: NotRequired[ClientResponseHook] + class InstrumentKwargs(TypedDict, total=False): + tracer_provider: TracerProvider + meter_provider: MeterProvider + server_request_hook: ServerRequestHook + client_request_hook: ClientRequestHook + client_response_hook: ClientResponseHook _excluded_urls = get_excluded_urls("STARLETTE") @@ -256,11 +256,7 @@ def instrument_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.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 @@ -270,15 +266,9 @@ def instrumentation_dependencies(self) -> Collection[str]: def _instrument(self, **kwargs: Unpack[InstrumentKwargs]): self._original_starlette = applications.Starlette _InstrumentedStarlette._tracer_provider = kwargs.get("tracer_provider") - _InstrumentedStarlette._server_request_hook = kwargs.get( - "server_request_hook" - ) - _InstrumentedStarlette._client_request_hook = kwargs.get( - "client_request_hook" - ) - _InstrumentedStarlette._client_response_hook = kwargs.get( - "client_response_hook" - ) + _InstrumentedStarlette._server_request_hook = kwargs.get("server_request_hook") + _InstrumentedStarlette._client_request_hook = kwargs.get("client_request_hook") + _InstrumentedStarlette._client_response_hook = kwargs.get("client_response_hook") _InstrumentedStarlette._meter_provider = kwargs.get("_meter_provider") applications.Starlette = _InstrumentedStarlette From 12f201774c63b720749c9f901ce5feac21dacb2a Mon Sep 17 00:00:00 2001 From: Marcelo Trylesinski Date: Tue, 26 Nov 2024 08:07:49 +0100 Subject: [PATCH 4/5] Add changelog --- .../instrumentation/starlette/__init__.py | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) 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 ca940f748c..befe166e59 100644 --- a/instrumentation/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/starlette/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/starlette/__init__.py @@ -256,7 +256,11 @@ def instrument_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.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 @@ -266,9 +270,15 @@ def instrumentation_dependencies(self) -> Collection[str]: def _instrument(self, **kwargs: Unpack[InstrumentKwargs]): self._original_starlette = applications.Starlette _InstrumentedStarlette._tracer_provider = kwargs.get("tracer_provider") - _InstrumentedStarlette._server_request_hook = kwargs.get("server_request_hook") - _InstrumentedStarlette._client_request_hook = kwargs.get("client_request_hook") - _InstrumentedStarlette._client_response_hook = kwargs.get("client_response_hook") + _InstrumentedStarlette._server_request_hook = kwargs.get( + "server_request_hook" + ) + _InstrumentedStarlette._client_request_hook = kwargs.get( + "client_request_hook" + ) + _InstrumentedStarlette._client_response_hook = kwargs.get( + "client_response_hook" + ) _InstrumentedStarlette._meter_provider = kwargs.get("_meter_provider") applications.Starlette = _InstrumentedStarlette From 0f52d0af715e47bec3c246c627d6341c4a0c2e45 Mon Sep 17 00:00:00 2001 From: Marcelo Trylesinski Date: Fri, 29 Nov 2024 14:46:10 +0100 Subject: [PATCH 5/5] Remove pyright ignore --- .../src/opentelemetry/instrumentation/starlette/__init__.py | 1 - 1 file changed, 1 deletion(-) 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 6069122e55..5007bda50a 100644 --- a/instrumentation/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/starlette/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/starlette/__init__.py @@ -169,7 +169,6 @@ def client_response_hook(span: Span, scope: dict[str, Any], message: dict[str, A API --- """ -# pyright: reportPrivateUsage=false from __future__ import annotations