Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions src/app/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@
from fastapi.middleware.cors import CORSMiddleware
from fastapi.responses import JSONResponse
from ogx_client import APIConnectionError, AsyncOgxClient
from starlette.routing import Mount, Route, WebSocketRoute
from fastapi.routing import iter_route_contexts

from starlette.types import ASGIApp, Message, Receive, Scope, Send

import version
Expand Down Expand Up @@ -212,7 +213,7 @@ async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None:
# requests with the full prefixed path (/api/lightspeed/v1/infer) but
# app_routes_paths contains only application-level paths (/v1/infer).
# Strip the prefix so the path check and metric labels match the routes.
root_path = scope.get("root_path", "")
root_path: str = app.root_path
path: str = scope["path"]
if root_path and path.startswith(root_path + "/"):
path = path[len(root_path) :]
Expand Down Expand Up @@ -292,9 +293,9 @@ async def send_wrapper(message: Message) -> None:
routers.include_routers(app)

app_routes_paths = [
route.path
for route in app.routes
if isinstance(route, (Mount, Route, WebSocketRoute))
rc.original_route.path
for rc in iter_route_contexts(app.routes)
if hasattr(rc.original_route, "path") and rc.original_route.path
]

# Register pure ASGI middlewares. Middleware execution order is the reverse of
Expand Down
68 changes: 65 additions & 3 deletions tests/unit/app/test_main_middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,14 @@
from pytest_mock import MockerFixture
from starlette.types import Message, Receive, Scope, Send

from app.main import GlobalExceptionMiddleware, RestApiMetricsMiddleware
from app.main import (
GlobalExceptionMiddleware,
RestApiMetricsMiddleware,
app_routes_paths,
)
from app.main import (
app as fastapi_app,
)
from models.api.responses.error import InternalServerErrorResponse


Expand Down Expand Up @@ -189,6 +196,7 @@ async def test_rest_api_metrics_strips_root_path(
) -> None:
"""Middleware must strip root_path so prefixed requests still match routes."""
mocker.patch("app.main.app_routes_paths", ["/v1/infer"])
mocker.patch.object(fastapi_app, "root_path", "/api/lightspeed")
mock_measure_duration = mocker.patch(
"app.main.recording.measure_response_duration", return_value=nullcontext()
)
Expand All @@ -201,9 +209,9 @@ async def ok_app(_scope: Scope, _receive: Receive, send: Send) -> None:
middleware = RestApiMetricsMiddleware(ok_app)
collector = _ResponseCollector()

# Simulate 3scale forwarding /api/lightspeed/v1/infer with root_path set.
# Simulate 3scale forwarding /api/lightspeed/v1/infer — scope carries no root_path.
await middleware(
_make_scope("/api/lightspeed/v1/infer", root_path="/api/lightspeed"),
_make_scope("/api/lightspeed/v1/infer"),
_noop_receive,
collector,
)
Expand Down Expand Up @@ -241,3 +249,57 @@ async def ok_app(_scope: Scope, _receive: Receive, send: Send) -> None:
assert collector.status_code == 200
mock_measure_duration.assert_called_once_with("/v1/infer")
mock_record_call.assert_called_once_with("/v1/infer", 200)


@pytest.mark.asyncio
async def test_rest_api_metrics_uses_app_root_path_not_scope(
mocker: MockerFixture,
) -> None:
"""Middleware must read root_path from app.root_path, not scope["root_path"].

The scope carries an empty root_path while app.root_path holds the real prefix.
If the middleware reads from the scope it will not strip the prefix, the path
will not match any route, and no metric will be recorded — causing both
mock_measure_duration and mock_record_call assertions to fail.
"""
mocker.patch("app.main.app_routes_paths", ["/v1/infer"])
mocker.patch.object(fastapi_app, "root_path", "/api/lightspeed")
mock_measure_duration = mocker.patch(
"app.main.recording.measure_response_duration", return_value=nullcontext()
)
mock_record_call = mocker.patch("app.main.recording.record_rest_api_call")

async def ok_app(_scope: Scope, _receive: Receive, send: Send) -> None:
await send({"type": "http.response.start", "status": 200, "headers": []})
await send({"type": "http.response.body", "body": b"ok"})

middleware = RestApiMetricsMiddleware(ok_app)
collector = _ResponseCollector()

# scope["root_path"] is explicitly empty while app.root_path is "/api/lightspeed".
# The middleware must use app.root_path to strip the prefix correctly.
scope = _make_scope("/api/lightspeed/v1/infer")
scope["root_path"] = ""
await middleware(scope, _noop_receive, collector)

assert collector.status_code == 200
mock_measure_duration.assert_called_once_with("/v1/infer")
mock_record_call.assert_called_once_with("/v1/infer", 200)
Comment thread
coderabbitai[bot] marked this conversation as resolved.


# ---------------------------------------------------------------------------
# app_routes_paths population
# ---------------------------------------------------------------------------


def test_app_routes_paths_contains_application_routes() -> None:
"""app_routes_paths must include routes registered via include_router.

FastAPI >= 0.137 stores included routers as _IncludedRouter objects that
the old isinstance(route, (Mount, Route, WebSocketRoute)) filter silently
drops. iter_route_contexts() resolves them correctly. If this test fails
with only 4 entries (the FastAPI built-ins), the fix has been reverted.
"""
assert "/liveness" in app_routes_paths
assert "/readiness" in app_routes_paths
assert len(app_routes_paths) > 4
Loading