From 2e1e696740f2c655bffc9ef93f278c94247c7c92 Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Thu, 28 Sep 2023 07:15:13 +0000 Subject: [PATCH 01/13] initial commit --- api/include/opentelemetry/metrics/meter.h | 21 ++++++++++ .../opentelemetry/metrics/sync_instruments.h | 42 +++++++++++++++++++ sdk/include/opentelemetry/sdk/metrics/meter.h | 10 +++++ .../sdk/metrics/sync_instruments.h | 31 ++++++++++++++ 4 files changed, 104 insertions(+) diff --git a/api/include/opentelemetry/metrics/meter.h b/api/include/opentelemetry/metrics/meter.h index 8784fe788e..f0e5d90df3 100644 --- a/api/include/opentelemetry/metrics/meter.h +++ b/api/include/opentelemetry/metrics/meter.h @@ -21,6 +21,9 @@ class Histogram; template class UpDownCounter; +template +class Gauge; + class ObservableInstrument; /** @@ -91,6 +94,24 @@ class Meter nostd::string_view description = "", nostd::string_view unit = "") noexcept = 0; + /** + * Creates a Gauge with the passed characteristics and returns a + * unique_ptr to that Gauge + * + * @param name the name of the new Gauge. + * @param description a brief description of what the Gauge is used for. + * @param unit the unit of metric values following https://unitsofmeasure.org/ucum.html. + */ + virtual nostd::unique_ptr> CreateInt64Gauge( + nostd::string_view name, + nostd::string_view description = "", + nostd::string_view unit = "") noexcept = 0; + + virtual nostd::unique_ptr> CreateDoubleGauge( + nostd::string_view name, + nostd::string_view description = "", + nostd::string_view unit = "") noexcept = 0; + /** * Creates a Asynchronouse (Observable) Gauge with the passed characteristics and returns a * shared_ptr to that Observable Gauge diff --git a/api/include/opentelemetry/metrics/sync_instruments.h b/api/include/opentelemetry/metrics/sync_instruments.h index b26e527c2c..1a299ef9a1 100644 --- a/api/include/opentelemetry/metrics/sync_instruments.h +++ b/api/include/opentelemetry/metrics/sync_instruments.h @@ -194,5 +194,47 @@ class UpDownCounter : public SynchronousInstrument } }; +/** An Gauge instrument that records value. */ + +template +class Gauge : public SynchronousInstrument +{ +public: + /** + * Records the current value. + * + * @param value The measurement value. MUST be non-negative. + */ + 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 value. + */ + virtual void Record(T value, + const common::KeyValueIterable &attributes, + const context::Context &context) noexcept = 0; + + template ::value> * = nullptr> + void Record(T value, const U &attributes, const context::Context &context) noexcept + { + this->Record(value, common::KeyValueIterableView{attributes}, context); + } + + void Record( + T value, + std::initializer_list> attributes, + const context::Context &context) noexcept + { + this->Record(value, + nostd::span>{ + attributes.begin(), attributes.end()}, + context); + } +}; + } // namespace metrics OPENTELEMETRY_END_NAMESPACE diff --git a/sdk/include/opentelemetry/sdk/metrics/meter.h b/sdk/include/opentelemetry/sdk/metrics/meter.h index 7e89d444ab..26d7e847ac 100644 --- a/sdk/include/opentelemetry/sdk/metrics/meter.h +++ b/sdk/include/opentelemetry/sdk/metrics/meter.h @@ -74,6 +74,16 @@ class Meter final : public opentelemetry::metrics::Meter nostd::string_view description = "", nostd::string_view unit = "") noexcept override; + nostd::unique_ptr> CreateInt64Gauge( + nostd::string_view name, + nostd::string_view description = "", + nostd::string_view unit = "") noexcept override; + + nostd::unique_ptr> CreateDoubleGauge( + nostd::string_view name, + nostd::string_view description = "", + nostd::string_view unit = "") noexcept override; + nostd::shared_ptr CreateInt64ObservableGauge( nostd::string_view name, nostd::string_view description = "", diff --git a/sdk/include/opentelemetry/sdk/metrics/sync_instruments.h b/sdk/include/opentelemetry/sdk/metrics/sync_instruments.h index 825547a846..fb54604d24 100644 --- a/sdk/include/opentelemetry/sdk/metrics/sync_instruments.h +++ b/sdk/include/opentelemetry/sdk/metrics/sync_instruments.h @@ -196,6 +196,37 @@ class DoubleHistogram : public Synchronous, public opentelemetry::metrics::Histo void Record(double value, const opentelemetry::context::Context &context) noexcept override; }; +class LongGauge : public Synchronous, public opentelemetry::metrics::Gauge +{ +{ +public: + /** + * Records the current value. + * + * @param value The measurement value. MUST be non-negative. + */ + virtual void Record(int64_t value, const context::Context &context) noexcept{ + + } + +}; + +class DoubleGauge : public Synchronous, public opentelemetry::metrics::Gauge +{ +{ +public: + /** + * Records the current value. + * + * @param value The measurement value. MUST be non-negative. + */ + virtual void Record(int64_t value, const context::Context &context) noexcept{ + + } + +}; + + } // namespace metrics } // namespace sdk OPENTELEMETRY_END_NAMESPACE From 6ac46fb21748f5cff49b76294261ee153db88bdd Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Thu, 28 Sep 2023 22:25:22 -0700 Subject: [PATCH 02/13] changes --- api/include/opentelemetry/metrics/meter.h | 3 + api/include/opentelemetry/metrics/noop.h | 46 +++ .../opentelemetry/metrics/sync_instruments.h | 148 ++++++-- .../metrics/aggregation/default_aggregation.h | 3 + .../opentelemetry/sdk/metrics/instruments.h | 3 + sdk/include/opentelemetry/sdk/metrics/meter.h | 2 + .../sdk/metrics/sync_instruments.h | 163 +++------ sdk/src/metrics/meter.cc | 49 ++- sdk/src/metrics/sync_instruments.cc | 339 ++++++++++++++++-- sdk/test/metrics/sync_instruments_test.cc | 4 +- 10 files changed, 599 insertions(+), 161 deletions(-) diff --git a/api/include/opentelemetry/metrics/meter.h b/api/include/opentelemetry/metrics/meter.h index f0e5d90df3..8f6e8e97a5 100644 --- a/api/include/opentelemetry/metrics/meter.h +++ b/api/include/opentelemetry/metrics/meter.h @@ -94,9 +94,11 @@ class Meter nostd::string_view description = "", nostd::string_view unit = "") noexcept = 0; +#if OPENTELEMETRY_ABI_VERSION_NO >= 2 /** * Creates a Gauge with the passed characteristics and returns a * unique_ptr to that Gauge + * @since ABI_VERSION 2 * * @param name the name of the new Gauge. * @param description a brief description of what the Gauge is used for. @@ -111,6 +113,7 @@ class Meter nostd::string_view name, nostd::string_view description = "", nostd::string_view unit = "") noexcept = 0; +#endif /** * Creates a Asynchronouse (Observable) Gauge with the passed characteristics and returns a diff --git a/api/include/opentelemetry/metrics/noop.h b/api/include/opentelemetry/metrics/noop.h index c5802f3dd3..2a7d56411f 100644 --- a/api/include/opentelemetry/metrics/noop.h +++ b/api/include/opentelemetry/metrics/noop.h @@ -44,6 +44,12 @@ 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 @@ -64,6 +70,27 @@ class NoopUpDownCounter : public UpDownCounter {} }; +#if OPENTELEMETRY_ABI_VERSION_NO >= 2 + +template +class NoopGauge : public Gauge +{ +public: + NoopGauge(nostd::string_view /* name */, + nostd::string_view /* description */, + nostd::string_view /* unit */) noexcept + {} + void Record(T /* value */, const context::Context & /* context */) noexcept override {} + void Record(T /* value */, + const common::KeyValueIterable & /* attributes */, + const context::Context & /* context */) noexcept override + {} + void Record(T value) noexcept override {} + void Record(T value, const opentelemetry::common::KeyValueIterable &attributes) noexcept override + {} +}; +#endif + class NoopObservableInstrument : public ObservableInstrument { public: @@ -133,6 +160,25 @@ class NoopMeter final : public Meter return nostd::unique_ptr>{new NoopHistogram(name, description, unit)}; } +#if OPENTELEMETRY_ABI_VERSION_NO >= 2 + + nostd::unique_ptr> CreateInt64Gauge( + nostd::string_view name, + nostd::string_view description = "", + nostd::string_view unit = "") noexcept override + { + return nostd::unique_ptr>(new NoopGauge(name, description, unit)); + } + + nostd::unique_ptr> CreateDoubleGauge(nostd::string_view name, + nostd::string_view description = "", + nostd::string_view unit = "") noexcept override + { + return nostd::unique_ptr>(new NoopGauge(name, description, unit)); + } + +#endif + nostd::shared_ptr CreateInt64ObservableGauge( nostd::string_view name, nostd::string_view description = "", diff --git a/api/include/opentelemetry/metrics/sync_instruments.h b/api/include/opentelemetry/metrics/sync_instruments.h index 1a299ef9a1..811e2b2d5a 100644 --- a/api/include/opentelemetry/metrics/sync_instruments.h +++ b/api/include/opentelemetry/metrics/sync_instruments.h @@ -22,31 +22,44 @@ 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 +68,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 +103,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 +182,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 +219,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, @@ -194,29 +248,58 @@ class UpDownCounter : public SynchronousInstrument } }; -/** An Gauge instrument that records value. */ +#if OPENTELEMETRY_ABI_VERSION_NO >= 2 + +/** An Gauge instrument that records value. + * @since ABI_VERSION 2 + */ template class Gauge : public SynchronousInstrument { public: /** - * Records the current value. + * Record a value * - * @param value The measurement value. MUST be non-negative. + * @param value The increment amount. MUST be non-negative. + */ + virtual void Record(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 Record(T value, const context::Context &context) noexcept = 0; /** - * Records a value with a set of attributes. + * Record a value with a set of attributes. * - * @param value The measurement value. MUST be non-negative. + * @param value The increment amount. MUST be non-negative. * @param attributes A set of attributes to associate with the value. */ + + virtual void Record(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 Record(T value, const common::KeyValueIterable &attributes, const context::Context &context) noexcept = 0; + template ::value> * = nullptr> + void Record(T value, const U &attributes) noexcept + { + this->Record(value, common::KeyValueIterableView{attributes}); + } + template ::value> * = nullptr> void Record(T value, const U &attributes, const context::Context &context) noexcept @@ -224,6 +307,14 @@ class Gauge : public SynchronousInstrument this->Record(value, common::KeyValueIterableView{attributes}, context); } + void Record(T value, + std::initializer_list> + attributes) noexcept + { + this->Record(value, nostd::span>{ + attributes.begin(), attributes.end()}); + } + void Record( T value, std::initializer_list> attributes, @@ -235,6 +326,7 @@ class Gauge : public SynchronousInstrument context); } }; +#endif } // namespace metrics OPENTELEMETRY_END_NAMESPACE diff --git a/sdk/include/opentelemetry/sdk/metrics/aggregation/default_aggregation.h b/sdk/include/opentelemetry/sdk/metrics/aggregation/default_aggregation.h index 6ae111fd85..8f5905e3a5 100644 --- a/sdk/include/opentelemetry/sdk/metrics/aggregation/default_aggregation.h +++ b/sdk/include/opentelemetry/sdk/metrics/aggregation/default_aggregation.h @@ -179,6 +179,9 @@ class DefaultAggregation return AggregationType::kSum; case InstrumentType::kHistogram: return AggregationType::kHistogram; +#if OPENTELEMETRY_ABI_VERSION_NO >= 2 + case InstrumentType::kGauge: +#endif case InstrumentType::kObservableGauge: return AggregationType::kLastValue; default: diff --git a/sdk/include/opentelemetry/sdk/metrics/instruments.h b/sdk/include/opentelemetry/sdk/metrics/instruments.h index 84244128a4..6d6375f644 100644 --- a/sdk/include/opentelemetry/sdk/metrics/instruments.h +++ b/sdk/include/opentelemetry/sdk/metrics/instruments.h @@ -19,6 +19,9 @@ enum class InstrumentType kCounter, kHistogram, kUpDownCounter, +#if OPENTELEMETRY_ABI_VERSION_NO >= 2 + kGauge, +#endif kObservableCounter, kObservableGauge, kObservableUpDownCounter diff --git a/sdk/include/opentelemetry/sdk/metrics/meter.h b/sdk/include/opentelemetry/sdk/metrics/meter.h index 26d7e847ac..fda25d0112 100644 --- a/sdk/include/opentelemetry/sdk/metrics/meter.h +++ b/sdk/include/opentelemetry/sdk/metrics/meter.h @@ -74,6 +74,7 @@ class Meter final : public opentelemetry::metrics::Meter nostd::string_view description = "", nostd::string_view unit = "") noexcept override; +#if OPENTELEMETRY_ABI_VERSION_NO >= 2 nostd::unique_ptr> CreateInt64Gauge( nostd::string_view name, nostd::string_view description = "", @@ -83,6 +84,7 @@ class Meter final : public opentelemetry::metrics::Meter nostd::string_view name, nostd::string_view description = "", nostd::string_view unit = "") noexcept override; +#endif nostd::shared_ptr CreateInt64ObservableGauge( nostd::string_view name, diff --git a/sdk/include/opentelemetry/sdk/metrics/sync_instruments.h b/sdk/include/opentelemetry/sdk/metrics/sync_instruments.h index fb54604d24..210ed805b0 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; @@ -196,36 +140,41 @@ class DoubleHistogram : public Synchronous, public opentelemetry::metrics::Histo void Record(double value, const opentelemetry::context::Context &context) noexcept override; }; -class LongGauge : public Synchronous, public opentelemetry::metrics::Gauge -{ +#if OPENTELEMETRY_ABI_VERSION_NO >= 2 + +class LongGauge : public Synchronous, public opentelemetry::metrics::Gauge { + public: - /** - * Records the current value. - * - * @param value The measurement value. MUST be non-negative. - */ - virtual void Record(int64_t value, const context::Context &context) noexcept{ + LongGauge(InstrumentDescriptor instrument_descriptor, + std::unique_ptr storage); - } + void Record(uint64_t value, + const opentelemetry::common::KeyValueIterable &attributes) noexcept override; + void Record(uint64_t value, + const opentelemetry::common::KeyValueIterable &attributes, + const opentelemetry::context::Context &context) noexcept override; + void Record(uint64_t value) noexcept override; + void Record(uint64_t value, const opentelemetry::context::Context &context) noexcept override; }; class DoubleGauge : public Synchronous, public opentelemetry::metrics::Gauge { -{ public: - /** - * Records the current value. - * - * @param value The measurement value. MUST be non-negative. - */ - virtual void Record(int64_t value, const context::Context &context) noexcept{ + DoubleGauge(InstrumentDescriptor instrument_descriptor, + std::unique_ptr storage); - } + void Record(double value, + const opentelemetry::common::KeyValueIterable &attributes) noexcept override; + void Record(double value, + const opentelemetry::common::KeyValueIterable &attributes, + const opentelemetry::context::Context &context) noexcept override; + void Record(double value) noexcept override; + void Record(double value, const opentelemetry::context::Context &context) noexcept override; }; - +#endif } // namespace metrics } // namespace sdk diff --git a/sdk/src/metrics/meter.cc b/sdk/src/metrics/meter.cc index 4f7c4c1207..9cc09e1119 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( @@ -163,6 +163,51 @@ nostd::unique_ptr> Meter::CreateDoubleHistogram( new DoubleHistogram(instrument_descriptor, std::move(storage))}; } +#if OPENTELEMETRY_ABI_VERSION_NO >= 2 + +nostd::unique_ptr> Meter::CreateInt64Gauge( + nostd::string_view name, + nostd::string_view description, + nostd::string_view unit) noexcept +{ + if (!ValidateInstrument(name, description, unit)) + { + OTEL_INTERNAL_LOG_ERROR("Meter::CreateInt64Gauge - failed. Invalid parameters." + << name << " " << description << " " << unit + << ". Measurements won't be recorded."); + return nostd::unique_ptr>( + new metrics::NoopGauge(name, description, unit)); + } + 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); + return nostd::unique_ptr>{ + new LongGauge(instrument_descriptor, std::move(storage))}; +} + +nostd::unique_ptr> Meter::CreateDoubleGauge( + nostd::string_view name, + nostd::string_view description, + nostd::string_view unit) noexcept +{ + if (!ValidateInstrument(name, description, unit)) + { + OTEL_INTERNAL_LOG_ERROR("Meter::CreateDoubleGauge - failed. Invalid parameters." + << name << " " << description << " " << unit + << ". Measurements won't be recorded."); + return nostd::unique_ptr>( + new metrics::NoopGauge(name, description, unit)); + } + 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); + return nostd::unique_ptr>{ + new DoubleGauge(instrument_descriptor, std::move(storage))}; +} +#endif + nostd::shared_ptr Meter::CreateInt64ObservableGauge( nostd::string_view name, nostd::string_view description, diff --git a/sdk/src/metrics/sync_instruments.cc b/sdk/src/metrics/sync_instruments.cc index 6fe72b55bf..ece2a094b9 100644 --- a/sdk/src/metrics/sync_instruments.cc +++ b/sdk/src/metrics/sync_instruments.cc @@ -10,16 +10,75 @@ 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_->RecordDouble(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_); } } @@ -29,6 +88,8 @@ void DoubleCounter::Add(double value, auto context = opentelemetry::context::Context{}; if (!storage_) { + OTEL_INTERNAL_LOG_WARN("[DoubleCounter::Add(V,A)] Value not recorded - invalid storage for: " + << instrument_descriptor_.name_); return; } return storage_->RecordDouble(value, attributes, context); @@ -40,6 +101,8 @@ void DoubleCounter::Add(double value, { 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); @@ -50,6 +113,8 @@ void DoubleCounter::Add(double value) noexcept auto context = opentelemetry::context::Context{}; if (!storage_) { + OTEL_INTERNAL_LOG_WARN("[DoubleCounter::Add(V)] Value not recorded - invalid storage for: " + << instrument_descriptor_.name_); return; } return storage_->RecordDouble(value, context); @@ -59,6 +124,8 @@ void DoubleCounter::Add(double value, const opentelemetry::context::Context &con { 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 +138,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 +149,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 +163,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 +176,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 +187,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 +202,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 +226,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 +238,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 +251,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 +329,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_); } } @@ -187,14 +339,9 @@ void DoubleHistogram::Record(double value, const opentelemetry::context::Context &context) noexcept { if (!storage_) - { - return; - } - if (value < 0 || std::isnan(value) || std::isinf(value)) { 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; } @@ -205,17 +352,165 @@ void DoubleHistogram::Record(double value, const opentelemetry::context::Context { if (!storage_) { + OTEL_INTERNAL_LOG_WARN( + "[DoubleHistogram::Record(V,C)] Value not recorded - invalid storage for: " + << instrument_descriptor_.name_); return; } - if (value < 0 || std::isnan(value) || std::isinf(value)) + return storage_->RecordDouble(value, context); +} + +#if OPENTELEMETRY_ABI_VERSION_NO >= 2 +void DoubleHistogram::Record(double value, + const opentelemetry::common::KeyValueIterable &attributes) noexcept +{ + 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 (!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 + +#if OPENTELEMETRY_ABI_VERSION_NO >= 2 + +LongGauge::LongGauge(InstrumentDescriptor instrument_descriptor, + std::unique_ptr storage) + : Synchronous(instrument_descriptor, std::move(storage)) +{ + if (!storage_) + { + OTEL_INTERNAL_LOG_ERROR("[LongGauge::LongGauge] - Error during constructing LongGauge." + << "The metric storage is invalid for " << instrument_descriptor.name_); + } +} + +void LongGauge::Record(uint64_t value, + const opentelemetry::common::KeyValueIterable &attributes, + const opentelemetry::context::Context &context) noexcept +{ + if (!storage_) + { + OTEL_INTERNAL_LOG_WARN("[LongGauge::Record(V,A,C)] Value not recorded - invalid storage for: " + << instrument_descriptor_.name_); + return; + } + return storage_->RecordLong(value, attributes, context); +} + +void LongGauge::Record(uint64_t value, const opentelemetry::context::Context &context) noexcept +{ + if (!storage_) + { + OTEL_INTERNAL_LOG_WARN("[LongGauge::Record(V,C)] Value not recorded - invalid storage for: " + << instrument_descriptor_.name_); + return; + } + return storage_->RecordLong(value, context); +} + +void LongGauge::Record(uint64_t value, + const opentelemetry::common::KeyValueIterable &attributes) noexcept +{ + if (!storage_) + { + OTEL_INTERNAL_LOG_WARN("[LongGauge::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 LongGauge::Record(uint64_t value) noexcept +{ + if (!storage_) + { + OTEL_INTERNAL_LOG_WARN("[LongGauge::Record(V)] Value not recorded - invalid storage for: " + << instrument_descriptor_.name_); + return; + } + auto context = opentelemetry::context::Context{}; + return storage_->RecordLong(value, context); +} + +DoubleGauge::DoubleGauge(InstrumentDescriptor instrument_descriptor, + std::unique_ptr storage) + : Synchronous(instrument_descriptor, std::move(storage)) +{ + if (!storage_) + { + OTEL_INTERNAL_LOG_ERROR( + "[DoubleHistogram::DoubleHistogram] - Error during constructing DoubleHistogram." + << "The metric storage is invalid for " << instrument_descriptor.name_); + } +} + +void DoubleGauge::Record(double value, + const opentelemetry::common::KeyValueIterable &attributes, + const opentelemetry::context::Context &context) noexcept +{ + if (!storage_) + { + OTEL_INTERNAL_LOG_WARN("[DoubleGauge::Record(V,A,C)] Value not recorded - invalid storage for: " + << instrument_descriptor_.name_); + return; + } + return storage_->RecordDouble(value, attributes, context); +} + +void DoubleGauge::Record(double value, const opentelemetry::context::Context &context) noexcept +{ + if (!storage_) + { + OTEL_INTERNAL_LOG_WARN("[DoubleGauge::Record(V,C)] Value not recorded - invalid storage for: " + << instrument_descriptor_.name_); + return; + } + return storage_->RecordDouble(value, context); +} + +void DoubleGauge::Record(double value, + const opentelemetry::common::KeyValueIterable &attributes) noexcept +{ + if (!storage_) + { + OTEL_INTERNAL_LOG_WARN("[DoubleGauge::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 DoubleGauge::Record(double value) noexcept +{ + if (!storage_) + { + OTEL_INTERNAL_LOG_WARN("[DoubleGauge::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..762f2f09c7 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{}); @@ -98,7 +98,7 @@ 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 From b6909af0782dec0f8f259ccb29f864627f162f87 Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Thu, 28 Sep 2023 22:45:15 -0700 Subject: [PATCH 03/13] validate double value --- sdk/src/metrics/sync_instruments.cc | 79 ++++++++++++++++++++++++++++- 1 file changed, 77 insertions(+), 2 deletions(-) diff --git a/sdk/src/metrics/sync_instruments.cc b/sdk/src/metrics/sync_instruments.cc index ece2a094b9..657571dc3d 100644 --- a/sdk/src/metrics/sync_instruments.cc +++ b/sdk/src/metrics/sync_instruments.cc @@ -85,13 +85,19 @@ DoubleCounter::DoubleCounter(InstrumentDescriptor instrument_descriptor, void DoubleCounter::Add(double value, const opentelemetry::common::KeyValueIterable &attributes) noexcept { - auto context = opentelemetry::context::Context{}; + if (double < 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); } @@ -99,6 +105,12 @@ void DoubleCounter::Add(double value, const opentelemetry::common::KeyValueIterable &attributes, const opentelemetry::context::Context &context) noexcept { + if (double < 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: " @@ -110,18 +122,30 @@ void DoubleCounter::Add(double value, void DoubleCounter::Add(double value) noexcept { - auto context = opentelemetry::context::Context{}; + if (double < 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 (double < 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: " @@ -338,6 +362,13 @@ void DoubleHistogram::Record(double value, const opentelemetry::common::KeyValueIterable &attributes, const opentelemetry::context::Context &context) noexcept { + if (value < 0) + { + OTEL_INTERNAL_LOG_WARN( + "[DoubleHistogram::Record(V,A,C)] Value not recorded - negative value for: " + << instrument_descriptor_.name_); + return; + } if (!storage_) { OTEL_INTERNAL_LOG_WARN( @@ -350,6 +381,13 @@ 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( @@ -364,6 +402,13 @@ void DoubleHistogram::Record(double value, const opentelemetry::context::Context 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 (!storage_) { OTEL_INTERNAL_LOG_WARN( @@ -377,6 +422,12 @@ void DoubleHistogram::Record(double value, 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: " @@ -466,6 +517,12 @@ void DoubleGauge::Record(double value, const opentelemetry::common::KeyValueIterable &attributes, const opentelemetry::context::Context &context) noexcept { + if (value < 0) + { + OTEL_INTERNAL_LOG_WARN("[DoubleGauge::Record(V,A,C)] Value not recorded - negative value for: " + << instrument_descriptor_.name_); + return; + } if (!storage_) { OTEL_INTERNAL_LOG_WARN("[DoubleGauge::Record(V,A,C)] Value not recorded - invalid storage for: " @@ -477,6 +534,12 @@ void DoubleGauge::Record(double value, void DoubleGauge::Record(double value, const opentelemetry::context::Context &context) noexcept { + if (value < 0) + { + OTEL_INTERNAL_LOG_WARN("[DoubleGauge::Record(V,C)] Value not recorded - negative value for: " + << instrument_descriptor_.name_); + return; + } if (!storage_) { OTEL_INTERNAL_LOG_WARN("[DoubleGauge::Record(V,C)] Value not recorded - invalid storage for: " @@ -489,6 +552,12 @@ void DoubleGauge::Record(double value, const opentelemetry::context::Context &co void DoubleGauge::Record(double value, const opentelemetry::common::KeyValueIterable &attributes) noexcept { + if (value < 0) + { + OTEL_INTERNAL_LOG_WARN("[DoubleGauge::Record(V,A)] Value not recorded - negative value for: " + << instrument_descriptor_.name_); + return; + } if (!storage_) { OTEL_INTERNAL_LOG_WARN("[DoubleGauge::Record(V,A)] Value not recorded - invalid storage for: " @@ -501,6 +570,12 @@ void DoubleGauge::Record(double value, void DoubleGauge::Record(double value) noexcept { + if (value < 0) + { + OTEL_INTERNAL_LOG_WARN("[DoubleGauge::Record(V)] Value not recorded - negative value for: " + << instrument_descriptor_.name_); + return; + } if (!storage_) { OTEL_INTERNAL_LOG_WARN("[DoubleGauge::Record(V)] Value not recorded - invalid storage for: " From c511147b7e650d1dace50a3b8d667edaaeb917b8 Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Thu, 28 Sep 2023 22:49:49 -0700 Subject: [PATCH 04/13] compile error --- sdk/src/metrics/sync_instruments.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/sdk/src/metrics/sync_instruments.cc b/sdk/src/metrics/sync_instruments.cc index 657571dc3d..4a9e2718a0 100644 --- a/sdk/src/metrics/sync_instruments.cc +++ b/sdk/src/metrics/sync_instruments.cc @@ -85,7 +85,7 @@ DoubleCounter::DoubleCounter(InstrumentDescriptor instrument_descriptor, void DoubleCounter::Add(double value, const opentelemetry::common::KeyValueIterable &attributes) noexcept { - if (double < 0) + if (value < 0) { OTEL_INTERNAL_LOG_WARN("[DoubleCounter::Add(V,A)] Value not recorded - negative value for: " << instrument_descriptor_.name_); @@ -105,7 +105,7 @@ void DoubleCounter::Add(double value, const opentelemetry::common::KeyValueIterable &attributes, const opentelemetry::context::Context &context) noexcept { - if (double < 0) + if (value < 0) { OTEL_INTERNAL_LOG_WARN("[DoubleCounter::Add(V,A,C)] Value not recorded - negative value for: " << instrument_descriptor_.name_); @@ -122,7 +122,7 @@ void DoubleCounter::Add(double value, void DoubleCounter::Add(double value) noexcept { - if (double < 0) + if (value < 0) { OTEL_INTERNAL_LOG_WARN("[DoubleCounter::Add(V)] Value not recorded - negative value for: " << instrument_descriptor_.name_); @@ -140,7 +140,7 @@ void DoubleCounter::Add(double value) noexcept void DoubleCounter::Add(double value, const opentelemetry::context::Context &context) noexcept { - if (double < 0) + if (value < 0) { OTEL_INTERNAL_LOG_WARN("[DoubleCounter::Add(V)] Value not recorded - negative value for: " << instrument_descriptor_.name_); From b9f538b68529ae9ad25240f29cf00c89370cb83d Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Thu, 28 Sep 2023 23:06:54 -0700 Subject: [PATCH 05/13] compile error --- sdk/src/metrics/sync_instruments.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/src/metrics/sync_instruments.cc b/sdk/src/metrics/sync_instruments.cc index 4a9e2718a0..a3096e053b 100644 --- a/sdk/src/metrics/sync_instruments.cc +++ b/sdk/src/metrics/sync_instruments.cc @@ -57,7 +57,7 @@ void LongCounter::Add(uint64_t value) noexcept << instrument_descriptor_.name_); return; } - return storage_->RecordDouble(value, context); + return storage_->RecordLong(value, context); } void LongCounter::Add(uint64_t value, const opentelemetry::context::Context &context) noexcept From 06cd640be209aa85af0442c2df0a47aba865fba4 Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Thu, 28 Sep 2023 23:46:54 -0700 Subject: [PATCH 06/13] msvc build --- sdk/test/metrics/sync_instruments_test.cc | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/sdk/test/metrics/sync_instruments_test.cc b/sdk/test/metrics/sync_instruments_test.cc index 762f2f09c7..5442dcc395 100644 --- a/sdk/test/metrics/sync_instruments_test.cc +++ b/sdk/test/metrics/sync_instruments_test.cc @@ -71,6 +71,17 @@ 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) @@ -100,7 +111,6 @@ TEST(SyncInstruments, LongHistogram) std::unique_ptr metric_storage(new SyncMultiMetricStorage()); 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"}}), From 9e34355e518c018e3e92f11394b5f76620ea6ef9 Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Thu, 28 Sep 2023 23:55:29 -0700 Subject: [PATCH 07/13] add tests.. --- sdk/test/metrics/sync_instruments_test.cc | 40 +++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/sdk/test/metrics/sync_instruments_test.cc b/sdk/test/metrics/sync_instruments_test.cc index 5442dcc395..2234fca1a8 100644 --- a/sdk/test/metrics/sync_instruments_test.cc +++ b/sdk/test/metrics/sync_instruments_test.cc @@ -139,3 +139,43 @@ TEST(SyncInstruments, DoubleHistogram) histogram.Record(10.10, opentelemetry::common::KeyValueIterableView({}), opentelemetry::context::Context{}); } + +#if OPEN_TELEMETRY_ABI_VERSION_NO >= 2 +TEST(SyncInstruments, DoubleGauge) +{ + InstrumentDescriptor instrument_descriptor = { + "double_gauge", "description", "1", InstrumentType::kGauge, InstrumentValueType::kDouble}; + std::unique_ptr metric_storage(new SyncMultiMetricStorage()); + DoubleGauge gauge(instrument_descriptor, std::move(metric_storage)); + gauge.Record(10.10, opentelemetry::context::Context{}); + gauge.Record(-10.10, opentelemetry::context::Context{}); // This is ignored. + gauge.Record(std::numeric_limits::quiet_NaN(), + opentelemetry::context::Context{}); // This is ignored too + gauge.Record(std::numeric_limits::infinity(), + opentelemetry::context::Context{}); // This is ignored too + + gauge.Record(10.10, + opentelemetry::common::KeyValueIterableView({{"abc", "123"}, {"xyz", "456"}}), + opentelemetry::context::Context{}); + gauge.Record(10.10, opentelemetry::common::KeyValueIterableView({}), + opentelemetry::context::Context{}); +} + +TEST(SyncInstruments, LongGauge) +{ + InstrumentDescriptor instrument_descriptor = {"double_gauge", "description", "1", + InstrumentType::kGauge, InstrumentValueType::kLong}; + std::unique_ptr metric_storage(new SyncMultiMetricStorage()); + DoubleGauge gauge(instrument_descriptor, std::move(metric_storage)); + gauge.Record(10, opentelemetry::context::Context{}); + gauge.Record(std::numeric_limits::quiet_NaN(), + opentelemetry::context::Context{}); // This is ignored too + gauge.Record(std::numeric_limits::infinity(), + opentelemetry::context::Context{}); // This is ignored too + + gauge.Record(10, opentelemetry::common::KeyValueIterableView({{"abc", "123"}, {"xyz", "456"}}), + opentelemetry::context::Context{}); + gauge.Record(10, opentelemetry::common::KeyValueIterableView({}), + opentelemetry::context::Context{}); +} +#endif \ No newline at end of file From 4134c001b41a28baba877243fee7d1dfc566e58d Mon Sep 17 00:00:00 2001 From: Lalit Date: Fri, 29 Sep 2023 10:25:54 -0700 Subject: [PATCH 08/13] review comments --- api/include/opentelemetry/metrics/noop.h | 8 +- .../opentelemetry/metrics/sync_instruments.h | 42 ++++++-- .../metrics/aggregation/default_aggregation.h | 2 - .../opentelemetry/sdk/metrics/instruments.h | 2 - .../sdk/metrics/sync_instruments.h | 19 +--- sdk/src/metrics/sync_instruments.cc | 101 ++---------------- 6 files changed, 43 insertions(+), 131 deletions(-) diff --git a/api/include/opentelemetry/metrics/noop.h b/api/include/opentelemetry/metrics/noop.h index 2a7d56411f..25f350e5fb 100644 --- a/api/include/opentelemetry/metrics/noop.h +++ b/api/include/opentelemetry/metrics/noop.h @@ -80,13 +80,9 @@ class NoopGauge : public Gauge nostd::string_view /* description */, nostd::string_view /* unit */) noexcept {} - void Record(T /* value */, const context::Context & /* context */) noexcept override {} void Record(T /* value */, - const common::KeyValueIterable & /* attributes */, - const context::Context & /* context */) noexcept override - {} - void Record(T value) noexcept override {} - void Record(T value, const opentelemetry::common::KeyValueIterable &attributes) noexcept override + const common::KeyValueIterable * /* attributes */, + const context::Context * /* context */) noexcept override {} }; #endif diff --git a/api/include/opentelemetry/metrics/sync_instruments.h b/api/include/opentelemetry/metrics/sync_instruments.h index 811e2b2d5a..4950c2c13d 100644 --- a/api/include/opentelemetry/metrics/sync_instruments.h +++ b/api/include/opentelemetry/metrics/sync_instruments.h @@ -259,39 +259,59 @@ class Gauge : public SynchronousInstrument { public: /** - * Record a value + * 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 Record(T value) noexcept = 0; + virtual void Record(T value, + const common::KeyValueIterable *attributes, + const context::Context *context) noexcept = 0; /** - * Record a value + * 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 Record(T value, const context::Context &context) noexcept = 0; + void Record(T value, + const common::KeyValueIterable &attributes, + const context::Context &context) noexcept + { + this->Record(value, &attributes, &context); + } /** - * Record a value with a set of attributes. + * Record a value * * @param value The increment amount. MUST be non-negative. - * @param attributes A set of attributes to associate with the value. */ + virtual void Record(T value) noexcept { this->Record(value, nullptr, nullptr); } - virtual void Record(T value, const common::KeyValueIterable &attributes) noexcept = 0; + /** + * Record a value + * + * @param value The increment amount. MUST be non-negative. + * @param context The explicit context to associate with this measurement. + */ + void Record(T value, const context::Context &context) noexcept + { + this->Record(value, nullptr, &context); + } /** * 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 Record(T value, - const common::KeyValueIterable &attributes, - const context::Context &context) noexcept = 0; + + void Record(T value, const common::KeyValueIterable &attributes) noexcept + { + this->Record(value, &attributes, nullptr); + } template ::value> * = nullptr> diff --git a/sdk/include/opentelemetry/sdk/metrics/aggregation/default_aggregation.h b/sdk/include/opentelemetry/sdk/metrics/aggregation/default_aggregation.h index 8f5905e3a5..ee5661046a 100644 --- a/sdk/include/opentelemetry/sdk/metrics/aggregation/default_aggregation.h +++ b/sdk/include/opentelemetry/sdk/metrics/aggregation/default_aggregation.h @@ -179,9 +179,7 @@ class DefaultAggregation return AggregationType::kSum; case InstrumentType::kHistogram: return AggregationType::kHistogram; -#if OPENTELEMETRY_ABI_VERSION_NO >= 2 case InstrumentType::kGauge: -#endif case InstrumentType::kObservableGauge: return AggregationType::kLastValue; default: diff --git a/sdk/include/opentelemetry/sdk/metrics/instruments.h b/sdk/include/opentelemetry/sdk/metrics/instruments.h index 6d6375f644..375c4d70e8 100644 --- a/sdk/include/opentelemetry/sdk/metrics/instruments.h +++ b/sdk/include/opentelemetry/sdk/metrics/instruments.h @@ -19,9 +19,7 @@ enum class InstrumentType kCounter, kHistogram, kUpDownCounter, -#if OPENTELEMETRY_ABI_VERSION_NO >= 2 kGauge, -#endif kObservableCounter, kObservableGauge, kObservableUpDownCounter diff --git a/sdk/include/opentelemetry/sdk/metrics/sync_instruments.h b/sdk/include/opentelemetry/sdk/metrics/sync_instruments.h index 210ed805b0..9f7485b970 100644 --- a/sdk/include/opentelemetry/sdk/metrics/sync_instruments.h +++ b/sdk/include/opentelemetry/sdk/metrics/sync_instruments.h @@ -144,19 +144,13 @@ class DoubleHistogram : public Synchronous, public opentelemetry::metrics::Histo class LongGauge : public Synchronous, public opentelemetry::metrics::Gauge { - public: LongGauge(InstrumentDescriptor instrument_descriptor, std::unique_ptr storage); void Record(uint64_t value, - const opentelemetry::common::KeyValueIterable &attributes) noexcept override; - void Record(uint64_t value, - const opentelemetry::common::KeyValueIterable &attributes, - const opentelemetry::context::Context &context) noexcept override; - - void Record(uint64_t value) noexcept override; - void Record(uint64_t value, const opentelemetry::context::Context &context) noexcept override; + const opentelemetry::common::KeyValueIterable *attributes, + const opentelemetry::context::Context *context) noexcept override; }; class DoubleGauge : public Synchronous, public opentelemetry::metrics::Gauge @@ -166,13 +160,8 @@ class DoubleGauge : public Synchronous, public opentelemetry::metrics::Gauge storage); void Record(double value, - const opentelemetry::common::KeyValueIterable &attributes) noexcept override; - void Record(double value, - const opentelemetry::common::KeyValueIterable &attributes, - const opentelemetry::context::Context &context) noexcept override; - - void Record(double value) noexcept override; - void Record(double value, const opentelemetry::context::Context &context) noexcept override; + const opentelemetry::common::KeyValueIterable *attributes, + const opentelemetry::context::Context *context) noexcept override; }; #endif diff --git a/sdk/src/metrics/sync_instruments.cc b/sdk/src/metrics/sync_instruments.cc index a3096e053b..511976ba80 100644 --- a/sdk/src/metrics/sync_instruments.cc +++ b/sdk/src/metrics/sync_instruments.cc @@ -453,8 +453,8 @@ LongGauge::LongGauge(InstrumentDescriptor instrument_descriptor, } void LongGauge::Record(uint64_t value, - const opentelemetry::common::KeyValueIterable &attributes, - const opentelemetry::context::Context &context) noexcept + const opentelemetry::common::KeyValueIterable *attributes, + const opentelemetry::context::Context *context) noexcept { if (!storage_) { @@ -462,43 +462,7 @@ void LongGauge::Record(uint64_t value, << instrument_descriptor_.name_); return; } - return storage_->RecordLong(value, attributes, context); -} - -void LongGauge::Record(uint64_t value, const opentelemetry::context::Context &context) noexcept -{ - if (!storage_) - { - OTEL_INTERNAL_LOG_WARN("[LongGauge::Record(V,C)] Value not recorded - invalid storage for: " - << instrument_descriptor_.name_); - return; - } - return storage_->RecordLong(value, context); -} - -void LongGauge::Record(uint64_t value, - const opentelemetry::common::KeyValueIterable &attributes) noexcept -{ - if (!storage_) - { - OTEL_INTERNAL_LOG_WARN("[LongGauge::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 LongGauge::Record(uint64_t value) noexcept -{ - if (!storage_) - { - OTEL_INTERNAL_LOG_WARN("[LongGauge::Record(V)] Value not recorded - invalid storage for: " - << instrument_descriptor_.name_); - return; - } - auto context = opentelemetry::context::Context{}; - return storage_->RecordLong(value, context); + return storage_->RecordLong(value, *attributes, *context); } DoubleGauge::DoubleGauge(InstrumentDescriptor instrument_descriptor, @@ -514,8 +478,8 @@ DoubleGauge::DoubleGauge(InstrumentDescriptor instrument_descriptor, } void DoubleGauge::Record(double value, - const opentelemetry::common::KeyValueIterable &attributes, - const opentelemetry::context::Context &context) noexcept + const opentelemetry::common::KeyValueIterable *attributes, + const opentelemetry::context::Context *context) noexcept { if (value < 0) { @@ -529,62 +493,9 @@ void DoubleGauge::Record(double value, << instrument_descriptor_.name_); return; } - return storage_->RecordDouble(value, attributes, context); -} - -void DoubleGauge::Record(double value, const opentelemetry::context::Context &context) noexcept -{ - if (value < 0) - { - OTEL_INTERNAL_LOG_WARN("[DoubleGauge::Record(V,C)] Value not recorded - negative value for: " - << instrument_descriptor_.name_); - return; - } - if (!storage_) - { - OTEL_INTERNAL_LOG_WARN("[DoubleGauge::Record(V,C)] Value not recorded - invalid storage for: " - << instrument_descriptor_.name_); - return; - } - return storage_->RecordDouble(value, context); -} - -void DoubleGauge::Record(double value, - const opentelemetry::common::KeyValueIterable &attributes) noexcept -{ - if (value < 0) - { - OTEL_INTERNAL_LOG_WARN("[DoubleGauge::Record(V,A)] Value not recorded - negative value for: " - << instrument_descriptor_.name_); - return; - } - if (!storage_) - { - OTEL_INTERNAL_LOG_WARN("[DoubleGauge::Record(V,A)] Value not recorded - invalid storage for: " - << instrument_descriptor_.name_); - return; - } - auto context = opentelemetry::context::Context{}; - return storage_->RecordDouble(value, attributes, context); + return storage_->RecordDouble(value, *attributes, *context); } -void DoubleGauge::Record(double value) noexcept -{ - if (value < 0) - { - OTEL_INTERNAL_LOG_WARN("[DoubleGauge::Record(V)] Value not recorded - negative value for: " - << instrument_descriptor_.name_); - return; - } - if (!storage_) - { - OTEL_INTERNAL_LOG_WARN("[DoubleGauge::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 From e0bdcccb806be01ee8c07d8b94cfb7be0acc7bf1 Mon Sep 17 00:00:00 2001 From: Lalit Date: Fri, 29 Sep 2023 16:56:00 -0700 Subject: [PATCH 09/13] fix test --- .../opentelemetry/metrics/sync_instruments.h | 2 +- sdk/test/metrics/sync_instruments_test.cc | 52 ++++++++++--------- 2 files changed, 28 insertions(+), 26 deletions(-) diff --git a/api/include/opentelemetry/metrics/sync_instruments.h b/api/include/opentelemetry/metrics/sync_instruments.h index 4950c2c13d..43a5d69fc6 100644 --- a/api/include/opentelemetry/metrics/sync_instruments.h +++ b/api/include/opentelemetry/metrics/sync_instruments.h @@ -288,7 +288,7 @@ class Gauge : public SynchronousInstrument * * @param value The increment amount. MUST be non-negative. */ - virtual void Record(T value) noexcept { this->Record(value, nullptr, nullptr); } + void Record(T value) noexcept { this->Record(value, nullptr, nullptr); } /** * Record a value diff --git a/sdk/test/metrics/sync_instruments_test.cc b/sdk/test/metrics/sync_instruments_test.cc index 2234fca1a8..d4cabbd37e 100644 --- a/sdk/test/metrics/sync_instruments_test.cc +++ b/sdk/test/metrics/sync_instruments_test.cc @@ -140,25 +140,25 @@ TEST(SyncInstruments, DoubleHistogram) opentelemetry::context::Context{}); } -#if OPEN_TELEMETRY_ABI_VERSION_NO >= 2 +#if OPENTELEMETRY_ABI_VERSION_NO >= 2 TEST(SyncInstruments, DoubleGauge) { InstrumentDescriptor instrument_descriptor = { "double_gauge", "description", "1", InstrumentType::kGauge, InstrumentValueType::kDouble}; std::unique_ptr metric_storage(new SyncMultiMetricStorage()); - DoubleGauge gauge(instrument_descriptor, std::move(metric_storage)); - gauge.Record(10.10, opentelemetry::context::Context{}); - gauge.Record(-10.10, opentelemetry::context::Context{}); // This is ignored. - gauge.Record(std::numeric_limits::quiet_NaN(), - opentelemetry::context::Context{}); // This is ignored too - gauge.Record(std::numeric_limits::infinity(), - opentelemetry::context::Context{}); // This is ignored too - - gauge.Record(10.10, - opentelemetry::common::KeyValueIterableView({{"abc", "123"}, {"xyz", "456"}}), - opentelemetry::context::Context{}); - gauge.Record(10.10, opentelemetry::common::KeyValueIterableView({}), - opentelemetry::context::Context{}); + auto gauge = std::unique_ptr>( + new DoubleGauge(instrument_descriptor, std::move(metric_storage))); + EXPECT_NO_THROW(gauge->Record(10.10, opentelemetry::context::Context{})); + EXPECT_NO_THROW(gauge->Record(-10.10, opentelemetry::context::Context{})); // This is ignored. + EXPECT_NO_THROW(gauge->Record(std::numeric_limits::quiet_NaN(), + opentelemetry::context::Context{})); // This is ignored too + EXPECT_NO_THROW(gauge->Record(std::numeric_limits::infinity(), + opentelemetry::context::Context{})); // This is ignored too + EXPECT_NO_THROW(gauge->Record( + 10.10, opentelemetry::common::KeyValueIterableView({{"abc", "123"}, {"xyz", "456"}}), + opentelemetry::context::Context{})); + EXPECT_NO_THROW(gauge->Record(10.10, opentelemetry::common::KeyValueIterableView({}), + opentelemetry::context::Context{})); } TEST(SyncInstruments, LongGauge) @@ -166,16 +166,18 @@ TEST(SyncInstruments, LongGauge) InstrumentDescriptor instrument_descriptor = {"double_gauge", "description", "1", InstrumentType::kGauge, InstrumentValueType::kLong}; std::unique_ptr metric_storage(new SyncMultiMetricStorage()); - DoubleGauge gauge(instrument_descriptor, std::move(metric_storage)); - gauge.Record(10, opentelemetry::context::Context{}); - gauge.Record(std::numeric_limits::quiet_NaN(), - opentelemetry::context::Context{}); // This is ignored too - gauge.Record(std::numeric_limits::infinity(), - opentelemetry::context::Context{}); // This is ignored too - - gauge.Record(10, opentelemetry::common::KeyValueIterableView({{"abc", "123"}, {"xyz", "456"}}), - opentelemetry::context::Context{}); - gauge.Record(10, opentelemetry::common::KeyValueIterableView({}), - opentelemetry::context::Context{}); + auto gauge = std::unique_ptr>( + new LongGauge(instrument_descriptor, std::move(metric_storage))); + EXPECT_NO_THROW(gauge->Record(10, opentelemetry::context::Context{})); + EXPECT_NO_THROW(gauge->Record(std::numeric_limits::quiet_NaN(), + opentelemetry::context::Context{})); // This is ignored too + EXPECT_NO_THROW(gauge->Record(std::numeric_limits::infinity(), + opentelemetry::context::Context{})); // This is ignored too + + EXPECT_NO_THROW(gauge->Record( + 10, opentelemetry::common::KeyValueIterableView({{"abc", "123"}, {"xyz", "456"}}), + opentelemetry::context::Context{})); + EXPECT_NO_THROW(gauge->Record(10, opentelemetry::common::KeyValueIterableView({}), + opentelemetry::context::Context{})); } #endif \ No newline at end of file From 127b1071febbef0743dfb671f307fb5d1778b52a Mon Sep 17 00:00:00 2001 From: Lalit Date: Sat, 30 Sep 2023 23:50:25 -0700 Subject: [PATCH 10/13] add test --- exporters/otlp/src/otlp_metric_utils.cc | 5 +- sdk/test/metrics/BUILD | 16 +++ sdk/test/metrics/CMakeLists.txt | 1 + sdk/test/metrics/sync_instruments_test.cc | 2 +- .../metrics/sync_metric_storage_gauge_test.cc | 111 ++++++++++++++++++ 5 files changed, 133 insertions(+), 2 deletions(-) create mode 100644 sdk/test/metrics/sync_metric_storage_gauge_test.cc diff --git a/exporters/otlp/src/otlp_metric_utils.cc b/exporters/otlp/src/otlp_metric_utils.cc index 50c5fdd9cc..2d82dcce28 100644 --- a/exporters/otlp/src/otlp_metric_utils.cc +++ b/exporters/otlp/src/otlp_metric_utils.cc @@ -39,7 +39,8 @@ metric_sdk::AggregationType OtlpMetricUtils::GetAggregationType( { return metric_sdk::AggregationType::kHistogram; } - else if (instrument_type == metric_sdk::InstrumentType::kObservableGauge) + else if (instrument_type == metric_sdk::InstrumentType::kObservableGauge || + instrument_type == metric_sdk::InstrumentTye::kGauge) { return metric_sdk::AggregationType::kLastValue; } @@ -269,6 +270,7 @@ sdk::metrics::AggregationTemporality OtlpMetricUtils::DeltaTemporalitySelector( case sdk::metrics::InstrumentType::kObservableCounter: case sdk::metrics::InstrumentType::kHistogram: case sdk::metrics::InstrumentType::kObservableGauge: + case sdk::metrics::InstrumentType::kGauge: return sdk::metrics::AggregationTemporality::kDelta; case sdk::metrics::InstrumentType::kUpDownCounter: case sdk::metrics::InstrumentType::kObservableUpDownCounter: @@ -292,6 +294,7 @@ sdk::metrics::AggregationTemporality OtlpMetricUtils::LowMemoryTemporalitySelect case sdk::metrics::InstrumentType::kHistogram: return sdk::metrics::AggregationTemporality::kDelta; case sdk::metrics::InstrumentType::kObservableCounter: + case sdk::metrics::InstrumentType::kGauge: case sdk::metrics::InstrumentType::kObservableGauge: case sdk::metrics::InstrumentType::kUpDownCounter: case sdk::metrics::InstrumentType::kObservableUpDownCounter: diff --git a/sdk/test/metrics/BUILD b/sdk/test/metrics/BUILD index 3cf379494d..06e277f4a7 100644 --- a/sdk/test/metrics/BUILD +++ b/sdk/test/metrics/BUILD @@ -161,6 +161,22 @@ cc_test( ], ) +cc_test( + name = "sync_metric_storage_gauge_test", + srcs = [ + "sync_metric_storage_gauge_test.cc", + ], + tags = [ + "metrics", + "test", + ], + deps = [ + "metrics_common_test_utils", + "//sdk/src/resource", + "@com_google_googletest//:gtest_main", + ], +) + cc_test( name = "sync_instruments_test", srcs = [ diff --git a/sdk/test/metrics/CMakeLists.txt b/sdk/test/metrics/CMakeLists.txt index b30d71ae22..019900a550 100644 --- a/sdk/test/metrics/CMakeLists.txt +++ b/sdk/test/metrics/CMakeLists.txt @@ -18,6 +18,7 @@ foreach( circular_buffer_counter_test histogram_test sync_metric_storage_counter_test + sync_metric_storage_gauge_test sync_metric_storage_histogram_test sync_metric_storage_up_down_counter_test async_metric_storage_test diff --git a/sdk/test/metrics/sync_instruments_test.cc b/sdk/test/metrics/sync_instruments_test.cc index d4cabbd37e..cf0e661aaf 100644 --- a/sdk/test/metrics/sync_instruments_test.cc +++ b/sdk/test/metrics/sync_instruments_test.cc @@ -180,4 +180,4 @@ TEST(SyncInstruments, LongGauge) EXPECT_NO_THROW(gauge->Record(10, opentelemetry::common::KeyValueIterableView({}), opentelemetry::context::Context{})); } -#endif \ No newline at end of file +#endif diff --git a/sdk/test/metrics/sync_metric_storage_gauge_test.cc b/sdk/test/metrics/sync_metric_storage_gauge_test.cc new file mode 100644 index 0000000000..ec2223da89 --- /dev/null +++ b/sdk/test/metrics/sync_metric_storage_gauge_test.cc @@ -0,0 +1,111 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +#include "common.h" + +#include "opentelemetry/common/key_value_iterable_view.h" +#include "opentelemetry/nostd/shared_ptr.h" +#include "opentelemetry/sdk/metrics/exemplar/histogram_exemplar_reservoir.h" +#include "opentelemetry/sdk/metrics/instruments.h" +#include "opentelemetry/sdk/metrics/state/sync_metric_storage.h" +#include "opentelemetry/sdk/metrics/view/attributes_processor.h" + +#include +#include +#include + +using namespace opentelemetry::sdk::metrics; +using namespace opentelemetry::common; +using M = std::map; +namespace nostd = opentelemetry::nostd; + +class WritableMetricStorageGaugeTestFixture + : public ::testing::TestWithParam +{}; + +TEST_P(WritableMetricStorageGaugeTestFixture, LongGaugeLastValueAggregation) +{ +#if OPENTELEMETRY_ABI_VERSION_NO >= 2 + AggregationTemporality temporality = GetParam(); + auto sdk_start_ts = std::chrono::system_clock::now(); + InstrumentDescriptor instr_desc = {"name", "desc", "1unit", InstrumentType::kGauge, + InstrumentValueType::kLong}; + std::map attributes_roomA = {{"Room.Id", "Rack A"}}; + std::map attributes_roomB = {{"Room.Id", "Rack B"}}; + + std::unique_ptr default_attributes_processor{ + new DefaultAttributesProcessor{}}; + opentelemetry::sdk::metrics::SyncMetricStorage storage( + instr_desc, AggregationType::kLastValue, default_attributes_processor.get(), + ExemplarReservoir::GetNoExemplarReservoir(), nullptr); + + uint64_t noiselevel_1_roomA = 10, noiselevel_1_roomB = 30; + storage.RecordLong(noiselevel_1_roomA, + KeyValueIterableView>(attributes_roomA), + opentelemetry::context::Context{}); + storage.RecordLong(noiselevel_1_roomB, + KeyValueIterableView>(attributes_roomB), + opentelemetry::context::Context{}); + + uint64_t noiselevel_2_roomA = 12, noiselevel_2_roomB = 29; + storage.RecordLong(noiselevel_2_roomA, + KeyValueIterableView>(attributes_roomA), + opentelemetry::context::Context{}); + storage.RecordLong(noiselevel_2_roomB, + KeyValueIterableView>(attributes_roomB), + opentelemetry::context::Context{}); + + std::shared_ptr collector(new MockCollectorHandle(temporality)); + std::vector> collectors; + collectors.push_back(collector); + + // Some computation here + auto collection_ts = std::chrono::system_clock::now(); + size_t count_attributes = 0; + storage.Collect( + collector.get(), collectors, sdk_start_ts, collection_ts, [&](const MetricData data) { + for (auto data_attr : data.point_data_attr_) + { + auto lastvalue_data = opentelemetry::nostd::get(data_attr.point_data); + if (opentelemetry::nostd::get( + data_attr.attributes.find("Room.Id")->second) == "Rack A") + { + if (temporality == AggregationTemporality::kCumulative) + { + EXPECT_EQ(opentelemetry::nostd::get(lastvalue_data.value_), + noiselevel_2_roomA); + } + else + { + EXPECT_EQ(opentelemetry::nostd::get(lastvalue_data.value_), + noiselevel_2_roomA - noiselevel_1_roomA); + } + count_attributes++; + } + else if (opentelemetry::nostd::get( + data_attr.attributes.find("Room.Id")->second) == "Rack B") + { + if (temporality == AggregationTemporality::kCumulative) + { + EXPECT_EQ(opentelemetry::nostd::get(lastvalue_data.value_), + noiselevel_2_roomB); + } + else + { + EXPECT_EQ(opentelemetry::nostd::get(lastvalue_data.value_), + noiselevel_2_roomB - noiselevel_1_roomB); + } + count_attributes++; + } + } + return true; + }); + EXPECT_EQ(count_attributes, 2); // RackA and RackB +#else + EXPECT_TRUE(true); +#endif +} + +INSTANTIATE_TEST_SUITE_P(WritableMetricStorageTestLong, + WritableMetricStorageGaugeTestFixture, + ::testing::Values(AggregationTemporality::kCumulative)); From e6abe54a7d1f16e428e01f8a87e5ab3055d4f4db Mon Sep 17 00:00:00 2001 From: Lalit Date: Sun, 1 Oct 2023 00:02:19 -0700 Subject: [PATCH 11/13] fix compile error --- exporters/otlp/src/otlp_metric_utils.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/exporters/otlp/src/otlp_metric_utils.cc b/exporters/otlp/src/otlp_metric_utils.cc index 2d82dcce28..f78a9618a3 100644 --- a/exporters/otlp/src/otlp_metric_utils.cc +++ b/exporters/otlp/src/otlp_metric_utils.cc @@ -40,7 +40,7 @@ metric_sdk::AggregationType OtlpMetricUtils::GetAggregationType( return metric_sdk::AggregationType::kHistogram; } else if (instrument_type == metric_sdk::InstrumentType::kObservableGauge || - instrument_type == metric_sdk::InstrumentTye::kGauge) + instrument_type == metric_sdk::InstrumentType::kGauge) { return metric_sdk::AggregationType::kLastValue; } From 03c3cab9b0a7b090c71e6ace4806572b3e707c50 Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Sun, 1 Oct 2023 19:14:59 -0700 Subject: [PATCH 12/13] fix unused variable --- api/include/opentelemetry/metrics/noop.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/api/include/opentelemetry/metrics/noop.h b/api/include/opentelemetry/metrics/noop.h index 5483748f86..ed61e43888 100644 --- a/api/include/opentelemetry/metrics/noop.h +++ b/api/include/opentelemetry/metrics/noop.h @@ -45,7 +45,8 @@ class NoopHistogram : public Histogram 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*/, + const opentelemetry::common::KeyValueIterable & /*attributes*/) noexcept override {} void Record(T value) noexcept override {} From 12489288afd0e5ecce3f94af96acd2483cbf3d5b Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Sun, 1 Oct 2023 19:53:10 -0700 Subject: [PATCH 13/13] more unused vars --- api/include/opentelemetry/metrics/noop.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/include/opentelemetry/metrics/noop.h b/api/include/opentelemetry/metrics/noop.h index ed61e43888..72140e5b40 100644 --- a/api/include/opentelemetry/metrics/noop.h +++ b/api/include/opentelemetry/metrics/noop.h @@ -49,7 +49,7 @@ class NoopHistogram : public Histogram const opentelemetry::common::KeyValueIterable & /*attributes*/) noexcept override {} - void Record(T value) noexcept override {} + void Record(T /*value*/) noexcept override {} #endif };