From 908d762631ee23bb28bc190956ca7b31c889dead Mon Sep 17 00:00:00 2001 From: erichsueh3 Date: Tue, 18 Aug 2020 10:32:04 -0700 Subject: [PATCH 1/7] PrometheusCollector header, source, test, and build files --- exporters/prometheus/BUILD | 46 ++ exporters/prometheus/CMakeLists.txt | 23 + .../prometheus/prometheus_collector.h | 100 +++ .../prometheus/src/prometheus_collector.cc | 169 ++++ exporters/prometheus/test/CMakeLists.txt | 8 + .../test/prometheus_collector_test.cc | 774 ++++++++++++++++++ 6 files changed, 1120 insertions(+) create mode 100644 exporters/prometheus/BUILD create mode 100644 exporters/prometheus/CMakeLists.txt create mode 100644 exporters/prometheus/include/opentelemetry/exporters/prometheus/prometheus_collector.h create mode 100644 exporters/prometheus/src/prometheus_collector.cc create mode 100644 exporters/prometheus/test/CMakeLists.txt create mode 100644 exporters/prometheus/test/prometheus_collector_test.cc diff --git a/exporters/prometheus/BUILD b/exporters/prometheus/BUILD new file mode 100644 index 0000000000..05ab8ee476 --- /dev/null +++ b/exporters/prometheus/BUILD @@ -0,0 +1,46 @@ +# Copyright 2020, OpenTelemetry Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +package(default_visibility = ["//visibility:public"]) + +cc_library( + name = "prometheus_collector", + srcs = [ + "src/prometheus_collector.cc", + ], + hdrs = [ + "include/opentelemetry/exporters/prometheus/prometheus_collector.h", + "include/opentelemetry/exporters/prometheus/prometheus_exporter_utils.h", + ], + strip_include_prefix = "include", + deps = [ + ":prometheus_utils", + "//api", + "//sdk:headers", + "@com_github_jupp0r_prometheus_cpp//core", + ], +) + +cc_test( + name = "prometheus_collector_test", + srcs = [ + "test/prometheus_collector_test.cc", + ], + deps = [ + ":prometheus_collector", + "@com_google_googletest//:gtest_main", + ], +) + + diff --git a/exporters/prometheus/CMakeLists.txt b/exporters/prometheus/CMakeLists.txt new file mode 100644 index 0000000000..f4195d616e --- /dev/null +++ b/exporters/prometheus/CMakeLists.txt @@ -0,0 +1,23 @@ +# Copyright 2020, OpenTelemetry Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +include_directories(include) + +find_package(prometheus-cpp CONFIG REQUIRED) +add_library( + prometheus_exporter src/prometheus_collector.cc) + +if (BUILD_TESTING) + add_subdirectory(test) +endif () \ No newline at end of file diff --git a/exporters/prometheus/include/opentelemetry/exporters/prometheus/prometheus_collector.h b/exporters/prometheus/include/opentelemetry/exporters/prometheus/prometheus_collector.h new file mode 100644 index 0000000000..71aad4d59b --- /dev/null +++ b/exporters/prometheus/include/opentelemetry/exporters/prometheus/prometheus_collector.h @@ -0,0 +1,100 @@ +/* + * Copyright The OpenTelemetry Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +#include +#include +#include + +#include "opentelemetry/sdk/metrics/record.h" +#include "prometheus/collectable.h" +#include "prometheus/metric_family.h" +#include "prometheus_exporter_utils.h" + +namespace prometheus_client = ::prometheus; +namespace metric_sdk = opentelemetry::sdk::metrics; + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace exporter +{ +namespace prometheus +{ +/** + * The Prometheus Collector maintains the intermediate collection in Prometheus Exporter + */ +class PrometheusCollector : public prometheus_client::Collectable +{ +public: + /** + * Default Constructor. + * + * This constructor initializes the collection for metrics to export + * in this class with default capacity + */ + explicit PrometheusCollector(int max_collection_size = 2048); + + /** + * Collects all metrics data from metricsToCollect collection. + * + * @return all metrics in the metricsToCollect snapshot + */ + std::vector Collect() const override; + + /** + * This function is called by export() function and add the collection of + * records to the metricsToCollect collection + * + * @param records a collection of records to add to the metricsToCollect collection + */ + void AddMetricData(const std::vector &records); + + /** + * Get the current collection in the collector. + * + * @return the current metricsToCollect collection + */ + std::vector GetCollection(); + + /** + * Gets the maximum size of the collection. + * + * @return max collection size + */ + int GetMaxCollectionSize() const; + +private: + /** + * Collection of metrics data from the export() function, and to be export + * to user when they send a pull request. This collection is a pointer + * to a collection so Collect() is able to clear the collection, even + * though it is a const function. + */ + std::unique_ptr> metrics_to_collect_; + + /** + * Maximum size of the metricsToCollect collection. + */ + int max_collection_size_; + + /* + * Lock when operating the metricsToCollect collection + */ + mutable std::mutex collection_lock_; +}; +} // namespace prometheus +} // namespace exporter +OPENTELEMETRY_END_NAMESPACE diff --git a/exporters/prometheus/src/prometheus_collector.cc b/exporters/prometheus/src/prometheus_collector.cc new file mode 100644 index 0000000000..dd60cd4f26 --- /dev/null +++ b/exporters/prometheus/src/prometheus_collector.cc @@ -0,0 +1,169 @@ +/* + * Copyright The OpenTelemetry Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include + +#include "opentelemetry/exporters/prometheus/prometheus_collector.h" + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace exporter +{ +namespace prometheus +{ +/** + * Default Constructor. + * + * This constructor initializes the collection for metrics to export + * in this class with default capacity + */ +PrometheusCollector::PrometheusCollector(int max_collection_size) + : max_collection_size_(max_collection_size) +{ + metrics_to_collect_ = + std::unique_ptr>(new std::vector); +} + +/** + * Collects all metrics data from metricsToCollect collection. + * + * @return all metrics in the metricsToCollect snapshot + */ +std::vector PrometheusCollector::Collect() const +{ + if (metrics_to_collect_->empty()) + { + return {}; + } + + std::vector result; + + // copy the intermediate collection, and then clear it + std::vector copied_data; + + collection_lock_.lock(); + copied_data = std::vector(*metrics_to_collect_); + metrics_to_collect_->clear(); + collection_lock_.unlock(); + + result = PrometheusExporterUtils::TranslateToPrometheus(copied_data); + return result; +} + +/** + * This function is called by export() function and add the collection of + * records to the metricsToCollect collection + * + * @param records a collection of records to add to the metricsToCollect collection + */ +void PrometheusCollector::AddMetricData(const std::vector &records) +{ + if (metrics_to_collect_->size() + records.size() <= max_collection_size_ && !records.empty()) + { + /** + * ValidAggregator is a lambda that checks a Record to see if its + * Aggregator is a valid nostd::shared_ptr and not a nullptr. + */ + auto ValidAggregator = [](sdk::metrics::Record record) { + auto aggregator_variant = record.GetAggregator(); + if (nostd::holds_alternative>>( + aggregator_variant)) + { + auto aggregator = + nostd::get>>(aggregator_variant); + if (!aggregator) + { + return false; + } + } + else if (nostd::holds_alternative>>( + aggregator_variant)) + { + auto aggregator = + nostd::get>>(aggregator_variant); + if (!aggregator) + { + return false; + } + } + else if (nostd::holds_alternative>>( + aggregator_variant)) + { + auto aggregator = + nostd::get>>(aggregator_variant); + if (!aggregator) + { + return false; + } + } + else if (nostd::holds_alternative>>( + aggregator_variant)) + { + auto aggregator = + nostd::get>>(aggregator_variant); + if (!aggregator) + { + return false; + } + } + + return true; + }; + + collection_lock_.lock(); + + for (auto &r : records) + { + if (ValidAggregator(r)) + { + metrics_to_collect_->push_back(r); + } + // Drop the record and write to std::cout + else + { + // Cannot call non const functions on const Record r + sdk::metrics::Record c = r; + std::cout << "Dropped Record containing invalid aggregator with name: " + c.GetName() + << std::endl; + } + } + + collection_lock_.unlock(); + } +} + +/** + * Get the current collection in the collector. + * + * @return the current metrics_to_collect collection + */ +std::vector PrometheusCollector::GetCollection() +{ + return *metrics_to_collect_; +} + +/** + * Gets the maximum size of the collection. + * + * @return max collection size + */ +int PrometheusCollector::GetMaxCollectionSize() const +{ + return max_collection_size_; +} + +} // namespace prometheus +} // namespace exporter +OPENTELEMETRY_END_NAMESPACE diff --git a/exporters/prometheus/test/CMakeLists.txt b/exporters/prometheus/test/CMakeLists.txt new file mode 100644 index 0000000000..4aa51418ff --- /dev/null +++ b/exporters/prometheus/test/CMakeLists.txt @@ -0,0 +1,8 @@ +foreach (testname prometheus_collector_test) + add_executable(${testname} "${testname}.cc") + target_link_libraries( + ${testname} ${GTEST_BOTH_LIBRARIES} ${CMAKE_THREAD_LIBS_INIT} + prometheus_exporter prometheus-cpp::pull) + gtest_add_tests(TARGET ${testname} TEST_PREFIX exporter. TEST_LIST ${testname}) +endforeach () + diff --git a/exporters/prometheus/test/prometheus_collector_test.cc b/exporters/prometheus/test/prometheus_collector_test.cc new file mode 100644 index 0000000000..61d2252957 --- /dev/null +++ b/exporters/prometheus/test/prometheus_collector_test.cc @@ -0,0 +1,774 @@ +/* + * Copyright The OpenTelemetry Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include +#include +#include +#include + +#include "opentelemetry/exporters/prometheus/prometheus_collector.h" +#include "opentelemetry/metrics/instrument.h" +#include "opentelemetry/sdk/metrics/aggregator/aggregator.h" +#include "opentelemetry/sdk/metrics/aggregator/counter_aggregator.h" +#include "opentelemetry/sdk/metrics/aggregator/exact_aggregator.h" +#include "opentelemetry/sdk/metrics/aggregator/gauge_aggregator.h" +#include "opentelemetry/sdk/metrics/aggregator/histogram_aggregator.h" +#include "opentelemetry/sdk/metrics/aggregator/min_max_sum_count_aggregator.h" +#include "opentelemetry/sdk/metrics/aggregator/sketch_aggregator.h" +#include "opentelemetry/sdk/metrics/record.h" +#include "opentelemetry/version.h" + +using opentelemetry::exporter::prometheus::PrometheusCollector; + +OPENTELEMETRY_BEGIN_NAMESPACE + +/** + * CreateAgg() is a helper function that returns a + * nostd::shared_ptr given an AggregatorKind + */ +template +std::shared_ptr> CreateAgg(metric_sdk::AggregatorKind kind, + bool exactMode = true) +{ + std::shared_ptr> aggregator; + switch (kind) + { + case metric_sdk::AggregatorKind::Counter: + { + aggregator = std::shared_ptr>( + new metric_sdk::CounterAggregator(opentelemetry::metrics::InstrumentKind::Counter)); + break; + } + case metric_sdk::AggregatorKind::MinMaxSumCount: + { + aggregator = + std::shared_ptr>(new metric_sdk::MinMaxSumCountAggregator( + opentelemetry::metrics::InstrumentKind::Counter)); + break; + } + case metric_sdk::AggregatorKind::Gauge: + { + aggregator = std::shared_ptr>( + new metric_sdk::GaugeAggregator(opentelemetry::metrics::InstrumentKind::Counter)); + break; + } + case metric_sdk::AggregatorKind::Sketch: + { + aggregator = std::shared_ptr>(new metric_sdk::SketchAggregator( + opentelemetry::metrics::InstrumentKind::Counter, 0.000005)); + break; + } + case metric_sdk::AggregatorKind::Histogram: + { + std::vector boundaries{10, 20}; + aggregator = + std::shared_ptr>(new metric_sdk::HistogramAggregator( + opentelemetry::metrics::InstrumentKind::Counter, boundaries)); + break; + } + case metric_sdk::AggregatorKind::Exact: + { + aggregator = std::shared_ptr>(new metric_sdk::ExactAggregator( + opentelemetry::metrics::InstrumentKind::Counter, exactMode)); + break; + } + default: + aggregator = nullptr; + } + return aggregator; +} + +/** + * Populate() updates the aggregator with values and checkpoints it based + * on what its AggregatorKind is + */ +template +void Populate(std::shared_ptr> &aggregator) +{ + if (aggregator->get_aggregator_kind() == metric_sdk::AggregatorKind::Counter) + { + aggregator->update(10.0); + aggregator->update(5.0); + aggregator->checkpoint(); + } + else if (aggregator->get_aggregator_kind() == metric_sdk::AggregatorKind::MinMaxSumCount) + { + aggregator->update(10); + aggregator->update(2); + aggregator->update(5); + aggregator->checkpoint(); + } + else if (aggregator->get_aggregator_kind() == metric_sdk::AggregatorKind::Gauge) + { + aggregator->update(10); + aggregator->update(5); + aggregator->checkpoint(); + } + else if (aggregator->get_aggregator_kind() == metric_sdk::AggregatorKind::Sketch) + { + for (double i = 0; i < 10.0; i++) + { + aggregator->update(i); + } + aggregator->checkpoint(); + } + else if (aggregator->get_aggregator_kind() == metric_sdk::AggregatorKind::Histogram) + { + for (float i = 0; i < 30.0; i++) + { + aggregator->update(i); + } + aggregator->checkpoint(); + } + else if (aggregator->get_aggregator_kind() == metric_sdk::AggregatorKind::Exact) + { + for (double i = 0; i < 10.0; i++) + { + aggregator->update(i); + } + aggregator->checkpoint(); + } +} + +/** + * Helper function to create a collection of records taken from + * a aggregator of specified AggregatorKind + */ +template +std::vector CreateRecords(int num, + metric_sdk::AggregatorKind kind, + bool exactMode = true) +{ + std::vector records; + + for (int i = 0; i < num; i++) + { + std::string name = "record-" + std::to_string(i); + std::string description = "record " + std::to_string(i) + " for test purpose"; + std::string labels = "{label1:v1,label2:v2,}"; + std::shared_ptr> aggregator = CreateAgg(kind, exactMode); + Populate(aggregator); + + metric_sdk::Record r{name, description, labels, aggregator}; + records.push_back(r); + } + return records; +} + +// ==================== Test for addMetricsData() function ====================== + +/** + * AddMetricData() should be able to successfully add a collection + * of Records with Counter Aggregators. It checks that the cumulative + * sum of updates to the aggregator of a record before and after AddMetricData() + * is called are equal. + */ +TEST(PrometheusCollector, AddMetricDataWithCounterRecordsSuccessfully) +{ + PrometheusCollector collector; + + // number of records to create + int num_records = 2; + + // construct a collection of records with CounterAggregators and double + std::vector records = + CreateRecords(num_records, metric_sdk::AggregatorKind::Counter); + + // add records to collection + collector.AddMetricData(records); + + // Collection size should be the same as the size + // of the records collection passed to addMetricData() + ASSERT_EQ(collector.GetCollection().size(), records.size()); + + // check values of records created vs records from metricsToCollect, + // accessed by getCollection() + + for (int i = 0; i < num_records; i++) + { + metric_sdk::Record before = records[i]; + metric_sdk::Record after = collector.GetCollection()[i]; + + ASSERT_EQ(before.GetName(), after.GetName()); + + ASSERT_EQ(before.GetDescription(), after.GetDescription()); + + ASSERT_EQ(before.GetLabels(), after.GetLabels()); + + auto before_agg_var = before.GetAggregator(); + auto before_agg = nostd::get>>(before_agg_var); + + auto after_agg_var = after.GetAggregator(); + auto after_agg = nostd::get>>(after_agg_var); + + ASSERT_EQ(before_agg->get_checkpoint().size(), after_agg->get_checkpoint().size()); + for (int i = 0; i < before_agg->get_checkpoint().size(); i++) + { + ASSERT_EQ(before_agg->get_checkpoint()[i], after_agg->get_checkpoint()[i]); + } + } +} + +/** + * AddMetricData() should be able to successfully add a collection + * of Records with MinMaxSumCount Aggregators. It checks that the min, max, + * sum, and count of updates to the aggregator of a record before and after AddMetricData() + * is called are equal. + */ +TEST(PrometheusCollector, AddMetricDataWithMinMaxSumCountRecordsSuccessfully) +{ + PrometheusCollector collector; + + // number of records to create + int num_records = 2; + + // construct a collection of records with MinMaxSumCountAggregators and short + std::vector records = + CreateRecords(num_records, metric_sdk::AggregatorKind::MinMaxSumCount); + + // add records to collection + collector.AddMetricData(records); + + // Collection size should be the same as the size + // of the records collection passed to addMetricData() + ASSERT_EQ(collector.GetCollection().size(), records.size()); + + // check values of records created vs records from metricsToCollect, + // accessed by getCollection() + + for (int i = 0; i < num_records; i++) + { + metric_sdk::Record before = records[i]; + metric_sdk::Record after = collector.GetCollection()[i]; + + ASSERT_EQ(before.GetName(), after.GetName()); + + ASSERT_EQ(before.GetDescription(), after.GetDescription()); + + ASSERT_EQ(before.GetLabels(), after.GetLabels()); + + auto before_agg_var = before.GetAggregator(); + auto before_agg = nostd::get>>(before_agg_var); + + auto after_agg_var = after.GetAggregator(); + auto after_agg = nostd::get>>(after_agg_var); + + ASSERT_EQ(before_agg->get_checkpoint().size(), after_agg->get_checkpoint().size()); + for (int i = 0; i < before_agg->get_checkpoint().size(); i++) + { + ASSERT_EQ(before_agg->get_checkpoint()[i], after_agg->get_checkpoint()[i]); + } + } +} + +/** + * AddMetricData() should be able to successfully add a collection + * of Records with Gauge Aggregators. It checks that the last update + * to the aggregator of a record before and after AddMetricData() + * is called are equal. + */ +TEST(PrometheusCollector, AddMetricDataWithGaugeRecordsSuccessfully) +{ + PrometheusCollector collector; + + // number of records to create + int num_records = 2; + + // construct a collection of records with GaugeAggregators and int + std::vector records = + CreateRecords(num_records, metric_sdk::AggregatorKind::Gauge); + + // add records to collection + collector.AddMetricData(records); + + // Collection size should be the same as the size + // of the records collection passed to addMetricData() + ASSERT_EQ(collector.GetCollection().size(), records.size()); + + // check values of records created vs records from metricsToCollect, + // accessed by getCollection() + + for (int i = 0; i < num_records; i++) + { + metric_sdk::Record before = records[i]; + metric_sdk::Record after = collector.GetCollection()[i]; + + ASSERT_EQ(before.GetName(), after.GetName()); + + ASSERT_EQ(before.GetDescription(), after.GetDescription()); + + ASSERT_EQ(before.GetLabels(), after.GetLabels()); + + auto before_agg_var = before.GetAggregator(); + auto before_agg = nostd::get>>(before_agg_var); + + auto after_agg_var = after.GetAggregator(); + auto after_agg = nostd::get>>(after_agg_var); + + ASSERT_EQ(before_agg->get_checkpoint().size(), after_agg->get_checkpoint().size()); + for (int i = 0; i < before_agg->get_checkpoint().size(); i++) + { + ASSERT_EQ(before_agg->get_checkpoint()[i], after_agg->get_checkpoint()[i]); + } + } +} + +/** + * AddMetricData() should be able to successfully add a collection + * of Records with Sketch Aggregators. It checks that the sum of updates + * and count of values added for a record before and after being added are + * equal using get_checkpoint(). It also checks the same for buckets, in + * get_boundaries(), and counts for buckets, in get_counts(). + */ +TEST(PrometheusCollector, AddMetricDataWithSketchRecordsSuccessfully) +{ + PrometheusCollector collector; + + // number of records to create + int num_records = 2; + + // construct a collection of records with SketchAggregators and double + std::vector records = + CreateRecords(num_records, metric_sdk::AggregatorKind::Sketch); + + // add records to collection + collector.AddMetricData(records); + + // Collection size should be the same as the size + // of the records collection passed to addMetricData() + ASSERT_EQ(collector.GetCollection().size(), records.size()); + + // check values of records created vs records from metricsToCollect, + // accessed by getCollection() + + for (int i = 0; i < num_records; i++) + { + metric_sdk::Record before = records[i]; + metric_sdk::Record after = collector.GetCollection()[i]; + + ASSERT_EQ(before.GetName(), after.GetName()); + + ASSERT_EQ(before.GetDescription(), after.GetDescription()); + + ASSERT_EQ(before.GetLabels(), after.GetLabels()); + + auto before_agg_var = before.GetAggregator(); + auto before_agg = nostd::get>>(before_agg_var); + + auto after_agg_var = after.GetAggregator(); + auto after_agg = nostd::get>>(after_agg_var); + + ASSERT_EQ(before_agg->get_checkpoint().size(), after_agg->get_checkpoint().size()); + for (int i = 0; i < before_agg->get_checkpoint().size(); i++) + { + ASSERT_EQ(before_agg->get_checkpoint()[i], after_agg->get_checkpoint()[i]); + } + for (int i = 0; i < before_agg->get_boundaries().size(); i++) + { + ASSERT_EQ(before_agg->get_boundaries()[i], after_agg->get_boundaries()[i]); + } + for (int i = 0; i < before_agg->get_counts().size(); i++) + { + ASSERT_EQ(before_agg->get_counts()[i], after_agg->get_counts()[i]); + } + } +} + +/** + * AddMetricData() should be able to successfully add a collection + * of Records with Histogram Aggregators. It checks that the sum of + * updates, number of updates, boundaries, and counts for each bucket + * for the aggregator of a record before and after AddMetricData() + * is called are equal. + */ +TEST(PrometheusCollector, AddMetricDataWithHistogramRecordsSuccessfully) +{ + PrometheusCollector collector; + + // number of records to create + int num_records = 2; + + // construct a collection of records with HistogramAggregators and float + std::vector records = + CreateRecords(num_records, metric_sdk::AggregatorKind::Histogram); + + // add records to collection + collector.AddMetricData(records); + + // Collection size should be the same as the size + // of the records collection passed to addMetricData() + ASSERT_EQ(collector.GetCollection().size(), records.size()); + + // check values of records created vs records from metricsToCollect, + // accessed by getCollection() + + for (int i = 0; i < num_records; i++) + { + metric_sdk::Record before = records[i]; + metric_sdk::Record after = collector.GetCollection()[i]; + + ASSERT_EQ(before.GetName(), after.GetName()); + + ASSERT_EQ(before.GetDescription(), after.GetDescription()); + + ASSERT_EQ(before.GetLabels(), after.GetLabels()); + + auto before_agg_var = before.GetAggregator(); + auto before_agg = nostd::get>>(before_agg_var); + + auto after_agg_var = after.GetAggregator(); + auto after_agg = nostd::get>>(after_agg_var); + + ASSERT_EQ(before_agg->get_checkpoint().size(), after_agg->get_checkpoint().size()); + for (int i = 0; i < before_agg->get_checkpoint().size(); i++) + { + ASSERT_EQ(before_agg->get_checkpoint()[i], after_agg->get_checkpoint()[i]); + } + for (int i = 0; i < before_agg->get_boundaries().size(); i++) + { + ASSERT_EQ(before_agg->get_boundaries()[i], after_agg->get_boundaries()[i]); + } + for (int i = 0; i < before_agg->get_counts().size(); i++) + { + ASSERT_EQ(before_agg->get_counts()[i], after_agg->get_counts()[i]); + } + } +} + +/** + * AddMetricData() should be able to successfully add a collection + * of Records with Exact Aggregators. If the Exact Aggregator is in + * quantile mode, it will check quantiles at selected values of 0, 0.25, + * 0.5, 0.75, and 1. If not, it will check the vector of checkpointed + * values in get_checkpoint(). + */ +TEST(PrometheusCollector, AddMetricDataWithExactRecordsSuccessfully) +{ + PrometheusCollector collector; + + // number of records to create + int num_records = 1; + + // construct a collection of a single record with a quantile + // estimation ExactAggregator and double + std::vector records = + CreateRecords(num_records, metric_sdk::AggregatorKind::Exact, true); + + // add records to collection + collector.AddMetricData(records); + + // construct a collection of a single record with an in-order + // ExactAggregator and double + records = CreateRecords(num_records, metric_sdk::AggregatorKind::Exact, false); + + // add records to collection + collector.AddMetricData(records); + + // Collection size should be the same as the size + // of the records collection passed to addMetricData() + ASSERT_EQ(collector.GetCollection().size(), records.size() * 2); + + // check values of records created vs records from metricsToCollect, + // accessed by getCollection() + + for (int i = 0; i < num_records; i++) + { + metric_sdk::Record before = records[i]; + metric_sdk::Record after = collector.GetCollection()[i]; + + ASSERT_EQ(before.GetName(), after.GetName()); + + ASSERT_EQ(before.GetDescription(), after.GetDescription()); + + ASSERT_EQ(before.GetLabels(), after.GetLabels()); + + auto before_agg_var = before.GetAggregator(); + auto before_agg = nostd::get>>(before_agg_var); + + auto after_agg_var = after.GetAggregator(); + auto after_agg = nostd::get>>(after_agg_var); + + if (before_agg->get_quant_estimation() && after_agg->get_quant_estimation()) + { + for (double i = 0; i <= 1;) + { + ASSERT_EQ(before_agg->get_quantiles(i), after_agg->get_quantiles(i)); + i += 0.25; + } + } + else + { + ASSERT_EQ(before_agg->get_checkpoint().size(), after_agg->get_checkpoint().size()); + for (int i = 0; i < before_agg->get_checkpoint().size(); i++) + { + ASSERT_EQ(before_agg->get_checkpoint()[i], after_agg->get_checkpoint()[i]); + } + } + } +} + +TEST(PrometheusCollector, AddMetricDataDoesNotAddWithInsufficentSpace) +{ + PrometheusCollector collector; + + // number of records to create + int num_records = collector.GetMaxCollectionSize() - 5; + + // construct a collection close to max capacity + std::vector records = + CreateRecords(num_records, metric_sdk::AggregatorKind::Counter); + + collector.AddMetricData(records); + + // Check if all the records have been added + ASSERT_EQ(collector.GetCollection().size(), num_records); + + // Try adding the same collection of records again to + // metricsToCollect. + collector.AddMetricData(records); + + // Check that the number of records in metricsToCollect + // has not changed. + ASSERT_EQ(collector.GetCollection().size(), num_records); +} + +TEST(PrometheusCollector, AddMetricDataDoesNotAddBadIndividualRecords) +{ + PrometheusCollector collector; + + // number of records to create + int num_records = 5; + + // construct a collection with the specified number of records + std::vector records = + CreateRecords(num_records, metric_sdk::AggregatorKind::Counter); + + // add records to collection + collector.AddMetricData(records); + + // Check if all the records have been added + ASSERT_EQ(collector.GetCollection().size(), num_records); + + // Creates a bad record, with a nullptr aggregator and adds + // it to the colelction of records + std::string name = "bad_record"; + std::string description = "nullptr_agg"; + std::string labels = "{label1:v1}"; + std::shared_ptr> aggregator; + metric_sdk::Record bad_record{name, description, labels, aggregator}; + + records.push_back(bad_record); + + // add records to collection + collector.AddMetricData(records); + + // Check if all the records except the bad + // record have been added; the number of records added + // should be twice the original number of records + // epecified to be created + ASSERT_EQ(collector.GetCollection().size(), num_records * 2); +} + +// ==================== Test for Constructor ====================== +TEST(PrometheusCollector, ConstructorInitializesCollector) +{ + PrometheusCollector collector; + + // current size should be 0, capacity should be set to default + ASSERT_EQ(collector.GetCollection().size(), 0); +} + +// ==================== Tests for collect() function ====================== + +/** + * When collector is initialized, the collection inside is should also be initialized + */ +TEST(PrometheusCollector, CollectInitializesMetricFamilyCollection) +{ + PrometheusCollector collector; + auto c1 = collector.Collect(); + ASSERT_EQ(c1.size(), 0); +} + +/** + * Collect function should collect all data and clear the intermediate collection + */ +TEST(PrometheusCollector, CollectClearsTheCollection) +{ + PrometheusCollector collector; + + // construct a collection to add metric records + int num_records = 2; + auto records = CreateRecords(num_records, metric_sdk::AggregatorKind::Counter); + collector.AddMetricData(records); + + // the collection should not be empty now + ASSERT_EQ(collector.GetCollection().size(), num_records); + + // don't care the collected result in this test + collector.Collect(); + + // after the collect() call, the collection should be empty + ASSERT_EQ(collector.GetCollection().size(), 0); +} + +/** + * Collected data should be already be parsed to Prometheus Metric format + */ +TEST(PrometheusCollector, CollectParsesDataToMetricFamily) +{ + PrometheusCollector collector; + + // construct a collection to add metric records + int num_records = 1; + auto records = CreateRecords(num_records, metric_sdk::AggregatorKind::Counter); + collector.AddMetricData(records); + + // the collection should not be empty now + ASSERT_EQ(collector.GetCollection().size(), num_records); + auto collected = collector.Collect(); + + ASSERT_EQ(collected.size(), num_records); + + auto metric_family = collected[0]; + + // Collect function really collects a vector of MetricFamily + ASSERT_EQ(metric_family.name, "record_0"); + ASSERT_EQ(metric_family.help, "record 0 for test purpose"); + ASSERT_EQ(metric_family.type, prometheus_client::MetricType::Counter); + ASSERT_EQ(metric_family.metric.size(), 1); + ASSERT_DOUBLE_EQ(metric_family.metric[0].counter.value, 15); +} + +/** + * Concurrency Test 1: After adding data concurrently, the intermediate collection should + * contain all data from all threads. + */ +TEST(PrometheusCollector, ConcurrencyAddingRecords) +{ + PrometheusCollector collector; + + // construct a collection to add metric records + int num_records = 2; + std::vector records1 = + CreateRecords(num_records, metric_sdk::AggregatorKind::Counter); + + std::vector records2 = + CreateRecords(num_records, metric_sdk::AggregatorKind::Gauge); + + std::thread first(&PrometheusCollector::AddMetricData, std::ref(collector), std::ref(records1)); + std::thread second(&PrometheusCollector::AddMetricData, std::ref(collector), std::ref(records2)); + + first.join(); + second.join(); + + ASSERT_EQ(collector.GetCollection().size(), 4); +} + +/** + * Concurrency Test 2: After adding data concurrently and collecting, the intermediate collection + * should be empty, and all data are collected in the result vector. + */ +TEST(PrometheusCollector, ConcurrentlyAddingAndThenCollecting) +{ + PrometheusCollector collector; + + // construct a collection to add metric records + int num_records = 2; + std::vector records1 = + CreateRecords(num_records, metric_sdk::AggregatorKind::Counter); + + std::vector records2 = + CreateRecords(num_records, metric_sdk::AggregatorKind::Gauge); + + std::thread first(&PrometheusCollector::AddMetricData, std::ref(collector), std::ref(records1)); + std::thread second(&PrometheusCollector::AddMetricData, std::ref(collector), std::ref(records2)); + + first.join(); + second.join(); + + auto collect_future = std::async(&PrometheusCollector::Collect, std::ref(collector)); + auto res = collect_future.get(); + + ASSERT_EQ(collector.GetCollection().size(), 0); + ASSERT_EQ(res.size(), 4); +} + +/** + * Concurrency Test 3: Concurrently adding and collecting. We don't know when the collect function + * is called, but all data entries are either collected or left in the collection. + */ +TEST(PrometheusCollector, ConcurrentlyAddingAndCollecting) +{ + PrometheusCollector collector; + + // construct a collection to add metric records + int num_records = 2; + std::vector records1 = + CreateRecords(num_records, metric_sdk::AggregatorKind::Counter); + + std::vector records2 = + CreateRecords(num_records, metric_sdk::AggregatorKind::Gauge); + + std::thread first(&PrometheusCollector::AddMetricData, std::ref(collector), std::ref(records1)); + std::thread second(&PrometheusCollector::AddMetricData, std::ref(collector), std::ref(records2)); + auto collect_future = std::async(&PrometheusCollector::Collect, std::ref(collector)); + + first.join(); + second.join(); + + auto res = collect_future.get(); + + // the size of collection can be 0, 2, 4, because we don't know when the collect() + // is really called. However, we claim that if the data in the collection is collected, + // they must be in the res. So res.size() + collection.size() must be the total number + // of data records we generated. + ASSERT_EQ(res.size() + collector.GetCollection().size(), 4); +} + +/** + * Concurrency Test 4: Concurrently adding then concurrently collecting. We don't know which + * collecting thread fetches all data, but either one should succeed. + */ +TEST(PrometheusCollector, ConcurrentlyAddingAndConcurrentlyCollecting) +{ + PrometheusCollector collector; + + // construct a collection to add metric records + int num_records = 2; + std::vector records1 = + CreateRecords(num_records, metric_sdk::AggregatorKind::Counter); + + std::vector records2 = + CreateRecords(num_records, metric_sdk::AggregatorKind::Gauge); + + // concurrently adding + std::thread first(&PrometheusCollector::AddMetricData, std::ref(collector), std::ref(records1)); + std::thread second(&PrometheusCollector::AddMetricData, std::ref(collector), std::ref(records2)); + first.join(); + second.join(); + + // after adding, then concurrently consuming + auto collect_future1 = std::async(&PrometheusCollector::Collect, std::ref(collector)); + auto collect_future2 = std::async(&PrometheusCollector::Collect, std::ref(collector)); + auto res1 = collect_future1.get(); + auto res2 = collect_future2.get(); + + // all added data must be collected in either res1 or res2 + ASSERT_EQ(res1.size() + res2.size(), 4); +} + +OPENTELEMETRY_END_NAMESPACE From 8f150257d665ddcfa1958568d993cb5a66ff3dd6 Mon Sep 17 00:00:00 2001 From: erichsueh3 Date: Tue, 25 Aug 2020 20:51:58 -0700 Subject: [PATCH 2/7] format --- exporters/prometheus/BUILD | 2 -- exporters/prometheus/CMakeLists.txt | 9 ++++----- exporters/prometheus/test/CMakeLists.txt | 16 ++++++++-------- .../prometheus/test/prometheus_collector_test.cc | 14 +++++++------- 4 files changed, 19 insertions(+), 22 deletions(-) diff --git a/exporters/prometheus/BUILD b/exporters/prometheus/BUILD index 05ab8ee476..6f569a0a8b 100644 --- a/exporters/prometheus/BUILD +++ b/exporters/prometheus/BUILD @@ -42,5 +42,3 @@ cc_test( "@com_google_googletest//:gtest_main", ], ) - - diff --git a/exporters/prometheus/CMakeLists.txt b/exporters/prometheus/CMakeLists.txt index f4195d616e..0e5942e053 100644 --- a/exporters/prometheus/CMakeLists.txt +++ b/exporters/prometheus/CMakeLists.txt @@ -15,9 +15,8 @@ include_directories(include) find_package(prometheus-cpp CONFIG REQUIRED) -add_library( - prometheus_exporter src/prometheus_collector.cc) +add_library(prometheus_exporter src/prometheus_collector.cc) -if (BUILD_TESTING) - add_subdirectory(test) -endif () \ No newline at end of file +if(BUILD_TESTING) + add_subdirectory(test) +endif() diff --git a/exporters/prometheus/test/CMakeLists.txt b/exporters/prometheus/test/CMakeLists.txt index 4aa51418ff..3b72880217 100644 --- a/exporters/prometheus/test/CMakeLists.txt +++ b/exporters/prometheus/test/CMakeLists.txt @@ -1,8 +1,8 @@ -foreach (testname prometheus_collector_test) - add_executable(${testname} "${testname}.cc") - target_link_libraries( - ${testname} ${GTEST_BOTH_LIBRARIES} ${CMAKE_THREAD_LIBS_INIT} - prometheus_exporter prometheus-cpp::pull) - gtest_add_tests(TARGET ${testname} TEST_PREFIX exporter. TEST_LIST ${testname}) -endforeach () - +foreach(testname prometheus_collector_test) + add_executable(${testname} "${testname}.cc") + target_link_libraries( + ${testname} ${GTEST_BOTH_LIBRARIES} ${CMAKE_THREAD_LIBS_INIT} + prometheus_exporter prometheus-cpp::pull) + gtest_add_tests(TARGET ${testname} TEST_PREFIX exporter. TEST_LIST + ${testname}) +endforeach() diff --git a/exporters/prometheus/test/prometheus_collector_test.cc b/exporters/prometheus/test/prometheus_collector_test.cc index 61d2252957..48ba74c200 100644 --- a/exporters/prometheus/test/prometheus_collector_test.cc +++ b/exporters/prometheus/test/prometheus_collector_test.cc @@ -41,7 +41,7 @@ OPENTELEMETRY_BEGIN_NAMESPACE */ template std::shared_ptr> CreateAgg(metric_sdk::AggregatorKind kind, - bool exactMode = true) + bool exactMode = true) { std::shared_ptr> aggregator; switch (kind) @@ -212,7 +212,7 @@ TEST(PrometheusCollector, AddMetricDataWithCounterRecordsSuccessfully) auto before_agg = nostd::get>>(before_agg_var); auto after_agg_var = after.GetAggregator(); - auto after_agg = nostd::get>>(after_agg_var); + auto after_agg = nostd::get>>(after_agg_var); ASSERT_EQ(before_agg->get_checkpoint().size(), after_agg->get_checkpoint().size()); for (int i = 0; i < before_agg->get_checkpoint().size(); i++) @@ -264,7 +264,7 @@ TEST(PrometheusCollector, AddMetricDataWithMinMaxSumCountRecordsSuccessfully) auto before_agg = nostd::get>>(before_agg_var); auto after_agg_var = after.GetAggregator(); - auto after_agg = nostd::get>>(after_agg_var); + auto after_agg = nostd::get>>(after_agg_var); ASSERT_EQ(before_agg->get_checkpoint().size(), after_agg->get_checkpoint().size()); for (int i = 0; i < before_agg->get_checkpoint().size(); i++) @@ -313,7 +313,7 @@ TEST(PrometheusCollector, AddMetricDataWithGaugeRecordsSuccessfully) ASSERT_EQ(before.GetLabels(), after.GetLabels()); auto before_agg_var = before.GetAggregator(); - auto before_agg = nostd::get>>(before_agg_var); + auto before_agg = nostd::get>>(before_agg_var); auto after_agg_var = after.GetAggregator(); auto after_agg = nostd::get>>(after_agg_var); @@ -369,7 +369,7 @@ TEST(PrometheusCollector, AddMetricDataWithSketchRecordsSuccessfully) auto before_agg = nostd::get>>(before_agg_var); auto after_agg_var = after.GetAggregator(); - auto after_agg = nostd::get>>(after_agg_var); + auto after_agg = nostd::get>>(after_agg_var); ASSERT_EQ(before_agg->get_checkpoint().size(), after_agg->get_checkpoint().size()); for (int i = 0; i < before_agg->get_checkpoint().size(); i++) @@ -430,7 +430,7 @@ TEST(PrometheusCollector, AddMetricDataWithHistogramRecordsSuccessfully) auto before_agg = nostd::get>>(before_agg_var); auto after_agg_var = after.GetAggregator(); - auto after_agg = nostd::get>>(after_agg_var); + auto after_agg = nostd::get>>(after_agg_var); ASSERT_EQ(before_agg->get_checkpoint().size(), after_agg->get_checkpoint().size()); for (int i = 0; i < before_agg->get_checkpoint().size(); i++) @@ -499,7 +499,7 @@ TEST(PrometheusCollector, AddMetricDataWithExactRecordsSuccessfully) auto before_agg = nostd::get>>(before_agg_var); auto after_agg_var = after.GetAggregator(); - auto after_agg = nostd::get>>(after_agg_var); + auto after_agg = nostd::get>>(after_agg_var); if (before_agg->get_quant_estimation() && after_agg->get_quant_estimation()) { From 4d75759e73db5f3186b31d16b96e14ca9cbf6a90 Mon Sep 17 00:00:00 2001 From: Cunjun Wang Date: Thu, 27 Aug 2020 00:42:17 -0400 Subject: [PATCH 3/7] fix race condition problem in prometheus collector --- .../prometheus/src/prometheus_collector.cc | 22 ++++++++++++------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/exporters/prometheus/src/prometheus_collector.cc b/exporters/prometheus/src/prometheus_collector.cc index dd60cd4f26..cc7256c925 100644 --- a/exporters/prometheus/src/prometheus_collector.cc +++ b/exporters/prometheus/src/prometheus_collector.cc @@ -43,20 +43,23 @@ PrometheusCollector::PrometheusCollector(int max_collection_size) */ std::vector PrometheusCollector::Collect() const { + this->collection_lock_.lock(); if (metrics_to_collect_->empty()) { + this->collection_lock_.unlock(); return {}; } + this->collection_lock_.unlock(); std::vector result; // copy the intermediate collection, and then clear it std::vector copied_data; - collection_lock_.lock(); + this->collection_lock_.lock(); copied_data = std::vector(*metrics_to_collect_); metrics_to_collect_->clear(); - collection_lock_.unlock(); + this->collection_lock_.unlock(); result = PrometheusExporterUtils::TranslateToPrometheus(copied_data); return result; @@ -70,7 +73,13 @@ std::vector PrometheusCollector::Collect() cons */ void PrometheusCollector::AddMetricData(const std::vector &records) { - if (metrics_to_collect_->size() + records.size() <= max_collection_size_ && !records.empty()) + if (records.empty()) + { + return; + } + + collection_lock_.lock(); + if (metrics_to_collect_->size() + records.size() <= max_collection_size_) { /** * ValidAggregator is a lambda that checks a Record to see if its @@ -122,13 +131,11 @@ void PrometheusCollector::AddMetricData(const std::vector return true; }; - collection_lock_.lock(); - for (auto &r : records) { if (ValidAggregator(r)) { - metrics_to_collect_->push_back(r); + metrics_to_collect_->emplace_back(r); } // Drop the record and write to std::cout else @@ -139,9 +146,8 @@ void PrometheusCollector::AddMetricData(const std::vector << std::endl; } } - - collection_lock_.unlock(); } + collection_lock_.unlock(); } /** From 354909986dd39604f06e1dfbb9dfc71df809eeb3 Mon Sep 17 00:00:00 2001 From: Cunjun Wang Date: Thu, 27 Aug 2020 01:04:58 -0400 Subject: [PATCH 4/7] format code --- exporters/prometheus/CMakeLists.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/exporters/prometheus/CMakeLists.txt b/exporters/prometheus/CMakeLists.txt index 172c73428b..1a58b175fb 100644 --- a/exporters/prometheus/CMakeLists.txt +++ b/exporters/prometheus/CMakeLists.txt @@ -16,7 +16,8 @@ include_directories(include) find_package(prometheus-cpp CONFIG REQUIRED) -add_library(prometheus_exporter src/prometheus_collector.cc src/prometheus_exporter_utils.cc) +add_library(prometheus_exporter src/prometheus_collector.cc + src/prometheus_exporter_utils.cc) if(BUILD_TESTING) add_subdirectory(test) From 19bbc8a3a86e9860663c829b445eee3dcfb4549c Mon Sep 17 00:00:00 2001 From: Cunjun Wang Date: Thu, 27 Aug 2020 10:32:47 -0400 Subject: [PATCH 5/7] change the way to test concurreny so that legacy bazel not complains --- .../test/prometheus_collector_test.cc | 35 +++++++------------ 1 file changed, 13 insertions(+), 22 deletions(-) diff --git a/exporters/prometheus/test/prometheus_collector_test.cc b/exporters/prometheus/test/prometheus_collector_test.cc index 48ba74c200..39d746322e 100644 --- a/exporters/prometheus/test/prometheus_collector_test.cc +++ b/exporters/prometheus/test/prometheus_collector_test.cc @@ -46,41 +46,35 @@ std::shared_ptr> CreateAgg(metric_sdk::AggregatorKind std::shared_ptr> aggregator; switch (kind) { - case metric_sdk::AggregatorKind::Counter: - { + case metric_sdk::AggregatorKind::Counter: { aggregator = std::shared_ptr>( new metric_sdk::CounterAggregator(opentelemetry::metrics::InstrumentKind::Counter)); break; } - case metric_sdk::AggregatorKind::MinMaxSumCount: - { + case metric_sdk::AggregatorKind::MinMaxSumCount: { aggregator = std::shared_ptr>(new metric_sdk::MinMaxSumCountAggregator( opentelemetry::metrics::InstrumentKind::Counter)); break; } - case metric_sdk::AggregatorKind::Gauge: - { + case metric_sdk::AggregatorKind::Gauge: { aggregator = std::shared_ptr>( new metric_sdk::GaugeAggregator(opentelemetry::metrics::InstrumentKind::Counter)); break; } - case metric_sdk::AggregatorKind::Sketch: - { + case metric_sdk::AggregatorKind::Sketch: { aggregator = std::shared_ptr>(new metric_sdk::SketchAggregator( opentelemetry::metrics::InstrumentKind::Counter, 0.000005)); break; } - case metric_sdk::AggregatorKind::Histogram: - { + case metric_sdk::AggregatorKind::Histogram: { std::vector boundaries{10, 20}; aggregator = std::shared_ptr>(new metric_sdk::HistogramAggregator( opentelemetry::metrics::InstrumentKind::Counter, boundaries)); break; } - case metric_sdk::AggregatorKind::Exact: - { + case metric_sdk::AggregatorKind::Exact: { aggregator = std::shared_ptr>(new metric_sdk::ExactAggregator( opentelemetry::metrics::InstrumentKind::Counter, exactMode)); break; @@ -700,8 +694,7 @@ TEST(PrometheusCollector, ConcurrentlyAddingAndThenCollecting) first.join(); second.join(); - auto collect_future = std::async(&PrometheusCollector::Collect, std::ref(collector)); - auto res = collect_future.get(); + auto res = collector.Collect(); ASSERT_EQ(collector.GetCollection().size(), 0); ASSERT_EQ(res.size(), 4); @@ -725,13 +718,11 @@ TEST(PrometheusCollector, ConcurrentlyAddingAndCollecting) std::thread first(&PrometheusCollector::AddMetricData, std::ref(collector), std::ref(records1)); std::thread second(&PrometheusCollector::AddMetricData, std::ref(collector), std::ref(records2)); - auto collect_future = std::async(&PrometheusCollector::Collect, std::ref(collector)); + auto res = collector.Collect(); first.join(); second.join(); - auto res = collect_future.get(); - // the size of collection can be 0, 2, 4, because we don't know when the collect() // is really called. However, we claim that if the data in the collection is collected, // they must be in the res. So res.size() + collection.size() must be the total number @@ -762,13 +753,13 @@ TEST(PrometheusCollector, ConcurrentlyAddingAndConcurrentlyCollecting) second.join(); // after adding, then concurrently consuming - auto collect_future1 = std::async(&PrometheusCollector::Collect, std::ref(collector)); - auto collect_future2 = std::async(&PrometheusCollector::Collect, std::ref(collector)); - auto res1 = collect_future1.get(); - auto res2 = collect_future2.get(); + std::thread first_collect(&PrometheusCollector::Collect, std::ref(collector)); + std::thread second_collect(&PrometheusCollector::Collect, std::ref(collector)); + first_collect.join(); + second_collect.join(); // all added data must be collected in either res1 or res2 - ASSERT_EQ(res1.size() + res2.size(), 4); + ASSERT_EQ(collector.GetCollection().size(), 0); } OPENTELEMETRY_END_NAMESPACE From c15fb5d3bf0c52d1e2e25a625c27d99046280c5d Mon Sep 17 00:00:00 2001 From: Cunjun Wang Date: Thu, 27 Aug 2020 10:35:23 -0400 Subject: [PATCH 6/7] format code --- .../test/prometheus_collector_test.cc | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/exporters/prometheus/test/prometheus_collector_test.cc b/exporters/prometheus/test/prometheus_collector_test.cc index 39d746322e..ec8a4879fc 100644 --- a/exporters/prometheus/test/prometheus_collector_test.cc +++ b/exporters/prometheus/test/prometheus_collector_test.cc @@ -46,35 +46,41 @@ std::shared_ptr> CreateAgg(metric_sdk::AggregatorKind std::shared_ptr> aggregator; switch (kind) { - case metric_sdk::AggregatorKind::Counter: { + case metric_sdk::AggregatorKind::Counter: + { aggregator = std::shared_ptr>( new metric_sdk::CounterAggregator(opentelemetry::metrics::InstrumentKind::Counter)); break; } - case metric_sdk::AggregatorKind::MinMaxSumCount: { + case metric_sdk::AggregatorKind::MinMaxSumCount: + { aggregator = std::shared_ptr>(new metric_sdk::MinMaxSumCountAggregator( opentelemetry::metrics::InstrumentKind::Counter)); break; } - case metric_sdk::AggregatorKind::Gauge: { + case metric_sdk::AggregatorKind::Gauge: + { aggregator = std::shared_ptr>( new metric_sdk::GaugeAggregator(opentelemetry::metrics::InstrumentKind::Counter)); break; } - case metric_sdk::AggregatorKind::Sketch: { + case metric_sdk::AggregatorKind::Sketch: + { aggregator = std::shared_ptr>(new metric_sdk::SketchAggregator( opentelemetry::metrics::InstrumentKind::Counter, 0.000005)); break; } - case metric_sdk::AggregatorKind::Histogram: { + case metric_sdk::AggregatorKind::Histogram: + { std::vector boundaries{10, 20}; aggregator = std::shared_ptr>(new metric_sdk::HistogramAggregator( opentelemetry::metrics::InstrumentKind::Counter, boundaries)); break; } - case metric_sdk::AggregatorKind::Exact: { + case metric_sdk::AggregatorKind::Exact: + { aggregator = std::shared_ptr>(new metric_sdk::ExactAggregator( opentelemetry::metrics::InstrumentKind::Counter, exactMode)); break; From 320d6742ca768f5e357c60269de3eed3f0442caf Mon Sep 17 00:00:00 2001 From: Cunjun Wang Date: Thu, 27 Aug 2020 13:29:25 -0400 Subject: [PATCH 7/7] handle bazel.legacy --- ci/do_ci.sh | 6 ++++-- .../test/prometheus_collector_test.cc | 18 ++++++++++-------- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/ci/do_ci.sh b/ci/do_ci.sh index 54e84b9e92..e97686b0b7 100755 --- a/ci/do_ci.sh +++ b/ci/do_ci.sh @@ -106,8 +106,10 @@ elif [[ "$1" == "bazel.test" ]]; then bazel test $BAZEL_TEST_OPTIONS //... exit 0 elif [[ "$1" == "bazel.legacy.test" ]]; then - bazel build $BAZEL_OPTIONS -- //... -//exporters/otlp/... - bazel test $BAZEL_TEST_OPTIONS -- //... -//exporters/otlp/... + # we uses C++ future and async() function to test the Prometheus Exporter functionality, + # that make this test always fail. ignore Prometheus exporter here. + bazel build $BAZEL_OPTIONS -- //... -//exporters/otlp/... -//exporters/prometheus/... + bazel test $BAZEL_TEST_OPTIONS -- //... -//exporters/otlp/... -//exporters/prometheus/... exit 0 elif [[ "$1" == "bazel.noexcept" ]]; then # there are some exceptions and error handling code from the Prometheus Client diff --git a/exporters/prometheus/test/prometheus_collector_test.cc b/exporters/prometheus/test/prometheus_collector_test.cc index ec8a4879fc..444302fa44 100644 --- a/exporters/prometheus/test/prometheus_collector_test.cc +++ b/exporters/prometheus/test/prometheus_collector_test.cc @@ -696,11 +696,11 @@ TEST(PrometheusCollector, ConcurrentlyAddingAndThenCollecting) std::thread first(&PrometheusCollector::AddMetricData, std::ref(collector), std::ref(records1)); std::thread second(&PrometheusCollector::AddMetricData, std::ref(collector), std::ref(records2)); - first.join(); second.join(); - auto res = collector.Collect(); + auto collect_future = std::async(&PrometheusCollector::Collect, std::ref(collector)); + auto res = collect_future.get(); ASSERT_EQ(collector.GetCollection().size(), 0); ASSERT_EQ(res.size(), 4); @@ -724,11 +724,13 @@ TEST(PrometheusCollector, ConcurrentlyAddingAndCollecting) std::thread first(&PrometheusCollector::AddMetricData, std::ref(collector), std::ref(records1)); std::thread second(&PrometheusCollector::AddMetricData, std::ref(collector), std::ref(records2)); - auto res = collector.Collect(); + auto collect_future = std::async(&PrometheusCollector::Collect, std::ref(collector)); first.join(); second.join(); + auto res = collect_future.get(); + // the size of collection can be 0, 2, 4, because we don't know when the collect() // is really called. However, we claim that if the data in the collection is collected, // they must be in the res. So res.size() + collection.size() must be the total number @@ -759,13 +761,13 @@ TEST(PrometheusCollector, ConcurrentlyAddingAndConcurrentlyCollecting) second.join(); // after adding, then concurrently consuming - std::thread first_collect(&PrometheusCollector::Collect, std::ref(collector)); - std::thread second_collect(&PrometheusCollector::Collect, std::ref(collector)); - first_collect.join(); - second_collect.join(); + auto collect_future1 = std::async(&PrometheusCollector::Collect, std::ref(collector)); + auto collect_future2 = std::async(&PrometheusCollector::Collect, std::ref(collector)); + auto res1 = collect_future1.get(); + auto res2 = collect_future2.get(); // all added data must be collected in either res1 or res2 - ASSERT_EQ(collector.GetCollection().size(), 0); + ASSERT_EQ(res1.size() + res2.size(), 4); } OPENTELEMETRY_END_NAMESPACE