diff --git a/.changelog/4729.fixed b/.changelog/4729.fixed new file mode 100644 index 0000000000..f53c42cb7a --- /dev/null +++ b/.changelog/4729.fixed @@ -0,0 +1 @@ +`opentelemetry-instrumentation-fastapi`: support instrumenting a FastAPI app wrapped in ASGI middleware such as `CORSMiddleware` 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 4fec55f769..48480ac733 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py @@ -231,7 +231,7 @@ class FastAPIInstrumentor(BaseInstrumentor): @staticmethod def instrument_app( - app: fastapi.FastAPI, + app: ASGIApp, server_request_hook: ServerRequestHook = None, client_request_hook: ClientRequestHook = None, client_response_hook: ClientResponseHook = None, @@ -263,6 +263,15 @@ def instrument_app( http_capture_headers_sanitize_fields: Optional list of HTTP headers to sanitize. exclude_spans: Optionally exclude HTTP `send` and/or `receive` spans from the trace. """ + # The application may be wrapped in one or more ASGI middlewares (e.g. + # ``CORSMiddleware``); unwrap it so we instrument the FastAPI instance. + app = _unwrap_middleware(app) + if not isinstance(app, Starlette): + _logger.warning( + "Skipping instrumentation: no FastAPI application found (got %s).", + type(app).__name__, + ) + return if not hasattr(app, "_is_instrumented_by_opentelemetry"): app._is_instrumented_by_opentelemetry = False @@ -416,7 +425,10 @@ async def traced_call(self): ) @staticmethod - def uninstrument_app(app: fastapi.FastAPI): + def uninstrument_app(app: ASGIApp): + app = _unwrap_middleware(app) + if not isinstance(app, Starlette): + return original_build_middleware_stack = getattr( app, "_original_build_middleware_stack", None ) @@ -551,3 +563,27 @@ def _get_default_span_details(scope): else: # fallback span_name = method return span_name, attributes + + +def _unwrap_middleware(app: ASGIApp) -> ASGIApp: + """Return the underlying FastAPI application wrapped by ``app``. + + ``instrument_app``/``uninstrument_app`` operate on the FastAPI application, + but callers may pass an application wrapped in one or more ASGI middlewares, + e.g. ``CORSMiddleware(app=fastapi_app)``. Middlewares store the application + they wrap in an ``app`` attribute by convention, so follow that chain, + stopping as soon as a ``Starlette`` instance (which ``FastAPI`` subclasses) + is reached. If none is found, the original ``app`` is returned unchanged so + the caller can report it. + + ``Starlette`` is used rather than ``fastapi.FastAPI`` because the latter is + monkeypatched to a subclass while global instrumentation is active, which + would make pre-existing app instances fail an ``isinstance`` check. + """ + original = app + while not isinstance(app, Starlette): + inner = getattr(app, "app", None) + if inner is None: + return original + app = inner + return app diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py index 077dae207b..89b3507df1 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py @@ -16,6 +16,7 @@ import pytest from fastapi.background import BackgroundTasks from fastapi.middleware.asyncexitstack import AsyncExitStackMiddleware +from fastapi.middleware.cors import CORSMiddleware from fastapi.middleware.httpsredirect import HTTPSRedirectMiddleware from fastapi.responses import JSONResponse, PlainTextResponse from fastapi.routing import APIRoute @@ -1551,6 +1552,110 @@ def test_mark_span_internal_in_presence_of_span_from_other_framework(self): ) +class TestMiddlewareWrappedApplication(TestBase): + """`instrument_app` should accept a FastAPI app wrapped in ASGI middleware.""" + + def setUp(self): + super().setUp() + self.fastapi_app = fastapi.FastAPI() + + @self.fastapi_app.get("/foobar") + async def _(): + return {"message": "hello world"} + + # The user passes the middleware-wrapped app, not the FastAPI instance. + self.app = CORSMiddleware(self.fastapi_app, allow_origins=["*"]) + otel_fastapi.FastAPIInstrumentor().instrument_app(self.app) + self.client = TestClient(self.app) + + def tearDown(self) -> None: + super().tearDown() + with self.disable_logging(): + otel_fastapi.FastAPIInstrumentor().uninstrument_app(self.app) + + def test_instrumentation_unwraps_middleware(self): + self.assertTrue( + self.fastapi_app._is_instrumented_by_opentelemetry, + "the wrapped FastAPI app should have been instrumented", + ) + + resp = self.client.get("/foobar") + self.assertEqual(200, resp.status_code) + + span_list = self.memory_exporter.get_finished_spans() + self.assertTrue(span_list, "expected spans to be emitted") + + server_spans = [ + span for span in span_list if span.kind == trace.SpanKind.SERVER + ] + self.assertEqual(len(server_spans), 1) + server_span = server_spans[0] + self.assertEqual(server_span.name, "GET /foobar") + # exact attribute name and value/type, per semconv + self.assertEqual(server_span.attributes[HTTP_ROUTE], "/foobar") + self.assertIsInstance(server_span.attributes[HTTP_ROUTE], str) + + def test_uninstrument_unwraps_middleware(self): + otel_fastapi.FastAPIInstrumentor().uninstrument_app(self.app) + self.assertFalse( + self.fastapi_app._is_instrumented_by_opentelemetry, + "uninstrument should reach the wrapped FastAPI app", + ) + # re-instrument so tearDown's uninstrument has a symmetric state + otel_fastapi.FastAPIInstrumentor().instrument_app(self.app) + + +class TestUnwrapMiddleware(unittest.TestCase): + def test_returns_bare_fastapi_app(self): + app = fastapi.FastAPI() + self.assertIs(otel_fastapi._unwrap_middleware(app), app) + + def test_unwraps_single_middleware(self): + app = fastapi.FastAPI() + wrapped = CORSMiddleware(app, allow_origins=["*"]) + self.assertIs(otel_fastapi._unwrap_middleware(wrapped), app) + + def test_unwraps_nested_middleware(self): + app = fastapi.FastAPI() + wrapped = HTTPSRedirectMiddleware( + CORSMiddleware(app, allow_origins=["*"]) + ) + self.assertIs(otel_fastapi._unwrap_middleware(wrapped), app) + + def test_stops_at_fastapi_even_with_app_attribute(self): + # a FastAPI instance is returned as-is even if it has an `app` attr + app = fastapi.FastAPI() + app.app = "should not be unwrapped" + self.assertIs(otel_fastapi._unwrap_middleware(app), app) + + def test_returns_original_when_no_fastapi_found(self): + async def plain_asgi(scope, receive, send): + pass + + self.assertIs(otel_fastapi._unwrap_middleware(plain_asgi), plain_asgi) + + +class TestInstrumentNonFastAPIApp(TestBase): + def test_instrument_app_skips_when_no_fastapi_found(self): + async def plain_asgi(scope, receive, send): + pass + + with self.assertLogs(level="WARNING") as cm: + otel_fastapi.FastAPIInstrumentor().instrument_app(plain_asgi) + self.assertIn("no FastAPI application found", "".join(cm.output)) + self.assertFalse(self.memory_exporter.get_finished_spans()) + + def test_uninstrument_app_is_noop_when_no_fastapi_found(self): + async def plain_asgi(scope, receive, send): + pass + + # should not raise and should leave the object untouched + otel_fastapi.FastAPIInstrumentor().uninstrument_app(plain_asgi) + self.assertFalse( + hasattr(plain_asgi, "_original_build_middleware_stack") + ) + + class TestFastAPIGarbageCollection(unittest.TestCase): def test_fastapi_app_is_collected_after_instrument(self): app = fastapi.FastAPI()