-
Notifications
You must be signed in to change notification settings - Fork 33
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
Add metrics to the catalog server #156
Add metrics to the catalog server #156
Conversation
Codecov ReportPatch and project coverage have no change.
Additional details and impacted files@@ Coverage Diff @@
## main #156 +/- ##
=======================================
Coverage 79.06% 79.06%
=======================================
Files 3 3
Lines 215 215
=======================================
Hits 170 170
Misses 28 28
Partials 17 17 ☔ View full report in Codecov by Sentry. 📢 Have feedback on the report? Share it here. |
dbb9540
to
efa5262
Compare
that can be used for calculating the Apdex Score and assess the health of the http server that is serving catalog contents to clients Signed-off-by: Bryce Palmer <[email protected]>
7fcb6ea
to
1c56928
Compare
Signed-off-by: Bryce Palmer <[email protected]>
Signed-off-by: Bryce Palmer <[email protected]>
Signed-off-by: Bryce Palmer <[email protected]>
// calculate Apdex Scores up to a T of 1 second, but using various mathmatical formulas we | ||
// should be able to estimate Apdex Scores up to a T of 2.5. Having a larger range of buckets | ||
// will allow us to more easily calculate health indicators other than the Apdex Score. | ||
Buckets: []float64{0.1, 0.2, 0.3, 0.4, 0.5, 0.6, 0.7, 0.8, 0.9, 1, 1.2, 1.6, 2, 2.4, 2.8, 3.2, 3.6, 4, 10}, |
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.
Does it matter that our write timeout is 10s and the max bucket duration is 10s?
Seems like we'll only ever get whatever error code maps to that timeout in the 10s bucket.
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 don't think so. If anything I think it means that we now have buckets that capture all possible response times and that allows us to calculate more metrics on the fly. This is all going based on #156 (comment) . Since if no requests take more than 10s we will never have anything in the "Inf" bucket.
That being said, I could be wrong - I don't have enough experience in this area to truly know and am making an assumption with what I currently know
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.
Seems like we'll only ever get whatever error code maps to that timeout in the 10s bucket.
Any response time > 4s and <= 10s will fall in that 10s bucket
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.
Ah, got it. sgtm.
The suggested change was accepted. Huzzah!
Description
Motivation
Storage
implementation #127