fix(starlette): handle FastAPI 0.137 _IncludedRouter in route name resolution

CHANGELOG.asciidoc

@@ -29,6 +29,14 @@ endif::[]
 //===== Bug fixes
 //
 
+[[release-notes-unreleased]]
+==== Unreleased
+
+[float]
+===== Bug fixes
+
+* Fix ``AttributeError`` on FastAPI >= 0.137 when resolving transaction names for routes added via ``include_router()``
+
 [[release-notes-6.x]]
 === Python Agent version 6.x
 

elasticapm/contrib/starlette/__init__.py

@@ -53,6 +53,42 @@
 
 logger = get_logger("elasticapm.errors.client")
 
+try:
+    # FastAPI >= 0.137.2 exposes a public helper that flattens routes added via
+    # include_router() (nested under _IncludedRouter in 0.137) into matchable
+    # RouteContext objects. Older versions don't have it; see _flatten_routes().
+    from fastapi.routing import iter_route_contexts
+except ImportError:
+    iter_route_contexts = None
+
+
+def _flatten_routes(routes):
+    """
+    Yield the matchable routes from an app's route list.
+
+    FastAPI 0.137 nests routes added via include_router() under _IncludedRouter
+    tree nodes, which expose no ``path`` attribute. They have to be flattened
+    into their effective route contexts (each providing matches() and the full
+    templated path) before they can be matched against a scope.
+
+    FastAPI >= 0.137.2 provides the public iter_route_contexts() for this, which
+    also wraps plain routes uniformly. On older versions fall back to the
+    private _IncludedRouter.effective_route_contexts(), and on FastAPI < 0.137
+    (no _IncludedRouter) the routes are already matchable as-is.
+    """
+    if any(hasattr(route, "effective_route_contexts") for route in routes):
+        if iter_route_contexts is not None:
+            yield from iter_route_contexts(routes)
+            return
+        for route in routes:
+            if hasattr(route, "effective_route_contexts"):
+                yield from route.effective_route_contexts()
+            else:
+                yield route
+        return
+    for route in routes:
+        yield route
+
 
 def make_apm_client(config: Optional[Dict] = None, client_cls=Client, **defaults) -> Client:
     """Builds ElasticAPM client.
@@ -268,10 +304,12 @@ def get_route_name(self, request: Request) -> str:
         return route_name
 
     def _get_route_name(self, scope, routes, route_name=None):
-        for route in routes:
+        for route in _flatten_routes(routes):
             match, child_scope = route.matches(scope)
             if match == Match.FULL:
-                route_name = route.path
+                route_name = getattr(route, "path", None)
+                if route_name is None:
+                    continue
                 child_scope = {**scope, **child_scope}
                 if isinstance(route, Mount) and route.routes:
                     child_route_name = self._get_route_name(child_scope, route.routes, route_name)
@@ -281,4 +319,6 @@ def _get_route_name(self, scope, routes, route_name=None):
                         route_name += child_route_name
                 return route_name
             elif match == Match.PARTIAL and route_name is None:
-                route_name = route.path
+                partial_path = getattr(route, "path", None)
+                if partial_path is not None:
+                    route_name = partial_path

tests/contrib/asyncio/fastapi_tests.py

@@ -0,0 +1,97 @@
+#  BSD 3-Clause License
+#
+#  Copyright (c) 2019, Elasticsearch BV
+#  All rights reserved.
+#
+#  Redistribution and use in source and binary forms, with or without
+#  modification, are permitted provided that the following conditions are met:
+#
+#  * Redistributions of source code must retain the above copyright notice, this
+#    list of conditions and the following disclaimer.
+#
+#  * Redistributions in binary form must reproduce the above copyright notice,
+#    this list of conditions and the following disclaimer in the documentation
+#    and/or other materials provided with the distribution.
+#
+#  * Neither the name of the copyright holder nor the names of its
+#    contributors may be used to endorse or promote products derived from
+#    this software without specific prior written permission.
+#
+#  THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+#  AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+#  IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+#  DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE
+#  FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+#  DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
+#  SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+#  CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+#  OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+#  OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+from tests.fixtures import TempStoreClient
+
+import pytest  # isort:skip
+
+fastapi = pytest.importorskip("fastapi")  # isort:skip
+
+from fastapi import APIRouter, FastAPI  # noqa: E402
+from starlette.testclient import TestClient  # noqa: E402
+
+import elasticapm  # noqa: E402
+from elasticapm.conf import constants  # noqa: E402
+from elasticapm.contrib.starlette import ElasticAPM, make_apm_client  # noqa: E402
+
+pytestmark = [pytest.mark.starlette]
+
+
+@pytest.fixture
+def fastapi_app(elasticapm_client):
+    router = APIRouter(prefix="/api")
+
+    @router.get("/items/{item_id}")
+    async def get_item(item_id: int):
+        return {"item_id": item_id}
+
+    app = FastAPI()
+    app.include_router(router)
+    app.add_middleware(ElasticAPM, client=elasticapm_client)
+
+    yield app
+
+    elasticapm.uninstrument()
+
+
+def test_included_router_request_succeeds(fastapi_app, elasticapm_client):
+    client = TestClient(fastapi_app)
+
+    response = client.get("/api/items/42")
+
+    assert response.status_code == 200
+    assert response.json() == {"item_id": 42}
+    assert len(elasticapm_client.events[constants.TRANSACTION]) == 1
+
+
+def test_included_router_transaction_name(fastapi_app, elasticapm_client):
+    client = TestClient(fastapi_app)
+
+    response = client.get("/api/items/42")
+
+    assert response.status_code == 200
+
+    transaction = elasticapm_client.events[constants.TRANSACTION][0]
+    assert transaction["name"] == "GET /api/items/{item_id}"
+
+
+def test_included_router_options_partial_match(fastapi_app, elasticapm_client):
+    client = TestClient(fastapi_app)
+
+    response = client.options("/api/items/42")
+
+    assert response.status_code == 405
+    assert len(elasticapm_client.events[constants.TRANSACTION]) == 1
+
+
+def test_make_client_with_fastapi_framework():
+    c = make_apm_client(config={"SERVICE_NAME": "foo"}, client_cls=TempStoreClient, framework_name="fastapi")
+    c.close()
+    assert c.config.service_name == "foo"

tests/requirements/reqs-starlette-newest.txt

@@ -1,4 +1,5 @@
 starlette>0.15,<1
+fastapi>=0.137.2
 aiofiles
 httpx
 flask