From 404fe1be1504405a0711745bef583776dfb9acd3 Mon Sep 17 00:00:00 2001 From: Aditya Bavadekar Date: Wed, 21 Oct 2026 12:10:58 +0530 Subject: [PATCH 1/3] [API/SDK] Add Enabled() to synchronous instruments - Instruments report disabled when their meter is disabled or every matching view drops them - The check is a single read, also on MSVC - No-op instruments always report disabled (ABI v2 only). --- CHANGELOG.md | 5 + api/include/opentelemetry/common/macros.h | 4 + api/include/opentelemetry/metrics/noop.h | 22 ++++- .../opentelemetry/metrics/sync_instruments.h | 40 ++++++++ api/test/metrics/noop_sync_instrument_test.cc | 15 +++ sdk/include/opentelemetry/sdk/metrics/meter.h | 8 +- .../sdk/metrics/meter_enabled_state.h | 26 ++++-- .../sdk/metrics/state/multi_metric_storage.h | 3 + sdk/src/metrics/meter.cc | 58 ++++++++---- sdk/src/metrics/sync_instruments.cc | 20 ++++ sdk/test/metrics/meter_provider_sdk_test.cc | 91 +++++++++++++++++++ 11 files changed, 261 insertions(+), 31 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0d51988d4d..f68ae312ad 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,11 @@ Increment the: ## [Unreleased] +* [API/SDK] Add `Enabled()` to synchronous metric instruments (ABI v2). It + returns `false` when the Meter is disabled or every matching view uses the + Drop aggregation + [#4721](https://github.com/open-telemetry/opentelemetry-cpp/pull/4721) + * [API/SDK] Add ASCII helpers and remove regex from InstrumentMetaDataValidator [#4715](https://github.com/open-telemetry/opentelemetry-cpp/pull/4715) diff --git a/api/include/opentelemetry/common/macros.h b/api/include/opentelemetry/common/macros.h index 4117a2f9a1..a566e02525 100644 --- a/api/include/opentelemetry/common/macros.h +++ b/api/include/opentelemetry/common/macros.h @@ -375,6 +375,7 @@ point. # define OPENTELEMETRY_ATOMIC_READ_8(ptr) __atomic_load_n(ptr, __ATOMIC_SEQ_CST) # define OPENTELEMETRY_ATOMIC_WRITE_8(ptr, value) __atomic_store_n(ptr, value, __ATOMIC_SEQ_CST) +# define OPENTELEMETRY_ATOMIC_READ_RELAXED_8(ptr) __atomic_load_n(ptr, __ATOMIC_RELAXED) #elif defined(_MSC_VER) @@ -384,6 +385,9 @@ point. static_cast(_InterlockedCompareExchange8(reinterpret_cast(ptr), 0, 0)) # define OPENTELEMETRY_ATOMIC_WRITE_8(ptr, value) \ _InterlockedExchange8(reinterpret_cast(ptr), static_cast(value)) +// A plain load, unlike OPENTELEMETRY_ATOMIC_READ_8 which locks the cache line on every read. +# define OPENTELEMETRY_ATOMIC_READ_RELAXED_8(ptr) \ + static_cast(__iso_volatile_load8(reinterpret_cast(ptr))) #else # error port atomics read/write for the current platform diff --git a/api/include/opentelemetry/metrics/noop.h b/api/include/opentelemetry/metrics/noop.h index cab881b20b..cd20e68df7 100644 --- a/api/include/opentelemetry/metrics/noop.h +++ b/api/include/opentelemetry/metrics/noop.h @@ -39,7 +39,11 @@ class NoopCounter : public Counter NoopCounter(nostd::string_view /* name */, nostd::string_view /* description */, nostd::string_view /* unit */) noexcept - {} + { +#if OPENTELEMETRY_ABI_VERSION_NO >= 2 + this->SetEnabledFlag(this->DisabledFlag()); +#endif + } void Add(T /* value */) noexcept override {} void Add(T /* value */, const context::Context & /* context */) noexcept override {} void Add(T /* value */, const common::KeyValueIterable & /* attributes */) noexcept override {} @@ -63,7 +67,11 @@ class NoopHistogram : public Histogram NoopHistogram(nostd::string_view /* name */, nostd::string_view /* description */, nostd::string_view /* unit */) noexcept - {} + { +#if OPENTELEMETRY_ABI_VERSION_NO >= 2 + this->SetEnabledFlag(this->DisabledFlag()); +#endif + } void Record(T /* value */, const context::Context & /* context */) noexcept override {} void Record(T /* value */, const common::KeyValueIterable & /* attributes */, @@ -93,7 +101,11 @@ class NoopUpDownCounter : public UpDownCounter NoopUpDownCounter(nostd::string_view /* name */, nostd::string_view /* description */, nostd::string_view /* unit */) noexcept - {} + { +#if OPENTELEMETRY_ABI_VERSION_NO >= 2 + this->SetEnabledFlag(this->DisabledFlag()); +#endif + } void Add(T /* value */) noexcept override {} void Add(T /* value */, const context::Context & /* context */) noexcept override {} void Add(T /* value */, const common::KeyValueIterable & /* attributes */) noexcept override {} @@ -111,7 +123,9 @@ class NoopGauge : public Gauge NoopGauge(nostd::string_view /* name */, nostd::string_view /* description */, nostd::string_view /* unit */) noexcept - {} + { + this->SetEnabledFlag(this->DisabledFlag()); + } void Record(T /* value */) noexcept override {} void Record(T /* value */, const context::Context & /* context */) noexcept override {} void Record(T /* value */, const common::KeyValueIterable & /* attributes */) noexcept override {} diff --git a/api/include/opentelemetry/metrics/sync_instruments.h b/api/include/opentelemetry/metrics/sync_instruments.h index 7f709129bb..8a2916a3f5 100644 --- a/api/include/opentelemetry/metrics/sync_instruments.h +++ b/api/include/opentelemetry/metrics/sync_instruments.h @@ -28,6 +28,46 @@ class SynchronousInstrument SynchronousInstrument &operator=(const SynchronousInstrument &) = default; SynchronousInstrument &operator=(SynchronousInstrument &&) noexcept = default; virtual ~SynchronousInstrument() = default; + +#if OPENTELEMETRY_ABI_VERSION_NO >= 2 + /** + * Reports if the instrument is enabled or not. A disabled instrument drops its measurements. + * + * The instrumentation authors should call this method each time before recording a + * measurement, to potentially avoid performing computationally expensive operations for + * disabled instruments. The returned value can change over time. + * + * @since ABI_VERSION 2 + */ + bool Enabled() const noexcept { return OPENTELEMETRY_ATOMIC_READ_RELAXED_8(enabled_) != 0; } + +protected: + /** + * Sets the flag read by Enabled(). SDKs point it at a flag they update; it must outlive this + * instrument. + * + * @since ABI_VERSION 2 + */ + void SetEnabledFlag(const bool *enabled) noexcept { enabled_ = enabled; } + + /** A flag that is always false, for instruments that are never enabled. */ + static const bool *DisabledFlag() noexcept + { + static const bool kDisabled = false; + return &kDisabled; + } + +private: + static const bool *EnabledFlag() noexcept + { + static const bool kEnabled = true; + return &kEnabled; + } + + // Variable to support implementation of Enabled method introduced in ABI V2. + // A pointer, so an SDK can flip many instruments with a single write. + const bool *enabled_ = EnabledFlag(); +#endif }; #ifdef OPENTELEMETRY_HAVE_METRICS_BOUND_INSTRUMENTS_PREVIEW diff --git a/api/test/metrics/noop_sync_instrument_test.cc b/api/test/metrics/noop_sync_instrument_test.cc index e5b9ed7f91..1815a587eb 100644 --- a/api/test/metrics/noop_sync_instrument_test.cc +++ b/api/test/metrics/noop_sync_instrument_test.cc @@ -53,3 +53,18 @@ TEST(UpDownCountr, Record) counter->Add(10, {{"k1", "1"}, {"k2", 2}}); counter->Add(10, {{"k1", "1"}, {"k2", 2}}, opentelemetry::context::Context{}); } + +#if OPENTELEMETRY_ABI_VERSION_NO >= 2 +TEST(SynchronousInstrument, NoopInstrumentsAreDisabled) +{ + opentelemetry::metrics::NoopCounter counter("test", "none", "unitless"); + opentelemetry::metrics::NoopHistogram histogram("test", "none", "unitless"); + opentelemetry::metrics::NoopUpDownCounter up_down_counter("test", "none", "unitless"); + opentelemetry::metrics::NoopGauge gauge("test", "none", "unitless"); + + EXPECT_FALSE(counter.Enabled()); + EXPECT_FALSE(histogram.Enabled()); + EXPECT_FALSE(up_down_counter.Enabled()); + EXPECT_FALSE(gauge.Enabled()); +} +#endif diff --git a/sdk/include/opentelemetry/sdk/metrics/meter.h b/sdk/include/opentelemetry/sdk/metrics/meter.h index be1ea3d95d..234f9c92f5 100644 --- a/sdk/include/opentelemetry/sdk/metrics/meter.h +++ b/sdk/include/opentelemetry/sdk/metrics/meter.h @@ -36,6 +36,7 @@ namespace metrics class MetricStorage; class SyncWritableMetricStorage; +class SyncMultiMetricStorage; class AsyncWritableMetricsStorge; class ObservableRegistry; @@ -169,8 +170,13 @@ class Meter final : public opentelemetry::metrics::Meter std::shared_ptr observable_registry_; // Shared with this meter's instruments so updates reach them; atomic so recording never blocks. std::shared_ptr meter_enabled_state_{new MeterEnabledState()}; - std::unique_ptr RegisterSyncMetricStorage( + // Shared with instruments whose matching views all use the Drop aggregation. Never enabled. + std::shared_ptr always_disabled_state_{new MeterEnabledState(false)}; + std::unique_ptr RegisterSyncMetricStorage( InstrumentDescriptor &instrument_descriptor); + /** Returns the enabled state an instrument with this storage should share. */ + std::shared_ptr GetInstrumentEnabledState( + const SyncMultiMetricStorage *storage) const noexcept; std::unique_ptr RegisterAsyncMetricStorage( InstrumentDescriptor &instrument_descriptor); std::mutex storage_lock_; diff --git a/sdk/include/opentelemetry/sdk/metrics/meter_enabled_state.h b/sdk/include/opentelemetry/sdk/metrics/meter_enabled_state.h index c13f53baf4..e46d477763 100644 --- a/sdk/include/opentelemetry/sdk/metrics/meter_enabled_state.h +++ b/sdk/include/opentelemetry/sdk/metrics/meter_enabled_state.h @@ -3,8 +3,7 @@ #pragma once -#include - +#include "opentelemetry/common/macros.h" #include "opentelemetry/version.h" OPENTELEMETRY_BEGIN_NAMESPACE @@ -16,11 +15,12 @@ namespace metrics /** * A Meter's enabled flag, shared with every instrument it creates so a configurator update * reaches instruments that already exist. Relaxed ordering: no dependent data, only visibility. + * A plain bool, not std::atomic, because API instruments read it through Flag(). */ class MeterEnabledState { public: - explicit MeterEnabledState(bool enabled = true) noexcept : enabled_(enabled) {} + explicit MeterEnabledState(bool enabled = true) noexcept { flag_.enabled = enabled; } ~MeterEnabledState() = default; MeterEnabledState(const MeterEnabledState &) = delete; @@ -28,12 +28,26 @@ class MeterEnabledState MeterEnabledState &operator=(const MeterEnabledState &) = delete; MeterEnabledState &operator=(MeterEnabledState &&) = delete; - bool IsEnabled() const noexcept { return enabled_.load(std::memory_order_relaxed); } + bool IsEnabled() const noexcept + { + return OPENTELEMETRY_ATOMIC_READ_RELAXED_8(&flag_.enabled) != 0; + } + + void SetEnabled(bool enabled) noexcept { OPENTELEMETRY_ATOMIC_WRITE_8(&flag_.enabled, enabled); } - void SetEnabled(bool enabled) noexcept { enabled_.store(enabled, std::memory_order_relaxed); } + /** The flag itself, valid for the lifetime of this object. */ + const bool *Flag() const noexcept { return &flag_.enabled; } private: - std::atomic enabled_; + // Padding keeps the flag off cache lines that neighbouring heap objects write to, since every + // instrument of the meter reads it on each Enabled() call. + struct PaddedFlag + { + char padding_before[64]; + bool enabled; + char padding_after[63]; + }; + PaddedFlag flag_{}; }; } // namespace metrics 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 2b5798a7f6..c0a29bb751 100644 --- a/sdk/include/opentelemetry/sdk/metrics/state/multi_metric_storage.h +++ b/sdk/include/opentelemetry/sdk/metrics/state/multi_metric_storage.h @@ -40,6 +40,9 @@ class SyncMultiMetricStorage : public SyncWritableMetricStorage return true; } + /** True when no storage was added, e.g. every matching view uses the Drop aggregation. */ + bool Empty() const noexcept { return storages_.empty(); } + void RecordLong(int64_t value, const opentelemetry::context::Context &context) noexcept override { for (auto &s : storages_) diff --git a/sdk/src/metrics/meter.cc b/sdk/src/metrics/meter.cc index f9bf08742d..897bd5c98e 100644 --- a/sdk/src/metrics/meter.cc +++ b/sdk/src/metrics/meter.cc @@ -155,6 +155,17 @@ void Meter::UpdateMeterConfig(MeterConfig config) noexcept meter_enabled_state_->SetEnabled(config.IsEnabled()); } +std::shared_ptr Meter::GetInstrumentEnabledState( + const SyncMultiMetricStorage *storage) const noexcept +{ + // Every matching view uses the Drop aggregation, so the instrument never records. + if (storage != nullptr && storage->Empty()) + { + return always_disabled_state_; + } + return meter_enabled_state_; +} + opentelemetry::nostd::unique_ptr> Meter::CreateUInt64Counter( opentelemetry::nostd::string_view name, opentelemetry::nostd::string_view description, @@ -171,9 +182,10 @@ opentelemetry::nostd::unique_ptr> Meter::CreateUInt64 InstrumentDescriptor instrument_descriptor = { std::string{name.data(), name.size()}, std::string{description.data(), description.size()}, std::string{unit.data(), unit.size()}, InstrumentType::kCounter, InstrumentValueType::kLong}; - auto storage = RegisterSyncMetricStorage(instrument_descriptor); + auto storage = RegisterSyncMetricStorage(instrument_descriptor); + auto enabled_state = GetInstrumentEnabledState(storage.get()); return opentelemetry::nostd::unique_ptr>( - new LongCounter(instrument_descriptor, std::move(storage), meter_enabled_state_)); + new LongCounter(instrument_descriptor, std::move(storage), std::move(enabled_state))); } opentelemetry::nostd::unique_ptr> Meter::CreateDoubleCounter( @@ -193,9 +205,10 @@ opentelemetry::nostd::unique_ptr> Meter::CreateDoubleCo std::string{name.data(), name.size()}, std::string{description.data(), description.size()}, std::string{unit.data(), unit.size()}, InstrumentType::kCounter, InstrumentValueType::kDouble}; - auto storage = RegisterSyncMetricStorage(instrument_descriptor); + auto storage = RegisterSyncMetricStorage(instrument_descriptor); + auto enabled_state = GetInstrumentEnabledState(storage.get()); return opentelemetry::nostd::unique_ptr>{ - new DoubleCounter(instrument_descriptor, std::move(storage), meter_enabled_state_)}; + new DoubleCounter(instrument_descriptor, std::move(storage), std::move(enabled_state))}; } opentelemetry::nostd::shared_ptr @@ -257,9 +270,10 @@ opentelemetry::nostd::unique_ptr> Meter::CreateUInt std::string{name.data(), name.size()}, std::string{description.data(), description.size()}, std::string{unit.data(), unit.size()}, InstrumentType::kHistogram, InstrumentValueType::kLong}; - auto storage = RegisterSyncMetricStorage(instrument_descriptor); + auto storage = RegisterSyncMetricStorage(instrument_descriptor); + auto enabled_state = GetInstrumentEnabledState(storage.get()); return opentelemetry::nostd::unique_ptr>{ - new LongHistogram(instrument_descriptor, std::move(storage), meter_enabled_state_)}; + new LongHistogram(instrument_descriptor, std::move(storage), std::move(enabled_state))}; } opentelemetry::nostd::unique_ptr> Meter::CreateDoubleHistogram( @@ -279,9 +293,10 @@ opentelemetry::nostd::unique_ptr> Meter::CreateDouble std::string{name.data(), name.size()}, std::string{description.data(), description.size()}, std::string{unit.data(), unit.size()}, InstrumentType::kHistogram, InstrumentValueType::kDouble}; - auto storage = RegisterSyncMetricStorage(instrument_descriptor); + auto storage = RegisterSyncMetricStorage(instrument_descriptor); + auto enabled_state = GetInstrumentEnabledState(storage.get()); return opentelemetry::nostd::unique_ptr>{ - new DoubleHistogram(instrument_descriptor, std::move(storage), meter_enabled_state_)}; + new DoubleHistogram(instrument_descriptor, std::move(storage), std::move(enabled_state))}; } #if OPENTELEMETRY_ABI_VERSION_NO >= 2 @@ -301,9 +316,10 @@ opentelemetry::nostd::unique_ptr> Meter::CreateInt64Gaug InstrumentDescriptor instrument_descriptor = { std::string{name.data(), name.size()}, std::string{description.data(), description.size()}, std::string{unit.data(), unit.size()}, InstrumentType::kGauge, InstrumentValueType::kLong}; - auto storage = RegisterSyncMetricStorage(instrument_descriptor); + auto storage = RegisterSyncMetricStorage(instrument_descriptor); + auto enabled_state = GetInstrumentEnabledState(storage.get()); return opentelemetry::nostd::unique_ptr>{ - new LongGauge(instrument_descriptor, std::move(storage), meter_enabled_state_)}; + new LongGauge(instrument_descriptor, std::move(storage), std::move(enabled_state))}; } opentelemetry::nostd::unique_ptr> Meter::CreateDoubleGauge( @@ -322,9 +338,10 @@ opentelemetry::nostd::unique_ptr> Meter::CreateDoubleGaug InstrumentDescriptor instrument_descriptor = { std::string{name.data(), name.size()}, std::string{description.data(), description.size()}, std::string{unit.data(), unit.size()}, InstrumentType::kGauge, InstrumentValueType::kDouble}; - auto storage = RegisterSyncMetricStorage(instrument_descriptor); + auto storage = RegisterSyncMetricStorage(instrument_descriptor); + auto enabled_state = GetInstrumentEnabledState(storage.get()); return opentelemetry::nostd::unique_ptr>{ - new DoubleGauge(instrument_descriptor, std::move(storage), meter_enabled_state_)}; + new DoubleGauge(instrument_descriptor, std::move(storage), std::move(enabled_state))}; } #endif @@ -387,9 +404,10 @@ opentelemetry::nostd::unique_ptr> Meter::CreateI std::string{name.data(), name.size()}, std::string{description.data(), description.size()}, std::string{unit.data(), unit.size()}, InstrumentType::kUpDownCounter, InstrumentValueType::kLong}; - auto storage = RegisterSyncMetricStorage(instrument_descriptor); + auto storage = RegisterSyncMetricStorage(instrument_descriptor); + auto enabled_state = GetInstrumentEnabledState(storage.get()); return opentelemetry::nostd::unique_ptr>{ - new LongUpDownCounter(instrument_descriptor, std::move(storage), meter_enabled_state_)}; + new LongUpDownCounter(instrument_descriptor, std::move(storage), std::move(enabled_state))}; } opentelemetry::nostd::unique_ptr> Meter::CreateDoubleUpDownCounter( @@ -409,9 +427,10 @@ opentelemetry::nostd::unique_ptr> Meter::CreateDo std::string{name.data(), name.size()}, std::string{description.data(), description.size()}, std::string{unit.data(), unit.size()}, InstrumentType::kUpDownCounter, InstrumentValueType::kDouble}; - auto storage = RegisterSyncMetricStorage(instrument_descriptor); + auto storage = RegisterSyncMetricStorage(instrument_descriptor); + auto enabled_state = GetInstrumentEnabledState(storage.get()); return opentelemetry::nostd::unique_ptr>{ - new DoubleUpDownCounter(instrument_descriptor, std::move(storage), meter_enabled_state_)}; + new DoubleUpDownCounter(instrument_descriptor, std::move(storage), std::move(enabled_state))}; } opentelemetry::nostd::shared_ptr @@ -462,7 +481,7 @@ const sdk::instrumentationscope::InstrumentationScope *Meter::GetInstrumentation return scope_.get(); } -std::unique_ptr Meter::RegisterSyncMetricStorage( +std::unique_ptr Meter::RegisterSyncMetricStorage( InstrumentDescriptor &instrument_descriptor) { std::lock_guard guard(storage_lock_); @@ -476,7 +495,7 @@ std::unique_ptr Meter::RegisterSyncMetricStorage( } auto view_registry = ctx->GetViewRegistry(); - std::unique_ptr storages(new SyncMultiMetricStorage()); + std::unique_ptr storages(new SyncMultiMetricStorage()); #ifdef ENABLE_METRICS_EXEMPLAR_PREVIEW auto exemplar_filter_type = ctx->GetExemplarFilter(); @@ -533,8 +552,7 @@ std::unique_ptr Meter::RegisterSyncMetricStorage( view.GetAggregationConfig())); storage_registry_.insert({view_instr_desc, sync_storage}); } - auto sync_multi_storage = static_cast(storages.get()); - if (!sync_multi_storage->AddStorage(sync_storage)) + if (!storages->AddStorage(sync_storage)) { WarnOnViewSemanticError(GetInstrumentationScope(), instrument_descriptor, view_instr_desc, view); diff --git a/sdk/src/metrics/sync_instruments.cc b/sdk/src/metrics/sync_instruments.cc index e90e67e830..16c7afeb12 100644 --- a/sdk/src/metrics/sync_instruments.cc +++ b/sdk/src/metrics/sync_instruments.cc @@ -48,6 +48,9 @@ LongCounter::LongCounter(const InstrumentDescriptor &instrument_descriptor, std::shared_ptr meter_enabled_state) : Synchronous(instrument_descriptor, std::move(storage), std::move(meter_enabled_state)) { +#if OPENTELEMETRY_ABI_VERSION_NO >= 2 + SetEnabledFlag(meter_enabled_state_->Flag()); +#endif if (!storage_) { OTEL_INTERNAL_LOG_ERROR("[LongCounter::LongCounter] - Error constructing LongCounter." @@ -141,6 +144,9 @@ DoubleCounter::DoubleCounter(const InstrumentDescriptor &instrument_descriptor, std::shared_ptr meter_enabled_state) : Synchronous(instrument_descriptor, std::move(storage), std::move(meter_enabled_state)) { +#if OPENTELEMETRY_ABI_VERSION_NO >= 2 + SetEnabledFlag(meter_enabled_state_->Flag()); +#endif if (!storage_) { OTEL_INTERNAL_LOG_ERROR("[DoubleCounter::DoubleCounter] - Error constructing DoubleCounter." @@ -242,6 +248,9 @@ LongUpDownCounter::LongUpDownCounter(const InstrumentDescriptor &instrument_desc std::shared_ptr meter_enabled_state) : Synchronous(instrument_descriptor, std::move(storage), std::move(meter_enabled_state)) { +#if OPENTELEMETRY_ABI_VERSION_NO >= 2 + SetEnabledFlag(meter_enabled_state_->Flag()); +#endif if (!storage_) { OTEL_INTERNAL_LOG_ERROR( @@ -323,6 +332,9 @@ DoubleUpDownCounter::DoubleUpDownCounter(const InstrumentDescriptor &instrument_ std::shared_ptr meter_enabled_state) : Synchronous(instrument_descriptor, std::move(storage), std::move(meter_enabled_state)) { +#if OPENTELEMETRY_ABI_VERSION_NO >= 2 + SetEnabledFlag(meter_enabled_state_->Flag()); +#endif if (!storage_) { OTEL_INTERNAL_LOG_ERROR( @@ -405,6 +417,7 @@ LongGauge::LongGauge(const InstrumentDescriptor &instrument_descriptor, std::shared_ptr meter_enabled_state) : Synchronous(instrument_descriptor, std::move(storage), std::move(meter_enabled_state)) { + SetEnabledFlag(meter_enabled_state_->Flag()); if (!storage_) { OTEL_INTERNAL_LOG_ERROR("[LongGauge::LongGauge] - Error constructing LongGauge." @@ -482,6 +495,7 @@ DoubleGauge::DoubleGauge(const InstrumentDescriptor &instrument_descriptor, std::shared_ptr meter_enabled_state) : Synchronous(instrument_descriptor, std::move(storage), std::move(meter_enabled_state)) { + SetEnabledFlag(meter_enabled_state_->Flag()); if (!storage_) { OTEL_INTERNAL_LOG_ERROR("[DoubleGauge::DoubleGauge] - Error constructing DoubleUpDownCounter." @@ -559,6 +573,9 @@ LongHistogram::LongHistogram(const InstrumentDescriptor &instrument_descriptor, std::shared_ptr meter_enabled_state) : Synchronous(instrument_descriptor, std::move(storage), std::move(meter_enabled_state)) { +#if OPENTELEMETRY_ABI_VERSION_NO >= 2 + SetEnabledFlag(meter_enabled_state_->Flag()); +#endif if (!storage_) { OTEL_INTERNAL_LOG_ERROR("[LongHistogram::LongHistogram] - Error constructing LongHistogram." @@ -655,6 +672,9 @@ DoubleHistogram::DoubleHistogram(const InstrumentDescriptor &instrument_descript std::shared_ptr meter_enabled_state) : Synchronous(instrument_descriptor, std::move(storage), std::move(meter_enabled_state)) { +#if OPENTELEMETRY_ABI_VERSION_NO >= 2 + SetEnabledFlag(meter_enabled_state_->Flag()); +#endif if (!storage_) { OTEL_INTERNAL_LOG_ERROR( diff --git a/sdk/test/metrics/meter_provider_sdk_test.cc b/sdk/test/metrics/meter_provider_sdk_test.cc index 4b0f2d9c5d..43109cae91 100644 --- a/sdk/test/metrics/meter_provider_sdk_test.cc +++ b/sdk/test/metrics/meter_provider_sdk_test.cc @@ -863,6 +863,97 @@ TEST(MeterProvider, UpdateMeterConfiguratorAppliesToAllViewsOfAnInstrument) CollectCounterSums(reader, "scope.views")); } +#if OPENTELEMETRY_ABI_VERSION_NO >= 2 +TEST(MeterProvider, SyncInstrumentEnabledFollowsMeterConfig) +{ + MetricReader *reader{}; + auto provider = MakeProvider(reader); + ASSERT_NE(nullptr, reader); + + auto meter = provider->GetMeter("scope.enabled"); + std::vector> instruments; + instruments.emplace_back(meter->CreateUInt64Counter("uint64_counter").release()); + instruments.emplace_back(meter->CreateDoubleCounter("double_counter").release()); + instruments.emplace_back(meter->CreateUInt64Histogram("uint64_histogram").release()); + instruments.emplace_back(meter->CreateDoubleHistogram("double_histogram").release()); + instruments.emplace_back(meter->CreateInt64UpDownCounter("int64_up_down_counter").release()); + instruments.emplace_back(meter->CreateDoubleUpDownCounter("double_up_down_counter").release()); + instruments.emplace_back(meter->CreateInt64Gauge("int64_gauge").release()); + instruments.emplace_back(meter->CreateDoubleGauge("double_gauge").release()); + + for (const auto &instrument : instruments) + { + EXPECT_TRUE(instrument->Enabled()); + } + + provider->UpdateMeterConfigurator(DisableAll()); + for (const auto &instrument : instruments) + { + EXPECT_FALSE(instrument->Enabled()); + } + + provider->UpdateMeterConfigurator(EnableAll()); + for (const auto &instrument : instruments) + { + EXPECT_TRUE(instrument->Enabled()); + } +} + +TEST(MeterProvider, SyncInstrumentEnabledCreatedWhileMeterDisabled) +{ + MetricReader *reader{}; + auto provider = MakeProvider(reader, DisableAll()); + ASSERT_NE(nullptr, reader); + + auto counter = provider->GetMeter("scope.revived")->CreateUInt64Counter("counter"); + EXPECT_FALSE(counter->Enabled()); + + provider->UpdateMeterConfigurator(EnableAll()); + EXPECT_TRUE(counter->Enabled()); +} + +TEST(MeterProvider, SyncInstrumentEnabledFalseWhenAllViewsDrop) +{ + MetricReader *reader{}; + auto provider = MakeProvider(reader); + ASSERT_NE(nullptr, reader); + provider->AddView(std::unique_ptr( + new InstrumentSelector(InstrumentType::kCounter, "dropped", "")), + std::unique_ptr(new MeterSelector("scope.drop", "", "")), + std::unique_ptr(new View("", "", AggregationType::kDrop))); + + auto meter = provider->GetMeter("scope.drop"); + auto dropped = meter->CreateUInt64Counter("dropped"); + auto kept = meter->CreateUInt64Counter("kept"); + + EXPECT_FALSE(dropped->Enabled()); + EXPECT_TRUE(kept->Enabled()); + + // A dropped instrument stays disabled when its meter is enabled again. + provider->UpdateMeterConfigurator(DisableAll()); + provider->UpdateMeterConfigurator(EnableAll()); + EXPECT_FALSE(dropped->Enabled()); + EXPECT_TRUE(kept->Enabled()); +} + +TEST(MeterProvider, SyncInstrumentEnabledWhenAnyViewKeepsData) +{ + MetricReader *reader{}; + auto provider = MakeProvider(reader); + ASSERT_NE(nullptr, reader); + for (auto aggregation : {AggregationType::kDrop, AggregationType::kSum}) + { + provider->AddView(std::unique_ptr( + new InstrumentSelector(InstrumentType::kCounter, "counter", "")), + std::unique_ptr(new MeterSelector("scope.mixed", "", "")), + std::unique_ptr(new View("", "", aggregation))); + } + + auto counter = provider->GetMeter("scope.mixed")->CreateUInt64Counter("counter"); + EXPECT_TRUE(counter->Enabled()); +} +#endif + #if OPENTELEMETRY_ABI_VERSION_NO >= 2 && \ defined(OPENTELEMETRY_HAVE_METRICS_BOUND_INSTRUMENTS_PREVIEW) // A handle bound while the meter is disabled records once it is enabled, and its pending data is From 841ad2a0f751b17ff81122ae3c00942a3a9c1788 Mon Sep 17 00:00:00 2001 From: Aditya Bavadekar Date: Sat, 31 Oct 2026 23:42:33 +0530 Subject: [PATCH 2/3] [API/SDK] Address feedback on Enabled() - Give each instrument a disabled flag, keep copies pointing at the correct one - Treat instruments without storage as disabled - Benchmark Enabled() both when the meter is on and when it is off --- api/include/opentelemetry/metrics/noop.h | 22 ++------ .../opentelemetry/metrics/sync_instruments.h | 52 +++++++++++++------ api/test/metrics/noop_sync_instrument_test.cc | 37 +++++++++++++ sdk/src/metrics/meter.cc | 5 +- sdk/test/metrics/meter_provider_sdk_test.cc | 19 +++++++ .../metrics/sync_instruments_benchmark.cc | 42 +++++++++++++++ 6 files changed, 140 insertions(+), 37 deletions(-) diff --git a/api/include/opentelemetry/metrics/noop.h b/api/include/opentelemetry/metrics/noop.h index cd20e68df7..cab881b20b 100644 --- a/api/include/opentelemetry/metrics/noop.h +++ b/api/include/opentelemetry/metrics/noop.h @@ -39,11 +39,7 @@ class NoopCounter : public Counter NoopCounter(nostd::string_view /* name */, nostd::string_view /* description */, nostd::string_view /* unit */) noexcept - { -#if OPENTELEMETRY_ABI_VERSION_NO >= 2 - this->SetEnabledFlag(this->DisabledFlag()); -#endif - } + {} void Add(T /* value */) noexcept override {} void Add(T /* value */, const context::Context & /* context */) noexcept override {} void Add(T /* value */, const common::KeyValueIterable & /* attributes */) noexcept override {} @@ -67,11 +63,7 @@ class NoopHistogram : public Histogram NoopHistogram(nostd::string_view /* name */, nostd::string_view /* description */, nostd::string_view /* unit */) noexcept - { -#if OPENTELEMETRY_ABI_VERSION_NO >= 2 - this->SetEnabledFlag(this->DisabledFlag()); -#endif - } + {} void Record(T /* value */, const context::Context & /* context */) noexcept override {} void Record(T /* value */, const common::KeyValueIterable & /* attributes */, @@ -101,11 +93,7 @@ class NoopUpDownCounter : public UpDownCounter NoopUpDownCounter(nostd::string_view /* name */, nostd::string_view /* description */, nostd::string_view /* unit */) noexcept - { -#if OPENTELEMETRY_ABI_VERSION_NO >= 2 - this->SetEnabledFlag(this->DisabledFlag()); -#endif - } + {} void Add(T /* value */) noexcept override {} void Add(T /* value */, const context::Context & /* context */) noexcept override {} void Add(T /* value */, const common::KeyValueIterable & /* attributes */) noexcept override {} @@ -123,9 +111,7 @@ class NoopGauge : public Gauge NoopGauge(nostd::string_view /* name */, nostd::string_view /* description */, nostd::string_view /* unit */) noexcept - { - this->SetEnabledFlag(this->DisabledFlag()); - } + {} void Record(T /* value */) noexcept override {} void Record(T /* value */, const context::Context & /* context */) noexcept override {} void Record(T /* value */, const common::KeyValueIterable & /* attributes */) noexcept override {} diff --git a/api/include/opentelemetry/metrics/sync_instruments.h b/api/include/opentelemetry/metrics/sync_instruments.h index 8a2916a3f5..3d6855ae7e 100644 --- a/api/include/opentelemetry/metrics/sync_instruments.h +++ b/api/include/opentelemetry/metrics/sync_instruments.h @@ -22,12 +22,36 @@ namespace metrics class SynchronousInstrument { public: - SynchronousInstrument() = default; + SynchronousInstrument() = default; +#if OPENTELEMETRY_ABI_VERSION_NO >= 2 + // Copies share the SDK flag, but never point at the source's own flag, which is destroyed with + // the source. + SynchronousInstrument(const SynchronousInstrument &other) noexcept + : enabled_(other.EnabledFlagForCopy(*this)) + {} + SynchronousInstrument(SynchronousInstrument &&other) noexcept + : enabled_(other.EnabledFlagForCopy(*this)) + {} + SynchronousInstrument &operator=(const SynchronousInstrument &other) noexcept + { + if (this != &other) + { + enabled_ = other.EnabledFlagForCopy(*this); + } + return *this; + } + SynchronousInstrument &operator=(SynchronousInstrument &&other) noexcept + { + enabled_ = other.EnabledFlagForCopy(*this); + return *this; + } +#else SynchronousInstrument(const SynchronousInstrument &) = default; SynchronousInstrument(SynchronousInstrument &&) noexcept = default; SynchronousInstrument &operator=(const SynchronousInstrument &) = default; SynchronousInstrument &operator=(SynchronousInstrument &&) noexcept = default; - virtual ~SynchronousInstrument() = default; +#endif + virtual ~SynchronousInstrument() = default; #if OPENTELEMETRY_ABI_VERSION_NO >= 2 /** @@ -43,30 +67,24 @@ class SynchronousInstrument protected: /** - * Sets the flag read by Enabled(). SDKs point it at a flag they update; it must outlive this - * instrument. + * Points Enabled() at a flag owned by the SDK. The flag must outlive this instrument, and must + * only be changed with atomic writes such as OPENTELEMETRY_ATOMIC_WRITE_8, since Enabled() can + * read it concurrently. * * @since ABI_VERSION 2 */ void SetEnabledFlag(const bool *enabled) noexcept { enabled_ = enabled; } - /** A flag that is always false, for instruments that are never enabled. */ - static const bool *DisabledFlag() noexcept - { - static const bool kDisabled = false; - return &kDisabled; - } - private: - static const bool *EnabledFlag() noexcept + const bool *EnabledFlagForCopy(const SynchronousInstrument ©) const noexcept { - static const bool kEnabled = true; - return &kEnabled; + return enabled_ == &enabled_in_api_ ? ©.enabled_in_api_ : enabled_; } - // Variable to support implementation of Enabled method introduced in ABI V2. - // A pointer, so an SDK can flip many instruments with a single write. - const bool *enabled_ = EnabledFlag(); + // By default enabled_ points at enabled_in_api_, which is always false. An SDK points it at + // its own flag with SetEnabledFlag(). + bool enabled_in_api_ = false; + const bool *enabled_ = &enabled_in_api_; #endif }; diff --git a/api/test/metrics/noop_sync_instrument_test.cc b/api/test/metrics/noop_sync_instrument_test.cc index 1815a587eb..539a5484e1 100644 --- a/api/test/metrics/noop_sync_instrument_test.cc +++ b/api/test/metrics/noop_sync_instrument_test.cc @@ -8,6 +8,7 @@ #include #include +#include "opentelemetry/common/macros.h" #include "opentelemetry/context/context.h" #include "opentelemetry/metrics/noop.h" #include "opentelemetry/metrics/sync_instruments.h" @@ -67,4 +68,40 @@ TEST(SynchronousInstrument, NoopInstrumentsAreDisabled) EXPECT_FALSE(up_down_counter.Enabled()); EXPECT_FALSE(gauge.Enabled()); } + +namespace +{ +// An instrument reading an external flag, the way an SDK instrument does. +class FlagCounter : public opentelemetry::metrics::NoopCounter +{ +public: + explicit FlagCounter(const bool *flag) : NoopCounter("test", "none", "unitless") + { + SetEnabledFlag(flag); + } +}; +} // namespace + +// A copy must not read the default flag stored in the instrument it was copied from. +TEST(SynchronousInstrument, CopyOfDefaultInstrumentIsDisabled) +{ + std::unique_ptr> original{ + new opentelemetry::metrics::NoopCounter("test", "none", "unitless")}; + opentelemetry::metrics::NoopCounter copy(*original); + original.reset(); + EXPECT_FALSE(copy.Enabled()); +} + +// A copy keeps reading the external flag after the instrument it was copied from is destroyed. +TEST(SynchronousInstrument, CopySharesExternalFlag) +{ + bool flag = true; + std::unique_ptr original{new FlagCounter(&flag)}; + FlagCounter copy(*original); + original.reset(); + EXPECT_TRUE(copy.Enabled()); + + OPENTELEMETRY_ATOMIC_WRITE_8(&flag, false); + EXPECT_FALSE(copy.Enabled()); +} #endif diff --git a/sdk/src/metrics/meter.cc b/sdk/src/metrics/meter.cc index 897bd5c98e..33a454cde9 100644 --- a/sdk/src/metrics/meter.cc +++ b/sdk/src/metrics/meter.cc @@ -158,8 +158,9 @@ void Meter::UpdateMeterConfig(MeterConfig config) noexcept std::shared_ptr Meter::GetInstrumentEnabledState( const SyncMultiMetricStorage *storage) const noexcept { - // Every matching view uses the Drop aggregation, so the instrument never records. - if (storage != nullptr && storage->Empty()) + // Without storage the instrument never records: either the meter context is gone, or every + // matching view uses the Drop aggregation. + if (storage == nullptr || storage->Empty()) { return always_disabled_state_; } diff --git a/sdk/test/metrics/meter_provider_sdk_test.cc b/sdk/test/metrics/meter_provider_sdk_test.cc index 43109cae91..c2f7b7b6a1 100644 --- a/sdk/test/metrics/meter_provider_sdk_test.cc +++ b/sdk/test/metrics/meter_provider_sdk_test.cc @@ -1185,6 +1185,25 @@ TEST(MeterProvider, MeterWithExpiredContextIsDisabled) counter->Add(1); // Dropped: the meter is disabled. } +#if OPENTELEMETRY_ABI_VERSION_NO >= 2 +// An instrument created after its meter's context is gone has no storage, so it is disabled. +TEST(MeterProvider, SyncInstrumentEnabledFalseWithoutContext) +{ + ScopedTestLogHandler log_handler{LogLevel::Error}; + + auto context = std::make_shared(std::unique_ptr(new ViewRegistry()), + opentelemetry::sdk::resource::Resource::Create({}), + EnableAll()); + Meter meter{context, + opentelemetry::sdk::instrumentationscope::InstrumentationScope::Create("scope")}; + context.reset(); + + auto counter = meter.CreateUInt64Counter("counter"); + ASSERT_NE(nullptr, counter); + EXPECT_FALSE(counter->Enabled()); +} +#endif + TEST(MeterProvider, ConstructorsAreNotNoexcept) { static_assert(!noexcept(MeterProvider()), "MeterProvider construction must be allowed to throw"); diff --git a/sdk/test/metrics/sync_instruments_benchmark.cc b/sdk/test/metrics/sync_instruments_benchmark.cc index bd95ed0990..f72dfc5d14 100644 --- a/sdk/test/metrics/sync_instruments_benchmark.cc +++ b/sdk/test/metrics/sync_instruments_benchmark.cc @@ -440,6 +440,48 @@ void BM_Record_Counter_Sum_ByThreads(benchmark::State &state) } BENCHMARK(BM_Record_Counter_Sum_ByThreads)->ThreadRange(1, static_cast(GetBenchmarkThreads())); +#if OPENTELEMETRY_ABI_VERSION_NO > 1 +// Same as BM_Record_Counter_Sum_ByThreads, checking Enabled() before each Add(). +void BM_Record_Counter_Sum_CheckEnabled_ByThreads(benchmark::State &state) +{ + static BenchmarkProvider provider; + static auto counter = provider.meter->CreateDoubleCounter("benchmark_counter"); + const auto attributes = MakeAttributes(kNominalAttributeCount); + const AttributesView attributes_view(attributes); + const opentelemetry::context::Context context{}; + for (auto _ : state) + { + benchmark::DoNotOptimize(counter); + if (counter->Enabled()) + { + counter->Add(1.0, attributes_view, context); + } + } +} +BENCHMARK(BM_Record_Counter_Sum_CheckEnabled_ByThreads) + ->ThreadRange(1, static_cast(GetBenchmarkThreads())); + +// Same as BM_Record_Counter_Disabled_ByThreads, checking Enabled() before each Add(). +void BM_Record_Counter_Disabled_CheckEnabled_ByThreads(benchmark::State &state) +{ + static BenchmarkProvider provider(false); + static auto counter = provider.meter->CreateDoubleCounter("benchmark_counter"); + const auto attributes = MakeAttributes(kNominalAttributeCount); + const AttributesView attributes_view(attributes); + const opentelemetry::context::Context context{}; + for (auto _ : state) + { + benchmark::DoNotOptimize(counter); + if (counter->Enabled()) + { + counter->Add(1.0, attributes_view, context); + } + } +} +BENCHMARK(BM_Record_Counter_Disabled_CheckEnabled_ByThreads) + ->ThreadRange(1, static_cast(GetBenchmarkThreads())); +#endif // OPENTELEMETRY_ABI_VERSION_NO > 1 + void BM_Record_Counter_Sum_ByAttributes(benchmark::State &state) { BenchmarkProvider provider; From 79f382d0fed5493aeaec332ea58741ac808227af Mon Sep 17 00:00:00 2001 From: Marc Alff Date: Sat, 31 Oct 2026 22:11:18 +0100 Subject: [PATCH 3/3] Apply batched suggestions from code review Co-authored-by: Marc Alff --- api/test/metrics/noop_sync_instrument_test.cc | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/api/test/metrics/noop_sync_instrument_test.cc b/api/test/metrics/noop_sync_instrument_test.cc index 539a5484e1..4379783e96 100644 --- a/api/test/metrics/noop_sync_instrument_test.cc +++ b/api/test/metrics/noop_sync_instrument_test.cc @@ -8,13 +8,16 @@ #include #include -#include "opentelemetry/common/macros.h" #include "opentelemetry/context/context.h" #include "opentelemetry/metrics/noop.h" #include "opentelemetry/metrics/sync_instruments.h" #include "opentelemetry/nostd/string_view.h" #include "opentelemetry/nostd/utility.h" +#if OPENTELEMETRY_ABI_VERSION_NO >= 2 +# include "opentelemetry/common/macros.h" +#endif + TEST(Counter, Add) { std::shared_ptr> counter{