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

Adds Histogram Aggregator #433

Merged
merged 16 commits into from
Jan 21, 2020
Merged

Adds Histogram Aggregator #433

merged 16 commits into from
Jan 21, 2020

Conversation

paivagustavo
Copy link
Member

This adds a new Aggregator implementation Histogram.

Fixes #317

It returns a Buckets which consists of the bucket's boundaries and the counts for each bucket. These counts are not cumulative, the reason for this is to make the update simpler and faster. Exporters may accumulate them if needed.

@paivagustavo paivagustavo added pkg:SDK Related to an SDK package area:metrics Part of OpenTelemetry Metrics labels Jan 16, 2020
Copy link
Contributor

@MrAlias MrAlias left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good from an overall structure. I think with the suggestions/fixes mentioned it should be good to go.

sdk/metric/aggregator/histogram/histogram.go Outdated Show resolved Hide resolved
sdk/metric/aggregator/histogram/histogram.go Outdated Show resolved Hide resolved
Change to less-than buckets. Add offset checks. Unexport fields that don't need to be exported. Fix tests when running on profile with int64 number kind.
@paivagustavo
Copy link
Member Author

I've addressed all the discussions, thanks everyone :)

@rghetia rghetia merged commit 2c460f0 into open-telemetry:master Jan 21, 2020
Copy link
Contributor

@jmacd jmacd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@paivagustavo paivagustavo deleted the histogram_aggregator branch January 21, 2020 18:06
@MrAlias MrAlias mentioned this pull request Feb 11, 2020
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:metrics Part of OpenTelemetry Metrics pkg:SDK Related to an SDK package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement Histogram aggregator
4 participants