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
support histograms #162
support histograms #162
Changes from 7 commits
4c2ee9d
6f51d04
50dd2ae
92c46c3
6582ea2
9308691
3fe3d43
7a9d9f3
338fbd5
7447b55
ac2a390
3feedf2
f0a9361
d246dab
2ca3d21
08eb71a
2d256e2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,6 +7,7 @@ var process_metrics = function (metrics, flushInterval, ts, flushCallback) { | |
var counters = metrics.counters; | ||
var timers = metrics.timers; | ||
var pctThreshold = metrics.pctThreshold; | ||
var histogram = metrics.histogram; | ||
|
||
for (key in counters) { | ||
var value = counters[key]; | ||
|
@@ -72,6 +73,28 @@ var process_metrics = function (metrics, flushInterval, ts, flushCallback) { | |
current_timer_data["sum"] = sum; | ||
current_timer_data["mean"] = mean; | ||
|
||
// note: values bigger than the upper limit of the last bin are ignored, by design | ||
conf = histogram || []; | ||
bins = []; | ||
for (var i = 0; i < conf.length; i++) { | ||
if (key.indexOf(conf[i].metric) > -1) { | ||
bins = conf[i].bins; | ||
break; | ||
} | ||
} | ||
// the outer loop iterates bins, the inner loop iterates timer values; | ||
// within each run of the inner loop we should only consider the timer value range that's within the scope of the current bin | ||
// so we leverage the fact that the values are already sorted to end up with only full 1 iteration of the entire values range | ||
var i = 0; | ||
for (var bin_i = 0; bin_i < bins.length; bin_i++) { | ||
var freq = 0; | ||
for (; i < count && (bins[bin_i] == 'inf' || values[i] < bins[bin_i]); i++) { | ||
freq += 1; | ||
} | ||
bin_name = ('bin_' + bins[bin_i]).replace('.','_'); | ||
current_timer_data[bin_name] = freq; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe it makes sense to namespace this under histogram? So that it get's added to something like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. good idea |
||
} | ||
|
||
timer_data[key] = current_timer_data; | ||
|
||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -54,7 +54,8 @@ function flushMetrics() { | |
sets: sets, | ||
counter_rates: counter_rates, | ||
timer_data: timer_data, | ||
pctThreshold: pctThreshold | ||
pctThreshold: pctThreshold, | ||
histogram: config.histogram | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this doesn't work. It has to be |
||
} | ||
|
||
// After all listeners, reset the stats | ||
|
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.
we generally want to push key name sanitization into the backends, since a
.
separator could be completely valid in one backend but not the_
. So I think we should remove this here.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.
that makes sense. would you be willing to add that commit, it's probably quicker than having a conversation on how exactly you want the sanitization be done.
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.
just remove the
replace()
part. The rest is ok I think.