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
Statsdcc <-> statsd compatibility improvements #1
Conversation
src/ledger.cc
Outdated
@@ -168,23 +168,34 @@ void Ledger::process() { | |||
++threshold_itr) { | |||
double pct = | |||
(*threshold_itr < 0) ? -(*threshold_itr) : (*threshold_itr); | |||
int num_in_threshold = 0; |
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.
Why make num_in_threshold
an int
instead of double
?
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.
It's used as an array pointer so it has to be an int to hit an index in the array.
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.
unsigned long
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.
lgtme, maybe add a comment that we always have a prefix
// generate pct name | ||
char clean_pct[17] = {0}; | ||
snprintf(clean_pct, sizeof(clean_pct), "upper_%g", pct); | ||
snprintf(clean_pct, sizeof(clean_pct), "%g", pct); |
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.
nit: can comment
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 recall what this was for. This takes the value of pct and injects it into the variable clean_pct with formatting before it get converted to a display value. So this is necessary I think.
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.
lgtm
Statsdcc is a multi threaded replacement for statsd that is written in c++. It is being tested as a viable replacement for our statsd installation to improve ingestion. Unfortunately statsdcc is not 100% compatible with statsd making is not a "drop-in" replacement for the product. The effort of this PR is to improve the compatibility between statsd and statsdcc.
The implemented improvement are: