-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Implement metrics reader #3004
Implement metrics reader #3004
Conversation
Signed-off-by: Albert Teoh <albert.teoh@logz.io>
Codecov Report
@@ Coverage Diff @@
## master #3004 +/- ##
==========================================
+ Coverage 95.94% 95.99% +0.04%
==========================================
Files 224 225 +1
Lines 9773 9891 +118
==========================================
+ Hits 9377 9495 +118
Misses 326 326
Partials 70 70
Continue to review full report at Codecov.
|
Friendly nudge on this, if anyone could take a look please; I'd like to move forward on this feature. I'm cognisant that this PR is on the larger side as I wanted to provide more context for the reviewer; however, if the preference is to make this smaller, I could break it up into smaller PRs too. The failing unit test is caused by a known flaky test issue, which I'm looking at now: #3025 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had a question about the usage of sum(rate()), but other than that, looks good to me.
Signed-off-by: albertteoh <albert.teoh@logz.io>
Signed-off-by: albertteoh <albert.teoh@logz.io>
Signed-off-by: albertteoh <albert.teoh@logz.io>
Signed-off-by: albertteoh <albert.teoh@logz.io>
Thank you for the review, @jpkrohling! |
Signed-off-by: Albert Teoh albert.teoh@logz.io
Which problem is this PR solving?
Short description of the changes
GetLatencies
,GetCallRates
andGetErrorRates
of the PrometheusMetricsReader
.