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

Clarify that exemplar reservoir default may change in a minor version #3943

Merged

Conversation

jack-berg
Copy link
Member

@jack-berg jack-berg commented Mar 15, 2024

Related to #3756.

See this conversation for context.

@jack-berg jack-berg requested review from a team March 15, 2024 15:26
@jack-berg
Copy link
Member Author

cc @jsuereth / @reyang

specification/metrics/sdk.md Outdated Show resolved Hide resolved
@jack-berg jack-berg merged commit 7145a5d into open-telemetry:main Mar 20, 2024
7 checks passed
@CodeBlanch
Copy link
Member

I have a question about this:

Exemplar default reservoirs MAY change in a minor version bump. No guarantees are made on the shape or statistical properties of returned exemplars.

The spec has pretty clear defaults defined: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#exemplar-defaults

@cijothomas Is recommending that in .NET we do a design that is this (more or less):

Exemplar defaults
The SDK will include three types of built-in exemplar reservoirs:

SimpleFixedSizeExemplarReservoir
AlignedHistogramBucketExemplarReservoir
NoopExemplarReservoir

By default:

All metrics will use the NoopExemplarReservoir by default. In order to capture Exemplars users MUST use the view API to select either SimpleFixedSizeExemplarReservoir, AlignedHistogramBucketExemplarReservoir, or a custom ExemplarReservoir.

This seems to be a big deviation from the spec switching Exemplars to be individually opt-in. My question is, was the change introduced on this PR intended to give SDKs such leeway?

@jack-berg
Copy link
Member Author

No, the intent of this change was to allow us to later introduce new spec-defined reservoirs, and be able to change the default reservoir for a particular instrument type. In particular, there are likely improvements possible to the reservoir strategy for exponential bucket histograms.

carlosalberto pushed a commit to carlosalberto/opentelemetry-specification that referenced this pull request Oct 31, 2024
…open-telemetry#3943)

Related to open-telemetry#3756. 

See this
[conversation](open-telemetry#3870 (comment))
for context.

---------

Co-authored-by: Reiley Yang <reyang@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants