prometheus extra #213
prometheus extra #213
Conversation
✅ Deploy Preview for phenomenal-crepe-0effec ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
Requesting review from @arnaudmiribel or @blackary @kajarenc could you take a quick look at the streamlit hooks for any concerns? I tested it a fair bit and looks like compatibility goes back quite a ways too (I looked back as far as 1.20) |
Revert changes from #7893 , and reimplement the same result by grouping stats on individual providers' level. We reworked this because the way how it was implemented previously was broke hack way to inject custom stats (see context here: arnaudmiribel/streamlit-extras#213 ).
|
Ready for review and merge |
|
|
||
| from prometheus_client import CollectorRegistry | ||
| from prometheus_client.openmetrics.exposition import generate_latest | ||
| from streamlit.runtime.stats import CacheStatsProvider |
There was a problem hiding this comment.
Is this import something that's been valid for a few releases of streamlit already? We don't pin streamlit to anything else then > 1.0.0 so if this was introduced only in 1.31.0, that may break for others. LMK
There was a problem hiding this comment.
(I read **Note:** This extra works best with Streamlit >= 1.31. There are known bugs with some earlier Streamlit versions, especially 1.30.0., just curious if it would bug or break with prior)
There was a problem hiding this comment.
Yes, this import has worked since 1.12.
There was a bug in Streamlit's native implementation of Prometheus since before 1.12 -> 1.30 where it printed duplicate metric rows that breaks the protocol format. I think in practice most collectors would be able to handle it but it might throw a warning or something (and we never surfaced the endpoint for real use so no one complained). So this collector would work prior to 1.30 but the Streamlit portion might be slightly weird (I think there's a hacky way to fix it if anyone complains about that, I could do another PR).
Karen fixed that issue in 1.30 streamlit/streamlit#7921 in a way that totally broke this prometheus collector. So the only version that won't work at all is 1.30. Then he fixed it to be compatible with this AND have the bug fix in 1.31.
I think it's OK with the note but LMK if you think we should handle another way!
There was a problem hiding this comment.
Thanks a lot for those precisions. I was mostly being careful for ImportErrors, but 1.12 is already a while back!
Let's do it!
|
|
||
| from prometheus_client import CollectorRegistry | ||
| from prometheus_client.openmetrics.exposition import generate_latest | ||
| from streamlit.runtime.stats import CacheStatsProvider |
There was a problem hiding this comment.
Thanks a lot for those precisions. I was mostly being careful for ImportErrors, but 1.12 is already a while back!
Let's do it!
Revert changes from streamlit#7893 , and reimplement the same result by grouping stats on individual providers' level. We reworked this because the way how it was implemented previously was broke hack way to inject custom stats (see context here: arnaudmiribel/streamlit-extras#213 ).
This will be broken until 1.31, do not merge
Add an extra for exposing Prometheus metrics via the Prometheus python client and Streamlit's existing (not really documented) metrics endpoint at
/_stcore/metrics.Note: I added prometheus-client dependency which is pretty lightweight. I can also remove it and just call out in the docs that it needs to be installed?? Not sure if we usually just add all global dependencies here or not.
Working example: https://github.com/sfc-gh-jcarroll/streamlit-extras/tree/prometheus-example/prometheus_example