From 7e1eeab7e4f8062118e0f77e1399f48c55d0c646 Mon Sep 17 00:00:00 2001 From: Ankit Bhargava Date: Wed, 15 Jul 2020 14:08:07 -0400 Subject: [PATCH 01/16] synchronous metric instruments --- .../opentelemetry/sdk/metrics/instrument.h | 182 +++++++++++ .../sdk/metrics/sync_instruments.h | 284 ++++++++++++++++++ sdk/test/metrics/BUILD | 11 + sdk/test/metrics/metric_instrument_test.cc | 158 ++++++++++ 4 files changed, 635 insertions(+) create mode 100644 sdk/include/opentelemetry/sdk/metrics/instrument.h create mode 100644 sdk/include/opentelemetry/sdk/metrics/sync_instruments.h create mode 100644 sdk/test/metrics/metric_instrument_test.cc diff --git a/sdk/include/opentelemetry/sdk/metrics/instrument.h b/sdk/include/opentelemetry/sdk/metrics/instrument.h new file mode 100644 index 0000000000..787bf234dd --- /dev/null +++ b/sdk/include/opentelemetry/sdk/metrics/instrument.h @@ -0,0 +1,182 @@ +#pragma once + +#include "opentelemetry/metrics/instrument.h" +#include "opentelemetry/sdk/metrics/aggregator/aggregator.h" +#include "opentelemetry/version.h" + +#include +#include + +namespace metrics_api = opentelemetry::metrics; +namespace trace_api = opentelemetry::trace; + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace sdk +{ +namespace metrics +{ + + +class Instrument : metrics_api::Instrument { + +public: + Instrument() = default; + + Instrument(nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + bool enabled): name_(name), description_(description), unit_(unit), enabled_(enabled) {} + + // Returns true if the instrument is enabled and collecting data + virtual bool IsEnabled() override { + return enabled_; + } + + // Return the instrument name + virtual nostd::string_view GetName() override { return name_; } + + // Return the instrument description + virtual nostd::string_view GetDescription() override { return description_; } + + // Return the insrument's units of measurement + virtual nostd::string_view GetUnits() override { return unit_; } + +protected: + std::string name_; + std::string description_; + std::string unit_; + bool enabled_; + std::mutex mu_; +}; + +template +class BoundSynchronousInstrument : public Instrument { + +public: + BoundSynchronousInstrument() = default; + + BoundSynchronousInstrument(nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + bool enabled, + metrics_api::BoundInstrumentKind kind, + std::shared_ptr> agg) + :Instrument(name, description, unit, enabled), agg_(agg), kind_(kind) + { this->inc_ref(); } // increase reference count when instantiated + + /** + * Frees the resources associated with this Bound Instrument. + * The Metric from which this instrument was created is not impacted. + * + * @param none + * @return void + */ + virtual void unbind() final { ref_ -= 1; } + + /** + * Increments the reference count. This function is used when binding or instantiating. + * + * @param none + * @return void + */ + virtual void inc_ref() final { ref_ += 1; } + + /** + * Returns the current reference count of the instrument. This value is used to + * later in the pipeline remove stale instruments. + * + * @param none + * @return current ref count of the instrument + */ + virtual int get_ref() final { return ref_; } + + /** + * Records a single synchronous metric event; a call to the aggregator + * Since this is a bound synchronous instrument, labels are not required in * metric capture + * calls. + * + * @param value is the numerical representation of the metric being captured + * @return void + */ + virtual void update(T value) final { + this->mu_.lock(); + agg_->update(value); + this->mu_.unlock(); + } + + /** + * Return this instrument's type + * + * @param none + * @return the BoundInstrumentKind describing this instrument + */ + metrics_api::BoundInstrumentKind get_kind(){ return kind_; } + + /** + * Returns the aggregator responsible for meaningfully combining update values. + * + * @param none + * @return the aggregator assigned to this instrument + */ + virtual std::shared_ptr> get_aggregator() final{ return agg_; } + +private: + std::shared_ptr> agg_; + metrics_api::BoundInstrumentKind kind_; + int ref_; +}; + +template +class SynchronousInstrument : public Instrument { + +public: + SynchronousInstrument() = default; + + SynchronousInstrument(nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + bool enabled, + metrics_api::InstrumentKind kind) + : Instrument(name, description, unit, enabled), kind_(kind) + {} + + /** + * Returns a Bound Instrument associated with the specified labels. * Multiples requests + * with the same set of labels may return the same Bound Instrument instance. + * + * It is recommended that callers keep a reference to the Bound Instrument + * instead of repeatedly calling this operation. + * + * @param labels the set of labels, as key-value pairs + * @return a Bound Instrument + */ + std::shared_ptr> bind(const nostd::string_view &labels); + + /** + * Return this instrument's type + * + * @param none + * @return the InstrumentKind describing this instrument + */ + metrics_api::InstrumentKind get_kind(){ + return kind_; + } + +private: + metrics_api::InstrumentKind kind_; +}; + +// Utility function which converts maps to strings for better performance +std::string mapToString(const std::map & conv){ + std::stringstream ss; + ss <<"{ "; + for (auto i:conv){ + ss < +#include +#include + +namespace metrics_api = opentelemetry::metrics; + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace sdk +{ +namespace metrics +{ + +template +class BoundCounter final: public BoundSynchronousInstrument { + +public: + BoundCounter() = default; + + BoundCounter(nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + bool enabled): + BoundSynchronousInstrument(name, description, unit, enabled, + metrics_api::BoundInstrumentKind::BoundIntCounter,std::shared_ptr>(new + CounterAggregator(metrics_api::BoundInstrumentKind::BoundIntCounter))) // Aggregator is chosen here + {} + + /* + * Add adds the value to the counter's sum. The labels are already linked to the instrument + * and are not specified. + * + * @param value the numerical representation of the metric being captured + * @param labels the set of labels, as key-value pairs + */ + void add(T value) { + this->mu_.lock(); + if (value < 0){ + throw std::invalid_argument("Counter instrument updates must be non-negative."); + } else { + this->update(value); + } + this->mu_.unlock(); + } + +}; + +template +class Counter final : public SynchronousInstrument +{ + +public: + Counter() = default; + + Counter(nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + bool enabled): + SynchronousInstrument(name, description, unit, enabled, metrics_api::InstrumentKind::IntCounter) + {} + + /* + * Bind creates a bound instrument for this counter. The labels are + * associated with values recorded via subsequent calls to Record. + * + * @param labels the set of labels, as key-value pairs. + * @return a BoundCounter tied to the specified labels + */ + std::shared_ptr> bind(const std::map &labels) + { + std::string labelset = mapToString(labels); + if (boundInstruments_.find(labelset) == boundInstruments_.end()) + { + auto sp1 = std::make_shared>(this->name_, this->description_, this->unit_, this->enabled_); + boundInstruments_[labelset]=sp1; + return sp1; + } + else + { + boundInstruments_[labelset]->inc_ref(); + return boundInstruments_[labelset]; + } + } + + /* + * Add adds the value to the counter's sum. The labels should contain + * the keys and values to be associated with this value. Counters only * accept positive + * valued updates. + * + * @param value the numerical representation of the metric being captured + * @param labels the set of labels, as key-value pairs + */ + void add(T value, const std::map &labels) + { + this->mu_.lock(); + auto sp = bind(labels); + sp->update(value); + sp->unbind(); + this->mu_.unlock(); + } + + // A collection of the bound instruments created by this unbound instrument identified by their labels. + std::unordered_map>> boundInstruments_; +}; + + +template +class BoundUpDownCounter final: public BoundSynchronousInstrument { + +public: + BoundUpDownCounter() = default; + + BoundUpDownCounter(nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + bool enabled): + BoundSynchronousInstrument(name, description, unit, enabled, + metrics_api::BoundInstrumentKind::BoundIntUpDownCounter, + std::shared_ptr>(new CounterAggregator(metrics_api::BoundInstrumentKind::BoundIntUpDownCounter))) + {} + + /* + * Add adds the value to the counter's sum. The labels are already linked * to the instrument + * and are not specified. + * + * @param value the numerical representation of the metric being captured + * @param labels the set of labels, as key-value pairs + */ + void add(T value) { + this->mu_.lock(); // does this need to be locked? + this->update(value); //update calls the aggregator which is itself locked + this->mu_.unlock(); + } +}; + +template +class UpDownCounter final : public SynchronousInstrument +{ + +public: + UpDownCounter() = default; + + UpDownCounter(nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + bool enabled): + SynchronousInstrument(name, description, unit, enabled, metrics_api::InstrumentKind::IntUpDownCounter) + {} + + /* + * Bind creates a bound instrument for this counter. The labels are + * associated with values recorded via subsequent calls to Record. + * + * @param labels the set of labels, as key-value pairs. + * @return a BoundIntCounter tied to the specified labels + */ + std::shared_ptr> bind(const std::map &labels) + { + std::string labelset = mapToString(labels); // COULD CUSTOM HASH THIS INSTEAD FOR PERFORMANCE + if (boundInstruments_.find(labelset) == boundInstruments_.end()) + { + auto sp1 = std::make_shared>(this->name_, this->description_, this->unit_, this->enabled_); + boundInstruments_[labelset]=sp1; // perhaps use emplace + return sp1; + } + else + { + boundInstruments_[labelset]->inc_ref(); + return boundInstruments_[labelset]; + } + } + + /* + * Add adds the value to the counter's sum. The labels should contain + * the keys and values to be associated with this value. Counters only * accept positive + * valued updates. + * + * @param value the numerical representation of the metric being captured + * @param labels the set of labels, as key-value pairs + */ + void add(T value, const std::map &labels) + { + this->mu_.lock(); + auto sp = bind(labels); + sp->update(value); + sp->unbind(); + this->mu_.unlock(); + } + + std::unordered_map>> boundInstruments_; +}; + +template +class BoundValueRecorder final: public BoundSynchronousInstrument { + +public: + BoundValueRecorder() = default; + + BoundValueRecorder(nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + bool enabled): BoundSynchronousInstrument(name, description, unit, enabled, + metrics_api::BoundInstrumentKind::BoundIntValueRecorder, + std::shared_ptr>(new MinMaxSumCountAggregator(metrics_api::BoundInstrumentKind::BoundIntValueRecorder))) // Aggregator is chosen here + {} + + /* + * Add adds the value to the counter's sum. The labels are already linked to the instrument + * and are not specified. + * + * @param value the numerical representation of the metric being captured + * @param labels the set of labels, as key-value pairs + */ + void record(T value) { + this->mu_.lock(); + update(value); + this->mu_.unlock(); + } +}; + +template +class ValueRecorder final : public SynchronousInstrument +{ + +public: + ValueRecorder() = default; + + ValueRecorder(nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + bool enabled): + SynchronousInstrument(name, description, unit, enabled, metrics_api::InstrumentKind::IntValueRecorder) + {} + + /* + * Bind creates a bound instrument for this counter. The labels are + * associated with values recorded via subsequent calls to Record. + * + * @param labels the set of labels, as key-value pairs. + * @return a BoundIntCounter tied to the specified labels + */ + std::shared_ptr> bind(const std::map &labels) + { + std::string labelset = mapToString(labels); // COULD CUSTOM HASH THIS INSTEAD FOR PERFORMANCE + if (boundInstruments_.find(labelset) == boundInstruments_.end()) + { + auto sp1 = std::make_shared>(this->name_, this->description_, this->unit_, this->enabled_); + boundInstruments_[labelset]=sp1; // perhaps use emplace + return sp1; + } + else + { + boundInstruments_[labelset]->inc_ref(); + return boundInstruments_[labelset]; + } + } + + /* + * Add adds the value to the counter's sum. The labels should contain + * the keys and values to be associated with this value. Counters only * accept positive + * valued updates. + * + * @param value the numerical representation of the metric being captured + * @param labels the set of labels, as key-value pairs + */ + void record(T value, const std::map &labels) + { + this->mu_.lock(); + auto sp = bind(labels); + sp->update(value); + sp->unbind(); + this->mu_.unlock(); + } + + std::unordered_map>> boundInstruments_; +}; + +} +} +OPENTELEMETRY_END_NAMESPACE diff --git a/sdk/test/metrics/BUILD b/sdk/test/metrics/BUILD index d24acd3e58..48c9276a21 100644 --- a/sdk/test/metrics/BUILD +++ b/sdk/test/metrics/BUILD @@ -63,3 +63,14 @@ cc_test( "@com_google_googletest//:gtest_main", ], ) + +cc_test( + name = "metric_instrument_test", + srcs = [ + "metric_instrument_test.cc", + ], + deps = [ + "//sdk/src/metrics", + "@com_google_googletest//:gtest_main", + ], +) \ No newline at end of file diff --git a/sdk/test/metrics/metric_instrument_test.cc b/sdk/test/metrics/metric_instrument_test.cc new file mode 100644 index 0000000000..1eab86b68b --- /dev/null +++ b/sdk/test/metrics/metric_instrument_test.cc @@ -0,0 +1,158 @@ +#include +#include "opentelemetry/sdk/metrics/sync_instruments.h" +#include "opentelemetry/sdk/metrics/async_instruments.h" + +#include +#include +#include +#include +#include + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace sdk +{ +namespace metrics +{ + +TEST(Counter, InstrumentFunctions) +{ + Counter alpha("enabled", "no description", "unitless", true); + Counter beta("not enabled", "some description", "units", false); + + EXPECT_EQ(alpha.GetName(), "enabled"); + EXPECT_EQ(alpha.GetDescription(), "no description"); + EXPECT_EQ(alpha.GetUnits(), "unitless"); + EXPECT_EQ(alpha.IsEnabled(), true); + + EXPECT_EQ(beta.GetName(), "not enabled"); + EXPECT_EQ(beta.GetDescription(), "some description"); + EXPECT_EQ(beta.GetUnits(), "units"); + EXPECT_EQ(beta.IsEnabled(), false); +} + +TEST(Counter, Binding) +{ + Counter alpha("test", "none", "unitless", true); + + std::map labels = {{"key", "value"}}; + std::map labels1 = {{"key1", "value1"}}; + std::map labels2 = {{"key2", "value2"}, {"key3", "value3"}}; + std::map labels3 = {{"key3", "value3"}, {"key2", "value2"}}; + + auto beta = alpha.bind(labels); + auto gamma = alpha.bind(labels1); + auto delta = alpha.bind(labels1); + auto epsilon = alpha.bind(labels1); + auto zeta = alpha.bind(labels2); + auto eta = alpha.bind(labels3); + + EXPECT_EQ (beta->get_ref(), 1); + EXPECT_EQ(gamma->get_ref(),3); + EXPECT_EQ(eta->get_ref(),2); + + delta->unbind(); + gamma->unbind(); + epsilon->unbind(); + + EXPECT_EQ(alpha.boundInstruments_[mapToString(labels1)]->get_ref(), 0); + EXPECT_EQ(alpha.boundInstruments_.size(), 3); +} + +void CounterCallback(std::shared_ptr> in, int freq, std::map labels){ + for (int i=0; iadd(1, labels); + } +} + +TEST(Counter, StressAdd){ + std::shared_ptr> alpha(new Counter("test", "none", "unitless", true)); + + std::map labels = {{"key", "value"}}; + std::map labels1 = {{"key1", "value1"}}; + + std::thread first (CounterCallback, alpha, 100000, labels); + std::thread second (CounterCallback, alpha, 100000, labels); + std::thread third (CounterCallback, alpha, 300000, labels1); + + first.join(); + second.join(); + third.join(); + + EXPECT_EQ(alpha->boundInstruments_[mapToString(labels)]->get_aggregator()->get_values()[0], 200000); + EXPECT_EQ(alpha->boundInstruments_[mapToString(labels1)]->get_aggregator()->get_values()[0], 300000); +} + +void UpDownCounterCallback(std::shared_ptr> in, int freq, std::map labels){ + for (int i=0; iadd(1, labels); + } +} + +void NegUpDownCounterCallback(std::shared_ptr> in, int freq, std::map labels){ + for (int i=0; iadd(-1, labels); + } +} + +TEST(IntUpDownCounter, StressAdd){ + std::shared_ptr> alpha(new UpDownCounter("test", "none", "unitless", true)); + + std::map labels = {{"key", "value"}}; + std::map labels1 = {{"key1", "value1"}}; + + std::thread first (UpDownCounterCallback, alpha, 123400, labels); // spawn new threads that call the callback + std::thread second (UpDownCounterCallback, alpha, 123400, labels); + std::thread third (UpDownCounterCallback, alpha, 567800, labels1); + std::thread fourth (NegUpDownCounterCallback, alpha, 123400, labels1); // negative values + + first.join(); + second.join(); + third.join(); + fourth.join(); + + EXPECT_EQ(alpha->boundInstruments_[mapToString(labels)]->get_aggregator()->get_values()[0], 123400*2); + EXPECT_EQ(alpha->boundInstruments_[mapToString(labels1)]->get_aggregator()->get_values()[0], 567800-123400); +} + +void RecorderCallback(std::shared_ptr> in, int freq, std::map labels){ + for (int i=0; irecord(i, labels); + } +} + +void NegRecorderCallback(std::shared_ptr> in, int freq, std::map labels){ + for (int i=0; irecord(-i, labels); + } +} + +TEST(IntValueRecorder, StressRecord){ + std::shared_ptr> alpha(new ValueRecorder("test", "none", "unitless", true)); + + std::map labels = {{"key", "value"}}; + std::map labels1 = {{"key1", "value1"}}; + + std::thread first (RecorderCallback, alpha, 25, labels); // spawn new threads that call the callback + std::thread second (RecorderCallback, alpha, 50, labels); + std::thread third (RecorderCallback, alpha, 25, labels1); + std::thread fourth (NegRecorderCallback, alpha, 100, labels1); // negative values + + first.join(); + second.join(); + third.join(); + fourth.join(); + + EXPECT_EQ(alpha->boundInstruments_[mapToString(labels)]->get_aggregator()->get_values()[0], 0); // min + EXPECT_EQ(alpha->boundInstruments_[mapToString(labels)]->get_aggregator()->get_values()[1], 49); // max + EXPECT_EQ(alpha->boundInstruments_[mapToString(labels)]->get_aggregator()->get_values()[2], 1525); // sum + EXPECT_EQ(alpha->boundInstruments_[mapToString(labels)]->get_aggregator()->get_values()[3], 75); // count + + EXPECT_EQ(alpha->boundInstruments_[mapToString(labels1)]->get_aggregator()->get_values()[0], -99); // min + EXPECT_EQ(alpha->boundInstruments_[mapToString(labels1)]->get_aggregator()->get_values()[1], 24); // max + EXPECT_EQ(alpha->boundInstruments_[mapToString(labels1)]->get_aggregator()->get_values()[2], -4650); // sum + EXPECT_EQ(alpha->boundInstruments_[mapToString(labels1)]->get_aggregator()->get_values()[3], 125); // count +} + +} //namespace sdk +} // namespace metrics +OPENTELEMETRY_END_NAMESPACE From 003dc8651abd33dea3a855e544a44db1c6c89ecb Mon Sep 17 00:00:00 2001 From: Ankit Bhargava Date: Sun, 26 Jul 2020 00:37:16 -0400 Subject: [PATCH 02/16] ABI compatibility compliance --- .../opentelemetry/sdk/metrics/instrument.h | 116 ++++++++--- .../sdk/metrics/sync_instruments.h | 194 +++++++++++------- sdk/test/metrics/metric_instrument_test.cc | 155 +++++++++----- 3 files changed, 311 insertions(+), 154 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/metrics/instrument.h b/sdk/include/opentelemetry/sdk/metrics/instrument.h index 787bf234dd..3754865db8 100644 --- a/sdk/include/opentelemetry/sdk/metrics/instrument.h +++ b/sdk/include/opentelemetry/sdk/metrics/instrument.h @@ -2,10 +2,16 @@ #include "opentelemetry/metrics/instrument.h" #include "opentelemetry/sdk/metrics/aggregator/aggregator.h" +#include "opentelemetry/sdk/metrics/record.h" #include "opentelemetry/version.h" #include #include +#include +#include +#include +#include +#include namespace metrics_api = opentelemetry::metrics; namespace trace_api = opentelemetry::trace; @@ -17,7 +23,7 @@ namespace metrics { -class Instrument : metrics_api::Instrument { +class Instrument : virtual public metrics_api::Instrument { public: Instrument() = default; @@ -25,7 +31,8 @@ class Instrument : metrics_api::Instrument { Instrument(nostd::string_view name, nostd::string_view description, nostd::string_view unit, - bool enabled): name_(name), description_(description), unit_(unit), enabled_(enabled) {} + bool enabled, + metrics_api::InstrumentKind kind): name_(name), description_(description), unit_(unit), enabled_(enabled), kind_(kind) {} // Returns true if the instrument is enabled and collecting data virtual bool IsEnabled() override { @@ -41,27 +48,31 @@ class Instrument : metrics_api::Instrument { // Return the insrument's units of measurement virtual nostd::string_view GetUnits() override { return unit_; } + virtual metrics_api::InstrumentKind GetKind() override { return this->kind_; } + protected: std::string name_; std::string description_; std::string unit_; bool enabled_; std::mutex mu_; + metrics_api::InstrumentKind kind_; }; template -class BoundSynchronousInstrument : public Instrument { +class BoundSynchronousInstrument : public Instrument, virtual public metrics_api::BoundSynchronousInstrument { public: + BoundSynchronousInstrument() = default; BoundSynchronousInstrument(nostd::string_view name, nostd::string_view description, nostd::string_view unit, bool enabled, - metrics_api::BoundInstrumentKind kind, - std::shared_ptr> agg) - :Instrument(name, description, unit, enabled), agg_(agg), kind_(kind) + metrics_api::InstrumentKind kind, + nostd::shared_ptr> agg) + :Instrument(name, description, unit, enabled, kind), agg_(agg) { this->inc_ref(); } // increase reference count when instantiated /** @@ -71,7 +82,7 @@ class BoundSynchronousInstrument : public Instrument { * @param none * @return void */ - virtual void unbind() final { ref_ -= 1; } + virtual void unbind() override { ref_ -= 1; } /** * Increments the reference count. This function is used when binding or instantiating. @@ -79,7 +90,7 @@ class BoundSynchronousInstrument : public Instrument { * @param none * @return void */ - virtual void inc_ref() final { ref_ += 1; } + virtual void inc_ref() override { ref_ += 1; } /** * Returns the current reference count of the instrument. This value is used to @@ -88,7 +99,7 @@ class BoundSynchronousInstrument : public Instrument { * @param none * @return current ref count of the instrument */ - virtual int get_ref() final { return ref_; } + virtual int get_ref() override { return ref_; } /** * Records a single synchronous metric event; a call to the aggregator @@ -98,46 +109,37 @@ class BoundSynchronousInstrument : public Instrument { * @param value is the numerical representation of the metric being captured * @return void */ - virtual void update(T value) final { + virtual void update(T value) override { this->mu_.lock(); agg_->update(value); this->mu_.unlock(); } - /** - * Return this instrument's type - * - * @param none - * @return the BoundInstrumentKind describing this instrument - */ - metrics_api::BoundInstrumentKind get_kind(){ return kind_; } - /** * Returns the aggregator responsible for meaningfully combining update values. * * @param none * @return the aggregator assigned to this instrument */ - virtual std::shared_ptr> get_aggregator() final{ return agg_; } + virtual nostd::shared_ptr> GetAggregator() final { return agg_; } private: - std::shared_ptr> agg_; - metrics_api::BoundInstrumentKind kind_; - int ref_; + nostd::shared_ptr> agg_; + int ref_ = 0; }; template -class SynchronousInstrument : public Instrument { +class SynchronousInstrument : public Instrument, virtual public metrics_api::SynchronousInstrument { public: + SynchronousInstrument() = default; SynchronousInstrument(nostd::string_view name, nostd::string_view description, nostd::string_view unit, bool enabled, - metrics_api::InstrumentKind kind) - : Instrument(name, description, unit, enabled), kind_(kind) + metrics_api::InstrumentKind kind):Instrument(name,description,unit,enabled,kind) {} /** @@ -150,24 +152,45 @@ class SynchronousInstrument : public Instrument { * @param labels the set of labels, as key-value pairs * @return a Bound Instrument */ - std::shared_ptr> bind(const nostd::string_view &labels); + virtual nostd::shared_ptr> bind(const trace::KeyValueIterable &labels) override { + return nostd::shared_ptr>(); + } + + virtual void update(T value, const trace::KeyValueIterable &labels) override = 0; /** - * Return this instrument's type + * Checkpoints instruments and returns a set of records which are ready for processing. + * This method should only be called by the Meter Class as part of the export pipeline. * * @param none - * @return the InstrumentKind describing this instrument + * @return vector of Records which hold the data attached to this synchronous instrument */ - metrics_api::InstrumentKind get_kind(){ - return kind_; - } + virtual std::vector GetRecords() = 0; -private: - metrics_api::InstrumentKind kind_; +}; + +// Helper functions for turning a trace::KeyValueIterable into a string +inline void print_value(std::stringstream &ss, + common::AttributeValue &value, + bool jsonTypes = false) +{ + switch (value.index()) + { + case common::AttributeType::TYPE_STRING: + if (jsonTypes) + ss << '"'; + ss << nostd::get(value); + if (jsonTypes) + ss << '"'; + break; + default: + throw std::invalid_argument("Labels must be strings"); + break; + } }; // Utility function which converts maps to strings for better performance -std::string mapToString(const std::map & conv){ +inline std::string mapToString(const std::map & conv){ std::stringstream ss; ss <<"{ "; for (auto i:conv){ @@ -177,6 +200,31 @@ std::string mapToString(const std::map & conv){ return ss.str(); } +inline std::string KvToString(const trace::KeyValueIterable &kv) noexcept +{ + std::stringstream ss; + ss << "{"; + size_t size = kv.size(); + if (size) + { + size_t i = 1; + // TODO: we need to do something with this iterator. It is not very convenient. + // Having range-based for loop would've been nicer + kv.ForEachKeyValue([&](nostd::string_view key, common::AttributeValue value) noexcept { + ss << "\"" << key << "\":"; + print_value(ss, value, true); + if (size != i) + { + ss << ","; + } + i++; + return true; + }); + }; + ss << "}"; + return ss.str(); +} + } // namespace metrics } // namespace sdk OPENTELEMETRY_END_NAMESPACE diff --git a/sdk/include/opentelemetry/sdk/metrics/sync_instruments.h b/sdk/include/opentelemetry/sdk/metrics/sync_instruments.h index 245d5b813a..be7a3ff622 100644 --- a/sdk/include/opentelemetry/sdk/metrics/sync_instruments.h +++ b/sdk/include/opentelemetry/sdk/metrics/sync_instruments.h @@ -3,9 +3,12 @@ #include "opentelemetry/sdk/metrics/instrument.h" #include "opentelemetry/sdk/metrics/aggregator/counter_aggregator.h" #include "opentelemetry/sdk/metrics/aggregator/min_max_sum_count_aggregator.h" +#include "opentelemetry/metrics/sync_instruments.h" #include #include #include +#include +#include namespace metrics_api = opentelemetry::metrics; @@ -16,20 +19,20 @@ namespace metrics { template -class BoundCounter final: public BoundSynchronousInstrument { - +class BoundCounter final: public BoundSynchronousInstrument, public metrics_api::BoundCounter { + public: BoundCounter() = default; - + BoundCounter(nostd::string_view name, nostd::string_view description, nostd::string_view unit, bool enabled): BoundSynchronousInstrument(name, description, unit, enabled, - metrics_api::BoundInstrumentKind::BoundIntCounter,std::shared_ptr>(new - CounterAggregator(metrics_api::BoundInstrumentKind::BoundIntCounter))) // Aggregator is chosen here + metrics_api::InstrumentKind::Counter,nostd::shared_ptr>(new + CounterAggregator(metrics_api::InstrumentKind::Counter))) // Aggregator is chosen here {} - + /* * Add adds the value to the counter's sum. The labels are already linked to the instrument * and are not specified. @@ -37,7 +40,7 @@ class BoundCounter final: public BoundSynchronousInstrument { * @param value the numerical representation of the metric being captured * @param labels the set of labels, as key-value pairs */ - void add(T value) { + virtual void add(T value) override { this->mu_.lock(); if (value < 0){ throw std::invalid_argument("Counter instrument updates must be non-negative."); @@ -50,19 +53,20 @@ class BoundCounter final: public BoundSynchronousInstrument { }; template -class Counter final : public SynchronousInstrument +class Counter final : public SynchronousInstrument, public metrics_api::Counter { - + public: + Counter() = default; - + Counter(nostd::string_view name, nostd::string_view description, nostd::string_view unit, bool enabled): - SynchronousInstrument(name, description, unit, enabled, metrics_api::InstrumentKind::IntCounter) + SynchronousInstrument(name, description, unit, enabled, metrics_api::InstrumentKind::Counter) {} - + /* * Bind creates a bound instrument for this counter. The labels are * associated with values recorded via subsequent calls to Record. @@ -70,12 +74,12 @@ class Counter final : public SynchronousInstrument * @param labels the set of labels, as key-value pairs. * @return a BoundCounter tied to the specified labels */ - std::shared_ptr> bind(const std::map &labels) - { - std::string labelset = mapToString(labels); + + virtual nostd::shared_ptr> bindCounter(const trace::KeyValueIterable &labels) override { + std::string labelset = KvToString(labels); if (boundInstruments_.find(labelset) == boundInstruments_.end()) { - auto sp1 = std::make_shared>(this->name_, this->description_, this->unit_, this->enabled_); + auto sp1 = nostd::shared_ptr>(new BoundCounter(this->name_, this->description_, this->unit_, this->enabled_)); boundInstruments_[labelset]=sp1; return sp1; } @@ -87,42 +91,59 @@ class Counter final : public SynchronousInstrument } /* - * Add adds the value to the counter's sum. The labels should contain - * the keys and values to be associated with this value. Counters only * accept positive - * valued updates. - * - * @param value the numerical representation of the metric being captured - * @param labels the set of labels, as key-value pairs - */ - void add(T value, const std::map &labels) - { + * Add adds the value to the counter's sum. The labels should contain + * the keys and values to be associated with this value. Counters only + * accept positive valued updates. + * + * @param value the numerical representation of the metric being captured + * @param labels the set of labels, as key-value pairs + */ + virtual void add(T value, const trace::KeyValueIterable &labels) override { this->mu_.lock(); - auto sp = bind(labels); - sp->update(value); - sp->unbind(); + if (value < 0){ + throw std::invalid_argument("Counter instrument updates must be non-negative."); + } else { + auto sp = bindCounter(labels); + sp->update(value); + sp->unbind(); + } this->mu_.unlock(); } + + virtual std::vector GetRecords() override { + std::vector ret; + for (auto x : boundInstruments_){ + auto agg_ptr = dynamic_cast*>(x.second.get())->GetAggregator(); + agg_ptr->checkpoint(); + ret.push_back(Record(x.second->GetName(), x.second->GetDescription(), x.first, agg_ptr)); + } + return ret; + } + virtual void update(T value, const trace::KeyValueIterable &labels) override { + add(value, labels); + } + // A collection of the bound instruments created by this unbound instrument identified by their labels. - std::unordered_map>> boundInstruments_; + std::unordered_map>> boundInstruments_; }; template -class BoundUpDownCounter final: public BoundSynchronousInstrument { - +class BoundUpDownCounter final: public BoundSynchronousInstrument, virtual public metrics_api::BoundUpDownCounter { + public: - BoundUpDownCounter() = default; - - BoundUpDownCounter(nostd::string_view name, + BoundUpDownCounter() = default; + + BoundUpDownCounter(nostd::string_view name, nostd::string_view description, nostd::string_view unit, bool enabled): BoundSynchronousInstrument(name, description, unit, enabled, - metrics_api::BoundInstrumentKind::BoundIntUpDownCounter, - std::shared_ptr>(new CounterAggregator(metrics_api::BoundInstrumentKind::BoundIntUpDownCounter))) + metrics_api::InstrumentKind::UpDownCounter, + nostd::shared_ptr>(new CounterAggregator(metrics_api::InstrumentKind::UpDownCounter))) {} - + /* * Add adds the value to the counter's sum. The labels are already linked * to the instrument * and are not specified. @@ -130,27 +151,27 @@ class BoundUpDownCounter final: public BoundSynchronousInstrument { * @param value the numerical representation of the metric being captured * @param labels the set of labels, as key-value pairs */ - void add(T value) { - this->mu_.lock(); // does this need to be locked? - this->update(value); //update calls the aggregator which is itself locked - this->mu_.unlock(); + virtual void add(T value) override { + this->mu_.lock(); + this->update(value); + this->mu_.unlock(); } }; template -class UpDownCounter final : public SynchronousInstrument +class UpDownCounter final : public SynchronousInstrument, public metrics_api::UpDownCounter { - + public: UpDownCounter() = default; - + UpDownCounter(nostd::string_view name, nostd::string_view description, nostd::string_view unit, bool enabled): - SynchronousInstrument(name, description, unit, enabled, metrics_api::InstrumentKind::IntUpDownCounter) + SynchronousInstrument(name, description, unit, enabled, metrics_api::InstrumentKind::UpDownCounter) {} - + /* * Bind creates a bound instrument for this counter. The labels are * associated with values recorded via subsequent calls to Record. @@ -158,12 +179,12 @@ class UpDownCounter final : public SynchronousInstrument * @param labels the set of labels, as key-value pairs. * @return a BoundIntCounter tied to the specified labels */ - std::shared_ptr> bind(const std::map &labels) + nostd::shared_ptr> bindUpDownCounter(const trace::KeyValueIterable &labels) override { - std::string labelset = mapToString(labels); // COULD CUSTOM HASH THIS INSTEAD FOR PERFORMANCE + std::string labelset = KvToString(labels); // COULD CUSTOM HASH THIS INSTEAD FOR PERFORMANCE if (boundInstruments_.find(labelset) == boundInstruments_.end()) { - auto sp1 = std::make_shared>(this->name_, this->description_, this->unit_, this->enabled_); + auto sp1 = nostd::shared_ptr>(new BoundUpDownCounter(this->name_, this->description_, this->unit_, this->enabled_)); boundInstruments_[labelset]=sp1; // perhaps use emplace return sp1; } @@ -173,7 +194,7 @@ class UpDownCounter final : public SynchronousInstrument return boundInstruments_[labelset]; } } - + /* * Add adds the value to the counter's sum. The labels should contain * the keys and values to be associated with this value. Counters only * accept positive @@ -182,32 +203,47 @@ class UpDownCounter final : public SynchronousInstrument * @param value the numerical representation of the metric being captured * @param labels the set of labels, as key-value pairs */ - void add(T value, const std::map &labels) + void add(T value, const trace::KeyValueIterable &labels) override { this->mu_.lock(); - auto sp = bind(labels); + auto sp = bindUpDownCounter(labels); sp->update(value); sp->unbind(); this->mu_.unlock(); } - std::unordered_map>> boundInstruments_; + virtual std::vector GetRecords() override { + std::vector ret; + for (auto x : boundInstruments_){ + auto agg_ptr = dynamic_cast*>(x.second.get())->GetAggregator(); + agg_ptr->checkpoint(); + ret.push_back(Record(x.second->GetName(), x.second->GetDescription(), x.first, agg_ptr)); + } + return ret; + } + + virtual void update(T val, const trace::KeyValueIterable &labels) override { + add(val, labels); + } + + + std::unordered_map>> boundInstruments_; }; template -class BoundValueRecorder final: public BoundSynchronousInstrument { - +class BoundValueRecorder final: public BoundSynchronousInstrument , public metrics_api::BoundValueRecorder{ + public: BoundValueRecorder() = default; - + BoundValueRecorder(nostd::string_view name, nostd::string_view description, nostd::string_view unit, bool enabled): BoundSynchronousInstrument(name, description, unit, enabled, - metrics_api::BoundInstrumentKind::BoundIntValueRecorder, - std::shared_ptr>(new MinMaxSumCountAggregator(metrics_api::BoundInstrumentKind::BoundIntValueRecorder))) // Aggregator is chosen here + metrics_api::InstrumentKind::ValueRecorder, + nostd::shared_ptr>(new MinMaxSumCountAggregator(metrics_api::InstrumentKind::ValueRecorder))) // Aggregator is chosen here {} - + /* * Add adds the value to the counter's sum. The labels are already linked to the instrument * and are not specified. @@ -217,25 +253,25 @@ class BoundValueRecorder final: public BoundSynchronousInstrument { */ void record(T value) { this->mu_.lock(); - update(value); + this->update(value); this->mu_.unlock(); } }; template -class ValueRecorder final : public SynchronousInstrument +class ValueRecorder final : public SynchronousInstrument, public metrics_api::ValueRecorder { - + public: ValueRecorder() = default; - + ValueRecorder(nostd::string_view name, nostd::string_view description, nostd::string_view unit, bool enabled): - SynchronousInstrument(name, description, unit, enabled, metrics_api::InstrumentKind::IntValueRecorder) + SynchronousInstrument(name, description, unit, enabled, metrics_api::InstrumentKind::ValueRecorder) {} - + /* * Bind creates a bound instrument for this counter. The labels are * associated with values recorded via subsequent calls to Record. @@ -243,12 +279,12 @@ class ValueRecorder final : public SynchronousInstrument * @param labels the set of labels, as key-value pairs. * @return a BoundIntCounter tied to the specified labels */ - std::shared_ptr> bind(const std::map &labels) + nostd::shared_ptr> bindValueRecorder(const trace::KeyValueIterable &labels) override { - std::string labelset = mapToString(labels); // COULD CUSTOM HASH THIS INSTEAD FOR PERFORMANCE + std::string labelset = KvToString(labels); // COULD CUSTOM HASH THIS INSTEAD FOR PERFORMANCE if (boundInstruments_.find(labelset) == boundInstruments_.end()) { - auto sp1 = std::make_shared>(this->name_, this->description_, this->unit_, this->enabled_); + auto sp1 = nostd::shared_ptr>(new BoundValueRecorder(this->name_, this->description_, this->unit_, this->enabled_)); boundInstruments_[labelset]=sp1; // perhaps use emplace return sp1; } @@ -258,7 +294,7 @@ class ValueRecorder final : public SynchronousInstrument return boundInstruments_[labelset]; } } - + /* * Add adds the value to the counter's sum. The labels should contain * the keys and values to be associated with this value. Counters only * accept positive @@ -267,16 +303,30 @@ class ValueRecorder final : public SynchronousInstrument * @param value the numerical representation of the metric being captured * @param labels the set of labels, as key-value pairs */ - void record(T value, const std::map &labels) + void record(T value, const trace::KeyValueIterable &labels) override { this->mu_.lock(); - auto sp = bind(labels); + auto sp = bindValueRecorder(labels); sp->update(value); sp->unbind(); this->mu_.unlock(); } - std::unordered_map>> boundInstruments_; + virtual std::vector GetRecords() override { + std::vector ret; + for (auto x : boundInstruments_){ + auto agg_ptr = dynamic_cast*>(x.second.get())->GetAggregator(); + agg_ptr->checkpoint(); + ret.push_back(Record(x.second->GetName(), x.second->GetDescription(), x.first, agg_ptr)); + } + return ret; + } + + virtual void update(T value, const trace::KeyValueIterable &labels) override { + record(value, labels); + } + + std::unordered_map>> boundInstruments_; }; } diff --git a/sdk/test/metrics/metric_instrument_test.cc b/sdk/test/metrics/metric_instrument_test.cc index 1eab86b68b..ad5eb68b6a 100644 --- a/sdk/test/metrics/metric_instrument_test.cc +++ b/sdk/test/metrics/metric_instrument_test.cc @@ -7,6 +7,10 @@ #include #include #include +#include + +namespace metrics_api = opentelemetry::metrics; + OPENTELEMETRY_BEGIN_NAMESPACE namespace sdk @@ -19,15 +23,15 @@ TEST(Counter, InstrumentFunctions) Counter alpha("enabled", "no description", "unitless", true); Counter beta("not enabled", "some description", "units", false); - EXPECT_EQ(alpha.GetName(), "enabled"); - EXPECT_EQ(alpha.GetDescription(), "no description"); - EXPECT_EQ(alpha.GetUnits(), "unitless"); - EXPECT_EQ(alpha.IsEnabled(), true); + EXPECT_EQ(static_cast(alpha.GetName()), "enabled"); + EXPECT_EQ(static_cast(alpha.GetDescription()), "no description"); + EXPECT_EQ(static_cast(alpha.GetUnits()), "unitless"); + EXPECT_EQ(alpha.IsEnabled(), true); - EXPECT_EQ(beta.GetName(), "not enabled"); - EXPECT_EQ(beta.GetDescription(), "some description"); - EXPECT_EQ(beta.GetUnits(), "units"); - EXPECT_EQ(beta.IsEnabled(), false); + EXPECT_EQ(static_cast(beta.GetName()), "not enabled"); + EXPECT_EQ(static_cast(beta.GetDescription()), "some description"); + EXPECT_EQ(static_cast(beta.GetUnits()), "units"); + EXPECT_EQ(beta.IsEnabled(), false); } TEST(Counter, Binding) @@ -39,12 +43,53 @@ TEST(Counter, Binding) std::map labels2 = {{"key2", "value2"}, {"key3", "value3"}}; std::map labels3 = {{"key3", "value3"}, {"key2", "value2"}}; - auto beta = alpha.bind(labels); - auto gamma = alpha.bind(labels1); - auto delta = alpha.bind(labels1); - auto epsilon = alpha.bind(labels1); - auto zeta = alpha.bind(labels2); - auto eta = alpha.bind(labels3); + + auto labelkv = trace::KeyValueIterableView{labels}; + auto labelkv1 = trace::KeyValueIterableView{labels1}; + auto labelkv2 = trace::KeyValueIterableView{labels2}; + auto labelkv3 = trace::KeyValueIterableView{labels3}; + + auto beta = alpha.bindCounter(labelkv); + auto gamma = alpha.bindCounter(labelkv1); + auto delta = alpha.bindCounter(labelkv1); + auto epsilon = alpha.bindCounter(labelkv1); + auto zeta = alpha.bindCounter(labelkv2); + auto eta = alpha.bindCounter(labelkv3); + + EXPECT_EQ (beta->get_ref(), 1); + EXPECT_EQ(gamma->get_ref(),3); + EXPECT_EQ(eta->get_ref(),2); + + delta->unbind(); + gamma->unbind(); + epsilon->unbind(); + + EXPECT_EQ(alpha.boundInstruments_[KvToString(labelkv1)]->get_ref(), 0); + EXPECT_EQ(alpha.boundInstruments_.size(), 3); +} + +TEST(Counter, getAggsandnewupdate) +{ + Counter alpha("test", "none", "unitless", true); + + std::map labels = {{"key", "value"}}; + std::map labels1 = {{"key1", "value1"}}; + + //labels 2 and 3 are actually the same + std::map labels2 = {{"key2", "value2"}, {"key3", "value3"}}; + std::map labels3 = {{"key3", "value3"}, {"key2", "value2"}}; + + auto labelkv = trace::KeyValueIterableView{labels}; + auto labelkv1 = trace::KeyValueIterableView{labels1}; + auto labelkv2 = trace::KeyValueIterableView{labels2}; + auto labelkv3 = trace::KeyValueIterableView{labels3}; + + auto beta = alpha.bindCounter(labelkv); + auto gamma = alpha.bindCounter(labelkv1); + auto delta = alpha.bindCounter(labelkv1); + auto epsilon = alpha.bindCounter(labelkv1); + auto zeta = alpha.bindCounter(labelkv2); + auto eta = alpha.bindCounter(labelkv3); EXPECT_EQ (beta->get_ref(), 1); EXPECT_EQ(gamma->get_ref(),3); @@ -54,11 +99,18 @@ TEST(Counter, Binding) gamma->unbind(); epsilon->unbind(); - EXPECT_EQ(alpha.boundInstruments_[mapToString(labels1)]->get_ref(), 0); + EXPECT_EQ(alpha.boundInstruments_[KvToString(labelkv1)]->get_ref(), 0); EXPECT_EQ(alpha.boundInstruments_.size(), 3); + + auto theta = alpha.GetRecords(); + EXPECT_EQ(theta.size(),3); + EXPECT_EQ(theta[0].GetName(), "test"); + EXPECT_EQ(theta[0].GetDescription(), "none"); + EXPECT_EQ(theta[0].GetLabels(), "{\"key2\":\"value2\",\"key3\":\"value3\"}"); + EXPECT_EQ(theta[1].GetLabels(), "{\"key1\":\"value1\"}"); } -void CounterCallback(std::shared_ptr> in, int freq, std::map labels){ +void CounterCallback(std::shared_ptr> in, int freq, const trace::KeyValueIterable &labels){ for (int i=0; iadd(1, labels); } @@ -70,25 +122,28 @@ TEST(Counter, StressAdd){ std::map labels = {{"key", "value"}}; std::map labels1 = {{"key1", "value1"}}; - std::thread first (CounterCallback, alpha, 100000, labels); - std::thread second (CounterCallback, alpha, 100000, labels); - std::thread third (CounterCallback, alpha, 300000, labels1); + auto labelkv = trace::KeyValueIterableView{labels}; + auto labelkv1 = trace::KeyValueIterableView{labels1}; - first.join(); + std::thread first (CounterCallback, alpha, 100000, labelkv); + std::thread second (CounterCallback, alpha, 100000, labelkv); + std::thread third (CounterCallback, alpha, 300000, labelkv1); + + first.join(); second.join(); third.join(); - EXPECT_EQ(alpha->boundInstruments_[mapToString(labels)]->get_aggregator()->get_values()[0], 200000); - EXPECT_EQ(alpha->boundInstruments_[mapToString(labels1)]->get_aggregator()->get_values()[0], 300000); + EXPECT_EQ(dynamic_cast*>(alpha->boundInstruments_[KvToString(labelkv)].get())->GetAggregator()->get_values()[0], 200000); + EXPECT_EQ(dynamic_cast*>(alpha->boundInstruments_[KvToString(labelkv1)].get())->GetAggregator()->get_values()[0], 300000); } -void UpDownCounterCallback(std::shared_ptr> in, int freq, std::map labels){ +void UpDownCounterCallback(std::shared_ptr> in, int freq, const trace::KeyValueIterable &labels){ for (int i=0; iadd(1, labels); } } -void NegUpDownCounterCallback(std::shared_ptr> in, int freq, std::map labels){ +void NegUpDownCounterCallback(std::shared_ptr> in, int freq, const trace::KeyValueIterable &labels){ for (int i=0; iadd(-1, labels); } @@ -99,28 +154,30 @@ TEST(IntUpDownCounter, StressAdd){ std::map labels = {{"key", "value"}}; std::map labels1 = {{"key1", "value1"}}; + auto labelkv = trace::KeyValueIterableView{labels}; + auto labelkv1 = trace::KeyValueIterableView{labels1}; - std::thread first (UpDownCounterCallback, alpha, 123400, labels); // spawn new threads that call the callback - std::thread second (UpDownCounterCallback, alpha, 123400, labels); - std::thread third (UpDownCounterCallback, alpha, 567800, labels1); - std::thread fourth (NegUpDownCounterCallback, alpha, 123400, labels1); // negative values + std::thread first (UpDownCounterCallback, alpha, 123400, labelkv); // spawn new threads that call the callback + std::thread second (UpDownCounterCallback, alpha, 123400, labelkv); + std::thread third (UpDownCounterCallback, alpha, 567800, labelkv1); + std::thread fourth (NegUpDownCounterCallback, alpha, 123400, labelkv1); // negative values - first.join(); - second.join(); + first.join(); + second.join(); third.join(); fourth.join(); - EXPECT_EQ(alpha->boundInstruments_[mapToString(labels)]->get_aggregator()->get_values()[0], 123400*2); - EXPECT_EQ(alpha->boundInstruments_[mapToString(labels1)]->get_aggregator()->get_values()[0], 567800-123400); + EXPECT_EQ(dynamic_cast*>(alpha->boundInstruments_[KvToString(labelkv)].get())->GetAggregator()->get_values()[0], 123400*2); + EXPECT_EQ(dynamic_cast*>(alpha->boundInstruments_[KvToString(labelkv1)].get())->GetAggregator()->get_values()[0], 567800-123400); } -void RecorderCallback(std::shared_ptr> in, int freq, std::map labels){ +void RecorderCallback(std::shared_ptr> in, int freq, const trace::KeyValueIterable &labels){ for (int i=0; irecord(i, labels); } } -void NegRecorderCallback(std::shared_ptr> in, int freq, std::map labels){ +void NegRecorderCallback(std::shared_ptr> in, int freq, const trace::KeyValueIterable &labels){ for (int i=0; irecord(-i, labels); } @@ -131,26 +188,28 @@ TEST(IntValueRecorder, StressRecord){ std::map labels = {{"key", "value"}}; std::map labels1 = {{"key1", "value1"}}; + auto labelkv = trace::KeyValueIterableView{labels}; + auto labelkv1 = trace::KeyValueIterableView{labels1}; - std::thread first (RecorderCallback, alpha, 25, labels); // spawn new threads that call the callback - std::thread second (RecorderCallback, alpha, 50, labels); - std::thread third (RecorderCallback, alpha, 25, labels1); - std::thread fourth (NegRecorderCallback, alpha, 100, labels1); // negative values + std::thread first (RecorderCallback, alpha, 25, labelkv); // spawn new threads that call the callback + std::thread second (RecorderCallback, alpha, 50, labelkv); + std::thread third (RecorderCallback, alpha, 25, labelkv1); + std::thread fourth (NegRecorderCallback, alpha, 100, labelkv1); // negative values - first.join(); - second.join(); + first.join(); + second.join(); third.join(); fourth.join(); - EXPECT_EQ(alpha->boundInstruments_[mapToString(labels)]->get_aggregator()->get_values()[0], 0); // min - EXPECT_EQ(alpha->boundInstruments_[mapToString(labels)]->get_aggregator()->get_values()[1], 49); // max - EXPECT_EQ(alpha->boundInstruments_[mapToString(labels)]->get_aggregator()->get_values()[2], 1525); // sum - EXPECT_EQ(alpha->boundInstruments_[mapToString(labels)]->get_aggregator()->get_values()[3], 75); // count + EXPECT_EQ(dynamic_cast*>(alpha->boundInstruments_[KvToString(labelkv)].get())->GetAggregator()->get_values()[0], 0); // min + EXPECT_EQ(dynamic_cast*>(alpha->boundInstruments_[KvToString(labelkv)].get())->GetAggregator()->get_values()[1], 49); // max + EXPECT_EQ(dynamic_cast*>(alpha->boundInstruments_[KvToString(labelkv)].get())->GetAggregator()->get_values()[2], 1525); // sum + EXPECT_EQ(dynamic_cast*>(alpha->boundInstruments_[KvToString(labelkv)].get())->GetAggregator()->get_values()[3], 75); // count - EXPECT_EQ(alpha->boundInstruments_[mapToString(labels1)]->get_aggregator()->get_values()[0], -99); // min - EXPECT_EQ(alpha->boundInstruments_[mapToString(labels1)]->get_aggregator()->get_values()[1], 24); // max - EXPECT_EQ(alpha->boundInstruments_[mapToString(labels1)]->get_aggregator()->get_values()[2], -4650); // sum - EXPECT_EQ(alpha->boundInstruments_[mapToString(labels1)]->get_aggregator()->get_values()[3], 125); // count + EXPECT_EQ(dynamic_cast*>(alpha->boundInstruments_[KvToString(labelkv1)].get())->GetAggregator()->get_values()[0], -99); // min + EXPECT_EQ(dynamic_cast*>(alpha->boundInstruments_[KvToString(labelkv1)].get())->GetAggregator()->get_values()[1], 24); // max + EXPECT_EQ(dynamic_cast*>(alpha->boundInstruments_[KvToString(labelkv1)].get())->GetAggregator()->get_values()[2], -4650); // sum + EXPECT_EQ(dynamic_cast*>(alpha->boundInstruments_[KvToString(labelkv1)].get())->GetAggregator()->get_values()[3], 125); // count } } //namespace sdk From 031d9914a515de076aede76c942aa85b5a0dec41 Mon Sep 17 00:00:00 2001 From: Ankit Bhargava Date: Sun, 26 Jul 2020 00:39:51 -0400 Subject: [PATCH 03/16] fixing test formatting --- sdk/test/metrics/metric_instrument_test.cc | 248 ++++++++++----------- 1 file changed, 124 insertions(+), 124 deletions(-) diff --git a/sdk/test/metrics/metric_instrument_test.cc b/sdk/test/metrics/metric_instrument_test.cc index ad5eb68b6a..c2f9dbda0e 100644 --- a/sdk/test/metrics/metric_instrument_test.cc +++ b/sdk/test/metrics/metric_instrument_test.cc @@ -20,88 +20,88 @@ namespace metrics TEST(Counter, InstrumentFunctions) { - Counter alpha("enabled", "no description", "unitless", true); - Counter beta("not enabled", "some description", "units", false); - - EXPECT_EQ(static_cast(alpha.GetName()), "enabled"); - EXPECT_EQ(static_cast(alpha.GetDescription()), "no description"); - EXPECT_EQ(static_cast(alpha.GetUnits()), "unitless"); + Counter alpha("enabled", "no description", "unitless", true); + Counter beta("not enabled", "some description", "units", false); + + EXPECT_EQ(static_cast(alpha.GetName()), "enabled"); + EXPECT_EQ(static_cast(alpha.GetDescription()), "no description"); + EXPECT_EQ(static_cast(alpha.GetUnits()), "unitless"); EXPECT_EQ(alpha.IsEnabled(), true); - - EXPECT_EQ(static_cast(beta.GetName()), "not enabled"); - EXPECT_EQ(static_cast(beta.GetDescription()), "some description"); - EXPECT_EQ(static_cast(beta.GetUnits()), "units"); + + EXPECT_EQ(static_cast(beta.GetName()), "not enabled"); + EXPECT_EQ(static_cast(beta.GetDescription()), "some description"); + EXPECT_EQ(static_cast(beta.GetUnits()), "units"); EXPECT_EQ(beta.IsEnabled(), false); } TEST(Counter, Binding) { - Counter alpha("test", "none", "unitless", true); - - std::map labels = {{"key", "value"}}; - std::map labels1 = {{"key1", "value1"}}; - std::map labels2 = {{"key2", "value2"}, {"key3", "value3"}}; - std::map labels3 = {{"key3", "value3"}, {"key2", "value2"}}; - - - auto labelkv = trace::KeyValueIterableView{labels}; + Counter alpha("test", "none", "unitless", true); + + std::map labels = {{"key", "value"}}; + std::map labels1 = {{"key1", "value1"}}; + std::map labels2 = {{"key2", "value2"}, {"key3", "value3"}}; + std::map labels3 = {{"key3", "value3"}, {"key2", "value2"}}; + + + auto labelkv = trace::KeyValueIterableView{labels}; auto labelkv1 = trace::KeyValueIterableView{labels1}; auto labelkv2 = trace::KeyValueIterableView{labels2}; auto labelkv3 = trace::KeyValueIterableView{labels3}; - - auto beta = alpha.bindCounter(labelkv); - auto gamma = alpha.bindCounter(labelkv1); - auto delta = alpha.bindCounter(labelkv1); - auto epsilon = alpha.bindCounter(labelkv1); - auto zeta = alpha.bindCounter(labelkv2); - auto eta = alpha.bindCounter(labelkv3); - - EXPECT_EQ (beta->get_ref(), 1); - EXPECT_EQ(gamma->get_ref(),3); - EXPECT_EQ(eta->get_ref(),2); - - delta->unbind(); - gamma->unbind(); - epsilon->unbind(); - - EXPECT_EQ(alpha.boundInstruments_[KvToString(labelkv1)]->get_ref(), 0); - EXPECT_EQ(alpha.boundInstruments_.size(), 3); + + auto beta = alpha.bindCounter(labelkv); + auto gamma = alpha.bindCounter(labelkv1); + auto delta = alpha.bindCounter(labelkv1); + auto epsilon = alpha.bindCounter(labelkv1); + auto zeta = alpha.bindCounter(labelkv2); + auto eta = alpha.bindCounter(labelkv3); + + EXPECT_EQ (beta->get_ref(), 1); + EXPECT_EQ(gamma->get_ref(),3); + EXPECT_EQ(eta->get_ref(),2); + + delta->unbind(); + gamma->unbind(); + epsilon->unbind(); + + EXPECT_EQ(alpha.boundInstruments_[KvToString(labelkv1)]->get_ref(), 0); + EXPECT_EQ(alpha.boundInstruments_.size(), 3); } TEST(Counter, getAggsandnewupdate) { - Counter alpha("test", "none", "unitless", true); - - std::map labels = {{"key", "value"}}; - std::map labels1 = {{"key1", "value1"}}; - + Counter alpha("test", "none", "unitless", true); + + std::map labels = {{"key", "value"}}; + std::map labels1 = {{"key1", "value1"}}; + //labels 2 and 3 are actually the same - std::map labels2 = {{"key2", "value2"}, {"key3", "value3"}}; - std::map labels3 = {{"key3", "value3"}, {"key2", "value2"}}; - + std::map labels2 = {{"key2", "value2"}, {"key3", "value3"}}; + std::map labels3 = {{"key3", "value3"}, {"key2", "value2"}}; + auto labelkv = trace::KeyValueIterableView{labels}; auto labelkv1 = trace::KeyValueIterableView{labels1}; auto labelkv2 = trace::KeyValueIterableView{labels2}; auto labelkv3 = trace::KeyValueIterableView{labels3}; - - auto beta = alpha.bindCounter(labelkv); - auto gamma = alpha.bindCounter(labelkv1); - auto delta = alpha.bindCounter(labelkv1); - auto epsilon = alpha.bindCounter(labelkv1); - auto zeta = alpha.bindCounter(labelkv2); - auto eta = alpha.bindCounter(labelkv3); - - EXPECT_EQ (beta->get_ref(), 1); - EXPECT_EQ(gamma->get_ref(),3); - EXPECT_EQ(eta->get_ref(),2); - - delta->unbind(); - gamma->unbind(); - epsilon->unbind(); - - EXPECT_EQ(alpha.boundInstruments_[KvToString(labelkv1)]->get_ref(), 0); - EXPECT_EQ(alpha.boundInstruments_.size(), 3); - + + auto beta = alpha.bindCounter(labelkv); + auto gamma = alpha.bindCounter(labelkv1); + auto delta = alpha.bindCounter(labelkv1); + auto epsilon = alpha.bindCounter(labelkv1); + auto zeta = alpha.bindCounter(labelkv2); + auto eta = alpha.bindCounter(labelkv3); + + EXPECT_EQ (beta->get_ref(), 1); + EXPECT_EQ(gamma->get_ref(),3); + EXPECT_EQ(eta->get_ref(),2); + + delta->unbind(); + gamma->unbind(); + epsilon->unbind(); + + EXPECT_EQ(alpha.boundInstruments_[KvToString(labelkv1)]->get_ref(), 0); + EXPECT_EQ(alpha.boundInstruments_.size(), 3); + auto theta = alpha.GetRecords(); EXPECT_EQ(theta.size(),3); EXPECT_EQ(theta[0].GetName(), "test"); @@ -111,101 +111,101 @@ TEST(Counter, getAggsandnewupdate) } void CounterCallback(std::shared_ptr> in, int freq, const trace::KeyValueIterable &labels){ - for (int i=0; iadd(1, labels); - } + for (int i=0; iadd(1, labels); + } } TEST(Counter, StressAdd){ - std::shared_ptr> alpha(new Counter("test", "none", "unitless", true)); - - std::map labels = {{"key", "value"}}; - std::map labels1 = {{"key1", "value1"}}; - + std::shared_ptr> alpha(new Counter("test", "none", "unitless", true)); + + std::map labels = {{"key", "value"}}; + std::map labels1 = {{"key1", "value1"}}; + auto labelkv = trace::KeyValueIterableView{labels}; auto labelkv1 = trace::KeyValueIterableView{labels1}; - - std::thread first (CounterCallback, alpha, 100000, labelkv); - std::thread second (CounterCallback, alpha, 100000, labelkv); - std::thread third (CounterCallback, alpha, 300000, labelkv1); - - first.join(); - second.join(); - third.join(); - + + std::thread first (CounterCallback, alpha, 100000, labelkv); + std::thread second (CounterCallback, alpha, 100000, labelkv); + std::thread third (CounterCallback, alpha, 300000, labelkv1); + + first.join(); + second.join(); + third.join(); + EXPECT_EQ(dynamic_cast*>(alpha->boundInstruments_[KvToString(labelkv)].get())->GetAggregator()->get_values()[0], 200000); EXPECT_EQ(dynamic_cast*>(alpha->boundInstruments_[KvToString(labelkv1)].get())->GetAggregator()->get_values()[0], 300000); } void UpDownCounterCallback(std::shared_ptr> in, int freq, const trace::KeyValueIterable &labels){ - for (int i=0; iadd(1, labels); - } + for (int i=0; iadd(1, labels); + } } void NegUpDownCounterCallback(std::shared_ptr> in, int freq, const trace::KeyValueIterable &labels){ - for (int i=0; iadd(-1, labels); - } + for (int i=0; iadd(-1, labels); + } } TEST(IntUpDownCounter, StressAdd){ - std::shared_ptr> alpha(new UpDownCounter("test", "none", "unitless", true)); - - std::map labels = {{"key", "value"}}; - std::map labels1 = {{"key1", "value1"}}; + std::shared_ptr> alpha(new UpDownCounter("test", "none", "unitless", true)); + + std::map labels = {{"key", "value"}}; + std::map labels1 = {{"key1", "value1"}}; auto labelkv = trace::KeyValueIterableView{labels}; auto labelkv1 = trace::KeyValueIterableView{labels1}; - - std::thread first (UpDownCounterCallback, alpha, 123400, labelkv); // spawn new threads that call the callback - std::thread second (UpDownCounterCallback, alpha, 123400, labelkv); - std::thread third (UpDownCounterCallback, alpha, 567800, labelkv1); - std::thread fourth (NegUpDownCounterCallback, alpha, 123400, labelkv1); // negative values - - first.join(); - second.join(); - third.join(); - fourth.join(); - + + std::thread first (UpDownCounterCallback, alpha, 123400, labelkv); // spawn new threads that call the callback + std::thread second (UpDownCounterCallback, alpha, 123400, labelkv); + std::thread third (UpDownCounterCallback, alpha, 567800, labelkv1); + std::thread fourth (NegUpDownCounterCallback, alpha, 123400, labelkv1); // negative values + + first.join(); + second.join(); + third.join(); + fourth.join(); + EXPECT_EQ(dynamic_cast*>(alpha->boundInstruments_[KvToString(labelkv)].get())->GetAggregator()->get_values()[0], 123400*2); EXPECT_EQ(dynamic_cast*>(alpha->boundInstruments_[KvToString(labelkv1)].get())->GetAggregator()->get_values()[0], 567800-123400); } void RecorderCallback(std::shared_ptr> in, int freq, const trace::KeyValueIterable &labels){ - for (int i=0; irecord(i, labels); - } + for (int i=0; irecord(i, labels); + } } void NegRecorderCallback(std::shared_ptr> in, int freq, const trace::KeyValueIterable &labels){ - for (int i=0; irecord(-i, labels); - } + for (int i=0; irecord(-i, labels); + } } TEST(IntValueRecorder, StressRecord){ - std::shared_ptr> alpha(new ValueRecorder("test", "none", "unitless", true)); - - std::map labels = {{"key", "value"}}; - std::map labels1 = {{"key1", "value1"}}; + std::shared_ptr> alpha(new ValueRecorder("test", "none", "unitless", true)); + + std::map labels = {{"key", "value"}}; + std::map labels1 = {{"key1", "value1"}}; auto labelkv = trace::KeyValueIterableView{labels}; auto labelkv1 = trace::KeyValueIterableView{labels1}; - - std::thread first (RecorderCallback, alpha, 25, labelkv); // spawn new threads that call the callback - std::thread second (RecorderCallback, alpha, 50, labelkv); - std::thread third (RecorderCallback, alpha, 25, labelkv1); - std::thread fourth (NegRecorderCallback, alpha, 100, labelkv1); // negative values - - first.join(); - second.join(); - third.join(); - fourth.join(); - + + std::thread first (RecorderCallback, alpha, 25, labelkv); // spawn new threads that call the callback + std::thread second (RecorderCallback, alpha, 50, labelkv); + std::thread third (RecorderCallback, alpha, 25, labelkv1); + std::thread fourth (NegRecorderCallback, alpha, 100, labelkv1); // negative values + + first.join(); + second.join(); + third.join(); + fourth.join(); + EXPECT_EQ(dynamic_cast*>(alpha->boundInstruments_[KvToString(labelkv)].get())->GetAggregator()->get_values()[0], 0); // min EXPECT_EQ(dynamic_cast*>(alpha->boundInstruments_[KvToString(labelkv)].get())->GetAggregator()->get_values()[1], 49); // max EXPECT_EQ(dynamic_cast*>(alpha->boundInstruments_[KvToString(labelkv)].get())->GetAggregator()->get_values()[2], 1525); // sum EXPECT_EQ(dynamic_cast*>(alpha->boundInstruments_[KvToString(labelkv)].get())->GetAggregator()->get_values()[3], 75); // count - + EXPECT_EQ(dynamic_cast*>(alpha->boundInstruments_[KvToString(labelkv1)].get())->GetAggregator()->get_values()[0], -99); // min EXPECT_EQ(dynamic_cast*>(alpha->boundInstruments_[KvToString(labelkv1)].get())->GetAggregator()->get_values()[1], 24); // max EXPECT_EQ(dynamic_cast*>(alpha->boundInstruments_[KvToString(labelkv1)].get())->GetAggregator()->get_values()[2], -4650); // sum From ad953f00040ac38cf6a1e74981c74b034825954f Mon Sep 17 00:00:00 2001 From: Ankit Bhargava Date: Sun, 26 Jul 2020 00:40:34 -0400 Subject: [PATCH 04/16] record class --- .../opentelemetry/sdk/metrics/record.h | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100644 sdk/include/opentelemetry/sdk/metrics/record.h diff --git a/sdk/include/opentelemetry/sdk/metrics/record.h b/sdk/include/opentelemetry/sdk/metrics/record.h new file mode 100644 index 0000000000..e7cf6f6067 --- /dev/null +++ b/sdk/include/opentelemetry/sdk/metrics/record.h @@ -0,0 +1,44 @@ +#pragma once + +#include "opentelemetry/metrics/instrument.h" +#include "opentelemetry/nostd/variant.h" +#include "opentelemetry/sdk/metrics/aggregator/aggregator.h" + +OPENTELEMETRY_BEGIN_NAMESPACE + +namespace metrics_api = opentelemetry::metrics; + +namespace sdk +{ +namespace metrics +{ +using AggregatorVariant = nostd::variant>, + nostd::shared_ptr>, + nostd::shared_ptr>, + nostd::shared_ptr>>; +class Record +{ +public: + explicit Record(nostd::string_view name, nostd::string_view description, + std::string labels, AggregatorVariant aggregator) + { + name_ = std::string(name); + description_ = std::string(description); + labels_ = labels; + aggregator_ = aggregator; + } + + std::string GetName() {return name_;} + std::string GetDescription() {return description_;} + std::string GetLabels() {return labels_;} + AggregatorVariant GetAggregator() {return aggregator_;} + +private: + std::string name_; + std::string description_; + std::string labels_; + AggregatorVariant aggregator_; +}; +} // namespace metrics +} // namespace sdk +OPENTELEMETRY_END_NAMESPACE From 33caab8239a33dfc129fd35507e5183c8f40fe18 Mon Sep 17 00:00:00 2001 From: Ankit Bhargava Date: Wed, 29 Jul 2020 16:50:12 -0400 Subject: [PATCH 05/16] improving formatting issues --- .../opentelemetry/sdk/metrics/instrument.h | 39 ++++++++++--------- .../sdk/metrics/sync_instruments.h | 28 ++++++------- 2 files changed, 36 insertions(+), 31 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/metrics/instrument.h b/sdk/include/opentelemetry/sdk/metrics/instrument.h index 3754865db8..1b488a0c41 100644 --- a/sdk/include/opentelemetry/sdk/metrics/instrument.h +++ b/sdk/include/opentelemetry/sdk/metrics/instrument.h @@ -32,12 +32,11 @@ class Instrument : virtual public metrics_api::Instrument { nostd::string_view description, nostd::string_view unit, bool enabled, - metrics_api::InstrumentKind kind): name_(name), description_(description), unit_(unit), enabled_(enabled), kind_(kind) {} + metrics_api::InstrumentKind kind): name_(name), description_(description), unit_(unit), enabled_(enabled), kind_(kind) + {} // Returns true if the instrument is enabled and collecting data - virtual bool IsEnabled() override { - return enabled_; - } + virtual bool IsEnabled() override { return enabled_; } // Return the instrument name virtual nostd::string_view GetName() override { return name_; } @@ -73,7 +72,9 @@ class BoundSynchronousInstrument : public Instrument, virtual public metrics_api metrics_api::InstrumentKind kind, nostd::shared_ptr> agg) :Instrument(name, description, unit, enabled, kind), agg_(agg) - { this->inc_ref(); } // increase reference count when instantiated + { + this->inc_ref(); // increase reference count when instantiated + } /** * Frees the resources associated with this Bound Instrument. @@ -102,14 +103,15 @@ class BoundSynchronousInstrument : public Instrument, virtual public metrics_api virtual int get_ref() override { return ref_; } /** - * Records a single synchronous metric event; a call to the aggregator - * Since this is a bound synchronous instrument, labels are not required in * metric capture - * calls. + * Records a single synchronous metric event via a call to the aggregator. + * Since this is a bound synchronous instrument, labels are not required in + * metric capture calls. * * @param value is the numerical representation of the metric being captured * @return void */ - virtual void update(T value) override { + virtual void update(T value) override + { this->mu_.lock(); agg_->update(value); this->mu_.unlock(); @@ -139,11 +141,12 @@ class SynchronousInstrument : public Instrument, virtual public metrics_api::Syn nostd::string_view description, nostd::string_view unit, bool enabled, - metrics_api::InstrumentKind kind):Instrument(name,description,unit,enabled,kind) + metrics_api::InstrumentKind kind) + :Instrument(name,description,unit,enabled,kind) {} /** - * Returns a Bound Instrument associated with the specified labels. * Multiples requests + * Returns a Bound Instrument associated with the specified labels. Multiples requests * with the same set of labels may return the same Bound Instrument instance. * * It is recommended that callers keep a reference to the Bound Instrument @@ -152,7 +155,8 @@ class SynchronousInstrument : public Instrument, virtual public metrics_api::Syn * @param labels the set of labels, as key-value pairs * @return a Bound Instrument */ - virtual nostd::shared_ptr> bind(const trace::KeyValueIterable &labels) override { + virtual nostd::shared_ptr> bind(const trace::KeyValueIterable &labels) override + { return nostd::shared_ptr>(); } @@ -190,13 +194,14 @@ inline void print_value(std::stringstream &ss, }; // Utility function which converts maps to strings for better performance -inline std::string mapToString(const std::map & conv){ +inline std::string mapToString(const std::map & conv) +{ std::stringstream ss; - ss <<"{ "; + ss << "{ "; for (auto i:conv){ - ss <, public metrics_a nostd::string_view unit, bool enabled): BoundSynchronousInstrument(name, description, unit, enabled, - metrics_api::InstrumentKind::Counter,nostd::shared_ptr>(new - CounterAggregator(metrics_api::InstrumentKind::Counter))) // Aggregator is chosen here + metrics_api::InstrumentKind::Counter, + nostd::shared_ptr>(new CounterAggregator(metrics_api::InstrumentKind::Counter))) // Aggregator is chosen here {} /* @@ -40,7 +40,8 @@ class BoundCounter final: public BoundSynchronousInstrument, public metrics_a * @param value the numerical representation of the metric being captured * @param labels the set of labels, as key-value pairs */ - virtual void add(T value) override { + virtual void add(T value) override + { this->mu_.lock(); if (value < 0){ throw std::invalid_argument("Counter instrument updates must be non-negative."); @@ -145,7 +146,7 @@ class BoundUpDownCounter final: public BoundSynchronousInstrument, virtual pu {} /* - * Add adds the value to the counter's sum. The labels are already linked * to the instrument + * Add adds the value to the counter's sum. The labels are already linked to the instrument * and are not specified. * * @param value the numerical representation of the metric being captured @@ -181,11 +182,11 @@ class UpDownCounter final : public SynchronousInstrument, public metrics_api: */ nostd::shared_ptr> bindUpDownCounter(const trace::KeyValueIterable &labels) override { - std::string labelset = KvToString(labels); // COULD CUSTOM HASH THIS INSTEAD FOR PERFORMANCE + std::string labelset = KvToString(labels); if (boundInstruments_.find(labelset) == boundInstruments_.end()) { auto sp1 = nostd::shared_ptr>(new BoundUpDownCounter(this->name_, this->description_, this->unit_, this->enabled_)); - boundInstruments_[labelset]=sp1; // perhaps use emplace + boundInstruments_[labelset]=sp1; return sp1; } else @@ -197,8 +198,8 @@ class UpDownCounter final : public SynchronousInstrument, public metrics_api: /* * Add adds the value to the counter's sum. The labels should contain - * the keys and values to be associated with this value. Counters only * accept positive - * valued updates. + * the keys and values to be associated with this value. Counters only + * accept positive valued updates. * * @param value the numerical representation of the metric being captured * @param labels the set of labels, as key-value pairs @@ -241,7 +242,7 @@ class BoundValueRecorder final: public BoundSynchronousInstrument , public me nostd::string_view unit, bool enabled): BoundSynchronousInstrument(name, description, unit, enabled, metrics_api::InstrumentKind::ValueRecorder, - nostd::shared_ptr>(new MinMaxSumCountAggregator(metrics_api::InstrumentKind::ValueRecorder))) // Aggregator is chosen here + nostd::shared_ptr>(new MinMaxSumCountAggregator(metrics_api::InstrumentKind::ValueRecorder))) {} /* @@ -256,6 +257,7 @@ class BoundValueRecorder final: public BoundSynchronousInstrument , public me this->update(value); this->mu_.unlock(); } + }; template @@ -281,11 +283,11 @@ class ValueRecorder final : public SynchronousInstrument, public metrics_api: */ nostd::shared_ptr> bindValueRecorder(const trace::KeyValueIterable &labels) override { - std::string labelset = KvToString(labels); // COULD CUSTOM HASH THIS INSTEAD FOR PERFORMANCE + std::string labelset = KvToString(labels); if (boundInstruments_.find(labelset) == boundInstruments_.end()) { auto sp1 = nostd::shared_ptr>(new BoundValueRecorder(this->name_, this->description_, this->unit_, this->enabled_)); - boundInstruments_[labelset]=sp1; // perhaps use emplace + boundInstruments_[labelset]=sp1; return sp1; } else @@ -297,8 +299,8 @@ class ValueRecorder final : public SynchronousInstrument, public metrics_api: /* * Add adds the value to the counter's sum. The labels should contain - * the keys and values to be associated with this value. Counters only * accept positive - * valued updates. + * the keys and values to be associated with this value. Counters only + * accept positive valued updates. * * @param value the numerical representation of the metric being captured * @param labels the set of labels, as key-value pairs From 725fb66b815d2585e061a0f3d39b15e05fd1690d Mon Sep 17 00:00:00 2001 From: Ankit Bhargava Date: Thu, 30 Jul 2020 10:17:05 -0400 Subject: [PATCH 06/16] storing std types internally --- sdk/include/opentelemetry/sdk/metrics/instrument.h | 6 +++--- sdk/include/opentelemetry/sdk/metrics/record.h | 8 ++++---- sdk/include/opentelemetry/sdk/metrics/sync_instruments.h | 6 +++--- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/metrics/instrument.h b/sdk/include/opentelemetry/sdk/metrics/instrument.h index 1b488a0c41..0bf02b69bb 100644 --- a/sdk/include/opentelemetry/sdk/metrics/instrument.h +++ b/sdk/include/opentelemetry/sdk/metrics/instrument.h @@ -70,7 +70,7 @@ class BoundSynchronousInstrument : public Instrument, virtual public metrics_api nostd::string_view unit, bool enabled, metrics_api::InstrumentKind kind, - nostd::shared_ptr> agg) + std::shared_ptr> agg) :Instrument(name, description, unit, enabled, kind), agg_(agg) { this->inc_ref(); // increase reference count when instantiated @@ -123,10 +123,10 @@ class BoundSynchronousInstrument : public Instrument, virtual public metrics_api * @param none * @return the aggregator assigned to this instrument */ - virtual nostd::shared_ptr> GetAggregator() final { return agg_; } + virtual std::shared_ptr> GetAggregator() final { return agg_; } private: - nostd::shared_ptr> agg_; + std::shared_ptr> agg_; int ref_ = 0; }; diff --git a/sdk/include/opentelemetry/sdk/metrics/record.h b/sdk/include/opentelemetry/sdk/metrics/record.h index e7cf6f6067..3d1284b805 100644 --- a/sdk/include/opentelemetry/sdk/metrics/record.h +++ b/sdk/include/opentelemetry/sdk/metrics/record.h @@ -12,10 +12,10 @@ namespace sdk { namespace metrics { -using AggregatorVariant = nostd::variant>, - nostd::shared_ptr>, - nostd::shared_ptr>, - nostd::shared_ptr>>; +using AggregatorVariant = nostd::variant>, + std::shared_ptr>, + std::shared_ptr>, + std::shared_ptr>>; class Record { public: diff --git a/sdk/include/opentelemetry/sdk/metrics/sync_instruments.h b/sdk/include/opentelemetry/sdk/metrics/sync_instruments.h index 302843d006..c521ccc32c 100644 --- a/sdk/include/opentelemetry/sdk/metrics/sync_instruments.h +++ b/sdk/include/opentelemetry/sdk/metrics/sync_instruments.h @@ -30,7 +30,7 @@ class BoundCounter final: public BoundSynchronousInstrument, public metrics_a bool enabled): BoundSynchronousInstrument(name, description, unit, enabled, metrics_api::InstrumentKind::Counter, - nostd::shared_ptr>(new CounterAggregator(metrics_api::InstrumentKind::Counter))) // Aggregator is chosen here + std::shared_ptr>(new CounterAggregator(metrics_api::InstrumentKind::Counter))) // Aggregator is chosen here {} /* @@ -142,7 +142,7 @@ class BoundUpDownCounter final: public BoundSynchronousInstrument, virtual pu bool enabled): BoundSynchronousInstrument(name, description, unit, enabled, metrics_api::InstrumentKind::UpDownCounter, - nostd::shared_ptr>(new CounterAggregator(metrics_api::InstrumentKind::UpDownCounter))) + std::shared_ptr>(new CounterAggregator(metrics_api::InstrumentKind::UpDownCounter))) {} /* @@ -242,7 +242,7 @@ class BoundValueRecorder final: public BoundSynchronousInstrument , public me nostd::string_view unit, bool enabled): BoundSynchronousInstrument(name, description, unit, enabled, metrics_api::InstrumentKind::ValueRecorder, - nostd::shared_ptr>(new MinMaxSumCountAggregator(metrics_api::InstrumentKind::ValueRecorder))) + std::shared_ptr>(new MinMaxSumCountAggregator(metrics_api::InstrumentKind::ValueRecorder))) {} /* From 0dd47de3acdce8fbd9dabca64651dcde44ba4320 Mon Sep 17 00:00:00 2001 From: Ankit Bhargava Date: Thu, 30 Jul 2020 10:23:47 -0400 Subject: [PATCH 07/16] test formatting --- sdk/test/metrics/metric_instrument_test.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/sdk/test/metrics/metric_instrument_test.cc b/sdk/test/metrics/metric_instrument_test.cc index c2f9dbda0e..fe3be5ef74 100644 --- a/sdk/test/metrics/metric_instrument_test.cc +++ b/sdk/test/metrics/metric_instrument_test.cc @@ -57,8 +57,8 @@ TEST(Counter, Binding) auto eta = alpha.bindCounter(labelkv3); EXPECT_EQ (beta->get_ref(), 1); - EXPECT_EQ(gamma->get_ref(),3); - EXPECT_EQ(eta->get_ref(),2); + EXPECT_EQ(gamma->get_ref(), 3); + EXPECT_EQ(eta->get_ref(), 2); delta->unbind(); gamma->unbind(); @@ -92,8 +92,8 @@ TEST(Counter, getAggsandnewupdate) auto eta = alpha.bindCounter(labelkv3); EXPECT_EQ (beta->get_ref(), 1); - EXPECT_EQ(gamma->get_ref(),3); - EXPECT_EQ(eta->get_ref(),2); + EXPECT_EQ(gamma->get_ref(), 3); + EXPECT_EQ(eta->get_ref(), 2); delta->unbind(); gamma->unbind(); From bf718370e9a33c6bf61f38b662268bd5a9d472a5 Mon Sep 17 00:00:00 2001 From: Ankit Bhargava Date: Thu, 30 Jul 2020 22:32:59 -0400 Subject: [PATCH 08/16] pruning unreferenced instruments --- .../opentelemetry/sdk/metrics/instrument.h | 3 +- .../sdk/metrics/sync_instruments.h | 31 ++++++++++++++----- 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/metrics/instrument.h b/sdk/include/opentelemetry/sdk/metrics/instrument.h index 0bf02b69bb..3d607e382c 100644 --- a/sdk/include/opentelemetry/sdk/metrics/instrument.h +++ b/sdk/include/opentelemetry/sdk/metrics/instrument.h @@ -164,7 +164,8 @@ class SynchronousInstrument : public Instrument, virtual public metrics_api::Syn /** * Checkpoints instruments and returns a set of records which are ready for processing. - * This method should only be called by the Meter Class as part of the export pipeline. + * This method should ONLY be called by the Meter Class as part of the export pipeline + * as it also prunes bound instruments with no active references. * * @param none * @return vector of Records which hold the data attached to this synchronous instrument diff --git a/sdk/include/opentelemetry/sdk/metrics/sync_instruments.h b/sdk/include/opentelemetry/sdk/metrics/sync_instruments.h index c521ccc32c..048908d842 100644 --- a/sdk/include/opentelemetry/sdk/metrics/sync_instruments.h +++ b/sdk/include/opentelemetry/sdk/metrics/sync_instruments.h @@ -113,17 +113,20 @@ class Counter final : public SynchronousInstrument, public metrics_api::Count virtual std::vector GetRecords() override { std::vector ret; - for (auto x : boundInstruments_){ + std::vector toDelete; + for (const auto &x : boundInstruments_){ + if (x.second->get_ref() == 0){ + toDelete.push_back(x.first); + } auto agg_ptr = dynamic_cast*>(x.second.get())->GetAggregator(); agg_ptr->checkpoint(); ret.push_back(Record(x.second->GetName(), x.second->GetDescription(), x.first, agg_ptr)); } + for (const auto &x : toDelete){ + boundInstruments_.erase(x); + } return ret; } - - virtual void update(T value, const trace::KeyValueIterable &labels) override { - add(value, labels); - } // A collection of the bound instruments created by this unbound instrument identified by their labels. std::unordered_map>> boundInstruments_; @@ -215,11 +218,18 @@ class UpDownCounter final : public SynchronousInstrument, public metrics_api: virtual std::vector GetRecords() override { std::vector ret; - for (auto x : boundInstruments_){ + std::vector toDelete; + for (const auto &x : boundInstruments_){ + if (x.second->get_ref() == 0){ + toDelete.push_back(x.first); + } auto agg_ptr = dynamic_cast*>(x.second.get())->GetAggregator(); agg_ptr->checkpoint(); ret.push_back(Record(x.second->GetName(), x.second->GetDescription(), x.first, agg_ptr)); } + for (const auto &x : toDelete){ + boundInstruments_.erase(x); + } return ret; } @@ -316,11 +326,18 @@ class ValueRecorder final : public SynchronousInstrument, public metrics_api: virtual std::vector GetRecords() override { std::vector ret; - for (auto x : boundInstruments_){ + std::vector toDelete; + for (const auto &x : boundInstruments_){ + if (x.second->get_ref() == 0){ + toDelete.push_back(x.first); + } auto agg_ptr = dynamic_cast*>(x.second.get())->GetAggregator(); agg_ptr->checkpoint(); ret.push_back(Record(x.second->GetName(), x.second->GetDescription(), x.first, agg_ptr)); } + for (const auto &x : toDelete){ + boundInstruments_.erase(x); + } return ret; } From 8392e4ad8b1fad11a5776c47b8b1f017ca7b15d3 Mon Sep 17 00:00:00 2001 From: Ankit Bhargava Date: Wed, 15 Jul 2020 14:08:07 -0400 Subject: [PATCH 09/16] synchronous metric instruments --- sdk/test/metrics/BUILD | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/test/metrics/BUILD b/sdk/test/metrics/BUILD index 48c9276a21..85081d8e18 100644 --- a/sdk/test/metrics/BUILD +++ b/sdk/test/metrics/BUILD @@ -73,4 +73,4 @@ cc_test( "//sdk/src/metrics", "@com_google_googletest//:gtest_main", ], -) \ No newline at end of file +) From 7e248c6d80e809c9147fb34c9bdb40d701d2572b Mon Sep 17 00:00:00 2001 From: Ankit Bhargava Date: Sun, 26 Jul 2020 00:37:16 -0400 Subject: [PATCH 10/16] ABI compatibility compliance --- .../opentelemetry/sdk/metrics/instrument.h | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/sdk/include/opentelemetry/sdk/metrics/instrument.h b/sdk/include/opentelemetry/sdk/metrics/instrument.h index 3d607e382c..5dd98df277 100644 --- a/sdk/include/opentelemetry/sdk/metrics/instrument.h +++ b/sdk/include/opentelemetry/sdk/metrics/instrument.h @@ -229,6 +229,31 @@ inline std::string KvToString(const trace::KeyValueIterable &kv) noexcept return ss.str(); } +inline std::string KvToString(const trace::KeyValueIterable &kv) noexcept +{ + std::stringstream ss; + ss << "{"; + size_t size = kv.size(); + if (size) + { + size_t i = 1; + // TODO: we need to do something with this iterator. It is not very convenient. + // Having range-based for loop would've been nicer + kv.ForEachKeyValue([&](nostd::string_view key, common::AttributeValue value) noexcept { + ss << "\"" << key << "\":"; + print_value(ss, value, true); + if (size != i) + { + ss << ","; + } + i++; + return true; + }); + }; + ss << "}"; + return ss.str(); +} + } // namespace metrics } // namespace sdk OPENTELEMETRY_END_NAMESPACE From e03613bb6f42455a71a66493bc3a848a52ba5045 Mon Sep 17 00:00:00 2001 From: Ankit Bhargava Date: Wed, 29 Jul 2020 16:50:12 -0400 Subject: [PATCH 11/16] improving formatting issues --- .../opentelemetry/sdk/metrics/instrument.h | 25 ------------------- 1 file changed, 25 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/metrics/instrument.h b/sdk/include/opentelemetry/sdk/metrics/instrument.h index 5dd98df277..3d607e382c 100644 --- a/sdk/include/opentelemetry/sdk/metrics/instrument.h +++ b/sdk/include/opentelemetry/sdk/metrics/instrument.h @@ -229,31 +229,6 @@ inline std::string KvToString(const trace::KeyValueIterable &kv) noexcept return ss.str(); } -inline std::string KvToString(const trace::KeyValueIterable &kv) noexcept -{ - std::stringstream ss; - ss << "{"; - size_t size = kv.size(); - if (size) - { - size_t i = 1; - // TODO: we need to do something with this iterator. It is not very convenient. - // Having range-based for loop would've been nicer - kv.ForEachKeyValue([&](nostd::string_view key, common::AttributeValue value) noexcept { - ss << "\"" << key << "\":"; - print_value(ss, value, true); - if (size != i) - { - ss << ","; - } - i++; - return true; - }); - }; - ss << "}"; - return ss.str(); -} - } // namespace metrics } // namespace sdk OPENTELEMETRY_END_NAMESPACE From 419b9a029c682a55710bb73772bb36d67b28b70a Mon Sep 17 00:00:00 2001 From: Ankit Bhargava Date: Fri, 31 Jul 2020 00:28:07 -0400 Subject: [PATCH 12/16] merge conflict formatting --- sdk/include/opentelemetry/sdk/metrics/instrument.h | 2 +- sdk/include/opentelemetry/sdk/metrics/record.h | 1 + sdk/include/opentelemetry/sdk/metrics/sync_instruments.h | 4 ++++ 3 files changed, 6 insertions(+), 1 deletion(-) diff --git a/sdk/include/opentelemetry/sdk/metrics/instrument.h b/sdk/include/opentelemetry/sdk/metrics/instrument.h index 3d607e382c..1edd519784 100644 --- a/sdk/include/opentelemetry/sdk/metrics/instrument.h +++ b/sdk/include/opentelemetry/sdk/metrics/instrument.h @@ -198,7 +198,7 @@ inline void print_value(std::stringstream &ss, inline std::string mapToString(const std::map & conv) { std::stringstream ss; - ss << "{ "; + ss << "{"; for (auto i:conv){ ss << i.first << ':' << i.second << ','; } diff --git a/sdk/include/opentelemetry/sdk/metrics/record.h b/sdk/include/opentelemetry/sdk/metrics/record.h index 3d1284b805..1ead4234d6 100644 --- a/sdk/include/opentelemetry/sdk/metrics/record.h +++ b/sdk/include/opentelemetry/sdk/metrics/record.h @@ -1,5 +1,6 @@ #pragma once +#include #include "opentelemetry/metrics/instrument.h" #include "opentelemetry/nostd/variant.h" #include "opentelemetry/sdk/metrics/aggregator/aggregator.h" diff --git a/sdk/include/opentelemetry/sdk/metrics/sync_instruments.h b/sdk/include/opentelemetry/sdk/metrics/sync_instruments.h index 048908d842..346b185e81 100644 --- a/sdk/include/opentelemetry/sdk/metrics/sync_instruments.h +++ b/sdk/include/opentelemetry/sdk/metrics/sync_instruments.h @@ -127,6 +127,10 @@ class Counter final : public SynchronousInstrument, public metrics_api::Count } return ret; } + + virtual void update(T val, const trace::KeyValueIterable &labels) override { + add(val, labels); + } // A collection of the bound instruments created by this unbound instrument identified by their labels. std::unordered_map>> boundInstruments_; From abbecc386f413be6b38a60d3657b4089569ec60a Mon Sep 17 00:00:00 2001 From: Ankit Bhargava Date: Fri, 31 Jul 2020 00:41:08 -0400 Subject: [PATCH 13/16] pass tests locally --- .../opentelemetry/common/attribute_value.h | 20 +++++++++++++++++++ sdk/test/metrics/metric_instrument_test.cc | 1 - 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/api/include/opentelemetry/common/attribute_value.h b/api/include/opentelemetry/common/attribute_value.h index 6340a4f97c..a4767192e1 100644 --- a/api/include/opentelemetry/common/attribute_value.h +++ b/api/include/opentelemetry/common/attribute_value.h @@ -25,4 +25,24 @@ using AttributeValue = nostd::variant, nostd::span>; } // namespace common + +enum AttributeType +{ + TYPE_BOOL, + TYPE_INT, + TYPE_INT64, + TYPE_UINT, + TYPE_UINT64, + TYPE_DOUBLE, + TYPE_STRING, + TYPE_CSTRING, + // TYPE_SPAN_BYTE, // TODO: not part of OT spec yet + TYPE_SPAN_BOOL, + TYPE_SPAN_INT, + TYPE_SPAN_INT64, + TYPE_SPAN_UINT, + TYPE_SPAN_UINT64, + TYPE_SPAN_DOUBLE, + TYPE_SPAN_STRING +}; OPENTELEMETRY_END_NAMESPACE diff --git a/sdk/test/metrics/metric_instrument_test.cc b/sdk/test/metrics/metric_instrument_test.cc index fe3be5ef74..0919291dfc 100644 --- a/sdk/test/metrics/metric_instrument_test.cc +++ b/sdk/test/metrics/metric_instrument_test.cc @@ -1,6 +1,5 @@ #include #include "opentelemetry/sdk/metrics/sync_instruments.h" -#include "opentelemetry/sdk/metrics/async_instruments.h" #include #include From e0d74572972a49d350ba68fba09b789a6e3b7e8a Mon Sep 17 00:00:00 2001 From: Ankit Bhargava Date: Fri, 31 Jul 2020 00:48:30 -0400 Subject: [PATCH 14/16] formatting test --- .../opentelemetry/common/attribute_value.h | 3 +- .../opentelemetry/sdk/metrics/instrument.h | 376 +++++------ .../opentelemetry/sdk/metrics/record.h | 24 +- .../sdk/metrics/sync_instruments.h | 626 ++++++++++-------- sdk/test/metrics/metric_instrument_test.cc | 413 +++++++----- 5 files changed, 788 insertions(+), 654 deletions(-) diff --git a/api/include/opentelemetry/common/attribute_value.h b/api/include/opentelemetry/common/attribute_value.h index a4767192e1..e68b6d8bb1 100644 --- a/api/include/opentelemetry/common/attribute_value.h +++ b/api/include/opentelemetry/common/attribute_value.h @@ -24,7 +24,6 @@ using AttributeValue = nostd::variant, nostd::span, nostd::span>; -} // namespace common enum AttributeType { @@ -45,4 +44,6 @@ enum AttributeType TYPE_SPAN_DOUBLE, TYPE_SPAN_STRING }; + +} // namespace common OPENTELEMETRY_END_NAMESPACE diff --git a/sdk/include/opentelemetry/sdk/metrics/instrument.h b/sdk/include/opentelemetry/sdk/metrics/instrument.h index 1edd519784..6ba0a30ffd 100644 --- a/sdk/include/opentelemetry/sdk/metrics/instrument.h +++ b/sdk/include/opentelemetry/sdk/metrics/instrument.h @@ -5,173 +5,176 @@ #include "opentelemetry/sdk/metrics/record.h" #include "opentelemetry/version.h" +#include +#include #include -#include -#include #include -#include +#include +#include #include -#include namespace metrics_api = opentelemetry::metrics; -namespace trace_api = opentelemetry::trace; +namespace trace_api = opentelemetry::trace; OPENTELEMETRY_BEGIN_NAMESPACE namespace sdk { -namespace metrics +namespace metrics { +class Instrument : virtual public metrics_api::Instrument +{ -class Instrument : virtual public metrics_api::Instrument { - public: - Instrument() = default; - - Instrument(nostd::string_view name, - nostd::string_view description, - nostd::string_view unit, - bool enabled, - metrics_api::InstrumentKind kind): name_(name), description_(description), unit_(unit), enabled_(enabled), kind_(kind) - {} - - // Returns true if the instrument is enabled and collecting data - virtual bool IsEnabled() override { return enabled_; } - - // Return the instrument name - virtual nostd::string_view GetName() override { return name_; } - - // Return the instrument description - virtual nostd::string_view GetDescription() override { return description_; } - - // Return the insrument's units of measurement - virtual nostd::string_view GetUnits() override { return unit_; } - - virtual metrics_api::InstrumentKind GetKind() override { return this->kind_; } - + Instrument() = default; + + Instrument(nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + bool enabled, + metrics_api::InstrumentKind kind) + : name_(name), description_(description), unit_(unit), enabled_(enabled), kind_(kind) + {} + + // Returns true if the instrument is enabled and collecting data + virtual bool IsEnabled() override { return enabled_; } + + // Return the instrument name + virtual nostd::string_view GetName() override { return name_; } + + // Return the instrument description + virtual nostd::string_view GetDescription() override { return description_; } + + // Return the insrument's units of measurement + virtual nostd::string_view GetUnits() override { return unit_; } + + virtual metrics_api::InstrumentKind GetKind() override { return this->kind_; } + protected: - std::string name_; - std::string description_; - std::string unit_; - bool enabled_; - std::mutex mu_; - metrics_api::InstrumentKind kind_; + std::string name_; + std::string description_; + std::string unit_; + bool enabled_; + std::mutex mu_; + metrics_api::InstrumentKind kind_; }; template -class BoundSynchronousInstrument : public Instrument, virtual public metrics_api::BoundSynchronousInstrument { - +class BoundSynchronousInstrument : public Instrument, + virtual public metrics_api::BoundSynchronousInstrument +{ + public: - - BoundSynchronousInstrument() = default; - - BoundSynchronousInstrument(nostd::string_view name, - nostd::string_view description, - nostd::string_view unit, - bool enabled, - metrics_api::InstrumentKind kind, - std::shared_ptr> agg) - :Instrument(name, description, unit, enabled, kind), agg_(agg) - { - this->inc_ref(); // increase reference count when instantiated - } - - /** - * Frees the resources associated with this Bound Instrument. - * The Metric from which this instrument was created is not impacted. - * - * @param none - * @return void - */ - virtual void unbind() override { ref_ -= 1; } - - /** - * Increments the reference count. This function is used when binding or instantiating. - * - * @param none - * @return void - */ - virtual void inc_ref() override { ref_ += 1; } - - /** - * Returns the current reference count of the instrument. This value is used to - * later in the pipeline remove stale instruments. - * - * @param none - * @return current ref count of the instrument - */ - virtual int get_ref() override { return ref_; } - - /** - * Records a single synchronous metric event via a call to the aggregator. - * Since this is a bound synchronous instrument, labels are not required in - * metric capture calls. - * - * @param value is the numerical representation of the metric being captured - * @return void - */ - virtual void update(T value) override - { - this->mu_.lock(); - agg_->update(value); - this->mu_.unlock(); - } - - /** - * Returns the aggregator responsible for meaningfully combining update values. - * - * @param none - * @return the aggregator assigned to this instrument - */ - virtual std::shared_ptr> GetAggregator() final { return agg_; } - + BoundSynchronousInstrument() = default; + + BoundSynchronousInstrument(nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + bool enabled, + metrics_api::InstrumentKind kind, + std::shared_ptr> agg) + : Instrument(name, description, unit, enabled, kind), agg_(agg) + { + this->inc_ref(); // increase reference count when instantiated + } + + /** + * Frees the resources associated with this Bound Instrument. + * The Metric from which this instrument was created is not impacted. + * + * @param none + * @return void + */ + virtual void unbind() override { ref_ -= 1; } + + /** + * Increments the reference count. This function is used when binding or instantiating. + * + * @param none + * @return void + */ + virtual void inc_ref() override { ref_ += 1; } + + /** + * Returns the current reference count of the instrument. This value is used to + * later in the pipeline remove stale instruments. + * + * @param none + * @return current ref count of the instrument + */ + virtual int get_ref() override { return ref_; } + + /** + * Records a single synchronous metric event via a call to the aggregator. + * Since this is a bound synchronous instrument, labels are not required in + * metric capture calls. + * + * @param value is the numerical representation of the metric being captured + * @return void + */ + virtual void update(T value) override + { + this->mu_.lock(); + agg_->update(value); + this->mu_.unlock(); + } + + /** + * Returns the aggregator responsible for meaningfully combining update values. + * + * @param none + * @return the aggregator assigned to this instrument + */ + virtual std::shared_ptr> GetAggregator() final { return agg_; } + private: - std::shared_ptr> agg_; - int ref_ = 0; + std::shared_ptr> agg_; + int ref_ = 0; }; template -class SynchronousInstrument : public Instrument, virtual public metrics_api::SynchronousInstrument { - +class SynchronousInstrument : public Instrument, + virtual public metrics_api::SynchronousInstrument +{ + public: - - SynchronousInstrument() = default; - - SynchronousInstrument(nostd::string_view name, - nostd::string_view description, - nostd::string_view unit, - bool enabled, - metrics_api::InstrumentKind kind) - :Instrument(name,description,unit,enabled,kind) - {} - - /** - * Returns a Bound Instrument associated with the specified labels. Multiples requests - * with the same set of labels may return the same Bound Instrument instance. - * - * It is recommended that callers keep a reference to the Bound Instrument - * instead of repeatedly calling this operation. - * - * @param labels the set of labels, as key-value pairs - * @return a Bound Instrument - */ - virtual nostd::shared_ptr> bind(const trace::KeyValueIterable &labels) override - { - return nostd::shared_ptr>(); - } - - virtual void update(T value, const trace::KeyValueIterable &labels) override = 0; - - /** - * Checkpoints instruments and returns a set of records which are ready for processing. - * This method should ONLY be called by the Meter Class as part of the export pipeline - * as it also prunes bound instruments with no active references. - * - * @param none - * @return vector of Records which hold the data attached to this synchronous instrument - */ - virtual std::vector GetRecords() = 0; - + SynchronousInstrument() = default; + + SynchronousInstrument(nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + bool enabled, + metrics_api::InstrumentKind kind) + : Instrument(name, description, unit, enabled, kind) + {} + + /** + * Returns a Bound Instrument associated with the specified labels. Multiples requests + * with the same set of labels may return the same Bound Instrument instance. + * + * It is recommended that callers keep a reference to the Bound Instrument + * instead of repeatedly calling this operation. + * + * @param labels the set of labels, as key-value pairs + * @return a Bound Instrument + */ + virtual nostd::shared_ptr> bind( + const trace::KeyValueIterable &labels) override + { + return nostd::shared_ptr>(); + } + + virtual void update(T value, const trace::KeyValueIterable &labels) override = 0; + + /** + * Checkpoints instruments and returns a set of records which are ready for processing. + * This method should ONLY be called by the Meter Class as part of the export pipeline + * as it also prunes bound instruments with no active references. + * + * @param none + * @return vector of Records which hold the data attached to this synchronous instrument + */ + virtual std::vector GetRecords() = 0; }; // Helper functions for turning a trace::KeyValueIterable into a string @@ -179,56 +182,57 @@ inline void print_value(std::stringstream &ss, common::AttributeValue &value, bool jsonTypes = false) { - switch (value.index()) - { - case common::AttributeType::TYPE_STRING: - if (jsonTypes) - ss << '"'; - ss << nostd::get(value); - if (jsonTypes) - ss << '"'; - break; - default: - throw std::invalid_argument("Labels must be strings"); - break; - } + switch (value.index()) + { + case common::AttributeType::TYPE_STRING: + if (jsonTypes) + ss << '"'; + ss << nostd::get(value); + if (jsonTypes) + ss << '"'; + break; + default: + throw std::invalid_argument("Labels must be strings"); + break; + } }; // Utility function which converts maps to strings for better performance -inline std::string mapToString(const std::map & conv) +inline std::string mapToString(const std::map &conv) { - std::stringstream ss; - ss << "{"; - for (auto i:conv){ - ss << i.first << ':' << i.second << ','; - } - ss << "}"; - return ss.str(); + std::stringstream ss; + ss << "{"; + for (auto i : conv) + { + ss << i.first << ':' << i.second << ','; + } + ss << "}"; + return ss.str(); } inline std::string KvToString(const trace::KeyValueIterable &kv) noexcept { - std::stringstream ss; - ss << "{"; - size_t size = kv.size(); - if (size) - { - size_t i = 1; - kv.ForEachKeyValue([&](nostd::string_view key, common::AttributeValue value) noexcept { - ss << "\"" << key << "\":"; - print_value(ss, value, true); - if (size != i) - { - ss << ","; - } - i++; - return true; - }); - }; - ss << "}"; - return ss.str(); + std::stringstream ss; + ss << "{"; + size_t size = kv.size(); + if (size) + { + size_t i = 1; + kv.ForEachKeyValue([&](nostd::string_view key, common::AttributeValue value) noexcept { + ss << "\"" << key << "\":"; + print_value(ss, value, true); + if (size != i) + { + ss << ","; + } + i++; + return true; + }); + }; + ss << "}"; + return ss.str(); } -} // namespace metrics -} // namespace sdk +} // namespace metrics +} // namespace sdk OPENTELEMETRY_END_NAMESPACE diff --git a/sdk/include/opentelemetry/sdk/metrics/record.h b/sdk/include/opentelemetry/sdk/metrics/record.h index 1ead4234d6..1c646faf2e 100644 --- a/sdk/include/opentelemetry/sdk/metrics/record.h +++ b/sdk/include/opentelemetry/sdk/metrics/record.h @@ -20,19 +20,21 @@ using AggregatorVariant = nostd::variant>, class Record { public: - explicit Record(nostd::string_view name, nostd::string_view description, - std::string labels, AggregatorVariant aggregator) + explicit Record(nostd::string_view name, + nostd::string_view description, + std::string labels, + AggregatorVariant aggregator) { - name_ = std::string(name); + name_ = std::string(name); description_ = std::string(description); - labels_ = labels; - aggregator_ = aggregator; + labels_ = labels; + aggregator_ = aggregator; } - std::string GetName() {return name_;} - std::string GetDescription() {return description_;} - std::string GetLabels() {return labels_;} - AggregatorVariant GetAggregator() {return aggregator_;} + std::string GetName() { return name_; } + std::string GetDescription() { return description_; } + std::string GetLabels() { return labels_; } + AggregatorVariant GetAggregator() { return aggregator_; } private: std::string name_; @@ -40,6 +42,6 @@ class Record std::string labels_; AggregatorVariant aggregator_; }; -} // namespace metrics -} // namespace sdk +} // namespace metrics +} // namespace sdk OPENTELEMETRY_END_NAMESPACE diff --git a/sdk/include/opentelemetry/sdk/metrics/sync_instruments.h b/sdk/include/opentelemetry/sdk/metrics/sync_instruments.h index 346b185e81..824eaa48f4 100644 --- a/sdk/include/opentelemetry/sdk/metrics/sync_instruments.h +++ b/sdk/include/opentelemetry/sdk/metrics/sync_instruments.h @@ -1,56 +1,64 @@ #pragma once -#include "opentelemetry/sdk/metrics/instrument.h" -#include "opentelemetry/sdk/metrics/aggregator/counter_aggregator.h" -#include "opentelemetry/sdk/metrics/aggregator/min_max_sum_count_aggregator.h" -#include "opentelemetry/metrics/sync_instruments.h" -#include #include +#include #include +#include #include -#include +#include "opentelemetry/metrics/sync_instruments.h" +#include "opentelemetry/sdk/metrics/aggregator/counter_aggregator.h" +#include "opentelemetry/sdk/metrics/aggregator/min_max_sum_count_aggregator.h" +#include "opentelemetry/sdk/metrics/instrument.h" namespace metrics_api = opentelemetry::metrics; OPENTELEMETRY_BEGIN_NAMESPACE namespace sdk { -namespace metrics +namespace metrics { template -class BoundCounter final: public BoundSynchronousInstrument, public metrics_api::BoundCounter { +class BoundCounter final : public BoundSynchronousInstrument, public metrics_api::BoundCounter +{ public: - BoundCounter() = default; - - BoundCounter(nostd::string_view name, - nostd::string_view description, - nostd::string_view unit, - bool enabled): - BoundSynchronousInstrument(name, description, unit, enabled, - metrics_api::InstrumentKind::Counter, - std::shared_ptr>(new CounterAggregator(metrics_api::InstrumentKind::Counter))) // Aggregator is chosen here - {} - - /* - * Add adds the value to the counter's sum. The labels are already linked to the instrument - * and are not specified. - * - * @param value the numerical representation of the metric being captured - * @param labels the set of labels, as key-value pairs - */ - virtual void add(T value) override + BoundCounter() = default; + + BoundCounter(nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + bool enabled) + : BoundSynchronousInstrument( + name, + description, + unit, + enabled, + metrics_api::InstrumentKind::Counter, + std::shared_ptr>(new CounterAggregator( + metrics_api::InstrumentKind::Counter))) // Aggregator is chosen here + {} + + /* + * Add adds the value to the counter's sum. The labels are already linked to the instrument + * and are not specified. + * + * @param value the numerical representation of the metric being captured + * @param labels the set of labels, as key-value pairs + */ + virtual void add(T value) override + { + this->mu_.lock(); + if (value < 0) { - this->mu_.lock(); - if (value < 0){ - throw std::invalid_argument("Counter instrument updates must be non-negative."); - } else { - this->update(value); - } - this->mu_.unlock(); + throw std::invalid_argument("Counter instrument updates must be non-negative."); } - + else + { + this->update(value); + } + this->mu_.unlock(); + } }; template @@ -58,112 +66,132 @@ class Counter final : public SynchronousInstrument, public metrics_api::Count { public: - - Counter() = default; - - Counter(nostd::string_view name, - nostd::string_view description, - nostd::string_view unit, - bool enabled): - SynchronousInstrument(name, description, unit, enabled, metrics_api::InstrumentKind::Counter) - {} - - /* - * Bind creates a bound instrument for this counter. The labels are - * associated with values recorded via subsequent calls to Record. - * - * @param labels the set of labels, as key-value pairs. - * @return a BoundCounter tied to the specified labels - */ - - virtual nostd::shared_ptr> bindCounter(const trace::KeyValueIterable &labels) override { - std::string labelset = KvToString(labels); - if (boundInstruments_.find(labelset) == boundInstruments_.end()) - { - auto sp1 = nostd::shared_ptr>(new BoundCounter(this->name_, this->description_, this->unit_, this->enabled_)); - boundInstruments_[labelset]=sp1; - return sp1; - } - else - { - boundInstruments_[labelset]->inc_ref(); - return boundInstruments_[labelset]; - } + Counter() = default; + + Counter(nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + bool enabled) + : SynchronousInstrument(name, + description, + unit, + enabled, + metrics_api::InstrumentKind::Counter) + {} + + /* + * Bind creates a bound instrument for this counter. The labels are + * associated with values recorded via subsequent calls to Record. + * + * @param labels the set of labels, as key-value pairs. + * @return a BoundCounter tied to the specified labels + */ + + virtual nostd::shared_ptr> bindCounter( + const trace::KeyValueIterable &labels) override + { + std::string labelset = KvToString(labels); + if (boundInstruments_.find(labelset) == boundInstruments_.end()) + { + auto sp1 = nostd::shared_ptr>( + new BoundCounter(this->name_, this->description_, this->unit_, this->enabled_)); + boundInstruments_[labelset] = sp1; + return sp1; } - - /* - * Add adds the value to the counter's sum. The labels should contain - * the keys and values to be associated with this value. Counters only - * accept positive valued updates. - * - * @param value the numerical representation of the metric being captured - * @param labels the set of labels, as key-value pairs - */ - virtual void add(T value, const trace::KeyValueIterable &labels) override { - this->mu_.lock(); - if (value < 0){ - throw std::invalid_argument("Counter instrument updates must be non-negative."); - } else { - auto sp = bindCounter(labels); - sp->update(value); - sp->unbind(); - } - this->mu_.unlock(); + else + { + boundInstruments_[labelset]->inc_ref(); + return boundInstruments_[labelset]; } - - virtual std::vector GetRecords() override { - std::vector ret; - std::vector toDelete; - for (const auto &x : boundInstruments_){ - if (x.second->get_ref() == 0){ - toDelete.push_back(x.first); - } - auto agg_ptr = dynamic_cast*>(x.second.get())->GetAggregator(); - agg_ptr->checkpoint(); - ret.push_back(Record(x.second->GetName(), x.second->GetDescription(), x.first, agg_ptr)); - } - for (const auto &x : toDelete){ - boundInstruments_.erase(x); - } - return ret; + } + + /* + * Add adds the value to the counter's sum. The labels should contain + * the keys and values to be associated with this value. Counters only + * accept positive valued updates. + * + * @param value the numerical representation of the metric being captured + * @param labels the set of labels, as key-value pairs + */ + virtual void add(T value, const trace::KeyValueIterable &labels) override + { + this->mu_.lock(); + if (value < 0) + { + throw std::invalid_argument("Counter instrument updates must be non-negative."); + } + else + { + auto sp = bindCounter(labels); + sp->update(value); + sp->unbind(); + } + this->mu_.unlock(); + } + + virtual std::vector GetRecords() override + { + std::vector ret; + std::vector toDelete; + for (const auto &x : boundInstruments_) + { + if (x.second->get_ref() == 0) + { + toDelete.push_back(x.first); + } + auto agg_ptr = dynamic_cast *>(x.second.get())->GetAggregator(); + agg_ptr->checkpoint(); + ret.push_back(Record(x.second->GetName(), x.second->GetDescription(), x.first, agg_ptr)); } - - virtual void update(T val, const trace::KeyValueIterable &labels) override { - add(val, labels); + for (const auto &x : toDelete) + { + boundInstruments_.erase(x); } + return ret; + } - // A collection of the bound instruments created by this unbound instrument identified by their labels. - std::unordered_map>> boundInstruments_; -}; + virtual void update(T val, const trace::KeyValueIterable &labels) override { add(val, labels); } + // A collection of the bound instruments created by this unbound instrument identified by their + // labels. + std::unordered_map>> + boundInstruments_; +}; template -class BoundUpDownCounter final: public BoundSynchronousInstrument, virtual public metrics_api::BoundUpDownCounter { +class BoundUpDownCounter final : public BoundSynchronousInstrument, + virtual public metrics_api::BoundUpDownCounter +{ public: - BoundUpDownCounter() = default; - - BoundUpDownCounter(nostd::string_view name, - nostd::string_view description, - nostd::string_view unit, - bool enabled): - BoundSynchronousInstrument(name, description, unit, enabled, - metrics_api::InstrumentKind::UpDownCounter, - std::shared_ptr>(new CounterAggregator(metrics_api::InstrumentKind::UpDownCounter))) - {} - - /* - * Add adds the value to the counter's sum. The labels are already linked to the instrument - * and are not specified. - * - * @param value the numerical representation of the metric being captured - * @param labels the set of labels, as key-value pairs - */ - virtual void add(T value) override { - this->mu_.lock(); - this->update(value); - this->mu_.unlock(); - } + BoundUpDownCounter() = default; + + BoundUpDownCounter(nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + bool enabled) + : BoundSynchronousInstrument(name, + description, + unit, + enabled, + metrics_api::InstrumentKind::UpDownCounter, + std::shared_ptr>(new CounterAggregator( + metrics_api::InstrumentKind::UpDownCounter))) + {} + + /* + * Add adds the value to the counter's sum. The labels are already linked to the instrument + * and are not specified. + * + * @param value the numerical representation of the metric being captured + * @param labels the set of labels, as key-value pairs + */ + virtual void add(T value) override + { + this->mu_.lock(); + this->update(value); + this->mu_.unlock(); + } }; template @@ -171,107 +199,123 @@ class UpDownCounter final : public SynchronousInstrument, public metrics_api: { public: - UpDownCounter() = default; - - UpDownCounter(nostd::string_view name, - nostd::string_view description, - nostd::string_view unit, - bool enabled): - SynchronousInstrument(name, description, unit, enabled, metrics_api::InstrumentKind::UpDownCounter) - {} - - /* - * Bind creates a bound instrument for this counter. The labels are - * associated with values recorded via subsequent calls to Record. - * - * @param labels the set of labels, as key-value pairs. - * @return a BoundIntCounter tied to the specified labels - */ - nostd::shared_ptr> bindUpDownCounter(const trace::KeyValueIterable &labels) override + UpDownCounter() = default; + + UpDownCounter(nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + bool enabled) + : SynchronousInstrument(name, + description, + unit, + enabled, + metrics_api::InstrumentKind::UpDownCounter) + {} + + /* + * Bind creates a bound instrument for this counter. The labels are + * associated with values recorded via subsequent calls to Record. + * + * @param labels the set of labels, as key-value pairs. + * @return a BoundIntCounter tied to the specified labels + */ + nostd::shared_ptr> bindUpDownCounter( + const trace::KeyValueIterable &labels) override + { + std::string labelset = KvToString(labels); + if (boundInstruments_.find(labelset) == boundInstruments_.end()) { - std::string labelset = KvToString(labels); - if (boundInstruments_.find(labelset) == boundInstruments_.end()) - { - auto sp1 = nostd::shared_ptr>(new BoundUpDownCounter(this->name_, this->description_, this->unit_, this->enabled_)); - boundInstruments_[labelset]=sp1; - return sp1; - } - else - { - boundInstruments_[labelset]->inc_ref(); - return boundInstruments_[labelset]; - } + auto sp1 = nostd::shared_ptr>( + new BoundUpDownCounter(this->name_, this->description_, this->unit_, this->enabled_)); + boundInstruments_[labelset] = sp1; + return sp1; } - - /* - * Add adds the value to the counter's sum. The labels should contain - * the keys and values to be associated with this value. Counters only - * accept positive valued updates. - * - * @param value the numerical representation of the metric being captured - * @param labels the set of labels, as key-value pairs - */ - void add(T value, const trace::KeyValueIterable &labels) override + else { - this->mu_.lock(); - auto sp = bindUpDownCounter(labels); - sp->update(value); - sp->unbind(); - this->mu_.unlock(); + boundInstruments_[labelset]->inc_ref(); + return boundInstruments_[labelset]; } - - virtual std::vector GetRecords() override { - std::vector ret; - std::vector toDelete; - for (const auto &x : boundInstruments_){ - if (x.second->get_ref() == 0){ - toDelete.push_back(x.first); - } - auto agg_ptr = dynamic_cast*>(x.second.get())->GetAggregator(); - agg_ptr->checkpoint(); - ret.push_back(Record(x.second->GetName(), x.second->GetDescription(), x.first, agg_ptr)); - } - for (const auto &x : toDelete){ - boundInstruments_.erase(x); - } - return ret; + } + + /* + * Add adds the value to the counter's sum. The labels should contain + * the keys and values to be associated with this value. Counters only + * accept positive valued updates. + * + * @param value the numerical representation of the metric being captured + * @param labels the set of labels, as key-value pairs + */ + void add(T value, const trace::KeyValueIterable &labels) override + { + this->mu_.lock(); + auto sp = bindUpDownCounter(labels); + sp->update(value); + sp->unbind(); + this->mu_.unlock(); + } + + virtual std::vector GetRecords() override + { + std::vector ret; + std::vector toDelete; + for (const auto &x : boundInstruments_) + { + if (x.second->get_ref() == 0) + { + toDelete.push_back(x.first); + } + auto agg_ptr = dynamic_cast *>(x.second.get())->GetAggregator(); + agg_ptr->checkpoint(); + ret.push_back(Record(x.second->GetName(), x.second->GetDescription(), x.first, agg_ptr)); } - - virtual void update(T val, const trace::KeyValueIterable &labels) override { - add(val, labels); + for (const auto &x : toDelete) + { + boundInstruments_.erase(x); } + return ret; + } + virtual void update(T val, const trace::KeyValueIterable &labels) override { add(val, labels); } - std::unordered_map>> boundInstruments_; + std::unordered_map>> + boundInstruments_; }; template -class BoundValueRecorder final: public BoundSynchronousInstrument , public metrics_api::BoundValueRecorder{ +class BoundValueRecorder final : public BoundSynchronousInstrument, + public metrics_api::BoundValueRecorder +{ public: - BoundValueRecorder() = default; - - BoundValueRecorder(nostd::string_view name, - nostd::string_view description, - nostd::string_view unit, - bool enabled): BoundSynchronousInstrument(name, description, unit, enabled, - metrics_api::InstrumentKind::ValueRecorder, - std::shared_ptr>(new MinMaxSumCountAggregator(metrics_api::InstrumentKind::ValueRecorder))) - {} - - /* - * Add adds the value to the counter's sum. The labels are already linked to the instrument - * and are not specified. - * - * @param value the numerical representation of the metric being captured - * @param labels the set of labels, as key-value pairs - */ - void record(T value) { - this->mu_.lock(); - this->update(value); - this->mu_.unlock(); - } - + BoundValueRecorder() = default; + + BoundValueRecorder(nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + bool enabled) + : BoundSynchronousInstrument( + name, + description, + unit, + enabled, + metrics_api::InstrumentKind::ValueRecorder, + std::shared_ptr>( + new MinMaxSumCountAggregator(metrics_api::InstrumentKind::ValueRecorder))) + {} + + /* + * Add adds the value to the counter's sum. The labels are already linked to the instrument + * and are not specified. + * + * @param value the numerical representation of the metric being captured + * @param labels the set of labels, as key-value pairs + */ + void record(T value) + { + this->mu_.lock(); + this->update(value); + this->mu_.unlock(); + } }; template @@ -279,79 +323,91 @@ class ValueRecorder final : public SynchronousInstrument, public metrics_api: { public: - ValueRecorder() = default; - - ValueRecorder(nostd::string_view name, - nostd::string_view description, - nostd::string_view unit, - bool enabled): - SynchronousInstrument(name, description, unit, enabled, metrics_api::InstrumentKind::ValueRecorder) - {} - - /* - * Bind creates a bound instrument for this counter. The labels are - * associated with values recorded via subsequent calls to Record. - * - * @param labels the set of labels, as key-value pairs. - * @return a BoundIntCounter tied to the specified labels - */ - nostd::shared_ptr> bindValueRecorder(const trace::KeyValueIterable &labels) override + ValueRecorder() = default; + + ValueRecorder(nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + bool enabled) + : SynchronousInstrument(name, + description, + unit, + enabled, + metrics_api::InstrumentKind::ValueRecorder) + {} + + /* + * Bind creates a bound instrument for this counter. The labels are + * associated with values recorded via subsequent calls to Record. + * + * @param labels the set of labels, as key-value pairs. + * @return a BoundIntCounter tied to the specified labels + */ + nostd::shared_ptr> bindValueRecorder( + const trace::KeyValueIterable &labels) override + { + std::string labelset = KvToString(labels); + if (boundInstruments_.find(labelset) == boundInstruments_.end()) { - std::string labelset = KvToString(labels); - if (boundInstruments_.find(labelset) == boundInstruments_.end()) - { - auto sp1 = nostd::shared_ptr>(new BoundValueRecorder(this->name_, this->description_, this->unit_, this->enabled_)); - boundInstruments_[labelset]=sp1; - return sp1; - } - else - { - boundInstruments_[labelset]->inc_ref(); - return boundInstruments_[labelset]; - } + auto sp1 = nostd::shared_ptr>( + new BoundValueRecorder(this->name_, this->description_, this->unit_, this->enabled_)); + boundInstruments_[labelset] = sp1; + return sp1; } - - /* - * Add adds the value to the counter's sum. The labels should contain - * the keys and values to be associated with this value. Counters only - * accept positive valued updates. - * - * @param value the numerical representation of the metric being captured - * @param labels the set of labels, as key-value pairs - */ - void record(T value, const trace::KeyValueIterable &labels) override + else { - this->mu_.lock(); - auto sp = bindValueRecorder(labels); - sp->update(value); - sp->unbind(); - this->mu_.unlock(); + boundInstruments_[labelset]->inc_ref(); + return boundInstruments_[labelset]; } - - virtual std::vector GetRecords() override { - std::vector ret; - std::vector toDelete; - for (const auto &x : boundInstruments_){ - if (x.second->get_ref() == 0){ - toDelete.push_back(x.first); - } - auto agg_ptr = dynamic_cast*>(x.second.get())->GetAggregator(); - agg_ptr->checkpoint(); - ret.push_back(Record(x.second->GetName(), x.second->GetDescription(), x.first, agg_ptr)); - } - for (const auto &x : toDelete){ - boundInstruments_.erase(x); - } - return ret; + } + + /* + * Add adds the value to the counter's sum. The labels should contain + * the keys and values to be associated with this value. Counters only + * accept positive valued updates. + * + * @param value the numerical representation of the metric being captured + * @param labels the set of labels, as key-value pairs + */ + void record(T value, const trace::KeyValueIterable &labels) override + { + this->mu_.lock(); + auto sp = bindValueRecorder(labels); + sp->update(value); + sp->unbind(); + this->mu_.unlock(); + } + + virtual std::vector GetRecords() override + { + std::vector ret; + std::vector toDelete; + for (const auto &x : boundInstruments_) + { + if (x.second->get_ref() == 0) + { + toDelete.push_back(x.first); + } + auto agg_ptr = dynamic_cast *>(x.second.get())->GetAggregator(); + agg_ptr->checkpoint(); + ret.push_back(Record(x.second->GetName(), x.second->GetDescription(), x.first, agg_ptr)); } - - virtual void update(T value, const trace::KeyValueIterable &labels) override { - record(value, labels); + for (const auto &x : toDelete) + { + boundInstruments_.erase(x); } + return ret; + } + + virtual void update(T value, const trace::KeyValueIterable &labels) override + { + record(value, labels); + } - std::unordered_map>> boundInstruments_; + std::unordered_map>> + boundInstruments_; }; -} -} +} // namespace metrics +} // namespace sdk OPENTELEMETRY_END_NAMESPACE diff --git a/sdk/test/metrics/metric_instrument_test.cc b/sdk/test/metrics/metric_instrument_test.cc index 0919291dfc..183b5b97e2 100644 --- a/sdk/test/metrics/metric_instrument_test.cc +++ b/sdk/test/metrics/metric_instrument_test.cc @@ -2,15 +2,14 @@ #include "opentelemetry/sdk/metrics/sync_instruments.h" #include -#include +#include #include -#include #include -#include +#include +#include namespace metrics_api = opentelemetry::metrics; - OPENTELEMETRY_BEGIN_NAMESPACE namespace sdk { @@ -19,198 +18,270 @@ namespace metrics TEST(Counter, InstrumentFunctions) { - Counter alpha("enabled", "no description", "unitless", true); - Counter beta("not enabled", "some description", "units", false); - - EXPECT_EQ(static_cast(alpha.GetName()), "enabled"); - EXPECT_EQ(static_cast(alpha.GetDescription()), "no description"); - EXPECT_EQ(static_cast(alpha.GetUnits()), "unitless"); - EXPECT_EQ(alpha.IsEnabled(), true); - - EXPECT_EQ(static_cast(beta.GetName()), "not enabled"); - EXPECT_EQ(static_cast(beta.GetDescription()), "some description"); - EXPECT_EQ(static_cast(beta.GetUnits()), "units"); - EXPECT_EQ(beta.IsEnabled(), false); + Counter alpha("enabled", "no description", "unitless", true); + Counter beta("not enabled", "some description", "units", false); + + EXPECT_EQ(static_cast(alpha.GetName()), "enabled"); + EXPECT_EQ(static_cast(alpha.GetDescription()), "no description"); + EXPECT_EQ(static_cast(alpha.GetUnits()), "unitless"); + EXPECT_EQ(alpha.IsEnabled(), true); + + EXPECT_EQ(static_cast(beta.GetName()), "not enabled"); + EXPECT_EQ(static_cast(beta.GetDescription()), "some description"); + EXPECT_EQ(static_cast(beta.GetUnits()), "units"); + EXPECT_EQ(beta.IsEnabled(), false); } TEST(Counter, Binding) { - Counter alpha("test", "none", "unitless", true); - - std::map labels = {{"key", "value"}}; - std::map labels1 = {{"key1", "value1"}}; - std::map labels2 = {{"key2", "value2"}, {"key3", "value3"}}; - std::map labels3 = {{"key3", "value3"}, {"key2", "value2"}}; - - - auto labelkv = trace::KeyValueIterableView{labels}; - auto labelkv1 = trace::KeyValueIterableView{labels1}; - auto labelkv2 = trace::KeyValueIterableView{labels2}; - auto labelkv3 = trace::KeyValueIterableView{labels3}; - - auto beta = alpha.bindCounter(labelkv); - auto gamma = alpha.bindCounter(labelkv1); - auto delta = alpha.bindCounter(labelkv1); - auto epsilon = alpha.bindCounter(labelkv1); - auto zeta = alpha.bindCounter(labelkv2); - auto eta = alpha.bindCounter(labelkv3); - - EXPECT_EQ (beta->get_ref(), 1); - EXPECT_EQ(gamma->get_ref(), 3); - EXPECT_EQ(eta->get_ref(), 2); - - delta->unbind(); - gamma->unbind(); - epsilon->unbind(); - - EXPECT_EQ(alpha.boundInstruments_[KvToString(labelkv1)]->get_ref(), 0); - EXPECT_EQ(alpha.boundInstruments_.size(), 3); + Counter alpha("test", "none", "unitless", true); + + std::map labels = {{"key", "value"}}; + std::map labels1 = {{"key1", "value1"}}; + std::map labels2 = {{"key2", "value2"}, {"key3", "value3"}}; + std::map labels3 = {{"key3", "value3"}, {"key2", "value2"}}; + + auto labelkv = trace::KeyValueIterableView{labels}; + auto labelkv1 = trace::KeyValueIterableView{labels1}; + auto labelkv2 = trace::KeyValueIterableView{labels2}; + auto labelkv3 = trace::KeyValueIterableView{labels3}; + + auto beta = alpha.bindCounter(labelkv); + auto gamma = alpha.bindCounter(labelkv1); + auto delta = alpha.bindCounter(labelkv1); + auto epsilon = alpha.bindCounter(labelkv1); + auto zeta = alpha.bindCounter(labelkv2); + auto eta = alpha.bindCounter(labelkv3); + + EXPECT_EQ(beta->get_ref(), 1); + EXPECT_EQ(gamma->get_ref(), 3); + EXPECT_EQ(eta->get_ref(), 2); + + delta->unbind(); + gamma->unbind(); + epsilon->unbind(); + + EXPECT_EQ(alpha.boundInstruments_[KvToString(labelkv1)]->get_ref(), 0); + EXPECT_EQ(alpha.boundInstruments_.size(), 3); } TEST(Counter, getAggsandnewupdate) { - Counter alpha("test", "none", "unitless", true); - - std::map labels = {{"key", "value"}}; - std::map labels1 = {{"key1", "value1"}}; - - //labels 2 and 3 are actually the same - std::map labels2 = {{"key2", "value2"}, {"key3", "value3"}}; - std::map labels3 = {{"key3", "value3"}, {"key2", "value2"}}; - - auto labelkv = trace::KeyValueIterableView{labels}; - auto labelkv1 = trace::KeyValueIterableView{labels1}; - auto labelkv2 = trace::KeyValueIterableView{labels2}; - auto labelkv3 = trace::KeyValueIterableView{labels3}; - - auto beta = alpha.bindCounter(labelkv); - auto gamma = alpha.bindCounter(labelkv1); - auto delta = alpha.bindCounter(labelkv1); - auto epsilon = alpha.bindCounter(labelkv1); - auto zeta = alpha.bindCounter(labelkv2); - auto eta = alpha.bindCounter(labelkv3); - - EXPECT_EQ (beta->get_ref(), 1); - EXPECT_EQ(gamma->get_ref(), 3); - EXPECT_EQ(eta->get_ref(), 2); - - delta->unbind(); - gamma->unbind(); - epsilon->unbind(); - - EXPECT_EQ(alpha.boundInstruments_[KvToString(labelkv1)]->get_ref(), 0); - EXPECT_EQ(alpha.boundInstruments_.size(), 3); - - auto theta = alpha.GetRecords(); - EXPECT_EQ(theta.size(),3); - EXPECT_EQ(theta[0].GetName(), "test"); - EXPECT_EQ(theta[0].GetDescription(), "none"); - EXPECT_EQ(theta[0].GetLabels(), "{\"key2\":\"value2\",\"key3\":\"value3\"}"); - EXPECT_EQ(theta[1].GetLabels(), "{\"key1\":\"value1\"}"); + Counter alpha("test", "none", "unitless", true); + + std::map labels = {{"key", "value"}}; + std::map labels1 = {{"key1", "value1"}}; + + // labels 2 and 3 are actually the same + std::map labels2 = {{"key2", "value2"}, {"key3", "value3"}}; + std::map labels3 = {{"key3", "value3"}, {"key2", "value2"}}; + + auto labelkv = trace::KeyValueIterableView{labels}; + auto labelkv1 = trace::KeyValueIterableView{labels1}; + auto labelkv2 = trace::KeyValueIterableView{labels2}; + auto labelkv3 = trace::KeyValueIterableView{labels3}; + + auto beta = alpha.bindCounter(labelkv); + auto gamma = alpha.bindCounter(labelkv1); + auto delta = alpha.bindCounter(labelkv1); + auto epsilon = alpha.bindCounter(labelkv1); + auto zeta = alpha.bindCounter(labelkv2); + auto eta = alpha.bindCounter(labelkv3); + + EXPECT_EQ(beta->get_ref(), 1); + EXPECT_EQ(gamma->get_ref(), 3); + EXPECT_EQ(eta->get_ref(), 2); + + delta->unbind(); + gamma->unbind(); + epsilon->unbind(); + + EXPECT_EQ(alpha.boundInstruments_[KvToString(labelkv1)]->get_ref(), 0); + EXPECT_EQ(alpha.boundInstruments_.size(), 3); + + auto theta = alpha.GetRecords(); + EXPECT_EQ(theta.size(), 3); + EXPECT_EQ(theta[0].GetName(), "test"); + EXPECT_EQ(theta[0].GetDescription(), "none"); + EXPECT_EQ(theta[0].GetLabels(), "{\"key2\":\"value2\",\"key3\":\"value3\"}"); + EXPECT_EQ(theta[1].GetLabels(), "{\"key1\":\"value1\"}"); } -void CounterCallback(std::shared_ptr> in, int freq, const trace::KeyValueIterable &labels){ - for (int i=0; iadd(1, labels); - } +void CounterCallback(std::shared_ptr> in, + int freq, + const trace::KeyValueIterable &labels) +{ + for (int i = 0; i < freq; i++) + { + in->add(1, labels); + } } -TEST(Counter, StressAdd){ - std::shared_ptr> alpha(new Counter("test", "none", "unitless", true)); - - std::map labels = {{"key", "value"}}; - std::map labels1 = {{"key1", "value1"}}; - - auto labelkv = trace::KeyValueIterableView{labels}; - auto labelkv1 = trace::KeyValueIterableView{labels1}; - - std::thread first (CounterCallback, alpha, 100000, labelkv); - std::thread second (CounterCallback, alpha, 100000, labelkv); - std::thread third (CounterCallback, alpha, 300000, labelkv1); - - first.join(); - second.join(); - third.join(); - - EXPECT_EQ(dynamic_cast*>(alpha->boundInstruments_[KvToString(labelkv)].get())->GetAggregator()->get_values()[0], 200000); - EXPECT_EQ(dynamic_cast*>(alpha->boundInstruments_[KvToString(labelkv1)].get())->GetAggregator()->get_values()[0], 300000); +TEST(Counter, StressAdd) +{ + std::shared_ptr> alpha(new Counter("test", "none", "unitless", true)); + + std::map labels = {{"key", "value"}}; + std::map labels1 = {{"key1", "value1"}}; + + auto labelkv = trace::KeyValueIterableView{labels}; + auto labelkv1 = trace::KeyValueIterableView{labels1}; + + std::thread first(CounterCallback, alpha, 100000, labelkv); + std::thread second(CounterCallback, alpha, 100000, labelkv); + std::thread third(CounterCallback, alpha, 300000, labelkv1); + + first.join(); + second.join(); + third.join(); + + EXPECT_EQ(dynamic_cast *>(alpha->boundInstruments_[KvToString(labelkv)].get()) + ->GetAggregator() + ->get_values()[0], + 200000); + EXPECT_EQ(dynamic_cast *>(alpha->boundInstruments_[KvToString(labelkv1)].get()) + ->GetAggregator() + ->get_values()[0], + 300000); } -void UpDownCounterCallback(std::shared_ptr> in, int freq, const trace::KeyValueIterable &labels){ - for (int i=0; iadd(1, labels); - } +void UpDownCounterCallback(std::shared_ptr> in, + int freq, + const trace::KeyValueIterable &labels) +{ + for (int i = 0; i < freq; i++) + { + in->add(1, labels); + } } -void NegUpDownCounterCallback(std::shared_ptr> in, int freq, const trace::KeyValueIterable &labels){ - for (int i=0; iadd(-1, labels); - } +void NegUpDownCounterCallback(std::shared_ptr> in, + int freq, + const trace::KeyValueIterable &labels) +{ + for (int i = 0; i < freq; i++) + { + in->add(-1, labels); + } } -TEST(IntUpDownCounter, StressAdd){ - std::shared_ptr> alpha(new UpDownCounter("test", "none", "unitless", true)); - - std::map labels = {{"key", "value"}}; - std::map labels1 = {{"key1", "value1"}}; - auto labelkv = trace::KeyValueIterableView{labels}; - auto labelkv1 = trace::KeyValueIterableView{labels1}; - - std::thread first (UpDownCounterCallback, alpha, 123400, labelkv); // spawn new threads that call the callback - std::thread second (UpDownCounterCallback, alpha, 123400, labelkv); - std::thread third (UpDownCounterCallback, alpha, 567800, labelkv1); - std::thread fourth (NegUpDownCounterCallback, alpha, 123400, labelkv1); // negative values - - first.join(); - second.join(); - third.join(); - fourth.join(); - - EXPECT_EQ(dynamic_cast*>(alpha->boundInstruments_[KvToString(labelkv)].get())->GetAggregator()->get_values()[0], 123400*2); - EXPECT_EQ(dynamic_cast*>(alpha->boundInstruments_[KvToString(labelkv1)].get())->GetAggregator()->get_values()[0], 567800-123400); +TEST(IntUpDownCounter, StressAdd) +{ + std::shared_ptr> alpha( + new UpDownCounter("test", "none", "unitless", true)); + + std::map labels = {{"key", "value"}}; + std::map labels1 = {{"key1", "value1"}}; + auto labelkv = trace::KeyValueIterableView{labels}; + auto labelkv1 = trace::KeyValueIterableView{labels1}; + + std::thread first(UpDownCounterCallback, alpha, 123400, + labelkv); // spawn new threads that call the callback + std::thread second(UpDownCounterCallback, alpha, 123400, labelkv); + std::thread third(UpDownCounterCallback, alpha, 567800, labelkv1); + std::thread fourth(NegUpDownCounterCallback, alpha, 123400, labelkv1); // negative values + + first.join(); + second.join(); + third.join(); + fourth.join(); + + EXPECT_EQ( + dynamic_cast *>(alpha->boundInstruments_[KvToString(labelkv)].get()) + ->GetAggregator() + ->get_values()[0], + 123400 * 2); + EXPECT_EQ( + dynamic_cast *>(alpha->boundInstruments_[KvToString(labelkv1)].get()) + ->GetAggregator() + ->get_values()[0], + 567800 - 123400); } -void RecorderCallback(std::shared_ptr> in, int freq, const trace::KeyValueIterable &labels){ - for (int i=0; irecord(i, labels); - } +void RecorderCallback(std::shared_ptr> in, + int freq, + const trace::KeyValueIterable &labels) +{ + for (int i = 0; i < freq; i++) + { + in->record(i, labels); + } } -void NegRecorderCallback(std::shared_ptr> in, int freq, const trace::KeyValueIterable &labels){ - for (int i=0; irecord(-i, labels); - } +void NegRecorderCallback(std::shared_ptr> in, + int freq, + const trace::KeyValueIterable &labels) +{ + for (int i = 0; i < freq; i++) + { + in->record(-i, labels); + } } -TEST(IntValueRecorder, StressRecord){ - std::shared_ptr> alpha(new ValueRecorder("test", "none", "unitless", true)); - - std::map labels = {{"key", "value"}}; - std::map labels1 = {{"key1", "value1"}}; - auto labelkv = trace::KeyValueIterableView{labels}; - auto labelkv1 = trace::KeyValueIterableView{labels1}; - - std::thread first (RecorderCallback, alpha, 25, labelkv); // spawn new threads that call the callback - std::thread second (RecorderCallback, alpha, 50, labelkv); - std::thread third (RecorderCallback, alpha, 25, labelkv1); - std::thread fourth (NegRecorderCallback, alpha, 100, labelkv1); // negative values - - first.join(); - second.join(); - third.join(); - fourth.join(); - - EXPECT_EQ(dynamic_cast*>(alpha->boundInstruments_[KvToString(labelkv)].get())->GetAggregator()->get_values()[0], 0); // min - EXPECT_EQ(dynamic_cast*>(alpha->boundInstruments_[KvToString(labelkv)].get())->GetAggregator()->get_values()[1], 49); // max - EXPECT_EQ(dynamic_cast*>(alpha->boundInstruments_[KvToString(labelkv)].get())->GetAggregator()->get_values()[2], 1525); // sum - EXPECT_EQ(dynamic_cast*>(alpha->boundInstruments_[KvToString(labelkv)].get())->GetAggregator()->get_values()[3], 75); // count - - EXPECT_EQ(dynamic_cast*>(alpha->boundInstruments_[KvToString(labelkv1)].get())->GetAggregator()->get_values()[0], -99); // min - EXPECT_EQ(dynamic_cast*>(alpha->boundInstruments_[KvToString(labelkv1)].get())->GetAggregator()->get_values()[1], 24); // max - EXPECT_EQ(dynamic_cast*>(alpha->boundInstruments_[KvToString(labelkv1)].get())->GetAggregator()->get_values()[2], -4650); // sum - EXPECT_EQ(dynamic_cast*>(alpha->boundInstruments_[KvToString(labelkv1)].get())->GetAggregator()->get_values()[3], 125); // count +TEST(IntValueRecorder, StressRecord) +{ + std::shared_ptr> alpha( + new ValueRecorder("test", "none", "unitless", true)); + + std::map labels = {{"key", "value"}}; + std::map labels1 = {{"key1", "value1"}}; + auto labelkv = trace::KeyValueIterableView{labels}; + auto labelkv1 = trace::KeyValueIterableView{labels1}; + + std::thread first(RecorderCallback, alpha, 25, + labelkv); // spawn new threads that call the callback + std::thread second(RecorderCallback, alpha, 50, labelkv); + std::thread third(RecorderCallback, alpha, 25, labelkv1); + std::thread fourth(NegRecorderCallback, alpha, 100, labelkv1); // negative values + + first.join(); + second.join(); + third.join(); + fourth.join(); + + EXPECT_EQ( + dynamic_cast *>(alpha->boundInstruments_[KvToString(labelkv)].get()) + ->GetAggregator() + ->get_values()[0], + 0); // min + EXPECT_EQ( + dynamic_cast *>(alpha->boundInstruments_[KvToString(labelkv)].get()) + ->GetAggregator() + ->get_values()[1], + 49); // max + EXPECT_EQ( + dynamic_cast *>(alpha->boundInstruments_[KvToString(labelkv)].get()) + ->GetAggregator() + ->get_values()[2], + 1525); // sum + EXPECT_EQ( + dynamic_cast *>(alpha->boundInstruments_[KvToString(labelkv)].get()) + ->GetAggregator() + ->get_values()[3], + 75); // count + + EXPECT_EQ( + dynamic_cast *>(alpha->boundInstruments_[KvToString(labelkv1)].get()) + ->GetAggregator() + ->get_values()[0], + -99); // min + EXPECT_EQ( + dynamic_cast *>(alpha->boundInstruments_[KvToString(labelkv1)].get()) + ->GetAggregator() + ->get_values()[1], + 24); // max + EXPECT_EQ( + dynamic_cast *>(alpha->boundInstruments_[KvToString(labelkv1)].get()) + ->GetAggregator() + ->get_values()[2], + -4650); // sum + EXPECT_EQ( + dynamic_cast *>(alpha->boundInstruments_[KvToString(labelkv1)].get()) + ->GetAggregator() + ->get_values()[3], + 125); // count } -} //namespace sdk } // namespace metrics +} // namespace sdk OPENTELEMETRY_END_NAMESPACE From 322e5c8489019daa1787e3a8f5fae8d46f37de58 Mon Sep 17 00:00:00 2001 From: Ankit Bhargava Date: Fri, 31 Jul 2020 01:04:11 -0400 Subject: [PATCH 15/16] exception handling --- sdk/include/opentelemetry/sdk/metrics/instrument.h | 4 ++++ sdk/include/opentelemetry/sdk/metrics/sync_instruments.h | 8 ++++++++ 2 files changed, 12 insertions(+) diff --git a/sdk/include/opentelemetry/sdk/metrics/instrument.h b/sdk/include/opentelemetry/sdk/metrics/instrument.h index 6ba0a30ffd..3a3e525714 100644 --- a/sdk/include/opentelemetry/sdk/metrics/instrument.h +++ b/sdk/include/opentelemetry/sdk/metrics/instrument.h @@ -192,7 +192,11 @@ inline void print_value(std::stringstream &ss, ss << '"'; break; default: +#if __EXCEPTIONS throw std::invalid_argument("Labels must be strings"); +#else + std::terminate(); +#endif break; } }; diff --git a/sdk/include/opentelemetry/sdk/metrics/sync_instruments.h b/sdk/include/opentelemetry/sdk/metrics/sync_instruments.h index 824eaa48f4..1b2d181118 100644 --- a/sdk/include/opentelemetry/sdk/metrics/sync_instruments.h +++ b/sdk/include/opentelemetry/sdk/metrics/sync_instruments.h @@ -51,7 +51,11 @@ class BoundCounter final : public BoundSynchronousInstrument, public metrics_ this->mu_.lock(); if (value < 0) { +#if __EXCEPTIONS throw std::invalid_argument("Counter instrument updates must be non-negative."); +#else + std::terminate(); +#endif } else { @@ -118,7 +122,11 @@ class Counter final : public SynchronousInstrument, public metrics_api::Count this->mu_.lock(); if (value < 0) { +#if __EXCEPTIONS throw std::invalid_argument("Counter instrument updates must be non-negative."); +#else + std::terminate(); +#endif } else { From 4d90ee77cf7f2f62c1558599620984e254405a9e Mon Sep 17 00:00:00 2001 From: Ankit Bhargava Date: Mon, 3 Aug 2020 11:53:18 -0400 Subject: [PATCH 16/16] found bug during pipeline testing --- sdk/include/opentelemetry/sdk/metrics/sync_instruments.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/metrics/sync_instruments.h b/sdk/include/opentelemetry/sdk/metrics/sync_instruments.h index 1b2d181118..d8b1f83d24 100644 --- a/sdk/include/opentelemetry/sdk/metrics/sync_instruments.h +++ b/sdk/include/opentelemetry/sdk/metrics/sync_instruments.h @@ -272,7 +272,7 @@ class UpDownCounter final : public SynchronousInstrument, public metrics_api: { toDelete.push_back(x.first); } - auto agg_ptr = dynamic_cast *>(x.second.get())->GetAggregator(); + auto agg_ptr = dynamic_cast *>(x.second.get())->GetAggregator(); agg_ptr->checkpoint(); ret.push_back(Record(x.second->GetName(), x.second->GetDescription(), x.first, agg_ptr)); } @@ -396,7 +396,7 @@ class ValueRecorder final : public SynchronousInstrument, public metrics_api: { toDelete.push_back(x.first); } - auto agg_ptr = dynamic_cast *>(x.second.get())->GetAggregator(); + auto agg_ptr = dynamic_cast *>(x.second.get())->GetAggregator(); agg_ptr->checkpoint(); ret.push_back(Record(x.second->GetName(), x.second->GetDescription(), x.first, agg_ptr)); }