Merge branch 'main' into hectorhdzg/aiohttpmetrics

Author: hectorhdzg

CHANGELOG.md

@@ -11,11 +11,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## Unreleased
 
+### Fixed
+
+- `opentelemetry-instrumentation-fastapi`: fix wrapping of middlewares
+  ([#3012](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3012))
+
 ### Breaking changes
 
 - `opentelemetry-instrumentation-botocore` Use `cloud.region` instead of `aws.region` span attribute as per semantic conventions.
   ([#3474](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3474))
-
+- `opentelemetry-instrumentation-fastapi`: Drop support for FastAPI versions earlier than `0.92`
+  ([#3012](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3012))
 
 ### Added
 - `opentelemetry-instrumentation-aiohttp-client` Add support for HTTP metrics

RELEASING.md

@@ -41,13 +41,16 @@ The workflow will create two pull requests, one against the `main` and one again
   * Press the "Run workflow" button, then select the release branch from the dropdown list,
     e.g. `release/v1.9.x`, then enter the pull request number that you want to backport,
     then click the "Run workflow" button below that.
+  * Add the label `backport` to the generated pull request.
+  * In case label automation doesn't work, just close and reopen the PR so that the workflow will take into account the label automation we have in place.
   * Review and merge the backport pull request that it generates.
 * Merge a pull request to the release branch updating the `CHANGELOG.md`.
   * The heading for the unreleased entries should be `## Unreleased`.
 * Run the [Prepare patch release workflow](https://github.com/open-telemetry/opentelemetry-python-contrib/actions/workflows/prepare-patch-release.yml).
   * Press the "Run workflow" button, then select the release branch from the dropdown list,
     e.g. `release/v1.9.x`, and click the "Run workflow" button below that.
   * Review and merge the pull request that it creates for updating the version.
+* Note: If you are doing a patch release in `-contrib` repo, you should also do an equivalent patch release in `-core` repo (even if there's no fix to release), otherwise tests in CI will fail.
 
 ### Preparing a patch release for individual package
 

docs/nitpick-exceptions.ini

@@ -44,6 +44,7 @@ py-class=
     psycopg.Connection
     psycopg.AsyncConnection
     ObjectProxy
+    fastapi.applications.FastAPI
 
 any=
     ; API

instrumentation/README.md

@@ -3,7 +3,7 @@
 | --------------- | ------------------ | --------------- | -------------- |
 | [opentelemetry-instrumentation-aio-pika](./opentelemetry-instrumentation-aio-pika) | aio_pika >= 7.2.0, < 10.0.0 | No | development
 | [opentelemetry-instrumentation-aiohttp-client](./opentelemetry-instrumentation-aiohttp-client) | aiohttp ~= 3.0 | Yes | migration
-| [opentelemetry-instrumentation-aiohttp-server](./opentelemetry-instrumentation-aiohttp-server) | aiohttp ~= 3.0 | No | development
+| [opentelemetry-instrumentation-aiohttp-server](./opentelemetry-instrumentation-aiohttp-server) | aiohttp ~= 3.0 | Yes | development
 | [opentelemetry-instrumentation-aiokafka](./opentelemetry-instrumentation-aiokafka) | aiokafka >= 0.8, < 1.0 | No | development
 | [opentelemetry-instrumentation-aiopg](./opentelemetry-instrumentation-aiopg) | aiopg >= 0.13.0, < 2.0.0 | No | development
 | [opentelemetry-instrumentation-asgi](./opentelemetry-instrumentation-asgi) | asgiref ~= 3.0 | Yes | migration
@@ -22,7 +22,7 @@
 | [opentelemetry-instrumentation-django](./opentelemetry-instrumentation-django) | django >= 1.10 | Yes | development
 | [opentelemetry-instrumentation-elasticsearch](./opentelemetry-instrumentation-elasticsearch) | elasticsearch >= 6.0 | No | development
 | [opentelemetry-instrumentation-falcon](./opentelemetry-instrumentation-falcon) | falcon >= 1.4.1, < 5.0.0 | Yes | migration
-| [opentelemetry-instrumentation-fastapi](./opentelemetry-instrumentation-fastapi) | fastapi ~= 0.58 | Yes | migration
+| [opentelemetry-instrumentation-fastapi](./opentelemetry-instrumentation-fastapi) | fastapi ~= 0.92 | Yes | migration
 | [opentelemetry-instrumentation-flask](./opentelemetry-instrumentation-flask) | flask >= 1.0 | Yes | migration
 | [opentelemetry-instrumentation-grpc](./opentelemetry-instrumentation-grpc) | grpcio >= 1.42.0 | No | development
 | [opentelemetry-instrumentation-httpx](./opentelemetry-instrumentation-httpx) | httpx >= 0.18.0 | No | migration

instrumentation/opentelemetry-instrumentation-aiohttp-server/src/opentelemetry/instrumentation/aiohttp_server/package.py

@@ -14,3 +14,5 @@
 
 
 _instruments = ("aiohttp ~= 3.0",)
+
+_supports_metrics = True

instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py

@@ -665,9 +665,9 @@ def __init__(
     # pylint: disable=too-many-statements
     async def __call__(
         self,
-        scope: dict[str, Any],
-        receive: Callable[[], Awaitable[dict[str, Any]]],
-        send: Callable[[dict[str, Any]], Awaitable[None]],
+        scope: typing.MutableMapping[str, Any],
+        receive: Callable[[], Awaitable[typing.MutableMapping[str, Any]]],
+        send: Callable[[typing.MutableMapping[str, Any]], Awaitable[None]],
     ) -> None:
         """The ASGI application
 

instrumentation/opentelemetry-instrumentation-elasticsearch/tests/test_elasticsearch.py

@@ -31,7 +31,10 @@
     ElasticsearchInstrumentor,
 )
 from opentelemetry.instrumentation.elasticsearch.utils import sanitize_body
-from opentelemetry.semconv.trace import SpanAttributes
+from opentelemetry.semconv._incubating.attributes.db_attributes import (
+    DB_STATEMENT,
+    DB_SYSTEM,
+)
 from opentelemetry.test.test_base import TestBase
 from opentelemetry.trace import StatusCode
 
@@ -76,15 +79,15 @@ def get_elasticsearch_client(*args, **kwargs):
 )
 class TestElasticsearchIntegration(TestBase):
     search_attributes = {
-        SpanAttributes.DB_SYSTEM: "elasticsearch",
+        DB_SYSTEM: "elasticsearch",
         "elasticsearch.url": "/test-index/_search",
         "elasticsearch.method": helpers.dsl_search_method,
         "elasticsearch.target": "test-index",
-        SpanAttributes.DB_STATEMENT: str({"query": {"bool": {"filter": "?"}}}),
+        DB_STATEMENT: str({"query": {"bool": {"filter": "?"}}}),
     }
 
     create_attributes = {
-        SpanAttributes.DB_SYSTEM: "elasticsearch",
+        DB_SYSTEM: "elasticsearch",
         "elasticsearch.url": "/test-index",
         "elasticsearch.method": "HEAD",
     }
@@ -361,13 +364,13 @@ def test_dsl_create(self, request_mock):
         )
 
         attributes = {
-            SpanAttributes.DB_SYSTEM: "elasticsearch",
+            DB_SYSTEM: "elasticsearch",
             "elasticsearch.url": "/test-index",
             "elasticsearch.method": "PUT",
         }
         self.assertSpanHasAttributes(span2, attributes)
         self.assertEqual(
-            literal_eval(span2.attributes[SpanAttributes.DB_STATEMENT]),
+            literal_eval(span2.attributes[DB_STATEMENT]),
             helpers.dsl_create_statement,
         )
 
@@ -408,13 +411,13 @@ def test_dsl_index(self, request_mock):
         span = spans[0]
         self.assertEqual(span.name, helpers.dsl_index_span_name)
         attributes = {
-            SpanAttributes.DB_SYSTEM: "elasticsearch",
+            DB_SYSTEM: "elasticsearch",
             "elasticsearch.url": helpers.dsl_index_url,
             "elasticsearch.method": "PUT",
         }
         self.assertSpanHasAttributes(span, attributes)
         self.assertEqual(
-            literal_eval(span.attributes[SpanAttributes.DB_STATEMENT]),
+            literal_eval(span.attributes[DB_STATEMENT]),
             {
                 "body": "A few words here, a few words there",
                 "title": "About searching",

instrumentation/opentelemetry-instrumentation-fastapi/pyproject.toml

@@ -35,7 +35,7 @@ dependencies = [
 
 [project.optional-dependencies]
 instruments = [
-    "fastapi ~= 0.58",
+    "fastapi ~= 0.92",
 ]
 
 [project.entry-points.opentelemetry_instrumentor]

instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py

@@ -182,11 +182,16 @@ def client_response_hook(span: Span, scope: dict[str, Any], message: dict[str, A
 
 from __future__ import annotations
 
+import functools
 import logging
+import types
 from typing import Collection, Literal
 
 import fastapi
+from starlette.applications import Starlette
+from starlette.middleware.errors import ServerErrorMiddleware
 from starlette.routing import Match
+from starlette.types import ASGIApp
 
 from opentelemetry.instrumentation._semconv import (
     _get_schema_url,
@@ -203,9 +208,9 @@ def client_response_hook(span: Span, scope: dict[str, Any], message: dict[str, A
 from opentelemetry.instrumentation.fastapi.package import _instruments
 from opentelemetry.instrumentation.fastapi.version import __version__
 from opentelemetry.instrumentation.instrumentor import BaseInstrumentor
-from opentelemetry.metrics import get_meter
+from opentelemetry.metrics import MeterProvider, get_meter
 from opentelemetry.semconv.attributes.http_attributes import HTTP_ROUTE
-from opentelemetry.trace import get_tracer
+from opentelemetry.trace import TracerProvider, get_tracer
 from opentelemetry.util.http import (
     get_excluded_urls,
     parse_excluded_urls,
@@ -226,13 +231,13 @@ class FastAPIInstrumentor(BaseInstrumentor):
 
     @staticmethod
     def instrument_app(
-        app,
+        app: fastapi.FastAPI,
         server_request_hook: ServerRequestHook = None,
         client_request_hook: ClientRequestHook = None,
         client_response_hook: ClientResponseHook = None,
-        tracer_provider=None,
-        meter_provider=None,
-        excluded_urls=None,
+        tracer_provider: TracerProvider | None = None,
+        meter_provider: MeterProvider | None = None,
+        excluded_urls: str | None = None,
         http_capture_headers_server_request: list[str] | None = None,
         http_capture_headers_server_response: list[str] | None = None,
         http_capture_headers_sanitize_fields: list[str] | None = None,
@@ -284,21 +289,56 @@ def instrument_app(
                 schema_url=_get_schema_url(sem_conv_opt_in_mode),
             )
 
-            app.add_middleware(
-                OpenTelemetryMiddleware,
-                excluded_urls=excluded_urls,
-                default_span_details=_get_default_span_details,
-                server_request_hook=server_request_hook,
-                client_request_hook=client_request_hook,
-                client_response_hook=client_response_hook,
-                # Pass in tracer/meter to get __name__and __version__ of fastapi instrumentation
-                tracer=tracer,
-                meter=meter,
-                http_capture_headers_server_request=http_capture_headers_server_request,
-                http_capture_headers_server_response=http_capture_headers_server_response,
-                http_capture_headers_sanitize_fields=http_capture_headers_sanitize_fields,
-                exclude_spans=exclude_spans,
+            # Instead of using `app.add_middleware` we monkey patch `build_middleware_stack` to insert our middleware
+            # as the outermost middleware.
+            # Otherwise `OpenTelemetryMiddleware` would have unhandled exceptions tearing through it and would not be able
+            # to faithfully record what is returned to the client since it technically cannot know what `ServerErrorMiddleware` is going to do.
+
+            def build_middleware_stack(self: Starlette) -> ASGIApp:
+                inner_server_error_middleware: ASGIApp = (  # type: ignore
+                    self._original_build_middleware_stack()  # type: ignore
+                )
+                otel_middleware = OpenTelemetryMiddleware(
+                    inner_server_error_middleware,
+                    excluded_urls=excluded_urls,
+                    default_span_details=_get_default_span_details,
+                    server_request_hook=server_request_hook,
+                    client_request_hook=client_request_hook,
+                    client_response_hook=client_response_hook,
+                    # Pass in tracer/meter to get __name__and __version__ of fastapi instrumentation
+                    tracer=tracer,
+                    meter=meter,
+                    http_capture_headers_server_request=http_capture_headers_server_request,
+                    http_capture_headers_server_response=http_capture_headers_server_response,
+                    http_capture_headers_sanitize_fields=http_capture_headers_sanitize_fields,
+                    exclude_spans=exclude_spans,
+                )
+                # Wrap in an outer layer of ServerErrorMiddleware so that any exceptions raised in OpenTelemetryMiddleware
+                # are handled.
+                # This should not happen unless there is a bug in OpenTelemetryMiddleware, but if there is we don't want that
+                # to impact the user's application just because we wrapped the middlewares in this order.
+                if isinstance(
+                    inner_server_error_middleware, ServerErrorMiddleware
+                ):  # usually true
+                    outer_server_error_middleware = ServerErrorMiddleware(
+                        app=otel_middleware,
+                    )
+                else:
+                    # Something else seems to have patched things, or maybe Starlette changed.
+                    # Just create a default ServerErrorMiddleware.
+                    outer_server_error_middleware = ServerErrorMiddleware(
+                        app=otel_middleware
+                    )
+                return outer_server_error_middleware
+
+            app._original_build_middleware_stack = app.build_middleware_stack
+            app.build_middleware_stack = types.MethodType(
+                functools.wraps(app.build_middleware_stack)(
+                    build_middleware_stack
+                ),
+                app,
             )
+
             app._is_instrumented_by_opentelemetry = True
             if app not in _InstrumentedFastAPI._instrumented_fastapi_apps:
                 _InstrumentedFastAPI._instrumented_fastapi_apps.add(app)
@@ -309,11 +349,12 @@ def instrument_app(
 
     @staticmethod
     def uninstrument_app(app: fastapi.FastAPI):
-        app.user_middleware = [
-            x
-            for x in app.user_middleware
-            if x.cls is not OpenTelemetryMiddleware
-        ]
+        original_build_middleware_stack = getattr(
+            app, "_original_build_middleware_stack", None
+        )
+        if original_build_middleware_stack:
+            app.build_middleware_stack = original_build_middleware_stack
+            del app._original_build_middleware_stack
         app.middleware_stack = app.build_middleware_stack()
         app._is_instrumented_by_opentelemetry = False
 
@@ -341,12 +382,7 @@ def _instrument(self, **kwargs):
         _InstrumentedFastAPI._http_capture_headers_sanitize_fields = (
             kwargs.get("http_capture_headers_sanitize_fields")
         )
-        _excluded_urls = kwargs.get("excluded_urls")
-        _InstrumentedFastAPI._excluded_urls = (
-            _excluded_urls_from_env
-            if _excluded_urls is None
-            else parse_excluded_urls(_excluded_urls)
-        )
+        _InstrumentedFastAPI._excluded_urls = kwargs.get("excluded_urls")
         _InstrumentedFastAPI._meter_provider = kwargs.get("meter_provider")
         _InstrumentedFastAPI._exclude_spans = kwargs.get("exclude_spans")
         fastapi.FastAPI = _InstrumentedFastAPI
@@ -365,43 +401,29 @@ class _InstrumentedFastAPI(fastapi.FastAPI):
     _server_request_hook: ServerRequestHook = None
     _client_request_hook: ClientRequestHook = None
     _client_response_hook: ClientResponseHook = None
+    _http_capture_headers_server_request: list[str] | None = None
+    _http_capture_headers_server_response: list[str] | None = None
+    _http_capture_headers_sanitize_fields: list[str] | None = None
+    _exclude_spans: list[Literal["receive", "send"]] | None = None
+
     _instrumented_fastapi_apps = set()
     _sem_conv_opt_in_mode = _StabilityMode.DEFAULT
 
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
-        tracer = get_tracer(
-            __name__,
-            __version__,
-            _InstrumentedFastAPI._tracer_provider,
-            schema_url=_get_schema_url(
-                _InstrumentedFastAPI._sem_conv_opt_in_mode
-            ),
-        )
-        meter = get_meter(
-            __name__,
-            __version__,
-            _InstrumentedFastAPI._meter_provider,
-            schema_url=_get_schema_url(
-                _InstrumentedFastAPI._sem_conv_opt_in_mode
-            ),
-        )
-        self.add_middleware(
-            OpenTelemetryMiddleware,
-            excluded_urls=_InstrumentedFastAPI._excluded_urls,
-            default_span_details=_get_default_span_details,
-            server_request_hook=_InstrumentedFastAPI._server_request_hook,
-            client_request_hook=_InstrumentedFastAPI._client_request_hook,
-            client_response_hook=_InstrumentedFastAPI._client_response_hook,
-            # Pass in tracer/meter to get __name__and __version__ of fastapi instrumentation
-            tracer=tracer,
-            meter=meter,
-            http_capture_headers_server_request=_InstrumentedFastAPI._http_capture_headers_server_request,
-            http_capture_headers_server_response=_InstrumentedFastAPI._http_capture_headers_server_response,
-            http_capture_headers_sanitize_fields=_InstrumentedFastAPI._http_capture_headers_sanitize_fields,
-            exclude_spans=_InstrumentedFastAPI._exclude_spans,
+        FastAPIInstrumentor.instrument_app(
+            self,
+            server_request_hook=self._server_request_hook,
+            client_request_hook=self._client_request_hook,
+            client_response_hook=self._client_response_hook,
+            tracer_provider=self._tracer_provider,
+            meter_provider=self._meter_provider,
+            excluded_urls=self._excluded_urls,
+            http_capture_headers_server_request=self._http_capture_headers_server_request,
+            http_capture_headers_server_response=self._http_capture_headers_server_response,
+            http_capture_headers_sanitize_fields=self._http_capture_headers_sanitize_fields,
+            exclude_spans=self._exclude_spans,
         )
-        self._is_instrumented_by_opentelemetry = True
         _InstrumentedFastAPI._instrumented_fastapi_apps.add(self)
 
     def __del__(self):

instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/package.py

@@ -13,7 +13,7 @@
 # limitations under the License.
 
 
-_instruments = ("fastapi ~= 0.58",)
+_instruments = ("fastapi ~= 0.92",)
 
 _supports_metrics = True