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

Correct location for DefaultExponentialHistogram #4347

Closed
MadVikingGod opened this issue Jul 20, 2023 · 2 comments
Closed

Correct location for DefaultExponentialHistogram #4347

MadVikingGod opened this issue Jul 20, 2023 · 2 comments
Labels
area:metrics Part of OpenTelemetry Metrics pkg:SDK Related to an SDK package

Comments

@MadVikingGod
Copy link
Contributor

MadVikingGod commented Jul 20, 2023

Originally Discussion in #4245 (comment)

Currently, for all Aggregators in the sdk/metric/aggregation package, the zero value, e.g. ExplicitBucketHistogram{}, will result in a data stream with the recommended settings from the specification.
The Exponential Histogram does not, specifically MaxScale is valid at 0 and the recommended starting value is 20.

The user experience of setting up a view to use the specification-recommended Exponential Histogram currently would be to find the recommendations in the specification and then construct the Aggregation to match.

The goal of this ticket is to improve that experience.

Proposed solutions:

DocStrings

Add the recommended configuration to the doc string. This has the least impact on code.

DefaultExponentialHistogram()

This would be a function that returns the specification-recommended Aggregation. It could be located in the sdk/metric/aggregation alongside the type definition of ExponentialHistogram, or in sdk/metric alongside the DefaultAggregationSelector and DefaultTemporalitySelector.

ExponentialHistogram Constructor

This would allow you to only build valid ExponentialHistograms via a Constructor. There are multiple ways that this could be achieved via unexporting fields, or making a HasValue like field.

@MadVikingGod MadVikingGod added pkg:SDK Related to an SDK package area:metrics Part of OpenTelemetry Metrics labels Jul 20, 2023
@MrAlias
Copy link
Contributor

MrAlias commented Jul 24, 2023

Moving to the post-GA project as this doesn't seem needed for GA (right?).

@MrAlias
Copy link
Contributor

MrAlias commented Jan 25, 2024

Closing as we have not seen a large user desire for this. If there is desire in the future, please comment and we can reopen.

@MrAlias MrAlias closed this as completed Jan 25, 2024
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
Development

No branches or pull requests

2 participants