From a03868070ae9766e7f4e6a69f51384bcc271c2d7 Mon Sep 17 00:00:00 2001 From: Lalit Date: Mon, 6 Jun 2022 10:56:19 -0700 Subject: [PATCH 01/26] multi instrument callback --- .../opentelemetry/metrics/async_instruments.h | 16 +++++++++- .../opentelemetry/metrics/observer_result.h | 30 +++++++++++++++++++ 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/api/include/opentelemetry/metrics/async_instruments.h b/api/include/opentelemetry/metrics/async_instruments.h index 065a47649a..d89cd92205 100644 --- a/api/include/opentelemetry/metrics/async_instruments.h +++ b/api/include/opentelemetry/metrics/async_instruments.h @@ -9,8 +9,22 @@ OPENTELEMETRY_BEGIN_NAMESPACE namespace metrics { + +template class AsynchronousInstrument -{}; +{ +public: + + /** + * Sets up a function that will be called whenever a metric collection is initiated. + */ + virtual void AddCallback(void (*callback)(ObserverResult &, void *), void *state); + + /** + * Sets up a function that will be called whenever a metric collection is initiated. + */ + virtual void RemoveCallback(void (*callback)(ObserverResult &, void *), void *state); +}; template class ObservableCounter : public AsynchronousInstrument diff --git a/api/include/opentelemetry/metrics/observer_result.h b/api/include/opentelemetry/metrics/observer_result.h index 84893deef0..aba762557f 100644 --- a/api/include/opentelemetry/metrics/observer_result.h +++ b/api/include/opentelemetry/metrics/observer_result.h @@ -44,6 +44,36 @@ class ObserverResult } }; +/** + * BatchObserverResult class is necessary for the batch callback recording asynchronous + * instrument use. + */ + +template +class BatchObserverResult +{ + +public: + virtual void Observe(Observable& metric, T value) noexcept = 0; + + virtual void Observe(Observable& metric, T value, const common::KeyValueIterable &attributes) noexcept = 0; + + template ::value> * = nullptr> + void Observe(Observable& metric, T value, const U &attributes) noexcept + { + this->Observe(metric, value, common::KeyValueIterableView{attributes}); + } + + void Observe(Observable& metric, T value, + std::initializer_list> + attributes) noexcept + { + this->Observe(metric, value, nostd::span>{ + attributes.begin(), attributes.end()}); + } +}; + } // namespace metrics OPENTELEMETRY_END_NAMESPACE #endif From 93b9509fb9a3f23428e82688550c780679f65bf3 Mon Sep 17 00:00:00 2001 From: Lalit Date: Mon, 6 Jun 2022 22:12:16 -0700 Subject: [PATCH 02/26] more changes --- .../opentelemetry/metrics/async_instruments.h | 13 +++++++++---- .../opentelemetry/metrics/observer_result.h | 16 ++++++++++------ .../sdk/metrics/async_instruments.h | 15 +++++++++++++++ .../sdk/metrics/state/observable_registry.h | 18 ++++++++++++++++++ 4 files changed, 52 insertions(+), 10 deletions(-) create mode 100644 sdk/include/opentelemetry/sdk/metrics/state/observable_registry.h diff --git a/api/include/opentelemetry/metrics/async_instruments.h b/api/include/opentelemetry/metrics/async_instruments.h index d89cd92205..3946304f0b 100644 --- a/api/include/opentelemetry/metrics/async_instruments.h +++ b/api/include/opentelemetry/metrics/async_instruments.h @@ -11,19 +11,24 @@ namespace metrics { template +using ObservableCallbackPtr = void (*callback)(ObserverResult &, void *); + class AsynchronousInstrument +{}; + +template +class ObservableInstrument { public: - /** * Sets up a function that will be called whenever a metric collection is initiated. */ - virtual void AddCallback(void (*callback)(ObserverResult &, void *), void *state); - + virtual void AddCallback(ObservableCallbackPtr, void *state); + /** * Sets up a function that will be called whenever a metric collection is initiated. */ - virtual void RemoveCallback(void (*callback)(ObserverResult &, void *), void *state); + virtual void RemoveCallback(ObservableCallbackPtr, void *state); }; template diff --git a/api/include/opentelemetry/metrics/observer_result.h b/api/include/opentelemetry/metrics/observer_result.h index aba762557f..ef0c8df744 100644 --- a/api/include/opentelemetry/metrics/observer_result.h +++ b/api/include/opentelemetry/metrics/observer_result.h @@ -54,23 +54,27 @@ class BatchObserverResult { public: - virtual void Observe(Observable& metric, T value) noexcept = 0; + virtual void Observe(Observable &metric, T value) noexcept = 0; - virtual void Observe(Observable& metric, T value, const common::KeyValueIterable &attributes) noexcept = 0; + virtual void Observe(Observable &metric, + T value, + const common::KeyValueIterable &attributes) noexcept = 0; template ::value> * = nullptr> - void Observe(Observable& metric, T value, const U &attributes) noexcept + void Observe(Observable &metric, T value, const U &attributes) noexcept { this->Observe(metric, value, common::KeyValueIterableView{attributes}); } - void Observe(Observable& metric, T value, + void Observe(Observable &metric, + T value, std::initializer_list> attributes) noexcept { - this->Observe(metric, value, nostd::span>{ - attributes.begin(), attributes.end()}); + this->Observe(metric, value, + nostd::span>{ + attributes.begin(), attributes.end()}); } }; diff --git a/sdk/include/opentelemetry/sdk/metrics/async_instruments.h b/sdk/include/opentelemetry/sdk/metrics/async_instruments.h index 8b1f76377b..13b312fa6f 100644 --- a/sdk/include/opentelemetry/sdk/metrics/async_instruments.h +++ b/sdk/include/opentelemetry/sdk/metrics/async_instruments.h @@ -13,6 +13,21 @@ namespace sdk namespace metrics { +template +class ObservableInstrument +{ +public: + /** + * Sets up a function that will be called whenever a metric collection is initiated. + */ + virtual void AddCallback(ObservableCallbackPtr, void *state); + + /** + * Sets up a function that will be called whenever a metric collection is initiated. + */ + virtual void RemoveCallback(ObservableCallbackPtr, void *state); +}; + template class Asynchronous { diff --git a/sdk/include/opentelemetry/sdk/metrics/state/observable_registry.h b/sdk/include/opentelemetry/sdk/metrics/state/observable_registry.h new file mode 100644 index 0000000000..0a5e45d2b7 --- /dev/null +++ b/sdk/include/opentelemetry/sdk/metrics/state/observable_registry.h @@ -0,0 +1,18 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +#pragma once +#ifndef ENABLE_METRICS_PREVIEW +# include "opentelemetry/common/key_value_iterable_view.h" +# include "opentelemetry/common/timestamp.h" +# include "opentelemetry/context/context.h" +# include "opentelemetry/sdk/metrics/data/metric_data.h" +# include "opentelemetry/sdk/metrics/instruments.h" +OPENTELEMETRY_BEGIN_NAMESPACE +namespace sdk +{ +namespace metrics +{} // namespace metrics +} // namespace sdk +OPENTELEMETRY_END_NAMESPACE +#endif \ No newline at end of file From 723d27a965005e06bdb9fea6b162b2e3312353c0 Mon Sep 17 00:00:00 2001 From: Lalit Date: Mon, 6 Jun 2022 23:43:23 -0700 Subject: [PATCH 03/26] change --- .../sdk/metrics/state/observable_registry.h | 22 ++++++++++++++----- 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/metrics/state/observable_registry.h b/sdk/include/opentelemetry/sdk/metrics/state/observable_registry.h index 0a5e45d2b7..04ce18b734 100644 --- a/sdk/include/opentelemetry/sdk/metrics/state/observable_registry.h +++ b/sdk/include/opentelemetry/sdk/metrics/state/observable_registry.h @@ -3,16 +3,26 @@ #pragma once #ifndef ENABLE_METRICS_PREVIEW -# include "opentelemetry/common/key_value_iterable_view.h" -# include "opentelemetry/common/timestamp.h" -# include "opentelemetry/context/context.h" -# include "opentelemetry/sdk/metrics/data/metric_data.h" -# include "opentelemetry/sdk/metrics/instruments.h" + +# include "opentelemetry/sdk/metrics/async_instruments.h" + +# include > OPENTELEMETRY_BEGIN_NAMESPACE namespace sdk { namespace metrics -{} // namespace metrics +{ + +struct ObservableCallbackRecord +{ + ObservableCallbackPtr callback_; + std::unordered_set +}; + +class ObservableRegistry +{}; + +} // namespace metrics } // namespace sdk OPENTELEMETRY_END_NAMESPACE #endif \ No newline at end of file From a4fd8583a4cfc1aafc6c5b68ad23ff80a04d5917 Mon Sep 17 00:00:00 2001 From: Lalit Date: Tue, 7 Jun 2022 23:28:16 -0700 Subject: [PATCH 04/26] more changes --- .../sdk/metrics/state/observable_registry.h | 33 ++++++++++++++++--- 1 file changed, 29 insertions(+), 4 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/metrics/state/observable_registry.h b/sdk/include/opentelemetry/sdk/metrics/state/observable_registry.h index 04ce18b734..d471af1861 100644 --- a/sdk/include/opentelemetry/sdk/metrics/state/observable_registry.h +++ b/sdk/include/opentelemetry/sdk/metrics/state/observable_registry.h @@ -6,7 +6,10 @@ # include "opentelemetry/sdk/metrics/async_instruments.h" -# include > +# include +# include +# include + OPENTELEMETRY_BEGIN_NAMESPACE namespace sdk { @@ -15,12 +18,34 @@ namespace metrics struct ObservableCallbackRecord { - ObservableCallbackPtr callback_; - std::unordered_set + ObservableCallbackPtr callback; + ObservableInstrument *instrument; }; class ObservableRegistry -{}; +{ +public: + void AddCallback(ObservableCallbackPtr callback, ObservableInstrument *instrument) + { + // TBD - Check if existing + auto record = + std::unique_ptr(ObservableCallbackRecord{callback, instrument}); + callbacks_.push_back(std::move(record)); + } + + void RemoveCallback(ObservableCallbackPtr callback, ObservableInstrument *instrument) + { + auto new_end = std::remove_if( + callbacks_.begin(), callbacks_.end(), + [callback, instrument](const std::unique_ptr &record) { + return record->callback == callback && record->instrument == instrument; + }); + callbacks_.erase(new_end, callbacks_.end()); + } + +private: + std::vector> callbacks_; +}; } // namespace metrics } // namespace sdk From c605ef4ec77008e5ee7d5053232681ff214d023a Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Thu, 30 Jun 2022 17:56:18 -0700 Subject: [PATCH 05/26] fix --- .../opentelemetry/metrics/async_instruments.h | 4 +- api/include/opentelemetry/metrics/meter.h | 67 +++++------- api/include/opentelemetry/metrics/noop.h | 101 ++++++++---------- .../opentelemetry/metrics/observer_result.h | 39 +------ .../sdk/metrics/async_instruments.h | 22 ++-- sdk/include/opentelemetry/sdk/metrics/meter.h | 81 ++++---------- .../sdk/metrics/state/metric_storage.h | 13 +++ sdk/src/metrics/meter.cc | 91 +++++++++------- 8 files changed, 168 insertions(+), 250 deletions(-) diff --git a/api/include/opentelemetry/metrics/async_instruments.h b/api/include/opentelemetry/metrics/async_instruments.h index 3946304f0b..331524ac85 100644 --- a/api/include/opentelemetry/metrics/async_instruments.h +++ b/api/include/opentelemetry/metrics/async_instruments.h @@ -10,13 +10,11 @@ OPENTELEMETRY_BEGIN_NAMESPACE namespace metrics { -template -using ObservableCallbackPtr = void (*callback)(ObserverResult &, void *); +typedef void (*ObservableCallbackPtr)(ObserverResult &, void *); class AsynchronousInstrument {}; -template class ObservableInstrument { public: diff --git a/api/include/opentelemetry/metrics/meter.h b/api/include/opentelemetry/metrics/meter.h index e0454e360b..b0c3e099c6 100644 --- a/api/include/opentelemetry/metrics/meter.h +++ b/api/include/opentelemetry/metrics/meter.h @@ -51,22 +51,18 @@ class Meter * shared_ptr to that Observable Counter * * @param name the name of the new Observable Counter. - * @param callback the function to be observed by the instrument. * @param description a brief description of what the Observable Counter is used for. * @param unit the unit of metric values following https://unitsofmeasure.org/ucum.html. - * @param state to be passed back to callback */ - virtual void CreateLongObservableCounter(nostd::string_view name, - void (*callback)(ObserverResult &, void *), - nostd::string_view description = "", - nostd::string_view unit = "", - void *state = nullptr) noexcept = 0; - - virtual void CreateDoubleObservableCounter(nostd::string_view name, - void (*callback)(ObserverResult &, void *), - nostd::string_view description = "", - nostd::string_view unit = "", - void *state = nullptr) noexcept = 0; + virtual nostd::shared_ptr> CreateLongObservableCounter( + nostd::string_view name, + nostd::string_view description = "", + nostd::string_view unit = "") noexcept = 0; + + virtual nostd::shared_ptr> CreateDoubleObservableCounter( + nostd::string_view name, + nostd::string_view description = "", + nostd::string_view unit = "") noexcept = 0; /** * Creates a Histogram with the passed characteristics and returns a shared_ptr to that Histogram. @@ -91,22 +87,18 @@ class Meter * shared_ptr to that Observable Counter * * @param name the name of the new Observable Gauge. - * @param callback the function to be observed by the instrument. * @param description a brief description of what the Observable Gauge is used for. * @param unit the unit of metric values following https://unitsofmeasure.org/ucum.html. - * @param state to be passed back to callback */ - virtual void CreateLongObservableGauge(nostd::string_view name, - void (*callback)(ObserverResult &, void *), - nostd::string_view description = "", - nostd::string_view unit = "", - void *state = nullptr) noexcept = 0; - - virtual void CreateDoubleObservableGauge(nostd::string_view name, - void (*callback)(ObserverResult &, void *), - nostd::string_view description = "", - nostd::string_view unit = "", - void *state = nullptr) noexcept = 0; + virtual nostd::shared_ptr> CreateLongObservableGauge( + nostd::string_view name, + nostd::string_view description = "", + nostd::string_view unit = "") noexcept = 0; + + virtual nostd::shared_ptr> CreateDoubleObservableGauge( + nostd::string_view name, + nostd::string_view description = "", + nostd::string_view unit = "") noexcept = 0; /** * Creates an UpDownCounter with the passed characteristics and returns a shared_ptr to that @@ -132,23 +124,18 @@ class Meter * a shared_ptr to that Observable UpDownCounter * * @param name the name of the new Observable UpDownCounter. - * @param callback the function to be observed by the instrument. * @param description a brief description of what the Observable UpDownCounter is used for. * @param unit the unit of metric values following https://unitsofmeasure.org/ucum.html. - * @param state to be passed back to callback */ - virtual void CreateLongObservableUpDownCounter(nostd::string_view name, - void (*callback)(ObserverResult &, void *), - nostd::string_view description = "", - nostd::string_view unit = "", - void *state = nullptr) noexcept = 0; - - virtual void CreateDoubleObservableUpDownCounter(nostd::string_view name, - void (*callback)(ObserverResult &, - void *), - nostd::string_view description = "", - nostd::string_view unit = "", - void *state = nullptr) noexcept = 0; + virtual nostd::shared_ptr> CreateLongObservableUpDownCounter( + nostd::string_view name, + nostd::string_view description = "", + nostd::string_view unit = "") noexcept = 0; + + virtual nostd::shared_ptr> CreateDoubleObservableUpDownCounter( + nostd::string_view name, + nostd::string_view description = "", + nostd::string_view unit = "") noexcept = 0; }; } // namespace metrics OPENTELEMETRY_END_NAMESPACE diff --git a/api/include/opentelemetry/metrics/noop.h b/api/include/opentelemetry/metrics/noop.h index c47f7489f0..d8348761ec 100644 --- a/api/include/opentelemetry/metrics/noop.h +++ b/api/include/opentelemetry/metrics/noop.h @@ -69,13 +69,6 @@ class NoopObservableCounter : public ObservableCounter { public: NoopObservableCounter(nostd::string_view name, - void (*callback)(ObserverResult &), - nostd::string_view description, - nostd::string_view unit) noexcept - {} - - NoopObservableCounter(nostd::string_view name, - void (*callback)(ObserverResult &, const common::KeyValueIterable &), nostd::string_view description, nostd::string_view unit) noexcept {} @@ -86,13 +79,6 @@ class NoopObservableGauge : public ObservableGauge { public: NoopObservableGauge(nostd::string_view name, - void (*callback)(ObserverResult &), - nostd::string_view description, - nostd::string_view unit) noexcept - {} - - NoopObservableGauge(nostd::string_view name, - void (*callback)(ObserverResult &, const common::KeyValueIterable &), nostd::string_view description, nostd::string_view unit) noexcept {} @@ -103,14 +89,6 @@ class NoopObservableUpDownCounter : public ObservableUpDownCounter { public: NoopObservableUpDownCounter(nostd::string_view name, - void (*callback)(ObserverResult &), - nostd::string_view description, - nostd::string_view unit) noexcept - {} - - NoopObservableUpDownCounter(nostd::string_view name, - void (*callback)(ObserverResult &, - const common::KeyValueIterable &), nostd::string_view description, nostd::string_view unit) noexcept {} @@ -137,19 +115,21 @@ class NoopMeter final : public Meter return nostd::shared_ptr>{new NoopCounter(name, description, unit)}; } - void CreateLongObservableCounter(nostd::string_view name, - void (*callback)(ObserverResult &, void *), - nostd::string_view description = "", - nostd::string_view unit = "", - void *state = nullptr) noexcept override - {} + nostd::shared_ptr> CreateLongObservableCounter( + nostd::string_view name, + nostd::string_view description = "", + nostd::string_view unit = "") noexcept override + { + return nostd::shared_ptr>(new ObservableCounter()); + } - void CreateDoubleObservableCounter(nostd::string_view name, - void (*callback)(ObserverResult &, void *), - nostd::string_view description = "", - nostd::string_view unit = "", - void *state = nullptr) noexcept override - {} + nostd::shared_ptr> CreateDoubleObservableCounter( + nostd::string_view name, + nostd::string_view description = "", + nostd::string_view unit = "") noexcept override + { + return nostd::shared_ptr>(new ObservableCounter()); + } nostd::shared_ptr> CreateLongHistogram( nostd::string_view name, @@ -167,19 +147,21 @@ class NoopMeter final : public Meter return nostd::shared_ptr>{new NoopHistogram(name, description, unit)}; } - void CreateLongObservableGauge(nostd::string_view name, - void (*callback)(ObserverResult &, void *), - nostd::string_view description = "", - nostd::string_view unit = "", - void *state = nullptr) noexcept override - {} + nostd::shared_ptr> CreateLongObservableGauge( + nostd::string_view name, + nostd::string_view description = "", + nostd::string_view unit = "") noexcept override + { + return nostd::shared_ptr>(new ObservableGauge()); + } - void CreateDoubleObservableGauge(nostd::string_view name, - void (*callback)(ObserverResult &, void *), - nostd::string_view description = "", - nostd::string_view unit = "", - void *state = nullptr) noexcept override - {} + nostd::shared_ptr> CreateDoubleObservableGauge( + nostd::string_view name, + nostd::string_view description = "", + nostd::string_view unit = "") noexcept override + { + return nostd::shared_ptr>(new ObservableGauge()); + } nostd::shared_ptr> CreateLongUpDownCounter( nostd::string_view name, @@ -199,19 +181,22 @@ class NoopMeter final : public Meter new NoopUpDownCounter(name, description, unit)}; } - void CreateLongObservableUpDownCounter(nostd::string_view name, - void (*callback)(ObserverResult &, void *), - nostd::string_view description = "", - nostd::string_view unit = "", - void *state = nullptr) noexcept override - {} + nostd::shared_ptr> CreateLongObservableUpDownCounter( + nostd::string_view name, + nostd::string_view description = "", + nostd::string_view unit = "") noexcept override + { + return nostd::shared_ptr>(new ObservableUpDownCounter()); + } - void CreateDoubleObservableUpDownCounter(nostd::string_view name, - void (*callback)(ObserverResult &, void *), - nostd::string_view description = "", - nostd::string_view unit = "", - void *state = nullptr) noexcept override - {} + nostd::shared_ptr> CreateDoubleObservableUpDownCounter( + nostd::string_view name, + nostd::string_view description = "", + nostd::string_view unit = "") noexcept override + { + return nostd::shared_ptr>( + new ObservableUpDownCounter()); + } }; /** diff --git a/api/include/opentelemetry/metrics/observer_result.h b/api/include/opentelemetry/metrics/observer_result.h index ef0c8df744..2f09beab11 100644 --- a/api/include/opentelemetry/metrics/observer_result.h +++ b/api/include/opentelemetry/metrics/observer_result.h @@ -15,12 +15,11 @@ namespace metrics { /** - * ObserverResult class is necessary for the callback recording asynchronous + * ObserverResultT class is necessary for the callback recording asynchronous * instrument use. */ - template -class ObserverResult +class ObserverResultT { public: @@ -44,39 +43,7 @@ class ObserverResult } }; -/** - * BatchObserverResult class is necessary for the batch callback recording asynchronous - * instrument use. - */ - -template -class BatchObserverResult -{ - -public: - virtual void Observe(Observable &metric, T value) noexcept = 0; - - virtual void Observe(Observable &metric, - T value, - const common::KeyValueIterable &attributes) noexcept = 0; - - template ::value> * = nullptr> - void Observe(Observable &metric, T value, const U &attributes) noexcept - { - this->Observe(metric, value, common::KeyValueIterableView{attributes}); - } - - void Observe(Observable &metric, - T value, - std::initializer_list> - attributes) noexcept - { - this->Observe(metric, value, - nostd::span>{ - attributes.begin(), attributes.end()}); - } -}; +using ObserverResult = nostd::variant, ObserverResultT>; } // namespace metrics OPENTELEMETRY_END_NAMESPACE diff --git a/sdk/include/opentelemetry/sdk/metrics/async_instruments.h b/sdk/include/opentelemetry/sdk/metrics/async_instruments.h index 13b312fa6f..e045781716 100644 --- a/sdk/include/opentelemetry/sdk/metrics/async_instruments.h +++ b/sdk/include/opentelemetry/sdk/metrics/async_instruments.h @@ -33,15 +33,13 @@ class Asynchronous { public: Asynchronous(nostd::string_view name, - void (*callback)(opentelemetry::metrics::ObserverResult &), nostd::string_view description = "", nostd::string_view unit = "") - : name_(name), callback_(callback), description_(description), unit_(unit) + : name_(name), description_(description), unit_(unit) {} protected: std::string name_; - void (*callback_)(opentelemetry::metrics::ObserverResult &); std::string description_; std::string unit_; }; @@ -51,10 +49,9 @@ class LongObservableCounter : public opentelemetry::metrics::ObservableCounter &), nostd::string_view description = "", nostd::string_view unit = "") - : Asynchronous(name, callback, description, unit) + : Asynchronous(name, description, unit) {} }; @@ -64,10 +61,9 @@ class DoubleObservableCounter : public opentelemetry::metrics::ObservableCounter { public: DoubleObservableCounter(nostd::string_view name, - void (*callback)(opentelemetry::metrics::ObserverResult &), nostd::string_view description = "", nostd::string_view unit = "") - : Asynchronous(name, callback, description, unit) + : Asynchronous(name, description, unit) {} }; @@ -77,10 +73,9 @@ class LongObservableGauge : public opentelemetry::metrics::ObservableGauge { public: LongObservableGauge(nostd::string_view name, - void (*callback)(opentelemetry::metrics::ObserverResult &), nostd::string_view description = "", nostd::string_view unit = "") - : Asynchronous(name, callback, description, unit) + : Asynchronous(name, description, unit) {} }; @@ -90,10 +85,9 @@ class DoubleObservableGauge : public opentelemetry::metrics::ObservableGauge &), nostd::string_view description = "", nostd::string_view unit = "") - : Asynchronous(name, callback, description, unit) + : Asynchronous(name, description, unit) {} }; @@ -103,10 +97,9 @@ class LongObservableUpDownCounter : public opentelemetry::metrics::ObservableUpD { public: LongObservableUpDownCounter(nostd::string_view name, - void (*callback)(opentelemetry::metrics::ObserverResult &), nostd::string_view description = "", nostd::string_view unit = "") - : Asynchronous(name, callback, description, unit) + : Asynchronous(name, description, unit) {} }; @@ -117,10 +110,9 @@ class DoubleObservableUpDownCounter { public: DoubleObservableUpDownCounter(nostd::string_view name, - void (*callback)(opentelemetry::metrics::ObserverResult &), nostd::string_view description = "", nostd::string_view unit = "") - : Asynchronous(name, callback, description, unit) + : Asynchronous(name, description, unit) {} }; diff --git a/sdk/include/opentelemetry/sdk/metrics/meter.h b/sdk/include/opentelemetry/sdk/metrics/meter.h index 44e54adf18..a1ae9af443 100644 --- a/sdk/include/opentelemetry/sdk/metrics/meter.h +++ b/sdk/include/opentelemetry/sdk/metrics/meter.h @@ -42,19 +42,15 @@ class Meter final : public opentelemetry::metrics::Meter nostd::string_view description = "", nostd::string_view unit = "") noexcept override; - void CreateLongObservableCounter(nostd::string_view name, - void (*callback)(opentelemetry::metrics::ObserverResult &, - void *), - nostd::string_view description = "", - nostd::string_view unit = "", - void *state = nullptr) noexcept override; - - void CreateDoubleObservableCounter( + nostd::shared_ptr> CreateLongObservableCounter( nostd::string_view name, - void (*callback)(opentelemetry::metrics::ObserverResult &, void *), nostd::string_view description = "", - nostd::string_view unit = "", - void *state = nullptr) noexcept override; + nostd::string_view unit = "") noexcept override; + + nostd::shared_ptr> + CreateDoubleObservableCounter(nostd::string_view name, + nostd::string_view description = "", + nostd::string_view unit = "") noexcept override; nostd::shared_ptr> CreateLongHistogram( nostd::string_view name, @@ -66,19 +62,15 @@ class Meter final : public opentelemetry::metrics::Meter nostd::string_view description = "", nostd::string_view unit = "") noexcept override; - void CreateLongObservableGauge(nostd::string_view name, - void (*callback)(opentelemetry::metrics::ObserverResult &, - void *), - nostd::string_view description = "", - nostd::string_view unit = "", - void *state = nullptr) noexcept override; + nostd::shared_ptr> CreateLongObservableGauge( + nostd::string_view name, + nostd::string_view description = "", + nostd::string_view unit = "") noexcept override; - void CreateDoubleObservableGauge( + nostd::shared_ptr> CreateDoubleObservableGauge( nostd::string_view name, - void (*callback)(opentelemetry::metrics::ObserverResult &, void *), nostd::string_view description = "", - nostd::string_view unit = "", - void *state = nullptr) noexcept override; + nostd::string_view unit = "") noexcept override; nostd::shared_ptr> CreateLongUpDownCounter( nostd::string_view name, @@ -90,19 +82,15 @@ class Meter final : public opentelemetry::metrics::Meter nostd::string_view description = "", nostd::string_view unit = "") noexcept override; - void CreateLongObservableUpDownCounter( - nostd::string_view name, - void (*callback)(opentelemetry::metrics::ObserverResult &, void *), - nostd::string_view description = "", - nostd::string_view unit = "", - void *state = nullptr) noexcept override; + nostd::shared_ptr> + CreateLongObservableUpDownCounter(nostd::string_view name, + nostd::string_view description = "", + nostd::string_view unit = "") noexcept override; - void CreateDoubleObservableUpDownCounter( - nostd::string_view name, - void (*callback)(opentelemetry::metrics::ObserverResult &, void *), - nostd::string_view description = "", - nostd::string_view unit = "", - void *state = nullptr) noexcept override; + nostd::shared_ptr> + CreateDoubleObservableUpDownCounter(nostd::string_view name, + nostd::string_view description = "", + nostd::string_view unit = "") noexcept override; /** Returns the associated instruementation library */ const sdk::instrumentationlibrary::InstrumentationLibrary *GetInstrumentationLibrary() @@ -123,32 +111,7 @@ class Meter final : public opentelemetry::metrics::Meter std::unique_ptr RegisterMetricStorage( InstrumentDescriptor &instrument_descriptor); - template - void RegisterAsyncMetricStorage(InstrumentDescriptor &instrument_descriptor, - void (*callback)(opentelemetry::metrics::ObserverResult &, - void *), - void *state = nullptr) - { - auto view_registry = meter_context_->GetViewRegistry(); - auto success = view_registry->FindViews( - instrument_descriptor, *instrumentation_library_, - [this, &instrument_descriptor, callback, state](const View &view) { - auto view_instr_desc = instrument_descriptor; - if (!view.GetName().empty()) - { - view_instr_desc.name_ = view.GetName(); - } - if (!view.GetDescription().empty()) - { - view_instr_desc.description_ = view.GetDescription(); - } - auto storage = std::shared_ptr>( - new AsyncMetricStorage(view_instr_desc, view.GetAggregationType(), callback, - &view.GetAttributesProcessor(), state)); - storage_registry_[instrument_descriptor.name_] = storage; - return true; - }); - } + void RegisterAsyncMetricStorage(InstrumentDescriptor &instrument_descriptor); }; } // namespace metrics } // namespace sdk diff --git a/sdk/include/opentelemetry/sdk/metrics/state/metric_storage.h b/sdk/include/opentelemetry/sdk/metrics/state/metric_storage.h index e0ba55cbfe..cd7c9dc898 100644 --- a/sdk/include/opentelemetry/sdk/metrics/state/metric_storage.h +++ b/sdk/include/opentelemetry/sdk/metrics/state/metric_storage.h @@ -28,6 +28,7 @@ class MetricStorage nostd::function_ref callback) noexcept = 0; }; +/* Represents the sync metric storage */ class WritableMetricStorage { public: @@ -47,6 +48,18 @@ class WritableMetricStorage virtual ~WritableMetricStorage() = default; }; +/* Represents the async metric stroage */ +class AsyncWritableMetricStorage +{ +public: + /* Records a batch of measurements */ + virtual void RecordLong( + std::unordered_map &measurements); + + virtual void RecordDouble( + std::unordered_map &measurements); +}; + class NoopMetricStorage : public MetricStorage { public: diff --git a/sdk/src/metrics/meter.cc b/sdk/src/metrics/meter.cc index 600bae9863..e8a53c80ee 100644 --- a/sdk/src/metrics/meter.cc +++ b/sdk/src/metrics/meter.cc @@ -57,31 +57,28 @@ nostd::shared_ptr> Meter::CreateDoubleCounter( new DoubleCounter(instrument_descriptor, std::move(storage))}; } -void Meter::CreateLongObservableCounter(nostd::string_view name, - void (*callback)(metrics::ObserverResult &, void *), - nostd::string_view description, - nostd::string_view unit, - void *state) noexcept +nostd::shared_ptr> +Meter::CreateLongObservableCounter(nostd::string_view name, + nostd::string_view description, + nostd::string_view unit) noexcept { InstrumentDescriptor instrument_descriptor = { std::string{name.data(), name.size()}, std::string{description.data(), description.size()}, std::string{unit.data(), unit.size()}, InstrumentType::kObservableCounter, InstrumentValueType::kLong}; - RegisterAsyncMetricStorage(instrument_descriptor, callback, state); + RegisterAsyncMetricStorage(instrument_descriptor); } -void Meter::CreateDoubleObservableCounter(nostd::string_view name, - void (*callback)(metrics::ObserverResult &, - void *), - nostd::string_view description, - nostd::string_view unit, - void *state) noexcept +nostd::shared_ptr> +Meter::CreateDoubleObservableCounter(nostd::string_view name, + nostd::string_view description, + nostd::string_view unit) noexcept { InstrumentDescriptor instrument_descriptor = { std::string{name.data(), name.size()}, std::string{description.data(), description.size()}, std::string{unit.data(), unit.size()}, InstrumentType::kObservableCounter, InstrumentValueType::kDouble}; - RegisterAsyncMetricStorage(instrument_descriptor, callback, state); + RegisterAsyncMetricStorage(instrument_descriptor); } nostd::shared_ptr> Meter::CreateLongHistogram( @@ -112,30 +109,28 @@ nostd::shared_ptr> Meter::CreateDoubleHistogram( new DoubleHistogram(instrument_descriptor, std::move(storage))}; } -void Meter::CreateLongObservableGauge(nostd::string_view name, - void (*callback)(metrics::ObserverResult &, void *), - nostd::string_view description, - nostd::string_view unit, - void *state) noexcept +nostd::shared_ptr> Meter::CreateLongObservableGauge( + nostd::string_view name, + nostd::string_view description, + nostd::string_view unit) noexcept { InstrumentDescriptor instrument_descriptor = { std::string{name.data(), name.size()}, std::string{description.data(), description.size()}, std::string{unit.data(), unit.size()}, InstrumentType::kObservableGauge, InstrumentValueType::kLong}; - RegisterAsyncMetricStorage(instrument_descriptor, callback, state); + RegisterAsyncMetricStorage(instrument_descriptor); } -void Meter::CreateDoubleObservableGauge(nostd::string_view name, - void (*callback)(metrics::ObserverResult &, void *), - nostd::string_view description, - nostd::string_view unit, - void *state) noexcept +nostd::shared_ptr> +Meter::CreateDoubleObservableGauge(nostd::string_view name, + nostd::string_view description, + nostd::string_view unit) noexcept { InstrumentDescriptor instrument_descriptor = { std::string{name.data(), name.size()}, std::string{description.data(), description.size()}, std::string{unit.data(), unit.size()}, InstrumentType::kObservableGauge, InstrumentValueType::kDouble}; - RegisterAsyncMetricStorage(instrument_descriptor, callback, state); + RegisterAsyncMetricStorage(instrument_descriptor); } nostd::shared_ptr> Meter::CreateLongUpDownCounter( @@ -166,32 +161,28 @@ nostd::shared_ptr> Meter::CreateDoubleUpDownCount new DoubleUpDownCounter(instrument_descriptor, std::move(storage))}; } -void Meter::CreateLongObservableUpDownCounter(nostd::string_view name, - void (*callback)(metrics::ObserverResult &, - void *), - nostd::string_view description, - nostd::string_view unit, - void *state) noexcept +nostd::shared_ptr> +Meter::CreateLongObservableUpDownCounter(nostd::string_view name, + nostd::string_view description, + nostd::string_view unit) noexcept { InstrumentDescriptor instrument_descriptor = { std::string{name.data(), name.size()}, std::string{description.data(), description.size()}, std::string{unit.data(), unit.size()}, InstrumentType::kObservableUpDownCounter, InstrumentValueType::kLong}; - RegisterAsyncMetricStorage(instrument_descriptor, callback, state); + RegisterAsyncMetricStorage(instrument_descriptor); } -void Meter::CreateDoubleObservableUpDownCounter(nostd::string_view name, - void (*callback)(metrics::ObserverResult &, - void *), - nostd::string_view description, - nostd::string_view unit, - void *state) noexcept +nostd::shared_ptr> +Meter::CreateDoubleObservableUpDownCounter(nostd::string_view name, + nostd::string_view description, + nostd::string_view unit) noexcept { InstrumentDescriptor instrument_descriptor = { std::string{name.data(), name.size()}, std::string{description.data(), description.size()}, std::string{unit.data(), unit.size()}, InstrumentType::kObservableUpDownCounter, InstrumentValueType::kDouble}; - RegisterAsyncMetricStorage(instrument_descriptor, callback, state); + RegisterAsyncMetricStorage(instrument_descriptor); } const sdk::instrumentationlibrary::InstrumentationLibrary *Meter::GetInstrumentationLibrary() @@ -236,6 +227,28 @@ std::unique_ptr Meter::RegisterMetricStorage( return storages; } +void Meter::RegisterAsyncMetricStorage(InstrumentDescriptor &instrument_descriptor) +{ + auto view_registry = meter_context_->GetViewRegistry(); + auto success = view_registry->FindViews( + instrument_descriptor, *instrumentation_library_, + [this, &instrument_descriptor](const View &view) { + auto view_instr_desc = instrument_descriptor; + if (!view.GetName().empty()) + { + view_instr_desc.name_ = view.GetName(); + } + if (!view.GetDescription().empty()) + { + view_instr_desc.description_ = view.GetDescription(); + } + auto storage = std::shared_ptr(new AsyncMetricStorage( + view_instr_desc, view.GetAggregationType(), &view.GetAttributesProcessor())); + storage_registry_[instrument_descriptor.name_] = storage; + return true; + }); +} + /** collect metrics across all the meters **/ std::vector Meter::Collect(CollectorHandle *collector, opentelemetry::common::SystemTimestamp collect_ts) noexcept From 64bc149468014466674239c121070a3c28fa1acb Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Thu, 7 Jul 2022 17:34:57 -0700 Subject: [PATCH 06/26] changes --- .../opentelemetry/metrics/async_instruments.h | 14 ++++++-------- .../opentelemetry/metrics/observer_result.h | 4 +++- examples/common/metrics_foo_library/foo_library.cc | 14 ++++++++++---- .../opentelemetry/sdk/metrics/async_instruments.h | 4 ++-- .../opentelemetry/sdk/metrics/observer_result.h | 4 ++-- .../sdk/metrics/state/async_metric_storage.h | 10 +++++----- .../sdk/metrics/state/metric_storage.h | 2 ++ 7 files changed, 30 insertions(+), 22 deletions(-) diff --git a/api/include/opentelemetry/metrics/async_instruments.h b/api/include/opentelemetry/metrics/async_instruments.h index 331524ac85..6e17718633 100644 --- a/api/include/opentelemetry/metrics/async_instruments.h +++ b/api/include/opentelemetry/metrics/async_instruments.h @@ -10,10 +10,8 @@ OPENTELEMETRY_BEGIN_NAMESPACE namespace metrics { -typedef void (*ObservableCallbackPtr)(ObserverResult &, void *); - -class AsynchronousInstrument -{}; +// typedef void (*ObservableCallbackPtr)(ObserverResult &, void *); +using ObservableCallbackPtr = void (*)(ObserverResult &, void *); class ObservableInstrument { @@ -24,21 +22,21 @@ class ObservableInstrument virtual void AddCallback(ObservableCallbackPtr, void *state); /** - * Sets up a function that will be called whenever a metric collection is initiated. + * Remove a function that was configured to be called whenever a metric collection is initiated. */ virtual void RemoveCallback(ObservableCallbackPtr, void *state); }; template -class ObservableCounter : public AsynchronousInstrument +class ObservableCounter : public ObservableInstrument {}; template -class ObservableGauge : public AsynchronousInstrument +class ObservableGauge : public ObservableInstrument {}; template -class ObservableUpDownCounter : public AsynchronousInstrument +class ObservableUpDownCounter : public ObservableInstrument {}; } // namespace metrics diff --git a/api/include/opentelemetry/metrics/observer_result.h b/api/include/opentelemetry/metrics/observer_result.h index 2f09beab11..82acaf9a0e 100644 --- a/api/include/opentelemetry/metrics/observer_result.h +++ b/api/include/opentelemetry/metrics/observer_result.h @@ -6,6 +6,7 @@ # include "opentelemetry/common/attribute_value.h" # include "opentelemetry/common/key_value_iterable_view.h" +# include "opentelemetry/nostd/shared_ptr.h" # include "opentelemetry/nostd/span.h" # include "opentelemetry/nostd/string_view.h" # include "opentelemetry/nostd/type_traits.h" @@ -43,7 +44,8 @@ class ObserverResultT } }; -using ObserverResult = nostd::variant, ObserverResultT>; +using ObserverResult = nostd::variant>, + nostd::shared_ptr>>; } // namespace metrics OPENTELEMETRY_END_NAMESPACE diff --git a/examples/common/metrics_foo_library/foo_library.cc b/examples/common/metrics_foo_library/foo_library.cc index 2fcbd660e0..0c6b906862 100644 --- a/examples/common/metrics_foo_library/foo_library.cc +++ b/examples/common/metrics_foo_library/foo_library.cc @@ -30,12 +30,18 @@ std::map get_random_attr() class MeasurementFetcher { public: - static void Fetcher(opentelemetry::metrics::ObserverResult &observer_result, void *state) + static void Fetcher(opentelemetry::metrics::ObserverResult &observer_result, void *state) { - double val = (rand() % 700) + 1.1; std::map labels = get_random_attr(); auto labelkv = opentelemetry::common::KeyValueIterableView{labels}; - observer_result.Observe(val /*, labelkv*/); + if (nostd::holds_alternative< + nostd::shared_ptr>>(observer_result)) + { + double val = (rand() % 700) + 1.1; + nostd::get>>( + observer_result) + ->Observe(val /*, labelkv */); + } } }; } // namespace @@ -60,7 +66,7 @@ void foo_library::observable_counter_example(const std::string &name) std::string counter_name = name + "_observable_counter"; auto provider = metrics_api::Provider::GetMeterProvider(); nostd::shared_ptr meter = provider->GetMeter(name, "1.2.0"); - meter->CreateDoubleObservableCounter(counter_name, MeasurementFetcher::Fetcher); + auto counter = meter->CreateDoubleObservableCounter(counter_name); while (true) { std::this_thread::sleep_for(std::chrono::milliseconds(500)); diff --git a/sdk/include/opentelemetry/sdk/metrics/async_instruments.h b/sdk/include/opentelemetry/sdk/metrics/async_instruments.h index e045781716..9b4e473e06 100644 --- a/sdk/include/opentelemetry/sdk/metrics/async_instruments.h +++ b/sdk/include/opentelemetry/sdk/metrics/async_instruments.h @@ -20,12 +20,12 @@ class ObservableInstrument /** * Sets up a function that will be called whenever a metric collection is initiated. */ - virtual void AddCallback(ObservableCallbackPtr, void *state); + virtual void AddCallback(opentelemetry::metrics::ObservableCallbackPtr, void *state); /** * Sets up a function that will be called whenever a metric collection is initiated. */ - virtual void RemoveCallback(ObservableCallbackPtr, void *state); + virtual void RemoveCallback(opentelemetry::metrics::ObservableCallbackPtr, void *state); }; template diff --git a/sdk/include/opentelemetry/sdk/metrics/observer_result.h b/sdk/include/opentelemetry/sdk/metrics/observer_result.h index ca7227bc5b..0d07ae87f4 100644 --- a/sdk/include/opentelemetry/sdk/metrics/observer_result.h +++ b/sdk/include/opentelemetry/sdk/metrics/observer_result.h @@ -16,10 +16,10 @@ namespace sdk namespace metrics { template -class ObserverResult final : public opentelemetry::metrics::ObserverResult +class ObserverResultT final : public opentelemetry::metrics::ObserverResultT { public: - ObserverResult(const AttributesProcessor *attributes_processor) + ObserverResultT(const AttributesProcessor *attributes_processor) : attributes_processor_(attributes_processor) {} diff --git a/sdk/include/opentelemetry/sdk/metrics/state/async_metric_storage.h b/sdk/include/opentelemetry/sdk/metrics/state/async_metric_storage.h index e5dcbc2738..970c2034a2 100644 --- a/sdk/include/opentelemetry/sdk/metrics/state/async_metric_storage.h +++ b/sdk/include/opentelemetry/sdk/metrics/state/async_metric_storage.h @@ -27,8 +27,7 @@ class AsyncMetricStorage : public MetricStorage public: AsyncMetricStorage(InstrumentDescriptor instrument_descriptor, const AggregationType aggregation_type, - void (*measurement_callback)(opentelemetry::metrics::ObserverResult &, - void *), + void (*measurement_callback)(opentelemetry::metrics::ObserverResult &, void *), const AttributesProcessor *attributes_processor, void *state = nullptr) : instrument_descriptor_(instrument_descriptor), @@ -46,13 +45,14 @@ class AsyncMetricStorage : public MetricStorage opentelemetry::common::SystemTimestamp collection_ts, nostd::function_ref metric_collection_callback) noexcept override { - opentelemetry::sdk::metrics::ObserverResult ob_res(attributes_processor_); + nostd::shared_ptr> ob_res( + new opentelemetry::sdk::metrics::ObserverResultT(attributes_processor_)); // read the measurement using configured callback measurement_collection_callback_(ob_res, state_); std::shared_ptr delta_hash_map(new AttributesHashMap()); // process the read measurements - aggregate and store in hashmap - for (auto &measurement : ob_res.GetMeasurements()) + for (auto &measurement : ob_res->GetMeasurements()) { auto aggr = DefaultAggregation::CreateAggregation(aggregation_type_, instrument_descriptor_); aggr->Aggregate(measurement.second); @@ -82,7 +82,7 @@ class AsyncMetricStorage : public MetricStorage private: InstrumentDescriptor instrument_descriptor_; AggregationType aggregation_type_; - void (*measurement_collection_callback_)(opentelemetry::metrics::ObserverResult &, void *); + void (*measurement_collection_callback_)(opentelemetry::metrics::ObserverResult &, void *); const AttributesProcessor *attributes_processor_; void *state_; std::unique_ptr cumulative_hash_map_; diff --git a/sdk/include/opentelemetry/sdk/metrics/state/metric_storage.h b/sdk/include/opentelemetry/sdk/metrics/state/metric_storage.h index cd7c9dc898..6a0ed604b6 100644 --- a/sdk/include/opentelemetry/sdk/metrics/state/metric_storage.h +++ b/sdk/include/opentelemetry/sdk/metrics/state/metric_storage.h @@ -8,6 +8,8 @@ # include "opentelemetry/context/context.h" # include "opentelemetry/sdk/metrics/data/metric_data.h" # include "opentelemetry/sdk/metrics/instruments.h" +# include "opentelemetry/sdk/metrics/state/attributes_hashmap.h" + OPENTELEMETRY_BEGIN_NAMESPACE namespace sdk { From c8150f46a67d00cde4c535dd32b8b049b9665e7b Mon Sep 17 00:00:00 2001 From: Lalit Date: Sun, 17 Jul 2022 00:37:37 -0700 Subject: [PATCH 07/26] more changes --- .../opentelemetry/metrics/async_instruments.h | 16 +-- api/include/opentelemetry/metrics/meter.h | 12 +- api/include/opentelemetry/metrics/noop.h | 3 +- .../sdk/metrics/async_instruments.h | 103 ++++-------------- .../opentelemetry/sdk/metrics/instruments.h | 19 ++++ sdk/include/opentelemetry/sdk/metrics/meter.h | 30 ++--- .../sdk/metrics/state/observable_registry.h | 23 +++- sdk/src/metrics/meter.cc | 23 +++- 8 files changed, 101 insertions(+), 128 deletions(-) diff --git a/api/include/opentelemetry/metrics/async_instruments.h b/api/include/opentelemetry/metrics/async_instruments.h index 6e17718633..18c6911a2e 100644 --- a/api/include/opentelemetry/metrics/async_instruments.h +++ b/api/include/opentelemetry/metrics/async_instruments.h @@ -19,26 +19,14 @@ class ObservableInstrument /** * Sets up a function that will be called whenever a metric collection is initiated. */ - virtual void AddCallback(ObservableCallbackPtr, void *state); + virtual void AddCallback(ObservableCallbackPtr, void *state) noexcept; /** * Remove a function that was configured to be called whenever a metric collection is initiated. */ - virtual void RemoveCallback(ObservableCallbackPtr, void *state); + virtual void RemoveCallback(ObservableCallbackPtr, void *state) noexcept; }; -template -class ObservableCounter : public ObservableInstrument -{}; - -template -class ObservableGauge : public ObservableInstrument -{}; - -template -class ObservableUpDownCounter : public ObservableInstrument -{}; - } // namespace metrics OPENTELEMETRY_END_NAMESPACE #endif diff --git a/api/include/opentelemetry/metrics/meter.h b/api/include/opentelemetry/metrics/meter.h index b0c3e099c6..0bc9b7116c 100644 --- a/api/include/opentelemetry/metrics/meter.h +++ b/api/include/opentelemetry/metrics/meter.h @@ -54,12 +54,12 @@ class Meter * @param description a brief description of what the Observable Counter is used for. * @param unit the unit of metric values following https://unitsofmeasure.org/ucum.html. */ - virtual nostd::shared_ptr> CreateLongObservableCounter( + virtual nostd::shared_ptr CreateLongObservableCounter( nostd::string_view name, nostd::string_view description = "", nostd::string_view unit = "") noexcept = 0; - virtual nostd::shared_ptr> CreateDoubleObservableCounter( + virtual nostd::shared_ptr CreateDoubleObservableCounter( nostd::string_view name, nostd::string_view description = "", nostd::string_view unit = "") noexcept = 0; @@ -90,12 +90,12 @@ class Meter * @param description a brief description of what the Observable Gauge is used for. * @param unit the unit of metric values following https://unitsofmeasure.org/ucum.html. */ - virtual nostd::shared_ptr> CreateLongObservableGauge( + virtual nostd::shared_ptr CreateLongObservableGauge( nostd::string_view name, nostd::string_view description = "", nostd::string_view unit = "") noexcept = 0; - virtual nostd::shared_ptr> CreateDoubleObservableGauge( + virtual nostd::shared_ptr CreateDoubleObservableGauge( nostd::string_view name, nostd::string_view description = "", nostd::string_view unit = "") noexcept = 0; @@ -127,12 +127,12 @@ class Meter * @param description a brief description of what the Observable UpDownCounter is used for. * @param unit the unit of metric values following https://unitsofmeasure.org/ucum.html. */ - virtual nostd::shared_ptr> CreateLongObservableUpDownCounter( + virtual nostd::shared_ptr CreateLongObservableUpDownCounter( nostd::string_view name, nostd::string_view description = "", nostd::string_view unit = "") noexcept = 0; - virtual nostd::shared_ptr> CreateDoubleObservableUpDownCounter( + virtual nostd::shared_ptr CreateDoubleObservableUpDownCounter( nostd::string_view name, nostd::string_view description = "", nostd::string_view unit = "") noexcept = 0; diff --git a/api/include/opentelemetry/metrics/noop.h b/api/include/opentelemetry/metrics/noop.h index d8348761ec..ba37f265ce 100644 --- a/api/include/opentelemetry/metrics/noop.h +++ b/api/include/opentelemetry/metrics/noop.h @@ -64,8 +64,7 @@ class NoopUpDownCounter : public UpDownCounter {} }; -template -class NoopObservableCounter : public ObservableCounter +class NoopObservableCounter : public ObservableCounter { public: NoopObservableCounter(nostd::string_view name, diff --git a/sdk/include/opentelemetry/sdk/metrics/async_instruments.h b/sdk/include/opentelemetry/sdk/metrics/async_instruments.h index 9b4e473e06..2a6988edaf 100644 --- a/sdk/include/opentelemetry/sdk/metrics/async_instruments.h +++ b/sdk/include/opentelemetry/sdk/metrics/async_instruments.h @@ -13,22 +13,6 @@ namespace sdk namespace metrics { -template -class ObservableInstrument -{ -public: - /** - * Sets up a function that will be called whenever a metric collection is initiated. - */ - virtual void AddCallback(opentelemetry::metrics::ObservableCallbackPtr, void *state); - - /** - * Sets up a function that will be called whenever a metric collection is initiated. - */ - virtual void RemoveCallback(opentelemetry::metrics::ObservableCallbackPtr, void *state); -}; - -template class Asynchronous { public: @@ -44,78 +28,35 @@ class Asynchronous std::string unit_; }; -class LongObservableCounter : public opentelemetry::metrics::ObservableCounter, - public Asynchronous -{ -public: - LongObservableCounter(nostd::string_view name, - nostd::string_view description = "", - nostd::string_view unit = "") - : Asynchronous(name, description, unit) - - {} -}; - -class DoubleObservableCounter : public opentelemetry::metrics::ObservableCounter, - public Asynchronous -{ -public: - DoubleObservableCounter(nostd::string_view name, - nostd::string_view description = "", - nostd::string_view unit = "") - : Asynchronous(name, description, unit) - - {} -}; - -class LongObservableGauge : public opentelemetry::metrics::ObservableGauge, - public Asynchronous +class ObservableInstrument : public opentelemetry::metrics::ObservableInstrument, + public Asynchronous { public: - LongObservableGauge(nostd::string_view name, - nostd::string_view description = "", - nostd::string_view unit = "") - : Asynchronous(name, description, unit) - + ObservableInstrument(InstrumentDescriptor instrument_descriptor, + std::unique_ptr storage) + : instrument_descriptor_(instrument_descriptor), + storage_(std::move(storage)), + observable_registry_{new ObservableRegistry()} {} -}; -class DoubleObservableGauge : public opentelemetry::metrics::ObservableGauge, - public Asynchronous -{ -public: - DoubleObservableGauge(nostd::string_view name, - nostd::string_view description = "", - nostd::string_view unit = "") - : Asynchronous(name, description, unit) + void AddCallback(opentelemetry::metrics::ObservableCallbackPtr callback, + void *state) noexcept override + { + observable_registry_->AddCallback(callback, state, this); + } - {} -}; - -class LongObservableUpDownCounter : public opentelemetry::metrics::ObservableUpDownCounter, - public Asynchronous -{ -public: - LongObservableUpDownCounter(nostd::string_view name, - nostd::string_view description = "", - nostd::string_view unit = "") - : Asynchronous(name, description, unit) + void RemoveCallback(opentelemetry::metrics::ObservableCallbackPtr callback, + void *state) noexcept override + { + observable_registry_->AddCallback(callback, state, this); + } - {} -}; - -class DoubleObservableUpDownCounter - : public opentelemetry::metrics::ObservableUpDownCounter, - public Asynchronous -{ -public: - DoubleObservableUpDownCounter(nostd::string_view name, - nostd::string_view description = "", - nostd::string_view unit = "") - : Asynchronous(name, description, unit) - {} +private: +protected: + InstrumentDescriptor instrument_descriptor_; + std::unique_ptr storage_; + std::unique_ptr observable_registry_; }; - } // namespace metrics } // namespace sdk OPENTELEMETRY_END_NAMESPACE diff --git a/sdk/include/opentelemetry/sdk/metrics/instruments.h b/sdk/include/opentelemetry/sdk/metrics/instruments.h index 5d85357143..845485f70d 100644 --- a/sdk/include/opentelemetry/sdk/metrics/instruments.h +++ b/sdk/include/opentelemetry/sdk/metrics/instruments.h @@ -19,6 +19,12 @@ enum class InstrumentType kObservableUpDownCounter }; +enum class InstrumentClass +{ + kSync, + kAsync +}; + enum class InstrumentValueType { kInt, @@ -52,6 +58,19 @@ struct InstrumentDescriptor InstrumentValueType value_type_; }; +static InstrumentClass GetInstrumentClass(InstrumentType type) +{ + if (type == InstrumentType::kCounter || type == InstrumentType::kHistogram || + type == InstrumentType::kUpDownCounter) + { + return InstrumentClass::kSync; + } + else + { + return InstrumentClass::kAsync; + } +} + using MetricAttributes = opentelemetry::sdk::common::OrderedAttributeMap; /*class InstrumentSelector { diff --git a/sdk/include/opentelemetry/sdk/metrics/meter.h b/sdk/include/opentelemetry/sdk/metrics/meter.h index a1ae9af443..964457d513 100644 --- a/sdk/include/opentelemetry/sdk/metrics/meter.h +++ b/sdk/include/opentelemetry/sdk/metrics/meter.h @@ -42,15 +42,15 @@ class Meter final : public opentelemetry::metrics::Meter nostd::string_view description = "", nostd::string_view unit = "") noexcept override; - nostd::shared_ptr> CreateLongObservableCounter( + nostd::shared_ptr CreateLongObservableCounter( nostd::string_view name, nostd::string_view description = "", nostd::string_view unit = "") noexcept override; - nostd::shared_ptr> - CreateDoubleObservableCounter(nostd::string_view name, - nostd::string_view description = "", - nostd::string_view unit = "") noexcept override; + nostd::shared_ptr CreateDoubleObservableCounter( + nostd::string_view name, + nostd::string_view description = "", + nostd::string_view unit = "") noexcept override; nostd::shared_ptr> CreateLongHistogram( nostd::string_view name, @@ -62,12 +62,12 @@ class Meter final : public opentelemetry::metrics::Meter nostd::string_view description = "", nostd::string_view unit = "") noexcept override; - nostd::shared_ptr> CreateLongObservableGauge( + nostd::shared_ptr CreateLongObservableGauge( nostd::string_view name, nostd::string_view description = "", nostd::string_view unit = "") noexcept override; - nostd::shared_ptr> CreateDoubleObservableGauge( + nostd::shared_ptr CreateDoubleObservableGauge( nostd::string_view name, nostd::string_view description = "", nostd::string_view unit = "") noexcept override; @@ -82,15 +82,15 @@ class Meter final : public opentelemetry::metrics::Meter nostd::string_view description = "", nostd::string_view unit = "") noexcept override; - nostd::shared_ptr> - CreateLongObservableUpDownCounter(nostd::string_view name, - nostd::string_view description = "", - nostd::string_view unit = "") noexcept override; + nostd::shared_ptr CreateLongObservableUpDownCounter( + nostd::string_view name, + nostd::string_view description = "", + nostd::string_view unit = "") noexcept override; - nostd::shared_ptr> - CreateDoubleObservableUpDownCounter(nostd::string_view name, - nostd::string_view description = "", - nostd::string_view unit = "") noexcept override; + nostd::shared_ptr CreateDoubleObservableUpDownCounter( + nostd::string_view name, + nostd::string_view description = "", + nostd::string_view unit = "") noexcept override; /** Returns the associated instruementation library */ const sdk::instrumentationlibrary::InstrumentationLibrary *GetInstrumentationLibrary() diff --git a/sdk/include/opentelemetry/sdk/metrics/state/observable_registry.h b/sdk/include/opentelemetry/sdk/metrics/state/observable_registry.h index d471af1861..4a41f16b25 100644 --- a/sdk/include/opentelemetry/sdk/metrics/state/observable_registry.h +++ b/sdk/include/opentelemetry/sdk/metrics/state/observable_registry.h @@ -19,32 +19,45 @@ namespace metrics struct ObservableCallbackRecord { ObservableCallbackPtr callback; + void *state; ObservableInstrument *instrument; }; class ObservableRegistry { public: - void AddCallback(ObservableCallbackPtr callback, ObservableInstrument *instrument) + void AddCallback(ObservableCallbackPtr callback, void *state, ObservableInstrument *instrument) { // TBD - Check if existing - auto record = - std::unique_ptr(ObservableCallbackRecord{callback, instrument}); + auto record = std::unique_ptr( + ObservableCallbackRecord{callback, state, instrument}); + std::unique_lock lk(callbacks_m_); callbacks_.push_back(std::move(record)); } - void RemoveCallback(ObservableCallbackPtr callback, ObservableInstrument *instrument) + void RemoveCallback(ObservableCallbackPtr callback, void *state, ObservableInstrument *instrument) { auto new_end = std::remove_if( callbacks_.begin(), callbacks_.end(), [callback, instrument](const std::unique_ptr &record) { - return record->callback == callback && record->instrument == instrument; + return record->callback == callback && record->state == state && + record->instrument == instrument; }); + std::unique_lock lk(callbacks_m_); callbacks_.erase(new_end, callbacks_.end()); } + void Observe(opentelemetry::common::SystemTimestamp collection_ts) + { + std::unique_lock lk(callbacks_m_); + for (auto callback : callbacks_) + { + } + } + private: std::vector> callbacks_; + std::mutex callbacks_m_; }; } // namespace metrics diff --git a/sdk/src/metrics/meter.cc b/sdk/src/metrics/meter.cc index e8a53c80ee..31cea8e2fb 100644 --- a/sdk/src/metrics/meter.cc +++ b/sdk/src/metrics/meter.cc @@ -209,10 +209,20 @@ std::unique_ptr Meter::RegisterMetricStorage( { view_instr_desc.description_ = view.GetDescription(); } - auto storage = std::shared_ptr(new SyncMetricStorage( - view_instr_desc, view.GetAggregationType(), &view.GetAttributesProcessor(), - NoExemplarReservoir::GetNoExemplarReservoir())); - storage_registry_[instrument_descriptor.name_] = storage; + if (GetInstrumentClass(instrument_descriptor.type_) == InstrumentClass::kSync) + { + auto storage = std::shared_ptr(new SyncMetricStorage( + view_instr_desc, view.GetAggregationType(), &view.GetAttributesProcessor(), + NoExemplarReservoir::GetNoExemplarReservoir())); + storage_registry_[instrument_descriptor.name_] = storage; + } + else + { + auto storage = std::shared_ptr(new AsyncMetricStorage( + view_instr_desc, view.GetAggregationType(), &view.GetAttributesProcessor())); + storage_registry_[instrument_descriptor.name_] = storage; + } + auto multi_storage = static_cast(storages.get()); multi_storage->AddStorage(storage); return true; @@ -227,9 +237,11 @@ std::unique_ptr Meter::RegisterMetricStorage( return storages; } -void Meter::RegisterAsyncMetricStorage(InstrumentDescriptor &instrument_descriptor) +# if 0 +std::vector> Meter::RegisterAsyncMetricStorage(InstrumentDescriptor &instrument_descriptor) { auto view_registry = meter_context_->GetViewRegistry(); + std::vector> storages; auto success = view_registry->FindViews( instrument_descriptor, *instrumentation_library_, [this, &instrument_descriptor](const View &view) { @@ -248,6 +260,7 @@ void Meter::RegisterAsyncMetricStorage(InstrumentDescriptor &instrument_descript return true; }); } +# endif /** collect metrics across all the meters **/ std::vector Meter::Collect(CollectorHandle *collector, From 2866d853a083e84f5b7895cedcc3f08cf7db657d Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Sun, 17 Jul 2022 01:17:24 -0700 Subject: [PATCH 08/26] more changes --- .../sdk/metrics/async_instruments.h | 8 ++++++- .../sdk/metrics/state/observable_registry.h | 21 ++++++++++++------- 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/metrics/async_instruments.h b/sdk/include/opentelemetry/sdk/metrics/async_instruments.h index 2a6988edaf..29b3b2ad56 100644 --- a/sdk/include/opentelemetry/sdk/metrics/async_instruments.h +++ b/sdk/include/opentelemetry/sdk/metrics/async_instruments.h @@ -7,12 +7,15 @@ # include "opentelemetry/metrics/observer_result.h" # include "opentelemetry/nostd/string_view.h" # include "opentelemetry/sdk/metrics/instruments.h" + OPENTELEMETRY_BEGIN_NAMESPACE namespace sdk { namespace metrics { +class WritableMetricStorage; + class Asynchronous { public: @@ -36,7 +39,10 @@ class ObservableInstrument : public opentelemetry::metrics::ObservableInstrument std::unique_ptr storage) : instrument_descriptor_(instrument_descriptor), storage_(std::move(storage)), - observable_registry_{new ObservableRegistry()} + observable_registry_{new ObservableRegistry()}, + Asynchronous(instrument_descriptor_.name_, + instrument_descriptor.description_, + instrument_descriptor_.unit_) {} void AddCallback(opentelemetry::metrics::ObservableCallbackPtr callback, diff --git a/sdk/include/opentelemetry/sdk/metrics/state/observable_registry.h b/sdk/include/opentelemetry/sdk/metrics/state/observable_registry.h index 4a41f16b25..4d1146a2b1 100644 --- a/sdk/include/opentelemetry/sdk/metrics/state/observable_registry.h +++ b/sdk/include/opentelemetry/sdk/metrics/state/observable_registry.h @@ -4,9 +4,11 @@ #pragma once #ifndef ENABLE_METRICS_PREVIEW +# include "opentelemetry/common/timestamp.h" # include "opentelemetry/sdk/metrics/async_instruments.h" # include +# include # include # include @@ -18,7 +20,7 @@ namespace metrics struct ObservableCallbackRecord { - ObservableCallbackPtr callback; + opentelemetry::metrics::ObservableCallbackPtr callback; void *state; ObservableInstrument *instrument; }; @@ -26,20 +28,24 @@ struct ObservableCallbackRecord class ObservableRegistry { public: - void AddCallback(ObservableCallbackPtr callback, void *state, ObservableInstrument *instrument) + void AddCallback(opentelemetry::metrics::ObservableCallbackPtr callback, + void *state, + ObservableInstrument *instrument) { // TBD - Check if existing - auto record = std::unique_ptr( - ObservableCallbackRecord{callback, state, instrument}); + std::unique_ptr record( + new ObservableCallbackRecord{callback, state, instrument}); std::unique_lock lk(callbacks_m_); callbacks_.push_back(std::move(record)); } - void RemoveCallback(ObservableCallbackPtr callback, void *state, ObservableInstrument *instrument) + void RemoveCallback(opentelemetry::metrics::ObservableCallbackPtr callback, + void *state, + ObservableInstrument *instrument) { auto new_end = std::remove_if( callbacks_.begin(), callbacks_.end(), - [callback, instrument](const std::unique_ptr &record) { + [callback, state, instrument](const std::unique_ptr &record) { return record->callback == callback && record->state == state && record->instrument == instrument; }); @@ -50,8 +56,9 @@ class ObservableRegistry void Observe(opentelemetry::common::SystemTimestamp collection_ts) { std::unique_lock lk(callbacks_m_); - for (auto callback : callbacks_) + for (auto &callback_wrap : callbacks_) { + callback_wrap->callback } } From 6deb26b3f6ddc23f2b26882949421851026fd760 Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Mon, 18 Jul 2022 11:46:17 -0700 Subject: [PATCH 09/26] fix --- .../opentelemetry/metrics/async_instruments.h | 2 +- .../opentelemetry/sdk/metrics/async_instruments.h | 4 ++++ .../opentelemetry/sdk/metrics/observer_result.h | 14 ++++++++++---- .../sdk/metrics/state/observable_registry.h | 15 ++++++++++++++- 4 files changed, 29 insertions(+), 6 deletions(-) diff --git a/api/include/opentelemetry/metrics/async_instruments.h b/api/include/opentelemetry/metrics/async_instruments.h index 18c6911a2e..3f2d614f81 100644 --- a/api/include/opentelemetry/metrics/async_instruments.h +++ b/api/include/opentelemetry/metrics/async_instruments.h @@ -11,7 +11,7 @@ namespace metrics { // typedef void (*ObservableCallbackPtr)(ObserverResult &, void *); -using ObservableCallbackPtr = void (*)(ObserverResult &, void *); +using ObservableCallbackPtr = void (*)(ObserverResult, void *); class ObservableInstrument { diff --git a/sdk/include/opentelemetry/sdk/metrics/async_instruments.h b/sdk/include/opentelemetry/sdk/metrics/async_instruments.h index 29b3b2ad56..9188a95839 100644 --- a/sdk/include/opentelemetry/sdk/metrics/async_instruments.h +++ b/sdk/include/opentelemetry/sdk/metrics/async_instruments.h @@ -57,6 +57,10 @@ class ObservableInstrument : public opentelemetry::metrics::ObservableInstrument observable_registry_->AddCallback(callback, state, this); } + const InstrumentDescriptor &GetInstrumentDescriptor() { return instrument_descriptor_; } + + const WritableMetricStorage *GetMetricStorage() { return storage_.get(); } + private: protected: InstrumentDescriptor instrument_descriptor_; diff --git a/sdk/include/opentelemetry/sdk/metrics/observer_result.h b/sdk/include/opentelemetry/sdk/metrics/observer_result.h index 0d07ae87f4..0c7463b556 100644 --- a/sdk/include/opentelemetry/sdk/metrics/observer_result.h +++ b/sdk/include/opentelemetry/sdk/metrics/observer_result.h @@ -19,7 +19,7 @@ template class ObserverResultT final : public opentelemetry::metrics::ObserverResultT { public: - ObserverResultT(const AttributesProcessor *attributes_processor) + ObserverResultT(const AttributesProcessor *attributes_processor = nullptr) : attributes_processor_(attributes_processor) {} @@ -27,8 +27,15 @@ class ObserverResultT final : public opentelemetry::metrics::ObserverResultT void Observe(T value, const opentelemetry::common::KeyValueIterable &attributes) noexcept override { - auto attr = attributes_processor_->process(attributes); - data_.insert({attr, value}); + if (attributes_processor_) + { + auto attr = attributes_processor_->process(attributes); + data_.insert({attr, value}); + } + else + { + data_.insert({MetricAttributes{attributes}, value}); + } } const std::unordered_map &GetMeasurements() @@ -38,7 +45,6 @@ class ObserverResultT final : public opentelemetry::metrics::ObserverResultT private: std::unordered_map data_; - const AttributesProcessor *attributes_processor_; }; } // namespace metrics diff --git a/sdk/include/opentelemetry/sdk/metrics/state/observable_registry.h b/sdk/include/opentelemetry/sdk/metrics/state/observable_registry.h index 4d1146a2b1..18094de6e1 100644 --- a/sdk/include/opentelemetry/sdk/metrics/state/observable_registry.h +++ b/sdk/include/opentelemetry/sdk/metrics/state/observable_registry.h @@ -6,6 +6,7 @@ # include "opentelemetry/common/timestamp.h" # include "opentelemetry/sdk/metrics/async_instruments.h" +# include "opentelemetry/sdk/metrics/observer_result.h" # include # include @@ -58,7 +59,19 @@ class ObservableRegistry std::unique_lock lk(callbacks_m_); for (auto &callback_wrap : callbacks_) { - callback_wrap->callback + if (callback_wrap->instrument->GetInstrumentDescriptor().value_type_ == + InstrumentValueType::kDouble) + { + nostd::shared_ptr> ob_res( + new opentelemetry::sdk::metrics::ObserverResultT()); + callback_wrap->callback(ob_res, callback_wrap->state); + } + else + { + nostd::shared_ptr> ob_res( + new opentelemetry::sdk::metrics::ObserverResultT()); + callback_wrap->callback(ob_res, callback_wrap->state); + } } } From 7a377c501537899cb1535dd4e40a470f99ee5ed8 Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Mon, 18 Jul 2022 18:14:55 -0700 Subject: [PATCH 10/26] fix --- api/include/opentelemetry/metrics/noop.h | 59 +++++++++--------------- sdk/src/metrics/meter.cc | 5 +- 2 files changed, 25 insertions(+), 39 deletions(-) diff --git a/api/include/opentelemetry/metrics/noop.h b/api/include/opentelemetry/metrics/noop.h index ba37f265ce..a1e27e296a 100644 --- a/api/include/opentelemetry/metrics/noop.h +++ b/api/include/opentelemetry/metrics/noop.h @@ -64,32 +64,12 @@ class NoopUpDownCounter : public UpDownCounter {} }; -class NoopObservableCounter : public ObservableCounter +class NoopObservableInstrument : public ObservableInstrument { public: - NoopObservableCounter(nostd::string_view name, - nostd::string_view description, - nostd::string_view unit) noexcept - {} -}; - -template -class NoopObservableGauge : public ObservableGauge -{ -public: - NoopObservableGauge(nostd::string_view name, - nostd::string_view description, - nostd::string_view unit) noexcept - {} -}; - -template -class NoopObservableUpDownCounter : public ObservableUpDownCounter -{ -public: - NoopObservableUpDownCounter(nostd::string_view name, - nostd::string_view description, - nostd::string_view unit) noexcept + NoopObservableInstrument(nostd::string_view name, + nostd::string_view description, + nostd::string_view unit) noexcept {} }; @@ -114,20 +94,22 @@ class NoopMeter final : public Meter return nostd::shared_ptr>{new NoopCounter(name, description, unit)}; } - nostd::shared_ptr> CreateLongObservableCounter( + nostd::shared_ptr CreateLongObservableCounter( nostd::string_view name, nostd::string_view description = "", nostd::string_view unit = "") noexcept override { - return nostd::shared_ptr>(new ObservableCounter()); + return nostd::shared_ptr( + new NoopObservableInstrument(name, description, unit)); } - nostd::shared_ptr> CreateDoubleObservableCounter( + nostd::shared_ptr CreateDoubleObservableCounter( nostd::string_view name, nostd::string_view description = "", nostd::string_view unit = "") noexcept override { - return nostd::shared_ptr>(new ObservableCounter()); + return nostd::shared_ptr( + new NoopObservableInstrument(name, description, unit)); } nostd::shared_ptr> CreateLongHistogram( @@ -146,20 +128,22 @@ class NoopMeter final : public Meter return nostd::shared_ptr>{new NoopHistogram(name, description, unit)}; } - nostd::shared_ptr> CreateLongObservableGauge( + nostd::shared_ptr CreateLongObservableGauge( nostd::string_view name, nostd::string_view description = "", nostd::string_view unit = "") noexcept override { - return nostd::shared_ptr>(new ObservableGauge()); + return nostd::shared_ptr( + new NoopObservableInstrument(name, description, unit)); } - nostd::shared_ptr> CreateDoubleObservableGauge( + nostd::shared_ptr CreateDoubleObservableGauge( nostd::string_view name, nostd::string_view description = "", nostd::string_view unit = "") noexcept override { - return nostd::shared_ptr>(new ObservableGauge()); + return nostd::shared_ptr( + new NoopObservableInstrument(name, description, unit)); } nostd::shared_ptr> CreateLongUpDownCounter( @@ -180,21 +164,22 @@ class NoopMeter final : public Meter new NoopUpDownCounter(name, description, unit)}; } - nostd::shared_ptr> CreateLongObservableUpDownCounter( + nostd::shared_ptr CreateLongObservableUpDownCounter( nostd::string_view name, nostd::string_view description = "", nostd::string_view unit = "") noexcept override { - return nostd::shared_ptr>(new ObservableUpDownCounter()); + return nostd::shared_ptr( + new NoopObservableInstrument(name, description, unit)); } - nostd::shared_ptr> CreateDoubleObservableUpDownCounter( + nostd::shared_ptr CreateDoubleObservableUpDownCounter( nostd::string_view name, nostd::string_view description = "", nostd::string_view unit = "") noexcept override { - return nostd::shared_ptr>( - new ObservableUpDownCounter()); + return nostd::shared_ptr( + new NoopObservableInstrument(name, description, unit)); } }; diff --git a/sdk/src/metrics/meter.cc b/sdk/src/metrics/meter.cc index 31cea8e2fb..b3ebc35be1 100644 --- a/sdk/src/metrics/meter.cc +++ b/sdk/src/metrics/meter.cc @@ -217,11 +217,12 @@ std::unique_ptr Meter::RegisterMetricStorage( storage_registry_[instrument_descriptor.name_] = storage; } else - { + {} + /*{ auto storage = std::shared_ptr(new AsyncMetricStorage( view_instr_desc, view.GetAggregationType(), &view.GetAttributesProcessor())); storage_registry_[instrument_descriptor.name_] = storage; - } + }*/ auto multi_storage = static_cast(storages.get()); multi_storage->AddStorage(storage); From 74e40d1f5c529a427966e2accc45d51b69ebdbe1 Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Mon, 18 Jul 2022 18:53:23 -0700 Subject: [PATCH 11/26] fix --- sdk/include/opentelemetry/sdk/metrics/meter.h | 18 +++++++++--------- sdk/src/metrics/meter.cc | 5 ++--- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/metrics/meter.h b/sdk/include/opentelemetry/sdk/metrics/meter.h index 964457d513..9909a489a4 100644 --- a/sdk/include/opentelemetry/sdk/metrics/meter.h +++ b/sdk/include/opentelemetry/sdk/metrics/meter.h @@ -42,12 +42,12 @@ class Meter final : public opentelemetry::metrics::Meter nostd::string_view description = "", nostd::string_view unit = "") noexcept override; - nostd::shared_ptr CreateLongObservableCounter( + nostd::shared_ptr CreateLongObservableCounter( nostd::string_view name, nostd::string_view description = "", nostd::string_view unit = "") noexcept override; - nostd::shared_ptr CreateDoubleObservableCounter( + nostd::shared_ptr CreateDoubleObservableCounter( nostd::string_view name, nostd::string_view description = "", nostd::string_view unit = "") noexcept override; @@ -62,12 +62,12 @@ class Meter final : public opentelemetry::metrics::Meter nostd::string_view description = "", nostd::string_view unit = "") noexcept override; - nostd::shared_ptr CreateLongObservableGauge( + nostd::shared_ptr CreateLongObservableGauge( nostd::string_view name, nostd::string_view description = "", nostd::string_view unit = "") noexcept override; - nostd::shared_ptr CreateDoubleObservableGauge( + nostd::shared_ptr CreateDoubleObservableGauge( nostd::string_view name, nostd::string_view description = "", nostd::string_view unit = "") noexcept override; @@ -82,15 +82,15 @@ class Meter final : public opentelemetry::metrics::Meter nostd::string_view description = "", nostd::string_view unit = "") noexcept override; - nostd::shared_ptr CreateLongObservableUpDownCounter( + nostd::shared_ptr CreateLongObservableUpDownCounter( nostd::string_view name, nostd::string_view description = "", nostd::string_view unit = "") noexcept override; - nostd::shared_ptr CreateDoubleObservableUpDownCounter( - nostd::string_view name, - nostd::string_view description = "", - nostd::string_view unit = "") noexcept override; + nostd::shared_ptr + CreateDoubleObservableUpDownCounter(nostd::string_view name, + nostd::string_view description = "", + nostd::string_view unit = "") noexcept override; /** Returns the associated instruementation library */ const sdk::instrumentationlibrary::InstrumentationLibrary *GetInstrumentationLibrary() diff --git a/sdk/src/metrics/meter.cc b/sdk/src/metrics/meter.cc index b3ebc35be1..31cea8e2fb 100644 --- a/sdk/src/metrics/meter.cc +++ b/sdk/src/metrics/meter.cc @@ -217,12 +217,11 @@ std::unique_ptr Meter::RegisterMetricStorage( storage_registry_[instrument_descriptor.name_] = storage; } else - {} - /*{ + { auto storage = std::shared_ptr(new AsyncMetricStorage( view_instr_desc, view.GetAggregationType(), &view.GetAttributesProcessor())); storage_registry_[instrument_descriptor.name_] = storage; - }*/ + } auto multi_storage = static_cast(storages.get()); multi_storage->AddStorage(storage); From 4b02f35035271e69c5cddbdc3b8d858e400983fa Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Tue, 19 Jul 2022 00:42:30 -0700 Subject: [PATCH 12/26] fix --- .../opentelemetry/metrics/async_instruments.h | 4 +- api/include/opentelemetry/metrics/noop.h | 3 + .../sdk/metrics/async_instruments.h | 1 + .../sdk/metrics/state/observable_registry.h | 48 +------ sdk/src/metrics/CMakeLists.txt | 1 + sdk/src/metrics/meter.cc | 122 ++++++++++-------- sdk/src/metrics/state/observable_registry.cc | 66 ++++++++++ 7 files changed, 143 insertions(+), 102 deletions(-) create mode 100644 sdk/src/metrics/state/observable_registry.cc diff --git a/api/include/opentelemetry/metrics/async_instruments.h b/api/include/opentelemetry/metrics/async_instruments.h index 3f2d614f81..6f61becbec 100644 --- a/api/include/opentelemetry/metrics/async_instruments.h +++ b/api/include/opentelemetry/metrics/async_instruments.h @@ -19,12 +19,12 @@ class ObservableInstrument /** * Sets up a function that will be called whenever a metric collection is initiated. */ - virtual void AddCallback(ObservableCallbackPtr, void *state) noexcept; + virtual void AddCallback(ObservableCallbackPtr, void *state) noexcept = 0; /** * Remove a function that was configured to be called whenever a metric collection is initiated. */ - virtual void RemoveCallback(ObservableCallbackPtr, void *state) noexcept; + virtual void RemoveCallback(ObservableCallbackPtr, void *state) noexcept = 0; }; } // namespace metrics diff --git a/api/include/opentelemetry/metrics/noop.h b/api/include/opentelemetry/metrics/noop.h index a1e27e296a..ffd7ad1340 100644 --- a/api/include/opentelemetry/metrics/noop.h +++ b/api/include/opentelemetry/metrics/noop.h @@ -71,6 +71,9 @@ class NoopObservableInstrument : public ObservableInstrument nostd::string_view description, nostd::string_view unit) noexcept {} + + void AddCallback(ObservableCallbackPtr, void *state) noexcept override {} + void RemoveCallback(ObservableCallbackPtr, void *state) noexcept override {} }; /** diff --git a/sdk/include/opentelemetry/sdk/metrics/async_instruments.h b/sdk/include/opentelemetry/sdk/metrics/async_instruments.h index 9188a95839..64d5509b89 100644 --- a/sdk/include/opentelemetry/sdk/metrics/async_instruments.h +++ b/sdk/include/opentelemetry/sdk/metrics/async_instruments.h @@ -7,6 +7,7 @@ # include "opentelemetry/metrics/observer_result.h" # include "opentelemetry/nostd/string_view.h" # include "opentelemetry/sdk/metrics/instruments.h" +# include "opentelemetry/sdk/metrics/state/observable_registry.h" OPENTELEMETRY_BEGIN_NAMESPACE namespace sdk diff --git a/sdk/include/opentelemetry/sdk/metrics/state/observable_registry.h b/sdk/include/opentelemetry/sdk/metrics/state/observable_registry.h index 18094de6e1..045b83c27b 100644 --- a/sdk/include/opentelemetry/sdk/metrics/state/observable_registry.h +++ b/sdk/include/opentelemetry/sdk/metrics/state/observable_registry.h @@ -5,12 +5,10 @@ #ifndef ENABLE_METRICS_PREVIEW # include "opentelemetry/common/timestamp.h" -# include "opentelemetry/sdk/metrics/async_instruments.h" -# include "opentelemetry/sdk/metrics/observer_result.h" +# include "opentelemetry/metrics/async_instruments.h" # include # include -# include # include OPENTELEMETRY_BEGIN_NAMESPACE @@ -23,7 +21,7 @@ struct ObservableCallbackRecord { opentelemetry::metrics::ObservableCallbackPtr callback; void *state; - ObservableInstrument *instrument; + opentelemetry::metrics::ObservableInstrument *instrument; }; class ObservableRegistry @@ -31,49 +29,13 @@ class ObservableRegistry public: void AddCallback(opentelemetry::metrics::ObservableCallbackPtr callback, void *state, - ObservableInstrument *instrument) - { - // TBD - Check if existing - std::unique_ptr record( - new ObservableCallbackRecord{callback, state, instrument}); - std::unique_lock lk(callbacks_m_); - callbacks_.push_back(std::move(record)); - } + opentelemetry::metrics::ObservableInstrument *instrument); void RemoveCallback(opentelemetry::metrics::ObservableCallbackPtr callback, void *state, - ObservableInstrument *instrument) - { - auto new_end = std::remove_if( - callbacks_.begin(), callbacks_.end(), - [callback, state, instrument](const std::unique_ptr &record) { - return record->callback == callback && record->state == state && - record->instrument == instrument; - }); - std::unique_lock lk(callbacks_m_); - callbacks_.erase(new_end, callbacks_.end()); - } + opentelemetry::metrics::ObservableInstrument *instrument); - void Observe(opentelemetry::common::SystemTimestamp collection_ts) - { - std::unique_lock lk(callbacks_m_); - for (auto &callback_wrap : callbacks_) - { - if (callback_wrap->instrument->GetInstrumentDescriptor().value_type_ == - InstrumentValueType::kDouble) - { - nostd::shared_ptr> ob_res( - new opentelemetry::sdk::metrics::ObserverResultT()); - callback_wrap->callback(ob_res, callback_wrap->state); - } - else - { - nostd::shared_ptr> ob_res( - new opentelemetry::sdk::metrics::ObserverResultT()); - callback_wrap->callback(ob_res, callback_wrap->state); - } - } - } + void Observe(opentelemetry::common::SystemTimestamp collection_ts); private: std::vector> callbacks_; diff --git a/sdk/src/metrics/CMakeLists.txt b/sdk/src/metrics/CMakeLists.txt index 77a371a80c..8d2a431eee 100644 --- a/sdk/src/metrics/CMakeLists.txt +++ b/sdk/src/metrics/CMakeLists.txt @@ -6,6 +6,7 @@ add_library( metric_reader.cc export/periodic_exporting_metric_reader.cc state/metric_collector.cc + state/observable_registry.cc state/sync_metric_storage.cc state/temporal_metric_storage.cc aggregation/histogram_aggregation.cc diff --git a/sdk/src/metrics/meter.cc b/sdk/src/metrics/meter.cc index 31cea8e2fb..cdf59aab15 100644 --- a/sdk/src/metrics/meter.cc +++ b/sdk/src/metrics/meter.cc @@ -57,10 +57,10 @@ nostd::shared_ptr> Meter::CreateDoubleCounter( new DoubleCounter(instrument_descriptor, std::move(storage))}; } -nostd::shared_ptr> -Meter::CreateLongObservableCounter(nostd::string_view name, - nostd::string_view description, - nostd::string_view unit) noexcept +nostd::shared_ptr Meter::CreateLongObservableCounter( + nostd::string_view name, + nostd::string_view description, + nostd::string_view unit) noexcept { InstrumentDescriptor instrument_descriptor = { std::string{name.data(), name.size()}, std::string{description.data(), description.size()}, @@ -69,7 +69,7 @@ Meter::CreateLongObservableCounter(nostd::string_view name, RegisterAsyncMetricStorage(instrument_descriptor); } -nostd::shared_ptr> +nostd::shared_ptr Meter::CreateDoubleObservableCounter(nostd::string_view name, nostd::string_view description, nostd::string_view unit) noexcept @@ -109,7 +109,7 @@ nostd::shared_ptr> Meter::CreateDoubleHistogram( new DoubleHistogram(instrument_descriptor, std::move(storage))}; } -nostd::shared_ptr> Meter::CreateLongObservableGauge( +nostd::shared_ptr Meter::CreateLongObservableGauge( nostd::string_view name, nostd::string_view description, nostd::string_view unit) noexcept @@ -121,10 +121,10 @@ nostd::shared_ptr> Meter::CreateLo RegisterAsyncMetricStorage(instrument_descriptor); } -nostd::shared_ptr> -Meter::CreateDoubleObservableGauge(nostd::string_view name, - nostd::string_view description, - nostd::string_view unit) noexcept +nostd::shared_ptr Meter::CreateDoubleObservableGauge( + nostd::string_view name, + nostd::string_view description, + nostd::string_view unit) noexcept { InstrumentDescriptor instrument_descriptor = { std::string{name.data(), name.size()}, std::string{description.data(), description.size()}, @@ -161,7 +161,7 @@ nostd::shared_ptr> Meter::CreateDoubleUpDownCount new DoubleUpDownCounter(instrument_descriptor, std::move(storage))}; } -nostd::shared_ptr> +nostd::shared_ptr Meter::CreateLongObservableUpDownCounter(nostd::string_view name, nostd::string_view description, nostd::string_view unit) noexcept @@ -173,7 +173,7 @@ Meter::CreateLongObservableUpDownCounter(nostd::string_view name, RegisterAsyncMetricStorage(instrument_descriptor); } -nostd::shared_ptr> +nostd::shared_ptr Meter::CreateDoubleObservableUpDownCounter(nostd::string_view name, nostd::string_view description, nostd::string_view unit) noexcept @@ -200,41 +200,49 @@ std::unique_ptr Meter::RegisterMetricStorage( auto success = view_registry->FindViews( instrument_descriptor, *instrumentation_library_, [this, &instrument_descriptor, &storages](const View &view) { - auto view_instr_desc = instrument_descriptor; - if (!view.GetName().empty()) - { - view_instr_desc.name_ = view.GetName(); - } - if (!view.GetDescription().empty()) - { - view_instr_desc.description_ = view.GetDescription(); - } - if (GetInstrumentClass(instrument_descriptor.type_) == InstrumentClass::kSync) - { - auto storage = std::shared_ptr(new SyncMetricStorage( - view_instr_desc, view.GetAggregationType(), &view.GetAttributesProcessor(), - NoExemplarReservoir::GetNoExemplarReservoir())); - storage_registry_[instrument_descriptor.name_] = storage; - } - else - { - auto storage = std::shared_ptr(new AsyncMetricStorage( - view_instr_desc, view.GetAggregationType(), &view.GetAttributesProcessor())); - storage_registry_[instrument_descriptor.name_] = storage; - } + auto view_instr_desc = instrument_descriptor; + if (!view.GetName().empty()) + { + view_instr_desc.name_ = view.GetName(); + } + if (!view.GetDescription().empty()) + { + view_instr_desc.description_ = view.GetDescription(); + } + if (GetInstrumentClass(instrument_descriptor.type_) == InstrumentClass::kSync) + { + auto storage = std::shared_ptr(new SyncMetricStorage( + view_instr_desc, view.GetAggregationType(), &view.GetAttributesProcessor(), + NoExemplarReservoir::GetNoExemplarReservoir())); + storage_registry_[instrument_descriptor.name_] = storage; + } + else + { + if (instrument_descriptor.value_type_ == InstrumentValueType::kDouble) + { + auto storage = std::shared_ptr>(new AsyncMetricStorage( + view_instr_desc, view.GetAggregationType(), &view.GetAttributesProcessor())); + storage_registry_[instrument_descriptor.name_] = storage; + } + else + { + auto storage = std::shared_ptr>(new AsyncMetricStorage( + view_instr_desc, view.GetAggregationType(), &view.GetAttributesProcessor())); + storage_registry_[instrument_descriptor.name_] = storage; + } - auto multi_storage = static_cast(storages.get()); - multi_storage->AddStorage(storage); - return true; - }); + auto multi_storage = static_cast(storages.get()); + multi_storage->AddStorage(storage); + return true; + }); - if (!success) - { - OTEL_INTERNAL_LOG_ERROR( - "[Meter::RegisterMetricStorage] - Error during finding matching views." - << "Some of the matching view configurations mayn't be used for metric collection"); - } - return storages; + if (!success) + { + OTEL_INTERNAL_LOG_ERROR( + "[Meter::RegisterMetricStorage] - Error during finding matching views." + << "Some of the matching view configurations mayn't be used for metric collection"); + } + return storages; } # if 0 @@ -266,20 +274,20 @@ std::vector> Meter::RegisterAsyncMetricSt std::vector Meter::Collect(CollectorHandle *collector, opentelemetry::common::SystemTimestamp collect_ts) noexcept { - std::vector metric_data_list; - for (auto &metric_storage : storage_registry_) - { - metric_storage.second->Collect(collector, meter_context_->GetCollectors(), - meter_context_->GetSDKStartTime(), collect_ts, - [&metric_data_list](MetricData metric_data) { - metric_data_list.push_back(metric_data); - return true; - }); - } - return metric_data_list; + std::vector metric_data_list; + for (auto &metric_storage : storage_registry_) + { + metric_storage.second->Collect(collector, meter_context_->GetCollectors(), + meter_context_->GetSDKStartTime(), collect_ts, + [&metric_data_list](MetricData metric_data) { + metric_data_list.push_back(metric_data); + return true; + }); + } + return metric_data_list; } } // namespace metrics -} // namespace sdk +} // namespace metrics OPENTELEMETRY_END_NAMESPACE #endif diff --git a/sdk/src/metrics/state/observable_registry.cc b/sdk/src/metrics/state/observable_registry.cc new file mode 100644 index 0000000000..7aa5ff7b95 --- /dev/null +++ b/sdk/src/metrics/state/observable_registry.cc @@ -0,0 +1,66 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +#ifndef ENABLE_METRICS_PREVIEW + +# include "opentelemetry/sdk/metrics/state/observable_registry.h" +# include "opentelemetry/sdk/metrics/async_instruments.h" +# include "opentelemetry/sdk/metrics/observer_result.h" + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace sdk +{ +namespace metrics +{ + +void ObservableRegistry::AddCallback(opentelemetry::metrics::ObservableCallbackPtr callback, + void *state, + opentelemetry::metrics::ObservableInstrument *instrument) +{ + // TBD - Check if existing + std::unique_ptr record( + new ObservableCallbackRecord{callback, state, instrument}); + std::unique_lock lk(callbacks_m_); + callbacks_.push_back(std::move(record)); +} + +void ObservableRegistry::RemoveCallback(opentelemetry::metrics::ObservableCallbackPtr callback, + void *state, + opentelemetry::metrics::ObservableInstrument *instrument) +{ + auto new_end = std::remove_if( + callbacks_.begin(), callbacks_.end(), + [callback, state, instrument](const std::unique_ptr &record) { + return record->callback == callback && record->state == state && + record->instrument == instrument; + }); + std::unique_lock lk(callbacks_m_); + callbacks_.erase(new_end, callbacks_.end()); +} + +void ObservableRegistry::Observe(opentelemetry::common::SystemTimestamp collection_ts) +{ + std::unique_lock lk(callbacks_m_); + for (auto &callback_wrap : callbacks_) + { + if (static_cast(callback_wrap->instrument) + ->GetInstrumentDescriptor() + .value_type_ == InstrumentValueType::kDouble) + { + nostd::shared_ptr> ob_res( + new opentelemetry::sdk::metrics::ObserverResultT()); + callback_wrap->callback(ob_res, callback_wrap->state); + } + else + { + nostd::shared_ptr> ob_res( + new opentelemetry::sdk::metrics::ObserverResultT()); + callback_wrap->callback(ob_res, callback_wrap->state); + } + } +} + +} // namespace metrics +} // namespace sdk +OPENTELEMETRY_END_NAMESPACE +#endif From 1642a5bca01175631e00f8639be68979028dbadd Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Wed, 20 Jul 2022 00:33:28 -0700 Subject: [PATCH 13/26] Fix --- .../sdk/metrics/async_instruments.h | 24 +--- sdk/include/opentelemetry/sdk/metrics/meter.h | 2 +- .../sdk/metrics/state/async_metric_storage.h | 15 ++- sdk/src/metrics/CMakeLists.txt | 1 + sdk/src/metrics/async_instruments.cc | 52 ++++++++ sdk/src/metrics/meter.cc | 121 +++++++++--------- sdk/test/metrics/async_instruments_test.cc | 52 ++------ sdk/test/metrics/async_metric_storage_test.cc | 4 +- sdk/test/metrics/observer_result_test.cc | 2 +- 9 files changed, 141 insertions(+), 132 deletions(-) create mode 100644 sdk/src/metrics/async_instruments.cc diff --git a/sdk/include/opentelemetry/sdk/metrics/async_instruments.h b/sdk/include/opentelemetry/sdk/metrics/async_instruments.h index 64d5509b89..a1ec0b8451 100644 --- a/sdk/include/opentelemetry/sdk/metrics/async_instruments.h +++ b/sdk/include/opentelemetry/sdk/metrics/async_instruments.h @@ -37,33 +37,19 @@ class ObservableInstrument : public opentelemetry::metrics::ObservableInstrument { public: ObservableInstrument(InstrumentDescriptor instrument_descriptor, - std::unique_ptr storage) - : instrument_descriptor_(instrument_descriptor), - storage_(std::move(storage)), - observable_registry_{new ObservableRegistry()}, - Asynchronous(instrument_descriptor_.name_, - instrument_descriptor.description_, - instrument_descriptor_.unit_) - {} + std::unique_ptr storage); void AddCallback(opentelemetry::metrics::ObservableCallbackPtr callback, - void *state) noexcept override - { - observable_registry_->AddCallback(callback, state, this); - } + void *state) noexcept override; void RemoveCallback(opentelemetry::metrics::ObservableCallbackPtr callback, - void *state) noexcept override - { - observable_registry_->AddCallback(callback, state, this); - } + void *state) noexcept override; - const InstrumentDescriptor &GetInstrumentDescriptor() { return instrument_descriptor_; } + const InstrumentDescriptor &GetInstrumentDescriptor(); - const WritableMetricStorage *GetMetricStorage() { return storage_.get(); } + const WritableMetricStorage *GetMetricStorage(); private: -protected: InstrumentDescriptor instrument_descriptor_; std::unique_ptr storage_; std::unique_ptr observable_registry_; diff --git a/sdk/include/opentelemetry/sdk/metrics/meter.h b/sdk/include/opentelemetry/sdk/metrics/meter.h index 9909a489a4..c3924d740a 100644 --- a/sdk/include/opentelemetry/sdk/metrics/meter.h +++ b/sdk/include/opentelemetry/sdk/metrics/meter.h @@ -111,7 +111,7 @@ class Meter final : public opentelemetry::metrics::Meter std::unique_ptr RegisterMetricStorage( InstrumentDescriptor &instrument_descriptor); - void RegisterAsyncMetricStorage(InstrumentDescriptor &instrument_descriptor); + // void RegisterAsyncMetricStorage(InstrumentDescriptor &instrument_descriptor); }; } // namespace metrics } // namespace sdk diff --git a/sdk/include/opentelemetry/sdk/metrics/state/async_metric_storage.h b/sdk/include/opentelemetry/sdk/metrics/state/async_metric_storage.h index 970c2034a2..900f657359 100644 --- a/sdk/include/opentelemetry/sdk/metrics/state/async_metric_storage.h +++ b/sdk/include/opentelemetry/sdk/metrics/state/async_metric_storage.h @@ -25,14 +25,15 @@ template class AsyncMetricStorage : public MetricStorage { public: - AsyncMetricStorage(InstrumentDescriptor instrument_descriptor, - const AggregationType aggregation_type, - void (*measurement_callback)(opentelemetry::metrics::ObserverResult &, void *), - const AttributesProcessor *attributes_processor, - void *state = nullptr) + AsyncMetricStorage( + InstrumentDescriptor instrument_descriptor, + const AggregationType aggregation_type, + // void (*measurement_callback)(opentelemetry::metrics::ObserverResult &, void *), + const AttributesProcessor *attributes_processor, + void *state = nullptr) : instrument_descriptor_(instrument_descriptor), aggregation_type_{aggregation_type}, - measurement_collection_callback_{measurement_callback}, + // measurement_collection_callback_{measurement_callback}, attributes_processor_{attributes_processor}, state_{state}, cumulative_hash_map_(new AttributesHashMap()), @@ -49,7 +50,7 @@ class AsyncMetricStorage : public MetricStorage new opentelemetry::sdk::metrics::ObserverResultT(attributes_processor_)); // read the measurement using configured callback - measurement_collection_callback_(ob_res, state_); + // measurement_collection_callback_(ob_res, state_); std::shared_ptr delta_hash_map(new AttributesHashMap()); // process the read measurements - aggregate and store in hashmap for (auto &measurement : ob_res->GetMeasurements()) diff --git a/sdk/src/metrics/CMakeLists.txt b/sdk/src/metrics/CMakeLists.txt index 8d2a431eee..dbbb465183 100644 --- a/sdk/src/metrics/CMakeLists.txt +++ b/sdk/src/metrics/CMakeLists.txt @@ -1,5 +1,6 @@ add_library( opentelemetry_metrics + async_instruments.cc meter_provider.cc meter.cc meter_context.cc diff --git a/sdk/src/metrics/async_instruments.cc b/sdk/src/metrics/async_instruments.cc new file mode 100644 index 0000000000..022b321f4c --- /dev/null +++ b/sdk/src/metrics/async_instruments.cc @@ -0,0 +1,52 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +#ifndef ENABLE_METRICS_PREVIEW +# include "opentelemetry/sdk/metrics/async_instruments.h" +# include "opentelemetry/sdk/metrics/state/metric_storage.h" +# include "opentelemetry/sdk_config.h" + +# include + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace sdk +{ +namespace metrics +{ + +ObservableInstrument::ObservableInstrument(InstrumentDescriptor instrument_descriptor, + std::unique_ptr storage) + : instrument_descriptor_(instrument_descriptor), + storage_(std::move(storage)), + observable_registry_{new ObservableRegistry()}, + Asynchronous(instrument_descriptor_.name_, + instrument_descriptor.description_, + instrument_descriptor_.unit_) +{} + +void ObservableInstrument::AddCallback(opentelemetry::metrics::ObservableCallbackPtr callback, + void *state) noexcept +{ + observable_registry_->AddCallback(callback, state, this); +} + +void ObservableInstrument::RemoveCallback(opentelemetry::metrics::ObservableCallbackPtr callback, + void *state) noexcept +{ + observable_registry_->AddCallback(callback, state, this); +} + +const InstrumentDescriptor &ObservableInstrument::GetInstrumentDescriptor() +{ + return instrument_descriptor_; +} + +const WritableMetricStorage *ObservableInstrument::GetMetricStorage() +{ + return storage_.get(); +} + +} // namespace metrics +} // namespace sdk +OPENTELEMETRY_END_NAMESPACE +#endif diff --git a/sdk/src/metrics/meter.cc b/sdk/src/metrics/meter.cc index cdf59aab15..85b234e60b 100644 --- a/sdk/src/metrics/meter.cc +++ b/sdk/src/metrics/meter.cc @@ -66,7 +66,7 @@ nostd::shared_ptr Meter::CreateLon std::string{name.data(), name.size()}, std::string{description.data(), description.size()}, std::string{unit.data(), unit.size()}, InstrumentType::kObservableCounter, InstrumentValueType::kLong}; - RegisterAsyncMetricStorage(instrument_descriptor); + RegisterMetricStorage(instrument_descriptor); } nostd::shared_ptr @@ -78,7 +78,7 @@ Meter::CreateDoubleObservableCounter(nostd::string_view name, std::string{name.data(), name.size()}, std::string{description.data(), description.size()}, std::string{unit.data(), unit.size()}, InstrumentType::kObservableCounter, InstrumentValueType::kDouble}; - RegisterAsyncMetricStorage(instrument_descriptor); + RegisterMetricStorage(instrument_descriptor); } nostd::shared_ptr> Meter::CreateLongHistogram( @@ -118,7 +118,7 @@ nostd::shared_ptr Meter::CreateLon std::string{name.data(), name.size()}, std::string{description.data(), description.size()}, std::string{unit.data(), unit.size()}, InstrumentType::kObservableGauge, InstrumentValueType::kLong}; - RegisterAsyncMetricStorage(instrument_descriptor); + RegisterMetricStorage(instrument_descriptor); } nostd::shared_ptr Meter::CreateDoubleObservableGauge( @@ -130,7 +130,7 @@ nostd::shared_ptr Meter::CreateDou std::string{name.data(), name.size()}, std::string{description.data(), description.size()}, std::string{unit.data(), unit.size()}, InstrumentType::kObservableGauge, InstrumentValueType::kDouble}; - RegisterAsyncMetricStorage(instrument_descriptor); + RegisterMetricStorage(instrument_descriptor); } nostd::shared_ptr> Meter::CreateLongUpDownCounter( @@ -170,7 +170,7 @@ Meter::CreateLongObservableUpDownCounter(nostd::string_view name, std::string{name.data(), name.size()}, std::string{description.data(), description.size()}, std::string{unit.data(), unit.size()}, InstrumentType::kObservableUpDownCounter, InstrumentValueType::kLong}; - RegisterAsyncMetricStorage(instrument_descriptor); + RegisterMetricStorage(instrument_descriptor); } nostd::shared_ptr @@ -182,7 +182,7 @@ Meter::CreateDoubleObservableUpDownCounter(nostd::string_view name, std::string{name.data(), name.size()}, std::string{description.data(), description.size()}, std::string{unit.data(), unit.size()}, InstrumentType::kObservableUpDownCounter, InstrumentValueType::kDouble}; - RegisterAsyncMetricStorage(instrument_descriptor); + RegisterMetricStorage(instrument_descriptor); } const sdk::instrumentationlibrary::InstrumentationLibrary *Meter::GetInstrumentationLibrary() @@ -200,49 +200,50 @@ std::unique_ptr Meter::RegisterMetricStorage( auto success = view_registry->FindViews( instrument_descriptor, *instrumentation_library_, [this, &instrument_descriptor, &storages](const View &view) { - auto view_instr_desc = instrument_descriptor; - if (!view.GetName().empty()) - { - view_instr_desc.name_ = view.GetName(); - } - if (!view.GetDescription().empty()) - { - view_instr_desc.description_ = view.GetDescription(); - } - if (GetInstrumentClass(instrument_descriptor.type_) == InstrumentClass::kSync) - { - auto storage = std::shared_ptr(new SyncMetricStorage( - view_instr_desc, view.GetAggregationType(), &view.GetAttributesProcessor(), - NoExemplarReservoir::GetNoExemplarReservoir())); - storage_registry_[instrument_descriptor.name_] = storage; - } - else - { - if (instrument_descriptor.value_type_ == InstrumentValueType::kDouble) - { - auto storage = std::shared_ptr>(new AsyncMetricStorage( - view_instr_desc, view.GetAggregationType(), &view.GetAttributesProcessor())); - storage_registry_[instrument_descriptor.name_] = storage; - } - else - { - auto storage = std::shared_ptr>(new AsyncMetricStorage( - view_instr_desc, view.GetAggregationType(), &view.GetAttributesProcessor())); - storage_registry_[instrument_descriptor.name_] = storage; - } - - auto multi_storage = static_cast(storages.get()); - multi_storage->AddStorage(storage); - return true; - }); + auto view_instr_desc = instrument_descriptor; + if (!view.GetName().empty()) + { + view_instr_desc.name_ = view.GetName(); + } + if (!view.GetDescription().empty()) + { + view_instr_desc.description_ = view.GetDescription(); + } + auto multi_storage = static_cast(storages.get()); + if (GetInstrumentClass(instrument_descriptor.type_) == InstrumentClass::kSync) + { + auto storage = std::shared_ptr(new SyncMetricStorage( + view_instr_desc, view.GetAggregationType(), &view.GetAttributesProcessor(), + NoExemplarReservoir::GetNoExemplarReservoir())); + storage_registry_[instrument_descriptor.name_] = storage; + multi_storage->AddStorage(storage); + } + else + { + if (instrument_descriptor.value_type_ == InstrumentValueType::kDouble) + { + auto storage = + std::shared_ptr>(new AsyncMetricStorage( + view_instr_desc, view.GetAggregationType(), &view.GetAttributesProcessor())); + storage_registry_[instrument_descriptor.name_] = storage; + } + else + { + auto storage = std::shared_ptr>(new AsyncMetricStorage( + view_instr_desc, view.GetAggregationType(), &view.GetAttributesProcessor())); + storage_registry_[instrument_descriptor.name_] = storage; + } + } + return true; + }); - if (!success) - { - OTEL_INTERNAL_LOG_ERROR( - "[Meter::RegisterMetricStorage] - Error during finding matching views." - << "Some of the matching view configurations mayn't be used for metric collection"); - } - return storages; + if (!success) + { + OTEL_INTERNAL_LOG_ERROR( + "[Meter::RegisterMetricStorage] - Error during finding matching views." + << "Some of the matching view configurations mayn't be used for metric collection"); + } + return storages; } # if 0 @@ -274,20 +275,20 @@ std::vector> Meter::RegisterAsyncMetricSt std::vector Meter::Collect(CollectorHandle *collector, opentelemetry::common::SystemTimestamp collect_ts) noexcept { - std::vector metric_data_list; - for (auto &metric_storage : storage_registry_) - { - metric_storage.second->Collect(collector, meter_context_->GetCollectors(), - meter_context_->GetSDKStartTime(), collect_ts, - [&metric_data_list](MetricData metric_data) { - metric_data_list.push_back(metric_data); - return true; - }); - } - return metric_data_list; + std::vector metric_data_list; + for (auto &metric_storage : storage_registry_) + { + metric_storage.second->Collect(collector, meter_context_->GetCollectors(), + meter_context_->GetSDKStartTime(), collect_ts, + [&metric_data_list](MetricData metric_data) { + metric_data_list.push_back(metric_data); + return true; + }); + } + return metric_data_list; } } // namespace metrics -} // namespace metrics +} // namespace sdk OPENTELEMETRY_END_NAMESPACE #endif diff --git a/sdk/test/metrics/async_instruments_test.cc b/sdk/test/metrics/async_instruments_test.cc index ff9504f78c..4190883a57 100644 --- a/sdk/test/metrics/async_instruments_test.cc +++ b/sdk/test/metrics/async_instruments_test.cc @@ -3,6 +3,7 @@ #ifndef ENABLE_METRICS_PREVIEW # include "opentelemetry/sdk/metrics/async_instruments.h" +# include "opentelemetry/sdk/metrics/state/multi_metric_storage.h" # include @@ -11,50 +12,17 @@ using namespace opentelemetry::sdk::metrics; using M = std::map; -void asyc_generate_measurements_long(opentelemetry::metrics::ObserverResult &observer) {} +void asyc_generate_measurements(opentelemetry::metrics::ObserverResult observer, void *state) {} -void asyc_generate_measurements_double(opentelemetry::metrics::ObserverResult &observer) {} - -TEST(AsyncInstruments, LongObservableCounter) -{ - auto asyc_generate_meas_long = [](opentelemetry::metrics::ObserverResult &observer) {}; - EXPECT_NO_THROW( - LongObservableCounter counter("long_counter", asyc_generate_meas_long, "description", "1")); -} - -TEST(AsyncInstruments, DoubleObservableCounter) -{ - auto asyc_generate_meas_double = [](opentelemetry::metrics::ObserverResult &observer) {}; - EXPECT_NO_THROW(DoubleObservableCounter counter("long_counter", asyc_generate_meas_double, - "description", "1")); -} - -TEST(AsyncInstruments, LongObservableGauge) -{ - auto asyc_generate_meas_long = [](opentelemetry::metrics::ObserverResult &observer) {}; - EXPECT_NO_THROW( - LongObservableGauge counter("long_counter", asyc_generate_meas_long, "description", "1")); -} - -TEST(AsyncInstruments, DoubleObservableGauge) -{ - auto asyc_generate_meas_double = [](opentelemetry::metrics::ObserverResult &observer) {}; - EXPECT_NO_THROW( - DoubleObservableGauge counter("long_counter", asyc_generate_meas_double, "description", "1")); -} - -TEST(AsyncInstruments, LongObservableUpDownCounter) -{ - auto asyc_generate_meas_long = [](opentelemetry::metrics::ObserverResult &observer) {}; - EXPECT_NO_THROW(LongObservableUpDownCounter counter("long_counter", asyc_generate_meas_long, - "description", "1")); -} - -TEST(AsyncInstruments, DoubleObservableUpDownCounter) +TEST(AsyncInstruments, ObservableInstrument) { - auto asyc_generate_meas_double = [](opentelemetry::metrics::ObserverResult &observer) {}; - EXPECT_NO_THROW(DoubleObservableUpDownCounter counter("long_counter", asyc_generate_meas_double, - "description", "1")); + InstrumentDescriptor instrument_descriptor = {"long_counter", "description", "1", + InstrumentType::kObservableCounter, + InstrumentValueType::kLong}; + std::unique_ptr metric_storage(new MultiMetricStorage()); + auto asyc_generate_meas_long = [](opentelemetry::metrics::ObserverResultT &observer) {}; + ObservableInstrument observable_counter_long(instrument_descriptor, std::move(metric_storage)); + observable_counter_long.AddCallback(asyc_generate_measurements, nullptr); } #endif diff --git a/sdk/test/metrics/async_metric_storage_test.cc b/sdk/test/metrics/async_metric_storage_test.cc index 2be5332a8d..c7a9669592 100644 --- a/sdk/test/metrics/async_metric_storage_test.cc +++ b/sdk/test/metrics/async_metric_storage_test.cc @@ -39,7 +39,7 @@ class WritableMetricStorageTestFixture : public ::testing::TestWithParam &observer_result, + static void Fetcher(opentelemetry::metrics::ObserverResultT &observer_result, void * /*state*/) { fetch_count++; @@ -97,7 +97,7 @@ TEST_P(WritableMetricStorageTestFixture, TestAggregation) MeasurementFetcher measurement_fetcher; opentelemetry::sdk::metrics::AsyncMetricStorage storage(instr_desc, AggregationType::kSum, - MeasurementFetcher::Fetcher, + // MeasurementFetcher::Fetcher, new DefaultAttributesProcessor()); storage.Collect(collector.get(), collectors, sdk_start_ts, collection_ts, diff --git a/sdk/test/metrics/observer_result_test.cc b/sdk/test/metrics/observer_result_test.cc index a4cc28ae54..55197a3cbf 100644 --- a/sdk/test/metrics/observer_result_test.cc +++ b/sdk/test/metrics/observer_result_test.cc @@ -12,7 +12,7 @@ TEST(ObserverResult, BasicTests) { const AttributesProcessor *attributes_processor = new DefaultAttributesProcessor(); - ObserverResult observer_result(attributes_processor); + ObserverResultT observer_result(attributes_processor); observer_result.Observe(10l); observer_result.Observe(20l); From 6db8c784d14b030e64731944ea17ac0069225371 Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Thu, 21 Jul 2022 17:30:23 -0700 Subject: [PATCH 14/26] fix --- .../sdk/metrics/async_instruments.h | 8 +-- sdk/include/opentelemetry/sdk/metrics/meter.h | 7 ++- .../sdk/metrics/observer_result.h | 2 +- .../sdk/metrics/state/async_metric_storage.h | 16 +++++- .../sdk/metrics/state/metric_storage.h | 25 +++++++-- .../sdk/metrics/state/multi_metric_storage.h | 37 +++++++++++- .../sdk/metrics/state/sync_metric_storage.h | 2 +- .../sdk/metrics/sync_instruments.h | 18 +++--- sdk/src/metrics/async_instruments.cc | 4 +- sdk/src/metrics/meter.cc | 56 ++++++++++--------- sdk/src/metrics/state/observable_registry.cc | 13 ++++- sdk/src/metrics/sync_instruments.cc | 12 ++-- 12 files changed, 134 insertions(+), 66 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/metrics/async_instruments.h b/sdk/include/opentelemetry/sdk/metrics/async_instruments.h index a1ec0b8451..4cff7032cf 100644 --- a/sdk/include/opentelemetry/sdk/metrics/async_instruments.h +++ b/sdk/include/opentelemetry/sdk/metrics/async_instruments.h @@ -15,7 +15,7 @@ namespace sdk namespace metrics { -class WritableMetricStorage; +class AsyncWritableMetricStorage; class Asynchronous { @@ -37,7 +37,7 @@ class ObservableInstrument : public opentelemetry::metrics::ObservableInstrument { public: ObservableInstrument(InstrumentDescriptor instrument_descriptor, - std::unique_ptr storage); + std::unique_ptr storage); void AddCallback(opentelemetry::metrics::ObservableCallbackPtr callback, void *state) noexcept override; @@ -47,11 +47,11 @@ class ObservableInstrument : public opentelemetry::metrics::ObservableInstrument const InstrumentDescriptor &GetInstrumentDescriptor(); - const WritableMetricStorage *GetMetricStorage(); + AsyncWritableMetricStorage *GetMetricStorage(); private: InstrumentDescriptor instrument_descriptor_; - std::unique_ptr storage_; + std::unique_ptr storage_; std::unique_ptr observable_registry_; }; } // namespace metrics diff --git a/sdk/include/opentelemetry/sdk/metrics/meter.h b/sdk/include/opentelemetry/sdk/metrics/meter.h index c3924d740a..a378480508 100644 --- a/sdk/include/opentelemetry/sdk/metrics/meter.h +++ b/sdk/include/opentelemetry/sdk/metrics/meter.h @@ -20,7 +20,8 @@ namespace metrics { class MetricStorage; -class WritableMetricStorage; +class SyncWritableMetricStorage; +class AsyncWritableMetricsStorge; class Meter final : public opentelemetry::metrics::Meter { @@ -108,10 +109,10 @@ class Meter final : public opentelemetry::metrics::Meter // Mapping between instrument-name and Aggregation Storage. std::unordered_map> storage_registry_; - std::unique_ptr RegisterMetricStorage( + std::unique_ptr RegisterSyncMetricStorage( InstrumentDescriptor &instrument_descriptor); - // void RegisterAsyncMetricStorage(InstrumentDescriptor &instrument_descriptor); + std::unique_ptr RegisterAsyncMetricStorage(InstrumentDescriptor &instrument_descriptor); }; } // namespace metrics } // namespace sdk diff --git a/sdk/include/opentelemetry/sdk/metrics/observer_result.h b/sdk/include/opentelemetry/sdk/metrics/observer_result.h index 0c7463b556..f371c152d4 100644 --- a/sdk/include/opentelemetry/sdk/metrics/observer_result.h +++ b/sdk/include/opentelemetry/sdk/metrics/observer_result.h @@ -19,7 +19,7 @@ template class ObserverResultT final : public opentelemetry::metrics::ObserverResultT { public: - ObserverResultT(const AttributesProcessor *attributes_processor = nullptr) + explicit ObserverResultT(const AttributesProcessor *attributes_processor = nullptr) : attributes_processor_(attributes_processor) {} diff --git a/sdk/include/opentelemetry/sdk/metrics/state/async_metric_storage.h b/sdk/include/opentelemetry/sdk/metrics/state/async_metric_storage.h index 900f657359..4839fa7715 100644 --- a/sdk/include/opentelemetry/sdk/metrics/state/async_metric_storage.h +++ b/sdk/include/opentelemetry/sdk/metrics/state/async_metric_storage.h @@ -22,7 +22,7 @@ namespace metrics { template -class AsyncMetricStorage : public MetricStorage +class AsyncMetricStorage : public MetricStorage, public AsyncWritableMetricStorage { public: AsyncMetricStorage( @@ -40,14 +40,24 @@ class AsyncMetricStorage : public MetricStorage temporal_metric_storage_(instrument_descriptor) {} + virtual void RecordLong( + const std::unordered_map &measurements, + opentelemetry::common::SystemTimestamp observation_time) noexcept override + {} + + virtual void RecordDouble( + const std::unordered_map &measurements, + opentelemetry::common::SystemTimestamp observation_time) noexcept override + {} + bool Collect(CollectorHandle *collector, nostd::span> collectors, opentelemetry::common::SystemTimestamp sdk_start_ts, opentelemetry::common::SystemTimestamp collection_ts, nostd::function_ref metric_collection_callback) noexcept override { - nostd::shared_ptr> ob_res( - new opentelemetry::sdk::metrics::ObserverResultT(attributes_processor_)); + nostd::shared_ptr> ob_res( + new opentelemetry::sdk::metrics::ObserverResultT(nullptr)); // read the measurement using configured callback // measurement_collection_callback_(ob_res, state_); diff --git a/sdk/include/opentelemetry/sdk/metrics/state/metric_storage.h b/sdk/include/opentelemetry/sdk/metrics/state/metric_storage.h index 6a0ed604b6..3676a0f707 100644 --- a/sdk/include/opentelemetry/sdk/metrics/state/metric_storage.h +++ b/sdk/include/opentelemetry/sdk/metrics/state/metric_storage.h @@ -31,7 +31,7 @@ class MetricStorage }; /* Represents the sync metric storage */ -class WritableMetricStorage +class SyncWritableMetricStorage { public: virtual void RecordLong(long value, const opentelemetry::context::Context &context) noexcept = 0; @@ -47,7 +47,7 @@ class WritableMetricStorage const opentelemetry::common::KeyValueIterable &attributes, const opentelemetry::context::Context &context) noexcept = 0; - virtual ~WritableMetricStorage() = default; + virtual ~SyncWritableMetricStorage() = default; }; /* Represents the async metric stroage */ @@ -56,10 +56,12 @@ class AsyncWritableMetricStorage public: /* Records a batch of measurements */ virtual void RecordLong( - std::unordered_map &measurements); + const std::unordered_map &measurements, + opentelemetry::common::SystemTimestamp observation_time) noexcept = 0; virtual void RecordDouble( - std::unordered_map &measurements); + const std::unordered_map &measurements, + opentelemetry::common::SystemTimestamp observation_time) noexcept = 0; }; class NoopMetricStorage : public MetricStorage @@ -76,7 +78,7 @@ class NoopMetricStorage : public MetricStorage } }; -class NoopWritableMetricStorage : public WritableMetricStorage +class NoopWritableMetricStorage : public SyncWritableMetricStorage { public: void RecordLong(long value, const opentelemetry::context::Context &context) noexcept = 0; @@ -95,6 +97,19 @@ class NoopWritableMetricStorage : public WritableMetricStorage {} }; +class NoopAsyncWritableMetricStorage : public AsyncWritableMetricStorage +{ +public: + void RecordLong(const std::unordered_map &measurements, + opentelemetry::common::SystemTimestamp observation_time) noexcept override + {} + + void RecordDouble( + const std::unordered_map &measurements, + opentelemetry::common::SystemTimestamp observation_time) noexcept override + {} +}; + } // namespace metrics } // namespace sdk OPENTELEMETRY_END_NAMESPACE diff --git a/sdk/include/opentelemetry/sdk/metrics/state/multi_metric_storage.h b/sdk/include/opentelemetry/sdk/metrics/state/multi_metric_storage.h index ceeafa0406..fbdcbf8407 100644 --- a/sdk/include/opentelemetry/sdk/metrics/state/multi_metric_storage.h +++ b/sdk/include/opentelemetry/sdk/metrics/state/multi_metric_storage.h @@ -15,10 +15,10 @@ namespace sdk namespace metrics { -class MultiMetricStorage : public WritableMetricStorage +class SyncMultiMetricStorage : public SyncWritableMetricStorage { public: - void AddStorage(std::shared_ptr storage) { storages_.push_back(storage); } + void AddStorage(std::shared_ptr storage) { storages_.push_back(storage); } virtual void RecordLong(long value, const opentelemetry::context::Context &context) noexcept override @@ -59,7 +59,38 @@ class MultiMetricStorage : public WritableMetricStorage } private: - std::vector> storages_; + std::vector> storages_; +}; + +class AsyncMultiMetricStorage : public AsyncWritableMetricStorage +{ +public: + void AddStorage(std::shared_ptr storage) + { + storages_.push_back(storage); + } + + void RecordLong(const std::unordered_map &measurements, + opentelemetry::common::SystemTimestamp observation_time) noexcept override + { + for (auto &s : storages_) + { + s->RecordLong(measurements, observation_time); + } + } + + void RecordDouble( + const std::unordered_map &measurements, + opentelemetry::common::SystemTimestamp observation_time) noexcept override + { + for (auto &s : storages_) + { + s->RecordDouble(measurements, observation_time); + } + } + +private: + std::vector> storages_; }; } // namespace metrics diff --git a/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h b/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h index 278f7dbe3f..0dfe6c07b8 100644 --- a/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h +++ b/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h @@ -22,7 +22,7 @@ namespace sdk { namespace metrics { -class SyncMetricStorage : public MetricStorage, public WritableMetricStorage +class SyncMetricStorage : public MetricStorage, public SyncWritableMetricStorage { public: diff --git a/sdk/include/opentelemetry/sdk/metrics/sync_instruments.h b/sdk/include/opentelemetry/sdk/metrics/sync_instruments.h index f4c9bae323..17a2069afe 100644 --- a/sdk/include/opentelemetry/sdk/metrics/sync_instruments.h +++ b/sdk/include/opentelemetry/sdk/metrics/sync_instruments.h @@ -17,26 +17,26 @@ namespace metrics { // forward declaration -class WritableMetricStorage; +class SyncWritableMetricStorage; class Synchronous { public: Synchronous(InstrumentDescriptor instrument_descriptor, - std::unique_ptr storage) + std::unique_ptr storage) : instrument_descriptor_(instrument_descriptor), storage_(std::move(storage)) {} protected: InstrumentDescriptor instrument_descriptor_; - std::unique_ptr storage_; + std::unique_ptr storage_; }; class LongCounter : public Synchronous, public opentelemetry::metrics::Counter { public: LongCounter(InstrumentDescriptor instrument_descriptor, - std::unique_ptr storage); + std::unique_ptr storage); void Add(long value, const opentelemetry::common::KeyValueIterable &attributes) noexcept override; void Add(long value, @@ -52,7 +52,7 @@ class DoubleCounter : public Synchronous, public opentelemetry::metrics::Counter public: DoubleCounter(InstrumentDescriptor instrument_descriptor, - std::unique_ptr storage); + std::unique_ptr storage); void Add(double value, const opentelemetry::common::KeyValueIterable &attributes) noexcept override; @@ -68,7 +68,7 @@ class LongUpDownCounter : public Synchronous, public opentelemetry::metrics::UpD { public: LongUpDownCounter(InstrumentDescriptor instrument_descriptor, - std::unique_ptr storage); + std::unique_ptr storage); void Add(long value, const opentelemetry::common::KeyValueIterable &attributes) noexcept override; void Add(long value, @@ -83,7 +83,7 @@ class DoubleUpDownCounter : public Synchronous, public opentelemetry::metrics::U { public: DoubleUpDownCounter(InstrumentDescriptor instrument_descriptor, - std::unique_ptr storage); + std::unique_ptr storage); void Add(double value, const opentelemetry::common::KeyValueIterable &attributes) noexcept override; @@ -99,7 +99,7 @@ class LongHistogram : public Synchronous, public opentelemetry::metrics::Histogr { public: LongHistogram(InstrumentDescriptor instrument_descriptor, - std::unique_ptr storage); + std::unique_ptr storage); void Record(long value, const opentelemetry::common::KeyValueIterable &attributes, @@ -112,7 +112,7 @@ class DoubleHistogram : public Synchronous, public opentelemetry::metrics::Histo { public: DoubleHistogram(InstrumentDescriptor instrument_descriptor, - std::unique_ptr storage); + std::unique_ptr storage); void Record(double value, const opentelemetry::common::KeyValueIterable &attributes, diff --git a/sdk/src/metrics/async_instruments.cc b/sdk/src/metrics/async_instruments.cc index 022b321f4c..9c5a9750bc 100644 --- a/sdk/src/metrics/async_instruments.cc +++ b/sdk/src/metrics/async_instruments.cc @@ -15,7 +15,7 @@ namespace metrics { ObservableInstrument::ObservableInstrument(InstrumentDescriptor instrument_descriptor, - std::unique_ptr storage) + std::unique_ptr storage) : instrument_descriptor_(instrument_descriptor), storage_(std::move(storage)), observable_registry_{new ObservableRegistry()}, @@ -41,7 +41,7 @@ const InstrumentDescriptor &ObservableInstrument::GetInstrumentDescriptor() return instrument_descriptor_; } -const WritableMetricStorage *ObservableInstrument::GetMetricStorage() +AsyncWritableMetricStorage *ObservableInstrument::GetMetricStorage() { return storage_.get(); } diff --git a/sdk/src/metrics/meter.cc b/sdk/src/metrics/meter.cc index 85b234e60b..cac0390499 100644 --- a/sdk/src/metrics/meter.cc +++ b/sdk/src/metrics/meter.cc @@ -38,7 +38,7 @@ nostd::shared_ptr> Meter::CreateLongCounter(nostd::string InstrumentDescriptor instrument_descriptor = { std::string{name.data(), name.size()}, std::string{description.data(), description.size()}, std::string{unit.data(), unit.size()}, InstrumentType::kCounter, InstrumentValueType::kLong}; - auto storage = RegisterMetricStorage(instrument_descriptor); + auto storage = RegisterSyncMetricStorage(instrument_descriptor); return nostd::shared_ptr>( new LongCounter(instrument_descriptor, std::move(storage))); } @@ -52,7 +52,7 @@ nostd::shared_ptr> Meter::CreateDoubleCounter( std::string{name.data(), name.size()}, std::string{description.data(), description.size()}, std::string{unit.data(), unit.size()}, InstrumentType::kCounter, InstrumentValueType::kDouble}; - auto storage = RegisterMetricStorage(instrument_descriptor); + auto storage = RegisterSyncMetricStorage(instrument_descriptor); return nostd::shared_ptr>{ new DoubleCounter(instrument_descriptor, std::move(storage))}; } @@ -66,7 +66,7 @@ nostd::shared_ptr Meter::CreateLon std::string{name.data(), name.size()}, std::string{description.data(), description.size()}, std::string{unit.data(), unit.size()}, InstrumentType::kObservableCounter, InstrumentValueType::kLong}; - RegisterMetricStorage(instrument_descriptor); + RegisterAsyncMetricStorage(instrument_descriptor); } nostd::shared_ptr @@ -78,7 +78,7 @@ Meter::CreateDoubleObservableCounter(nostd::string_view name, std::string{name.data(), name.size()}, std::string{description.data(), description.size()}, std::string{unit.data(), unit.size()}, InstrumentType::kObservableCounter, InstrumentValueType::kDouble}; - RegisterMetricStorage(instrument_descriptor); + RegisterAsyncMetricStorage(instrument_descriptor); } nostd::shared_ptr> Meter::CreateLongHistogram( @@ -90,7 +90,7 @@ nostd::shared_ptr> Meter::CreateLongHistogram( std::string{name.data(), name.size()}, std::string{description.data(), description.size()}, std::string{unit.data(), unit.size()}, InstrumentType::kHistogram, InstrumentValueType::kLong}; - auto storage = RegisterMetricStorage(instrument_descriptor); + auto storage = RegisterSyncMetricStorage(instrument_descriptor); return nostd::shared_ptr>{ new LongHistogram(instrument_descriptor, std::move(storage))}; } @@ -104,7 +104,7 @@ nostd::shared_ptr> Meter::CreateDoubleHistogram( std::string{name.data(), name.size()}, std::string{description.data(), description.size()}, std::string{unit.data(), unit.size()}, InstrumentType::kHistogram, InstrumentValueType::kDouble}; - auto storage = RegisterMetricStorage(instrument_descriptor); + auto storage = RegisterSyncMetricStorage(instrument_descriptor); return nostd::shared_ptr>{ new DoubleHistogram(instrument_descriptor, std::move(storage))}; } @@ -118,7 +118,7 @@ nostd::shared_ptr Meter::CreateLon std::string{name.data(), name.size()}, std::string{description.data(), description.size()}, std::string{unit.data(), unit.size()}, InstrumentType::kObservableGauge, InstrumentValueType::kLong}; - RegisterMetricStorage(instrument_descriptor); + RegisterAsyncMetricStorage(instrument_descriptor); } nostd::shared_ptr Meter::CreateDoubleObservableGauge( @@ -130,7 +130,7 @@ nostd::shared_ptr Meter::CreateDou std::string{name.data(), name.size()}, std::string{description.data(), description.size()}, std::string{unit.data(), unit.size()}, InstrumentType::kObservableGauge, InstrumentValueType::kDouble}; - RegisterMetricStorage(instrument_descriptor); + RegisterAsyncMetricStorage(instrument_descriptor); } nostd::shared_ptr> Meter::CreateLongUpDownCounter( @@ -142,7 +142,7 @@ nostd::shared_ptr> Meter::CreateLongUpDownCounter( std::string{name.data(), name.size()}, std::string{description.data(), description.size()}, std::string{unit.data(), unit.size()}, InstrumentType::kUpDownCounter, InstrumentValueType::kLong}; - auto storage = RegisterMetricStorage(instrument_descriptor); + auto storage = RegisterSyncMetricStorage(instrument_descriptor); return nostd::shared_ptr>{ new LongUpDownCounter(instrument_descriptor, std::move(storage))}; } @@ -156,7 +156,7 @@ nostd::shared_ptr> Meter::CreateDoubleUpDownCount std::string{name.data(), name.size()}, std::string{description.data(), description.size()}, std::string{unit.data(), unit.size()}, InstrumentType::kUpDownCounter, InstrumentValueType::kDouble}; - auto storage = RegisterMetricStorage(instrument_descriptor); + auto storage = RegisterSyncMetricStorage(instrument_descriptor); return nostd::shared_ptr>{ new DoubleUpDownCounter(instrument_descriptor, std::move(storage))}; } @@ -170,7 +170,7 @@ Meter::CreateLongObservableUpDownCounter(nostd::string_view name, std::string{name.data(), name.size()}, std::string{description.data(), description.size()}, std::string{unit.data(), unit.size()}, InstrumentType::kObservableUpDownCounter, InstrumentValueType::kLong}; - RegisterMetricStorage(instrument_descriptor); + RegisterAsyncMetricStorage(instrument_descriptor); } nostd::shared_ptr @@ -182,7 +182,7 @@ Meter::CreateDoubleObservableUpDownCounter(nostd::string_view name, std::string{name.data(), name.size()}, std::string{description.data(), description.size()}, std::string{unit.data(), unit.size()}, InstrumentType::kObservableUpDownCounter, InstrumentValueType::kDouble}; - RegisterMetricStorage(instrument_descriptor); + RegisterAsyncMetricStorage(instrument_descriptor); } const sdk::instrumentationlibrary::InstrumentationLibrary *Meter::GetInstrumentationLibrary() @@ -191,11 +191,11 @@ const sdk::instrumentationlibrary::InstrumentationLibrary *Meter::GetInstrumenta return instrumentation_library_.get(); } -std::unique_ptr Meter::RegisterMetricStorage( +std::unique_ptr Meter::RegisterSyncMetricStorage( InstrumentDescriptor &instrument_descriptor) { auto view_registry = meter_context_->GetViewRegistry(); - std::unique_ptr storages(new MultiMetricStorage()); + std::unique_ptr storages(new SyncMultiMetricStorage()); auto success = view_registry->FindViews( instrument_descriptor, *instrumentation_library_, @@ -209,16 +209,16 @@ std::unique_ptr Meter::RegisterMetricStorage( { view_instr_desc.description_ = view.GetDescription(); } - auto multi_storage = static_cast(storages.get()); - if (GetInstrumentClass(instrument_descriptor.type_) == InstrumentClass::kSync) - { + auto multi_storage = static_cast(storages.get()); + //if (GetInstrumentClass(instrument_descriptor.type_) == InstrumentClass::kSync) + //{ auto storage = std::shared_ptr(new SyncMetricStorage( view_instr_desc, view.GetAggregationType(), &view.GetAttributesProcessor(), NoExemplarReservoir::GetNoExemplarReservoir())); storage_registry_[instrument_descriptor.name_] = storage; multi_storage->AddStorage(storage); - } - else + //} + /*else { if (instrument_descriptor.value_type_ == InstrumentValueType::kDouble) { @@ -226,14 +226,16 @@ std::unique_ptr Meter::RegisterMetricStorage( std::shared_ptr>(new AsyncMetricStorage( view_instr_desc, view.GetAggregationType(), &view.GetAttributesProcessor())); storage_registry_[instrument_descriptor.name_] = storage; + multi_storage->AddStorage(storage); } else { auto storage = std::shared_ptr>(new AsyncMetricStorage( view_instr_desc, view.GetAggregationType(), &view.GetAttributesProcessor())); storage_registry_[instrument_descriptor.name_] = storage; - } - } + multi_storage->AddStorage(storage); + }*/ + //} return true; }); @@ -246,14 +248,13 @@ std::unique_ptr Meter::RegisterMetricStorage( return storages; } -# if 0 -std::vector> Meter::RegisterAsyncMetricStorage(InstrumentDescriptor &instrument_descriptor) +std::unique_ptr Meter::RegisterAsyncMetricStorage(InstrumentDescriptor &instrument_descriptor) { auto view_registry = meter_context_->GetViewRegistry(); - std::vector> storages; + std::unique_ptr storages(new AsyncMultiMetricStorage()); auto success = view_registry->FindViews( instrument_descriptor, *instrumentation_library_, - [this, &instrument_descriptor](const View &view) { + [this, &instrument_descriptor, &storages](const View &view) { auto view_instr_desc = instrument_descriptor; if (!view.GetName().empty()) { @@ -263,18 +264,19 @@ std::vector> Meter::RegisterAsyncMetricSt { view_instr_desc.description_ = view.GetDescription(); } - auto storage = std::shared_ptr(new AsyncMetricStorage( + auto storage = std::shared_ptr>(new AsyncMetricStorage( view_instr_desc, view.GetAggregationType(), &view.GetAttributesProcessor())); storage_registry_[instrument_descriptor.name_] = storage; + static_cast(storages.get())->AddStorage(storage); return true; }); } -# endif /** collect metrics across all the meters **/ std::vector Meter::Collect(CollectorHandle *collector, opentelemetry::common::SystemTimestamp collect_ts) noexcept { + std::vector metric_data_list; for (auto &metric_storage : storage_registry_) { diff --git a/sdk/src/metrics/state/observable_registry.cc b/sdk/src/metrics/state/observable_registry.cc index 7aa5ff7b95..6998705d16 100644 --- a/sdk/src/metrics/state/observable_registry.cc +++ b/sdk/src/metrics/state/observable_registry.cc @@ -6,6 +6,7 @@ # include "opentelemetry/sdk/metrics/state/observable_registry.h" # include "opentelemetry/sdk/metrics/async_instruments.h" # include "opentelemetry/sdk/metrics/observer_result.h" +# include "opentelemetry/sdk/metrics/state/metric_storage.h" OPENTELEMETRY_BEGIN_NAMESPACE namespace sdk @@ -43,20 +44,28 @@ void ObservableRegistry::Observe(opentelemetry::common::SystemTimestamp collecti std::unique_lock lk(callbacks_m_); for (auto &callback_wrap : callbacks_) { - if (static_cast(callback_wrap->instrument) + auto value_type = + static_cast(callback_wrap->instrument) ->GetInstrumentDescriptor() - .value_type_ == InstrumentValueType::kDouble) + .value_type_; + auto storage = + static_cast(callback_wrap->instrument) + ->GetMetricStorage(); + if (value_type == InstrumentValueType::kDouble) { nostd::shared_ptr> ob_res( new opentelemetry::sdk::metrics::ObserverResultT()); callback_wrap->callback(ob_res, callback_wrap->state); + storage->RecordDouble(static_cast *>(ob_res.get())->GetMeasurements() , collection_ts); } else { nostd::shared_ptr> ob_res( new opentelemetry::sdk::metrics::ObserverResultT()); callback_wrap->callback(ob_res, callback_wrap->state); + storage->RecordLong(static_cast *>(ob_res.get())->GetMeasurements() , collection_ts); } + // record observerd measurements to all configured metric storage } } diff --git a/sdk/src/metrics/sync_instruments.cc b/sdk/src/metrics/sync_instruments.cc index 7cf9034cdc..61e30248c3 100644 --- a/sdk/src/metrics/sync_instruments.cc +++ b/sdk/src/metrics/sync_instruments.cc @@ -14,7 +14,7 @@ namespace sdk namespace metrics { LongCounter::LongCounter(InstrumentDescriptor instrument_descriptor, - std::unique_ptr storage) + std::unique_ptr storage) : Synchronous(instrument_descriptor, std::move(storage)) {} @@ -44,7 +44,7 @@ void LongCounter::Add(long value, const opentelemetry::context::Context &context } DoubleCounter::DoubleCounter(InstrumentDescriptor instrument_descriptor, - std::unique_ptr storage) + std::unique_ptr storage) : Synchronous(instrument_descriptor, std::move(storage)) {} @@ -74,7 +74,7 @@ void DoubleCounter::Add(double value, const opentelemetry::context::Context &con } LongUpDownCounter::LongUpDownCounter(InstrumentDescriptor instrument_descriptor, - std::unique_ptr storage) + std::unique_ptr storage) : Synchronous(instrument_descriptor, std::move(storage)) {} @@ -104,7 +104,7 @@ void LongUpDownCounter::Add(long value, const opentelemetry::context::Context &c } DoubleUpDownCounter::DoubleUpDownCounter(InstrumentDescriptor instrument_descriptor, - std::unique_ptr storage) + std::unique_ptr storage) : Synchronous(instrument_descriptor, std::move(storage)) {} @@ -134,7 +134,7 @@ void DoubleUpDownCounter::Add(double value, const opentelemetry::context::Contex } LongHistogram::LongHistogram(InstrumentDescriptor instrument_descriptor, - std::unique_ptr storage) + std::unique_ptr storage) : Synchronous(instrument_descriptor, std::move(storage)) {} @@ -165,7 +165,7 @@ void LongHistogram::Record(long value, const opentelemetry::context::Context &co } DoubleHistogram::DoubleHistogram(InstrumentDescriptor instrument_descriptor, - std::unique_ptr storage) + std::unique_ptr storage) : Synchronous(instrument_descriptor, std::move(storage)) {} From 497a34eb5398d68403ce7e93436d618309da19ae Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Thu, 21 Jul 2022 22:51:19 -0700 Subject: [PATCH 15/26] fix --- sdk/include/opentelemetry/sdk/metrics/meter.h | 3 ++- .../sdk/metrics/state/async_metric_storage.h | 2 +- .../sdk/metrics/state/metric_storage.h | 5 +++-- .../sdk/metrics/state/multi_metric_storage.h | 10 +++++++--- sdk/src/metrics/meter.cc | 19 ++++++++++--------- sdk/src/metrics/state/observable_registry.cc | 10 ++++++++-- sdk/test/metrics/async_instruments_test.cc | 2 +- sdk/test/metrics/multi_metric_storage_test.cc | 6 +++--- sdk/test/metrics/sync_instruments_test.cc | 12 ++++++------ 9 files changed, 41 insertions(+), 28 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/metrics/meter.h b/sdk/include/opentelemetry/sdk/metrics/meter.h index a378480508..a7355dff1f 100644 --- a/sdk/include/opentelemetry/sdk/metrics/meter.h +++ b/sdk/include/opentelemetry/sdk/metrics/meter.h @@ -112,7 +112,8 @@ class Meter final : public opentelemetry::metrics::Meter std::unique_ptr RegisterSyncMetricStorage( InstrumentDescriptor &instrument_descriptor); - std::unique_ptr RegisterAsyncMetricStorage(InstrumentDescriptor &instrument_descriptor); + std::unique_ptr RegisterAsyncMetricStorage( + InstrumentDescriptor &instrument_descriptor); }; } // namespace metrics } // namespace sdk diff --git a/sdk/include/opentelemetry/sdk/metrics/state/async_metric_storage.h b/sdk/include/opentelemetry/sdk/metrics/state/async_metric_storage.h index 4839fa7715..56c7ee90ed 100644 --- a/sdk/include/opentelemetry/sdk/metrics/state/async_metric_storage.h +++ b/sdk/include/opentelemetry/sdk/metrics/state/async_metric_storage.h @@ -56,7 +56,7 @@ class AsyncMetricStorage : public MetricStorage, public AsyncWritableMetricStora opentelemetry::common::SystemTimestamp collection_ts, nostd::function_ref metric_collection_callback) noexcept override { - nostd::shared_ptr> ob_res( + nostd::shared_ptr> ob_res( new opentelemetry::sdk::metrics::ObserverResultT(nullptr)); // read the measurement using configured callback diff --git a/sdk/include/opentelemetry/sdk/metrics/state/metric_storage.h b/sdk/include/opentelemetry/sdk/metrics/state/metric_storage.h index 3676a0f707..18ee235158 100644 --- a/sdk/include/opentelemetry/sdk/metrics/state/metric_storage.h +++ b/sdk/include/opentelemetry/sdk/metrics/state/metric_storage.h @@ -100,8 +100,9 @@ class NoopWritableMetricStorage : public SyncWritableMetricStorage class NoopAsyncWritableMetricStorage : public AsyncWritableMetricStorage { public: - void RecordLong(const std::unordered_map &measurements, - opentelemetry::common::SystemTimestamp observation_time) noexcept override + void RecordLong( + const std::unordered_map &measurements, + opentelemetry::common::SystemTimestamp observation_time) noexcept override {} void RecordDouble( diff --git a/sdk/include/opentelemetry/sdk/metrics/state/multi_metric_storage.h b/sdk/include/opentelemetry/sdk/metrics/state/multi_metric_storage.h index fbdcbf8407..b3cae8694c 100644 --- a/sdk/include/opentelemetry/sdk/metrics/state/multi_metric_storage.h +++ b/sdk/include/opentelemetry/sdk/metrics/state/multi_metric_storage.h @@ -18,7 +18,10 @@ namespace metrics class SyncMultiMetricStorage : public SyncWritableMetricStorage { public: - void AddStorage(std::shared_ptr storage) { storages_.push_back(storage); } + void AddStorage(std::shared_ptr storage) + { + storages_.push_back(storage); + } virtual void RecordLong(long value, const opentelemetry::context::Context &context) noexcept override @@ -70,8 +73,9 @@ class AsyncMultiMetricStorage : public AsyncWritableMetricStorage storages_.push_back(storage); } - void RecordLong(const std::unordered_map &measurements, - opentelemetry::common::SystemTimestamp observation_time) noexcept override + void RecordLong( + const std::unordered_map &measurements, + opentelemetry::common::SystemTimestamp observation_time) noexcept override { for (auto &s : storages_) { diff --git a/sdk/src/metrics/meter.cc b/sdk/src/metrics/meter.cc index cac0390499..cf6b62726f 100644 --- a/sdk/src/metrics/meter.cc +++ b/sdk/src/metrics/meter.cc @@ -210,13 +210,13 @@ std::unique_ptr Meter::RegisterSyncMetricStorage( view_instr_desc.description_ = view.GetDescription(); } auto multi_storage = static_cast(storages.get()); - //if (GetInstrumentClass(instrument_descriptor.type_) == InstrumentClass::kSync) - //{ - auto storage = std::shared_ptr(new SyncMetricStorage( - view_instr_desc, view.GetAggregationType(), &view.GetAttributesProcessor(), - NoExemplarReservoir::GetNoExemplarReservoir())); - storage_registry_[instrument_descriptor.name_] = storage; - multi_storage->AddStorage(storage); + // if (GetInstrumentClass(instrument_descriptor.type_) == InstrumentClass::kSync) + //{ + auto storage = std::shared_ptr(new SyncMetricStorage( + view_instr_desc, view.GetAggregationType(), &view.GetAttributesProcessor(), + NoExemplarReservoir::GetNoExemplarReservoir())); + storage_registry_[instrument_descriptor.name_] = storage; + multi_storage->AddStorage(storage); //} /*else { @@ -248,11 +248,12 @@ std::unique_ptr Meter::RegisterSyncMetricStorage( return storages; } -std::unique_ptr Meter::RegisterAsyncMetricStorage(InstrumentDescriptor &instrument_descriptor) +std::unique_ptr Meter::RegisterAsyncMetricStorage( + InstrumentDescriptor &instrument_descriptor) { auto view_registry = meter_context_->GetViewRegistry(); std::unique_ptr storages(new AsyncMultiMetricStorage()); - auto success = view_registry->FindViews( + auto success = view_registry->FindViews( instrument_descriptor, *instrumentation_library_, [this, &instrument_descriptor, &storages](const View &view) { auto view_instr_desc = instrument_descriptor; diff --git a/sdk/src/metrics/state/observable_registry.cc b/sdk/src/metrics/state/observable_registry.cc index 6998705d16..d0adcbefff 100644 --- a/sdk/src/metrics/state/observable_registry.cc +++ b/sdk/src/metrics/state/observable_registry.cc @@ -56,14 +56,20 @@ void ObservableRegistry::Observe(opentelemetry::common::SystemTimestamp collecti nostd::shared_ptr> ob_res( new opentelemetry::sdk::metrics::ObserverResultT()); callback_wrap->callback(ob_res, callback_wrap->state); - storage->RecordDouble(static_cast *>(ob_res.get())->GetMeasurements() , collection_ts); + storage->RecordDouble( + static_cast *>(ob_res.get()) + ->GetMeasurements(), + collection_ts); } else { nostd::shared_ptr> ob_res( new opentelemetry::sdk::metrics::ObserverResultT()); callback_wrap->callback(ob_res, callback_wrap->state); - storage->RecordLong(static_cast *>(ob_res.get())->GetMeasurements() , collection_ts); + storage->RecordLong( + static_cast *>(ob_res.get()) + ->GetMeasurements(), + collection_ts); } // record observerd measurements to all configured metric storage } diff --git a/sdk/test/metrics/async_instruments_test.cc b/sdk/test/metrics/async_instruments_test.cc index 4190883a57..5ac6caeb9a 100644 --- a/sdk/test/metrics/async_instruments_test.cc +++ b/sdk/test/metrics/async_instruments_test.cc @@ -19,7 +19,7 @@ TEST(AsyncInstruments, ObservableInstrument) InstrumentDescriptor instrument_descriptor = {"long_counter", "description", "1", InstrumentType::kObservableCounter, InstrumentValueType::kLong}; - std::unique_ptr metric_storage(new MultiMetricStorage()); + std::unique_ptr metric_storage(new AsyncMultiMetricStorage()); auto asyc_generate_meas_long = [](opentelemetry::metrics::ObserverResultT &observer) {}; ObservableInstrument observable_counter_long(instrument_descriptor, std::move(metric_storage)); observable_counter_long.AddCallback(asyc_generate_measurements, nullptr); diff --git a/sdk/test/metrics/multi_metric_storage_test.cc b/sdk/test/metrics/multi_metric_storage_test.cc index d88946485c..537af8d03c 100644 --- a/sdk/test/metrics/multi_metric_storage_test.cc +++ b/sdk/test/metrics/multi_metric_storage_test.cc @@ -13,7 +13,7 @@ using namespace opentelemetry; using namespace opentelemetry::sdk::instrumentationlibrary; using namespace opentelemetry::sdk::metrics; -class TestMetricStorage : public WritableMetricStorage +class TestMetricStorage : public SyncWritableMetricStorage { public: void RecordLong(long value, const opentelemetry::context::Context &context) noexcept override @@ -46,9 +46,9 @@ class TestMetricStorage : public WritableMetricStorage TEST(MultiMetricStorageTest, BasicTests) { - std::shared_ptr storage( + std::shared_ptr storage( new TestMetricStorage()); - MultiMetricStorage storages{}; + SyncMultiMetricStorage storages{}; storages.AddStorage(storage); EXPECT_NO_THROW(storages.RecordLong(10l, opentelemetry::context::Context{})); EXPECT_NO_THROW(storages.RecordLong(20l, opentelemetry::context::Context{})); diff --git a/sdk/test/metrics/sync_instruments_test.cc b/sdk/test/metrics/sync_instruments_test.cc index 1a590d8d26..7802361938 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 MultiMetricStorage()); + std::unique_ptr metric_storage(new SyncMultiMetricStorage()); LongCounter counter(instrument_descriptor, std::move(metric_storage)); EXPECT_NO_THROW(counter.Add(10l)); EXPECT_NO_THROW(counter.Add(10l, opentelemetry::context::Context{})); @@ -43,7 +43,7 @@ TEST(SyncInstruments, DoubleCounter) { InstrumentDescriptor instrument_descriptor = { "double_counter", "description", "1", InstrumentType::kCounter, InstrumentValueType::kDouble}; - std::unique_ptr metric_storage(new MultiMetricStorage()); + std::unique_ptr metric_storage(new SyncMultiMetricStorage()); DoubleCounter counter(instrument_descriptor, std::move(metric_storage)); EXPECT_NO_THROW(counter.Add(10.10)); EXPECT_NO_THROW(counter.Add(10.10, opentelemetry::context::Context{})); @@ -63,7 +63,7 @@ TEST(SyncInstruments, LongUpDownCounter) InstrumentDescriptor instrument_descriptor = {"long_updowncounter", "description", "1", InstrumentType::kUpDownCounter, InstrumentValueType::kLong}; - std::unique_ptr metric_storage(new MultiMetricStorage()); + std::unique_ptr metric_storage(new SyncMultiMetricStorage()); LongUpDownCounter counter(instrument_descriptor, std::move(metric_storage)); EXPECT_NO_THROW(counter.Add(10l)); EXPECT_NO_THROW(counter.Add(10l, opentelemetry::context::Context{})); @@ -83,7 +83,7 @@ TEST(SyncInstruments, DoubleUpDownCounter) InstrumentDescriptor instrument_descriptor = {"double_updowncounter", "description", "1", InstrumentType::kUpDownCounter, InstrumentValueType::kDouble}; - std::unique_ptr metric_storage(new MultiMetricStorage()); + std::unique_ptr metric_storage(new SyncMultiMetricStorage()); DoubleUpDownCounter counter(instrument_descriptor, std::move(metric_storage)); EXPECT_NO_THROW(counter.Add(10.10)); EXPECT_NO_THROW(counter.Add(10.10, opentelemetry::context::Context{})); @@ -102,7 +102,7 @@ TEST(SyncInstruments, LongHistogram) { InstrumentDescriptor instrument_descriptor = { "long_histogram", "description", "1", InstrumentType::kHistogram, InstrumentValueType::kLong}; - std::unique_ptr metric_storage(new MultiMetricStorage()); + std::unique_ptr metric_storage(new SyncMultiMetricStorage()); LongHistogram counter(instrument_descriptor, std::move(metric_storage)); EXPECT_NO_THROW(counter.Record(10l, opentelemetry::context::Context{})); EXPECT_NO_THROW(counter.Record(-10l, opentelemetry::context::Context{})); // This is ignored @@ -119,7 +119,7 @@ TEST(SyncInstruments, DoubleHistogram) InstrumentDescriptor instrument_descriptor = {"double_histogram", "description", "1", InstrumentType::kHistogram, InstrumentValueType::kDouble}; - std::unique_ptr metric_storage(new MultiMetricStorage()); + std::unique_ptr metric_storage(new SyncMultiMetricStorage()); DoubleHistogram counter(instrument_descriptor, std::move(metric_storage)); EXPECT_NO_THROW(counter.Record(10.10, opentelemetry::context::Context{})); EXPECT_NO_THROW(counter.Record(-10.10, opentelemetry::context::Context{})); // This is ignored. From 659aa61c134512b96ccf07d233c57660e862cf34 Mon Sep 17 00:00:00 2001 From: Lalit Date: Fri, 22 Jul 2022 21:39:24 -0700 Subject: [PATCH 16/26] changes --- .../sdk/metrics/state/async_metric_storage.h | 14 +++++- sdk/src/metrics/meter.cc | 47 ++++++++----------- 2 files changed, 32 insertions(+), 29 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/metrics/state/async_metric_storage.h b/sdk/include/opentelemetry/sdk/metrics/state/async_metric_storage.h index 56c7ee90ed..b4fd5d0361 100644 --- a/sdk/include/opentelemetry/sdk/metrics/state/async_metric_storage.h +++ b/sdk/include/opentelemetry/sdk/metrics/state/async_metric_storage.h @@ -43,12 +43,22 @@ class AsyncMetricStorage : public MetricStorage, public AsyncWritableMetricStora virtual void RecordLong( const std::unordered_map &measurements, opentelemetry::common::SystemTimestamp observation_time) noexcept override - {} + { + if (instrument_descriptor_.value_type_ != InstrumentValueType::kLong) + { + return; + } + } virtual void RecordDouble( const std::unordered_map &measurements, opentelemetry::common::SystemTimestamp observation_time) noexcept override - {} + { + if (instrument_descriptor_.value_type_ != InstrumentValueType::kDouble) + { + return; + } + } bool Collect(CollectorHandle *collector, nostd::span> collectors, diff --git a/sdk/src/metrics/meter.cc b/sdk/src/metrics/meter.cc index cf6b62726f..e365271172 100644 --- a/sdk/src/metrics/meter.cc +++ b/sdk/src/metrics/meter.cc @@ -66,7 +66,9 @@ nostd::shared_ptr Meter::CreateLon std::string{name.data(), name.size()}, std::string{description.data(), description.size()}, std::string{unit.data(), unit.size()}, InstrumentType::kObservableCounter, InstrumentValueType::kLong}; - RegisterAsyncMetricStorage(instrument_descriptor); + auto storage = RegisterAsyncMetricStorage(instrument_descriptor); + return nostd::shared_ptr{ + new ObservableInstrument(instrument_descriptor, std::move(storage))}; } nostd::shared_ptr @@ -78,7 +80,9 @@ Meter::CreateDoubleObservableCounter(nostd::string_view name, std::string{name.data(), name.size()}, std::string{description.data(), description.size()}, std::string{unit.data(), unit.size()}, InstrumentType::kObservableCounter, InstrumentValueType::kDouble}; - RegisterAsyncMetricStorage(instrument_descriptor); + auto storage = RegisterAsyncMetricStorage(instrument_descriptor); + return nostd::shared_ptr{ + new ObservableInstrument(instrument_descriptor, std::move(storage))}; } nostd::shared_ptr> Meter::CreateLongHistogram( @@ -118,7 +122,9 @@ nostd::shared_ptr Meter::CreateLon std::string{name.data(), name.size()}, std::string{description.data(), description.size()}, std::string{unit.data(), unit.size()}, InstrumentType::kObservableGauge, InstrumentValueType::kLong}; - RegisterAsyncMetricStorage(instrument_descriptor); + auto storage = RegisterAsyncMetricStorage(instrument_descriptor); + return nostd::shared_ptr{ + new ObservableInstrument(instrument_descriptor, std::move(storage))}; } nostd::shared_ptr Meter::CreateDoubleObservableGauge( @@ -130,7 +136,9 @@ nostd::shared_ptr Meter::CreateDou std::string{name.data(), name.size()}, std::string{description.data(), description.size()}, std::string{unit.data(), unit.size()}, InstrumentType::kObservableGauge, InstrumentValueType::kDouble}; - RegisterAsyncMetricStorage(instrument_descriptor); + auto storage = RegisterAsyncMetricStorage(instrument_descriptor); + return nostd::shared_ptr{ + new ObservableInstrument(instrument_descriptor, std::move(storage))}; } nostd::shared_ptr> Meter::CreateLongUpDownCounter( @@ -170,7 +178,9 @@ Meter::CreateLongObservableUpDownCounter(nostd::string_view name, std::string{name.data(), name.size()}, std::string{description.data(), description.size()}, std::string{unit.data(), unit.size()}, InstrumentType::kObservableUpDownCounter, InstrumentValueType::kLong}; - RegisterAsyncMetricStorage(instrument_descriptor); + auto storage = RegisterAsyncMetricStorage(instrument_descriptor); + return nostd::shared_ptr{ + new ObservableInstrument(instrument_descriptor, std::move(storage))}; } nostd::shared_ptr @@ -182,7 +192,9 @@ Meter::CreateDoubleObservableUpDownCounter(nostd::string_view name, std::string{name.data(), name.size()}, std::string{description.data(), description.size()}, std::string{unit.data(), unit.size()}, InstrumentType::kObservableUpDownCounter, InstrumentValueType::kDouble}; - RegisterAsyncMetricStorage(instrument_descriptor); + auto storage = RegisterAsyncMetricStorage(instrument_descriptor); + return nostd::shared_ptr{ + new ObservableInstrument(instrument_descriptor, std::move(storage))}; } const sdk::instrumentationlibrary::InstrumentationLibrary *Meter::GetInstrumentationLibrary() @@ -210,32 +222,12 @@ std::unique_ptr Meter::RegisterSyncMetricStorage( view_instr_desc.description_ = view.GetDescription(); } auto multi_storage = static_cast(storages.get()); - // if (GetInstrumentClass(instrument_descriptor.type_) == InstrumentClass::kSync) - //{ + auto storage = std::shared_ptr(new SyncMetricStorage( view_instr_desc, view.GetAggregationType(), &view.GetAttributesProcessor(), NoExemplarReservoir::GetNoExemplarReservoir())); storage_registry_[instrument_descriptor.name_] = storage; multi_storage->AddStorage(storage); - //} - /*else - { - if (instrument_descriptor.value_type_ == InstrumentValueType::kDouble) - { - auto storage = - std::shared_ptr>(new AsyncMetricStorage( - view_instr_desc, view.GetAggregationType(), &view.GetAttributesProcessor())); - storage_registry_[instrument_descriptor.name_] = storage; - multi_storage->AddStorage(storage); - } - else - { - auto storage = std::shared_ptr>(new AsyncMetricStorage( - view_instr_desc, view.GetAggregationType(), &view.GetAttributesProcessor())); - storage_registry_[instrument_descriptor.name_] = storage; - multi_storage->AddStorage(storage); - }*/ - //} return true; }); @@ -271,6 +263,7 @@ std::unique_ptr Meter::RegisterAsyncMetricStorage( static_cast(storages.get())->AddStorage(storage); return true; }); + return storages; } /** collect metrics across all the meters **/ From 8d543350c7f3be1b1b26a35c4a49c61c480c2b91 Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Sun, 24 Jul 2022 21:53:26 -0700 Subject: [PATCH 17/26] changes --- .../sdk/metrics/state/async_metric_storage.h | 79 ++++++++++--------- sdk/src/metrics/meter.cc | 2 +- sdk/test/metrics/async_metric_storage_test.cc | 6 +- 3 files changed, 44 insertions(+), 43 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/metrics/state/async_metric_storage.h b/sdk/include/opentelemetry/sdk/metrics/state/async_metric_storage.h index b4fd5d0361..f0b32057ae 100644 --- a/sdk/include/opentelemetry/sdk/metrics/state/async_metric_storage.h +++ b/sdk/include/opentelemetry/sdk/metrics/state/async_metric_storage.h @@ -21,26 +21,52 @@ namespace sdk namespace metrics { -template class AsyncMetricStorage : public MetricStorage, public AsyncWritableMetricStorage { public: - AsyncMetricStorage( - InstrumentDescriptor instrument_descriptor, - const AggregationType aggregation_type, - // void (*measurement_callback)(opentelemetry::metrics::ObserverResult &, void *), - const AttributesProcessor *attributes_processor, - void *state = nullptr) + AsyncMetricStorage(InstrumentDescriptor instrument_descriptor, + const AggregationType aggregation_type, + const AttributesProcessor *attributes_processor, + void *state = nullptr) : instrument_descriptor_(instrument_descriptor), aggregation_type_{aggregation_type}, - // measurement_collection_callback_{measurement_callback}, attributes_processor_{attributes_processor}, state_{state}, + delta_hash_map_(new AttributesHashMap()), cumulative_hash_map_(new AttributesHashMap()), temporal_metric_storage_(instrument_descriptor) {} - virtual void RecordLong( + template + void Record(const std::unordered_map &measurements, + opentelemetry::common::SystemTimestamp observation_time) noexcept + { + // std::shared_ptr delta_hash_map(new AttributesHashMap()); + // process the read measurements - aggregate and store in hashmap + for (auto &measurement : measurements) + { + auto aggr = DefaultAggregation::CreateAggregation(aggregation_type_, instrument_descriptor_); + aggr->Aggregate(measurement.second); + auto prev = cumulative_hash_map_->Get(measurement.first); + if (prev) + { + auto delta = prev->Diff(*aggr); + cumulative_hash_map_->Set(measurement.first, + DefaultAggregation::CloneAggregation( + aggregation_type_, instrument_descriptor_, *delta)); + delta_hash_map_->Set(measurement.first, std::move(delta)); + } + else + { + cumulative_hash_map_->Set( + measurement.first, + DefaultAggregation::CloneAggregation(aggregation_type_, instrument_descriptor_, *aggr)); + delta_hash_map_->Set(measurement.first, std::move(aggr)); + } + } + } + + void RecordLong( const std::unordered_map &measurements, opentelemetry::common::SystemTimestamp observation_time) noexcept override { @@ -48,9 +74,10 @@ class AsyncMetricStorage : public MetricStorage, public AsyncWritableMetricStora { return; } + Record(measurements, observation_time); } - virtual void RecordDouble( + void RecordDouble( const std::unordered_map &measurements, opentelemetry::common::SystemTimestamp observation_time) noexcept override { @@ -58,6 +85,7 @@ class AsyncMetricStorage : public MetricStorage, public AsyncWritableMetricStora { return; } + Record(measurements, observation_time); } bool Collect(CollectorHandle *collector, @@ -66,37 +94,9 @@ class AsyncMetricStorage : public MetricStorage, public AsyncWritableMetricStora opentelemetry::common::SystemTimestamp collection_ts, nostd::function_ref metric_collection_callback) noexcept override { - nostd::shared_ptr> ob_res( - new opentelemetry::sdk::metrics::ObserverResultT(nullptr)); - - // read the measurement using configured callback - // measurement_collection_callback_(ob_res, state_); - std::shared_ptr delta_hash_map(new AttributesHashMap()); - // process the read measurements - aggregate and store in hashmap - for (auto &measurement : ob_res->GetMeasurements()) - { - auto aggr = DefaultAggregation::CreateAggregation(aggregation_type_, instrument_descriptor_); - aggr->Aggregate(measurement.second); - auto prev = cumulative_hash_map_->Get(measurement.first); - if (prev) - { - auto delta = prev->Diff(*aggr); - cumulative_hash_map_->Set(measurement.first, - DefaultAggregation::CloneAggregation( - aggregation_type_, instrument_descriptor_, *delta)); - delta_hash_map->Set(measurement.first, std::move(delta)); - } - else - { - cumulative_hash_map_->Set( - measurement.first, - DefaultAggregation::CloneAggregation(aggregation_type_, instrument_descriptor_, *aggr)); - delta_hash_map->Set(measurement.first, std::move(aggr)); - } - } return temporal_metric_storage_.buildMetrics(collector, collectors, sdk_start_ts, collection_ts, - std::move(delta_hash_map), + std::move(delta_hash_map_), metric_collection_callback); } @@ -107,6 +107,7 @@ class AsyncMetricStorage : public MetricStorage, public AsyncWritableMetricStora const AttributesProcessor *attributes_processor_; void *state_; std::unique_ptr cumulative_hash_map_; + std::unique_ptr delta_hash_map_; TemporalMetricStorage temporal_metric_storage_; }; diff --git a/sdk/src/metrics/meter.cc b/sdk/src/metrics/meter.cc index e365271172..1d57fffcb8 100644 --- a/sdk/src/metrics/meter.cc +++ b/sdk/src/metrics/meter.cc @@ -257,7 +257,7 @@ std::unique_ptr Meter::RegisterAsyncMetricStorage( { view_instr_desc.description_ = view.GetDescription(); } - auto storage = std::shared_ptr>(new AsyncMetricStorage( + auto storage = std::shared_ptr(new AsyncMetricStorage( view_instr_desc, view.GetAggregationType(), &view.GetAttributesProcessor())); storage_registry_[instrument_descriptor.name_] = storage; static_cast(storages.get())->AddStorage(storage); diff --git a/sdk/test/metrics/async_metric_storage_test.cc b/sdk/test/metrics/async_metric_storage_test.cc index c7a9669592..eaa8b17eb5 100644 --- a/sdk/test/metrics/async_metric_storage_test.cc +++ b/sdk/test/metrics/async_metric_storage_test.cc @@ -96,9 +96,9 @@ TEST_P(WritableMetricStorageTestFixture, TestAggregation) long value = 0; MeasurementFetcher measurement_fetcher; - opentelemetry::sdk::metrics::AsyncMetricStorage storage(instr_desc, AggregationType::kSum, - // MeasurementFetcher::Fetcher, - new DefaultAttributesProcessor()); + opentelemetry::sdk::metrics::AsyncMetricStorage storage(instr_desc, AggregationType::kSum, + // MeasurementFetcher::Fetcher, + new DefaultAttributesProcessor()); storage.Collect(collector.get(), collectors, sdk_start_ts, collection_ts, [&](const MetricData data) { From 12b9539e1636fdb8caad3514b7196517c36ace46 Mon Sep 17 00:00:00 2001 From: Lalit Date: Sun, 24 Jul 2022 23:02:16 -0700 Subject: [PATCH 18/26] changes --- sdk/test/metrics/async_metric_storage_test.cc | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/sdk/test/metrics/async_metric_storage_test.cc b/sdk/test/metrics/async_metric_storage_test.cc index eaa8b17eb5..75811cc3a2 100644 --- a/sdk/test/metrics/async_metric_storage_test.cc +++ b/sdk/test/metrics/async_metric_storage_test.cc @@ -4,12 +4,15 @@ #ifndef ENABLE_METRICS_PREVIEW # include "opentelemetry/sdk/metrics/state/async_metric_storage.h" # include "opentelemetry/common/key_value_iterable_view.h" +//#include "opentelemetry/nostd/shared_ptr.h" +# include "opentelemetry/sdk/metrics/async_instruments.h" # include "opentelemetry/sdk/metrics/instruments.h" # include "opentelemetry/sdk/metrics/meter_context.h" # include "opentelemetry/sdk/metrics/metric_exporter.h" # include "opentelemetry/sdk/metrics/metric_reader.h" # include "opentelemetry/sdk/metrics/observer_result.h" # include "opentelemetry/sdk/metrics/state/metric_collector.h" +# include "opentelemetry/sdk/metrics/state/observable_registry.h" # include # include @@ -39,21 +42,20 @@ class WritableMetricStorageTestFixture : public ::testing::TestWithParam &observer_result, - void * /*state*/) + static void Fetcher(opentelemetry::metrics::ObserverResult observer_result, void * /*state*/) { fetch_count++; if (fetch_count == 1) { - observer_result.Observe(20l, {{"RequestType", "GET"}}); - observer_result.Observe(10l, {{"RequestType", "PUT"}}); + opentelemetry::nostd::get<0>(observer_result)->Observe(20l, {{"RequestType", "GET"}}); + opentelemetry::nostd::get<0>(observer_result)->Observe(10l, {{"RequestType", "PUT"}}); number_of_get += 20l; number_of_put += 10l; } else if (fetch_count == 2) { - observer_result.Observe(40l, {{"RequestType", "GET"}}); - observer_result.Observe(20l, {{"RequestType", "PUT"}}); + opentelemetry::nostd::get<0>(observer_result)->Observe(40l, {{"RequestType", "GET"}}); + opentelemetry::nostd::get<0>(observer_result)->Observe(20l, {{"RequestType", "PUT"}}); number_of_get += 40l; number_of_put += 20l; } @@ -81,6 +83,8 @@ TEST_P(WritableMetricStorageTestFixture, TestAggregation) { MeasurementFetcher::init_values(); AggregationTemporality temporality = GetParam(); + ObservableRegistry registry; + registry.AddCallback(MeasurementFetcher::Fetcher, nullptr, nullptr); InstrumentDescriptor instr_desc = {"name", "desc", "1unit", InstrumentType::kObservableCounter, InstrumentValueType::kLong}; From 72a5d020beb9ff7f571f8586f9967f9d6f9d17ae Mon Sep 17 00:00:00 2001 From: Lalit Date: Mon, 25 Jul 2022 00:00:40 -0700 Subject: [PATCH 19/26] change --- sdk/test/metrics/async_instruments_test.cc | 4 ++-- sdk/test/metrics/async_metric_storage_test.cc | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/sdk/test/metrics/async_instruments_test.cc b/sdk/test/metrics/async_instruments_test.cc index 5ac6caeb9a..92745fb174 100644 --- a/sdk/test/metrics/async_instruments_test.cc +++ b/sdk/test/metrics/async_instruments_test.cc @@ -21,8 +21,8 @@ TEST(AsyncInstruments, ObservableInstrument) InstrumentValueType::kLong}; std::unique_ptr metric_storage(new AsyncMultiMetricStorage()); auto asyc_generate_meas_long = [](opentelemetry::metrics::ObserverResultT &observer) {}; - ObservableInstrument observable_counter_long(instrument_descriptor, std::move(metric_storage)); - observable_counter_long.AddCallback(asyc_generate_measurements, nullptr); + // ObservableInstrument observable_counter_long(instrument_descriptor, std::move(metric_storage)); + // observable_counter_long.AddCallback(asyc_generate_measurements, nullptr); } #endif diff --git a/sdk/test/metrics/async_metric_storage_test.cc b/sdk/test/metrics/async_metric_storage_test.cc index 75811cc3a2..e418b4d90a 100644 --- a/sdk/test/metrics/async_metric_storage_test.cc +++ b/sdk/test/metrics/async_metric_storage_test.cc @@ -71,7 +71,7 @@ class MeasurementFetcher static size_t fetch_count; static long number_of_get; static long number_of_put; - static const size_t number_of_attributes = 2; // GET , PUT + static const size_t number_of_attributes = 0; // GET , PUT fixme - should be 2 }; size_t MeasurementFetcher::fetch_count; From 959eb042202976847ff3c4016439648c06a8848e Mon Sep 17 00:00:00 2001 From: Lalit Date: Mon, 25 Jul 2022 09:15:30 -0700 Subject: [PATCH 20/26] spell --- sdk/src/metrics/state/observable_registry.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/sdk/src/metrics/state/observable_registry.cc b/sdk/src/metrics/state/observable_registry.cc index d0adcbefff..5c3451975c 100644 --- a/sdk/src/metrics/state/observable_registry.cc +++ b/sdk/src/metrics/state/observable_registry.cc @@ -71,7 +71,6 @@ void ObservableRegistry::Observe(opentelemetry::common::SystemTimestamp collecti ->GetMeasurements(), collection_ts); } - // record observerd measurements to all configured metric storage } } From de5b32efc38dd5d1ce12d34bd77abe43a86dc647 Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Tue, 26 Jul 2022 17:01:14 -0700 Subject: [PATCH 21/26] fix --- .../sdk/metrics/async_instruments.h | 18 +----------------- sdk/src/metrics/async_instruments.cc | 5 +---- sdk/test/metrics/async_instruments_test.cc | 4 ++-- 3 files changed, 4 insertions(+), 23 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/metrics/async_instruments.h b/sdk/include/opentelemetry/sdk/metrics/async_instruments.h index 4cff7032cf..f14d8f3a27 100644 --- a/sdk/include/opentelemetry/sdk/metrics/async_instruments.h +++ b/sdk/include/opentelemetry/sdk/metrics/async_instruments.h @@ -17,23 +17,7 @@ namespace metrics class AsyncWritableMetricStorage; -class Asynchronous -{ -public: - Asynchronous(nostd::string_view name, - nostd::string_view description = "", - nostd::string_view unit = "") - : name_(name), description_(description), unit_(unit) - {} - -protected: - std::string name_; - std::string description_; - std::string unit_; -}; - -class ObservableInstrument : public opentelemetry::metrics::ObservableInstrument, - public Asynchronous +class ObservableInstrument : public opentelemetry::metrics::ObservableInstrument { public: ObservableInstrument(InstrumentDescriptor instrument_descriptor, diff --git a/sdk/src/metrics/async_instruments.cc b/sdk/src/metrics/async_instruments.cc index 9c5a9750bc..e41cb304a8 100644 --- a/sdk/src/metrics/async_instruments.cc +++ b/sdk/src/metrics/async_instruments.cc @@ -18,10 +18,7 @@ ObservableInstrument::ObservableInstrument(InstrumentDescriptor instrument_descr std::unique_ptr storage) : instrument_descriptor_(instrument_descriptor), storage_(std::move(storage)), - observable_registry_{new ObservableRegistry()}, - Asynchronous(instrument_descriptor_.name_, - instrument_descriptor.description_, - instrument_descriptor_.unit_) + observable_registry_{new ObservableRegistry()} {} void ObservableInstrument::AddCallback(opentelemetry::metrics::ObservableCallbackPtr callback, diff --git a/sdk/test/metrics/async_instruments_test.cc b/sdk/test/metrics/async_instruments_test.cc index 92745fb174..5ac6caeb9a 100644 --- a/sdk/test/metrics/async_instruments_test.cc +++ b/sdk/test/metrics/async_instruments_test.cc @@ -21,8 +21,8 @@ TEST(AsyncInstruments, ObservableInstrument) InstrumentValueType::kLong}; std::unique_ptr metric_storage(new AsyncMultiMetricStorage()); auto asyc_generate_meas_long = [](opentelemetry::metrics::ObserverResultT &observer) {}; - // ObservableInstrument observable_counter_long(instrument_descriptor, std::move(metric_storage)); - // observable_counter_long.AddCallback(asyc_generate_measurements, nullptr); + ObservableInstrument observable_counter_long(instrument_descriptor, std::move(metric_storage)); + observable_counter_long.AddCallback(asyc_generate_measurements, nullptr); } #endif From c17e71076df56732bd904853b64251c2ca00c755 Mon Sep 17 00:00:00 2001 From: Lalit Date: Wed, 27 Jul 2022 00:26:13 -0700 Subject: [PATCH 22/26] fix --- sdk/test/metrics/async_metric_storage_test.cc | 21 +++++++++++-------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/sdk/test/metrics/async_metric_storage_test.cc b/sdk/test/metrics/async_metric_storage_test.cc index 9e4be26826..78e3d20253 100644 --- a/sdk/test/metrics/async_metric_storage_test.cc +++ b/sdk/test/metrics/async_metric_storage_test.cc @@ -71,7 +71,7 @@ class MeasurementFetcher static size_t fetch_count; static long number_of_get; static long number_of_put; - static const size_t number_of_attributes = 0; // GET , PUT fixme - should be 2 + static const size_t number_of_attributes = 2; }; size_t MeasurementFetcher::fetch_count; @@ -83,8 +83,6 @@ TEST_P(WritableMetricStorageTestFixture, TestAggregation) { MeasurementFetcher::init_values(); AggregationTemporality temporality = GetParam(); - ObservableRegistry registry; - registry.AddCallback(MeasurementFetcher::Fetcher, nullptr, nullptr); InstrumentDescriptor instr_desc = {"name", "desc", "1unit", InstrumentType::kObservableCounter, InstrumentValueType::kLong}; @@ -101,6 +99,14 @@ TEST_P(WritableMetricStorageTestFixture, TestAggregation) MeasurementFetcher measurement_fetcher; opentelemetry::sdk::metrics::AsyncMetricStorage storage(instr_desc, AggregationType::kSum, new DefaultAttributesProcessor()); + long get_count = 20l; + long put_count = 10l; + size_t attribute_count = 2; + std::unordered_map measurements = { + {{{"RequestType", "GET"}}, get_count}, {{{"RequestType", "PUT"}}, put_count}}; + storage.RecordLong(measurements, + opentelemetry::common::SystemTimestamp(std::chrono::system_clock::now())); + storage.Collect(collector.get(), collectors, sdk_start_ts, collection_ts, [&](const MetricData data) { for (auto data_attr : data.point_data_attr_) @@ -109,20 +115,17 @@ TEST_P(WritableMetricStorageTestFixture, TestAggregation) if (opentelemetry::nostd::get( data_attr.attributes.find("RequestType")->second) == "GET") { - EXPECT_EQ(opentelemetry::nostd::get(data.value_), - MeasurementFetcher::number_of_get); + EXPECT_EQ(opentelemetry::nostd::get(data.value_), get_count); } else if (opentelemetry::nostd::get( data_attr.attributes.find("RequestType")->second) == "PUT") { - EXPECT_EQ(opentelemetry::nostd::get(data.value_), - MeasurementFetcher::number_of_put); + EXPECT_EQ(opentelemetry::nostd::get(data.value_), put_count); } - count_attributes++; } return true; }); - EXPECT_EQ(MeasurementFetcher::number_of_attributes, count_attributes); + EXPECT_EQ(MeasurementFetcher::number_of_attributes, attribute_count); } INSTANTIATE_TEST_SUITE_P(WritableMetricStorageTestLong, From 7888fe3f0592788a9897d79ac5166396f7a2f0c4 Mon Sep 17 00:00:00 2001 From: Lalit Date: Mon, 1 Aug 2022 17:01:48 -0700 Subject: [PATCH 23/26] Fix --- sdk/src/metrics/meter.cc | 2 +- sdk/test/metrics/CMakeLists.txt | 1 + sdk/test/metrics/observable_registry_test.cc | 77 ++++++++++++++++++++ 3 files changed, 79 insertions(+), 1 deletion(-) create mode 100644 sdk/test/metrics/observable_registry_test.cc diff --git a/sdk/src/metrics/meter.cc b/sdk/src/metrics/meter.cc index 12e01efd33..f800ba9ecc 100644 --- a/sdk/src/metrics/meter.cc +++ b/sdk/src/metrics/meter.cc @@ -243,7 +243,7 @@ std::unique_ptr Meter::RegisterAsyncMetricStorage( auto view_registry = meter_context_->GetViewRegistry(); std::unique_ptr storages(new AsyncMultiMetricStorage()); auto success = view_registry->FindViews( - instrument_descriptor, *instrumentation_library_, + instrument_descriptor, *GetInstrumentationScope(), [this, &instrument_descriptor, &storages](const View &view) { auto view_instr_desc = instrument_descriptor; if (!view.GetName().empty()) diff --git a/sdk/test/metrics/CMakeLists.txt b/sdk/test/metrics/CMakeLists.txt index faf2f2b49f..7038506739 100644 --- a/sdk/test/metrics/CMakeLists.txt +++ b/sdk/test/metrics/CMakeLists.txt @@ -12,6 +12,7 @@ foreach( sync_instruments_test async_instruments_test metric_reader_test + observable_registry_test periodic_exporting_metric_reader_test) add_executable(${testname} "${testname}.cc") target_link_libraries( diff --git a/sdk/test/metrics/observable_registry_test.cc b/sdk/test/metrics/observable_registry_test.cc new file mode 100644 index 0000000000..77b5fc13fa --- /dev/null +++ b/sdk/test/metrics/observable_registry_test.cc @@ -0,0 +1,77 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +#ifndef ENABLE_METRICS_PREVIEW +# include "opentelemetry/sdk/metrics/state/observable_registry.h" +# include "opentelemetry/metrics/observer_result.h" + +# include + +using namespace opentelemetry::sdk::metrics; + +# if 0 +class MeasurementFetcher +{ +public: + static void Fetcher1(opentelemetry::metrics::ObserverResult &observer_result, + void * /*state*/) + { + fetch_count1++; + if (fetch_count1 == 1) + { + std::get Date: Mon, 1 Aug 2022 18:57:09 -0700 Subject: [PATCH 24/26] fix --- sdk/src/metrics/meter.cc | 5 +++-- sdk/test/metrics/async_metric_storage_test.cc | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/sdk/src/metrics/meter.cc b/sdk/src/metrics/meter.cc index abb56b3a49..047157a770 100644 --- a/sdk/src/metrics/meter.cc +++ b/sdk/src/metrics/meter.cc @@ -254,8 +254,9 @@ std::unique_ptr Meter::RegisterAsyncMetricStorage( { view_instr_desc.description_ = view.GetDescription(); } - auto storage = std::shared_ptr(new AsyncMetricStorage( - view_instr_desc, view.GetAggregationType(), &view.GetAttributesProcessor())); + auto storage = std::shared_ptr( + new AsyncMetricStorage(view_instr_desc, view.GetAggregationType(), + &view.GetAttributesProcessor(), view.GetAggregationConfig())); storage_registry_[instrument_descriptor.name_] = storage; static_cast(storages.get())->AddStorage(storage); return true; diff --git a/sdk/test/metrics/async_metric_storage_test.cc b/sdk/test/metrics/async_metric_storage_test.cc index ddb1568d9b..8bb13ada8f 100644 --- a/sdk/test/metrics/async_metric_storage_test.cc +++ b/sdk/test/metrics/async_metric_storage_test.cc @@ -96,8 +96,9 @@ TEST_P(WritableMetricStorageTestFixture, TestAggregation) collectors.push_back(collector); size_t count_attributes = 0; - opentelemetry::sdk::metrics::AsyncMetricStorage storage(instr_desc, AggregationType::kSum, - new DefaultAttributesProcessor()); + opentelemetry::sdk::metrics::AsyncMetricStorage storage( + instr_desc, AggregationType::kSum, new DefaultAttributesProcessor(), + std::shared_ptr{}); long get_count = 20l; long put_count = 10l; size_t attribute_count = 2; From 152a3a3d1280e4b281a15d3d10de75ea405835ea Mon Sep 17 00:00:00 2001 From: Lalit Date: Mon, 1 Aug 2022 19:29:09 -0700 Subject: [PATCH 25/26] fix --- api/include/opentelemetry/metrics/async_instruments.h | 1 - examples/common/metrics_foo_library/foo_library.cc | 3 ++- .../opentelemetry/sdk/metrics/state/async_metric_storage.h | 4 +--- sdk/src/metrics/meter.cc | 6 ++++++ sdk/src/metrics/state/observable_registry.cc | 6 +++--- sdk/test/metrics/async_instruments_test.cc | 1 - 6 files changed, 12 insertions(+), 9 deletions(-) diff --git a/api/include/opentelemetry/metrics/async_instruments.h b/api/include/opentelemetry/metrics/async_instruments.h index 6f61becbec..568ba602a3 100644 --- a/api/include/opentelemetry/metrics/async_instruments.h +++ b/api/include/opentelemetry/metrics/async_instruments.h @@ -10,7 +10,6 @@ OPENTELEMETRY_BEGIN_NAMESPACE namespace metrics { -// typedef void (*ObservableCallbackPtr)(ObserverResult &, void *); using ObservableCallbackPtr = void (*)(ObserverResult, void *); class ObservableInstrument diff --git a/examples/common/metrics_foo_library/foo_library.cc b/examples/common/metrics_foo_library/foo_library.cc index d8a00a5068..ec4fec736d 100644 --- a/examples/common/metrics_foo_library/foo_library.cc +++ b/examples/common/metrics_foo_library/foo_library.cc @@ -32,7 +32,7 @@ std::map get_random_attr() class MeasurementFetcher { public: - static void Fetcher(opentelemetry::metrics::ObserverResult &observer_result, void *state) + static void Fetcher(opentelemetry::metrics::ObserverResult observer_result, void *state) { std::map labels = get_random_attr(); auto labelkv = opentelemetry::common::KeyValueIterableView{labels}; @@ -69,6 +69,7 @@ void foo_library::observable_counter_example(const std::string &name) auto provider = metrics_api::Provider::GetMeterProvider(); nostd::shared_ptr meter = provider->GetMeter(name, "1.2.0"); auto counter = meter->CreateDoubleObservableCounter(counter_name); + counter->AddCallback(MeasurementFetcher::Fetcher, nullptr); while (true) { std::this_thread::sleep_for(std::chrono::milliseconds(500)); diff --git a/sdk/include/opentelemetry/sdk/metrics/state/async_metric_storage.h b/sdk/include/opentelemetry/sdk/metrics/state/async_metric_storage.h index 3fe95ee499..848be333a0 100644 --- a/sdk/include/opentelemetry/sdk/metrics/state/async_metric_storage.h +++ b/sdk/include/opentelemetry/sdk/metrics/state/async_metric_storage.h @@ -34,8 +34,8 @@ class AsyncMetricStorage : public MetricStorage, public AsyncWritableMetricStora aggregation_type_{aggregation_type}, attributes_processor_{attributes_processor}, state_{state}, - delta_hash_map_(new AttributesHashMap()), cumulative_hash_map_(new AttributesHashMap()), + delta_hash_map_(new AttributesHashMap()), temporal_metric_storage_(instrument_descriptor, aggregation_config) {} @@ -43,7 +43,6 @@ class AsyncMetricStorage : public MetricStorage, public AsyncWritableMetricStora void Record(const std::unordered_map &measurements, opentelemetry::common::SystemTimestamp observation_time) noexcept { - // std::shared_ptr delta_hash_map(new AttributesHashMap()); // process the read measurements - aggregate and store in hashmap for (auto &measurement : measurements) { @@ -105,7 +104,6 @@ class AsyncMetricStorage : public MetricStorage, public AsyncWritableMetricStora private: InstrumentDescriptor instrument_descriptor_; AggregationType aggregation_type_; - void (*measurement_collection_callback_)(opentelemetry::metrics::ObserverResult &, void *); const AttributesProcessor *attributes_processor_; void *state_; std::unique_ptr cumulative_hash_map_; diff --git a/sdk/src/metrics/meter.cc b/sdk/src/metrics/meter.cc index 047157a770..c8d5901382 100644 --- a/sdk/src/metrics/meter.cc +++ b/sdk/src/metrics/meter.cc @@ -261,6 +261,12 @@ std::unique_ptr Meter::RegisterAsyncMetricStorage( static_cast(storages.get())->AddStorage(storage); return true; }); + if (!success) + { + OTEL_INTERNAL_LOG_ERROR( + "[Meter::RegisterAsyncMetricStorage] - Error during finding matching views." + << "Some of the matching view configurations mayn't be used for metric collection"); + } return storages; } diff --git a/sdk/src/metrics/state/observable_registry.cc b/sdk/src/metrics/state/observable_registry.cc index 5c3451975c..7e12d4a190 100644 --- a/sdk/src/metrics/state/observable_registry.cc +++ b/sdk/src/metrics/state/observable_registry.cc @@ -21,7 +21,7 @@ void ObservableRegistry::AddCallback(opentelemetry::metrics::ObservableCallbackP // TBD - Check if existing std::unique_ptr record( new ObservableCallbackRecord{callback, state, instrument}); - std::unique_lock lk(callbacks_m_); + std::lock_guard lock_guard{callbacks_m_}; callbacks_.push_back(std::move(record)); } @@ -29,19 +29,19 @@ void ObservableRegistry::RemoveCallback(opentelemetry::metrics::ObservableCallba void *state, opentelemetry::metrics::ObservableInstrument *instrument) { + std::lock_guard lock_guard{callbacks_m_}; auto new_end = std::remove_if( callbacks_.begin(), callbacks_.end(), [callback, state, instrument](const std::unique_ptr &record) { return record->callback == callback && record->state == state && record->instrument == instrument; }); - std::unique_lock lk(callbacks_m_); callbacks_.erase(new_end, callbacks_.end()); } void ObservableRegistry::Observe(opentelemetry::common::SystemTimestamp collection_ts) { - std::unique_lock lk(callbacks_m_); + std::lock_guard lock_guard{callbacks_m_}; for (auto &callback_wrap : callbacks_) { auto value_type = diff --git a/sdk/test/metrics/async_instruments_test.cc b/sdk/test/metrics/async_instruments_test.cc index 5ac6caeb9a..67f5d919cd 100644 --- a/sdk/test/metrics/async_instruments_test.cc +++ b/sdk/test/metrics/async_instruments_test.cc @@ -20,7 +20,6 @@ TEST(AsyncInstruments, ObservableInstrument) InstrumentType::kObservableCounter, InstrumentValueType::kLong}; std::unique_ptr metric_storage(new AsyncMultiMetricStorage()); - auto asyc_generate_meas_long = [](opentelemetry::metrics::ObserverResultT &observer) {}; ObservableInstrument observable_counter_long(instrument_descriptor, std::move(metric_storage)); observable_counter_long.AddCallback(asyc_generate_measurements, nullptr); } From a2c9ceb89fa8d44520ea1c1778b721103eb86951 Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Wed, 3 Aug 2022 15:21:39 -0700 Subject: [PATCH 26/26] fix --- .../sdk/metrics/state/async_metric_storage.h | 8 +++++--- sdk/test/metrics/async_metric_storage_test.cc | 3 +++ 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/metrics/state/async_metric_storage.h b/sdk/include/opentelemetry/sdk/metrics/state/async_metric_storage.h index 848be333a0..97072f2efb 100644 --- a/sdk/include/opentelemetry/sdk/metrics/state/async_metric_storage.h +++ b/sdk/include/opentelemetry/sdk/metrics/state/async_metric_storage.h @@ -96,9 +96,11 @@ class AsyncMetricStorage : public MetricStorage, public AsyncWritableMetricStora nostd::function_ref metric_collection_callback) noexcept override { - return temporal_metric_storage_.buildMetrics(collector, collectors, sdk_start_ts, collection_ts, - std::move(delta_hash_map_), - metric_collection_callback); + auto status = temporal_metric_storage_.buildMetrics(collector, collectors, sdk_start_ts, + collection_ts, std::move(delta_hash_map_), + metric_collection_callback); + delta_hash_map_.reset(new AttributesHashMap()); + return status; } private: diff --git a/sdk/test/metrics/async_metric_storage_test.cc b/sdk/test/metrics/async_metric_storage_test.cc index 8bb13ada8f..f41f40182e 100644 --- a/sdk/test/metrics/async_metric_storage_test.cc +++ b/sdk/test/metrics/async_metric_storage_test.cc @@ -125,6 +125,9 @@ TEST_P(WritableMetricStorageTestFixture, TestAggregation) } return true; }); + // subsequent recording after collection shouldn't fail + EXPECT_NO_THROW(storage.RecordLong( + measurements, opentelemetry::common::SystemTimestamp(std::chrono::system_clock::now()))); EXPECT_EQ(MeasurementFetcher::number_of_attributes, attribute_count); }