instrumentation: fastapi prometheus middleware #1007
Conversation
Coverage report
Click to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||||||||||||||||||||
| setattr(request.state, prometheus.START_TIME_REQUEST_KEY, time.time()) | ||
|
|
||
| # Extract request labels | ||
| path = request.url.path |
There was a problem hiding this comment.
This can cause quite high cardinality, can we align this with the path template. E.g. /subjects/<subject>/versions?
There was a problem hiding this comment.
Yes I can think around it, it was similar thought I had on the tracing PR #1009, where I extract the resource from the path as I didn't find a way to get the template, as this was the traces, it looked fine to trace the resource, as further spans contain relevant information and this reduces the trace cardinality.
In prometheus tho, I do not think this should be a problem, major cardinality issues arise with too many labels. To instrument, might be necessary to see exactly the path that's being checked, but I also get your point. Let me reason around it and get back.
There was a problem hiding this comment.
Looking at this discussion - fastapi/fastapi#12975, the route template is only accessed after a successful response, I do need the path already to instrument the requests in progress.
Unless we want to fallback and extract the base resource, i.e. resource = request.url.path.split("/")[1]. From a template like /subjects/{subject}/versions and a request for /subjects/test-topic-value/versions, this will yield subjects.
Added a middleware to instrument the fastapi requests
nosahama
force-pushed
the
nosahama/EC-627-fastapi-prometheus-metrics
branch
from
d95a95c to
dac8fd2
Compare
|
I will close this in favour of the OTel metrics adding in #1011 |
About this change - What it does
Added a middleware to instrument the fastapi requests.