[PLAT-906] counters for sainted read and write (#487) #489
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
* [PLAT-870] Problem: stats for types of requests not distinct Solution: Account for their number and latency distinctly. With the Prometheus Statsd exporter in place, we have real histograms that are fed by `stats_timer`, which means we can ditch related calls to `stats_counter` and we can just reconfigure the bucket widths in the exporter config rather than having a const that says how long is too long. We instrument in `print_errors` because it has the error and latency information as well as the function name. To that end, I've changed the function name passed to `process_status` to more closely align with what someone looking at logs or timeseries might understand because 1) that's easier than more extensive refactoring 2) the other people who care are the ones reading the source and they can just as easily search for calls to `print_errors` * move the timers to process_status to observe successes also * counters for sainted read/write
jerryblakley
approved these changes
May 7, 2019
winmillwill
added a commit
that referenced
this pull request
May 8, 2019
* [PLAT-870] Problem: stats for types of requests not distinct Solution: Account for their number and latency distinctly. With the Prometheus Statsd exporter in place, we have real histograms that are fed by `stats_timer`, which means we can ditch related calls to `stats_counter` and we can just reconfigure the bucket widths in the exporter config rather than having a const that says how long is too long. We instrument in `print_errors` because it has the error and latency information as well as the function name. To that end, I've changed the function name passed to `process_status` to more closely align with what someone looking at logs or timeseries might understand because 1) that's easier than more extensive refactoring 2) the other people who care are the ones reading the source and they can just as easily search for calls to `print_errors` * move the timers to process_status to observe successes also * counters for sainted read/write
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.
Solution: Account for their number and latency distinctly.
With the Prometheus Statsd exporter in place, we have real histograms that are
fed by
stats_timer, which means we can ditch related calls tostats_counterand we can just reconfigure the bucket widths in the exporter config rather than
having a const that says how long is too long.
We instrument in
print_errorsbecause it has the error and latency informationas well as the function name. To that end, I've changed the function name passed
to
process_statusto more closely align with what someone looking at logs ortimeseries might understand because
as easily search for calls to
print_errorsmove the timers to process_status to observe successes also
counters for sainted read/write