From 064fef0d871c57ffac6739d3311659a5770a9db4 Mon Sep 17 00:00:00 2001 From: Tom Tan Date: Wed, 29 Nov 2023 09:45:09 -0800 Subject: [PATCH] [Metrics] Make context optional for histogram instruments in Metrics SDK (#2416) --- api/include/opentelemetry/metrics/noop.h | 7 + .../opentelemetry/metrics/sync_instruments.h | 99 +++++-- .../sdk/metrics/sync_instruments.h | 122 +++----- sdk/src/metrics/meter.cc | 4 +- sdk/src/metrics/sync_instruments.cc | 265 ++++++++++++++++-- sdk/test/metrics/sync_instruments_test.cc | 17 +- 6 files changed, 375 insertions(+), 139 deletions(-) diff --git a/api/include/opentelemetry/metrics/noop.h b/api/include/opentelemetry/metrics/noop.h index f20e855cb7..d34a0e681b 100644 --- a/api/include/opentelemetry/metrics/noop.h +++ b/api/include/opentelemetry/metrics/noop.h @@ -44,6 +44,13 @@ class NoopHistogram : public Histogram const common::KeyValueIterable & /* attributes */, const context::Context & /* context */) noexcept override {} +#if OPENTELEMETRY_ABI_VERSION_NO >= 2 + void Record(T /*value*/, + const opentelemetry::common::KeyValueIterable & /*attributes*/) noexcept override + {} + + void Record(T /*value*/) noexcept override {} +#endif }; template diff --git a/api/include/opentelemetry/metrics/sync_instruments.h b/api/include/opentelemetry/metrics/sync_instruments.h index b26e527c2c..4471677433 100644 --- a/api/include/opentelemetry/metrics/sync_instruments.h +++ b/api/include/opentelemetry/metrics/sync_instruments.h @@ -22,31 +22,43 @@ class SynchronousInstrument virtual ~SynchronousInstrument() = default; }; +/* A Counter instrument that adds values. */ template class Counter : public SynchronousInstrument { public: /** - * Add adds the value to the counter's sum + * Record a value * * @param value The increment amount. MUST be non-negative. */ virtual void Add(T value) noexcept = 0; + /** + * Record a value + * + * @param value The increment amount. MUST be non-negative. + * @param context The explicit context to associate with this measurement. + */ virtual void Add(T value, const context::Context &context) noexcept = 0; /** - * Add adds the value to the counter's sum. The attributes should contain - * the keys and values to be associated with this value. Counters only - * accept positive valued updates. + * Record a value with a set of attributes. * * @param value The increment amount. MUST be non-negative. - * @param attributes the set of attributes, as key-value pairs + * @param attributes A set of attributes to associate with the value. */ virtual void Add(T value, const common::KeyValueIterable &attributes) noexcept = 0; + /** + * Record a value with a set of attributes. + * + * @param value The increment amount. MUST be non-negative. + * @param attributes A set of attributes to associate with the value. + * @param context The explicit context to associate with this measurement. + */ virtual void Add(T value, const common::KeyValueIterable &attributes, const context::Context &context) noexcept = 0; @@ -55,8 +67,7 @@ class Counter : public SynchronousInstrument nostd::enable_if_t::value> * = nullptr> void Add(T value, const U &attributes) noexcept { - auto context = context::Context{}; - this->Add(value, common::KeyValueIterableView{attributes}, context); + this->Add(value, common::KeyValueIterableView{attributes}); } template > attributes) noexcept { - auto context = context::Context{}; - this->Add(value, - nostd::span>{ - attributes.begin(), attributes.end()}, - context); + this->Add(value, nostd::span>{ + attributes.begin(), attributes.end()}); } void Add(T value, @@ -94,18 +102,54 @@ template class Histogram : public SynchronousInstrument { public: +#if OPENTELEMETRY_ABI_VERSION_NO >= 2 /** + * @since ABI_VERSION 2 * Records a value. * * @param value The measurement value. MUST be non-negative. */ + virtual void Record(T value) noexcept = 0; + + /** + * @since ABI_VERSION 2 + * Records a value with a set of attributes. + * + * @param value The measurement value. MUST be non-negative. + * @param attribute A set of attributes to associate with the value. + */ + virtual void Record(T value, const common::KeyValueIterable &attribute) noexcept = 0; + + template ::value> * = nullptr> + void Record(T value, const U &attributes) noexcept + { + this->Record(value, common::KeyValueIterableView{attributes}); + } + + void Record(T value, + std::initializer_list> + attributes) noexcept + { + this->Record(value, nostd::span>{ + attributes.begin(), attributes.end()}); + } +#endif + + /** + * Records a value. + * + * @param value The measurement value. MUST be non-negative. + * @param context The explicit context to associate with this measurement. + */ virtual void Record(T value, const context::Context &context) noexcept = 0; /** * Records a value with a set of attributes. * * @param value The measurement value. MUST be non-negative. - * @param attributes A set of attributes to associate with the count. + * @param attributes A set of attributes to associate with the value.. + * @param context The explicit context to associate with this measurement. */ virtual void Record(T value, const common::KeyValueIterable &attributes, @@ -137,22 +181,35 @@ class UpDownCounter : public SynchronousInstrument { public: /** - * Adds a value. + * Record a value. * - * @param value The amount of the measurement. + * @param value The increment amount. May be positive, negative or zero. */ virtual void Add(T value) noexcept = 0; + /** + * Record a value. + * + * @param value The increment amount. May be positive, negative or zero. + * @param context The explicit context to associate with this measurement. + */ virtual void Add(T value, const context::Context &context) noexcept = 0; /** - * Add a value with a set of attributes. + * Record a value with a set of attributes. * * @param value The increment amount. May be positive, negative or zero. * @param attributes A set of attributes to associate with the count. */ virtual void Add(T value, const common::KeyValueIterable &attributes) noexcept = 0; + /** + * Record a value with a set of attributes. + * + * @param value The increment amount. May be positive, negative or zero. + * @param attributes A set of attributes to associate with the count. + * @param context The explicit context to associate with this measurement. + */ virtual void Add(T value, const common::KeyValueIterable &attributes, const context::Context &context) noexcept = 0; @@ -161,8 +218,7 @@ class UpDownCounter : public SynchronousInstrument nostd::enable_if_t::value> * = nullptr> void Add(T value, const U &attributes) noexcept { - auto context = context::Context{}; - this->Add(value, common::KeyValueIterableView{attributes}, context); + this->Add(value, common::KeyValueIterableView{attributes}); } template > attributes) noexcept { - auto context = context::Context{}; - this->Add(value, - nostd::span>{ - attributes.begin(), attributes.end()}, - context); + this->Add(value, nostd::span>{ + attributes.begin(), attributes.end()}); } void Add(T value, diff --git a/sdk/include/opentelemetry/sdk/metrics/sync_instruments.h b/sdk/include/opentelemetry/sdk/metrics/sync_instruments.h index 825547a846..f08b629fe2 100644 --- a/sdk/include/opentelemetry/sdk/metrics/sync_instruments.h +++ b/sdk/include/opentelemetry/sdk/metrics/sync_instruments.h @@ -33,61 +33,22 @@ class Synchronous std::unique_ptr storage_; }; -template -class LongCounter : public Synchronous, public opentelemetry::metrics::Counter +class LongCounter : public Synchronous, public opentelemetry::metrics::Counter { public: LongCounter(InstrumentDescriptor instrument_descriptor, - std::unique_ptr storage) - : Synchronous(instrument_descriptor, std::move(storage)) - { - if (!storage_) - { - OTEL_INTERNAL_LOG_ERROR("[LongCounter::LongCounter] - Error during constructing LongCounter." - << "The metric storage is invalid" - << "No value will be added"); - } - } - - void Add(T value, const opentelemetry::common::KeyValueIterable &attributes) noexcept override - { - if (!storage_) - { - return; - } - auto context = opentelemetry::context::Context{}; - return storage_->RecordLong(value, attributes, context); - } - - void Add(T value, + std::unique_ptr storage); + + void Add(uint64_t value, + const opentelemetry::common::KeyValueIterable &attributes) noexcept override; + + void Add(uint64_t value, const opentelemetry::common::KeyValueIterable &attributes, - const opentelemetry::context::Context &context) noexcept override - { - if (!storage_) - { - return; - } - return storage_->RecordLong(value, attributes, context); - } - - void Add(T value) noexcept override - { - auto context = opentelemetry::context::Context{}; - if (!storage_) - { - return; - } - return storage_->RecordLong(value, context); - } - - void Add(T value, const opentelemetry::context::Context &context) noexcept override - { - if (!storage_) - { - return; - } - return storage_->RecordLong(value, context); - } + const opentelemetry::context::Context &context) noexcept override; + + void Add(uint64_t value) noexcept override; + + void Add(uint64_t value, const opentelemetry::context::Context &context) noexcept override; }; class DoubleCounter : public Synchronous, public opentelemetry::metrics::Counter @@ -139,48 +100,24 @@ class DoubleUpDownCounter : public Synchronous, public opentelemetry::metrics::U void Add(double value, const opentelemetry::context::Context &context) noexcept override; }; -template -class LongHistogram : public Synchronous, public opentelemetry::metrics::Histogram +class LongHistogram : public Synchronous, public opentelemetry::metrics::Histogram { public: LongHistogram(InstrumentDescriptor instrument_descriptor, - std::unique_ptr storage) - : Synchronous(instrument_descriptor, std::move(storage)) - { - if (!storage_) - { - OTEL_INTERNAL_LOG_ERROR( - "[LongHistogram::LongHistogram] - Error during constructing LongHistogram." - << "The metric storage is invalid" - << "No value will be added"); - } - } - - void Record(T value, + std::unique_ptr storage); + +#if OPENTELEMETRY_ABI_VERSION_NO >= 2 + void Record(uint64_t value, + const opentelemetry::common::KeyValueIterable &attributes) noexcept override; + + void Record(uint64_t value) noexcept override; +#endif + + void Record(uint64_t value, const opentelemetry::common::KeyValueIterable &attributes, - const opentelemetry::context::Context &context) noexcept override - { - if (value < 0) - { - OTEL_INTERNAL_LOG_WARN( - "[LongHistogram::Record(value, attributes)] negative value provided to histogram Name:" - << instrument_descriptor_.name_ << " Value:" << value); - return; - } - return storage_->RecordLong(value, attributes, context); - } - - void Record(T value, const opentelemetry::context::Context &context) noexcept override - { - if (value < 0) - { - OTEL_INTERNAL_LOG_WARN( - "[LongHistogram::Record(value)] negative value provided to histogram Name:" - << instrument_descriptor_.name_ << " Value:" << value); - return; - } - return storage_->RecordLong(value, context); - } + const opentelemetry::context::Context &context) noexcept override; + + void Record(uint64_t value, const opentelemetry::context::Context &context) noexcept override; }; class DoubleHistogram : public Synchronous, public opentelemetry::metrics::Histogram @@ -189,6 +126,13 @@ class DoubleHistogram : public Synchronous, public opentelemetry::metrics::Histo DoubleHistogram(InstrumentDescriptor instrument_descriptor, std::unique_ptr storage); +#if OPENTELEMETRY_ABI_VERSION_NO >= 2 + void Record(double value, + const opentelemetry::common::KeyValueIterable &attributes) noexcept override; + + void Record(double value) 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/meter.cc b/sdk/src/metrics/meter.cc index 4f7c4c1207..0ae3776111 100644 --- a/sdk/src/metrics/meter.cc +++ b/sdk/src/metrics/meter.cc @@ -52,7 +52,7 @@ nostd::unique_ptr> Meter::CreateUInt64Counter( std::string{unit.data(), unit.size()}, InstrumentType::kCounter, InstrumentValueType::kLong}; auto storage = RegisterSyncMetricStorage(instrument_descriptor); return nostd::unique_ptr>( - new LongCounter(instrument_descriptor, std::move(storage))); + new LongCounter(instrument_descriptor, std::move(storage))); } nostd::unique_ptr> Meter::CreateDoubleCounter( @@ -138,7 +138,7 @@ nostd::unique_ptr> Meter::CreateUInt64Histogram( InstrumentValueType::kLong}; auto storage = RegisterSyncMetricStorage(instrument_descriptor); return nostd::unique_ptr>{ - new LongHistogram(instrument_descriptor, std::move(storage))}; + new LongHistogram(instrument_descriptor, std::move(storage))}; } nostd::unique_ptr> Meter::CreateDoubleHistogram( diff --git a/sdk/src/metrics/sync_instruments.cc b/sdk/src/metrics/sync_instruments.cc index 6fe72b55bf..bdeff150ae 100644 --- a/sdk/src/metrics/sync_instruments.cc +++ b/sdk/src/metrics/sync_instruments.cc @@ -10,27 +10,93 @@ namespace sdk { namespace metrics { +LongCounter::LongCounter(InstrumentDescriptor instrument_descriptor, + std::unique_ptr storage) + : Synchronous(instrument_descriptor, std::move(storage)) +{ + if (!storage_) + { + OTEL_INTERNAL_LOG_ERROR("[LongCounter::LongCounter] - Error constructing LongCounter." + << "The metric storage is invalid for " << instrument_descriptor.name_); + } +} + +void LongCounter::Add(uint64_t value, + const opentelemetry::common::KeyValueIterable &attributes) noexcept +{ + auto context = opentelemetry::context::Context{}; + if (!storage_) + { + OTEL_INTERNAL_LOG_WARN("[LongCounter::Add(V,A)] Value not recorded - invalid storage for: " + << instrument_descriptor_.name_); + return; + } + return storage_->RecordLong(value, attributes, context); +} + +void LongCounter::Add(uint64_t value, + const opentelemetry::common::KeyValueIterable &attributes, + const opentelemetry::context::Context &context) noexcept +{ + if (!storage_) + { + OTEL_INTERNAL_LOG_WARN("[LongCounter::Add(V,A,C)] Value not recorded - invalid storage for: " + << instrument_descriptor_.name_); + return; + } + return storage_->RecordLong(value, attributes, context); +} + +void LongCounter::Add(uint64_t value) noexcept +{ + auto context = opentelemetry::context::Context{}; + if (!storage_) + { + OTEL_INTERNAL_LOG_WARN("[LongCounter::Add(V)] Value not recorded - invalid storage for: " + << instrument_descriptor_.name_); + return; + } + return storage_->RecordLong(value, context); +} + +void LongCounter::Add(uint64_t value, const opentelemetry::context::Context &context) noexcept +{ + if (!storage_) + { + OTEL_INTERNAL_LOG_WARN("[LongCounter::Add(V,C)] Value not recorded - invalid storage for: " + << instrument_descriptor_.name_); + return; + } + return storage_->RecordLong(value, context); +} + DoubleCounter::DoubleCounter(InstrumentDescriptor instrument_descriptor, std::unique_ptr storage) : Synchronous(instrument_descriptor, std::move(storage)) { if (!storage_) { - OTEL_INTERNAL_LOG_ERROR( - "[DoubleCounter::DoubleCounter] - Error during constructing DoubleCounter." - << "The metric storage is invalid" - << "No value will be added"); + OTEL_INTERNAL_LOG_ERROR("[DoubleCounter::DoubleCounter] - Error constructing DoubleCounter." + << "The metric storage is invalid for " << instrument_descriptor.name_); } } void DoubleCounter::Add(double value, const opentelemetry::common::KeyValueIterable &attributes) noexcept { - auto context = opentelemetry::context::Context{}; + if (value < 0) + { + OTEL_INTERNAL_LOG_WARN("[DoubleCounter::Add(V,A)] Value not recorded - negative value for: " + << instrument_descriptor_.name_); + return; + } if (!storage_) { + OTEL_INTERNAL_LOG_WARN("[DoubleCounter::Add(V,A)] Value not recorded - invalid storage for: " + << instrument_descriptor_.name_); return; } + auto context = opentelemetry::context::Context{}; return storage_->RecordDouble(value, attributes, context); } @@ -38,8 +104,16 @@ void DoubleCounter::Add(double value, const opentelemetry::common::KeyValueIterable &attributes, const opentelemetry::context::Context &context) noexcept { + if (value < 0) + { + OTEL_INTERNAL_LOG_WARN("[DoubleCounter::Add(V,A,C)] Value not recorded - negative value for: " + << instrument_descriptor_.name_); + return; + } if (!storage_) { + OTEL_INTERNAL_LOG_WARN("[DoubleCounter::Add(V,A,C)] Value not recorded - invalid storage for: " + << instrument_descriptor_.name_); return; } return storage_->RecordDouble(value, attributes, context); @@ -47,18 +121,34 @@ void DoubleCounter::Add(double value, void DoubleCounter::Add(double value) noexcept { - auto context = opentelemetry::context::Context{}; + if (value < 0) + { + OTEL_INTERNAL_LOG_WARN("[DoubleCounter::Add(V)] Value not recorded - negative value for: " + << instrument_descriptor_.name_); + return; + } if (!storage_) { + OTEL_INTERNAL_LOG_WARN("[DoubleCounter::Add(V)] Value not recorded - invalid storage for: " + << instrument_descriptor_.name_); return; } + auto context = opentelemetry::context::Context{}; return storage_->RecordDouble(value, context); } void DoubleCounter::Add(double value, const opentelemetry::context::Context &context) noexcept { + if (value < 0) + { + OTEL_INTERNAL_LOG_WARN("[DoubleCounter::Add(V)] Value not recorded - negative value for: " + << instrument_descriptor_.name_); + return; + } if (!storage_) { + OTEL_INTERNAL_LOG_WARN("[DoubleCounter::Add(V,C)] Value not recorded - invalid storage for: " + << instrument_descriptor_.name_); return; } return storage_->RecordDouble(value, context); @@ -71,9 +161,8 @@ LongUpDownCounter::LongUpDownCounter(InstrumentDescriptor instrument_descriptor, if (!storage_) { OTEL_INTERNAL_LOG_ERROR( - "[LongUpDownCounter::LongUpDownCounter] - Error during constructing LongUpDownCounter." - << "The metric storage is invalid" - << "No value will be added"); + "[LongUpDownCounter::LongUpDownCounter] - Error constructing LongUpDownCounter." + << "The metric storage is invalid for " << instrument_descriptor.name_); } } @@ -83,6 +172,9 @@ void LongUpDownCounter::Add(int64_t value, auto context = opentelemetry::context::Context{}; if (!storage_) { + OTEL_INTERNAL_LOG_WARN( + "[LongUpDownCounter::Add(V,A)] Value not recorded - invalid storage for: " + << instrument_descriptor_.name_); return; } return storage_->RecordLong(value, attributes, context); @@ -94,6 +186,9 @@ void LongUpDownCounter::Add(int64_t value, { if (!storage_) { + OTEL_INTERNAL_LOG_WARN( + "[LongUpDownCounter::Add(V,A,C)] Value not recorded - invalid storage for: " + << instrument_descriptor_.name_); return; } return storage_->RecordLong(value, attributes, context); @@ -104,6 +199,8 @@ void LongUpDownCounter::Add(int64_t value) noexcept auto context = opentelemetry::context::Context{}; if (!storage_) { + OTEL_INTERNAL_LOG_WARN("[LongUpDownCounter::Add(V)] Value not recorded - invalid storage for: " + << instrument_descriptor_.name_); return; } return storage_->RecordLong(value, context); @@ -113,6 +210,9 @@ void LongUpDownCounter::Add(int64_t value, const opentelemetry::context::Context { if (!storage_) { + OTEL_INTERNAL_LOG_WARN( + "[LongUpDownCounter::Add(V,C)] Value not recorded - invalid storage for: " + << instrument_descriptor_.name_); return; } return storage_->RecordLong(value, context); @@ -125,16 +225,20 @@ DoubleUpDownCounter::DoubleUpDownCounter(InstrumentDescriptor instrument_descrip if (!storage_) { OTEL_INTERNAL_LOG_ERROR( - "[DoubleUpDownCounter::DoubleUpDownCounter] - Error during constructing " - "DoubleUpDownCounter." - << "The metric storage is invalid" - << "No value will be added"); + "[DoubleUpDownCounter::DoubleUpDownCounter] - Error constructing DoubleUpDownCounter." + << "The metric storage is invalid for " << instrument_descriptor.name_); } } void DoubleUpDownCounter::Add(double value, const opentelemetry::common::KeyValueIterable &attributes) noexcept { + if (!storage_) + { + OTEL_INTERNAL_LOG_WARN( + "[DoubleUpDownCounter::Add(V,A)] Value not recorded - invalid storage for: " + << instrument_descriptor_.name_); + } auto context = opentelemetry::context::Context{}; return storage_->RecordDouble(value, attributes, context); } @@ -145,6 +249,9 @@ void DoubleUpDownCounter::Add(double value, { if (!storage_) { + OTEL_INTERNAL_LOG_WARN( + "[DoubleUpDownCounter::Add(V,A,C)] Value not recorded - invalid storage for: " + << instrument_descriptor_.name_); return; } return storage_->RecordDouble(value, attributes, context); @@ -154,6 +261,9 @@ void DoubleUpDownCounter::Add(double value) noexcept { if (!storage_) { + OTEL_INTERNAL_LOG_WARN( + "[DoubleUpDownCounter::Add(V)] Value not recorded - invalid storage for: " + << instrument_descriptor_.name_); return; } auto context = opentelemetry::context::Context{}; @@ -164,11 +274,77 @@ void DoubleUpDownCounter::Add(double value, const opentelemetry::context::Contex { if (!storage_) { + OTEL_INTERNAL_LOG_WARN( + "[DoubleUpDownCounter::Add(V,C)] Value not recorded - invalid storage for: " + << instrument_descriptor_.name_); return; } return storage_->RecordDouble(value, context); } +LongHistogram::LongHistogram(InstrumentDescriptor instrument_descriptor, + std::unique_ptr storage) + : Synchronous(instrument_descriptor, std::move(storage)) +{ + if (!storage_) + { + OTEL_INTERNAL_LOG_ERROR("[LongHistogram::LongHistogram] - Error constructing LongHistogram." + << "The metric storage is invalid for " << instrument_descriptor.name_); + } +} + +void LongHistogram::Record(uint64_t value, + const opentelemetry::common::KeyValueIterable &attributes, + const opentelemetry::context::Context &context) noexcept +{ + if (!storage_) + { + OTEL_INTERNAL_LOG_WARN( + "[LongHistogram::Record(V,A,C)] Value not recorded - invalid storage for: " + << instrument_descriptor_.name_); + return; + } + return storage_->RecordLong(value, attributes, context); +} + +void LongHistogram::Record(uint64_t value, const opentelemetry::context::Context &context) noexcept +{ + if (!storage_) + { + OTEL_INTERNAL_LOG_WARN("[LongHistogram::Record(V,C)] Value not recorded - invalid storage for: " + << instrument_descriptor_.name_); + return; + } + return storage_->RecordLong(value, context); +} + +#if OPENTELEMETRY_ABI_VERSION_NO >= 2 +void LongHistogram::Record(uint64_t value, + const opentelemetry::common::KeyValueIterable &attributes) noexcept +{ + if (!storage_) + { + OTEL_INTERNAL_LOG_WARN("[LongHistogram::Record(V,A)] Value not recorded - invalid storage for: " + << instrument_descriptor_.name_); + return; + } + auto context = opentelemetry::context::Context{}; + return storage_->RecordLong(value, attributes, context); +} + +void LongHistogram::Record(uint64_t value) noexcept +{ + if (!storage_) + { + OTEL_INTERNAL_LOG_WARN("[LongHistogram::Record(V)] Value not recorded - invalid storage for: " + << instrument_descriptor_.name_); + return; + } + auto context = opentelemetry::context::Context{}; + return storage_->RecordLong(value, context); +} +#endif + DoubleHistogram::DoubleHistogram(InstrumentDescriptor instrument_descriptor, std::unique_ptr storage) : Synchronous(instrument_descriptor, std::move(storage)) @@ -176,9 +352,8 @@ DoubleHistogram::DoubleHistogram(InstrumentDescriptor instrument_descriptor, if (!storage_) { OTEL_INTERNAL_LOG_ERROR( - "[DoubleHistogram::DoubleHistogram] - Error during constructing DoubleHistogram." - << "The metric storage is invalid" - << "No value will be added"); + "[DoubleHistogram::DoubleHistogram] - Error constructing DoubleHistogram." + << "The metric storage is invalid for " << instrument_descriptor.name_); } } @@ -186,15 +361,17 @@ void DoubleHistogram::Record(double value, const opentelemetry::common::KeyValueIterable &attributes, const opentelemetry::context::Context &context) noexcept { - if (!storage_) + if (value < 0) { + OTEL_INTERNAL_LOG_WARN( + "[DoubleHistogram::Record(V,A,C)] Value not recorded - negative value for: " + << instrument_descriptor_.name_); return; } - if (value < 0 || std::isnan(value) || std::isinf(value)) + if (!storage_) { OTEL_INTERNAL_LOG_WARN( - "[DoubleHistogram::Record(value, attributes)] negative/nan/infinite value provided to " - "histogram Name:" + "[DoubleHistogram::Record(V,A,C)] Value not recorded - invalid storage for: " << instrument_descriptor_.name_); return; } @@ -203,19 +380,63 @@ void DoubleHistogram::Record(double value, void DoubleHistogram::Record(double value, const opentelemetry::context::Context &context) noexcept { + if (value < 0) + { + OTEL_INTERNAL_LOG_WARN( + "[DoubleHistogram::Record(V,C)] Value not recorded - negative value for: " + << instrument_descriptor_.name_); + return; + } if (!storage_) { + OTEL_INTERNAL_LOG_WARN( + "[DoubleHistogram::Record(V,C)] Value not recorded - invalid storage for: " + << instrument_descriptor_.name_); + return; + } + return storage_->RecordDouble(value, context); +} + +#if OPENTELEMETRY_ABI_VERSION_NO >= 2 +void DoubleHistogram::Record(double value, + const opentelemetry::common::KeyValueIterable &attributes) noexcept +{ + if (value < 0) + { + OTEL_INTERNAL_LOG_WARN( + "[DoubleHistogram::Record(V,A)] Value not recorded - negative value for: " + << instrument_descriptor_.name_); return; } - if (value < 0 || std::isnan(value) || std::isinf(value)) + if (!storage_) { OTEL_INTERNAL_LOG_WARN( - "[DoubleHistogram::Record(value)] negative/nan/infinite value provided to histogram Name:" + "[DoubleHistogram::Record(V,A)] Value not recorded - invalid storage for: " << instrument_descriptor_.name_); return; } + auto context = opentelemetry::context::Context{}; + return storage_->RecordDouble(value, attributes, context); +} + +void DoubleHistogram::Record(double value) noexcept +{ + if (value < 0) + { + OTEL_INTERNAL_LOG_WARN("[DoubleHistogram::Record(V)] Value not recorded - negative value for: " + << instrument_descriptor_.name_); + return; + } + if (!storage_) + { + OTEL_INTERNAL_LOG_WARN("[DoubleHistogram::Record(V)] Value not recorded - invalid storage for: " + << instrument_descriptor_.name_); + return; + } + auto context = opentelemetry::context::Context{}; return storage_->RecordDouble(value, context); } +#endif } // namespace metrics } // namespace sdk diff --git a/sdk/test/metrics/sync_instruments_test.cc b/sdk/test/metrics/sync_instruments_test.cc index a1ff2325e2..723d29f20e 100644 --- a/sdk/test/metrics/sync_instruments_test.cc +++ b/sdk/test/metrics/sync_instruments_test.cc @@ -24,7 +24,7 @@ TEST(SyncInstruments, LongCounter) InstrumentDescriptor instrument_descriptor = { "long_counter", "description", "1", InstrumentType::kCounter, InstrumentValueType::kLong}; std::unique_ptr metric_storage(new SyncMultiMetricStorage()); - LongCounter counter(instrument_descriptor, std::move(metric_storage)); + LongCounter counter(instrument_descriptor, std::move(metric_storage)); counter.Add(10); counter.Add(10, opentelemetry::context::Context{}); @@ -71,6 +71,18 @@ TEST(SyncInstruments, LongUpDownCounter) counter.Add(10, opentelemetry::common::KeyValueIterableView({})); counter.Add(10, opentelemetry::common::KeyValueIterableView({}), opentelemetry::context::Context{}); + + // negative values + counter.Add(-10); + counter.Add(-10, opentelemetry::context::Context{}); + + counter.Add(-10, + opentelemetry::common::KeyValueIterableView({{"abc", "123"}, {"xyz", "456"}})); + counter.Add(-10, opentelemetry::common::KeyValueIterableView({{"abc", "123"}, {"xyz", "456"}}), + opentelemetry::context::Context{}); + counter.Add(-10, opentelemetry::common::KeyValueIterableView({})); + counter.Add(-10, opentelemetry::common::KeyValueIterableView({}), + opentelemetry::context::Context{}); } TEST(SyncInstruments, DoubleUpDownCounter) @@ -98,9 +110,8 @@ TEST(SyncInstruments, LongHistogram) InstrumentDescriptor instrument_descriptor = { "long_histogram", "description", "1", InstrumentType::kHistogram, InstrumentValueType::kLong}; std::unique_ptr metric_storage(new SyncMultiMetricStorage()); - LongHistogram histogram(instrument_descriptor, std::move(metric_storage)); + LongHistogram histogram(instrument_descriptor, std::move(metric_storage)); histogram.Record(10, opentelemetry::context::Context{}); - histogram.Record(-10, opentelemetry::context::Context{}); // This is ignored histogram.Record(10, opentelemetry::common::KeyValueIterableView({{"abc", "123"}, {"xyz", "456"}}),