fix(asgi): resolve path template for routers added with include_router

src/instana/instrumentation/asgi.py

@@ -5,7 +5,7 @@
 Instana ASGI Middleware
 """
 
-from typing import TYPE_CHECKING, Any, Awaitable, Callable, Dict
+from typing import TYPE_CHECKING, Any, Awaitable, Callable, Dict, Iterable, Iterator
 
 from opentelemetry.semconv.trace import SpanAttributes
 
@@ -15,12 +15,45 @@
 from instana.util.secrets import strip_secrets_from_query
 from instana.util.traceutils import extract_custom_headers
 
+try:
+    from starlette.routing import Match
+except ImportError:  # pragma: no cover
+    Match = None
+
+try:
+    # FastAPI >= 0.137.2
+    from fastapi.routing import iter_route_contexts
+except ImportError:
+    iter_route_contexts = None
+
 if TYPE_CHECKING:
     from starlette.middleware.exceptions import ExceptionMiddleware
 
     from instana.span.span import InstanaSpan
 
 
+def _iter_routes(routes: Iterable[Any]) -> Iterator[Any]:
+    """
+    Yield the routes of an app in a form that can be matched against a scope
+    and carries its full path template.
+
+    FastAPI >= 0.137 keeps the routes added with include_router() in a tree
+    whose nodes have no path of their own, so the tree has to be flattened.
+    """
+    if iter_route_contexts is not None:
+        # FastAPI >= 0.137.2
+        yield from iter_route_contexts(routes)
+        return
+
+    for route in routes:
+        if hasattr(route, "effective_route_contexts"):
+            # FastAPI 0.137.0 and 0.137.1
+            yield from route.effective_route_contexts()
+        else:
+            # FastAPI < 0.137 and Starlette: the routes are already flat
+            yield route
+
+
 class InstanaASGIMiddleware:
     """
     Instana ASGI Middleware
@@ -51,11 +84,11 @@ def _collect_kvs(self, scope: Dict[str, Any], span: "InstanaSpan") -> None:
             if app and hasattr(app, "routes"):
                 # Attempt to detect the Starlette routes registered.
                 # If Starlette isn't present, we harmlessly dump out.
-                from starlette.routing import Match
-
-                for route in scope["app"].routes:
+                for route in _iter_routes(app.routes):
                     if route.matches(scope)[0] == Match.FULL:
-                        span.set_attribute("http.path_tpl", route.path)
+                        path_tpl = getattr(route, "path", None)
+                        if path_tpl:
+                            span.set_attribute("http.path_tpl", path_tpl)
         except Exception:
             logger.debug("ASGI collect_kvs: ", exc_info=True)
 

tests/apps/fastapi_app/app.py

@@ -1,7 +1,7 @@
 # (c) Copyright IBM Corp. 2021
 # (c) Copyright Instana Inc. 2020
 
-from fastapi import FastAPI, HTTPException, Response
+from fastapi import APIRouter, FastAPI, HTTPException, Response
 from fastapi.concurrency import run_in_threadpool
 from fastapi.testclient import TestClient
 from starlette.exceptions import HTTPException as StarletteHTTPException
@@ -79,3 +79,21 @@ def non_async_threadpool():
     return {
         "message": "non async functions executed on a thread pool can't be followed through thread boundaries"
     }
+
+
+included_router = APIRouter(prefix="/included")
+nested_router = APIRouter(prefix="/nested")
+
+
+@included_router.get("/users/{user_id}")
+async def included_user(user_id):
+    return {"user": user_id}
+
+
+@nested_router.get("/items/{item_id}")
+async def nested_item(item_id):
+    return {"item": item_id}
+
+
+included_router.include_router(nested_router)
+fastapi_server.include_router(included_router)

tests/frameworks/test_fastapi.py

@@ -254,6 +254,54 @@ def test_path_templates(self) -> None:
         assert not asgi_span.data["http"]["error"]
         assert not asgi_span.data["http"]["params"]
 
+    @pytest.mark.parametrize(
+        "path, path_tpl",
+        [
+            ("/included/users/1", "/included/users/{user_id}"),
+            ("/included/nested/items/1", "/included/nested/items/{item_id}"),
+        ],
+    )
+    def test_path_templates_with_included_routers(
+        self, path: str, path_tpl: str
+    ) -> None:
+        result = self.client.get(path)
+
+        assert result
+        assert result.status_code == 200
+
+        spans = self.recorder.queued_spans()
+        assert len(spans) == 1
+
+        asgi_span = get_first_span_by_filter(spans, lambda span: span.n == "asgi")
+        assert asgi_span
+
+        assert not asgi_span.ec
+        assert asgi_span.data["http"]["path"] == path
+        assert asgi_span.data["http"]["path_tpl"] == path_tpl
+        assert asgi_span.data["http"]["method"] == "GET"
+        assert asgi_span.data["http"]["status"] == 200
+
+    def test_iter_routes_without_iter_route_contexts(
+        self, monkeypatch: pytest.MonkeyPatch
+    ) -> None:
+        # FastAPI 0.137.0 and 0.137.1 have no iter_route_contexts(), but the nodes
+        # created by include_router() can flatten themselves. Older versions and
+        # Starlette keep their routes flat.
+        import instana.instrumentation.asgi as asgi
+
+        class IncludedRouterNode:
+            def __init__(self, contexts: list) -> None:
+                self._contexts = contexts
+
+            def effective_route_contexts(self):
+                return iter(self._contexts)
+
+        flat_route, nested_route = object(), object()
+        monkeypatch.setattr(asgi, "iter_route_contexts", None)
+
+        routes = [flat_route, IncludedRouterNode([nested_route])]
+        assert list(asgi._iter_routes(routes)) == [flat_route, nested_route]
+
     def test_secret_scrubbing(self) -> None:
         result = None
         with self.tracer.start_as_current_span("test") as span: