Skip to content

Allow make_asgi_app to be added as a Starlette/FastAPI route - #1218

Open
bhaskargurram-ai wants to merge 2 commits into
prometheus:masterfrom
bhaskargurram-ai:fix/asgi-app-route-without-redirect
Open

bhaskargurram-ai wants to merge 2 commits into
prometheus:masterfrom
bhaskargurram-ai:fix/asgi-app-route-without-redirect

Conversation

@bhaskargurram-ai

Copy link
Copy Markdown

make_asgi_app() returns a plain function. Starlette, and therefore FastAPI, treats a plain function passed to add_route() as a request -> response endpoint rather than an ASGI app (Route.__init__ checks inspect.isfunction(...)). So

app.add_route("/metrics", make_asgi_app())

fails with TypeError: prometheus_app() missing 2 required positional arguments: 'receive' and 'send'. That leaves app.mount("/metrics", ...), which is a path prefix: metrics are served at /metrics/ and GET /metrics gets a 307 redirect. Projects work around this today; vLLM, for example, carries a workaround for it (vllm-project/vllm#2730, #2764, #4511).

This PR makes make_asgi_app() return a small callable ASGI application object that forwards (scope, receive, send) to the existing handler. Starlette passes non-function callables through to Route as raw ASGI apps, so add_route("/metrics", make_asgi_app()) now serves /metrics directly with no redirect.

Backward compatibility:

  • Same signature and arguments (registry, disable_compression), and the result is still a callable used as app(scope, receive, send).
  • app.mount(...) and standalone ASGI use behave exactly as before. asgiref/uvicorn still detect it as an ASGI 3 app.
  • The handler body is unchanged, so this doesn't overlap with Honor HTTP methods in make_asgi_app like make_wsgi_app #1211.

Changes:

  • prometheus_client/asgi.py: return an ASGI app object instead of the bare closure.
  • tests/test_asgi.py: a test that the result is a valid ASGI application object (no extra deps), plus Starlette add_route and mount tests that are skipped when starlette isn't installed.
  • docs/.../fastapi-gunicorn.md: use add_route in the examples and explain the mount/prefix behaviour.

Fixes #1016

make_asgi_app() returned a plain function. Starlette (and therefore
FastAPI) treats a plain function passed to add_route() as a
request -> response endpoint rather than an ASGI app, so
app.add_route("/metrics", make_asgi_app()) failed with a TypeError.
The only working option was app.mount("/metrics", ...), which is a
path prefix: metrics are served at /metrics/ and requests for /metrics
get a 307 redirect.

Return a callable ASGI application object instead. Starlette passes
non-function callables through to Route as raw ASGI apps, so
add_route("/metrics", make_asgi_app()) now serves /metrics directly.
The returned object is called with the same (scope, receive, send)
signature, so mounting it or using it as a standalone ASGI app keeps
working.

Update the FastAPI docs to use add_route and explain the mount
behaviour.

Fixes prometheus#1016

Signed-off-by: Bhaskar Gurram <gurrambhaskar.ai@gmail.com>

@chrikrah chrikrah left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bhaskargurram-ai approving at 272700d. Returning an object instead of the closure is the smallest change that gets add_route to treat it as raw ASGI, and asgiref still reads it as a single-callable ASGI 3 app.

Merge base 9cd073c, Python 3.12.3, starlette 1.7.0:

$ python -m pytest tests/test_asgi.py -q -p no:cacheprovider
13 passed in 0.06s

$ # prometheus_client/asgi.py from 9cd073c, your tests kept
2 failed, 11 passed, 1 error in 0.19s
FAILED tests/test_asgi.py::ASGITest::test_app_is_asgi_application_object
FAILED tests/test_asgi.py::ASGITest::test_starlette_add_route - TypeError: ma...

non-blocking: the docs site deploys from master on every push, so add_route becomes the documented example before a release ships it, and on 0.26.0 it is the #1016 TypeError. A note that add_route needs the next release, with mount for older versions, would cover the gap.

non-blocking: tox.ini installs no starlette in any env, so CI never runs the add_route path; both Starlette tests skip:

$ python -m pytest tests/test_asgi.py -q -p no:cacheprovider -rs
SKIPPED [1] tests/test_asgi.py:293: Don't have starlette installed.
SKIPPED [1] tests/test_asgi.py:302: Don't have starlette installed.
11 passed, 2 skipped in 0.09s

test_starlette_add_route is the test that reproduces #1016, so adding starlette to [testenv] deps next to asgiref would keep it running.

@csmarchbanks would you take starlette as a test dependency, or keep these two tests optional?

Signed-off-by: Bhaskar Gurram <gurrambhaskar.ai@gmail.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ASGI mounted to FastAPI mounts metrics to /metrics/ instead of /metrics

2 participants