diff --git a/CMakeLists.txt b/CMakeLists.txt index fa28c26e22..80b774856c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -60,6 +60,12 @@ endif() option(WITH_ABI_VERSION_1 "ABI version 1" ON) option(WITH_ABI_VERSION_2 "EXPERIMENTAL: ABI version 2 preview" OFF) +# Experimental: bound synchronous metric instruments (Counter, Histogram). +# Requires WITH_ABI_VERSION_2. Context-bearing bound operations, exemplar +# parity, Gauge, and UpDownCounter bound support are follow-ups. +option(WITH_METRICS_BOUND_INSTRUMENTS_PREVIEW + "EXPERIMENTAL: bound synchronous metric instruments preview" OFF) + option(WITH_CONFIGURATION "EXPERIMENTAL: YAML configuration file" OFF) # @@ -94,6 +100,12 @@ else() set(OPENTELEMETRY_ABI_VERSION_NO "${OPENTELEMETRY_ABI_VERSION_DEFAULT}") endif() +if(WITH_METRICS_BOUND_INSTRUMENTS_PREVIEW AND NOT WITH_ABI_VERSION_2) + message( + FATAL_ERROR + "WITH_METRICS_BOUND_INSTRUMENTS_PREVIEW requires WITH_ABI_VERSION_2") +endif() + option(WITH_NO_DEPRECATED_CODE "Do not include deprecated code" OFF) set(WITH_STL @@ -487,6 +499,10 @@ message(STATUS "WITH_API_ONLY: ${WITH_API_ONLY}") message(STATUS "WITH_NO_DEPRECATED_CODE: ${WITH_NO_DEPRECATED_CODE}") message(STATUS "WITH_ABI_VERSION_1: ${WITH_ABI_VERSION_1}") message(STATUS "WITH_ABI_VERSION_2: ${WITH_ABI_VERSION_2}") +message( + STATUS + "WITH_METRICS_BOUND_INSTRUMENTS_PREVIEW: ${WITH_METRICS_BOUND_INSTRUMENTS_PREVIEW}" +) message(STATUS "OTELCPP_VERSIONED_LIBS: ${OTELCPP_VERSIONED_LIBS}") message(STATUS "OTELCPP_MAINTAINER_MODE: ${OTELCPP_MAINTAINER_MODE}") message(STATUS "WITH_STL: ${WITH_STL}") diff --git a/api/BUILD b/api/BUILD index bfd3af6014..0dfdedaa7d 100644 --- a/api/BUILD +++ b/api/BUILD @@ -39,6 +39,14 @@ cc_library( }) + select({ ":abi_version_no_1": ["OPENTELEMETRY_ABI_VERSION_NO=1"], ":abi_version_no_2": ["OPENTELEMETRY_ABI_VERSION_NO=2"], + }) + select({ + # Experimental: bound synchronous metric instruments preview. + # NOTE: this define only takes effect when ABI v2 is also enabled + # (`--//api:abi_version_no=2`); see api/include/opentelemetry/version.h + # which gates OPENTELEMETRY_HAVE_METRICS_BOUND_INSTRUMENTS_PREVIEW on + # both ABI v2 and ENABLE_METRICS_BOUND_INSTRUMENTS_PREVIEW. + ":metrics_bound_instruments_preview_enabled": ["ENABLE_METRICS_BOUND_INSTRUMENTS_PREVIEW"], + "//conditions:default": [], }), strip_include_prefix = "include", tags = ["api"], @@ -80,3 +88,15 @@ config_setting( name = "abi_version_no_2", flag_values = {":abi_version_no": "2"}, ) + +# Experimental: bound synchronous metric instruments preview. +# Only effective with `--//api:abi_version_no=2`. +bool_flag( + name = "with_metrics_bound_instruments_preview", + build_setting_default = False, +) + +config_setting( + name = "metrics_bound_instruments_preview_enabled", + flag_values = {":with_metrics_bound_instruments_preview": "true"}, +) diff --git a/api/CMakeLists.txt b/api/CMakeLists.txt index 069957a251..7e3ffd4f7f 100644 --- a/api/CMakeLists.txt +++ b/api/CMakeLists.txt @@ -81,6 +81,11 @@ target_compile_definitions( opentelemetry_api INTERFACE OPENTELEMETRY_ABI_VERSION_NO=${OPENTELEMETRY_ABI_VERSION_NO}) +if(WITH_METRICS_BOUND_INSTRUMENTS_PREVIEW) + target_compile_definitions(opentelemetry_api + INTERFACE ENABLE_METRICS_BOUND_INSTRUMENTS_PREVIEW) +endif() + if(WITH_OTLP_RETRY_PREVIEW) target_compile_definitions(opentelemetry_api INTERFACE ENABLE_OTLP_RETRY_PREVIEW) diff --git a/api/include/opentelemetry/metrics/noop.h b/api/include/opentelemetry/metrics/noop.h index 507e809bdc..cab881b20b 100644 --- a/api/include/opentelemetry/metrics/noop.h +++ b/api/include/opentelemetry/metrics/noop.h @@ -14,6 +14,24 @@ OPENTELEMETRY_BEGIN_NAMESPACE namespace metrics { +#ifdef OPENTELEMETRY_HAVE_METRICS_BOUND_INSTRUMENTS_PREVIEW +template +class NoopBoundCounter : public BoundCounter +{ +public: + NoopBoundCounter() noexcept = default; + void Add(T /* value */) noexcept override {} +}; + +template +class NoopBoundHistogram : public BoundHistogram +{ +public: + NoopBoundHistogram() noexcept = default; + void Record(T /* value */) noexcept override {} +}; +#endif + template class NoopCounter : public Counter { @@ -29,6 +47,13 @@ class NoopCounter : public Counter const common::KeyValueIterable & /* attributes */, const context::Context & /* context */) noexcept override {} +#ifdef OPENTELEMETRY_HAVE_METRICS_BOUND_INSTRUMENTS_PREVIEW + nostd::unique_ptr> Bind( + const common::KeyValueIterable & /* attributes */) noexcept override + { + return nostd::unique_ptr>{new NoopBoundCounter()}; + } +#endif }; template @@ -51,6 +76,14 @@ class NoopHistogram : public Histogram void Record(T /*value*/) noexcept override {} #endif + +#ifdef OPENTELEMETRY_HAVE_METRICS_BOUND_INSTRUMENTS_PREVIEW + nostd::unique_ptr> Bind( + const common::KeyValueIterable & /* attributes */) noexcept override + { + return nostd::unique_ptr>{new NoopBoundHistogram()}; + } +#endif }; template diff --git a/api/include/opentelemetry/metrics/sync_instruments.h b/api/include/opentelemetry/metrics/sync_instruments.h index 4e36231abd..9e93f246a8 100644 --- a/api/include/opentelemetry/metrics/sync_instruments.h +++ b/api/include/opentelemetry/metrics/sync_instruments.h @@ -11,6 +11,10 @@ #include "opentelemetry/nostd/type_traits.h" #include "opentelemetry/version.h" +#ifdef OPENTELEMETRY_HAVE_METRICS_BOUND_INSTRUMENTS_PREVIEW +# include "opentelemetry/nostd/unique_ptr.h" +#endif + OPENTELEMETRY_BEGIN_NAMESPACE namespace metrics { @@ -26,6 +30,62 @@ class SynchronousInstrument virtual ~SynchronousInstrument() = default; }; +#ifdef OPENTELEMETRY_HAVE_METRICS_BOUND_INSTRUMENTS_PREVIEW +// Bound synchronous instrument support intentionally covers Counter and +// Histogram only. UpDownCounter, Gauge, exemplar parity, and context-bearing +// bound operations are follow-ups. This API is experimental and is gated +// behind both ABI v2 and ENABLE_METRICS_BOUND_INSTRUMENTS_PREVIEW +// (see OPENTELEMETRY_HAVE_METRICS_BOUND_INSTRUMENTS_PREVIEW in version.h). +/** + * @since ABI_VERSION 2 + * A bound counter handle obtained via Counter::Bind(...). The associated + * attribute set is captured at Bind time so the hot path avoids per-call + * attribute processing and hashmap lookup. The handle must not outlive the + * Counter instrument from which it was obtained. + */ +template +class BoundCounter +{ +public: + BoundCounter() = default; + BoundCounter(const BoundCounter &) = delete; + BoundCounter(BoundCounter &&) noexcept = delete; + BoundCounter &operator=(const BoundCounter &) = delete; + BoundCounter &operator=(BoundCounter &&) noexcept = delete; + virtual ~BoundCounter() = default; + + /** + * Record a value against the bound attribute set. + * + * @param value The increment amount. MUST be non-negative. + */ + virtual void Add(T value) noexcept = 0; +}; + +/** + * @since ABI_VERSION 2 + * A bound histogram handle obtained via Histogram::Bind(...). + */ +template +class BoundHistogram +{ +public: + BoundHistogram() = default; + BoundHistogram(const BoundHistogram &) = delete; + BoundHistogram(BoundHistogram &&) noexcept = delete; + BoundHistogram &operator=(const BoundHistogram &) = delete; + BoundHistogram &operator=(BoundHistogram &&) noexcept = delete; + virtual ~BoundHistogram() = default; + + /** + * Record a value against the bound attribute set. + * + * @param value The measurement value. MUST be non-negative. + */ + virtual void Record(T value) noexcept = 0; +}; +#endif + /* A Counter instrument that adds values. */ template class Counter : public SynchronousInstrument @@ -98,6 +158,32 @@ class Counter : public SynchronousInstrument attributes.begin(), attributes.end()}, context); } + +#ifdef OPENTELEMETRY_HAVE_METRICS_BOUND_INSTRUMENTS_PREVIEW + /** + * @since ABI_VERSION 2 + * Returns a bound counter handle for the given attribute set. Repeated calls + * to BoundCounter::Add(value) avoid per-call attribute processing and + * hashmap lookup. The bound handle MUST NOT outlive this Counter instrument. + */ + virtual nostd::unique_ptr> Bind( + const common::KeyValueIterable &attributes) noexcept = 0; + + template ::value> * = nullptr> + nostd::unique_ptr> Bind(const U &attributes) noexcept + { + return this->Bind(common::KeyValueIterableView{attributes}); + } + + nostd::unique_ptr> Bind( + std::initializer_list> + attributes) noexcept + { + return this->Bind(nostd::span>{ + attributes.begin(), attributes.end()}); + } +#endif }; /** A histogram instrument that records values. */ @@ -140,6 +226,33 @@ class Histogram : public SynchronousInstrument } #endif +#ifdef OPENTELEMETRY_HAVE_METRICS_BOUND_INSTRUMENTS_PREVIEW + /** + * @since ABI_VERSION 2 + * Returns a bound histogram handle for the given attribute set. Repeated + * calls to BoundHistogram::Record(value) avoid per-call attribute + * processing and hashmap lookup. The bound handle MUST NOT outlive this + * Histogram instrument. + */ + virtual nostd::unique_ptr> Bind( + const common::KeyValueIterable &attributes) noexcept = 0; + + template ::value> * = nullptr> + nostd::unique_ptr> Bind(const U &attributes) noexcept + { + return this->Bind(common::KeyValueIterableView{attributes}); + } + + nostd::unique_ptr> Bind( + std::initializer_list> + attributes) noexcept + { + return this->Bind(nostd::span>{ + attributes.begin(), attributes.end()}); + } +#endif + /** * Records a value. * diff --git a/api/include/opentelemetry/version.h b/api/include/opentelemetry/version.h index 694d2ee925..1b6388d98e 100644 --- a/api/include/opentelemetry/version.h +++ b/api/include/opentelemetry/version.h @@ -30,3 +30,10 @@ #define OPENTELEMETRY_NAMESPACE opentelemetry :: OPENTELEMETRY_CONCAT(v, OPENTELEMETRY_ABI_VERSION_NO) // clang-format on + +// Experimental: bound synchronous metric instruments (Counter, Histogram). +// This public API is available only in ABI v2 preview builds. Guard bound +// instrument code with OPENTELEMETRY_HAVE_METRICS_BOUND_INSTRUMENTS_PREVIEW. +#if OPENTELEMETRY_ABI_VERSION_NO >= 2 && defined(ENABLE_METRICS_BOUND_INSTRUMENTS_PREVIEW) +# define OPENTELEMETRY_HAVE_METRICS_BOUND_INSTRUMENTS_PREVIEW 1 +#endif diff --git a/sdk/include/opentelemetry/sdk/metrics/state/metric_storage.h b/sdk/include/opentelemetry/sdk/metrics/state/metric_storage.h index af743bea9f..bbcbf55d22 100644 --- a/sdk/include/opentelemetry/sdk/metrics/state/metric_storage.h +++ b/sdk/include/opentelemetry/sdk/metrics/state/metric_storage.h @@ -29,6 +29,28 @@ namespace metrics /* Represent the storage from which to collect the metrics */ class CollectorHandle; +#ifdef OPENTELEMETRY_HAVE_METRICS_BOUND_INSTRUMENTS_PREVIEW +/** + * @since ABI_VERSION 2 + * Storage-side interface for a bound sync metric handle. Created by + * SyncWritableMetricStorage::Bind(...). The hot path RecordLong/RecordDouble + * skips per-call attribute filtering and hashmap lookup. + */ +class BoundSyncWritableMetricStorage +{ +public: + BoundSyncWritableMetricStorage() = default; + BoundSyncWritableMetricStorage(const BoundSyncWritableMetricStorage &) = delete; + BoundSyncWritableMetricStorage(BoundSyncWritableMetricStorage &&) = delete; + BoundSyncWritableMetricStorage &operator=(const BoundSyncWritableMetricStorage &) = delete; + BoundSyncWritableMetricStorage &operator=(BoundSyncWritableMetricStorage &&) = delete; + virtual ~BoundSyncWritableMetricStorage() = default; + + virtual void RecordLong(int64_t value) noexcept = 0; + virtual void RecordDouble(double value) noexcept = 0; +}; +#endif + class MetricStorage { public: @@ -75,6 +97,19 @@ class SyncWritableMetricStorage virtual void RecordDouble(double value, const opentelemetry::common::KeyValueIterable &attributes, const opentelemetry::context::Context &context) noexcept = 0; + +#ifdef OPENTELEMETRY_HAVE_METRICS_BOUND_INSTRUMENTS_PREVIEW + /** + * @since ABI_VERSION 2 + * Returns a bound storage handle for the given attribute set, or nullptr if + * the storage does not support binding. Default returns nullptr. + */ + virtual std::shared_ptr Bind( + const opentelemetry::common::KeyValueIterable & /* attributes */) noexcept + { + return nullptr; + } +#endif }; /* Represents the async metric stroage */ diff --git a/sdk/include/opentelemetry/sdk/metrics/state/multi_metric_storage.h b/sdk/include/opentelemetry/sdk/metrics/state/multi_metric_storage.h index 38ebb52fe8..074d25edcc 100644 --- a/sdk/include/opentelemetry/sdk/metrics/state/multi_metric_storage.h +++ b/sdk/include/opentelemetry/sdk/metrics/state/multi_metric_storage.h @@ -16,6 +16,10 @@ #include "opentelemetry/sdk/metrics/view/attributes_processor.h" #include "opentelemetry/version.h" +#ifdef OPENTELEMETRY_HAVE_METRICS_BOUND_INSTRUMENTS_PREVIEW +# include +#endif + OPENTELEMETRY_BEGIN_NAMESPACE namespace sdk { @@ -66,6 +70,11 @@ class SyncMultiMetricStorage : public SyncWritableMetricStorage } } +#ifdef OPENTELEMETRY_HAVE_METRICS_BOUND_INSTRUMENTS_PREVIEW + std::shared_ptr Bind( + const opentelemetry::common::KeyValueIterable &attributes) noexcept override; +#endif + private: std::vector> storages_; }; @@ -102,6 +111,61 @@ class AsyncMultiMetricStorage : public AsyncWritableMetricStorage std::vector> storages_; }; +#ifdef OPENTELEMETRY_HAVE_METRICS_BOUND_INSTRUMENTS_PREVIEW +// Bound handle that fans out to per-view children that support binding. +// Children whose Bind() returns nullptr (no-op storages) are skipped, matching +// the behavior of their unbound RecordLong/RecordDouble. +class MultiBoundEntry : public BoundSyncWritableMetricStorage +{ +public: + explicit MultiBoundEntry( + std::vector> children) noexcept + : children_(std::move(children)) + {} + + void RecordLong(int64_t value) noexcept override + { + for (auto &c : children_) + { + c->RecordLong(value); + } + } + + void RecordDouble(double value) noexcept override + { + for (auto &c : children_) + { + c->RecordDouble(value); + } + } + +private: + std::vector> children_; +}; + +inline std::shared_ptr SyncMultiMetricStorage::Bind( + const opentelemetry::common::KeyValueIterable &attributes) noexcept +{ + std::vector> children; + children.reserve(storages_.size()); + for (auto &s : storages_) + { + auto child = s->Bind(attributes); + if (child) + { + children.push_back(std::move(child)); + } + } + // If no child supports binding, return nullptr. The instrument layer will + // return a no-op bound handle, matching no-op storage behavior. + if (children.empty()) + { + return nullptr; + } + return std::make_shared(std::move(children)); +} +#endif + } // namespace metrics } // namespace sdk OPENTELEMETRY_END_NAMESPACE 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 0961333add..c9f46a2c72 100644 --- a/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h +++ b/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h @@ -8,6 +8,7 @@ #include #include #include +#include #include "opentelemetry/common/key_value_iterable.h" #include "opentelemetry/common/spin_lock_mutex.h" @@ -30,6 +31,10 @@ #include "opentelemetry/sdk/metrics/view/attributes_processor.h" #include "opentelemetry/version.h" +#ifdef OPENTELEMETRY_HAVE_METRICS_BOUND_INSTRUMENTS_PREVIEW +# include +#endif + #ifdef ENABLE_METRICS_EXEMPLAR_PREVIEW # include "opentelemetry/sdk/metrics/exemplar/filter_type.h" # include "opentelemetry/sdk/metrics/exemplar/reservoir.h" @@ -99,7 +104,13 @@ class SyncMetricStorage : public MetricStorage, public SyncWritableMetricStorage #endif static MetricAttributes attr = MetricAttributes{}; std::lock_guard guard(attribute_hashmap_lock_); +#ifdef OPENTELEMETRY_HAVE_METRICS_BOUND_INSTRUMENTS_PREVIEW + MetricAttributes resolved = ResolveCardinality(attr); + attributes_hashmap_->GetOrSetDefault(std::move(resolved), create_default_aggregation_) + ->Aggregate(value); +#else attributes_hashmap_->GetOrSetDefault(attr, create_default_aggregation_)->Aggregate(value); +#endif } void RecordLong(int64_t value, @@ -121,9 +132,18 @@ class SyncMetricStorage : public MetricStorage, public SyncWritableMetricStorage MetricAttributes attr{attributes, attributes_processor_.get()}; std::lock_guard guard(attribute_hashmap_lock_); +#ifdef OPENTELEMETRY_HAVE_METRICS_BOUND_INSTRUMENTS_PREVIEW + // Resolve via the unified cardinality policy so unbound and bound paths + // share one combined limit (see ResolveCardinality()). + MetricAttributes resolved = ResolveCardinality(std::move(attr)); + // cppcheck-suppress accessMoved + attributes_hashmap_->GetOrSetDefault(std::move(resolved), create_default_aggregation_) + ->Aggregate(value); +#else // cppcheck-suppress accessMoved attributes_hashmap_->GetOrSetDefault(std::move(attr), create_default_aggregation_) ->Aggregate(value); +#endif } void RecordDouble(double value, @@ -142,7 +162,13 @@ class SyncMetricStorage : public MetricStorage, public SyncWritableMetricStorage #endif static MetricAttributes attr = MetricAttributes{}; std::lock_guard guard(attribute_hashmap_lock_); +#ifdef OPENTELEMETRY_HAVE_METRICS_BOUND_INSTRUMENTS_PREVIEW + MetricAttributes resolved = ResolveCardinality(attr); + attributes_hashmap_->GetOrSetDefault(std::move(resolved), create_default_aggregation_) + ->Aggregate(value); +#else attributes_hashmap_->GetOrSetDefault(attr, create_default_aggregation_)->Aggregate(value); +#endif } void RecordDouble(double value, @@ -163,9 +189,16 @@ class SyncMetricStorage : public MetricStorage, public SyncWritableMetricStorage #endif MetricAttributes attr{attributes, attributes_processor_.get()}; std::lock_guard guard(attribute_hashmap_lock_); +#ifdef OPENTELEMETRY_HAVE_METRICS_BOUND_INSTRUMENTS_PREVIEW + MetricAttributes resolved = ResolveCardinality(std::move(attr)); + // cppcheck-suppress accessMoved + attributes_hashmap_->GetOrSetDefault(std::move(resolved), create_default_aggregation_) + ->Aggregate(value); +#else // cppcheck-suppress accessMoved attributes_hashmap_->GetOrSetDefault(std::move(attr), create_default_aggregation_) ->Aggregate(value); +#endif } bool Collect(CollectorHandle *collector, @@ -174,7 +207,91 @@ class SyncMetricStorage : public MetricStorage, public SyncWritableMetricStorage opentelemetry::common::SystemTimestamp collection_ts, nostd::function_ref callback) noexcept override; +#ifdef OPENTELEMETRY_HAVE_METRICS_BOUND_INSTRUMENTS_PREVIEW + std::shared_ptr Bind( + const opentelemetry::common::KeyValueIterable &attributes) noexcept override; + + // Internal: stable bound entry. Self-contained: owns its own spinlock and + // aggregation so the user-held handle stays safe to call even if the parent + // SyncMetricStorage is destroyed first (writes simply have no observer). + // Collect() rotates current_ when dirty so bound + unbound writes for the + // same post-filter attribute set merge into one delta datapoint via the + // existing TemporalMetricStorage pipeline. + // + // Exemplar note: the bound fast path has no per-call Context, so it does not + // offer measurements to the exemplar reservoir. Callers that need exemplars + // should use the unbound RecordLong/RecordDouble path. + class BoundEntry : public BoundSyncWritableMetricStorage + { + public: + BoundEntry(InstrumentValueType value_type, + MetricAttributes attributes, + std::unique_ptr initial_aggregation) noexcept + : value_type_(value_type), + attributes_(std::move(attributes)), + current_(std::move(initial_aggregation)), + dirty_(false) + {} + + void RecordLong(int64_t value) noexcept override; + void RecordDouble(double value) noexcept override; + + private: + friend class SyncMetricStorage; + InstrumentValueType value_type_; + MetricAttributes attributes_; + // Protected by lock_. + opentelemetry::common::SpinLockMutex lock_; + std::unique_ptr current_; + bool dirty_; + }; +#endif + private: +#ifdef OPENTELEMETRY_HAVE_METRICS_BOUND_INSTRUMENTS_PREVIEW + // Unified cardinality resolver. Returns either `filtered` unchanged or + // kOverflowAttributes. Existing keys (already present in active_keys_) pass + // through unchanged so we never retroactively reroute admitted keys to + // overflow. New keys are admitted or routed to overflow using the same + // two-branch logic as AttributesHashMap::IsOverflowAttributes() so the + // bound and unbound streams share one coherent O(1) cardinality limit + // without scanning the hashmap. + // + // active_keys_ is the union of: + // - unbound attribute keys admitted in the current collection interval, and + // - retained bound entry keys. + // It is reset to bound entry keys at every Collect(). + // + // Must be called with attribute_hashmap_lock_ held. + MetricAttributes ResolveCardinality(const MetricAttributes &filtered) noexcept + { + if (filtered == kOverflowAttributes) + { + active_keys_.insert(kOverflowAttributes); + return kOverflowAttributes; + } + if (active_keys_.find(filtered) != active_keys_.end()) + { + return filtered; + } + const size_t limit = aggregation_config_->cardinality_limit_; + const bool has_overflow = active_keys_.find(kOverflowAttributes) != active_keys_.end(); + // Mirror AttributesHashMap::IsOverflowAttributes() exactly. When the + // overflow slot is already counted in active_keys_, only route to + // overflow when total active keys reach the limit. Otherwise simulate + // adding the overflow slot too, matching the (size + 1) >= limit branch. + const bool would_overflow = + has_overflow ? (active_keys_.size() >= limit) : (active_keys_.size() + 1 >= limit); + if (would_overflow) + { + active_keys_.insert(kOverflowAttributes); + return kOverflowAttributes; + } + active_keys_.insert(filtered); + return filtered; + } +#endif + InstrumentDescriptor instrument_descriptor_; // hashmap to maintain the metrics for delta collection (i.e, collection since last Collect call) const AggregationConfig *aggregation_config_; @@ -187,6 +304,25 @@ class SyncMetricStorage : public MetricStorage, public SyncWritableMetricStorage #endif TemporalMetricStorage temporal_metric_storage_; opentelemetry::common::SpinLockMutex attribute_hashmap_lock_; +#ifdef OPENTELEMETRY_HAVE_METRICS_BOUND_INSTRUMENTS_PREVIEW + // NOTE: ENABLE_METRICS_BOUND_INSTRUMENTS_PREVIEW changes the layout and + // vtable of SyncMetricStorage (these conditional members and the virtual + // Bind() method on SyncWritableMetricStorage). It MUST be defined + // consistently across the SDK library build and every consumer translation + // unit, otherwise ODR violations and ABI mismatches will result. + // Bound entries deduped by post-filter attribute set. Lifetime of entries is + // tied to user-held shared_ptrs returned by Bind() plus this storage. The + // storage retains a shared_ptr so collect-time rotation always finds them. + std::unordered_map, AttributeHashGenerator> + bound_entries_; + // Active union of admitted unbound + bound attribute keys for O(1) + // cardinality decisions. Intentionally duplicates keys also stored in + // attributes_hashmap_ and bound_entries_ so ResolveCardinality() avoids + // scanning either container. Guarded by attribute_hashmap_lock_. Reset to + // bound entry keys at every Collect(), mirroring the per-interval reset of + // attributes_hashmap_ while retaining bound-entry cardinality cost. + std::unordered_set active_keys_; +#endif }; } // namespace metrics diff --git a/sdk/include/opentelemetry/sdk/metrics/sync_instruments.h b/sdk/include/opentelemetry/sdk/metrics/sync_instruments.h index 74ef0f6b31..315accfc41 100644 --- a/sdk/include/opentelemetry/sdk/metrics/sync_instruments.h +++ b/sdk/include/opentelemetry/sdk/metrics/sync_instruments.h @@ -49,6 +49,11 @@ class LongCounter : public Synchronous, public opentelemetry::metrics::Counter> Bind( + const opentelemetry::common::KeyValueIterable &attributes) noexcept override; +#endif }; class DoubleCounter : public Synchronous, public opentelemetry::metrics::Counter @@ -66,6 +71,11 @@ class DoubleCounter : public Synchronous, public opentelemetry::metrics::Counter void Add(double value) noexcept override; void Add(double value, const opentelemetry::context::Context &context) noexcept override; + +#ifdef OPENTELEMETRY_HAVE_METRICS_BOUND_INSTRUMENTS_PREVIEW + opentelemetry::nostd::unique_ptr> Bind( + const opentelemetry::common::KeyValueIterable &attributes) noexcept override; +#endif }; class LongUpDownCounter : public Synchronous, public opentelemetry::metrics::UpDownCounter @@ -147,6 +157,11 @@ class LongHistogram : public Synchronous, public opentelemetry::metrics::Histogr void Record(uint64_t value) noexcept override; #endif +#ifdef OPENTELEMETRY_HAVE_METRICS_BOUND_INSTRUMENTS_PREVIEW + opentelemetry::nostd::unique_ptr> Bind( + const opentelemetry::common::KeyValueIterable &attributes) noexcept override; +#endif + void Record(uint64_t value, const opentelemetry::common::KeyValueIterable &attributes, const opentelemetry::context::Context &context) noexcept override; @@ -167,6 +182,11 @@ class DoubleHistogram : public Synchronous, public opentelemetry::metrics::Histo void Record(double value) noexcept override; #endif +#ifdef OPENTELEMETRY_HAVE_METRICS_BOUND_INSTRUMENTS_PREVIEW + opentelemetry::nostd::unique_ptr> Bind( + const opentelemetry::common::KeyValueIterable &attributes) noexcept override; +#endif + void Record(double value, const opentelemetry::common::KeyValueIterable &attributes, const opentelemetry::context::Context &context) noexcept override; diff --git a/sdk/src/metrics/state/sync_metric_storage.cc b/sdk/src/metrics/state/sync_metric_storage.cc index 3844d2b05d..01b049f70b 100644 --- a/sdk/src/metrics/state/sync_metric_storage.cc +++ b/sdk/src/metrics/state/sync_metric_storage.cc @@ -1,22 +1,36 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 +#include +#include #include #include +#include +#include #include +#include "opentelemetry/common/key_value_iterable.h" #include "opentelemetry/common/spin_lock_mutex.h" #include "opentelemetry/common/timestamp.h" #include "opentelemetry/nostd/function_ref.h" #include "opentelemetry/nostd/span.h" +#include "opentelemetry/sdk/metrics/aggregation/aggregation.h" #include "opentelemetry/sdk/metrics/aggregation/aggregation_config.h" +#include "opentelemetry/sdk/metrics/data/exemplar_data.h" #include "opentelemetry/sdk/metrics/data/metric_data.h" +#include "opentelemetry/sdk/metrics/instruments.h" #include "opentelemetry/sdk/metrics/state/attributes_hashmap.h" +#include "opentelemetry/sdk/metrics/state/filtered_ordered_attribute_map.h" #include "opentelemetry/sdk/metrics/state/metric_collector.h" +#include "opentelemetry/sdk/metrics/state/metric_storage.h" #include "opentelemetry/sdk/metrics/state/sync_metric_storage.h" #include "opentelemetry/sdk/metrics/state/temporal_metric_storage.h" #include "opentelemetry/version.h" +#ifdef OPENTELEMETRY_HAVE_METRICS_BOUND_INSTRUMENTS_PREVIEW +# include +#endif + OPENTELEMETRY_BEGIN_NAMESPACE namespace sdk { @@ -33,16 +47,203 @@ bool SyncMetricStorage::Collect(CollectorHandle *collector, // this will also empty the delta metrics hashmap, and make it available for // recordings std::shared_ptr delta_metrics = nullptr; +#ifdef OPENTELEMETRY_HAVE_METRICS_BOUND_INSTRUMENTS_PREVIEW + // Snapshot of bound entries (under map lock) that we will rotate without + // holding the map lock. Each entry has its own spinlock for the swap. + std::vector> entry_snapshot; +#endif { std::lock_guard guard(attribute_hashmap_lock_); delta_metrics = std::move(attributes_hashmap_); attributes_hashmap_.reset(new AttributesHashMap(aggregation_config_->cardinality_limit_)); +#ifdef OPENTELEMETRY_HAVE_METRICS_BOUND_INSTRUMENTS_PREVIEW + // Garbage-collect entries the user has dropped that have no pending data. + // Cleanup happens during Collect(); if no collection runs, dropped bound + // entries remain until storage destruction. We only erase entries the user + // has released (use_count == 1). `dirty_` must be read under the entry's + // own lock_ because BoundEntry::RecordLong/RecordDouble and collect-time + // rotation mutate `dirty_` only while holding that lock. + for (auto it = bound_entries_.begin(); it != bound_entries_.end();) + { + bool can_erase = false; + if (it->second.use_count() == 1) + { + std::lock_guard g(it->second->lock_); + can_erase = !it->second->dirty_; + } + if (can_erase) + { + it = bound_entries_.erase(it); + } + else + { + ++it; + } + } + // Rebuild the active key set after GC so dropped bound keys no longer + // count toward cardinality. attributes_hashmap_ has just been reset, so + // only retained bound entry keys should remain. + active_keys_.clear(); + for (auto &kv : bound_entries_) + { + active_keys_.insert(kv.first); + } + entry_snapshot.reserve(bound_entries_.size()); + for (auto &kv : bound_entries_) + { + entry_snapshot.push_back(kv.second); + } +#endif + } + +#ifdef OPENTELEMETRY_HAVE_METRICS_BOUND_INSTRUMENTS_PREVIEW + // Rotate dirty bound entries: under each entry's own spinlock, swap out the + // current aggregation and merge it into delta_metrics so bound + unbound + // writes for the same post-filter attribute set produce one datapoint. + for (auto &entry : entry_snapshot) + { + std::unique_ptr rotated; + MetricAttributes attrs_copy; + { + std::lock_guard g(entry->lock_); + if (!entry->dirty_) + { + continue; + } + rotated = std::move(entry->current_); + entry->current_ = create_default_aggregation_(); + entry->dirty_ = false; + attrs_copy = entry->attributes_; + } + auto *existing = delta_metrics->Get(attrs_copy); + if (existing) + { + delta_metrics->Set(attrs_copy, existing->Merge(*rotated)); + } + else + { + delta_metrics->Set(std::move(attrs_copy), std::move(rotated)); + } + } + + // Targeted post-rotation cleanup. The pre-rotation GC pass cannot remove + // entries that were dropped by the user but still had pending data, since + // they were dirty at that point. After rotation cleared their dirty_ flag, + // walk the snapshot once more and erase any entry the user has now fully + // released (use_count == 1, owned only by bound_entries_) and that has not + // been redirtied. The map lock is briefly released between snapshot and + // rotation, so concurrent unbound writes may have legitimately added the + // same key into attributes_hashmap_; in that case we must NOT remove the + // key from active_keys_ or we would drop their cardinality slot. + std::vector snapshot_keys; + snapshot_keys.reserve(entry_snapshot.size()); + for (auto &entry : entry_snapshot) + { + snapshot_keys.push_back(entry->attributes_); + } + // Drop snapshot refs so use_count reflects only storage + user holders. + entry_snapshot.clear(); + { + std::lock_guard guard(attribute_hashmap_lock_); + for (const auto &k : snapshot_keys) + { + auto it = bound_entries_.find(k); + if (it == bound_entries_.end()) + { + continue; + } + // User still holds the handle (or rebound during rotation): keep. + if (it->second.use_count() != 1) + { + continue; + } + // use_count == 1 means no other thread can be calling RecordLong/Double + // on this entry, so reading dirty_ is race-free here. Lock anyway to + // satisfy the documented invariant on dirty_. + bool entry_dirty; + { + std::lock_guard g(it->second->lock_); + entry_dirty = it->second->dirty_; + } + if (entry_dirty) + { + continue; + } + // Only remove from active_keys_ if no concurrent unbound write has + // claimed the same key in this interval. + if (!attributes_hashmap_->Has(k)) + { + active_keys_.erase(k); + } + bound_entries_.erase(it); + } } +#endif return temporal_metric_storage_.buildMetrics(collector, collectors, sdk_start_ts, collection_ts, delta_metrics, callback); } +#ifdef OPENTELEMETRY_HAVE_METRICS_BOUND_INSTRUMENTS_PREVIEW +std::shared_ptr SyncMetricStorage::Bind( + const opentelemetry::common::KeyValueIterable &attributes) noexcept +{ + // Filter attributes once, at bind time. + MetricAttributes filtered{attributes, attributes_processor_.get()}; + + std::lock_guard guard(attribute_hashmap_lock_); + + // Dedupe: same post-filter attribute set returns the same bound entry. + auto it = bound_entries_.find(filtered); + if (it != bound_entries_.end()) + { + return it->second; + } + + // Apply the unified cardinality policy. ResolveCardinality returns either + // `filtered` (already-admitted or capacity available) or kOverflowAttributes, + // updating active_keys_ as needed. + MetricAttributes key = ResolveCardinality(filtered); + + // If we ended up at overflow, dedupe against an existing overflow entry. + if (key == kOverflowAttributes) + { + auto ov_it = bound_entries_.find(key); + if (ov_it != bound_entries_.end()) + { + return ov_it->second; + } + } + + auto entry = std::make_shared(instrument_descriptor_.value_type_, key, + create_default_aggregation_()); + bound_entries_.emplace(std::move(key), entry); + return entry; +} + +void SyncMetricStorage::BoundEntry::RecordLong(int64_t value) noexcept +{ + if (value_type_ != InstrumentValueType::kLong) + { + return; + } + std::lock_guard guard(lock_); + current_->Aggregate(value); + dirty_ = true; +} + +void SyncMetricStorage::BoundEntry::RecordDouble(double value) noexcept +{ + if (value_type_ != InstrumentValueType::kDouble) + { + return; + } + std::lock_guard guard(lock_); + current_->Aggregate(value); + dirty_ = true; +} +#endif + } // namespace metrics } // namespace sdk OPENTELEMETRY_END_NAMESPACE diff --git a/sdk/src/metrics/sync_instruments.cc b/sdk/src/metrics/sync_instruments.cc index ab40ad7336..2aee8672af 100644 --- a/sdk/src/metrics/sync_instruments.cc +++ b/sdk/src/metrics/sync_instruments.cc @@ -2,13 +2,14 @@ // SPDX-License-Identifier: Apache-2.0 #include -#include #include #include #include #include "opentelemetry/common/key_value_iterable.h" #include "opentelemetry/context/context.h" +#include "opentelemetry/metrics/sync_instruments.h" +#include "opentelemetry/nostd/unique_ptr.h" #include "opentelemetry/sdk/common/global_log_handler.h" #include "opentelemetry/sdk/metrics/instruments.h" #include "opentelemetry/sdk/metrics/state/metric_storage.h" @@ -569,6 +570,145 @@ void DoubleHistogram::Record(double value) noexcept } #endif +#ifdef OPENTELEMETRY_HAVE_METRICS_BOUND_INSTRUMENTS_PREVIEW +namespace +{ + +class BoundLongCounterImpl : public opentelemetry::metrics::BoundCounter +{ +public: + explicit BoundLongCounterImpl(std::shared_ptr storage) noexcept + : storage_(std::move(storage)) + {} + void Add(uint64_t value) noexcept override + { + if (storage_) + { + storage_->RecordLong(static_cast(value)); + } + } + +private: + std::shared_ptr storage_; +}; + +class BoundDoubleCounterImpl : public opentelemetry::metrics::BoundCounter +{ +public: + explicit BoundDoubleCounterImpl(std::shared_ptr storage) noexcept + : storage_(std::move(storage)) + {} + void Add(double value) noexcept override + { + if (value < 0) + { + OTEL_INTERNAL_LOG_WARN("[BoundDoubleCounter::Add(V)] Value not recorded - negative value"); + return; + } + if (storage_) + { + storage_->RecordDouble(value); + } + } + +private: + std::shared_ptr storage_; +}; + +class BoundLongHistogramImpl : public opentelemetry::metrics::BoundHistogram +{ +public: + explicit BoundLongHistogramImpl(std::shared_ptr storage) noexcept + : storage_(std::move(storage)) + {} + void Record(uint64_t value) noexcept override + { + if (storage_) + { + storage_->RecordLong(static_cast(value)); + } + } + +private: + std::shared_ptr storage_; +}; + +class BoundDoubleHistogramImpl : public opentelemetry::metrics::BoundHistogram +{ +public: + explicit BoundDoubleHistogramImpl( + std::shared_ptr storage) noexcept + : storage_(std::move(storage)) + {} + void Record(double value) noexcept override + { + if (value < 0) + { + OTEL_INTERNAL_LOG_WARN( + "[BoundDoubleHistogram::Record(V)] Value not recorded - negative value"); + return; + } + if (storage_) + { + storage_->RecordDouble(value); + } + } + +private: + std::shared_ptr storage_; +}; + +} // namespace + +opentelemetry::nostd::unique_ptr> LongCounter::Bind( + const opentelemetry::common::KeyValueIterable &attributes) noexcept +{ + std::shared_ptr bound; + if (storage_) + { + bound = storage_->Bind(attributes); + } + return opentelemetry::nostd::unique_ptr>{ + new BoundLongCounterImpl(std::move(bound))}; +} + +opentelemetry::nostd::unique_ptr> DoubleCounter::Bind( + const opentelemetry::common::KeyValueIterable &attributes) noexcept +{ + std::shared_ptr bound; + if (storage_) + { + bound = storage_->Bind(attributes); + } + return opentelemetry::nostd::unique_ptr>{ + new BoundDoubleCounterImpl(std::move(bound))}; +} + +opentelemetry::nostd::unique_ptr> +LongHistogram::Bind(const opentelemetry::common::KeyValueIterable &attributes) noexcept +{ + std::shared_ptr bound; + if (storage_) + { + bound = storage_->Bind(attributes); + } + return opentelemetry::nostd::unique_ptr>{ + new BoundLongHistogramImpl(std::move(bound))}; +} + +opentelemetry::nostd::unique_ptr> +DoubleHistogram::Bind(const opentelemetry::common::KeyValueIterable &attributes) noexcept +{ + std::shared_ptr bound; + if (storage_) + { + bound = storage_->Bind(attributes); + } + return opentelemetry::nostd::unique_ptr>{ + new BoundDoubleHistogramImpl(std::move(bound))}; +} +#endif + } // namespace metrics } // namespace sdk OPENTELEMETRY_END_NAMESPACE diff --git a/sdk/test/metrics/CMakeLists.txt b/sdk/test/metrics/CMakeLists.txt index 63e6e58d31..a3115e1269 100644 --- a/sdk/test/metrics/CMakeLists.txt +++ b/sdk/test/metrics/CMakeLists.txt @@ -57,6 +57,19 @@ if(OPENTELEMETRY_ABI_VERSION_NO GREATER_EQUAL 2) TARGET multi_observer_test TEST_PREFIX metrics. TEST_LIST multi_observer_test) + + if(WITH_METRICS_BOUND_INSTRUMENTS_PREVIEW) + add_executable(bound_sync_instruments_test bound_sync_instruments_test.cc) + target_link_libraries( + bound_sync_instruments_test ${GTEST_BOTH_LIBRARIES} ${GMOCK_LIB} + ${CMAKE_THREAD_LIBS_INIT} metrics_common_test_utils + opentelemetry_resources) + target_compile_definitions(bound_sync_instruments_test PRIVATE UNIT_TESTING) + gtest_add_tests( + TARGET bound_sync_instruments_test + TEST_PREFIX metrics. + TEST_LIST bound_sync_instruments_test) + endif() endif() if(WITH_BENCHMARK) diff --git a/sdk/test/metrics/bound_sync_instruments_test.cc b/sdk/test/metrics/bound_sync_instruments_test.cc new file mode 100644 index 0000000000..5d5626d515 --- /dev/null +++ b/sdk/test/metrics/bound_sync_instruments_test.cc @@ -0,0 +1,908 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +#include "opentelemetry/version.h" + +#ifdef OPENTELEMETRY_HAVE_METRICS_BOUND_INSTRUMENTS_PREVIEW + +# include +# include +# include +# include +# include +# include +# include +# include +# include + +# include "common.h" + +# include "opentelemetry/common/key_value_iterable_view.h" +# include "opentelemetry/common/timestamp.h" +# include "opentelemetry/context/context.h" +# include "opentelemetry/metrics/noop.h" +# include "opentelemetry/metrics/sync_instruments.h" +# include "opentelemetry/nostd/function_ref.h" +# include "opentelemetry/nostd/span.h" +# include "opentelemetry/nostd/string_view.h" +# include "opentelemetry/nostd/variant.h" +# include "opentelemetry/sdk/metrics/aggregation/aggregation_config.h" +# include "opentelemetry/sdk/metrics/data/metric_data.h" +# include "opentelemetry/sdk/metrics/data/point_data.h" +# include "opentelemetry/sdk/metrics/instruments.h" +# include "opentelemetry/sdk/metrics/state/metric_collector.h" +# include "opentelemetry/sdk/metrics/state/metric_storage.h" +# include "opentelemetry/sdk/metrics/state/multi_metric_storage.h" +# include "opentelemetry/sdk/metrics/state/sync_metric_storage.h" +# include "opentelemetry/sdk/metrics/view/attributes_processor.h" + +# ifdef ENABLE_METRICS_EXEMPLAR_PREVIEW +# include "opentelemetry/sdk/metrics/exemplar/filter_type.h" +# include "opentelemetry/sdk/metrics/exemplar/reservoir.h" +# endif + +using namespace opentelemetry::sdk::metrics; +using namespace opentelemetry::common; +using M = std::map; + +namespace +{ +// Owns AggregationConfig + processor so they outlive the storage. +class StorageHolder +{ +public: + StorageHolder(InstrumentType type = InstrumentType::kCounter, + InstrumentValueType vtype = InstrumentValueType::kLong, + size_t cardinality_limit = 2000) + : proc_(new DefaultAttributesProcessor{}) + { + InstrumentDescriptor desc{"name", "desc", "1unit", type, vtype}; + AggregationType agg_type = + (type == InstrumentType::kHistogram) ? AggregationType::kHistogram : AggregationType::kSum; + if (type == InstrumentType::kHistogram) + { + hist_cfg_ = std::unique_ptr( + new HistogramAggregationConfig(cardinality_limit)); + cfg_ = hist_cfg_.get(); + } + else + { + sum_cfg_ = std::unique_ptr(new AggregationConfig(cardinality_limit)); + cfg_ = sum_cfg_.get(); + } + storage_ = std::make_shared(desc, agg_type, proc_, +# ifdef ENABLE_METRICS_EXEMPLAR_PREVIEW + ExemplarFilterType::kAlwaysOff, + ExemplarReservoir::GetNoExemplarReservoir(), +# endif + cfg_); + } + + SyncMetricStorage &operator*() noexcept { return *storage_; } + SyncMetricStorage *operator->() noexcept { return storage_.get(); } + std::shared_ptr share() const noexcept { return storage_; } + +private: + std::unique_ptr sum_cfg_; + std::unique_ptr hist_cfg_; + const AggregationConfig *cfg_ = nullptr; + std::shared_ptr proc_; + std::shared_ptr storage_; +}; + +int64_t SumLongFor(SyncMetricStorage &storage, + AggregationTemporality temporality, + const std::map &filter) +{ + std::shared_ptr collector(new MockCollectorHandle(temporality)); + std::vector> collectors{collector}; + int64_t total = 0; + storage.Collect(collector.get(), collectors, std::chrono::system_clock::now(), + std::chrono::system_clock::now(), [&](const MetricData &md) { + for (const auto &p : md.point_data_attr_) + { + bool match = true; + for (const auto &kv : filter) + { + auto it = p.attributes.find(kv.first); + if (it == p.attributes.end() || + opentelemetry::nostd::get(it->second) != kv.second) + { + match = false; + break; + } + } + if (match) + { + const auto &sp = opentelemetry::nostd::get(p.point_data); + total += opentelemetry::nostd::get(sp.value_); + } + } + return true; + }); + return total; +} + +size_t CollectAndCountPoints(SyncMetricStorage &storage, AggregationTemporality temporality) +{ + std::shared_ptr collector(new MockCollectorHandle(temporality)); + std::vector> collectors{collector}; + size_t count = 0; + storage.Collect(collector.get(), collectors, std::chrono::system_clock::now(), + std::chrono::system_clock::now(), [&](const MetricData &md) { + count += md.point_data_attr_.size(); + return true; + }); + return count; +} + +bool HasOverflowPoint(SyncMetricStorage &storage, AggregationTemporality temporality) +{ + std::shared_ptr collector(new MockCollectorHandle(temporality)); + std::vector> collectors{collector}; + bool found = false; + storage.Collect(collector.get(), collectors, std::chrono::system_clock::now(), + std::chrono::system_clock::now(), [&](const MetricData &md) { + for (const auto &p : md.point_data_attr_) + { + if (p.attributes.find("otel.metrics.overflow") != p.attributes.end()) + { + found = true; + } + } + return true; + }); + return found; +} +} // namespace + +// 1) Bound counter records and exports same datapoint as unbound counter with same attributes. +TEST(BoundSyncInstruments, BoundCounterMatchesUnbound) +{ + StorageHolder holder(InstrumentType::kCounter, InstrumentValueType::kLong); + M attrs = {{"key", "v"}}; + KeyValueIterableView kv(attrs); + + holder->RecordLong(7, kv, opentelemetry::context::Context{}); + auto bound = holder->Bind(kv); + ASSERT_NE(bound, nullptr); + bound->RecordLong(3); + + EXPECT_EQ(SumLongFor(*holder, AggregationTemporality::kDelta, attrs), 10); + EXPECT_EQ(CollectAndCountPoints(*holder, AggregationTemporality::kDelta), 0u); +} + +// 2) Bound histogram records and exports same datapoint as unbound. +TEST(BoundSyncInstruments, BoundHistogramMatchesUnbound) +{ + StorageHolder holder(InstrumentType::kHistogram, InstrumentValueType::kLong); + M attrs = {{"k", "v"}}; + KeyValueIterableView kv(attrs); + + holder->RecordLong(5, kv, opentelemetry::context::Context{}); + auto bound = holder->Bind(kv); + ASSERT_NE(bound, nullptr); + bound->RecordLong(15); + + std::shared_ptr collector( + new MockCollectorHandle(AggregationTemporality::kDelta)); + std::vector> collectors{collector}; + bool seen = false; + holder->Collect(collector.get(), collectors, std::chrono::system_clock::now(), + std::chrono::system_clock::now(), [&](const MetricData &md) { + for (const auto &p : md.point_data_attr_) + { + const auto &h = opentelemetry::nostd::get(p.point_data); + EXPECT_EQ(h.count_, 2u); + EXPECT_EQ(opentelemetry::nostd::get(h.sum_), 20); + seen = true; + } + return true; + }); + EXPECT_TRUE(seen); +} + +// Bound counter must respect non-default aggregation overrides. +TEST(BoundSyncInstruments, BoundCounterRespectsDropAggregation) +{ + InstrumentDescriptor desc{"name", "desc", "1unit", InstrumentType::kCounter, + InstrumentValueType::kLong}; + std::shared_ptr proc(new DefaultAttributesProcessor{}); + AggregationConfig cfg; + SyncMetricStorage storage(desc, AggregationType::kDrop, proc, +# ifdef ENABLE_METRICS_EXEMPLAR_PREVIEW + ExemplarFilterType::kAlwaysOff, + ExemplarReservoir::GetNoExemplarReservoir(), +# endif + &cfg); + M attrs = {{"k", "v"}}; + auto bound = storage.Bind(KeyValueIterableView(attrs)); + ASSERT_NE(bound, nullptr); + + bound->RecordLong(7); + bound->RecordLong(11); + + std::shared_ptr collector( + new MockCollectorHandle(AggregationTemporality::kDelta)); + std::vector> collectors{collector}; + size_t points = 0; + storage.Collect( + collector.get(), collectors, std::chrono::system_clock::now(), + std::chrono::system_clock::now(), [&](const MetricData &md) { + for (const auto &p : md.point_data_attr_) + { + ++points; + EXPECT_TRUE(opentelemetry::nostd::holds_alternative(p.point_data)); + } + return true; + }); + EXPECT_EQ(points, 1u); +} + +TEST(BoundSyncInstruments, BoundCounterRespectsLastValueAggregation) +{ + InstrumentDescriptor desc{"name", "desc", "1unit", InstrumentType::kCounter, + InstrumentValueType::kLong}; + std::shared_ptr proc(new DefaultAttributesProcessor{}); + AggregationConfig cfg; + SyncMetricStorage storage(desc, AggregationType::kLastValue, proc, +# ifdef ENABLE_METRICS_EXEMPLAR_PREVIEW + ExemplarFilterType::kAlwaysOff, + ExemplarReservoir::GetNoExemplarReservoir(), +# endif + &cfg); + M attrs = {{"k", "v"}}; + auto bound = storage.Bind(KeyValueIterableView(attrs)); + ASSERT_NE(bound, nullptr); + + bound->RecordLong(1); + bound->RecordLong(7); + + std::shared_ptr collector( + new MockCollectorHandle(AggregationTemporality::kDelta)); + std::vector> collectors{collector}; + bool seen = false; + storage.Collect(collector.get(), collectors, std::chrono::system_clock::now(), + std::chrono::system_clock::now(), [&](const MetricData &md) { + for (const auto &p : md.point_data_attr_) + { + const auto &last = + opentelemetry::nostd::get(p.point_data); + EXPECT_TRUE(last.is_lastvalue_valid_); + EXPECT_EQ(opentelemetry::nostd::get(last.value_), 7); + seen = true; + } + return true; + }); + EXPECT_TRUE(seen); +} + +TEST(BoundSyncInstruments, BoundHistogramRespectsCustomBuckets) +{ + InstrumentDescriptor desc{"name", "desc", "1unit", InstrumentType::kHistogram, + InstrumentValueType::kLong}; + std::shared_ptr proc(new DefaultAttributesProcessor{}); + HistogramAggregationConfig cfg; + cfg.boundaries_ = {10.0, 20.0}; + SyncMetricStorage storage(desc, AggregationType::kHistogram, proc, +# ifdef ENABLE_METRICS_EXEMPLAR_PREVIEW + ExemplarFilterType::kAlwaysOff, + ExemplarReservoir::GetNoExemplarReservoir(), +# endif + &cfg); + M attrs = {{"k", "v"}}; + auto bound = storage.Bind(KeyValueIterableView(attrs)); + ASSERT_NE(bound, nullptr); + + bound->RecordLong(5); + bound->RecordLong(15); + bound->RecordLong(25); + + std::shared_ptr collector( + new MockCollectorHandle(AggregationTemporality::kDelta)); + std::vector> collectors{collector}; + bool seen = false; + storage.Collect(collector.get(), collectors, std::chrono::system_clock::now(), + std::chrono::system_clock::now(), [&](const MetricData &md) { + for (const auto &p : md.point_data_attr_) + { + const auto &h = opentelemetry::nostd::get(p.point_data); + EXPECT_EQ(h.boundaries_, std::vector({10.0, 20.0})); + EXPECT_EQ(h.counts_, std::vector({1, 1, 1})); + EXPECT_EQ(h.count_, 3u); + EXPECT_EQ(opentelemetry::nostd::get(h.sum_), 45); + seen = true; + } + return true; + }); + EXPECT_TRUE(seen); +} + +// 3) Bound handles work across delta collection cycles. +TEST(BoundSyncInstruments, BoundSurvivesDeltaCollect) +{ + StorageHolder holder; + M attrs = {{"k", "v"}}; + KeyValueIterableView kv(attrs); + auto bound = holder->Bind(kv); + + bound->RecordLong(2); + EXPECT_EQ(SumLongFor(*holder, AggregationTemporality::kDelta, attrs), 2); + + bound->RecordLong(5); + bound->RecordLong(7); + EXPECT_EQ(SumLongFor(*holder, AggregationTemporality::kDelta, attrs), 12); + + EXPECT_EQ(CollectAndCountPoints(*holder, AggregationTemporality::kDelta), 0u); + + bound->RecordLong(1); + EXPECT_EQ(SumLongFor(*holder, AggregationTemporality::kDelta, attrs), 1); +} + +// 4) Bound handles work across cumulative collection cycles. +TEST(BoundSyncInstruments, BoundSurvivesCumulativeCollect) +{ + StorageHolder holder; + M attrs = {{"k", "v"}}; + KeyValueIterableView kv(attrs); + auto bound = holder->Bind(kv); + + // Cumulative temporality tracks per-collector accumulated state, so we must + // reuse the same CollectorHandle across cycles. + std::shared_ptr collector( + new MockCollectorHandle(AggregationTemporality::kCumulative)); + std::vector> collectors{collector}; + + auto sum_now = [&]() -> int64_t { + int64_t total = 0; + holder->Collect(collector.get(), collectors, std::chrono::system_clock::now(), + std::chrono::system_clock::now(), [&](const MetricData &md) { + for (const auto &p : md.point_data_attr_) + { + const auto &sp = opentelemetry::nostd::get(p.point_data); + total += opentelemetry::nostd::get(sp.value_); + } + return true; + }); + return total; + }; + + bound->RecordLong(10); + EXPECT_EQ(sum_now(), 10); + + bound->RecordLong(5); + EXPECT_EQ(sum_now(), 15); + + bound->RecordLong(20); + EXPECT_EQ(sum_now(), 35); +} + +// 5) Bound + unbound share post-view-filtered attributes -> single merged datapoint. +TEST(BoundSyncInstruments, BoundAndUnboundShareDatapoint) +{ + StorageHolder holder; + M attrs = {{"k", "v"}}; + KeyValueIterableView kv(attrs); + + auto bound = holder->Bind(kv); + bound->RecordLong(4); + holder->RecordLong(6, kv, opentelemetry::context::Context{}); + bound->RecordLong(10); + holder->RecordLong(2, kv, opentelemetry::context::Context{}); + + size_t count = 0; + std::shared_ptr collector( + new MockCollectorHandle(AggregationTemporality::kDelta)); + std::vector> collectors{collector}; + int64_t total = 0; + holder->Collect(collector.get(), collectors, std::chrono::system_clock::now(), + std::chrono::system_clock::now(), [&](const MetricData &md) { + count += md.point_data_attr_.size(); + for (const auto &p : md.point_data_attr_) + { + const auto &sp = opentelemetry::nostd::get(p.point_data); + total += opentelemetry::nostd::get(sp.value_); + } + return true; + }); + EXPECT_EQ(count, 1u); + EXPECT_EQ(total, 22); +} + +// 6) Binding at cardinality overflow records to the overflow bucket and +// multiple overflow-bound handles aggregate into the same overflow datapoint. +TEST(BoundSyncInstruments, BindingAtOverflowGoesToOverflowBucket) +{ + // limit = 3: two real distinct keys are admitted; the third new distinct + // key overflows because the overflow attribute set itself consumes a slot. + StorageHolder holder(InstrumentType::kCounter, InstrumentValueType::kLong, 3); + + M a1 = {{"k", "1"}}; + auto b1 = holder->Bind(KeyValueIterableView(a1)); + b1->RecordLong(1); + + M a2 = {{"k", "2"}}; + auto b2 = holder->Bind(KeyValueIterableView(a2)); + b2->RecordLong(1); + + M a3 = {{"k", "3"}}; + auto b3 = holder->Bind(KeyValueIterableView(a3)); + b3->RecordLong(100); + + M a4 = {{"k", "4"}}; + auto b4 = holder->Bind(KeyValueIterableView(a4)); + b4->RecordLong(50); + + // Overflow datapoint must equal sum of the overflow-bound writes (100 + 50). + std::shared_ptr collector( + new MockCollectorHandle(AggregationTemporality::kDelta)); + std::vector> collectors{collector}; + bool seen = false; + int64_t ov_value = 0; + holder->Collect(collector.get(), collectors, std::chrono::system_clock::now(), + std::chrono::system_clock::now(), [&](const MetricData &md) { + for (const auto &p : md.point_data_attr_) + { + if (p.attributes.find("otel.metrics.overflow") != p.attributes.end()) + { + seen = true; + const auto &sp = opentelemetry::nostd::get(p.point_data); + ov_value += opentelemetry::nostd::get(sp.value_); + } + } + return true; + }); + EXPECT_TRUE(seen); + EXPECT_EQ(ov_value, 150); +} + +// 7) Noop bound instruments compile and no-op. +TEST(BoundSyncInstruments, NoopBoundCompilesAndNoOps) +{ + opentelemetry::metrics::NoopCounter counter("name", "", ""); + M attrs = {{"k", "v"}}; + auto bound = counter.Bind(KeyValueIterableView(attrs)); + ASSERT_NE(bound, nullptr); + bound->Add(1); + bound->Add(42); + + opentelemetry::metrics::NoopHistogram hist("name", "", ""); + auto hb = hist.Bind(KeyValueIterableView(attrs)); + ASSERT_NE(hb, nullptr); + hb->Record(3.14); +} + +// Bonus: SyncMultiMetricStorage::Bind fans out to children. +TEST(BoundSyncInstruments, MultiStorageBindFansOut) +{ + StorageHolder h1; + StorageHolder h2; + SyncMultiMetricStorage multi; + multi.AddStorage(h1.share()); + multi.AddStorage(h2.share()); + + M attrs = {{"k", "v"}}; + KeyValueIterableView kv(attrs); + auto bound = multi.Bind(kv); + ASSERT_NE(bound, nullptr); + + bound->RecordLong(7); + bound->RecordLong(5); + + EXPECT_EQ(SumLongFor(*h1, AggregationTemporality::kDelta, attrs), 12); + EXPECT_EQ(SumLongFor(*h2, AggregationTemporality::kDelta, attrs), 12); +} + +// Bonus: duplicate Bind() with same attrs returns same entry; combined writes. +TEST(BoundSyncInstruments, DuplicateBindReturnsSameEntry) +{ + StorageHolder holder(InstrumentType::kCounter, InstrumentValueType::kLong, 5); + M attrs = {{"k", "v"}}; + KeyValueIterableView kv(attrs); + auto b1 = holder->Bind(kv); + auto b2 = holder->Bind(kv); + b1->RecordLong(3); + b2->RecordLong(4); + EXPECT_EQ(SumLongFor(*holder, AggregationTemporality::kDelta, attrs), 7); +} + +// Bonus: dropped bound entries with no pending data are eventually GC'd at Collect. +TEST(BoundSyncInstruments, DroppedBoundEntriesAreGarbageCollected) +{ + StorageHolder holder(InstrumentType::kCounter, InstrumentValueType::kLong, 5); + M attrs = {{"k", "v"}}; + KeyValueIterableView kv(attrs); + + // Bind, record once, Collect → dirty rotated to false, entry retained while + // user holds the shared_ptr. + { + auto bound = holder->Bind(kv); + bound->RecordLong(1); + EXPECT_EQ(SumLongFor(*holder, AggregationTemporality::kDelta, attrs), 1); + // bound dropped here; on next Collect, entry will be GC'd because + // use_count == 1 (only the storage map holds it) and dirty_ is false. + } + + // Drive a Collect to allow GC. + EXPECT_EQ(CollectAndCountPoints(*holder, AggregationTemporality::kDelta), 0u); + + // Now binding again: with limit=5 and no live entries, this must succeed + // without overflow. Fill up to just below the limit using fresh keys. + std::vector> handles; + for (int i = 0; i < 3; ++i) + { + M a = {{"x", std::to_string(i)}}; + auto h = holder->Bind(KeyValueIterableView(a)); + h->RecordLong(1); + handles.push_back(h); + } + // Expect no overflow yet (3 + 1 < 5). + EXPECT_FALSE(HasOverflowPoint(*holder, AggregationTemporality::kDelta)); +} + +// Unified cardinality: an unbound key that already has a datapoint must allow +// Bind() to reuse it without consuming new cardinality. +TEST(BoundSyncInstruments, BindOnExistingUnboundKeyDoesNotOverflow) +{ + // limit = 3: two real keys are admitted; a third new distinct key would + // overflow (overflow itself consumes a slot). + StorageHolder holder(InstrumentType::kCounter, InstrumentValueType::kLong, 3); + + M a1 = {{"k", "1"}}; + M a2 = {{"k", "2"}}; + // Two distinct unbound keys consume the two admitted slots (limit=3 with + // overflow consuming a slot leaves room for two real keys). + holder->RecordLong(10, KeyValueIterableView(a1), opentelemetry::context::Context{}); + holder->RecordLong(20, KeyValueIterableView(a2), opentelemetry::context::Context{}); + + // Bind one of the existing keys: must NOT overflow, must NOT consume a new + // slot. Bound write goes to the same logical datapoint as the unbound write. + auto b1 = holder->Bind(KeyValueIterableView(a1)); + ASSERT_NE(b1, nullptr); + b1->RecordLong(5); + + // Single delta collect: assert no overflow and bound+unbound merge to one point. + std::shared_ptr collector( + new MockCollectorHandle(AggregationTemporality::kDelta)); + std::vector> collectors{collector}; + bool overflow_seen = false; + int64_t a1_sum = 0; + holder->Collect( + collector.get(), collectors, std::chrono::system_clock::now(), + std::chrono::system_clock::now(), [&](const MetricData &md) { + for (const auto &p : md.point_data_attr_) + { + if (p.attributes.find("otel.metrics.overflow") != p.attributes.end()) + overflow_seen = true; + auto it = p.attributes.find("k"); + if (it != p.attributes.end() && opentelemetry::nostd::get(it->second) == "1") + { + const auto &sp = opentelemetry::nostd::get(p.point_data); + a1_sum += opentelemetry::nostd::get(sp.value_); + } + } + return true; + }); + EXPECT_FALSE(overflow_seen); + EXPECT_EQ(a1_sum, 15); +} + +// Unified cardinality: an existing bound key must allow unbound Record() +// to reuse it without consuming new cardinality, and merge into one datapoint. +TEST(BoundSyncInstruments, UnboundOnExistingBoundKeyDoesNotOverflow) +{ + StorageHolder holder(InstrumentType::kCounter, InstrumentValueType::kLong, 3); + + M a1 = {{"k", "1"}}; + M a2 = {{"k", "2"}}; + auto b1 = holder->Bind(KeyValueIterableView(a1)); + auto b2 = holder->Bind(KeyValueIterableView(a2)); + b1->RecordLong(7); + b2->RecordLong(3); + + // Unbound write to existing bound key — no overflow, merges with bound. + holder->RecordLong(13, KeyValueIterableView(a1), opentelemetry::context::Context{}); + + // Single delta collect captures all points. + std::shared_ptr collector( + new MockCollectorHandle(AggregationTemporality::kDelta)); + std::vector> collectors{collector}; + bool overflow_seen = false; + int64_t a1_sum = 0, a2_sum = 0; + holder->Collect(collector.get(), collectors, std::chrono::system_clock::now(), + std::chrono::system_clock::now(), [&](const MetricData &md) { + for (const auto &p : md.point_data_attr_) + { + if (p.attributes.find("otel.metrics.overflow") != p.attributes.end()) + overflow_seen = true; + auto it = p.attributes.find("k"); + if (it == p.attributes.end()) + continue; + const auto &sp = opentelemetry::nostd::get(p.point_data); + auto v = opentelemetry::nostd::get(sp.value_); + auto s = opentelemetry::nostd::get(it->second); + if (s == "1") + a1_sum += v; + else if (s == "2") + a2_sum += v; + } + return true; + }); + EXPECT_FALSE(overflow_seen); + EXPECT_EQ(a1_sum, 20); + EXPECT_EQ(a2_sum, 3); +} + +// Unified cardinality: after delta Collect resets attributes_hashmap_, retained +// bound entries must still count toward the cardinality limit so that new +// unbound keys correctly overflow. +TEST(BoundSyncInstruments, RetainedBoundEntriesCountAfterDeltaCollect) +{ + StorageHolder holder(InstrumentType::kCounter, InstrumentValueType::kLong, 3); + + // Two bound keys consume the two real admitted slots (overflow takes the 3rd). + M a1 = {{"bound", "1"}}; + M a2 = {{"bound", "2"}}; + auto b1 = holder->Bind(KeyValueIterableView(a1)); + auto b2 = holder->Bind(KeyValueIterableView(a2)); + b1->RecordLong(1); + b2->RecordLong(1); + + // Delta collect resets attributes_hashmap_ but retains bound_entries_. + EXPECT_EQ(CollectAndCountPoints(*holder, AggregationTemporality::kDelta), 2u); + + // New unbound key now must overflow because bound entries still count. + M new_key = {{"unbound", "fresh"}}; + holder->RecordLong(99, KeyValueIterableView(new_key), opentelemetry::context::Context{}); + + EXPECT_TRUE(HasOverflowPoint(*holder, AggregationTemporality::kDelta)); +} + +// Strengthened: assert the overflow datapoint VALUE, not just its presence. +TEST(BoundSyncInstruments, RetainedBoundEntriesOverflowValue) +{ + StorageHolder holder(InstrumentType::kCounter, InstrumentValueType::kLong, 3); + M a1 = {{"bound", "1"}}; + M a2 = {{"bound", "2"}}; + auto b1 = holder->Bind(KeyValueIterableView(a1)); + auto b2 = holder->Bind(KeyValueIterableView(a2)); + b1->RecordLong(1); + b2->RecordLong(1); + EXPECT_EQ(CollectAndCountPoints(*holder, AggregationTemporality::kDelta), 2u); + + M new_key = {{"unbound", "fresh"}}; + holder->RecordLong(99, KeyValueIterableView(new_key), opentelemetry::context::Context{}); + + std::shared_ptr collector( + new MockCollectorHandle(AggregationTemporality::kDelta)); + std::vector> collectors{collector}; + int64_t overflow_sum = 0; + holder->Collect(collector.get(), collectors, std::chrono::system_clock::now(), + std::chrono::system_clock::now(), [&](const MetricData &md) { + for (const auto &p : md.point_data_attr_) + { + if (p.attributes.find("otel.metrics.overflow") == p.attributes.end()) + continue; + const auto &sp = opentelemetry::nostd::get(p.point_data); + overflow_sum += opentelemetry::nostd::get(sp.value_); + } + return true; + }); + EXPECT_EQ(overflow_sum, 99); +} + +// No-attribute unbound RecordLong must follow the unified cardinality policy. +// With limit=3, two real distinct bound keys are admitted; the empty-attr +// unbound write would be a third new key and routes to overflow (overflow +// itself consumes a slot). +TEST(BoundSyncInstruments, NoAttributeUnboundFollowsUnifiedPolicyLong) +{ + StorageHolder holder(InstrumentType::kCounter, InstrumentValueType::kLong, 3); + M a1 = {{"k", "1"}}; + M a2 = {{"k", "2"}}; + auto b1 = holder->Bind(KeyValueIterableView(a1)); + auto b2 = holder->Bind(KeyValueIterableView(a2)); + b1->RecordLong(1); + b2->RecordLong(1); + + // Empty-attribute unbound write — no slots remain, must overflow. + holder->RecordLong(42, opentelemetry::context::Context{}); + + std::shared_ptr collector( + new MockCollectorHandle(AggregationTemporality::kDelta)); + std::vector> collectors{collector}; + int64_t overflow_sum = 0; + holder->Collect(collector.get(), collectors, std::chrono::system_clock::now(), + std::chrono::system_clock::now(), [&](const MetricData &md) { + for (const auto &p : md.point_data_attr_) + { + if (p.attributes.find("otel.metrics.overflow") == p.attributes.end()) + continue; + const auto &sp = opentelemetry::nostd::get(p.point_data); + overflow_sum += opentelemetry::nostd::get(sp.value_); + } + return true; + }); + EXPECT_EQ(overflow_sum, 42); +} + +// Same coverage for double counter no-attribute path. +TEST(BoundSyncInstruments, NoAttributeUnboundFollowsUnifiedPolicyDouble) +{ + StorageHolder holder(InstrumentType::kCounter, InstrumentValueType::kDouble, 3); + M a1 = {{"k", "1"}}; + M a2 = {{"k", "2"}}; + auto b1 = holder->Bind(KeyValueIterableView(a1)); + auto b2 = holder->Bind(KeyValueIterableView(a2)); + b1->RecordDouble(1.0); + b2->RecordDouble(1.0); + + holder->RecordDouble(7.5, opentelemetry::context::Context{}); + + std::shared_ptr collector( + new MockCollectorHandle(AggregationTemporality::kDelta)); + std::vector> collectors{collector}; + double overflow_sum = 0.0; + holder->Collect(collector.get(), collectors, std::chrono::system_clock::now(), + std::chrono::system_clock::now(), [&](const MetricData &md) { + for (const auto &p : md.point_data_attr_) + { + if (p.attributes.find("otel.metrics.overflow") == p.attributes.end()) + continue; + const auto &sp = opentelemetry::nostd::get(p.point_data); + overflow_sum += opentelemetry::nostd::get(sp.value_); + } + return true; + }); + EXPECT_DOUBLE_EQ(overflow_sum, 7.5); +} + +// Bound handle outliving its storage must not crash. BoundEntry is self-contained. +TEST(BoundSyncInstruments, BoundOutlivesStorage) +{ + std::shared_ptr retained; + { + StorageHolder holder; + M attrs = {{"k", "v"}}; + retained = holder->Bind(KeyValueIterableView(attrs)); + ASSERT_NE(retained, nullptr); + retained->RecordLong(1); + } + // Storage destroyed; bound handle still alive. Recording must not crash. + retained->RecordLong(2); + retained.reset(); + SUCCEED(); +} + +// Mixed bound + unbound across multiple cumulative collections. +TEST(BoundSyncInstruments, MixedBoundUnboundAcrossCumulativeCollections) +{ + StorageHolder holder; + M attrs = {{"k", "v"}}; + KeyValueIterableView kv(attrs); + auto bound = holder->Bind(kv); + + std::shared_ptr collector( + new MockCollectorHandle(AggregationTemporality::kCumulative)); + std::vector> collectors{collector}; + auto sum_now = [&]() -> int64_t { + int64_t total = 0; + holder->Collect(collector.get(), collectors, std::chrono::system_clock::now(), + std::chrono::system_clock::now(), [&](const MetricData &md) { + for (const auto &p : md.point_data_attr_) + { + const auto &sp = opentelemetry::nostd::get(p.point_data); + total += opentelemetry::nostd::get(sp.value_); + } + return true; + }); + return total; + }; + + bound->RecordLong(3); + holder->RecordLong(7, kv, opentelemetry::context::Context{}); + EXPECT_EQ(sum_now(), 10); + + holder->RecordLong(5, kv, opentelemetry::context::Context{}); + bound->RecordLong(11); + EXPECT_EQ(sum_now(), 26); + + bound->RecordLong(4); + EXPECT_EQ(sum_now(), 30); +} + +// Regression for M1: dropped-but-dirty bound entries must not retain their +// cardinality slot for the next interval after their data is collected. +TEST(BoundSyncInstruments, DirtyDroppedBoundEntriesReleaseCardinality) +{ + StorageHolder holder(InstrumentType::kCounter, InstrumentValueType::kLong, 3); + + // Bind two distinct keys (consume the two real admitted slots), record so + // they're dirty, then drop the handles BEFORE Collect(). The pre-rotation + // GC cannot remove them (they're dirty); rotation flushes their data; the + // post-rotation cleanup must release their slots. + M a1 = {{"k", "1"}}; + M a2 = {{"k", "2"}}; + { + auto b1 = holder->Bind(KeyValueIterableView(a1)); + auto b2 = holder->Bind(KeyValueIterableView(a2)); + b1->RecordLong(7); + b2->RecordLong(11); + } + + // Collect rotates dirty data and (via the M1 fix) drops the stale entries. + EXPECT_EQ(SumLongFor(*holder, AggregationTemporality::kDelta, a1), 7); + + // In the next interval, two fresh distinct keys must be admittable without + // overflow because the dropped bound entries no longer count. + M a3 = {{"k", "3"}}; + M a4 = {{"k", "4"}}; + holder->RecordLong(5, KeyValueIterableView(a3), opentelemetry::context::Context{}); + holder->RecordLong(9, KeyValueIterableView(a4), opentelemetry::context::Context{}); + + EXPECT_FALSE(HasOverflowPoint(*holder, AggregationTemporality::kDelta)); +} + +// Regression for M2: ResolveCardinality must mirror +// AttributesHashMap::IsOverflowAttributes() exactly. When the overflow slot +// is already counted in active_keys_ but room remains under the limit, a +// fresh real key must still be admitted, not routed to overflow. +TEST(BoundSyncInstruments, OverflowParityAllowsFillingRemainingSlot) +{ + StorageHolder holder(InstrumentType::kCounter, InstrumentValueType::kLong, 3); + + // Trigger overflow first: bind k1, k2, then k3 (which routes to overflow, + // so bound_entries_ ends up with k1, k2, and the overflow entry). + M a1 = {{"k", "1"}}; + M a2 = {{"k", "2"}}; + M a3 = {{"k", "3"}}; + auto b1 = holder->Bind(KeyValueIterableView(a1)); + auto b2 = holder->Bind(KeyValueIterableView(a2)); + auto bov = holder->Bind(KeyValueIterableView(a3)); // routes to overflow + b1->RecordLong(1); + b2->RecordLong(1); + bov->RecordLong(100); + + // Drop k1 only. After Collect(), the M1 cleanup releases its slot, leaving + // active_keys_ = { k2, overflow }. With limit=3, has_overflow=true and + // active_keys_.size()==2, the existing AttributesHashMap semantics admit + // one more real key. The pre-fix preview path would have routed it to + // overflow (off-by-one). After M2, it must be admitted. + b1.reset(); + EXPECT_EQ(CollectAndCountPoints(*holder, AggregationTemporality::kDelta), 3u); + + // Fresh real key in the next interval must be admitted, not routed to overflow. + M a4 = {{"fresh", "key"}}; + holder->RecordLong(42, KeyValueIterableView(a4), opentelemetry::context::Context{}); + + std::shared_ptr collector( + new MockCollectorHandle(AggregationTemporality::kDelta)); + std::vector> collectors{collector}; + bool overflow_seen = false; + bool a4_seen = false; + int64_t a4_value = 0; + holder->Collect(collector.get(), collectors, std::chrono::system_clock::now(), + std::chrono::system_clock::now(), [&](const MetricData &md) { + for (const auto &p : md.point_data_attr_) + { + if (p.attributes.find("otel.metrics.overflow") != p.attributes.end()) + { + overflow_seen = true; + } + auto it = p.attributes.find("fresh"); + if (it != p.attributes.end() && + opentelemetry::nostd::get(it->second) == "key") + { + a4_seen = true; + const auto &sp = opentelemetry::nostd::get(p.point_data); + a4_value = opentelemetry::nostd::get(sp.value_); + } + } + return true; + }); + EXPECT_TRUE(a4_seen); + EXPECT_EQ(a4_value, 42); + EXPECT_FALSE(overflow_seen); +} + +#endif // OPENTELEMETRY_HAVE_METRICS_BOUND_INSTRUMENTS_PREVIEW diff --git a/sdk/test/metrics/measurements_benchmark.cc b/sdk/test/metrics/measurements_benchmark.cc index 4b41d490f4..2284dc57f1 100644 --- a/sdk/test/metrics/measurements_benchmark.cc +++ b/sdk/test/metrics/measurements_benchmark.cc @@ -26,6 +26,7 @@ #include "opentelemetry/sdk/metrics/instruments.h" #include "opentelemetry/sdk/metrics/meter_provider.h" #include "opentelemetry/sdk/metrics/metric_reader.h" +#include "opentelemetry/version.h" using namespace opentelemetry; using namespace opentelemetry::sdk::instrumentationscope; @@ -212,5 +213,56 @@ void BM_MeasurementsPerThreadCounterTest(benchmark::State &state) } BENCHMARK(BM_MeasurementsPerThreadCounterTest); +#ifdef OPENTELEMETRY_HAVE_METRICS_BOUND_INSTRUMENTS_PREVIEW +// Fixed-attribute hot path: unbound vs bound. The intent is to isolate +// per-call attribute processing and hashmap lookup overhead, so the same +// fixed attribute set is used for every Add() call. +namespace +{ +std::map MakeFixedAttributes() +{ + return {{"dim1", "value1"}, {"dim2", "value2"}, {"dim3", "value3"}}; +} +} // namespace + +void BM_UnboundFixedAttrsCounter(benchmark::State &state) +{ + MeterProvider mp; + std::shared_ptr exporter(new MockMetricExporter()); + mp.AddMetricReader(exporter); + auto m = mp.GetMeter("meter1", "version1", "schema1"); + auto counter = m->CreateDoubleCounter("counter_unbound_fixed", "fixed-attrs unbound", "unit"); + auto attrs = MakeFixedAttributes(); + auto context = opentelemetry::context::Context{}; + while (state.KeepRunning()) + { + counter->Add( + 1.0, opentelemetry::common::KeyValueIterableView>(attrs), + context); + } + exporter->Collect([&](ResourceMetrics & /*rm*/) { return true; }); +} +BENCHMARK(BM_UnboundFixedAttrsCounter); + +void BM_BoundFixedAttrsCounter(benchmark::State &state) +{ + MeterProvider mp; + std::shared_ptr exporter(new MockMetricExporter()); + mp.AddMetricReader(exporter); + auto m = mp.GetMeter("meter1", "version1", "schema1"); + auto counter = m->CreateDoubleCounter("counter_bound_fixed", "fixed-attrs bound", "unit"); + auto attrs = MakeFixedAttributes(); + auto bound = counter->Bind( + opentelemetry::common::KeyValueIterableView>(attrs)); + benchmark::DoNotOptimize(bound.get()); + while (state.KeepRunning()) + { + bound->Add(1.0); + } + exporter->Collect([&](ResourceMetrics & /*rm*/) { return true; }); +} +BENCHMARK(BM_BoundFixedAttrsCounter); +#endif // OPENTELEMETRY_HAVE_METRICS_BOUND_INSTRUMENTS_PREVIEW + } // namespace BENCHMARK_MAIN(); diff --git a/test_common/cmake/preview-options.cmake b/test_common/cmake/preview-options.cmake index f6c3e90f5e..04fe4aab11 100644 --- a/test_common/cmake/preview-options.cmake +++ b/test_common/cmake/preview-options.cmake @@ -17,3 +17,6 @@ set(WITH_RESOURCE_DETECTORS_PREVIEW ${ENABLE_PREVIEW} CACHE BOOL "" FORCE) set(WITH_OTLP_HTTP_COMPRESSION ${ENABLE_PREVIEW} CACHE BOOL "" FORCE) set(WITH_CURL_LOGGING ${ENABLE_PREVIEW} CACHE BOOL "" FORCE) set(WITH_CONFIGURATION ${ENABLE_PREVIEW} CACHE BOOL "" FORCE) +if(WITH_ABI_VERSION_2) + set(WITH_METRICS_BOUND_INSTRUMENTS_PREVIEW ${ENABLE_PREVIEW} CACHE BOOL "" FORCE) +endif()