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

Add the Exponential Histogram Aggregator. #4245

Merged
merged 34 commits into from Aug 4, 2023

Conversation

MadVikingGod
Copy link
Contributor

This is the final part of #2966.

This adds the Aggregator and the mechanism to configure it.

Some notes when reading this:

  • When Scale is 20, every normal floating point can be mapped neatly into a bucket at some offset.
  • When Scale is -10 every number maps to bucket -1 or 0.
  • The spec says we can round subnormal numbers to the smallest normal number for bucketing. This is done because most of the math.Log returns inaccurate results for subnormal numbers.

@codecov
Copy link

codecov bot commented Jun 21, 2023

Codecov Report

Merging #4245 (e626be7) into main (f67ecb3) will increase coverage by 0.1%.
The diff coverage is 92.5%.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##            main   #4245     +/-   ##
=======================================
+ Coverage   83.0%   83.2%   +0.1%     
=======================================
  Files        217     218      +1     
  Lines      17166   17460    +294     
=======================================
+ Hits       14264   14538    +274     
- Misses      2611    2629     +18     
- Partials     291     293      +2     
Files Changed Coverage Δ
sdk/metric/pipeline.go 88.3% <10.0%> (-2.4%) ⬇️
sdk/metric/aggregation/aggregation.go 80.9% <72.7%> (-3.0%) ⬇️
...metric/internal/aggregate/exponential_histogram.go 96.2% <96.2%> (ø)
sdk/metric/internal/aggregate/aggregate.go 100.0% <100.0%> (ø)

... and 1 file with indirect coverage changes

@MadVikingGod MadVikingGod mentioned this pull request Jun 15, 2023
3 tasks
Copy link
Member

@pellared pellared left a comment

Choose a reason for hiding this comment

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

Can we add a "test example" to demonstrate how one can configure an exponential histogram?

sdk/metric/aggregation/aggregation.go Outdated Show resolved Hide resolved
@MadVikingGod
Copy link
Contributor Author

Can we add a "test example" to demonstrate how one can configure an exponential histogram?

Done: added in b41b1e4

@MadVikingGod MadVikingGod linked an issue Jun 22, 2023 that may be closed by this pull request
3 tasks
@MadVikingGod MadVikingGod self-assigned this Jun 22, 2023
@pellared
Copy link
Member

Changelog is missing 😉

@pellared pellared added area:metrics Part of OpenTelemetry Metrics pkg:SDK Related to an SDK package labels Jun 22, 2023
CHANGELOG.md Outdated Show resolved Hide resolved
sdk/metric/aggregation/aggregation.go Show resolved Hide resolved
sdk/metric/aggregation/aggregation.go Show resolved Hide resolved
sdk/metric/aggregation/aggregation.go Outdated Show resolved Hide resolved
sdk/metric/internal/aggregate/exponential_histogram.go Outdated Show resolved Hide resolved
sdk/metric/internal/aggregate/float64.go Outdated Show resolved Hide resolved
sdk/metric/internal/aggregate/float64.go Outdated Show resolved Hide resolved
sdk/metric/internal/aggregate/exponential_histogram.go Outdated Show resolved Hide resolved
@MrAlias
Copy link
Contributor

MrAlias commented Aug 4, 2023

Can you add "inOut" to the ignored codespell words?

@MrAlias MrAlias merged commit 248413d into open-telemetry:main Aug 4, 2023
22 checks passed
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
No open projects
Development

Successfully merging this pull request may close these issues.

Support exponential histograms
5 participants