From 798f2cab41492a96c58dca561e7926fd846c168d Mon Sep 17 00:00:00 2001 From: proost Date: Mon, 24 Aug 2026 23:19:59 +0900 Subject: [PATCH 1/4] perf: avoid temp allocation for offer measurement --- ...gned_histogram_bucket_exemplar_reservoir.h | 44 +++-- .../exemplar/fixed_size_exemplar_reservoir.h | 35 ++++ .../metrics/exemplar/no_exemplar_reservoir.h | 15 ++ .../sdk/metrics/exemplar/reservoir.h | 11 ++ .../exemplar/reservoir_cell_selector.h | 13 ++ .../simple_fixed_size_exemplar_reservoir.h | 38 +++- .../sdk/metrics/state/sync_metric_storage.h | 6 +- sdk/test/metrics/exemplar/BUILD | 35 ++++ sdk/test/metrics/exemplar/CMakeLists.txt | 8 +- .../exemplar/exemplar_offer_benchmark.cc | 170 ++++++++++++++++++ .../reservoir_attribute_offer_test.cc | 158 ++++++++++++++++ 11 files changed, 511 insertions(+), 22 deletions(-) create mode 100644 sdk/test/metrics/exemplar/exemplar_offer_benchmark.cc create mode 100644 sdk/test/metrics/exemplar/reservoir_attribute_offer_test.cc diff --git a/sdk/include/opentelemetry/sdk/metrics/exemplar/aligned_histogram_bucket_exemplar_reservoir.h b/sdk/include/opentelemetry/sdk/metrics/exemplar/aligned_histogram_bucket_exemplar_reservoir.h index 0ae31b4be3..d21032594e 100644 --- a/sdk/include/opentelemetry/sdk/metrics/exemplar/aligned_histogram_bucket_exemplar_reservoir.h +++ b/sdk/include/opentelemetry/sdk/metrics/exemplar/aligned_histogram_bucket_exemplar_reservoir.h @@ -8,6 +8,7 @@ # include # include +# include "opentelemetry/common/key_value_iterable.h" # include "opentelemetry/sdk/common/global_log_handler.h" # include "opentelemetry/sdk/metrics/data/exemplar_data.h" # include "opentelemetry/sdk/metrics/exemplar/filter_type.h" @@ -49,18 +50,46 @@ class AlignedHistogramBucketExemplarReservoir : public FixedSizeExemplarReservoi public: HistogramCellSelector(const std::vector &boundaries) : boundaries_(boundaries) {} - int ReservoirCellIndexFor(const std::vector &cells, + int ReservoirCellIndexFor(const std::vector & /* cells */, int64_t value, - const MetricAttributes &attributes, - const opentelemetry::context::Context &context) override + const MetricAttributes & /* attributes */, + const opentelemetry::context::Context & /* context */) override { - return ReservoirCellIndexFor(cells, static_cast(value), attributes, context); + return FindCellIndex(static_cast(value)); } int ReservoirCellIndexFor(const std::vector & /* cells */, double value, const MetricAttributes & /* attributes */, const opentelemetry::context::Context & /* context */) override + { + return FindCellIndex(value); + } + + int ReservoirCellIndexFor(const std::vector & /* cells */, + int64_t value, + const opentelemetry::common::KeyValueIterable & /* attributes */, + const opentelemetry::context::Context & /* context */) override + { + return FindCellIndex(static_cast(value)); + } + + int ReservoirCellIndexFor(const std::vector & /* cells */, + double value, + const opentelemetry::common::KeyValueIterable & /* attributes */, + const opentelemetry::context::Context & /* context */) override + { + return FindCellIndex(value); + } + + public: + void reset() override + { + // Do nothing + } + + private: + int FindCellIndex(double value) const { size_t max_size = boundaries_.size(); for (size_t i = 0; i < max_size; ++i) @@ -75,13 +104,6 @@ class AlignedHistogramBucketExemplarReservoir : public FixedSizeExemplarReservoi return static_cast(max_size); } - public: - void reset() override - { - // Do nothing - } - - private: std::vector boundaries_; }; }; diff --git a/sdk/include/opentelemetry/sdk/metrics/exemplar/fixed_size_exemplar_reservoir.h b/sdk/include/opentelemetry/sdk/metrics/exemplar/fixed_size_exemplar_reservoir.h index c455c0188b..9c6fb03f1a 100644 --- a/sdk/include/opentelemetry/sdk/metrics/exemplar/fixed_size_exemplar_reservoir.h +++ b/sdk/include/opentelemetry/sdk/metrics/exemplar/fixed_size_exemplar_reservoir.h @@ -8,6 +8,7 @@ # include # include +# include "opentelemetry/common/key_value_iterable.h" # include "opentelemetry/context/context.h" # include "opentelemetry/nostd/function_ref.h" # include "opentelemetry/nostd/shared_ptr.h" @@ -68,6 +69,40 @@ class FixedSizeExemplarReservoir : public ExemplarReservoir } } + void OfferMeasurement(int64_t value, + const opentelemetry::common::KeyValueIterable &attributes, + const opentelemetry::context::Context &context) noexcept override + { + if (!reservoir_cell_selector_) + { + return; + } + auto idx = + reservoir_cell_selector_->ReservoirCellIndexFor(storage_, value, attributes, context); + if (idx != -1) + { + MetricAttributes owned_attributes{attributes}; + storage_[idx].RecordLongMeasurement(value, owned_attributes, context); + } + } + + void OfferMeasurement(double value, + const opentelemetry::common::KeyValueIterable &attributes, + const opentelemetry::context::Context &context) noexcept override + { + if (!reservoir_cell_selector_) + { + return; + } + auto idx = + reservoir_cell_selector_->ReservoirCellIndexFor(storage_, value, attributes, context); + if (idx != -1) + { + MetricAttributes owned_attributes{attributes}; + storage_[idx].RecordDoubleMeasurement(value, owned_attributes, context); + } + } + std::vector> CollectAndReset( const MetricAttributes &pointAttributes) noexcept override { diff --git a/sdk/include/opentelemetry/sdk/metrics/exemplar/no_exemplar_reservoir.h b/sdk/include/opentelemetry/sdk/metrics/exemplar/no_exemplar_reservoir.h index 905745abf3..c7bb960f0a 100644 --- a/sdk/include/opentelemetry/sdk/metrics/exemplar/no_exemplar_reservoir.h +++ b/sdk/include/opentelemetry/sdk/metrics/exemplar/no_exemplar_reservoir.h @@ -9,6 +9,7 @@ # include # include +# include "opentelemetry/common/key_value_iterable.h" # include "opentelemetry/common/timestamp.h" # include "opentelemetry/context/context.h" # include "opentelemetry/sdk/metrics/data/exemplar_data.h" @@ -40,6 +41,20 @@ class NoExemplarReservoir final : public ExemplarReservoir // Stores nothing. } + void OfferMeasurement(int64_t /* value */, + const opentelemetry::common::KeyValueIterable & /* attributes */, + const opentelemetry::context::Context & /* context */) noexcept override + { + // Stores nothing. + } + + void OfferMeasurement(double /* value */, + const opentelemetry::common::KeyValueIterable & /* attributes */, + const opentelemetry::context::Context & /* context */) noexcept override + { + // Stores nothing. + } + std::vector> CollectAndReset( const MetricAttributes & /* pointAttributes */) noexcept override { diff --git a/sdk/include/opentelemetry/sdk/metrics/exemplar/reservoir.h b/sdk/include/opentelemetry/sdk/metrics/exemplar/reservoir.h index 23b6d67770..84ce71af80 100644 --- a/sdk/include/opentelemetry/sdk/metrics/exemplar/reservoir.h +++ b/sdk/include/opentelemetry/sdk/metrics/exemplar/reservoir.h @@ -8,6 +8,7 @@ # include # include +# include "opentelemetry/common/key_value_iterable.h" # include "opentelemetry/sdk/metrics/exemplar/reservoir_cell_selector.h" # include "opentelemetry/version.h" @@ -68,6 +69,16 @@ class ExemplarReservoir virtual std::vector> CollectAndReset( const MetricAttributes &pointAttributes) noexcept = 0; + /** Offers a long measurement to be sampled. */ + virtual void OfferMeasurement(int64_t value, + const opentelemetry::common::KeyValueIterable &attributes, + const opentelemetry::context::Context &context) noexcept = 0; + + /** Offers a long measurement to be sampled. */ + virtual void OfferMeasurement(double value, + const opentelemetry::common::KeyValueIterable &attributes, + const opentelemetry::context::Context &context) noexcept = 0; + static nostd::shared_ptr GetSimpleFixedSizeExemplarReservoir( size_t size, const std::shared_ptr &reservoir_cell_selector, diff --git a/sdk/include/opentelemetry/sdk/metrics/exemplar/reservoir_cell_selector.h b/sdk/include/opentelemetry/sdk/metrics/exemplar/reservoir_cell_selector.h index 9ea0eed643..91a4f0a9f8 100644 --- a/sdk/include/opentelemetry/sdk/metrics/exemplar/reservoir_cell_selector.h +++ b/sdk/include/opentelemetry/sdk/metrics/exemplar/reservoir_cell_selector.h @@ -8,6 +8,7 @@ # include # include +# include "opentelemetry/common/key_value_iterable.h" # include "opentelemetry/sdk/metrics/exemplar/filter_type.h" # include "opentelemetry/sdk/metrics/exemplar/reservoir_cell.h" # include "opentelemetry/version.h" @@ -49,6 +50,18 @@ class ReservoirCellSelector /** Called when {@link FixedSizeExemplarReservoir#CollectAndReset(Attributes)}. */ virtual void reset() = 0; + + /** Determine the index of the {@code cells} to record the measurement to. */ + virtual int ReservoirCellIndexFor(const std::vector &cells, + int64_t value, + const opentelemetry::common::KeyValueIterable &attributes, + const opentelemetry::context::Context &context) = 0; + + /** Determine the index of the {@code cells} to record the measurement to. */ + virtual int ReservoirCellIndexFor(const std::vector &cells, + double value, + const opentelemetry::common::KeyValueIterable &attributes, + const opentelemetry::context::Context &context) = 0; }; } // namespace metrics diff --git a/sdk/include/opentelemetry/sdk/metrics/exemplar/simple_fixed_size_exemplar_reservoir.h b/sdk/include/opentelemetry/sdk/metrics/exemplar/simple_fixed_size_exemplar_reservoir.h index 3465431d65..9f57698c7b 100644 --- a/sdk/include/opentelemetry/sdk/metrics/exemplar/simple_fixed_size_exemplar_reservoir.h +++ b/sdk/include/opentelemetry/sdk/metrics/exemplar/simple_fixed_size_exemplar_reservoir.h @@ -8,6 +8,7 @@ # include # include +# include "opentelemetry/common/key_value_iterable.h" # include "opentelemetry/sdk/metrics/data/exemplar_data.h" # include "opentelemetry/sdk/metrics/exemplar/filter_type.h" # include "opentelemetry/sdk/metrics/exemplar/fixed_size_exemplar_reservoir.h" @@ -50,18 +51,42 @@ class SimpleFixedSizeExemplarReservoir : public FixedSizeExemplarReservoir public: SimpleFixedSizeCellSelector(size_t size) : size_(size) {} - int ReservoirCellIndexFor(const std::vector &cells, - int64_t value, - const MetricAttributes &attributes, - const opentelemetry::context::Context &context) override + int ReservoirCellIndexFor(const std::vector & /* cells */, + int64_t /* value */, + const MetricAttributes & /* attributes */, + const opentelemetry::context::Context & /* context */) override { - return ReservoirCellIndexFor(cells, static_cast(value), attributes, context); + return SelectCell(); } int ReservoirCellIndexFor(const std::vector & /* cells */, double /* value */, const MetricAttributes & /* attributes */, const opentelemetry::context::Context & /* context */) override + { + return SelectCell(); + } + + int ReservoirCellIndexFor(const std::vector & /* cells */, + int64_t /* value */, + const opentelemetry::common::KeyValueIterable & /* attributes */, + const opentelemetry::context::Context & /* context */) override + { + return SelectCell(); + } + + int ReservoirCellIndexFor(const std::vector & /* cells */, + double /* value */, + const opentelemetry::common::KeyValueIterable & /* attributes */, + const opentelemetry::context::Context & /* context */) override + { + return SelectCell(); + } + + void reset() override {} + + private: + int SelectCell() { // // The simple reservoir sampling algorithm from the spec below is used. @@ -88,9 +113,6 @@ class SimpleFixedSizeExemplarReservoir : public FixedSizeExemplarReservoir return static_cast(index); } - void reset() override {} - - private: size_t measurements_seen_ = 0; size_t size_; }; // class SimpleFixedSizeCellSelector diff --git a/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h b/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h index c8d58bb7b2..6570eb54a0 100644 --- a/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h +++ b/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h @@ -87,7 +87,8 @@ class SyncMetricStorage : public MetricStorage, public SyncWritableMetricStorage #ifdef ENABLE_METRICS_EXEMPLAR_PREVIEW if (ExemplarFilterEnabled(exemplar_filter_type_, context)) { - exemplar_reservoir_->OfferMeasurement(value, {}, context); + exemplar_reservoir_->OfferMeasurement(value, opentelemetry::common::NoopKeyValueIterable{}, + context); } #endif static MetricAttributes attr = MetricAttributes{}; @@ -144,7 +145,8 @@ class SyncMetricStorage : public MetricStorage, public SyncWritableMetricStorage #ifdef ENABLE_METRICS_EXEMPLAR_PREVIEW if (ExemplarFilterEnabled(exemplar_filter_type_, context)) { - exemplar_reservoir_->OfferMeasurement(value, {}, context); + exemplar_reservoir_->OfferMeasurement(value, opentelemetry::common::NoopKeyValueIterable{}, + context); } #endif static MetricAttributes attr = MetricAttributes{}; diff --git a/sdk/test/metrics/exemplar/BUILD b/sdk/test/metrics/exemplar/BUILD index 2dbfd39f8d..b709a6dba4 100644 --- a/sdk/test/metrics/exemplar/BUILD +++ b/sdk/test/metrics/exemplar/BUILD @@ -2,6 +2,24 @@ # SPDX-License-Identifier: Apache-2.0 load("@rules_cc//cc:cc_test.bzl", "cc_test") +load("//bazel:otel_cc_benchmark.bzl", "otel_cc_benchmark") + +otel_cc_benchmark( + name = "exemplar_offer_benchmark", + srcs = [ + "exemplar_offer_benchmark.cc", + ], + tags = [ + "benchmark", + "metrics", + "test", + ], + deps = [ + "//api", + "//sdk:headers", + "//sdk/src/metrics", + ], +) cc_test( name = "no_exemplar_reservoir_test", @@ -37,6 +55,23 @@ cc_test( ], ) +cc_test( + name = "reservoir_attribute_offer_test", + srcs = [ + "reservoir_attribute_offer_test.cc", + ], + tags = [ + "metrics", + "test", + ], + deps = [ + "//api", + "//sdk:headers", + "//sdk/src/metrics", + "@com_google_googletest//:gtest_main", + ], +) + cc_test( name = "aligned_histogram_bucket_exemplar_reservoir_test", srcs = [ diff --git a/sdk/test/metrics/exemplar/CMakeLists.txt b/sdk/test/metrics/exemplar/CMakeLists.txt index 2dfb891ee2..03aed4c539 100644 --- a/sdk/test/metrics/exemplar/CMakeLists.txt +++ b/sdk/test/metrics/exemplar/CMakeLists.txt @@ -4,7 +4,7 @@ foreach( testname no_exemplar_reservoir_test aligned_histogram_bucket_exemplar_reservoir_test - reservoir_cell_test filter_predicate_test) + reservoir_attribute_offer_test reservoir_cell_test filter_predicate_test) add_executable(${testname} "${testname}.cc") target_link_libraries( ${testname} ${GTEST_BOTH_LIBRARIES} ${CMAKE_THREAD_LIBS_INIT} @@ -14,3 +14,9 @@ foreach( TEST_PREFIX metrics. TEST_LIST ${testname}) endforeach() + +if(OTELCPP_WITH_BENCHMARK) + add_executable(exemplar_offer_benchmark exemplar_offer_benchmark.cc) + target_link_libraries(exemplar_offer_benchmark benchmark::benchmark + ${CMAKE_THREAD_LIBS_INIT} opentelemetry_metrics) +endif() diff --git a/sdk/test/metrics/exemplar/exemplar_offer_benchmark.cc b/sdk/test/metrics/exemplar/exemplar_offer_benchmark.cc new file mode 100644 index 0000000000..b65739df63 --- /dev/null +++ b/sdk/test/metrics/exemplar/exemplar_offer_benchmark.cc @@ -0,0 +1,170 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +#include + +#ifdef ENABLE_METRICS_EXEMPLAR_PREVIEW + +# include +# include +# include +# include +# include +# include + +# include "opentelemetry/common/key_value_iterable_view.h" +# include "opentelemetry/context/context.h" +# include "opentelemetry/nostd/shared_ptr.h" +# include "opentelemetry/nostd/utility.h" +# include "opentelemetry/sdk/metrics/exemplar/filter_type.h" +# include "opentelemetry/sdk/metrics/exemplar/reservoir.h" +# include "opentelemetry/sdk/metrics/instruments.h" +# include "opentelemetry/sdk/metrics/state/sync_metric_storage.h" +# include "opentelemetry/sdk/metrics/view/attributes_processor.h" +# include "opentelemetry/version.h" + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace sdk +{ +namespace metrics +{ +namespace +{ + +using AttributeMap = std::map; + +class DeterministicReservoir final : public ExemplarReservoir +{ +public: + explicit DeterministicReservoir(size_t accept_every) noexcept : accept_every_{accept_every} {} + + void OfferMeasurement(int64_t value, + const MetricAttributes &attributes, + const opentelemetry::context::Context & /* context */) noexcept override + { + if (ShouldAccept()) + { + stored_attributes_ = attributes; + benchmark::DoNotOptimize(value); + benchmark::DoNotOptimize(stored_attributes_); + } + } + + void OfferMeasurement(double value, + const MetricAttributes &attributes, + const opentelemetry::context::Context & /* context */) noexcept override + { + if (ShouldAccept()) + { + stored_attributes_ = attributes; + benchmark::DoNotOptimize(value); + benchmark::DoNotOptimize(stored_attributes_); + } + } + + void OfferMeasurement(int64_t value, + const opentelemetry::common::KeyValueIterable &attributes, + const opentelemetry::context::Context & /* context */) noexcept override + { + if (ShouldAccept()) + { + MetricAttributes owned_attributes{attributes}; + stored_attributes_ = owned_attributes; + benchmark::DoNotOptimize(value); + benchmark::DoNotOptimize(stored_attributes_); + } + } + + void OfferMeasurement(double value, + const opentelemetry::common::KeyValueIterable &attributes, + const opentelemetry::context::Context & /* context */) noexcept override + { + if (ShouldAccept()) + { + MetricAttributes owned_attributes{attributes}; + stored_attributes_ = owned_attributes; + benchmark::DoNotOptimize(value); + benchmark::DoNotOptimize(stored_attributes_); + } + } + + std::vector> CollectAndReset( + const MetricAttributes & /* point_attributes */) noexcept override + { + return {}; + } + + size_t GetOffersSeen() const noexcept { return offers_seen_; } + +private: + bool ShouldAccept() noexcept + { + ++offers_seen_; + return accept_every_ != 0 && offers_seen_ % accept_every_ == 0; + } + + size_t accept_every_; + size_t offers_seen_ = 0; + MetricAttributes stored_attributes_; +}; + +AttributeMap MakeAttributes(size_t attribute_count) +{ + AttributeMap attributes; + for (size_t i = 0; i < attribute_count; ++i) + { + attributes.emplace("attribute-" + std::to_string(i), + std::string(32, static_cast('a' + i % 26))); + } + return attributes; +} + +void BenchmarkArguments(benchmark::internal::Benchmark *benchmark) +{ + constexpr int64_t kAttributeCounts[] = {0, 3, 10, 30}; + constexpr int64_t kAcceptEvery[] = {0, 100, 10, 1}; + for (int64_t attribute_count : kAttributeCounts) + { + for (int64_t accept_every : kAcceptEvery) + { + benchmark->Args({attribute_count, accept_every}); + } + } +} + +void BM_RecordWithExemplarSelection(benchmark::State &state) +{ + const auto attribute_count = static_cast(state.range(0)); + const auto accept_every = static_cast(state.range(1)); + auto attributes = MakeAttributes(attribute_count); + opentelemetry::common::KeyValueIterableView attribute_view{attributes}; + + auto *reservoir = new DeterministicReservoir{accept_every}; + nostd::shared_ptr reservoir_handle{reservoir}; + std::shared_ptr attributes_processor{ + new DefaultAttributesProcessor{}}; + InstrumentDescriptor instrument_descriptor{"benchmark.counter", "", "", InstrumentType::kCounter, + InstrumentValueType::kDouble}; + SyncMetricStorage storage{instrument_descriptor, AggregationType::kSum, + attributes_processor, ExemplarFilterType::kAlwaysOn, + std::move(reservoir_handle), nullptr}; + + for (auto _ : state) + { + storage.RecordDouble(7.0, attribute_view, opentelemetry::context::Context{}); + } + + benchmark::DoNotOptimize(reservoir->GetOffersSeen()); + state.SetItemsProcessed(state.iterations()); +} + +BENCHMARK(BM_RecordWithExemplarSelection)->Apply(BenchmarkArguments); + +} // namespace +} // namespace metrics +} // namespace sdk +OPENTELEMETRY_END_NAMESPACE + +#endif // ENABLE_METRICS_EXEMPLAR_PREVIEW + +BENCHMARK_MAIN(); diff --git a/sdk/test/metrics/exemplar/reservoir_attribute_offer_test.cc b/sdk/test/metrics/exemplar/reservoir_attribute_offer_test.cc new file mode 100644 index 0000000000..f7dca5c255 --- /dev/null +++ b/sdk/test/metrics/exemplar/reservoir_attribute_offer_test.cc @@ -0,0 +1,158 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +#ifdef ENABLE_METRICS_EXEMPLAR_PREVIEW + +# include + +# include +# include +# include +# include +# include + +# include "opentelemetry/common/attribute_value.h" +# include "opentelemetry/common/key_value_iterable.h" +# include "opentelemetry/context/context.h" +# include "opentelemetry/nostd/function_ref.h" +# include "opentelemetry/nostd/string_view.h" +# include "opentelemetry/nostd/variant.h" +# include "opentelemetry/sdk/metrics/data/exemplar_data.h" +# include "opentelemetry/sdk/metrics/exemplar/fixed_size_exemplar_reservoir.h" +# include "opentelemetry/sdk/metrics/exemplar/reservoir.h" +# include "opentelemetry/sdk/metrics/exemplar/reservoir_cell.h" +# include "opentelemetry/sdk/metrics/exemplar/reservoir_cell_selector.h" +# include "opentelemetry/version.h" + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace sdk +{ +namespace metrics +{ +namespace +{ + +class CountingKeyValueIterable final : public opentelemetry::common::KeyValueIterable +{ +public: + bool ForEachKeyValue( + nostd::function_ref callback) + const noexcept override + { + ++for_each_calls_; + return callback("key", nostd::string_view{"value"}); + } + + size_t size() const noexcept override { return 1; } + + size_t GetForEachCalls() const noexcept { return for_each_calls_; } + +private: + mutable size_t for_each_calls_ = 0; +}; + +class DeterministicSelector final : public ReservoirCellSelector +{ +public: + explicit DeterministicSelector(bool accept) noexcept : accept_{accept} {} + + int ReservoirCellIndexFor(const std::vector & /* cells */, + int64_t /* value */, + const MetricAttributes & /* attributes */, + const opentelemetry::context::Context & /* context */) override + { + return accept_ ? 0 : -1; + } + + int ReservoirCellIndexFor(const std::vector & /* cells */, + double /* value */, + const MetricAttributes & /* attributes */, + const opentelemetry::context::Context & /* context */) override + { + return accept_ ? 0 : -1; + } + + int ReservoirCellIndexFor(const std::vector & /* cells */, + int64_t /* value */, + const opentelemetry::common::KeyValueIterable & /* attributes */, + const opentelemetry::context::Context & /* context */) override + { + return accept_ ? 0 : -1; + } + + int ReservoirCellIndexFor(const std::vector & /* cells */, + double /* value */, + const opentelemetry::common::KeyValueIterable & /* attributes */, + const opentelemetry::context::Context & /* context */) override + { + return accept_ ? 0 : -1; + } + + void reset() override {} + +private: + bool accept_; +}; + +class OwnedOnlyReservoir final : public ExemplarReservoir +{ +public: + using ExemplarReservoir::OfferMeasurement; + + void OfferMeasurement(int64_t /* value */, + const MetricAttributes & /* attributes */, + const opentelemetry::context::Context & /* context */) noexcept override + {} + + void OfferMeasurement(double /* value */, + const MetricAttributes & /* attributes */, + const opentelemetry::context::Context & /* context */) noexcept override + {} + + std::vector> CollectAndReset( + const MetricAttributes & /* point_attributes */) noexcept override + { + return {}; + } +}; + +TEST(ExemplarAttributeOffer, RejectedLongOfferDoesNotMaterializeAttributes) +{ + auto selector = std::shared_ptr{new DeterministicSelector{false}}; + FixedSizeExemplarReservoir reservoir{1, selector, &ReservoirCell::GetAndResetLong}; + CountingKeyValueIterable attributes; + + reservoir.OfferMeasurement(static_cast(1), attributes, + opentelemetry::context::Context{}); + + EXPECT_EQ(attributes.GetForEachCalls(), 0); +} + +TEST(ExemplarAttributeOffer, RejectedDoubleOfferDoesNotMaterializeAttributes) +{ + auto selector = std::shared_ptr{new DeterministicSelector{false}}; + FixedSizeExemplarReservoir reservoir{1, selector, &ReservoirCell::GetAndResetDouble}; + CountingKeyValueIterable attributes; + + reservoir.OfferMeasurement(1.0, attributes, opentelemetry::context::Context{}); + + EXPECT_EQ(attributes.GetForEachCalls(), 0); +} + +TEST(ExemplarAttributeOffer, AcceptedOfferMaterializesAttributesAfterSelection) +{ + auto selector = std::shared_ptr{new DeterministicSelector{true}}; + FixedSizeExemplarReservoir reservoir{1, selector, &ReservoirCell::GetAndResetDouble}; + CountingKeyValueIterable attributes; + + reservoir.OfferMeasurement(1.0, attributes, opentelemetry::context::Context{}); + + EXPECT_EQ(attributes.GetForEachCalls(), 1); +} + +} // namespace +} // namespace metrics +} // namespace sdk +OPENTELEMETRY_END_NAMESPACE + +#endif // ENABLE_METRICS_EXEMPLAR_PREVIEW From 1c253e28e347bb32718ca559771ae99b94d9db71 Mon Sep 17 00:00:00 2001 From: proost Date: Mon, 24 Aug 2026 23:32:18 +0900 Subject: [PATCH 2/4] doc: update changelog --- CHANGELOG.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 805c832e50..c4f519260a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -230,6 +230,10 @@ Increment the: `AlwaysOff`/`TraceBased`) [#4267](https://github.com/open-telemetry/opentelemetry-cpp/pull/4267) +* [METRICS SDK] Avoid materializing owned exemplar attributes before + fixed-size reservoir selection. + [#4475](https://github.com/open-telemetry/opentelemetry-cpp/pull/4475) + Important changes: * [API] Never set a null global provider or propagator @@ -337,6 +341,13 @@ Breaking changes: * This is an incompatible API and ABI change for custom exemplar reservoirs. Implementations and callers must remove the timestamp parameter. +* [METRICS SDK] Add non-owning `KeyValueIterable` overloads to the preview + `ExemplarReservoir` and `ReservoirCellSelector` interfaces. Custom reservoir + implementations inherit compatibility adapters but must be rebuilt because + the SDK vtable changes. Custom selector implementations must additionally + implement the new `int64_t` and `double` overloads. + [#4475](https://github.com/open-telemetry/opentelemetry-cpp/pull/4475) + * [METRICS SDK] Breaking change to the preview metrics exemplar surface: the `SyncMetricStorage`/`AsyncMetricStorage` constructors now take an `ExemplarFilterType`, and `ExemplarData::Create` takes the `SpanContext` From 0c8c61a8e05a261ad22fe83ac25dfe6e37ef273f Mon Sep 17 00:00:00 2001 From: proost Date: Mon, 24 Aug 2026 23:33:12 +0900 Subject: [PATCH 3/4] style: trim whitespace --- sdk/test/metrics/exemplar/exemplar_offer_benchmark.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/test/metrics/exemplar/exemplar_offer_benchmark.cc b/sdk/test/metrics/exemplar/exemplar_offer_benchmark.cc index b65739df63..179d7a5e40 100644 --- a/sdk/test/metrics/exemplar/exemplar_offer_benchmark.cc +++ b/sdk/test/metrics/exemplar/exemplar_offer_benchmark.cc @@ -15,7 +15,7 @@ # include "opentelemetry/common/key_value_iterable_view.h" # include "opentelemetry/context/context.h" # include "opentelemetry/nostd/shared_ptr.h" -# include "opentelemetry/nostd/utility.h" +# include "opentelemetry/nostd/utility.h" # include "opentelemetry/sdk/metrics/exemplar/filter_type.h" # include "opentelemetry/sdk/metrics/exemplar/reservoir.h" # include "opentelemetry/sdk/metrics/instruments.h" From 3bba0cef20126dafcda2779212c0d30ffc02b601 Mon Sep 17 00:00:00 2001 From: proost Date: Tue, 25 Aug 2026 00:39:19 +0900 Subject: [PATCH 4/4] style: follow linter --- .../exemplar/exemplar_offer_benchmark.cc | 4 ++-- .../reservoir_attribute_offer_test.cc | 24 ------------------- 2 files changed, 2 insertions(+), 26 deletions(-) diff --git a/sdk/test/metrics/exemplar/exemplar_offer_benchmark.cc b/sdk/test/metrics/exemplar/exemplar_offer_benchmark.cc index 179d7a5e40..c2d627d0f1 100644 --- a/sdk/test/metrics/exemplar/exemplar_offer_benchmark.cc +++ b/sdk/test/metrics/exemplar/exemplar_offer_benchmark.cc @@ -5,7 +5,7 @@ #ifdef ENABLE_METRICS_EXEMPLAR_PREVIEW -# include +# include # include # include # include @@ -36,7 +36,7 @@ using AttributeMap = std::map; class DeterministicReservoir final : public ExemplarReservoir { public: - explicit DeterministicReservoir(size_t accept_every) noexcept : accept_every_{accept_every} {} + explicit DeterministicReservoir(size_t accept_every) : accept_every_{accept_every} {} void OfferMeasurement(int64_t value, const MetricAttributes &attributes, diff --git a/sdk/test/metrics/exemplar/reservoir_attribute_offer_test.cc b/sdk/test/metrics/exemplar/reservoir_attribute_offer_test.cc index f7dca5c255..a025b6519f 100644 --- a/sdk/test/metrics/exemplar/reservoir_attribute_offer_test.cc +++ b/sdk/test/metrics/exemplar/reservoir_attribute_offer_test.cc @@ -17,9 +17,7 @@ # include "opentelemetry/nostd/function_ref.h" # include "opentelemetry/nostd/string_view.h" # include "opentelemetry/nostd/variant.h" -# include "opentelemetry/sdk/metrics/data/exemplar_data.h" # include "opentelemetry/sdk/metrics/exemplar/fixed_size_exemplar_reservoir.h" -# include "opentelemetry/sdk/metrics/exemplar/reservoir.h" # include "opentelemetry/sdk/metrics/exemplar/reservoir_cell.h" # include "opentelemetry/sdk/metrics/exemplar/reservoir_cell_selector.h" # include "opentelemetry/version.h" @@ -94,28 +92,6 @@ class DeterministicSelector final : public ReservoirCellSelector bool accept_; }; -class OwnedOnlyReservoir final : public ExemplarReservoir -{ -public: - using ExemplarReservoir::OfferMeasurement; - - void OfferMeasurement(int64_t /* value */, - const MetricAttributes & /* attributes */, - const opentelemetry::context::Context & /* context */) noexcept override - {} - - void OfferMeasurement(double /* value */, - const MetricAttributes & /* attributes */, - const opentelemetry::context::Context & /* context */) noexcept override - {} - - std::vector> CollectAndReset( - const MetricAttributes & /* point_attributes */) noexcept override - { - return {}; - } -}; - TEST(ExemplarAttributeOffer, RejectedLongOfferDoesNotMaterializeAttributes) { auto selector = std::shared_ptr{new DeterministicSelector{false}};