[SDK] fix: exemplar filters by proost · Pull Request #4267 · open-telemetry/opentelemetry-cpp

proost · 2026-07-18T05:50:43Z

Changes

Breaking Change is accompanied, But Because this is preview version, I think acceptable.

I will send another PR for configuration from envs.

CHANGELOG.md updated for non-trivial changes
Unit tests have been added
Changes in public API reviewed

codecov · 2026-07-18T06:28:24Z

Codecov Report

❌ Patch coverage is 84.37500% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.47%. Comparing base (5ef557b) to head (685210e).

Files with missing lines	Patch %	Lines
...ntelemetry/sdk/metrics/state/sync_metric_storage.h	55.56%	4 Missing ⚠️
...telemetry/sdk/metrics/state/async_metric_storage.h	75.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4267      +/-   ##
==========================================
+ Coverage   81.30%   81.47%   +0.17%     
==========================================
  Files         447      448       +1     
  Lines       19028    19032       +4     
==========================================
+ Hits        15469    15504      +35     
+ Misses       3559     3528      -31

Files with missing lines	Coverage Δ
...ntelemetry/sdk/metrics/exemplar/filter_predicate.h	`100.00% <100.00%> (ø)`
...k/metrics/exemplar/fixed_size_exemplar_reservoir.h	`89.66% <100.00%> (ø)`
...metry/sdk/metrics/exemplar/no_exemplar_reservoir.h	`100.00% <100.00%> (ø)`
...ude/opentelemetry/sdk/metrics/exemplar/reservoir.h	`100.00% <ø> (ø)`
...pentelemetry/sdk/metrics/exemplar/reservoir_cell.h	`97.78% <100.00%> (+41.26%)`	⬆️
...entelemetry/sdk/metrics/exemplar/reservoir_utils.h	`100.00% <100.00%> (ø)`
sdk/src/metrics/meter.cc	`81.36% <ø> (ø)`
...telemetry/sdk/metrics/state/async_metric_storage.h	`93.19% <75.00%> (+2.28%)`	⬆️
...ntelemetry/sdk/metrics/state/sync_metric_storage.h	`84.94% <55.56%> (+1.19%)`	⬆️

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

lalitb · 2026-07-20T22:04:21Z

@proost - I think this won't yet close #4178 - We can't declare exemplar stable till we have evaluated their performance on hot-path of metrics.

…to feat-exemplar-filters

dbarker

Thanks for the PR! Please see some initial feedback below.

…ry-cpp into feat-exemplar-filters

…to feat-exemplar-filters

proost · 2026-07-31T14:31:29Z

@dbarker @lalitb
I'm sorry to bothering you. I missunderstand the issue and spec.

So this PR change:

removing span context check so that alwaysOn works correctly.
little bit refactoring(removing useless "GetSimpleFilteredExemplarReservoir")
tiny performance improving that removing timestamp.

…y-cpp into feat-exemplar-filters

Copilot

🟡 Not ready to approve

There are correctness and robustness issues in the updated exemplar code paths (notably async exemplar attribute handling and reservoir cell reset semantics) that should be addressed before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Pull request overview

This PR updates the Metrics SDK exemplar “preview” pipeline to align with spec changes by moving exemplar eligibility logic into a predicate, simplifying reservoir selection based on the filter type, and removing timestamps from the ExemplarReservoir::OfferMeasurement() API.

Changes:

Remove the SystemTimestamp parameter from preview ExemplarReservoir::OfferMeasurement() and update all call sites/tests accordingly.
Centralize exemplar eligibility via ExemplarFilterEnabled(...) and add unit tests for the filter predicate and span-context behavior.
Adjust exemplar reservoir construction to respect ExemplarFilterType (notably kAlwaysOff) and update build/test wiring (CMake + Bazel).

File summaries

File	Description
sdk/test/metrics/exemplar/with_trace_sample_filter_test.cc	Removes legacy tests tied to the old `ExemplarFilter` interface.
sdk/test/metrics/exemplar/always_sample_filter_test.cc	Removes legacy tests tied to the old `ExemplarFilter` interface.
sdk/test/metrics/exemplar/reservoir_cell_test.cc	Adds coverage for producing exemplars without an active span context.
sdk/test/metrics/exemplar/no_exemplar_reservoir_test.cc	Updates tests for the new `OfferMeasurement` signature (no timestamp).
sdk/test/metrics/exemplar/aligned_histogram_bucket_exemplar_reservoir_test.cc	Updates tests for the new `OfferMeasurement` signature (no timestamp).
sdk/test/metrics/exemplar/filter_predicate_test.cc	Adds new unit tests for `ExemplarFilterEnabled(...)` behavior.
sdk/test/metrics/exemplar/CMakeLists.txt	Registers the new filter predicate test target.
sdk/test/metrics/exemplar/BUILD	Registers the new Bazel test target for filter predicate tests.
sdk/src/metrics/meter.cc	Passes the filter type into reservoir selection.
sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h	Uses `ExemplarFilterEnabled` and removes timestamp from reservoir offers.
sdk/include/opentelemetry/sdk/metrics/state/async_metric_storage.h	Uses `ExemplarFilterEnabled` and removes timestamp from reservoir offers.
sdk/include/opentelemetry/sdk/metrics/exemplar/reservoir.h	Updates the reservoir interface to drop timestamps and removes filtered-reservoir factory.
sdk/include/opentelemetry/sdk/metrics/exemplar/reservoir_utils.h	Selects `NoExemplarReservoir` when filter is `kAlwaysOff` and uses factory helpers.
sdk/include/opentelemetry/sdk/metrics/exemplar/reservoir_cell.h	Changes how span context is captured (always overwritten).
sdk/include/opentelemetry/sdk/metrics/exemplar/no_exemplar_reservoir.h	Updates overrides for the new `OfferMeasurement` signature.
sdk/include/opentelemetry/sdk/metrics/exemplar/fixed_size_exemplar_reservoir.h	Updates overrides for the new `OfferMeasurement` signature.
sdk/include/opentelemetry/sdk/metrics/exemplar/filter_predicate.h	Introduces the shared exemplar filter predicate helper.
ci/do_ci.sh	Increases Bazel valgrind test timeout.
CHANGELOG.md	Documents exemplar filtering changes and the breaking preview API update.

Review details

Files reviewed: 19/19 changed files
Comments generated: 5
Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

+  auto data = reservoir_cell.GetAndResetLong(MetricAttributes{});
+  ASSERT_NE(data, nullptr);
+  EXPECT_FALSE(data->GetSpanContext().IsValid());
+}


                        const opentelemetry::context::Context &context)
  {
    attributes_  = attributes;
    record_time_ = opentelemetry::common::SystemTimestamp(std::chrono::system_clock::now());
-    const auto current_ctx = opentelemetry::trace::GetSpanContext(context);
-    if (current_ctx.IsValid())
-    {
-      context_.reset(new opentelemetry::trace::SpanContext{current_ctx});
-    }
+    context_.reset(
+        new opentelemetry::trace::SpanContext{opentelemetry::trace::GetSpanContext(context)});
  }


+      if (offer_exemplars)
      {
-        exemplar_reservoir_->OfferMeasurement(measurement.second, {}, {},
-                                              std::chrono::system_clock::now());
+        exemplar_reservoir_->OfferMeasurement(measurement.second, {}, {});
      }


+  switch (filter_type)
+  {
+    case ExemplarFilterType::kAlwaysOn:
+      return true;
+    case ExemplarFilterType::kAlwaysOff:
+      return false;
+    default:  // TraceBased.
+      const opentelemetry::trace::SpanContext span_context =
+          opentelemetry::trace::GetSpanContext(context);
+      return span_context.IsValid() && span_context.IsSampled();
+  }


+* [SDK] Complete exemplar filtering: the exemplar filter(`AlwaysOn`/
+  `AlwaysOff`/`TraceBased`)


proost added 2 commits July 18, 2026 14:23

feat: examplar filters

21c3ab6

test: using mock

b13986f

proost requested a review from a team as a code owner July 18, 2026 05:50

proost added 2 commits July 18, 2026 14:51

doc: update change log

8dc295b

style: follow lint

f99c77b

lalitb reviewed Jul 20, 2026

View reviewed changes

Comment thread sdk/include/opentelemetry/sdk/metrics/exemplar/reservoir_utils.h Outdated

lalitb reviewed Jul 20, 2026

View reviewed changes

Comment thread sdk/include/opentelemetry/sdk/metrics/exemplar/filtered_exemplar_reservoir.h Outdated

Merge branch 'main' of github.com:open-telemetry/opentelemetry-cpp in…

99d868a

…to feat-exemplar-filters

proost requested a review from lalitb July 22, 2026 15:05

dbarker reviewed Jul 23, 2026

View reviewed changes

Comment thread sdk/test/metrics/exemplar/filtered_exemplar_reservoir_test.cc Outdated

proost added 3 commits July 24, 2026 23:23

ci: higher test timeout

a12d4c3

Merge branch 'main' of github.com:open-telemetry/opentelemetry-cpp in…

d793892

…to feat-exemplar-filters

Merge branch 'main' of github.com:open-telemetry/opentelemetry-cpp in…

60b9440

…to feat-exemplar-filters

proost requested a review from dbarker July 25, 2026 04:14

proost and others added 2 commits July 28, 2026 17:38

Merge branch 'main' of github.com:open-telemetry/opentelemetry-cpp in…

fd50a26

…to feat-exemplar-filters

Merge branch 'main' into feat-exemplar-filters

5733f19

dbarker reviewed Jul 29, 2026

View reviewed changes

Comment thread sdk/include/opentelemetry/sdk/metrics/state/async_metric_storage.h Outdated

Comment thread sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h Outdated

Comment thread sdk/include/opentelemetry/sdk/metrics/exemplar/filtered_exemplar_reservoir.h Outdated

proost added 7 commits July 30, 2026 16:53

BREAKING: remove timestamp from OfferMeasurement

ca5c78e

Merge branch 'feat-exemplar-filters' of github.com:proost/opentelemet…

f4e1873

…ry-cpp into feat-exemplar-filters

Merge branch 'main' of github.com:open-telemetry/opentelemetry-cpp in…

27b0702

…to feat-exemplar-filters

perf: use ExemplarFilterEnabled

af8d052

fix: remove span context validation

30496c0

style: rollback ctor order

47625e6

Merge branch 'main' of github.com:open-telemetry/opentelemetry-cpp in…

cdac424

…to feat-exemplar-filters

proost changed the title ~~[SDK] feat: exemplar filters~~ [SDK] fix: exemplar filters Jul 31, 2026

proost requested a review from dbarker July 31, 2026 14:31

ThomsonTan reviewed Jul 31, 2026

View reviewed changes

Comment thread CHANGELOG.md

proost added 2 commits August 1, 2026 14:52

Merge branch 'main' of https://github.com/open-telemetry/opentelemetr…

acd26fd

…y-cpp into feat-exemplar-filters

doc: update breaking change

5eddcd2

proost requested a review from ThomsonTan August 1, 2026 06:00

proost mentioned this pull request Aug 2, 2026

feat: OTEL_METRICS_EXEMPLAR_FILTER env #4328

Open

3 tasks

proost and others added 2 commits August 2, 2026 23:52

Merge branch 'main' of https://github.com/open-telemetry/opentelemetr…

058a013

…y-cpp into feat-exemplar-filters

Merge branch 'main' into feat-exemplar-filters

685210e

dbarker requested a review from Copilot August 2, 2026 22:22

Copilot started reviewing on behalf of dbarker August 2, 2026 22:23 View session

Copilot AI reviewed Aug 2, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[SDK] fix: exemplar filters - #4267

[SDK] fix: exemplar filters#4267
proost wants to merge 21 commits into
open-telemetry:mainfrom
proost:feat-exemplar-filters

proost commented Jul 18, 2026 •

edited

Loading

Uh oh!

codecov Bot commented Jul 18, 2026 •

edited

Loading

Uh oh!

lalitb commented Jul 20, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dbarker left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

proost commented Jul 31, 2026 •

edited

Loading

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

		* [SDK] Complete exemplar filtering: the exemplar filter(`AlwaysOn`/
		`AlwaysOff`/`TraceBased`)

Uh oh!

Conversation

proost commented Jul 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Uh oh!

codecov Bot commented Jul 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

lalitb commented Jul 20, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dbarker left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

proost commented Jul 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Not ready to approve

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

proost commented Jul 18, 2026 •

edited

Loading

codecov Bot commented Jul 18, 2026 •

edited

Loading

proost commented Jul 31, 2026 •

edited

Loading