From fb2c087e38eb8ee476b388bf0fa925b250050579 Mon Sep 17 00:00:00 2001 From: Max Golovanov Date: Fri, 28 Aug 2020 03:30:25 -0700 Subject: [PATCH 01/18] Add json.hpp to CI and Build Tools --- ci/setup_cmake.sh | 6 ++++-- ci/setup_windows_ci_environment.ps1 | 5 +++++ ext/CMakeLists.txt | 2 ++ tools/setup-buildtools-mac.sh | 3 +++ 4 files changed, 14 insertions(+), 2 deletions(-) diff --git a/ci/setup_cmake.sh b/ci/setup_cmake.sh index 34097dbaa3..b4011c6930 100755 --- a/ci/setup_cmake.sh +++ b/ci/setup_cmake.sh @@ -4,13 +4,15 @@ set -e export DEBIAN_FRONTEND=noninteractive apt-get update -apt-get install --no-install-recommends --no-install-suggests -y \ +apt-get install --ignore-missing --no-install-recommends --no-install-suggests -y \ cmake \ libbenchmark-dev \ libgtest-dev \ zlib1g-dev \ sudo \ - libcurl4-openssl-dev + libcurl4-openssl-dev \ + nlohmann-json-dev \ + nlohmann-json3-dev # Follows these instructions for setting up gtest # https://www.eriksmistad.no/getting-started-with-google-test-on-ubuntu/ diff --git a/ci/setup_windows_ci_environment.ps1 b/ci/setup_windows_ci_environment.ps1 index 6e2c91e467..8a935d6a5d 100755 --- a/ci/setup_windows_ci_environment.ps1 +++ b/ci/setup_windows_ci_environment.ps1 @@ -11,5 +11,10 @@ $VCPKG_DIR=(Get-Item -Path ".\").FullName # Patched Google Benchmark can be shared between vs2017 and vs2019 compilers ./vcpkg install --overlay-ports="$PSScriptRoot\ports" benchmark:x64-windows +# Google Test ./vcpkg install gtest:x64-windows + +# nlohmann-json +./vcpkg package nlohmann-json:x64-windows + Pop-Location diff --git a/ext/CMakeLists.txt b/ext/CMakeLists.txt index 75205ac71e..7891366866 100644 --- a/ext/CMakeLists.txt +++ b/ext/CMakeLists.txt @@ -1,5 +1,7 @@ add_subdirectory(src) +find_package(nlohmann_json REQUIRED) + if(BUILD_TESTING) add_subdirectory(test) endif() diff --git a/tools/setup-buildtools-mac.sh b/tools/setup-buildtools-mac.sh index b74c727df4..aee85a456f 100644 --- a/tools/setup-buildtools-mac.sh +++ b/tools/setup-buildtools-mac.sh @@ -26,3 +26,6 @@ sudo chown -R $(whoami) /usr/local/etc/bash_completion.d /usr/local/include /usr brew install cmake brew install wget brew install clang-format +brew install google-benchmark +brew tap nlohmann/json +brew install nlohmann-json From 4b7f1202df07cc78aab92db986703f2bf50619ba Mon Sep 17 00:00:00 2001 From: Max Golovanov Date: Fri, 28 Aug 2020 03:38:38 -0700 Subject: [PATCH 02/18] Fix an issue with Ubuntu 18.04 not liking --ignore-missing --- ci/setup_cmake.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ci/setup_cmake.sh b/ci/setup_cmake.sh index b4011c6930..6a7bef7fa7 100755 --- a/ci/setup_cmake.sh +++ b/ci/setup_cmake.sh @@ -4,7 +4,7 @@ set -e export DEBIAN_FRONTEND=noninteractive apt-get update -apt-get install --ignore-missing --no-install-recommends --no-install-suggests -y \ +echo \ cmake \ libbenchmark-dev \ libgtest-dev \ @@ -12,7 +12,7 @@ apt-get install --ignore-missing --no-install-recommends --no-install-suggests - sudo \ libcurl4-openssl-dev \ nlohmann-json-dev \ - nlohmann-json3-dev + nlohmann-json3-dev | xargs -n 1 apt-get install --ignore-missing --no-install-recommends --no-install-suggests -y # Follows these instructions for setting up gtest # https://www.eriksmistad.no/getting-started-with-google-test-on-ubuntu/ From 6a8f1e4d0c8cba9c0c1a4ca02c445bfae383a7a9 Mon Sep 17 00:00:00 2001 From: Max Golovanov Date: Fri, 28 Aug 2020 03:43:58 -0700 Subject: [PATCH 03/18] Package name for json.hpp is different between Ubuntu 18.xx and 20.xx - ignore if either of these isn't found --- ci/setup_cmake.sh | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ci/setup_cmake.sh b/ci/setup_cmake.sh index 6a7bef7fa7..019e073f15 100755 --- a/ci/setup_cmake.sh +++ b/ci/setup_cmake.sh @@ -4,6 +4,8 @@ set -e export DEBIAN_FRONTEND=noninteractive apt-get update + +set +e echo \ cmake \ libbenchmark-dev \ @@ -13,6 +15,7 @@ echo \ libcurl4-openssl-dev \ nlohmann-json-dev \ nlohmann-json3-dev | xargs -n 1 apt-get install --ignore-missing --no-install-recommends --no-install-suggests -y +set -e # Follows these instructions for setting up gtest # https://www.eriksmistad.no/getting-started-with-google-test-on-ubuntu/ From a4d33da21efd0d671f67bf71506c4baf29e38de7 Mon Sep 17 00:00:00 2001 From: Max Golovanov Date: Fri, 28 Aug 2020 03:57:19 -0700 Subject: [PATCH 04/18] Install either v2 or v3 of nlohmann json.hpp depending on distro --- ci/setup_cmake.sh | 1 + ext/CMakeLists.txt | 2 -- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/ci/setup_cmake.sh b/ci/setup_cmake.sh index 019e073f15..5693064c65 100755 --- a/ci/setup_cmake.sh +++ b/ci/setup_cmake.sh @@ -14,6 +14,7 @@ echo \ sudo \ libcurl4-openssl-dev \ nlohmann-json-dev \ + nlohmann-json3 \ nlohmann-json3-dev | xargs -n 1 apt-get install --ignore-missing --no-install-recommends --no-install-suggests -y set -e diff --git a/ext/CMakeLists.txt b/ext/CMakeLists.txt index 7891366866..75205ac71e 100644 --- a/ext/CMakeLists.txt +++ b/ext/CMakeLists.txt @@ -1,7 +1,5 @@ add_subdirectory(src) -find_package(nlohmann_json REQUIRED) - if(BUILD_TESTING) add_subdirectory(test) endif() From d11c003603c2432c5ba42f420907d0101da3ed00 Mon Sep 17 00:00:00 2001 From: Max Golovanov Date: Thu, 3 Sep 2020 12:38:51 -0700 Subject: [PATCH 05/18] Based off latest opent-telemetry:master --- ci/do_ci.sh | 6 +- ci/setup_windows_ci_environment.ps1 | 2 +- examples/otlp/CMakeLists.txt | 13 +- exporters/prometheus/BUILD | 29 + exporters/prometheus/CMakeLists.txt | 4 +- .../prometheus/prometheus_collector.h | 100 +++ .../prometheus/src/prometheus_collector.cc | 175 ++++ exporters/prometheus/test/CMakeLists.txt | 2 +- .../test/prometheus_collector_test.cc | 773 ++++++++++++++++++ .../opentelemetry/sdk/metrics/instrument.h | 8 +- sdk/src/trace/span.cc | 15 +- sdk/src/trace/tracer.cc | 20 +- sdk/test/metrics/metric_instrument_test.cc | 4 +- sdk/test/trace/tracer_test.cc | 7 +- 14 files changed, 1132 insertions(+), 26 deletions(-) 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/prometheus_collector_test.cc 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/ci/setup_windows_ci_environment.ps1 b/ci/setup_windows_ci_environment.ps1 index 8a935d6a5d..e7cdfeda97 100755 --- a/ci/setup_windows_ci_environment.ps1 +++ b/ci/setup_windows_ci_environment.ps1 @@ -15,6 +15,6 @@ $VCPKG_DIR=(Get-Item -Path ".\").FullName ./vcpkg install gtest:x64-windows # nlohmann-json -./vcpkg package nlohmann-json:x64-windows +./vcpkg install nlohmann-json:x64-windows Pop-Location diff --git a/examples/otlp/CMakeLists.txt b/examples/otlp/CMakeLists.txt index 4c5f2bbd57..23dc75fe6d 100644 --- a/examples/otlp/CMakeLists.txt +++ b/examples/otlp/CMakeLists.txt @@ -1,6 +1,11 @@ -add_library(foo_library foo_library/foo_library.cc) -target_link_libraries(foo_library ${CMAKE_THREAD_LIBS_INIT} opentelemetry_api) +include_directories( + ${CMAKE_BINARY_DIR}/generated/third_party/opentelemetry-proto) +include_directories(${CMAKE_SOURCE_DIR}/exporters/otlp/include) + +add_library(otlp_foo_library foo_library/foo_library.cc) +target_link_libraries(otlp_foo_library ${CMAKE_THREAD_LIBS_INIT} + opentelemetry_api) add_executable(example_otlp main.cc) -target_link_libraries(example_otlp ${CMAKE_THREAD_LIBS_INIT} foo_library - opentelemetry_trace) +target_link_libraries(example_otlp ${CMAKE_THREAD_LIBS_INIT} otlp_foo_library + opentelemetry_trace opentelemetry_exporter_otprotocol) diff --git a/exporters/prometheus/BUILD b/exporters/prometheus/BUILD index 0852586ec4..01a2fe14f3 100644 --- a/exporters/prometheus/BUILD +++ b/exporters/prometheus/BUILD @@ -14,6 +14,24 @@ 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_library( name = "prometheus_utils", srcs = [ @@ -30,6 +48,17 @@ cc_library( ], ) +cc_test( + name = "prometheus_collector_test", + srcs = [ + "test/prometheus_collector_test.cc", + ], + deps = [ + ":prometheus_collector", + "@com_google_googletest//:gtest_main", + ], +) + cc_test( name = "prometheus_exporter_utils_test", srcs = [ diff --git a/exporters/prometheus/CMakeLists.txt b/exporters/prometheus/CMakeLists.txt index 8a2658e97b..1a58b175fb 100644 --- a/exporters/prometheus/CMakeLists.txt +++ b/exporters/prometheus/CMakeLists.txt @@ -15,7 +15,9 @@ include_directories(include) find_package(prometheus-cpp CONFIG REQUIRED) -add_library(prometheus_exporter src/prometheus_exporter_utils.cc) + +add_library(prometheus_exporter src/prometheus_collector.cc + src/prometheus_exporter_utils.cc) if(BUILD_TESTING) add_subdirectory(test) 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..cc7256c925 --- /dev/null +++ b/exporters/prometheus/src/prometheus_collector.cc @@ -0,0 +1,175 @@ +/* + * 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 +{ + 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; + + this->collection_lock_.lock(); + copied_data = std::vector(*metrics_to_collect_); + metrics_to_collect_->clear(); + this->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 (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 + * 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; + }; + + for (auto &r : records) + { + if (ValidAggregator(r)) + { + metrics_to_collect_->emplace_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 index a42feed797..c57f900347 100644 --- a/exporters/prometheus/test/CMakeLists.txt +++ b/exporters/prometheus/test/CMakeLists.txt @@ -1,4 +1,4 @@ -foreach(testname prometheus_exporter_utils_test) +foreach(testname prometheus_collector_test prometheus_exporter_utils_test) add_executable(${testname} "${testname}.cc") target_link_libraries( ${testname} ${GTEST_BOTH_LIBRARIES} ${CMAKE_THREAD_LIBS_INIT} diff --git a/exporters/prometheus/test/prometheus_collector_test.cc b/exporters/prometheus/test/prometheus_collector_test.cc new file mode 100644 index 0000000000..444302fa44 --- /dev/null +++ b/exporters/prometheus/test/prometheus_collector_test.cc @@ -0,0 +1,773 @@ +/* + * 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 diff --git a/sdk/include/opentelemetry/sdk/metrics/instrument.h b/sdk/include/opentelemetry/sdk/metrics/instrument.h index 85de5ee701..802e201bc6 100644 --- a/sdk/include/opentelemetry/sdk/metrics/instrument.h +++ b/sdk/include/opentelemetry/sdk/metrics/instrument.h @@ -244,11 +244,9 @@ inline void print_value(std::stringstream &ss, switch (value.index()) { case common::AttributeType::TYPE_STRING: - if (jsonTypes) - ss << '"'; + ss << nostd::get(value); - if (jsonTypes) - ss << '"'; + break; default: #if __EXCEPTIONS @@ -282,7 +280,7 @@ inline std::string KvToString(const trace::KeyValueIterable &kv) noexcept { size_t i = 1; kv.ForEachKeyValue([&](nostd::string_view key, common::AttributeValue value) noexcept { - ss << "\"" << key << "\":"; + ss << key << ":"; print_value(ss, value, true); if (size != i) { diff --git a/sdk/src/trace/span.cc b/sdk/src/trace/span.cc index c6c3151a7f..c27c071b62 100644 --- a/sdk/src/trace/span.cc +++ b/sdk/src/trace/span.cc @@ -2,6 +2,7 @@ #include "src/common/random.h" #include "opentelemetry/context/runtime_context.h" +#include "opentelemetry/trace/trace_flags.h" #include "opentelemetry/version.h" OPENTELEMETRY_BEGIN_NAMESPACE @@ -77,16 +78,22 @@ Span::Span(std::shared_ptr &&tracer, } recordable_->SetName(name); + trace_api::TraceId trace_id; + trace_api::SpanId span_id = GenerateRandomSpanId(); + if (parent_span_context.IsValid()) { - recordable_->SetIds(parent_span_context.trace_id(), GenerateRandomSpanId(), - parent_span_context.span_id()); + trace_id = parent_span_context.trace_id(); + recordable_->SetIds(trace_id, span_id, parent_span_context.span_id()); } else { - recordable_->SetIds(GenerateRandomTraceId(), GenerateRandomSpanId(), trace_api::SpanId()); + trace_id = GenerateRandomTraceId(); + recordable_->SetIds(trace_id, span_id, trace_api::SpanId()); } - // TODO: Create and populate SpanContext for this span when SpanContext is fully implemented + + span_context_ = std::unique_ptr( + new trace_api::SpanContext(trace_id, span_id, trace_api::TraceFlags(), false)); attributes.ForEachKeyValue([&](nostd::string_view key, opentelemetry::common::AttributeValue value) noexcept { diff --git a/sdk/src/trace/tracer.cc b/sdk/src/trace/tracer.cc index 3923a47161..f7b2d38033 100644 --- a/sdk/src/trace/tracer.cc +++ b/sdk/src/trace/tracer.cc @@ -30,6 +30,20 @@ std::shared_ptr Tracer::GetSampler() const noexcept return sampler_; } +// Helper function to extract the current span context from the runtime context. +// Returns an invalid span context if the runtime context doesn't contain a span. +trace_api::SpanContext GetCurrentSpanContext() +{ + context::ContextValue curr_span_context = context::RuntimeContext::GetValue(SpanKey); + + if (nostd::holds_alternative>(curr_span_context)) + { + auto curr_span = nostd::get>(curr_span_context); + return curr_span->GetContext(); + } + return trace_api::SpanContext(); +} + nostd::shared_ptr Tracer::StartSpan( nostd::string_view name, const trace_api::KeyValueIterable &attributes, @@ -47,13 +61,9 @@ nostd::shared_ptr Tracer::StartSpan( } else { - // TODO: Get parent span context from current context. For now we assume all spans - // have no parent, and pass an invalid parent span context to the Span constructor. - const trace_api::SpanContext kInvalidParentSpanContext(false, false); - auto span = nostd::shared_ptr{ new (std::nothrow) Span{this->shared_from_this(), processor_.load(), name, attributes, - options, kInvalidParentSpanContext}}; + options, GetCurrentSpanContext()}}; span->SetToken( nostd::unique_ptr(new context::Token(context::RuntimeContext::Attach( diff --git a/sdk/test/metrics/metric_instrument_test.cc b/sdk/test/metrics/metric_instrument_test.cc index 3caf526010..28ca482375 100644 --- a/sdk/test/metrics/metric_instrument_test.cc +++ b/sdk/test/metrics/metric_instrument_test.cc @@ -35,7 +35,7 @@ TEST(ApiSdkConversion, async) alpha->observe(123456, labelkv); EXPECT_EQ(dynamic_cast *>(alpha.get())->GetRecords()[0].GetLabels(), - "{\"key587\":\"value264\"}"); + "{key587:value264}"); alpha->observe(123456, labelkv); AggregatorVariant canCollect = @@ -258,7 +258,7 @@ TEST(Counter, getAggsandnewupdate) EXPECT_EQ(theta.size(), 1); EXPECT_EQ(theta[0].GetName(), "test"); EXPECT_EQ(theta[0].GetDescription(), "none"); - EXPECT_EQ(theta[0].GetLabels(), "{\"key2\":\"value2\",\"key3\":\"value3\"}"); + EXPECT_EQ(theta[0].GetLabels(), "{key2:value2,key3:value3}"); } void CounterCallback(std::shared_ptr> in, diff --git a/sdk/test/trace/tracer_test.cc b/sdk/test/trace/tracer_test.cc index efdc6b6f8b..891cce22d4 100644 --- a/sdk/test/trace/tracer_test.cc +++ b/sdk/test/trace/tracer_test.cc @@ -113,11 +113,16 @@ TEST(Tracer, ToMockSpanExporter) ASSERT_EQ("span 2", spans_received->at(0)->GetName()); EXPECT_TRUE(spans_received->at(0)->GetTraceId().IsValid()); EXPECT_TRUE(spans_received->at(0)->GetSpanId().IsValid()); - EXPECT_FALSE(spans_received->at(0)->GetParentSpanId().IsValid()); + EXPECT_TRUE(spans_received->at(0)->GetParentSpanId().IsValid()); span_first->End(); ASSERT_EQ(2, spans_received->size()); + // Verify trace and parent span id propagation + EXPECT_EQ(span_second->GetContext().trace_id(), span_first->GetContext().trace_id()); + EXPECT_EQ(spans_received->at(0)->GetTraceId(), spans_received->at(1)->GetTraceId()); + EXPECT_EQ(spans_received->at(0)->GetParentSpanId(), spans_received->at(1)->GetSpanId()); + ASSERT_EQ("span 1", spans_received->at(1)->GetName()); EXPECT_TRUE(spans_received->at(1)->GetTraceId().IsValid()); EXPECT_TRUE(spans_received->at(1)->GetSpanId().IsValid()); From b667c845396547058b5dbb882b45d5522188de20 Mon Sep 17 00:00:00 2001 From: Max Golovanov Date: Thu, 3 Sep 2020 13:36:30 -0700 Subject: [PATCH 06/18] Compiler warnings clean-up --- api/include/opentelemetry/context/context.h | 5 +++-- api/include/opentelemetry/nostd/string_view.h | 20 +++++++++++++++++++ api/include/opentelemetry/version.h | 3 +++ api/test/metrics/noop_metrics_test.cc | 1 + api/test/nostd/utility_test.cc | 7 +++++-- .../trace/key_value_iterable_view_test.cc | 2 +- .../exporters/ostream/metrics_exporter.h | 11 +++++----- .../exporters/ostream/span_exporter.h | 4 ++-- .../zpages/tracez_data_aggregator_test.cc | 4 ++-- ext/test/zpages/tracez_processor_test.cc | 10 +++++----- .../sdk/common/atomic_unique_ptr.h | 2 +- .../sdk/common/empty_attributes.h | 3 +++ .../sdk/metrics/aggregator/exact_aggregator.h | 4 ++-- .../metrics/aggregator/histogram_aggregator.h | 2 +- .../metrics/aggregator/sketch_aggregator.h | 2 +- sdk/test/common/circular_buffer_benchmark.cc | 8 ++++---- sdk/test/common/circular_buffer_test.cc | 1 + 17 files changed, 61 insertions(+), 28 deletions(-) diff --git a/api/include/opentelemetry/context/context.h b/api/include/opentelemetry/context/context.h index a1379cdc64..f14311a5b0 100644 --- a/api/include/opentelemetry/context/context.h +++ b/api/include/opentelemetry/context/context.h @@ -3,6 +3,7 @@ #include "opentelemetry/context/context_value.h" #include "opentelemetry/nostd/shared_ptr.h" #include "opentelemetry/nostd/string_view.h" +#include OPENTELEMETRY_BEGIN_NAMESPACE namespace context @@ -65,7 +66,7 @@ class Context { if (key.size() == data->key_length_) { - if (memcmp(key.data(), data->key_, data->key_length_) == 0) + if (std::memcmp(key.data(), data->key_, data->key_length_) == 0) { return data->value_; } @@ -81,7 +82,7 @@ class Context { if (key.size() == data->key_length_) { - if (memcmp(key.data(), data->key_, data->key_length_) == 0) + if (std::memcmp(key.data(), data->key_, data->key_length_) == 0) { return true; } diff --git a/api/include/opentelemetry/nostd/string_view.h b/api/include/opentelemetry/nostd/string_view.h index 315d038b3f..a9853e9d71 100644 --- a/api/include/opentelemetry/nostd/string_view.h +++ b/api/include/opentelemetry/nostd/string_view.h @@ -118,7 +118,12 @@ class string_view inline bool operator==(string_view lhs, string_view rhs) noexcept { return lhs.length() == rhs.length() && +#if _MSC_VER == 1900 + // Avoid SCL error in Visual Studio 2015 + (std::memcmp(lhs.data(), rhs.data(), lhs.length()) == 0); +#else std::equal(lhs.data(), lhs.data() + lhs.length(), rhs.data()); +#endif } inline bool operator==(string_view lhs, const std::string &rhs) noexcept @@ -172,3 +177,18 @@ inline std::ostream &operator<<(std::ostream &os, string_view s) } } // namespace nostd OPENTELEMETRY_END_NAMESPACE + +namespace std { + template <> + struct hash + { + std::size_t operator()(const OPENTELEMETRY_NAMESPACE::nostd::string_view& k) const + { + // TODO: for C++17 that has native support for std::basic_string_view it would + // be more performance-efficient to provide a zero-copy hash. + auto s = std::string(k.data(), k.size()); + return std::hash{}(s); + } + }; +} +#endif diff --git a/api/include/opentelemetry/version.h b/api/include/opentelemetry/version.h index 1bbe71a217..aa0d16b0cc 100644 --- a/api/include/opentelemetry/version.h +++ b/api/include/opentelemetry/version.h @@ -12,4 +12,7 @@ #define OPENTELEMETRY_END_NAMESPACE \ }} + +#define OPENTELEMETRY_NAMESPACE opentelemetry :: OPENTELEMETRY_CONCAT(v, OPENTELEMETRY_ABI_VERSION_NO) + // clang-format on diff --git a/api/test/metrics/noop_metrics_test.cc b/api/test/metrics/noop_metrics_test.cc index 85533e070e..a75ed623f0 100644 --- a/api/test/metrics/noop_metrics_test.cc +++ b/api/test/metrics/noop_metrics_test.cc @@ -5,6 +5,7 @@ #include "opentelemetry/metrics/sync_instruments.h" #include +#include OPENTELEMETRY_BEGIN_NAMESPACE diff --git a/api/test/nostd/utility_test.cc b/api/test/nostd/utility_test.cc index ee38f3f474..2bd7be50a5 100644 --- a/api/test/nostd/utility_test.cc +++ b/api/test/nostd/utility_test.cc @@ -2,6 +2,7 @@ #include #include +#include #include @@ -20,7 +21,8 @@ TEST(UtilityTest, Data) std::vector v = {1, 2, 3}; int array[3] = {1, 2, 3}; std::initializer_list list{1, 2, 3}; - int x; + int x = 0; + std::ignore = x; EXPECT_EQ(opentelemetry::nostd::data(v), v.data()); EXPECT_EQ(opentelemetry::nostd::data(array), array); @@ -32,7 +34,8 @@ TEST(UtilityTest, Size) { std::vector v = {1, 2, 3}; int array[3] = {1, 2, 3}; - int x; + int x = 0; + std::ignore = x; EXPECT_EQ(opentelemetry::nostd::size(v), v.size()); EXPECT_EQ(opentelemetry::nostd::size(array), 3); diff --git a/api/test/trace/key_value_iterable_view_test.cc b/api/test/trace/key_value_iterable_view_test.cc index 9c262eb3f1..88c0a64de7 100644 --- a/api/test/trace/key_value_iterable_view_test.cc +++ b/api/test/trace/key_value_iterable_view_test.cc @@ -1,7 +1,7 @@ #include "opentelemetry/trace/key_value_iterable_view.h" #include - +#include "opentelemetry/nostd/type_traits.h" #include using namespace opentelemetry; diff --git a/exporters/ostream/include/opentelemetry/exporters/ostream/metrics_exporter.h b/exporters/ostream/include/opentelemetry/exporters/ostream/metrics_exporter.h index 312aecb6ea..9dbb3ab362 100644 --- a/exporters/ostream/include/opentelemetry/exporters/ostream/metrics_exporter.h +++ b/exporters/ostream/include/opentelemetry/exporters/ostream/metrics_exporter.h @@ -1,6 +1,7 @@ #pragma once #include +#include #include "opentelemetry/sdk/metrics/aggregator/exact_aggregator.h" #include "opentelemetry/sdk/metrics/aggregator/gauge_aggregator.h" #include "opentelemetry/sdk/metrics/aggregator/histogram_aggregator.h" @@ -83,7 +84,7 @@ class OStreamMetricsExporter final : public sdkmetrics::MetricsExporter else { auto vec = agg->get_checkpoint(); - int size = vec.size(); + size_t size = vec.size(); int i = 1; sout_ << "\n values : " << '['; @@ -104,8 +105,8 @@ class OStreamMetricsExporter final : public sdkmetrics::MetricsExporter auto boundaries = agg->get_boundaries(); auto counts = agg->get_counts(); - int boundaries_size = boundaries.size(); - int counts_size = counts.size(); + size_t boundaries_size = boundaries.size(); + size_t counts_size = counts.size(); sout_ << "\n buckets : " << '['; @@ -134,8 +135,8 @@ class OStreamMetricsExporter final : public sdkmetrics::MetricsExporter auto boundaries = agg->get_boundaries(); auto counts = agg->get_counts(); - int boundaries_size = boundaries.size(); - int counts_size = counts.size(); + size_t boundaries_size = boundaries.size(); + size_t counts_size = counts.size(); sout_ << "\n buckets : " << '['; diff --git a/exporters/ostream/include/opentelemetry/exporters/ostream/span_exporter.h b/exporters/ostream/include/opentelemetry/exporters/ostream/span_exporter.h index e7dd1ed42a..a5077d1e8a 100644 --- a/exporters/ostream/include/opentelemetry/exporters/ostream/span_exporter.h +++ b/exporters/ostream/include/opentelemetry/exporters/ostream/span_exporter.h @@ -146,8 +146,8 @@ class OStreamSpanExporter final : public sdktrace::SpanExporter void printAttributes(std::unordered_map map) { - int size = map.size(); - int i = 1; + size_t size = map.size(); + size_t i = 1; for (auto kv : map) { sout_ << kv.first << ": "; diff --git a/ext/test/zpages/tracez_data_aggregator_test.cc b/ext/test/zpages/tracez_data_aggregator_test.cc index 035b2d8f26..64a873d2fb 100644 --- a/ext/test/zpages/tracez_data_aggregator_test.cc +++ b/ext/test/zpages/tracez_data_aggregator_test.cc @@ -51,8 +51,8 @@ class TracezDataAggregatorTest : public ::testing::Test void VerifySpanCountsInTracezData( const std::string &span_name, const TracezData &aggregated_data, - unsigned int running_span_count, - unsigned int error_span_count, + size_t running_span_count, + size_t error_span_count, std::array completed_span_count_per_latency_bucket) { // Asserts are needed to check the size of the container because they may need diff --git a/ext/test/zpages/tracez_processor_test.cc b/ext/test/zpages/tracez_processor_test.cc index 7b4c633926..68e2d49104 100644 --- a/ext/test/zpages/tracez_processor_test.cc +++ b/ext/test/zpages/tracez_processor_test.cc @@ -49,8 +49,8 @@ void UpdateSpans(std::shared_ptr &processor, */ bool ContainsNames(const std::vector &names, std::unordered_set &running, - unsigned int name_start = 0, - unsigned int name_end = 0, + size_t name_start = 0, + size_t name_end = 0, bool one_to_one_correspondence = false) { if (name_end == 0) @@ -97,15 +97,15 @@ bool ContainsNames(const std::vector &names, */ bool ContainsNames(const std::vector &names, std::vector> &completed, - unsigned int name_start = 0, - unsigned int name_end = 0, + size_t name_start = 0, + size_t name_end = 0, bool one_to_one_correspondence = false) { if (name_end == 0) name_end = names.size(); - unsigned int num_names = name_end - name_start; + size_t num_names = name_end - name_start; if (num_names > completed.size() || (one_to_one_correspondence && num_names != completed.size())) { diff --git a/sdk/include/opentelemetry/sdk/common/atomic_unique_ptr.h b/sdk/include/opentelemetry/sdk/common/atomic_unique_ptr.h index 1fdfee618c..d93d9b72b2 100644 --- a/sdk/include/opentelemetry/sdk/common/atomic_unique_ptr.h +++ b/sdk/include/opentelemetry/sdk/common/atomic_unique_ptr.h @@ -34,7 +34,7 @@ class AtomicUniquePtr /** * @return true if the pointer is null */ - bool IsNull() const noexcept { return ptr_ == nullptr; } + bool IsNull() const noexcept { return ptr_.load() == nullptr; } /** * Atomically swap the pointer only if it's null. diff --git a/sdk/include/opentelemetry/sdk/common/empty_attributes.h b/sdk/include/opentelemetry/sdk/common/empty_attributes.h index b4c69abe7d..b9741cd3a9 100644 --- a/sdk/include/opentelemetry/sdk/common/empty_attributes.h +++ b/sdk/include/opentelemetry/sdk/common/empty_attributes.h @@ -1,6 +1,9 @@ #include "opentelemetry/trace/key_value_iterable_view.h" +#include #include +#include +#include OPENTELEMETRY_BEGIN_NAMESPACE namespace sdk diff --git a/sdk/include/opentelemetry/sdk/metrics/aggregator/exact_aggregator.h b/sdk/include/opentelemetry/sdk/metrics/aggregator/exact_aggregator.h index f1a5242c48..b34bfd0643 100644 --- a/sdk/include/opentelemetry/sdk/metrics/aggregator/exact_aggregator.h +++ b/sdk/include/opentelemetry/sdk/metrics/aggregator/exact_aggregator.h @@ -145,8 +145,8 @@ class ExactAggregator : public Aggregator } else { - float position = float(this->checkpoint_.size() - 1) * q; - int ceiling = ceil(position); + float position = float(float(this->checkpoint_.size() - 1) * q); + int ceiling = int(ceil(position)); return this->checkpoint_[ceiling]; } } diff --git a/sdk/include/opentelemetry/sdk/metrics/aggregator/histogram_aggregator.h b/sdk/include/opentelemetry/sdk/metrics/aggregator/histogram_aggregator.h index 07b05c247c..130dde99bd 100644 --- a/sdk/include/opentelemetry/sdk/metrics/aggregator/histogram_aggregator.h +++ b/sdk/include/opentelemetry/sdk/metrics/aggregator/histogram_aggregator.h @@ -64,7 +64,7 @@ class HistogramAggregator final : public Aggregator { this->mu_.lock(); this->updated_ = true; - int bucketID = boundaries_.size(); + size_t bucketID = boundaries_.size(); for (size_t i = 0; i < boundaries_.size(); i++) { if (val < boundaries_[i]) // concurrent read is thread-safe diff --git a/sdk/include/opentelemetry/sdk/metrics/aggregator/sketch_aggregator.h b/sdk/include/opentelemetry/sdk/metrics/aggregator/sketch_aggregator.h index bc29868320..f0c07e0587 100644 --- a/sdk/include/opentelemetry/sdk/metrics/aggregator/sketch_aggregator.h +++ b/sdk/include/opentelemetry/sdk/metrics/aggregator/sketch_aggregator.h @@ -118,7 +118,7 @@ class SketchAggregator final : public Aggregator idx = iter->first; count += iter->second; } - return round(2 * pow(gamma, idx) / (gamma + 1)); + return (T)(round(2 * pow(gamma, idx) / (gamma + 1))); } /** diff --git a/sdk/test/common/circular_buffer_benchmark.cc b/sdk/test/common/circular_buffer_benchmark.cc index cec8c95f5f..cb6eba6482 100644 --- a/sdk/test/common/circular_buffer_benchmark.cc +++ b/sdk/test/common/circular_buffer_benchmark.cc @@ -102,8 +102,8 @@ static void RunSimulation(Buffer &buffer, int num_threads, int n) noexcept static void BM_BaselineBuffer(benchmark::State &state) { const size_t max_elements = 500; - auto num_threads = state.range(0); - const int n = N / num_threads; + const int num_threads = static_cast(state.range(0)); + const int n = static_cast(N / num_threads); BaselineCircularBuffer buffer{max_elements}; for (auto _ : state) { @@ -116,8 +116,8 @@ BENCHMARK(BM_BaselineBuffer)->Arg(1)->Arg(2)->Arg(4); static void BM_LockFreeBuffer(benchmark::State &state) { const size_t max_elements = 500; - auto num_threads = state.range(0); - const int n = N / num_threads; + const int num_threads = static_cast(state.range(0)); + const int n = static_cast(N / num_threads); CircularBuffer buffer{max_elements}; for (auto _ : state) { diff --git a/sdk/test/common/circular_buffer_test.cc b/sdk/test/common/circular_buffer_test.cc index 2fd9dc4dcc..04fc173045 100644 --- a/sdk/test/common/circular_buffer_test.cc +++ b/sdk/test/common/circular_buffer_test.cc @@ -3,6 +3,7 @@ #include #include #include +#include #include using opentelemetry::sdk::common::AtomicUniquePtr; From 806296fa1feb26dd7aaf1db680e39916dd286339 Mon Sep 17 00:00:00 2001 From: Max Golovanov Date: Thu, 3 Sep 2020 13:51:11 -0700 Subject: [PATCH 07/18] Remove extra #endif --- api/include/opentelemetry/nostd/string_view.h | 1 - 1 file changed, 1 deletion(-) diff --git a/api/include/opentelemetry/nostd/string_view.h b/api/include/opentelemetry/nostd/string_view.h index a9853e9d71..16bb0e33db 100644 --- a/api/include/opentelemetry/nostd/string_view.h +++ b/api/include/opentelemetry/nostd/string_view.h @@ -191,4 +191,3 @@ namespace std { } }; } -#endif From d94bd2fd8469ced02b9b8518eec7609f6c10544c Mon Sep 17 00:00:00 2001 From: Max Golovanov Date: Thu, 3 Sep 2020 14:20:46 -0700 Subject: [PATCH 08/18] clang format --- api/include/opentelemetry/context/context.h | 2 +- api/include/opentelemetry/nostd/string_view.h | 25 ++++++------- api/test/metrics/noop_metrics_test.cc | 2 +- api/test/nostd/utility_test.cc | 8 ++--- ext/test/zpages/tracez_processor_test.cc | 8 ++--- .../opentelemetry/sdk/trace/attribute_utils.h | 10 +++--- sdk/test/common/circular_buffer_test.cc | 36 +++++++++---------- 7 files changed, 45 insertions(+), 46 deletions(-) diff --git a/api/include/opentelemetry/context/context.h b/api/include/opentelemetry/context/context.h index f14311a5b0..06b95a6248 100644 --- a/api/include/opentelemetry/context/context.h +++ b/api/include/opentelemetry/context/context.h @@ -1,9 +1,9 @@ #pragma once +#include #include "opentelemetry/context/context_value.h" #include "opentelemetry/nostd/shared_ptr.h" #include "opentelemetry/nostd/string_view.h" -#include OPENTELEMETRY_BEGIN_NAMESPACE namespace context diff --git a/api/include/opentelemetry/nostd/string_view.h b/api/include/opentelemetry/nostd/string_view.h index 16bb0e33db..a5d65d00aa 100644 --- a/api/include/opentelemetry/nostd/string_view.h +++ b/api/include/opentelemetry/nostd/string_view.h @@ -178,16 +178,17 @@ inline std::ostream &operator<<(std::ostream &os, string_view s) } // namespace nostd OPENTELEMETRY_END_NAMESPACE -namespace std { - template <> - struct hash +namespace std +{ +template <> +struct hash +{ + std::size_t operator()(const OPENTELEMETRY_NAMESPACE::nostd::string_view &k) const { - std::size_t operator()(const OPENTELEMETRY_NAMESPACE::nostd::string_view& k) const - { - // TODO: for C++17 that has native support for std::basic_string_view it would - // be more performance-efficient to provide a zero-copy hash. - auto s = std::string(k.data(), k.size()); - return std::hash{}(s); - } - }; -} + // TODO: for C++17 that has native support for std::basic_string_view it would + // be more performance-efficient to provide a zero-copy hash. + auto s = std::string(k.data(), k.size()); + return std::hash{}(s); + } +}; +} // namespace std diff --git a/api/test/metrics/noop_metrics_test.cc b/api/test/metrics/noop_metrics_test.cc index a75ed623f0..9580ebede5 100644 --- a/api/test/metrics/noop_metrics_test.cc +++ b/api/test/metrics/noop_metrics_test.cc @@ -4,8 +4,8 @@ #include "opentelemetry/metrics/observer_result.h" #include "opentelemetry/metrics/sync_instruments.h" -#include #include +#include OPENTELEMETRY_BEGIN_NAMESPACE diff --git a/api/test/nostd/utility_test.cc b/api/test/nostd/utility_test.cc index 2bd7be50a5..edf5cd3f9e 100644 --- a/api/test/nostd/utility_test.cc +++ b/api/test/nostd/utility_test.cc @@ -1,8 +1,8 @@ #include "opentelemetry/nostd/utility.h" +#include #include #include -#include #include @@ -21,7 +21,7 @@ TEST(UtilityTest, Data) std::vector v = {1, 2, 3}; int array[3] = {1, 2, 3}; std::initializer_list list{1, 2, 3}; - int x = 0; + int x = 0; std::ignore = x; EXPECT_EQ(opentelemetry::nostd::data(v), v.data()); @@ -34,8 +34,8 @@ TEST(UtilityTest, Size) { std::vector v = {1, 2, 3}; int array[3] = {1, 2, 3}; - int x = 0; - std::ignore = x; + int x = 0; + std::ignore = x; EXPECT_EQ(opentelemetry::nostd::size(v), v.size()); EXPECT_EQ(opentelemetry::nostd::size(array), 3); diff --git a/ext/test/zpages/tracez_processor_test.cc b/ext/test/zpages/tracez_processor_test.cc index 68e2d49104..ad356d880f 100644 --- a/ext/test/zpages/tracez_processor_test.cc +++ b/ext/test/zpages/tracez_processor_test.cc @@ -49,8 +49,8 @@ void UpdateSpans(std::shared_ptr &processor, */ bool ContainsNames(const std::vector &names, std::unordered_set &running, - size_t name_start = 0, - size_t name_end = 0, + size_t name_start = 0, + size_t name_end = 0, bool one_to_one_correspondence = false) { if (name_end == 0) @@ -97,8 +97,8 @@ bool ContainsNames(const std::vector &names, */ bool ContainsNames(const std::vector &names, std::vector> &completed, - size_t name_start = 0, - size_t name_end = 0, + size_t name_start = 0, + size_t name_end = 0, bool one_to_one_correspondence = false) { diff --git a/sdk/include/opentelemetry/sdk/trace/attribute_utils.h b/sdk/include/opentelemetry/sdk/trace/attribute_utils.h index b836afbaee..2e6d1a3701 100644 --- a/sdk/include/opentelemetry/sdk/trace/attribute_utils.h +++ b/sdk/include/opentelemetry/sdk/trace/attribute_utils.h @@ -96,11 +96,11 @@ class AttributeMap // Contruct attribute map and populate with attributes AttributeMap(const opentelemetry::trace::KeyValueIterable &attributes) { - attributes.ForEachKeyValue([&](nostd::string_view key, - opentelemetry::common::AttributeValue value) noexcept { - SetAttribute(key, value); - return true; - }); + attributes.ForEachKeyValue( + [&](nostd::string_view key, opentelemetry::common::AttributeValue value) noexcept { + SetAttribute(key, value); + return true; + }); } const std::unordered_map &GetAttributes() const noexcept diff --git a/sdk/test/common/circular_buffer_test.cc b/sdk/test/common/circular_buffer_test.cc index 04fc173045..4d993920fd 100644 --- a/sdk/test/common/circular_buffer_test.cc +++ b/sdk/test/common/circular_buffer_test.cc @@ -1,9 +1,9 @@ #include "opentelemetry/sdk/common/circular_buffer.h" +#include #include #include #include -#include #include using opentelemetry::sdk::common::AtomicUniquePtr; @@ -62,16 +62,15 @@ void RunNumberConsumer(CircularBuffer &buffer, return; } auto n = std::uniform_int_distribution{0, allotment.size()}(RandomNumberGenerator); - buffer.Consume( - n, [&](CircularBufferRange> range) noexcept { - assert(range.size() == n); - range.ForEach([&](AtomicUniquePtr & ptr) noexcept { - assert(!ptr.IsNull()); - numbers.push_back(*ptr); - ptr.Reset(); - return true; - }); - }); + buffer.Consume(n, [&](CircularBufferRange> range) noexcept { + assert(range.size() == n); + range.ForEach([&](AtomicUniquePtr &ptr) noexcept { + assert(!ptr.IsNull()); + numbers.push_back(*ptr); + ptr.Reset(); + return true; + }); + }); } } @@ -124,14 +123,13 @@ TEST(CircularBufferTest, Consume) EXPECT_TRUE(buffer.Add(x)); } int count = 0; - buffer.Consume( - 5, [&](CircularBufferRange> range) noexcept { - range.ForEach([&](AtomicUniquePtr &ptr) { - EXPECT_EQ(*ptr, count++); - ptr.Reset(); - return true; - }); - }); + buffer.Consume(5, [&](CircularBufferRange> range) noexcept { + range.ForEach([&](AtomicUniquePtr &ptr) { + EXPECT_EQ(*ptr, count++); + ptr.Reset(); + return true; + }); + }); EXPECT_EQ(count, 5); } From aad6e5eba95a9691401a906185428eaf9668dfc9 Mon Sep 17 00:00:00 2001 From: Max Golovanov Date: Thu, 3 Sep 2020 14:24:37 -0700 Subject: [PATCH 09/18] Format --- api/test/trace/key_value_iterable_view_test.cc | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/api/test/trace/key_value_iterable_view_test.cc b/api/test/trace/key_value_iterable_view_test.cc index 88c0a64de7..0d296ad730 100644 --- a/api/test/trace/key_value_iterable_view_test.cc +++ b/api/test/trace/key_value_iterable_view_test.cc @@ -1,8 +1,8 @@ #include "opentelemetry/trace/key_value_iterable_view.h" +#include #include #include "opentelemetry/nostd/type_traits.h" -#include using namespace opentelemetry; @@ -55,11 +55,11 @@ TEST(KeyValueIterableViewTest, ForEachKeyValueWithExit) M m1 = {{"abc", "123"}, {"xyz", "456"}}; trace::KeyValueIterableView iterable{m1}; int count = 0; - auto exit = iterable.ForEachKeyValue([&count](nostd::string_view /*key*/, - common::AttributeValue /*value*/) noexcept { - ++count; - return false; - }); + auto exit = iterable.ForEachKeyValue( + [&count](nostd::string_view /*key*/, common::AttributeValue /*value*/) noexcept { + ++count; + return false; + }); EXPECT_EQ(count, 1); EXPECT_FALSE(exit); } From 069ab1d59dad123757c63de0ad4b63002b6d837c Mon Sep 17 00:00:00 2001 From: Max Golovanov Date: Thu, 3 Sep 2020 14:27:04 -0700 Subject: [PATCH 10/18] Format --- .../exporters/ostream/metrics_exporter.h | 22 +++++++------------ 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/exporters/ostream/include/opentelemetry/exporters/ostream/metrics_exporter.h b/exporters/ostream/include/opentelemetry/exporters/ostream/metrics_exporter.h index 9dbb3ab362..3de94cefdf 100644 --- a/exporters/ostream/include/opentelemetry/exporters/ostream/metrics_exporter.h +++ b/exporters/ostream/include/opentelemetry/exporters/ostream/metrics_exporter.h @@ -49,28 +49,24 @@ class OStreamMetricsExporter final : public sdkmetrics::MetricsExporter return; switch (aggKind) { - case sdkmetrics::AggregatorKind::Counter: - { + case sdkmetrics::AggregatorKind::Counter: { sout_ << "\n sum : " << agg->get_checkpoint()[0]; } break; - case sdkmetrics::AggregatorKind::MinMaxSumCount: - { + case sdkmetrics::AggregatorKind::MinMaxSumCount: { auto mmsc = agg->get_checkpoint(); sout_ << "\n min : " << mmsc[0] << "\n max : " << mmsc[1] << "\n sum : " << mmsc[2] << "\n count : " << mmsc[3]; } break; - case sdkmetrics::AggregatorKind::Gauge: - { + case sdkmetrics::AggregatorKind::Gauge: { auto timestamp = agg->get_checkpoint_timestamp(); sout_ << "\n last value : " << agg->get_checkpoint()[0] << "\n timestamp : " << std::to_string(timestamp.time_since_epoch().count()); } break; - case sdkmetrics::AggregatorKind::Exact: - { + case sdkmetrics::AggregatorKind::Exact: { // TODO: Find better way to print quantiles if (agg->get_quant_estimation()) { @@ -83,9 +79,9 @@ class OStreamMetricsExporter final : public sdkmetrics::MetricsExporter } else { - auto vec = agg->get_checkpoint(); + auto vec = agg->get_checkpoint(); size_t size = vec.size(); - int i = 1; + int i = 1; sout_ << "\n values : " << '['; @@ -100,8 +96,7 @@ class OStreamMetricsExporter final : public sdkmetrics::MetricsExporter } } break; - case sdkmetrics::AggregatorKind::Histogram: - { + case sdkmetrics::AggregatorKind::Histogram: { auto boundaries = agg->get_boundaries(); auto counts = agg->get_counts(); @@ -130,8 +125,7 @@ class OStreamMetricsExporter final : public sdkmetrics::MetricsExporter sout_ << ']'; } break; - case sdkmetrics::AggregatorKind::Sketch: - { + case sdkmetrics::AggregatorKind::Sketch: { auto boundaries = agg->get_boundaries(); auto counts = agg->get_counts(); From 17eb8cce62048f6f5cb3c9e3d8c580f6af57aa61 Mon Sep 17 00:00:00 2001 From: Max Golovanov Date: Thu, 3 Sep 2020 14:27:38 -0700 Subject: [PATCH 11/18] Format --- .../sdk/metrics/aggregator/histogram_aggregator.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/metrics/aggregator/histogram_aggregator.h b/sdk/include/opentelemetry/sdk/metrics/aggregator/histogram_aggregator.h index 130dde99bd..785ae2733d 100644 --- a/sdk/include/opentelemetry/sdk/metrics/aggregator/histogram_aggregator.h +++ b/sdk/include/opentelemetry/sdk/metrics/aggregator/histogram_aggregator.h @@ -63,8 +63,8 @@ class HistogramAggregator final : public Aggregator void update(T val) override { this->mu_.lock(); - this->updated_ = true; - size_t bucketID = boundaries_.size(); + this->updated_ = true; + size_t bucketID = boundaries_.size(); for (size_t i = 0; i < boundaries_.size(); i++) { if (val < boundaries_[i]) // concurrent read is thread-safe From 9ba6d62f426e8e5b23e19e341c3ec50ea68c4e54 Mon Sep 17 00:00:00 2001 From: Max Golovanov Date: Thu, 3 Sep 2020 14:29:11 -0700 Subject: [PATCH 12/18] Format --- sdk/src/trace/span.cc | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/sdk/src/trace/span.cc b/sdk/src/trace/span.cc index c27c071b62..4320630657 100644 --- a/sdk/src/trace/span.cc +++ b/sdk/src/trace/span.cc @@ -95,11 +95,11 @@ Span::Span(std::shared_ptr &&tracer, span_context_ = std::unique_ptr( new trace_api::SpanContext(trace_id, span_id, trace_api::TraceFlags(), false)); - attributes.ForEachKeyValue([&](nostd::string_view key, - opentelemetry::common::AttributeValue value) noexcept { - recordable_->SetAttribute(key, value); - return true; - }); + attributes.ForEachKeyValue( + [&](nostd::string_view key, opentelemetry::common::AttributeValue value) noexcept { + recordable_->SetAttribute(key, value); + return true; + }); recordable_->SetStartTime(NowOr(options.start_system_time)); start_steady_time = NowOr(options.start_steady_time); From 6a132efd2af6eea4875c21a43b33c34cd7cb65b3 Mon Sep 17 00:00:00 2001 From: Max Golovanov Date: Thu, 3 Sep 2020 14:29:26 -0700 Subject: [PATCH 13/18] Format --- .../test/prometheus_collector_test.cc | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/exporters/prometheus/test/prometheus_collector_test.cc b/exporters/prometheus/test/prometheus_collector_test.cc index 444302fa44..a7e5d4d1b2 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; From 5b594b06a090b5114d8f20bef396526d96ef725b Mon Sep 17 00:00:00 2001 From: Max Golovanov Date: Thu, 3 Sep 2020 14:29:36 -0700 Subject: [PATCH 14/18] Format --- .../opentelemetry/sdk/common/circular_buffer.h | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/common/circular_buffer.h b/sdk/include/opentelemetry/sdk/common/circular_buffer.h index a8d5d44a50..f0d1bd7331 100644 --- a/sdk/include/opentelemetry/sdk/common/circular_buffer.h +++ b/sdk/include/opentelemetry/sdk/common/circular_buffer.h @@ -63,13 +63,12 @@ class CircularBuffer */ void Consume(size_t n) noexcept { - Consume( - n, [](CircularBufferRange> & range) noexcept { - range.ForEach([](AtomicUniquePtr & ptr) noexcept { - ptr.Reset(); - return true; - }); - }); + Consume(n, [](CircularBufferRange> &range) noexcept { + range.ForEach([](AtomicUniquePtr &ptr) noexcept { + ptr.Reset(); + return true; + }); + }); } /** From 49098c72f4bf2eb3d42ed186986652bd83906329 Mon Sep 17 00:00:00 2001 From: Max Golovanov Date: Thu, 3 Sep 2020 14:29:53 -0700 Subject: [PATCH 15/18] Format --- sdk/test/common/circular_buffer_benchmark.cc | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/sdk/test/common/circular_buffer_benchmark.cc b/sdk/test/common/circular_buffer_benchmark.cc index cb6eba6482..7308893361 100644 --- a/sdk/test/common/circular_buffer_benchmark.cc +++ b/sdk/test/common/circular_buffer_benchmark.cc @@ -30,14 +30,14 @@ static uint64_t ConsumeBufferNumbers(BaselineCircularBuffer &buffer) n static uint64_t ConsumeBufferNumbers(CircularBuffer &buffer) noexcept { uint64_t result = 0; - buffer.Consume( - buffer.size(), [&](CircularBufferRange> & range) noexcept { - range.ForEach([&](AtomicUniquePtr & ptr) noexcept { - result += *ptr; - ptr.Reset(); - return true; - }); - }); + buffer.Consume(buffer.size(), + [&](CircularBufferRange> &range) noexcept { + range.ForEach([&](AtomicUniquePtr &ptr) noexcept { + result += *ptr; + ptr.Reset(); + return true; + }); + }); return result; } From d8eb8d9e360c429c2ee1d98fdc0d5b5a70dd9fa5 Mon Sep 17 00:00:00 2001 From: Max Golovanov Date: Thu, 3 Sep 2020 14:30:04 -0700 Subject: [PATCH 16/18] Format --- sdk/src/trace/batch_span_processor.cc | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/sdk/src/trace/batch_span_processor.cc b/sdk/src/trace/batch_span_processor.cc index 111fbae0d5..a358a73f3c 100644 --- a/sdk/src/trace/batch_span_processor.cc +++ b/sdk/src/trace/batch_span_processor.cc @@ -142,15 +142,15 @@ void BatchSpanProcessor::Export(const bool was_force_flush_called) buffer_.size() >= max_export_batch_size_ ? max_export_batch_size_ : buffer_.size(); } - buffer_.Consume( - num_spans_to_export, [&](CircularBufferRange> range) noexcept { - range.ForEach([&](AtomicUniquePtr &ptr) { - std::unique_ptr swap_ptr = std::unique_ptr(nullptr); - ptr.Swap(swap_ptr); - spans_arr.push_back(std::unique_ptr(swap_ptr.release())); - return true; - }); - }); + buffer_.Consume(num_spans_to_export, + [&](CircularBufferRange> range) noexcept { + range.ForEach([&](AtomicUniquePtr &ptr) { + std::unique_ptr swap_ptr = std::unique_ptr(nullptr); + ptr.Swap(swap_ptr); + spans_arr.push_back(std::unique_ptr(swap_ptr.release())); + return true; + }); + }); exporter_->Export(nostd::span>(spans_arr.data(), spans_arr.size())); From ab2611ce76da9da14f5fb03de9cdbdb55ab9c13a Mon Sep 17 00:00:00 2001 From: Max Golovanov Date: Thu, 3 Sep 2020 14:36:46 -0700 Subject: [PATCH 17/18] One more try to format :D --- .../trace/key_value_iterable_view_test.cc | 10 +++--- .../exporters/ostream/metrics_exporter.h | 18 ++++++---- .../test/prometheus_collector_test.cc | 18 ++++++---- .../sdk/common/circular_buffer.h | 13 +++---- .../opentelemetry/sdk/trace/attribute_utils.h | 10 +++--- sdk/src/trace/batch_span_processor.cc | 18 +++++----- sdk/src/trace/span.cc | 10 +++--- sdk/test/common/circular_buffer_benchmark.cc | 16 ++++----- sdk/test/common/circular_buffer_test.cc | 34 ++++++++++--------- 9 files changed, 81 insertions(+), 66 deletions(-) diff --git a/api/test/trace/key_value_iterable_view_test.cc b/api/test/trace/key_value_iterable_view_test.cc index 0d296ad730..641e649473 100644 --- a/api/test/trace/key_value_iterable_view_test.cc +++ b/api/test/trace/key_value_iterable_view_test.cc @@ -55,11 +55,11 @@ TEST(KeyValueIterableViewTest, ForEachKeyValueWithExit) M m1 = {{"abc", "123"}, {"xyz", "456"}}; trace::KeyValueIterableView iterable{m1}; int count = 0; - auto exit = iterable.ForEachKeyValue( - [&count](nostd::string_view /*key*/, common::AttributeValue /*value*/) noexcept { - ++count; - return false; - }); + auto exit = iterable.ForEachKeyValue([&count](nostd::string_view /*key*/, + common::AttributeValue /*value*/) noexcept { + ++count; + return false; + }); EXPECT_EQ(count, 1); EXPECT_FALSE(exit); } diff --git a/exporters/ostream/include/opentelemetry/exporters/ostream/metrics_exporter.h b/exporters/ostream/include/opentelemetry/exporters/ostream/metrics_exporter.h index 3de94cefdf..0b31c779db 100644 --- a/exporters/ostream/include/opentelemetry/exporters/ostream/metrics_exporter.h +++ b/exporters/ostream/include/opentelemetry/exporters/ostream/metrics_exporter.h @@ -49,24 +49,28 @@ class OStreamMetricsExporter final : public sdkmetrics::MetricsExporter return; switch (aggKind) { - case sdkmetrics::AggregatorKind::Counter: { + case sdkmetrics::AggregatorKind::Counter: + { sout_ << "\n sum : " << agg->get_checkpoint()[0]; } break; - case sdkmetrics::AggregatorKind::MinMaxSumCount: { + case sdkmetrics::AggregatorKind::MinMaxSumCount: + { auto mmsc = agg->get_checkpoint(); sout_ << "\n min : " << mmsc[0] << "\n max : " << mmsc[1] << "\n sum : " << mmsc[2] << "\n count : " << mmsc[3]; } break; - case sdkmetrics::AggregatorKind::Gauge: { + case sdkmetrics::AggregatorKind::Gauge: + { auto timestamp = agg->get_checkpoint_timestamp(); sout_ << "\n last value : " << agg->get_checkpoint()[0] << "\n timestamp : " << std::to_string(timestamp.time_since_epoch().count()); } break; - case sdkmetrics::AggregatorKind::Exact: { + case sdkmetrics::AggregatorKind::Exact: + { // TODO: Find better way to print quantiles if (agg->get_quant_estimation()) { @@ -96,7 +100,8 @@ class OStreamMetricsExporter final : public sdkmetrics::MetricsExporter } } break; - case sdkmetrics::AggregatorKind::Histogram: { + case sdkmetrics::AggregatorKind::Histogram: + { auto boundaries = agg->get_boundaries(); auto counts = agg->get_counts(); @@ -125,7 +130,8 @@ class OStreamMetricsExporter final : public sdkmetrics::MetricsExporter sout_ << ']'; } break; - case sdkmetrics::AggregatorKind::Sketch: { + case sdkmetrics::AggregatorKind::Sketch: + { auto boundaries = agg->get_boundaries(); auto counts = agg->get_counts(); diff --git a/exporters/prometheus/test/prometheus_collector_test.cc b/exporters/prometheus/test/prometheus_collector_test.cc index a7e5d4d1b2..444302fa44 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; diff --git a/sdk/include/opentelemetry/sdk/common/circular_buffer.h b/sdk/include/opentelemetry/sdk/common/circular_buffer.h index f0d1bd7331..a8d5d44a50 100644 --- a/sdk/include/opentelemetry/sdk/common/circular_buffer.h +++ b/sdk/include/opentelemetry/sdk/common/circular_buffer.h @@ -63,12 +63,13 @@ class CircularBuffer */ void Consume(size_t n) noexcept { - Consume(n, [](CircularBufferRange> &range) noexcept { - range.ForEach([](AtomicUniquePtr &ptr) noexcept { - ptr.Reset(); - return true; - }); - }); + Consume( + n, [](CircularBufferRange> & range) noexcept { + range.ForEach([](AtomicUniquePtr & ptr) noexcept { + ptr.Reset(); + return true; + }); + }); } /** diff --git a/sdk/include/opentelemetry/sdk/trace/attribute_utils.h b/sdk/include/opentelemetry/sdk/trace/attribute_utils.h index 2e6d1a3701..b836afbaee 100644 --- a/sdk/include/opentelemetry/sdk/trace/attribute_utils.h +++ b/sdk/include/opentelemetry/sdk/trace/attribute_utils.h @@ -96,11 +96,11 @@ class AttributeMap // Contruct attribute map and populate with attributes AttributeMap(const opentelemetry::trace::KeyValueIterable &attributes) { - attributes.ForEachKeyValue( - [&](nostd::string_view key, opentelemetry::common::AttributeValue value) noexcept { - SetAttribute(key, value); - return true; - }); + attributes.ForEachKeyValue([&](nostd::string_view key, + opentelemetry::common::AttributeValue value) noexcept { + SetAttribute(key, value); + return true; + }); } const std::unordered_map &GetAttributes() const noexcept diff --git a/sdk/src/trace/batch_span_processor.cc b/sdk/src/trace/batch_span_processor.cc index a358a73f3c..111fbae0d5 100644 --- a/sdk/src/trace/batch_span_processor.cc +++ b/sdk/src/trace/batch_span_processor.cc @@ -142,15 +142,15 @@ void BatchSpanProcessor::Export(const bool was_force_flush_called) buffer_.size() >= max_export_batch_size_ ? max_export_batch_size_ : buffer_.size(); } - buffer_.Consume(num_spans_to_export, - [&](CircularBufferRange> range) noexcept { - range.ForEach([&](AtomicUniquePtr &ptr) { - std::unique_ptr swap_ptr = std::unique_ptr(nullptr); - ptr.Swap(swap_ptr); - spans_arr.push_back(std::unique_ptr(swap_ptr.release())); - return true; - }); - }); + buffer_.Consume( + num_spans_to_export, [&](CircularBufferRange> range) noexcept { + range.ForEach([&](AtomicUniquePtr &ptr) { + std::unique_ptr swap_ptr = std::unique_ptr(nullptr); + ptr.Swap(swap_ptr); + spans_arr.push_back(std::unique_ptr(swap_ptr.release())); + return true; + }); + }); exporter_->Export(nostd::span>(spans_arr.data(), spans_arr.size())); diff --git a/sdk/src/trace/span.cc b/sdk/src/trace/span.cc index 4320630657..c27c071b62 100644 --- a/sdk/src/trace/span.cc +++ b/sdk/src/trace/span.cc @@ -95,11 +95,11 @@ Span::Span(std::shared_ptr &&tracer, span_context_ = std::unique_ptr( new trace_api::SpanContext(trace_id, span_id, trace_api::TraceFlags(), false)); - attributes.ForEachKeyValue( - [&](nostd::string_view key, opentelemetry::common::AttributeValue value) noexcept { - recordable_->SetAttribute(key, value); - return true; - }); + attributes.ForEachKeyValue([&](nostd::string_view key, + opentelemetry::common::AttributeValue value) noexcept { + recordable_->SetAttribute(key, value); + return true; + }); recordable_->SetStartTime(NowOr(options.start_system_time)); start_steady_time = NowOr(options.start_steady_time); diff --git a/sdk/test/common/circular_buffer_benchmark.cc b/sdk/test/common/circular_buffer_benchmark.cc index 7308893361..cb6eba6482 100644 --- a/sdk/test/common/circular_buffer_benchmark.cc +++ b/sdk/test/common/circular_buffer_benchmark.cc @@ -30,14 +30,14 @@ static uint64_t ConsumeBufferNumbers(BaselineCircularBuffer &buffer) n static uint64_t ConsumeBufferNumbers(CircularBuffer &buffer) noexcept { uint64_t result = 0; - buffer.Consume(buffer.size(), - [&](CircularBufferRange> &range) noexcept { - range.ForEach([&](AtomicUniquePtr &ptr) noexcept { - result += *ptr; - ptr.Reset(); - return true; - }); - }); + buffer.Consume( + buffer.size(), [&](CircularBufferRange> & range) noexcept { + range.ForEach([&](AtomicUniquePtr & ptr) noexcept { + result += *ptr; + ptr.Reset(); + return true; + }); + }); return result; } diff --git a/sdk/test/common/circular_buffer_test.cc b/sdk/test/common/circular_buffer_test.cc index 4d993920fd..404e8a0325 100644 --- a/sdk/test/common/circular_buffer_test.cc +++ b/sdk/test/common/circular_buffer_test.cc @@ -62,15 +62,16 @@ void RunNumberConsumer(CircularBuffer &buffer, return; } auto n = std::uniform_int_distribution{0, allotment.size()}(RandomNumberGenerator); - buffer.Consume(n, [&](CircularBufferRange> range) noexcept { - assert(range.size() == n); - range.ForEach([&](AtomicUniquePtr &ptr) noexcept { - assert(!ptr.IsNull()); - numbers.push_back(*ptr); - ptr.Reset(); - return true; - }); - }); + buffer.Consume( + n, [&](CircularBufferRange> range) noexcept { + assert(range.size() == n); + range.ForEach([&](AtomicUniquePtr & ptr) noexcept { + assert(!ptr.IsNull()); + numbers.push_back(*ptr); + ptr.Reset(); + return true; + }); + }); } } @@ -123,13 +124,14 @@ TEST(CircularBufferTest, Consume) EXPECT_TRUE(buffer.Add(x)); } int count = 0; - buffer.Consume(5, [&](CircularBufferRange> range) noexcept { - range.ForEach([&](AtomicUniquePtr &ptr) { - EXPECT_EQ(*ptr, count++); - ptr.Reset(); - return true; - }); - }); + buffer.Consume( + 5, [&](CircularBufferRange> range) noexcept { + range.ForEach([&](AtomicUniquePtr &ptr) { + EXPECT_EQ(*ptr, count++); + ptr.Reset(); + return true; + }); + }); EXPECT_EQ(count, 5); } From fc805cd2795923bccf03ad4cf1e5be859c329885 Mon Sep 17 00:00:00 2001 From: Max Golovanov Date: Fri, 4 Sep 2020 15:23:51 -0700 Subject: [PATCH 18/18] Addressing code review comments --- .../opentelemetry/sdk/metrics/aggregator/sketch_aggregator.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/include/opentelemetry/sdk/metrics/aggregator/sketch_aggregator.h b/sdk/include/opentelemetry/sdk/metrics/aggregator/sketch_aggregator.h index f0c07e0587..617d320570 100644 --- a/sdk/include/opentelemetry/sdk/metrics/aggregator/sketch_aggregator.h +++ b/sdk/include/opentelemetry/sdk/metrics/aggregator/sketch_aggregator.h @@ -118,7 +118,7 @@ class SketchAggregator final : public Aggregator idx = iter->first; count += iter->second; } - return (T)(round(2 * pow(gamma, idx) / (gamma + 1))); + return static_cast(round(2 * pow(gamma, idx) / (gamma + 1))); } /**