Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Request body and path parameters in Flask/FastAPI/Starlette #80

Open
unflxw opened this issue Jun 26, 2023 · 5 comments
Open

Request body and path parameters in Flask/FastAPI/Starlette #80

unflxw opened this issue Jun 26, 2023 · 5 comments

Comments

@unflxw
Copy link
Contributor

unflxw commented Jun 26, 2023

Currently these integrations show no params. It would be nice to try and get params showing, ideally both from the request body JSON and from the path parameters.

@backlog-helper
Copy link

backlog-helper bot commented Jun 26, 2023

✔️ All good!

New issue guide | Backlog management | Rules | Feedback

@unflxw unflxw changed the title Params in Flask/FastAPI/Starlette? Request body and path parameters in Flask/FastAPI/Starlette Jun 26, 2023
@tombruijn
Copy link
Member

tombruijn commented Jun 27, 2023

@unflxw you mentioned something in a DM about reading the params from a request not being as easy as asking request.params() but having to read the request body too. Can you elaborate here on the issue description?

@unflxw
Copy link
Contributor Author

unflxw commented Jul 6, 2023

Ah, yes, thanks for bringing it up @tombruijn! Here's some words.

First, some context: the Flask instrumentation is a tiny wrapper over the WSGI instrumentation, with the main difference being that it wraps the WSGI app inside a Flask app automatically, and adds Flask routing information. Similarly, the FastAPI and Starlette instrumentations are tiny wrappers over the ASGI instrumentation.

All of these frameworks (Flask, FastAPI, Starlette) offer a convenient way to read the request body as a JSON (request.json in Flask, await request.json() in FastAPI and Starlette) which would be the ideal way to do this task.

But! Because the instrumentations are tiny wrappers over the WSGI and ASGI instrumentations, what we get in the configurable request_hook for the instrumentations is not a Flask or FastAPI or Starlette request object, so we can't use those convenient methods. Instead, we get a WSGI environ or an ASGI scope.

For a WSGI environ, there is a wsgi.input key which contains an I/O stream object. We'd have to read it, which would empty its contents, meaning the actual framework wouldn't be able to read the body -- so we'd also have to put another stream back with the contents. This is doable, most likely, as we already do something like this for Gorilla Mux in Go. In that code snippet, r.Body is the stream that is read and replaced.

For an ASGI scope... I have no idea how we'd do it. The body isn't even in the ASGI scope -- it's obtained by awaiting this separate receive() channel thing.

This might require modifying the ASGI instrumentation so that this channel is passed along -- confusingly, there is a client_request_hook for when receive() is called, but the return value of awaiting receive() is not sent to the hook. Even then, though, I don't know how we'd do it to put a message back into this thing, so that the framework can await it again.

Instead, we might want to modify the FastAPI and Starlette instrumentations, so that we can define a hook that receives a Starlette request as an argument, instead of an ASGI scope. Then we can call the convenience methods there.

@unflxw
Copy link
Contributor Author

unflxw commented Oct 24, 2023

@unflxw
Copy link
Contributor Author

unflxw commented Oct 27, 2023

Just writing a note that we're seeing customers implement this on their own endpoints by calling set_params(request.json), so it's clear that this is a desired feature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants