diff --git a/CHANGELOG.md b/CHANGELOG.md index 3677666955..606d0ea613 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) + * [SDK] `BatchLogRecordProcessor` now exports at most one batch on normal wakeups instead of continuously draining the buffer, matching `BatchSpanProcessor` 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/sync_instruments.h b/api/include/opentelemetry/metrics/sync_instruments.h index 7f709129bb..3d6855ae7e 100644 --- a/api/include/opentelemetry/metrics/sync_instruments.h +++ b/api/include/opentelemetry/metrics/sync_instruments.h @@ -22,12 +22,70 @@ 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 + /** + * 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: + /** + * 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; } + +private: + const bool *EnabledFlagForCopy(const SynchronousInstrument ©) const noexcept + { + return enabled_ == &enabled_in_api_ ? ©.enabled_in_api_ : enabled_; + } + + // 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 }; #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..4379783e96 100644 --- a/api/test/metrics/noop_sync_instrument_test.cc +++ b/api/test/metrics/noop_sync_instrument_test.cc @@ -14,6 +14,10 @@ #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{ @@ -53,3 +57,54 @@ 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()); +} + +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/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..33a454cde9 100644 --- a/sdk/src/metrics/meter.cc +++ b/sdk/src/metrics/meter.cc @@ -155,6 +155,18 @@ void Meter::UpdateMeterConfig(MeterConfig config) noexcept meter_enabled_state_->SetEnabled(config.IsEnabled()); } +std::shared_ptr Meter::GetInstrumentEnabledState( + const SyncMultiMetricStorage *storage) const noexcept +{ + // 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_; + } + return meter_enabled_state_; +} + opentelemetry::nostd::unique_ptr> Meter::CreateUInt64Counter( opentelemetry::nostd::string_view name, opentelemetry::nostd::string_view description, @@ -171,9 +183,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 +206,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 +271,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 +294,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 +317,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 +339,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 +405,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 +428,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 +482,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 +496,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 +553,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..c2f7b7b6a1 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 @@ -1094,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;