appsignal/appsignal-python

Request body and path parameters in Flask/FastAPI/Starlette

unflxw opened this issue · 5 comments

unflxw commented

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.

tombruijn commented

@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 commented

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 commented

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.