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

Allow configuring the exemplar filter on the metrics SDK #5850

Merged
merged 15 commits into from
Oct 11, 2024

Conversation

dashpole
Copy link
Contributor

@dashpole dashpole commented Sep 27, 2024

Part of #5249

Spec

https://opentelemetry.io/docs/specs/otel/metrics/sdk/#exemplarfilter

The ExemplarFilter configuration MUST allow users to select between one of the built-in ExemplarFilters. While ExemplarFilter determines which measurements are eligible for becoming an Exemplar, the ExemplarReservoir makes the final decision if a measurement becomes an exemplar and is stored.

The ExemplarFilter SHOULD be a configuration parameter of a MeterProvider for an SDK. The default value SHOULD be TraceBased. The filter configuration SHOULD follow the environment variable specification.

An OpenTelemetry SDK MUST support the following filters:

Changes

  • adds exemplar.AlwaysOffFilter, which is one of the required filters from the SDK: https://opentelemetry.io/docs/specs/otel/metrics/sdk/#alwaysoff
  • adds metric.WithExemplarFilter as an option for the metrics SDK.
  • moves handling of OTEL_METRICS_EXEMPLAR_FILTER to the same location as config handling to make code easier to navigate.

dropReservoir can actually be removed, but I plan to do that in a follow-up refactor, since it will be a large diff.

Copy link

codecov bot commented Sep 27, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.5%. Comparing base (bc2fe88) to head (a653d53).
Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@          Coverage Diff          @@
##            main   #5850   +/-   ##
=====================================
  Coverage   84.5%   84.5%           
=====================================
  Files        272     272           
  Lines      22824   22840   +16     
=====================================
+ Hits       19293   19311   +18     
+ Misses      3182    3180    -2     
  Partials     349     349           

see 5 files with indirect coverage changes

@dashpole dashpole force-pushed the exemplar_filter branch 2 times, most recently from 76f6c41 to 988e0db Compare September 27, 2024 20:48
@dashpole dashpole marked this pull request as draft September 27, 2024 20:48
@dashpole dashpole marked this pull request as ready for review September 30, 2024 20:15
sdk/metric/config.go Outdated Show resolved Hide resolved
@dashpole dashpole mentioned this pull request Oct 1, 2024
6 tasks
sdk/metric/config.go Outdated Show resolved Hide resolved
sdk/metric/config.go Outdated Show resolved Hide resolved
sdk/metric/config.go Outdated Show resolved Hide resolved
sdk/metric/config.go Outdated Show resolved Hide resolved
sdk/metric/config.go Outdated Show resolved Hide resolved
sdk/metric/config_test.go Outdated Show resolved Hide resolved
@dashpole
Copy link
Contributor Author

dashpole commented Oct 4, 2024

@MrAlias lets wait until after the release to merge. I don't like merging public API changes right before a release.

@MrAlias MrAlias added this to the v1.32.0 milestone Oct 10, 2024
@dashpole dashpole merged commit 6b251b8 into open-telemetry:main Oct 11, 2024
32 checks passed
@dashpole dashpole deleted the exemplar_filter branch October 11, 2024 20:02
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.

4 participants