From 0f378bef0dc6064a17f5a814adff83516ec4fa32 Mon Sep 17 00:00:00 2001 From: Brandon Kimberly Date: Tue, 21 Jul 2020 15:55:53 -0400 Subject: [PATCH 01/20] Create exact_aggregator.h --- .../sdk/metrics/aggreator/exact_aggregator.h | 144 ++++++++++++++++++ 1 file changed, 144 insertions(+) create mode 100644 sdk/include/opentelemetry/sdk/metrics/aggreator/exact_aggregator.h diff --git a/sdk/include/opentelemetry/sdk/metrics/aggreator/exact_aggregator.h b/sdk/include/opentelemetry/sdk/metrics/aggreator/exact_aggregator.h new file mode 100644 index 0000000000..7ffb09a568 --- /dev/null +++ b/sdk/include/opentelemetry/sdk/metrics/aggreator/exact_aggregator.h @@ -0,0 +1,144 @@ +#pragma once + +#include "opentelemetry/metrics/instrument.h" +#include "opentelemetry/sdk/metrics/aggregator/aggregator.h" +#include "opentelemetry/version.h" + +#include +#include +#include +#include +#include + +namespace metrics_api = opentelemetry::metrics; + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace sdk +{ +namespace metrics +{ +/** + * This aggregator has two modes. In-order and quantile estimation. + * + * The first mode simply stores all values sent to the Update() + * function in a vector and maintains the order they were sent in. + * + * The second mode also stores all values sent to the Update() + * function in a vector but sorts this vector when Checkpoint() + * is called. This mode also includes a function, Quantile(), + * that estimates the quantiles of the recorded data. + * + * @tparam T the type of values stored in this aggregator. + */ +template +class ExactAggregator : public Aggregator +{ +public: + ExactAggregator(metrics_api::InstrumentKind kind, bool quant_estimation = false) + { + static_assert(std::is_arithmetic::value, "Not an arithmetic type"); + this->kind_ = kind; + this->checkpoint_ = this->values_; + this->agg_kind_ = AggregatorKind::Exact; + quant_estimation_ = quant_estimation; + } + + /** + * Receives a captured value from the instrument and adds it to the values_ vector. + * + * @param val, the raw value used in aggregation + */ + void update(T val) override + { + this->mu_.lock(); + this->values_.push_back(val); + this->mu_.unlock(); + } + + /** + * Checkpoints the current values. This function will overwrite the current checkpoint with the + * current value. Sorts the values_ vector if quant_estimation_ == true + * + */ + void checkpoint() override + { + this->mu_.lock(); + if (quant_estimation_) + { + std::sort(this->values_.begin(), this->values_.end()); + } + this->checkpoint_ = this->values_; + this->values_.clear(); + this->mu_.unlock(); + } + + /** + * Merges two exact aggregators' values_ vectors together. + * + * @param other the aggregator to merge with this aggregator + */ + void merge(const ExactAggregator &other) + { + if (this->kind_ == other.kind_) + { + this->mu_.lock(); + this->values_.insert(this->values_.end(), other.values_.begin(), other.values_.end()); + this->mu_.unlock(); + } + else + { + // Log error + return; + } + } + + /** + * Performs quantile estimation on the checkpoint vector in this aggregator. + * This function only works if quant_estimation_ == true. + * @param q the quantile to estimate. 0 <= q <= 1 + * @return the nearest value in the vector to the exact quantile. + */ + T quantile(float q) + { + if (!quant_estimation_) + { + // Log error + throw std::domain_error("Exact aggregator is not in quantile estimation mode!"); + } + if (this->checkpoint_.size() == 0 || q < 0 || q > 1) + { + // Log error + throw std::invalid_argument("Arg 'q' must be between 0 and 1, inclusive"); + } + else if (q == 0 || this->checkpoint_.size() == 1) + { + return this->checkpoint_[0]; + } + else if (q == 1) + { + return this->checkpoint_[this->checkpoint_.size() - 1]; + } + else + { + float position = float(this->checkpoint_.size() - 1) * q; + int ceiling = ceil(position); + return this->checkpoint_[ceiling]; + } + } + + //////////////////////////ACCESSOR FUNCTIONS////////////////////////// + std::vector get_checkpoint() override + { + return this->checkpoint_; + } + + std::vector get_values() override + { + return this->values_; + } +private: + bool quant_estimation_; // Used to switch between in-order and quantile estimation modes +}; +} +} +OPENTELEMETRY_END_NAMESPACE From 16fc838edd15a46235f5b3ed6909bc992bbe257f Mon Sep 17 00:00:00 2001 From: Brandon Kimberly Date: Tue, 21 Jul 2020 15:56:13 -0400 Subject: [PATCH 02/20] Create exact_aggregator_test.cc --- sdk/test/metrics/exact_aggregator_test.cc | 213 ++++++++++++++++++++++ 1 file changed, 213 insertions(+) create mode 100644 sdk/test/metrics/exact_aggregator_test.cc diff --git a/sdk/test/metrics/exact_aggregator_test.cc b/sdk/test/metrics/exact_aggregator_test.cc new file mode 100644 index 0000000000..1beb2bb3d1 --- /dev/null +++ b/sdk/test/metrics/exact_aggregator_test.cc @@ -0,0 +1,213 @@ +#include +#include + +#include "opentelemetry/sdk/metrics/aggregator/exact_aggregator.h" + +using namespace opentelemetry::sdk::metrics; + +TEST(ExactAggregatorOrdered, Update) +{ + auto agg = new ExactAggregator(opentelemetry::metrics::InstrumentKind::IntCounter); + + std::vector correct; + + ASSERT_EQ(agg->get_values(), correct); + + agg->update(1); + correct.push_back(1); + + ASSERT_EQ(agg->get_values(), std::vector{1}); + + for (int i = 2; i <= 5; ++i) + { + correct.push_back(i); + agg->update(i); + } + ASSERT_EQ(agg->get_values(), correct); +} + +TEST(ExactAggregatorOrdered, Checkpoint) +{ + auto agg = new ExactAggregator(opentelemetry::metrics::InstrumentKind::IntCounter); + + std::vector correct; + + ASSERT_EQ(agg->get_checkpoint(), correct); + + agg->update(1); + correct.push_back(1); + agg->checkpoint(); + + ASSERT_EQ(agg->get_checkpoint(), correct); +} + +TEST(ExactAggregatorOrdered, Merge) +{ + auto agg1 = new ExactAggregator(opentelemetry::metrics::InstrumentKind::IntCounter); + auto agg2 = new ExactAggregator(opentelemetry::metrics::InstrumentKind::IntCounter); + + agg1->update(1); + agg2->update(2); + agg1->merge(*agg2); + + std::vector correct{1, 2}; + + ASSERT_EQ(agg1->get_values(), correct); +} + +TEST(ExactAggregatorOrdered, BadMerge) +{ + // This verifies that we encounter and error when we try to merge + // two aggregators of different numeric types together. + auto agg1 = new ExactAggregator(opentelemetry::metrics::InstrumentKind::IntCounter); + auto agg2 = new ExactAggregator(opentelemetry::metrics::InstrumentKind::DoubleCounter); + + agg1->update(1); + agg2->update(2); + + agg1->merge(*agg2); + + // Verify that the aggregators did NOT merge + std::vector correct{1}; + ASSERT_EQ(agg1->get_values(), correct); +} + +TEST(ExactAggregatorOrdered, Types) +{ + // This test verifies that we do not encounter any errors when + // using various numeric types. + auto agg_int = new ExactAggregator(opentelemetry::metrics::InstrumentKind::IntCounter); + auto agg_long = new ExactAggregator(opentelemetry::metrics::InstrumentKind::IntCounter); + auto agg_float = new ExactAggregator(opentelemetry::metrics::InstrumentKind::IntCounter); + auto agg_double = new ExactAggregator(opentelemetry::metrics::InstrumentKind::IntCounter); + + for (int i = 1; i <= 5; ++i) + { + agg_int->update(i); + agg_long->update(i); + } + + for (float i = 1.0; i <= 5.0; i += 1) + { + agg_float->update(i); + agg_double->update(i); + } + + std::vector correct_int{1, 2, 3, 4, 5}; + std::vector correct_long{1, 2, 3, 4, 5}; + std::vector correct_float{1.0, 2.0, 3.0, 4.0, 5.0}; + std::vector correct_double{1.0, 2.0, 3.0, 4.0, 5.0}; + + ASSERT_EQ(agg_int->get_values(), correct_int); + ASSERT_EQ(agg_long->get_values(), correct_long); + ASSERT_EQ(agg_float->get_values(), correct_float); + ASSERT_EQ(agg_double->get_values(), correct_double); +} + +TEST(ExactAggregatorQuant, Update) +{ + auto agg = new ExactAggregator(opentelemetry::metrics::InstrumentKind::IntCounter, true); + + std::vector correct; + + ASSERT_EQ(agg->get_values(), correct); + + agg->update(1); + correct.push_back(1); + + ASSERT_EQ(agg->get_values(), std::vector{1}); + + for (int i = 2; i <= 5; ++i) + { + correct.push_back(i); + agg->update(i); + } + ASSERT_EQ(agg->get_values(), correct); +} + +TEST(ExactAggregatorQuant, Checkpoint) +{ + // This test verifies that the aggregator updates correctly when + // quantile estimation is turned on. + + auto agg = new ExactAggregator(opentelemetry::metrics::InstrumentKind::IntCounter, true); + + std::vector correct; + + ASSERT_EQ(agg->get_checkpoint(), correct); + + agg->update(1); + agg->update(0); + agg->update(-1); + + // The vector MUST be sorted when checkpointed + correct.push_back(-1); correct.push_back(0); correct.push_back(1); + agg->checkpoint(); + + ASSERT_EQ(agg->get_checkpoint(), correct); +} + + +TEST(ExactAggregatorQuant, Quantile) +{ + // This test verifies that the quantile estimation function returns + // the correct values. + + auto agg = new ExactAggregator(opentelemetry::metrics::InstrumentKind::IntCounter, true); + + std::vector tmp {3, 9, 42, 57, 163, 210, 272, 300}; + for (int i : tmp) + { + agg->update(i); + } + agg->checkpoint(); + ASSERT_EQ(agg->quantile(.25), 42); + ASSERT_EQ(agg->quantile(0.5), 163); + ASSERT_EQ(agg->quantile(0.75), 272); +} + +TEST(ExactAggregatorInOrder, Quantile) +{ + // This test verifies that if the user has an exact aggregator in "in-order" mode + // an exception will be thrown if they call the quantile() function. + auto agg = new ExactAggregator(opentelemetry::metrics::InstrumentKind::IntCounter); + + std::vector tmp {3, 9, 42, 57, 163, 210, 272, 300}; + for (int i : tmp) + { + agg->update(i); + } + agg->checkpoint(); + ASSERT_THROW(agg->quantile(0.5), std::domain_error); +} + +void callback(ExactAggregator &agg) +{ + for (int i = 1; i <= 10000; ++i) + { + agg.update(i); + } +} + +TEST(ExactAggregatorQuant, Concurrency) +{ + // This test checks that the aggregator updates appropriately + // when called in a multi-threaded context. + auto agg = new ExactAggregator(opentelemetry::metrics::InstrumentKind::IntCounter, true); + + std::thread first(&callback, std::ref(*agg)); + std::thread second(&callback, std::ref(*agg)); + + first.join(); + second.join(); + + std::vector correct; + for (int i = 1; i <= 10000; ++i) + { + correct.push_back(i); + correct.push_back(i); + } + agg->checkpoint(); + + ASSERT_EQ(agg->get_checkpoint(), correct); +} From 068bc8286cbc8fd40a467da12d2fce72f26a156c Mon Sep 17 00:00:00 2001 From: Brandon Kimberly Date: Tue, 21 Jul 2020 15:56:28 -0400 Subject: [PATCH 03/20] Add exact aggregator test --- sdk/test/metrics/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/test/metrics/CMakeLists.txt b/sdk/test/metrics/CMakeLists.txt index 800b57557a..6acf20b14c 100644 --- a/sdk/test/metrics/CMakeLists.txt +++ b/sdk/test/metrics/CMakeLists.txt @@ -1,4 +1,4 @@ -foreach(testname meter_provider_sdk_test) +foreach(testname meter_provider_sdk_test exact_aggregator) add_executable(${testname} "${testname}.cc") target_link_libraries(${testname} ${GTEST_BOTH_LIBRARIES} ${CMAKE_THREAD_LIBS_INIT} opentelemetry_common opentelemetry_metrics) From 28abe7f877937cae8b17f5b0972044ca8d7e62b1 Mon Sep 17 00:00:00 2001 From: Brandon Kimberly Date: Tue, 21 Jul 2020 15:56:59 -0400 Subject: [PATCH 04/20] Add exact aggregator test --- sdk/test/metrics/BUILD | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/sdk/test/metrics/BUILD b/sdk/test/metrics/BUILD index e2d5c5cb71..2d4ac35997 100644 --- a/sdk/test/metrics/BUILD +++ b/sdk/test/metrics/BUILD @@ -8,3 +8,14 @@ cc_test( "@com_google_googletest//:gtest_main", ], ) + +cc_test( + name = "exact_aggregator_test", + srcs = [ + "exact_aggregator_test.cc", + ], + deps = [ + "//sdk/src/metrics", + "@com_google_googletest//:gtest_main", + ], +) From 2f22b33213ae7d625d01886f26ff00e3f9a25541 Mon Sep 17 00:00:00 2001 From: Brandon Kimberly Date: Thu, 23 Jul 2020 15:03:42 -0400 Subject: [PATCH 05/20] Delete exact_aggregator.h temporarily due to name typo --- .../sdk/metrics/aggreator/exact_aggregator.h | 144 ------------------ 1 file changed, 144 deletions(-) delete mode 100644 sdk/include/opentelemetry/sdk/metrics/aggreator/exact_aggregator.h diff --git a/sdk/include/opentelemetry/sdk/metrics/aggreator/exact_aggregator.h b/sdk/include/opentelemetry/sdk/metrics/aggreator/exact_aggregator.h deleted file mode 100644 index 7ffb09a568..0000000000 --- a/sdk/include/opentelemetry/sdk/metrics/aggreator/exact_aggregator.h +++ /dev/null @@ -1,144 +0,0 @@ -#pragma once - -#include "opentelemetry/metrics/instrument.h" -#include "opentelemetry/sdk/metrics/aggregator/aggregator.h" -#include "opentelemetry/version.h" - -#include -#include -#include -#include -#include - -namespace metrics_api = opentelemetry::metrics; - -OPENTELEMETRY_BEGIN_NAMESPACE -namespace sdk -{ -namespace metrics -{ -/** - * This aggregator has two modes. In-order and quantile estimation. - * - * The first mode simply stores all values sent to the Update() - * function in a vector and maintains the order they were sent in. - * - * The second mode also stores all values sent to the Update() - * function in a vector but sorts this vector when Checkpoint() - * is called. This mode also includes a function, Quantile(), - * that estimates the quantiles of the recorded data. - * - * @tparam T the type of values stored in this aggregator. - */ -template -class ExactAggregator : public Aggregator -{ -public: - ExactAggregator(metrics_api::InstrumentKind kind, bool quant_estimation = false) - { - static_assert(std::is_arithmetic::value, "Not an arithmetic type"); - this->kind_ = kind; - this->checkpoint_ = this->values_; - this->agg_kind_ = AggregatorKind::Exact; - quant_estimation_ = quant_estimation; - } - - /** - * Receives a captured value from the instrument and adds it to the values_ vector. - * - * @param val, the raw value used in aggregation - */ - void update(T val) override - { - this->mu_.lock(); - this->values_.push_back(val); - this->mu_.unlock(); - } - - /** - * Checkpoints the current values. This function will overwrite the current checkpoint with the - * current value. Sorts the values_ vector if quant_estimation_ == true - * - */ - void checkpoint() override - { - this->mu_.lock(); - if (quant_estimation_) - { - std::sort(this->values_.begin(), this->values_.end()); - } - this->checkpoint_ = this->values_; - this->values_.clear(); - this->mu_.unlock(); - } - - /** - * Merges two exact aggregators' values_ vectors together. - * - * @param other the aggregator to merge with this aggregator - */ - void merge(const ExactAggregator &other) - { - if (this->kind_ == other.kind_) - { - this->mu_.lock(); - this->values_.insert(this->values_.end(), other.values_.begin(), other.values_.end()); - this->mu_.unlock(); - } - else - { - // Log error - return; - } - } - - /** - * Performs quantile estimation on the checkpoint vector in this aggregator. - * This function only works if quant_estimation_ == true. - * @param q the quantile to estimate. 0 <= q <= 1 - * @return the nearest value in the vector to the exact quantile. - */ - T quantile(float q) - { - if (!quant_estimation_) - { - // Log error - throw std::domain_error("Exact aggregator is not in quantile estimation mode!"); - } - if (this->checkpoint_.size() == 0 || q < 0 || q > 1) - { - // Log error - throw std::invalid_argument("Arg 'q' must be between 0 and 1, inclusive"); - } - else if (q == 0 || this->checkpoint_.size() == 1) - { - return this->checkpoint_[0]; - } - else if (q == 1) - { - return this->checkpoint_[this->checkpoint_.size() - 1]; - } - else - { - float position = float(this->checkpoint_.size() - 1) * q; - int ceiling = ceil(position); - return this->checkpoint_[ceiling]; - } - } - - //////////////////////////ACCESSOR FUNCTIONS////////////////////////// - std::vector get_checkpoint() override - { - return this->checkpoint_; - } - - std::vector get_values() override - { - return this->values_; - } -private: - bool quant_estimation_; // Used to switch between in-order and quantile estimation modes -}; -} -} -OPENTELEMETRY_END_NAMESPACE From 8a185839074dfef87aea1a1fc1fa11e916dd6c59 Mon Sep 17 00:00:00 2001 From: Brandon Kimberly Date: Thu, 23 Jul 2020 15:03:58 -0400 Subject: [PATCH 06/20] Create exact_aggregator.h --- .../sdk/metrics/aggregator/exact_aggregator.h | 150 ++++++++++++++++++ 1 file changed, 150 insertions(+) create mode 100644 sdk/include/opentelemetry/sdk/metrics/aggregator/exact_aggregator.h diff --git a/sdk/include/opentelemetry/sdk/metrics/aggregator/exact_aggregator.h b/sdk/include/opentelemetry/sdk/metrics/aggregator/exact_aggregator.h new file mode 100644 index 0000000000..15d00d56f5 --- /dev/null +++ b/sdk/include/opentelemetry/sdk/metrics/aggregator/exact_aggregator.h @@ -0,0 +1,150 @@ +#pragma once + +#include "opentelemetry/metrics/instrument.h" +#include "opentelemetry/sdk/metrics/aggregator/aggregator.h" +#include "opentelemetry/version.h" + +#include +#include +#include +#include +#include + +namespace metrics_api = opentelemetry::metrics; + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace sdk +{ +namespace metrics +{ +/** + * This aggregator has two modes. In-order and quantile estimation. + * + * The first mode simply stores all values sent to the Update() + * function in a vector and maintains the order they were sent in. + * + * The second mode also stores all values sent to the Update() + * function in a vector but sorts this vector when Checkpoint() + * is called. This mode also includes a function, Quantile(), + * that estimates the quantiles of the recorded data. + * + * @tparam T the type of values stored in this aggregator. + */ +template +class ExactAggregator : public Aggregator +{ +public: + ExactAggregator(metrics_api::InstrumentKind kind, bool quant_estimation = false) + { + static_assert(std::is_arithmetic::value, "Not an arithmetic type"); + this->kind_ = kind; + this->checkpoint_ = this->values_; + this->agg_kind_ = AggregatorKind::Exact; + quant_estimation_ = quant_estimation; + } + + /** + * Receives a captured value from the instrument and adds it to the values_ vector. + * + * @param val, the raw value used in aggregation + */ + void update(T val) override + { + this->mu_.lock(); + this->values_.push_back(val); + this->mu_.unlock(); + } + + /** + * Checkpoints the current values. This function will overwrite the current checkpoint with the + * current value. Sorts the values_ vector if quant_estimation_ == true + * + */ + void checkpoint() override + { + this->mu_.lock(); + if (quant_estimation_) + { + std::sort(this->values_.begin(), this->values_.end()); + } + this->checkpoint_ = this->values_; + this->values_.clear(); + this->mu_.unlock(); + } + + /** + * Merges two exact aggregators' values_ vectors together. + * + * @param other the aggregator to merge with this aggregator + */ + void merge(const ExactAggregator &other) + { + if (this->kind_ == other.kind_) + { + this->mu_.lock(); + this->values_.insert(this->values_.end(), other.values_.begin(), other.values_.end()); + this->mu_.unlock(); + } + else + { + // Log error + return; + } + } + + /** + * Performs quantile estimation on the checkpoint vector in this aggregator. + * This function only works if quant_estimation_ == true. + * @param q the quantile to estimate. 0 <= q <= 1 + * @return the nearest value in the vector to the exact quantile. + */ + T quantile(float q) + { + if (!quant_estimation_) + { + // Log error + throw std::domain_error("Exact aggregator is not in quantile estimation mode!"); + } + if (this->checkpoint_.size() == 0 || q < 0 || q > 1) + { + // Log error + throw std::invalid_argument("Arg 'q' must be between 0 and 1, inclusive"); + } + else if (q == 0 || this->checkpoint_.size() == 1) + { + return this->checkpoint_[0]; + } + else if (q == 1) + { + return this->checkpoint_[this->checkpoint_.size() - 1]; + } + else + { + float position = float(this->checkpoint_.size() - 1) * q; + int ceiling = ceil(position); + return this->checkpoint_[ceiling]; + } + } + + //////////////////////////ACCESSOR FUNCTIONS////////////////////////// + std::vector get_checkpoint() override + { + return this->checkpoint_; + } + + std::vector get_values() override + { + return this->values_; + } + + bool get_quant_estimation() override + { + return quant_estimation_; + } + +private: + bool quant_estimation_; // Used to switch between in-order and quantile estimation modes +}; +} +} +OPENTELEMETRY_END_NAMESPACE From 5a56a781832c0b60b399d70851d4b79b2be8c73b Mon Sep 17 00:00:00 2001 From: Brandon Kimberly Date: Fri, 24 Jul 2020 15:45:52 -0400 Subject: [PATCH 07/20] Update enums used to be in line with Aggregator updates --- sdk/test/metrics/exact_aggregator_test.cc | 30 +++++++++++------------ 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/sdk/test/metrics/exact_aggregator_test.cc b/sdk/test/metrics/exact_aggregator_test.cc index 1beb2bb3d1..d00101365e 100644 --- a/sdk/test/metrics/exact_aggregator_test.cc +++ b/sdk/test/metrics/exact_aggregator_test.cc @@ -7,7 +7,7 @@ using namespace opentelemetry::sdk::metrics; TEST(ExactAggregatorOrdered, Update) { - auto agg = new ExactAggregator(opentelemetry::metrics::InstrumentKind::IntCounter); + auto agg = new ExactAggregator(opentelemetry::metrics::InstrumentKind::Counter); std::vector correct; @@ -28,7 +28,7 @@ TEST(ExactAggregatorOrdered, Update) TEST(ExactAggregatorOrdered, Checkpoint) { - auto agg = new ExactAggregator(opentelemetry::metrics::InstrumentKind::IntCounter); + auto agg = new ExactAggregator(opentelemetry::metrics::InstrumentKind::Counter); std::vector correct; @@ -43,8 +43,8 @@ TEST(ExactAggregatorOrdered, Checkpoint) TEST(ExactAggregatorOrdered, Merge) { - auto agg1 = new ExactAggregator(opentelemetry::metrics::InstrumentKind::IntCounter); - auto agg2 = new ExactAggregator(opentelemetry::metrics::InstrumentKind::IntCounter); + auto agg1 = new ExactAggregator(opentelemetry::metrics::InstrumentKind::Counter); + auto agg2 = new ExactAggregator(opentelemetry::metrics::InstrumentKind::Counter); agg1->update(1); agg2->update(2); @@ -59,8 +59,8 @@ TEST(ExactAggregatorOrdered, BadMerge) { // This verifies that we encounter and error when we try to merge // two aggregators of different numeric types together. - auto agg1 = new ExactAggregator(opentelemetry::metrics::InstrumentKind::IntCounter); - auto agg2 = new ExactAggregator(opentelemetry::metrics::InstrumentKind::DoubleCounter); + auto agg1 = new ExactAggregator(opentelemetry::metrics::InstrumentKind::Counter); + auto agg2 = new ExactAggregator(opentelemetry::metrics::InstrumentKind::ValueRecorder); agg1->update(1); agg2->update(2); @@ -76,10 +76,10 @@ TEST(ExactAggregatorOrdered, Types) { // This test verifies that we do not encounter any errors when // using various numeric types. - auto agg_int = new ExactAggregator(opentelemetry::metrics::InstrumentKind::IntCounter); - auto agg_long = new ExactAggregator(opentelemetry::metrics::InstrumentKind::IntCounter); - auto agg_float = new ExactAggregator(opentelemetry::metrics::InstrumentKind::IntCounter); - auto agg_double = new ExactAggregator(opentelemetry::metrics::InstrumentKind::IntCounter); + auto agg_int = new ExactAggregator(opentelemetry::metrics::InstrumentKind::Counter); + auto agg_long = new ExactAggregator(opentelemetry::metrics::InstrumentKind::Counter); + auto agg_float = new ExactAggregator(opentelemetry::metrics::InstrumentKind::Counter); + auto agg_double = new ExactAggregator(opentelemetry::metrics::InstrumentKind::Counter); for (int i = 1; i <= 5; ++i) { @@ -106,7 +106,7 @@ TEST(ExactAggregatorOrdered, Types) TEST(ExactAggregatorQuant, Update) { - auto agg = new ExactAggregator(opentelemetry::metrics::InstrumentKind::IntCounter, true); + auto agg = new ExactAggregator(opentelemetry::metrics::InstrumentKind::Counter, true); std::vector correct; @@ -130,7 +130,7 @@ TEST(ExactAggregatorQuant, Checkpoint) // This test verifies that the aggregator updates correctly when // quantile estimation is turned on. - auto agg = new ExactAggregator(opentelemetry::metrics::InstrumentKind::IntCounter, true); + auto agg = new ExactAggregator(opentelemetry::metrics::InstrumentKind::Counter, true); std::vector correct; @@ -153,7 +153,7 @@ TEST(ExactAggregatorQuant, Quantile) // This test verifies that the quantile estimation function returns // the correct values. - auto agg = new ExactAggregator(opentelemetry::metrics::InstrumentKind::IntCounter, true); + auto agg = new ExactAggregator(opentelemetry::metrics::InstrumentKind::Counter, true); std::vector tmp {3, 9, 42, 57, 163, 210, 272, 300}; for (int i : tmp) @@ -170,7 +170,7 @@ TEST(ExactAggregatorInOrder, Quantile) { // This test verifies that if the user has an exact aggregator in "in-order" mode // an exception will be thrown if they call the quantile() function. - auto agg = new ExactAggregator(opentelemetry::metrics::InstrumentKind::IntCounter); + auto agg = new ExactAggregator(opentelemetry::metrics::InstrumentKind::Counter); std::vector tmp {3, 9, 42, 57, 163, 210, 272, 300}; for (int i : tmp) @@ -193,7 +193,7 @@ TEST(ExactAggregatorQuant, Concurrency) { // This test checks that the aggregator updates appropriately // when called in a multi-threaded context. - auto agg = new ExactAggregator(opentelemetry::metrics::InstrumentKind::IntCounter, true); + auto agg = new ExactAggregator(opentelemetry::metrics::InstrumentKind::Counter, true); std::thread first(&callback, std::ref(*agg)); std::thread second(&callback, std::ref(*agg)); From a5bd9ee40e706782867f19a1aaee12a766b3059a Mon Sep 17 00:00:00 2001 From: Brandon Kimberly Date: Fri, 24 Jul 2020 15:49:44 -0400 Subject: [PATCH 08/20] Update to support Aggregator base class changes --- .../sdk/metrics/aggregator/exact_aggregator.h | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/metrics/aggregator/exact_aggregator.h b/sdk/include/opentelemetry/sdk/metrics/aggregator/exact_aggregator.h index 15d00d56f5..047116da3f 100644 --- a/sdk/include/opentelemetry/sdk/metrics/aggregator/exact_aggregator.h +++ b/sdk/include/opentelemetry/sdk/metrics/aggregator/exact_aggregator.h @@ -43,6 +43,8 @@ class ExactAggregator : public Aggregator quant_estimation_ = quant_estimation; } + ~ExactAggregator() = default; + /** * Receives a captured value from the instrument and adds it to the values_ vector. * @@ -98,7 +100,7 @@ class ExactAggregator : public Aggregator * @param q the quantile to estimate. 0 <= q <= 1 * @return the nearest value in the vector to the exact quantile. */ - T quantile(float q) + T get_quantiles(double q) override { if (!quant_estimation_) { @@ -136,12 +138,6 @@ class ExactAggregator : public Aggregator { return this->values_; } - - bool get_quant_estimation() override - { - return quant_estimation_; - } - private: bool quant_estimation_; // Used to switch between in-order and quantile estimation modes }; From 1ed40ac457497f60740f7d7f62943654209ea920 Mon Sep 17 00:00:00 2001 From: Brandon Kimberly Date: Fri, 24 Jul 2020 15:50:17 -0400 Subject: [PATCH 09/20] Change name quantile() to get_quantiles() --- sdk/test/metrics/exact_aggregator_test.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/sdk/test/metrics/exact_aggregator_test.cc b/sdk/test/metrics/exact_aggregator_test.cc index d00101365e..201421a48c 100644 --- a/sdk/test/metrics/exact_aggregator_test.cc +++ b/sdk/test/metrics/exact_aggregator_test.cc @@ -161,9 +161,9 @@ TEST(ExactAggregatorQuant, Quantile) agg->update(i); } agg->checkpoint(); - ASSERT_EQ(agg->quantile(.25), 42); - ASSERT_EQ(agg->quantile(0.5), 163); - ASSERT_EQ(agg->quantile(0.75), 272); + ASSERT_EQ(agg->get_quantiles(.25), 42); + ASSERT_EQ(agg->get_quantiles(0.5), 163); + ASSERT_EQ(agg->get_quantiles(0.75), 272); } TEST(ExactAggregatorInOrder, Quantile) @@ -178,7 +178,7 @@ TEST(ExactAggregatorInOrder, Quantile) agg->update(i); } agg->checkpoint(); - ASSERT_THROW(agg->quantile(0.5), std::domain_error); + ASSERT_THROW(agg->get_quantiles(0.5), std::domain_error); } void callback(ExactAggregator &agg) From 926a97c46248be5be8eadb668971baf7cfc08859 Mon Sep 17 00:00:00 2001 From: Brandon Kimberly Date: Sun, 26 Jul 2020 00:03:15 -0400 Subject: [PATCH 10/20] Add copy constructor --- .../sdk/metrics/aggregator/exact_aggregator.h | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/sdk/include/opentelemetry/sdk/metrics/aggregator/exact_aggregator.h b/sdk/include/opentelemetry/sdk/metrics/aggregator/exact_aggregator.h index 047116da3f..2794087458 100644 --- a/sdk/include/opentelemetry/sdk/metrics/aggregator/exact_aggregator.h +++ b/sdk/include/opentelemetry/sdk/metrics/aggregator/exact_aggregator.h @@ -45,6 +45,16 @@ class ExactAggregator : public Aggregator ~ExactAggregator() = default; + ExactAggregator(const ExactAggregator &cp) + { + this->values_ = cp.values_; + this->checkpoint_ = cp.checkpoint_; + this->kind_ = cp.kind_; + this->agg_kind_ = cp.agg_kind_; + quant_estimation_ = cp.quant_estimation_; + // use default initialized mutex as they cannot be copied + } + /** * Receives a captured value from the instrument and adds it to the values_ vector. * From bc4dcb9dea5760c2c12be20b90612729870bc37a Mon Sep 17 00:00:00 2001 From: Brandon Kimberly Date: Sun, 26 Jul 2020 00:11:49 -0400 Subject: [PATCH 11/20] Add get_quant_estimation() function --- .../opentelemetry/sdk/metrics/aggregator/exact_aggregator.h | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/sdk/include/opentelemetry/sdk/metrics/aggregator/exact_aggregator.h b/sdk/include/opentelemetry/sdk/metrics/aggregator/exact_aggregator.h index 2794087458..4050ca5694 100644 --- a/sdk/include/opentelemetry/sdk/metrics/aggregator/exact_aggregator.h +++ b/sdk/include/opentelemetry/sdk/metrics/aggregator/exact_aggregator.h @@ -148,6 +148,12 @@ class ExactAggregator : public Aggregator { return this->values_; } + + bool get_quant_estimation() override + { + return quant_estimation_; + } + private: bool quant_estimation_; // Used to switch between in-order and quantile estimation modes }; From 3fc6fd1a94c3568b4460767f4201cb098b0f8fe6 Mon Sep 17 00:00:00 2001 From: Brandon Kimberly Date: Mon, 27 Jul 2020 18:54:18 -0400 Subject: [PATCH 12/20] Add check to __EXCEPTIONS env var before throwing --- .../sdk/metrics/aggregator/exact_aggregator.h | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/metrics/aggregator/exact_aggregator.h b/sdk/include/opentelemetry/sdk/metrics/aggregator/exact_aggregator.h index 4050ca5694..5fe2113014 100644 --- a/sdk/include/opentelemetry/sdk/metrics/aggregator/exact_aggregator.h +++ b/sdk/include/opentelemetry/sdk/metrics/aggregator/exact_aggregator.h @@ -115,12 +115,20 @@ class ExactAggregator : public Aggregator if (!quant_estimation_) { // Log error - throw std::domain_error("Exact aggregator is not in quantile estimation mode!"); + #if __EXCEPTIONS + throw std::domain_error("Exact aggregator is not in quantile estimation mode!"); + #else + std::terminate(); + #endif } if (this->checkpoint_.size() == 0 || q < 0 || q > 1) { // Log error - throw std::invalid_argument("Arg 'q' must be between 0 and 1, inclusive"); + #if __EXCEPTIONS + throw std::invalid_argument("Arg 'q' must be between 0 and 1, inclusive"); + #else + std::terminate(); + #endif } else if (q == 0 || this->checkpoint_.size() == 1) { @@ -148,12 +156,12 @@ class ExactAggregator : public Aggregator { return this->values_; } - + bool get_quant_estimation() override { return quant_estimation_; } - + private: bool quant_estimation_; // Used to switch between in-order and quantile estimation modes }; From 483b6a9c5ea5083e0883c5428e5b31fd51108f65 Mon Sep 17 00:00:00 2001 From: Brandon Kimberly Date: Tue, 28 Jul 2020 09:09:34 -0400 Subject: [PATCH 13/20] Fix exact aggregator test name --- sdk/test/metrics/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/test/metrics/CMakeLists.txt b/sdk/test/metrics/CMakeLists.txt index 6acf20b14c..9a41923b17 100644 --- a/sdk/test/metrics/CMakeLists.txt +++ b/sdk/test/metrics/CMakeLists.txt @@ -1,4 +1,4 @@ -foreach(testname meter_provider_sdk_test exact_aggregator) +foreach(testname meter_provider_sdk_test exact_aggregator_test) add_executable(${testname} "${testname}.cc") target_link_libraries(${testname} ${GTEST_BOTH_LIBRARIES} ${CMAKE_THREAD_LIBS_INIT} opentelemetry_common opentelemetry_metrics) From 1b7889506b2b652d8bdfd6b59b62003f742d44f4 Mon Sep 17 00:00:00 2001 From: Brandon Kimberly Date: Tue, 28 Jul 2020 19:52:21 -0400 Subject: [PATCH 14/20] Merge checkpoints as well as values --- .../opentelemetry/sdk/metrics/aggregator/exact_aggregator.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/sdk/include/opentelemetry/sdk/metrics/aggregator/exact_aggregator.h b/sdk/include/opentelemetry/sdk/metrics/aggregator/exact_aggregator.h index 5fe2113014..63f338f229 100644 --- a/sdk/include/opentelemetry/sdk/metrics/aggregator/exact_aggregator.h +++ b/sdk/include/opentelemetry/sdk/metrics/aggregator/exact_aggregator.h @@ -94,7 +94,10 @@ class ExactAggregator : public Aggregator if (this->kind_ == other.kind_) { this->mu_.lock(); + // First merge values this->values_.insert(this->values_.end(), other.values_.begin(), other.values_.end()); + // Now merge checkpoints + this->checkpoint_.insert(this->checkpoint_.end(), other.checkpoint_.begin(), other.checkpoint_.end()); this->mu_.unlock(); } else From 291bd696f5916a942f10376f2fda48f01a4b767f Mon Sep 17 00:00:00 2001 From: Brandon Kimberly Date: Wed, 29 Jul 2020 16:31:13 -0400 Subject: [PATCH 15/20] Format --- .../sdk/metrics/aggregator/exact_aggregator.h | 62 ++++++++----------- sdk/test/metrics/CMakeLists.txt | 5 +- sdk/test/metrics/exact_aggregator_test.cc | 15 ++--- 3 files changed, 38 insertions(+), 44 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/metrics/aggregator/exact_aggregator.h b/sdk/include/opentelemetry/sdk/metrics/aggregator/exact_aggregator.h index 63f338f229..d292141c97 100644 --- a/sdk/include/opentelemetry/sdk/metrics/aggregator/exact_aggregator.h +++ b/sdk/include/opentelemetry/sdk/metrics/aggregator/exact_aggregator.h @@ -30,16 +30,16 @@ namespace metrics * * @tparam T the type of values stored in this aggregator. */ -template +template class ExactAggregator : public Aggregator { public: ExactAggregator(metrics_api::InstrumentKind kind, bool quant_estimation = false) { static_assert(std::is_arithmetic::value, "Not an arithmetic type"); - this->kind_ = kind; + this->kind_ = kind; this->checkpoint_ = this->values_; - this->agg_kind_ = AggregatorKind::Exact; + this->agg_kind_ = AggregatorKind::Exact; quant_estimation_ = quant_estimation; } @@ -47,10 +47,10 @@ class ExactAggregator : public Aggregator ExactAggregator(const ExactAggregator &cp) { - this->values_ = cp.values_; + this->values_ = cp.values_; this->checkpoint_ = cp.checkpoint_; - this->kind_ = cp.kind_; - this->agg_kind_ = cp.agg_kind_; + this->kind_ = cp.kind_; + this->agg_kind_ = cp.agg_kind_; quant_estimation_ = cp.quant_estimation_; // use default initialized mutex as they cannot be copied } @@ -97,7 +97,8 @@ class ExactAggregator : public Aggregator // First merge values this->values_.insert(this->values_.end(), other.values_.begin(), other.values_.end()); // Now merge checkpoints - this->checkpoint_.insert(this->checkpoint_.end(), other.checkpoint_.begin(), other.checkpoint_.end()); + this->checkpoint_.insert(this->checkpoint_.end(), other.checkpoint_.begin(), + other.checkpoint_.end()); this->mu_.unlock(); } else @@ -117,21 +118,21 @@ class ExactAggregator : public Aggregator { if (!quant_estimation_) { - // Log error - #if __EXCEPTIONS - throw std::domain_error("Exact aggregator is not in quantile estimation mode!"); - #else - std::terminate(); - #endif +// Log error +#if __EXCEPTIONS + throw std::domain_error("Exact aggregator is not in quantile estimation mode!"); +#else + std::terminate(); +#endif } if (this->checkpoint_.size() == 0 || q < 0 || q > 1) { - // Log error - #if __EXCEPTIONS - throw std::invalid_argument("Arg 'q' must be between 0 and 1, inclusive"); - #else - std::terminate(); - #endif +// Log error +#if __EXCEPTIONS + throw std::invalid_argument("Arg 'q' must be between 0 and 1, inclusive"); +#else + std::terminate(); +#endif } else if (q == 0 || this->checkpoint_.size() == 1) { @@ -144,30 +145,21 @@ class ExactAggregator : public Aggregator else { float position = float(this->checkpoint_.size() - 1) * q; - int ceiling = ceil(position); + int ceiling = ceil(position); return this->checkpoint_[ceiling]; } } //////////////////////////ACCESSOR FUNCTIONS////////////////////////// - std::vector get_checkpoint() override - { - return this->checkpoint_; - } + std::vector get_checkpoint() override { return this->checkpoint_; } - std::vector get_values() override - { - return this->values_; - } + std::vector get_values() override { return this->values_; } - bool get_quant_estimation() override - { - return quant_estimation_; - } + bool get_quant_estimation() override { return quant_estimation_; } private: - bool quant_estimation_; // Used to switch between in-order and quantile estimation modes + bool quant_estimation_; // Used to switch between in-order and quantile estimation modes }; -} -} +} // namespace metrics +} // namespace sdk OPENTELEMETRY_END_NAMESPACE diff --git a/sdk/test/metrics/CMakeLists.txt b/sdk/test/metrics/CMakeLists.txt index 9a41923b17..5d94f3b8d6 100644 --- a/sdk/test/metrics/CMakeLists.txt +++ b/sdk/test/metrics/CMakeLists.txt @@ -1,6 +1,7 @@ foreach(testname meter_provider_sdk_test exact_aggregator_test) add_executable(${testname} "${testname}.cc") - target_link_libraries(${testname} ${GTEST_BOTH_LIBRARIES} - ${CMAKE_THREAD_LIBS_INIT} opentelemetry_common opentelemetry_metrics) + target_link_libraries( + ${testname} ${GTEST_BOTH_LIBRARIES} ${CMAKE_THREAD_LIBS_INIT} + opentelemetry_common opentelemetry_metrics) gtest_add_tests(TARGET ${testname} TEST_PREFIX metrics. TEST_LIST ${testname}) endforeach() diff --git a/sdk/test/metrics/exact_aggregator_test.cc b/sdk/test/metrics/exact_aggregator_test.cc index 201421a48c..55bf9d5703 100644 --- a/sdk/test/metrics/exact_aggregator_test.cc +++ b/sdk/test/metrics/exact_aggregator_test.cc @@ -76,9 +76,9 @@ TEST(ExactAggregatorOrdered, Types) { // This test verifies that we do not encounter any errors when // using various numeric types. - auto agg_int = new ExactAggregator(opentelemetry::metrics::InstrumentKind::Counter); - auto agg_long = new ExactAggregator(opentelemetry::metrics::InstrumentKind::Counter); - auto agg_float = new ExactAggregator(opentelemetry::metrics::InstrumentKind::Counter); + auto agg_int = new ExactAggregator(opentelemetry::metrics::InstrumentKind::Counter); + auto agg_long = new ExactAggregator(opentelemetry::metrics::InstrumentKind::Counter); + auto agg_float = new ExactAggregator(opentelemetry::metrics::InstrumentKind::Counter); auto agg_double = new ExactAggregator(opentelemetry::metrics::InstrumentKind::Counter); for (int i = 1; i <= 5; ++i) @@ -141,13 +141,14 @@ TEST(ExactAggregatorQuant, Checkpoint) agg->update(-1); // The vector MUST be sorted when checkpointed - correct.push_back(-1); correct.push_back(0); correct.push_back(1); + correct.push_back(-1); + correct.push_back(0); + correct.push_back(1); agg->checkpoint(); ASSERT_EQ(agg->get_checkpoint(), correct); } - TEST(ExactAggregatorQuant, Quantile) { // This test verifies that the quantile estimation function returns @@ -155,7 +156,7 @@ TEST(ExactAggregatorQuant, Quantile) auto agg = new ExactAggregator(opentelemetry::metrics::InstrumentKind::Counter, true); - std::vector tmp {3, 9, 42, 57, 163, 210, 272, 300}; + std::vector tmp{3, 9, 42, 57, 163, 210, 272, 300}; for (int i : tmp) { agg->update(i); @@ -172,7 +173,7 @@ TEST(ExactAggregatorInOrder, Quantile) // an exception will be thrown if they call the quantile() function. auto agg = new ExactAggregator(opentelemetry::metrics::InstrumentKind::Counter); - std::vector tmp {3, 9, 42, 57, 163, 210, 272, 300}; + std::vector tmp{3, 9, 42, 57, 163, 210, 272, 300}; for (int i : tmp) { agg->update(i); From e7acbf8d58b9da920f6ac92e92212d095a33e43a Mon Sep 17 00:00:00 2001 From: Brandon Kimberly Date: Thu, 30 Jul 2020 09:59:16 -0400 Subject: [PATCH 16/20] Allocate aggs on the stack --- sdk/test/metrics/exact_aggregator_test.cc | 137 +++++++++++----------- 1 file changed, 68 insertions(+), 69 deletions(-) diff --git a/sdk/test/metrics/exact_aggregator_test.cc b/sdk/test/metrics/exact_aggregator_test.cc index 55bf9d5703..4543c865bd 100644 --- a/sdk/test/metrics/exact_aggregator_test.cc +++ b/sdk/test/metrics/exact_aggregator_test.cc @@ -7,90 +7,90 @@ using namespace opentelemetry::sdk::metrics; TEST(ExactAggregatorOrdered, Update) { - auto agg = new ExactAggregator(opentelemetry::metrics::InstrumentKind::Counter); + ExactAggregator agg(opentelemetry::metrics::InstrumentKind::Counter); std::vector correct; - ASSERT_EQ(agg->get_values(), correct); + ASSERT_EQ(agg.get_values(), correct); - agg->update(1); + agg.update(1); correct.push_back(1); - ASSERT_EQ(agg->get_values(), std::vector{1}); + ASSERT_EQ(agg.get_values(), std::vector{1}); for (int i = 2; i <= 5; ++i) { correct.push_back(i); - agg->update(i); + agg.update(i); } - ASSERT_EQ(agg->get_values(), correct); + ASSERT_EQ(agg.get_values(), correct); } TEST(ExactAggregatorOrdered, Checkpoint) { - auto agg = new ExactAggregator(opentelemetry::metrics::InstrumentKind::Counter); + ExactAggregator agg(opentelemetry::metrics::InstrumentKind::Counter); std::vector correct; - ASSERT_EQ(agg->get_checkpoint(), correct); + ASSERT_EQ(agg.get_checkpoint(), correct); - agg->update(1); + agg.update(1); correct.push_back(1); - agg->checkpoint(); + agg.checkpoint(); - ASSERT_EQ(agg->get_checkpoint(), correct); + ASSERT_EQ(agg.get_checkpoint(), correct); } TEST(ExactAggregatorOrdered, Merge) { - auto agg1 = new ExactAggregator(opentelemetry::metrics::InstrumentKind::Counter); - auto agg2 = new ExactAggregator(opentelemetry::metrics::InstrumentKind::Counter); + ExactAggregator agg1(opentelemetry::metrics::InstrumentKind::Counter); + ExactAggregator agg2(opentelemetry::metrics::InstrumentKind::Counter); - agg1->update(1); - agg2->update(2); - agg1->merge(*agg2); + agg1.update(1); + agg2.update(2); + agg1.merge(agg2); std::vector correct{1, 2}; - ASSERT_EQ(agg1->get_values(), correct); + ASSERT_EQ(agg1.get_values(), correct); } TEST(ExactAggregatorOrdered, BadMerge) { // This verifies that we encounter and error when we try to merge // two aggregators of different numeric types together. - auto agg1 = new ExactAggregator(opentelemetry::metrics::InstrumentKind::Counter); - auto agg2 = new ExactAggregator(opentelemetry::metrics::InstrumentKind::ValueRecorder); + ExactAggregator agg1(opentelemetry::metrics::InstrumentKind::Counter); + ExactAggregator agg2(opentelemetry::metrics::InstrumentKind::ValueRecorder); - agg1->update(1); - agg2->update(2); + agg1.update(1); + agg2.update(2); - agg1->merge(*agg2); + agg1.merge(agg2); // Verify that the aggregators did NOT merge std::vector correct{1}; - ASSERT_EQ(agg1->get_values(), correct); + ASSERT_EQ(agg1.get_values(), correct); } TEST(ExactAggregatorOrdered, Types) { // This test verifies that we do not encounter any errors when // using various numeric types. - auto agg_int = new ExactAggregator(opentelemetry::metrics::InstrumentKind::Counter); - auto agg_long = new ExactAggregator(opentelemetry::metrics::InstrumentKind::Counter); - auto agg_float = new ExactAggregator(opentelemetry::metrics::InstrumentKind::Counter); - auto agg_double = new ExactAggregator(opentelemetry::metrics::InstrumentKind::Counter); + ExactAggregator agg_int(opentelemetry::metrics::InstrumentKind::Counter); + ExactAggregator agg_long(opentelemetry::metrics::InstrumentKind::Counter); + ExactAggregator agg_float(opentelemetry::metrics::InstrumentKind::Counter); + ExactAggregator agg_double(opentelemetry::metrics::InstrumentKind::Counter); for (int i = 1; i <= 5; ++i) { - agg_int->update(i); - agg_long->update(i); + agg_int.update(i); + agg_long.update(i); } for (float i = 1.0; i <= 5.0; i += 1) { - agg_float->update(i); - agg_double->update(i); + agg_float.update(i); + agg_double.update(i); } std::vector correct_int{1, 2, 3, 4, 5}; @@ -98,31 +98,31 @@ TEST(ExactAggregatorOrdered, Types) std::vector correct_float{1.0, 2.0, 3.0, 4.0, 5.0}; std::vector correct_double{1.0, 2.0, 3.0, 4.0, 5.0}; - ASSERT_EQ(agg_int->get_values(), correct_int); - ASSERT_EQ(agg_long->get_values(), correct_long); - ASSERT_EQ(agg_float->get_values(), correct_float); - ASSERT_EQ(agg_double->get_values(), correct_double); + ASSERT_EQ(agg_int.get_values(), correct_int); + ASSERT_EQ(agg_long.get_values(), correct_long); + ASSERT_EQ(agg_float.get_values(), correct_float); + ASSERT_EQ(agg_double.get_values(), correct_double); } TEST(ExactAggregatorQuant, Update) { - auto agg = new ExactAggregator(opentelemetry::metrics::InstrumentKind::Counter, true); + ExactAggregator agg(opentelemetry::metrics::InstrumentKind::Counter, true); std::vector correct; - ASSERT_EQ(agg->get_values(), correct); + ASSERT_EQ(agg.get_values(), correct); - agg->update(1); + agg.update(1); correct.push_back(1); - ASSERT_EQ(agg->get_values(), std::vector{1}); + ASSERT_EQ(agg.get_values(), std::vector{1}); for (int i = 2; i <= 5; ++i) { correct.push_back(i); - agg->update(i); + agg.update(i); } - ASSERT_EQ(agg->get_values(), correct); + ASSERT_EQ(agg.get_values(), correct); } TEST(ExactAggregatorQuant, Checkpoint) @@ -130,56 +130,55 @@ TEST(ExactAggregatorQuant, Checkpoint) // This test verifies that the aggregator updates correctly when // quantile estimation is turned on. - auto agg = new ExactAggregator(opentelemetry::metrics::InstrumentKind::Counter, true); + ExactAggregator agg(opentelemetry::metrics::InstrumentKind::Counter, true); std::vector correct; - ASSERT_EQ(agg->get_checkpoint(), correct); + ASSERT_EQ(agg.get_checkpoint(), correct); - agg->update(1); - agg->update(0); - agg->update(-1); + agg.update(1); + agg.update(0); + agg.update(-1); // The vector MUST be sorted when checkpointed - correct.push_back(-1); - correct.push_back(0); - correct.push_back(1); - agg->checkpoint(); + correct.push_back(-1); correct.push_back(0); correct.push_back(1); + agg.checkpoint(); - ASSERT_EQ(agg->get_checkpoint(), correct); + ASSERT_EQ(agg.get_checkpoint(), correct); } + TEST(ExactAggregatorQuant, Quantile) { // This test verifies that the quantile estimation function returns // the correct values. - auto agg = new ExactAggregator(opentelemetry::metrics::InstrumentKind::Counter, true); + ExactAggregator agg(opentelemetry::metrics::InstrumentKind::Counter, true); - std::vector tmp{3, 9, 42, 57, 163, 210, 272, 300}; + std::vector tmp {3, 9, 42, 57, 163, 210, 272, 300}; for (int i : tmp) { - agg->update(i); + agg.update(i); } - agg->checkpoint(); - ASSERT_EQ(agg->get_quantiles(.25), 42); - ASSERT_EQ(agg->get_quantiles(0.5), 163); - ASSERT_EQ(agg->get_quantiles(0.75), 272); + agg.checkpoint(); + ASSERT_EQ(agg.get_quantiles(.25), 42); + ASSERT_EQ(agg.get_quantiles(0.5), 163); + ASSERT_EQ(agg.get_quantiles(0.75), 272); } TEST(ExactAggregatorInOrder, Quantile) { // This test verifies that if the user has an exact aggregator in "in-order" mode // an exception will be thrown if they call the quantile() function. - auto agg = new ExactAggregator(opentelemetry::metrics::InstrumentKind::Counter); + ExactAggregator agg(opentelemetry::metrics::InstrumentKind::Counter); - std::vector tmp{3, 9, 42, 57, 163, 210, 272, 300}; + std::vector tmp {3, 9, 42, 57, 163, 210, 272, 300}; for (int i : tmp) { - agg->update(i); + agg.update(i); } - agg->checkpoint(); - ASSERT_THROW(agg->get_quantiles(0.5), std::domain_error); + agg.checkpoint(); + ASSERT_THROW(agg.get_quantiles(0.5), std::domain_error); } void callback(ExactAggregator &agg) @@ -194,10 +193,10 @@ TEST(ExactAggregatorQuant, Concurrency) { // This test checks that the aggregator updates appropriately // when called in a multi-threaded context. - auto agg = new ExactAggregator(opentelemetry::metrics::InstrumentKind::Counter, true); + ExactAggregator agg(opentelemetry::metrics::InstrumentKind::Counter, true); - std::thread first(&callback, std::ref(*agg)); - std::thread second(&callback, std::ref(*agg)); + std::thread first(&callback, std::ref(agg)); + std::thread second(&callback, std::ref(agg)); first.join(); second.join(); @@ -208,7 +207,7 @@ TEST(ExactAggregatorQuant, Concurrency) correct.push_back(i); correct.push_back(i); } - agg->checkpoint(); + agg.checkpoint(); - ASSERT_EQ(agg->get_checkpoint(), correct); -} + ASSERT_EQ(agg.get_checkpoint(), correct); +} \ No newline at end of file From b571d092c2afae87016209cb4cf2e780325f9f2d Mon Sep 17 00:00:00 2001 From: Brandon Kimberly Date: Thu, 30 Jul 2020 10:05:25 -0400 Subject: [PATCH 17/20] Format --- sdk/test/metrics/exact_aggregator_test.cc | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/sdk/test/metrics/exact_aggregator_test.cc b/sdk/test/metrics/exact_aggregator_test.cc index 4543c865bd..0afafdf821 100644 --- a/sdk/test/metrics/exact_aggregator_test.cc +++ b/sdk/test/metrics/exact_aggregator_test.cc @@ -141,13 +141,14 @@ TEST(ExactAggregatorQuant, Checkpoint) agg.update(-1); // The vector MUST be sorted when checkpointed - correct.push_back(-1); correct.push_back(0); correct.push_back(1); + correct.push_back(-1); + correct.push_back(0); + correct.push_back(1); agg.checkpoint(); ASSERT_EQ(agg.get_checkpoint(), correct); } - TEST(ExactAggregatorQuant, Quantile) { // This test verifies that the quantile estimation function returns @@ -155,7 +156,7 @@ TEST(ExactAggregatorQuant, Quantile) ExactAggregator agg(opentelemetry::metrics::InstrumentKind::Counter, true); - std::vector tmp {3, 9, 42, 57, 163, 210, 272, 300}; + std::vector tmp{3, 9, 42, 57, 163, 210, 272, 300}; for (int i : tmp) { agg.update(i); @@ -172,7 +173,7 @@ TEST(ExactAggregatorInOrder, Quantile) // an exception will be thrown if they call the quantile() function. ExactAggregator agg(opentelemetry::metrics::InstrumentKind::Counter); - std::vector tmp {3, 9, 42, 57, 163, 210, 272, 300}; + std::vector tmp{3, 9, 42, 57, 163, 210, 272, 300}; for (int i : tmp) { agg.update(i); From c19b6076207cf17d19072ccf5adc7282e1565daf Mon Sep 17 00:00:00 2001 From: Brandon Kimberly Date: Thu, 30 Jul 2020 10:22:18 -0400 Subject: [PATCH 18/20] Remove include --- .../opentelemetry/sdk/metrics/aggregator/exact_aggregator.h | 1 - 1 file changed, 1 deletion(-) diff --git a/sdk/include/opentelemetry/sdk/metrics/aggregator/exact_aggregator.h b/sdk/include/opentelemetry/sdk/metrics/aggregator/exact_aggregator.h index d292141c97..00ec1ae12d 100644 --- a/sdk/include/opentelemetry/sdk/metrics/aggregator/exact_aggregator.h +++ b/sdk/include/opentelemetry/sdk/metrics/aggregator/exact_aggregator.h @@ -7,7 +7,6 @@ #include #include #include -#include #include namespace metrics_api = opentelemetry::metrics; From 234813d8d2131787650257a011d30eaa7ae546d4 Mon Sep 17 00:00:00 2001 From: Brandon Kimberly Date: Thu, 30 Jul 2020 10:30:46 -0400 Subject: [PATCH 19/20] Add exception checks --- sdk/test/metrics/exact_aggregator_test.cc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/sdk/test/metrics/exact_aggregator_test.cc b/sdk/test/metrics/exact_aggregator_test.cc index 0afafdf821..be32b87704 100644 --- a/sdk/test/metrics/exact_aggregator_test.cc +++ b/sdk/test/metrics/exact_aggregator_test.cc @@ -179,7 +179,10 @@ TEST(ExactAggregatorInOrder, Quantile) agg.update(i); } agg.checkpoint(); +#if __EXCEPTIONS ASSERT_THROW(agg.get_quantiles(0.5), std::domain_error); +#else +#endif } void callback(ExactAggregator &agg) From 8f5db42416764a1450971a4da50f3b08f9e8d63d Mon Sep 17 00:00:00 2001 From: Brandon Kimberly Date: Thu, 30 Jul 2020 11:00:57 -0400 Subject: [PATCH 20/20] Format --- sdk/test/metrics/CMakeLists.txt | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/sdk/test/metrics/CMakeLists.txt b/sdk/test/metrics/CMakeLists.txt index d8f75a197f..7ad1d9e81e 100644 --- a/sdk/test/metrics/CMakeLists.txt +++ b/sdk/test/metrics/CMakeLists.txt @@ -1,7 +1,5 @@ - foreach(testname meter_provider_sdk_test gauge_aggregator_test - min_max_sum_count_aggregator_test - exact_aggregator_test) + min_max_sum_count_aggregator_test exact_aggregator_test) add_executable(${testname} "${testname}.cc") target_link_libraries(${testname} ${GTEST_BOTH_LIBRARIES} ${CMAKE_THREAD_LIBS_INIT} opentelemetry_metrics)