Fix FastAPI issue with APIRoute subclasses (#3681)

* fix(fastapi): issue with APIRoute subclasses

Fixes: #3671

When an APIRoute subclass would overwrite the matches method with an implementation that depended
on non-standard fields existing on the HTTP connection scope, this would cause a failure when the
OpenTelemetryMiddleware tried to get the default span details for the incoming request. This has
been fixed by using the matches implementation on the Route class for any subclass of Route. This
should be sufficient since the only information we are trying to get from that method is the path
for the request.

* test(fastapi): added tests for custom api route implementation fix

This commit adds tests that illustrate the original issue that was being
experienced for custom api route implementations when they depended on
non-standard fields existing on the ASGI HTTP connection scope. Before
the fix was implemented, the inclusion of a custom API route in the
FastAPI application would cause an exception to be raised inside the
OpenTelemetryMiddleware since the non-standard fields do not exist on
the ASGI HTTP connection scope until after the subsequent middleware
runs and adds the expected fields.

* test(fastapi): added span assertions for custom api route tests

* Update CHANGELOG.md

---------

Co-authored-by: Emídio Neto <9735060+emdneto@users.noreply.github.com>
Co-authored-by: Riccardo Magliocchetti <riccardo.magliocchetti@gmail.com>

Author: 3 people

CHANGELOG.md

@@ -17,6 +17,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   ([#3624](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3624))
 - `opentelemetry-instrumentation-dbapi`: fix crash retrieving libpq version when enabling commenter with psycopg
   ([#3796](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3796))
+- `opentelemetry-instrumentation-fastapi`: Fix handling of APIRoute subclasses
+  ([#3681](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3681))
 
 ### Added
 

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

@@ -191,7 +191,7 @@ def client_response_hook(span: Span, scope: dict[str, Any], message: dict[str, A
 import fastapi
 from starlette.applications import Starlette
 from starlette.middleware.errors import ServerErrorMiddleware
-from starlette.routing import Match
+from starlette.routing import Match, Route
 from starlette.types import ASGIApp, Receive, Scope, Send
 
 from opentelemetry.instrumentation._semconv import (
@@ -474,7 +474,11 @@ def _get_route_details(scope):
     route = None
 
     for starlette_route in app.routes:
-        match, _ = starlette_route.matches(scope)
+        match, _ = (
+            Route.matches(starlette_route, scope)
+            if isinstance(starlette_route, Route)
+            else starlette_route.matches(scope)
+        )
         if match == Match.FULL:
             try:
                 route = starlette_route.path

instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py

@@ -20,14 +20,17 @@
 import weakref as _weakref
 from contextlib import ExitStack
 from timeit import default_timer
-from typing import Any, cast
+from typing import Any, Final, cast
 from unittest.mock import Mock, call, patch
 
 import fastapi
 import pytest
 from fastapi.middleware.httpsredirect import HTTPSRedirectMiddleware
 from fastapi.responses import JSONResponse, PlainTextResponse
+from fastapi.routing import APIRoute
 from fastapi.testclient import TestClient
+from starlette.routing import Match
+from starlette.types import Receive, Scope, Send
 
 import opentelemetry.instrumentation.fastapi as otel_fastapi
 from opentelemetry import trace
@@ -131,6 +134,23 @@
 )
 
 
+class CustomMiddleware:
+    def __init__(self, app: fastapi.FastAPI) -> None:
+        self.app = app
+
+    async def __call__(
+        self, scope: Scope, receive: Receive, send: Send
+    ) -> None:
+        scope["nonstandard_field"] = "here"
+        await self.app(scope, receive, send)
+
+
+class CustomRoute(APIRoute):
+    def matches(self, scope: Scope) -> tuple[Match, Scope]:
+        assert "nonstandard_field" in scope
+        return super().matches(scope)
+
+
 class TestBaseFastAPI(TestBase):
     def _create_app(self):
         app = self._create_fastapi_app()
@@ -191,6 +211,7 @@ def setUp(self):
         self._instrumentor = otel_fastapi.FastAPIInstrumentor()
         self._app = self._create_app()
         self._app.add_middleware(HTTPSRedirectMiddleware)
+        self._app.add_middleware(CustomMiddleware)
         self._client = TestClient(self._app, base_url="https://testserver:443")
         # run the lifespan, initialize the middleware stack
         # this is more in-line with what happens in a real application when the server starts up
@@ -210,6 +231,7 @@ def tearDown(self):
     def _create_fastapi_app():
         app = fastapi.FastAPI()
         sub_app = fastapi.FastAPI()
+        custom_router = fastapi.APIRouter(route_class=CustomRoute)
 
         @sub_app.get("/home")
         async def _():
@@ -235,6 +257,12 @@ async def _():
         async def _():
             raise UnhandledException("This is an unhandled exception")
 
+        @custom_router.get("/success")
+        async def _():
+            return None
+
+        app.include_router(custom_router, prefix="/custom-router")
+
         app.mount("/sub", app=sub_app)
         app.host("testserver2", sub_app)
 
@@ -313,6 +341,28 @@ def test_sub_app_fastapi_call(self):
             span.attributes[HTTP_URL],
         )
 
+    def test_custom_api_router(self):
+        """
+        This test is to ensure that custom API routers the OpenTelemetryMiddleware does not cause issues with
+        custom API routers that depend on non-standard fields on the ASGI scope.
+        """
+        resp: Final = self._client.get("/custom-router/success")
+        spans: Final = self.memory_exporter.get_finished_spans()
+        spans_with_http_attributes = [
+            span
+            for span in spans
+            if (HTTP_URL in span.attributes or HTTP_TARGET in span.attributes)
+        ]
+        self.assertEqual(200, resp.status_code)
+        for span in spans_with_http_attributes:
+            self.assertEqual(
+                "/custom-router/success", span.attributes[HTTP_TARGET]
+            )
+            self.assertEqual(
+                "https://testserver/custom-router/success",
+                span.attributes[HTTP_URL],
+            )
+
     def test_host_fastapi_call(self):
         client = TestClient(self._app, base_url="https://testserver2:443")
         client.get("/")
@@ -1017,6 +1067,7 @@ def test_metric_uninstrument(self):
     def _create_fastapi_app():
         app = fastapi.FastAPI()
         sub_app = fastapi.FastAPI()
+        custom_router = fastapi.APIRouter(route_class=CustomRoute)
 
         @sub_app.get("/home")
         async def _():
@@ -1042,6 +1093,12 @@ async def _():
         async def _():
             raise UnhandledException("This is an unhandled exception")
 
+        @custom_router.get("/success")
+        async def _():
+            return None
+
+        app.include_router(custom_router, prefix="/custom-router")
+
         app.mount("/sub", app=sub_app)
 
         return app