[API/SDK] feat: exemplar filters#4191
Open
proost wants to merge 9 commits into
Open
Conversation
…to feat-exemplar-filters
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4191 +/- ##
==========================================
+ Coverage 77.92% 78.00% +0.08%
==========================================
Files 439 440 +1
Lines 18612 18628 +16
==========================================
+ Hits 14502 14529 +27
+ Misses 4110 4099 -11
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR completes exemplar filtering by moving filter decisions into a FilteredExemplarReservoir wrapper (instead of scattering filter checks across metric storage code), and updates the logs SDK to reuse a new trace::GetSpanContext() helper for extracting the active span context from a Context.
Changes:
- Add
FilteredExemplarReservoirand wire exemplar filter behavior throughExemplarReservoirfactories (GetSimpleFilteredExemplarReservoir,GetExemplarReservoir). - Remove exemplar-filter branching from sync/async metric storages and update meter/storage call sites accordingly.
- Add
trace::GetSpanContext()API helper, use it in logs SDK, and update/remove related tests/build targets.
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h | Removes in-storage exemplar filtering; always forwards to reservoir (filter now handled by reservoir wrapper). |
| sdk/include/opentelemetry/sdk/metrics/state/async_metric_storage.h | Removes in-storage exemplar filtering; always forwards to reservoir (filter now handled by reservoir wrapper). |
| sdk/include/opentelemetry/sdk/metrics/exemplar/filtered_exemplar_reservoir.h | Adds the filtering reservoir wrapper that enforces ExemplarFilterType. |
| sdk/src/metrics/exemplar/reservoir.cc | Implements GetSimpleFilteredExemplarReservoir() factory creating the wrapper reservoir. |
| sdk/include/opentelemetry/sdk/metrics/exemplar/reservoir.h | Updates filtered-reservoir factory signature to use nostd::shared_ptr. |
| sdk/include/opentelemetry/sdk/metrics/exemplar/reservoir_utils.h | Wraps concrete reservoirs (fixed-size / aligned histogram) with filtered reservoir based on configured filter type. |
| sdk/src/metrics/meter.cc | Passes exemplar filter type into GetExemplarReservoir(...) so storages receive already-filtering reservoirs. |
| api/include/opentelemetry/trace/context.h | Adds trace::GetSpanContext(context) helper for extracting active SpanContext from a Context. |
| sdk/src/logs/logger.cc | Replaces duplicated span-context extraction helper with trace::GetSpanContext(). |
| sdk/test/metrics/exemplar/filtered_exemplar_reservoir_test.cc | Adds unit coverage for AlwaysOn/AlwaysOff/TraceBased behavior in the filtered reservoir wrapper. |
| sdk/test/metrics/exemplar/CMakeLists.txt | Registers the new filtered-reservoir test target. |
| sdk/test/metrics/exemplar/BUILD | Adds Bazel cc_test for filtered_exemplar_reservoir_test. |
| sdk/test/metrics/exemplar/always_sample_filter_test.cc | Removes old ExemplarFilter interface-based test (API removed/changed). |
| sdk/test/metrics/exemplar/with_trace_sample_filter_test.cc | Removes old ExemplarFilter interface-based test (API removed/changed). |
| sdk/test/metrics/sync_metric_storage_counter_test.cc | Updates SyncMetricStorage construction for changed exemplar args (filter removed from ctor). |
| sdk/test/metrics/sync_metric_storage_up_down_counter_test.cc | Updates SyncMetricStorage construction for changed exemplar args (filter removed from ctor). |
| sdk/test/metrics/sync_metric_storage_histogram_test.cc | Updates SyncMetricStorage construction for changed exemplar args (filter removed from ctor). |
| sdk/test/metrics/sync_metric_storage_gauge_test.cc | Updates SyncMetricStorage construction for changed exemplar args (filter removed from ctor). |
| sdk/test/metrics/async_metric_storage_test.cc | Updates AsyncMetricStorage construction for changed exemplar args (filter removed from ctor). |
| sdk/test/metrics/cardinality_limit_test.cc | Updates SyncMetricStorage construction for changed exemplar args (filter removed from ctor). |
| sdk/test/metrics/bound_sync_instruments_test.cc | Updates SyncMetricStorage construction for changed exemplar args (filter removed from ctor). |
| CHANGELOG.md | Documents new trace::GetSpanContext() and exemplar filter application via filtered reservoirs. |
Comment on lines
+72
to
+90
| static bool AlwaysOn(const opentelemetry::context::Context & /* context */) noexcept | ||
| { | ||
| return true; | ||
| } | ||
|
|
||
| static bool AlwaysOff(const opentelemetry::context::Context & /* context */) noexcept | ||
| { | ||
| return false; | ||
| } | ||
|
|
||
| static bool TraceBased(const opentelemetry::context::Context &context) noexcept | ||
| { | ||
| const opentelemetry::trace::SpanContext span_context = | ||
| opentelemetry::trace::GetSpanContext(context); | ||
| return span_context.IsValid() && span_context.IsSampled(); | ||
| } | ||
|
|
||
| static ShouldSampleFn SelectFilter(ExemplarFilterType filter_type) noexcept | ||
| { |
…to feat-exemplar-filters
…to feat-exemplar-filters
…to feat-exemplar-filters
…to feat-exemplar-filters
dbarker
reviewed
Jul 15, 2026
dbarker
left a comment
dbarker
left a comment
Member
There was a problem hiding this comment.
Thanks for the PR. I haven't reviewed fully but wanted to share this initial feedback on breaking out the GetSpanContext method to a separate PR.
|
|
||
| // Get the SpanContext of the active span from explicit context. Falls back to a SpanContext | ||
| // stored directly in the context, and returns an invalid SpanContext if neither is present. | ||
| inline SpanContext GetSpanContext(const context::Context &context) noexcept |
Member
There was a problem hiding this comment.
This is a nice addition for performance and reducing dynamic memory allocation. The GetSpan method above is commonly used just to access the SpanContext from the span inside a full Context with this pattern:
auto span_context = opentelemetry:::trace::GetSpan(context)->GetContext();
When the context does not have a span then one is dynamically allocated just to create an invalid span context.
Consider breaking this change out to a separate PR and replacing that pattern where appropriate.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes
Changes
Complete exemplar filter.
This changes accompany breaking change. When I read versioning policy, flagged feature is unstable, so API breaking change is accepted, right?
I tried to abstract filte logic into
FilteredExemplarReservoirto avoid filtering logic scattered to metric storage.Please provide a brief description of the changes here.
For significant contributions please make sure you have completed the following items:
CHANGELOG.mdupdated for non-trivial changes