Prometheus metrics #174
Merged
Prometheus metrics #174
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thanks for the PR! Will take a review next week. |
|
And it will be great if you can get the CI pass meanwhile. I have updated the toolchain version on master. |
xlc
reviewed
May 14, 2024
src/extensions/prometheus/mod.rs
Outdated
| Some(p) if p.is_empty() => None, | ||
| p => p, | ||
| }; | ||
| let registry = Registry::new_custom(prefix, labels).expect("Can't happen"); |
There was a problem hiding this comment.
It only can fail if the prefix is equal to Some(p) where p.is_mepty() and we take care of that in the match above
There was a problem hiding this comment.
can you explain it in the expect? otherwise the next reader is likely going to have the same question
There was a problem hiding this comment.
Sure, I think now it should be better
Co-authored-by: Xiliang Chen <xlchen1291@gmail.com>
xlc
approved these changes
May 16, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We are using your proxy for quite some time now and it works great for us, thank you for this :)
We needed metrics for our internal reporting & stuff so quite a while ago I have added them to our fork of subway. I also saw you have them marked as a TODO in the readme. So this pr is a work I have already done to introduce prometheus metrics to subway. If you think this would be nice to have here you go, of course Im happy to go through rounds of review :)
This pr contains 2 major changes.
To see changes related only to
jsonrpseebump look at the commitupdate jsonrpseeand simiraly to see chanegs related only to prometheus inclusion take a look at theAdd prometheus metricsWhy jsonrpsee bump?
While it was not strictly need, without it the metrics related to the ws connection would be nearly impossible to write (and for sure it would be quite painfull to do).
Also since I did not have an access to your fork of the jsonrpsee this was the quickest thing I could do to make those changes.
What kind of metrics there are?
So there is ws/http counter metrics for open/closed sessions. Also metrics for the methods that are based on the ones that are used in the substrate - same names etc. Additionally metrics for cache misses per method.
Cc @xlc