Skip to content
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

In-line documentation on histogram buckets is incorrect #1426

Closed
TigerHe7 opened this issue Aug 14, 2020 · 0 comments · Fixed by #1427
Closed

In-line documentation on histogram buckets is incorrect #1426

TigerHe7 opened this issue Aug 14, 2020 · 0 comments · Fixed by #1427
Assignees
Labels
bug Something isn't working

Comments

@TigerHe7
Copy link
Contributor

Bug:
The documentation for the histogram type is incorrect in stating that the explicit bucket boundaries are inclusive upper bounds when they are actually inclusive lower bounds.

In-line comment:

* Buckets are implemented using two different array:
* - boundaries contains every boundary (which are upper boundary for each slice)
* - counts contains count of event for each slice
*
* Note that we'll always have n+1 (where n is the number of boundaries) slice
* because we need to count event that are above the highest boundary. This is the
* reason why it's not implement using array of object, because the last slice
* dont have any boundary.
*
* Example if we measure the values: [5, 30, 5, 40, 5, 15, 15, 15, 25]
* with the boundaries [ 10, 20, 30 ], we will have the following state:
*
* buckets: {
* boundaries: [10, 20, 30],
* counts: [3, 3, 2, 1],

Histogram aggregator implementation:

update(value: number): void {
this._current.count += 1;
this._current.sum += value;
for (let i = 0; i < this._boundaries.length; i++) {
if (value < this._boundaries[i]) {
this._current.buckets.counts[i] += 1;
return;
}
}
// value is above all observed boundaries
this._current.buckets.counts[this._boundaries.length] += 1;
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant