From 32af3529efc50bd42d558be06b9206ee0dadb1e0 Mon Sep 17 00:00:00 2001 From: Marc Alff Date: Fri, 3 Feb 2023 06:22:19 +0100 Subject: [PATCH 1/5] [BUILD] OTLP HTTP Exporter has build warnings in maintainer mode (#1943) --- .github/workflows/ci.yml | 4 +++ CHANGELOG.md | 2 ++ ci/do_ci.sh | 2 ++ ci/install_protobuf.sh | 36 ++++++++++++++++--- exporters/otlp/src/otlp_metric_utils.cc | 2 +- .../sdk/metrics/state/sync_metric_storage.h | 17 ++++++--- 6 files changed, 52 insertions(+), 11 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7a692615fe..47d30524b8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -41,9 +41,11 @@ jobs: CC: /usr/bin/gcc-12 CXX: /usr/bin/g++-12 GOOGLETEST_VERSION: 1.12.1 + PROTOBUF_VERSION: 21.12 run: | sudo -E ./ci/setup_cmake.sh sudo -E ./ci/setup_ci_environment.sh + sudo -E ./ci/install_protobuf.sh - name: run cmake gcc (maintainer mode) env: CC: /usr/bin/gcc-12 @@ -64,9 +66,11 @@ jobs: CC: /usr/bin/clang-14 CXX: /usr/bin/clang++-14 GOOGLETEST_VERSION: 1.12.1 + PROTOBUF_VERSION: 21.12 run: | sudo -E ./ci/setup_cmake.sh sudo -E ./ci/setup_ci_environment.sh + sudo -E ./ci/install_protobuf.sh - name: run cmake clang (maintainer mode) env: CC: /usr/bin/clang-14 diff --git a/CHANGELOG.md b/CHANGELOG.md index 0a412ef10b..b73e3f1a7f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -64,6 +64,8 @@ Increment the: * [MAINTAINER DOC] Define and document a deprecation process, [DEPRECATION] Deprecate the Jaeger exporter, implemented by [#1923](https://github.com/open-telemetry/opentelemetry-cpp/pull/1923) +* [BUILD] OTLP HTTP Exporter has build warnings in maintainer mode + [#1943](https://github.com/open-telemetry/opentelemetry-cpp/pull/1943) Deprecations: diff --git a/ci/do_ci.sh b/ci/do_ci.sh index 4af28ba41e..5154618952 100755 --- a/ci/do_ci.sh +++ b/ci/do_ci.sh @@ -94,6 +94,8 @@ elif [[ "$1" == "cmake.maintainer.test" ]]; then cd "${BUILD_DIR}" rm -rf * cmake -DCMAKE_BUILD_TYPE=Debug \ + -DWITH_OTLP=ON \ + -DWITH_OTLP_HTTP=ON \ -DWITH_PROMETHEUS=ON \ -DWITH_EXAMPLES=ON \ -DWITH_EXAMPLES_HTTP=ON \ diff --git a/ci/install_protobuf.sh b/ci/install_protobuf.sh index 8fff91d062..dde94111e1 100755 --- a/ci/install_protobuf.sh +++ b/ci/install_protobuf.sh @@ -5,12 +5,38 @@ set -e -[ -z "${PROTOBUF_VERSION}" ] && export PROTOBUF_VERSION="3.11.4" +[ -z "${PROTOBUF_VERSION}" ] && export PROTOBUF_VERSION="21.12" + +# +# Note +# +# protobuf uses two release number schemes, +# for example 3.21.12 and 21.12, +# and both tags corresponds to the same commit: +# +# commit f0dc78d7e6e331b8c6bb2d5283e06aa26883ca7c (HEAD -> release-3.21.12, tag: v3.21.12, tag: v21.12) +# Author: Protobuf Team Bot +# Date: Mon Dec 12 16:03:12 2022 -0800 +# +# Updating version.json and repo version numbers to: 21.12 +# +# tag v21.12 corresponds to the 'protoc version', or repo version +# tag v3.21.12 corresponds to the 'cpp version' +# +# protobuf-cpp-3.21.12.tar.gz: +# - is provided under releases/download/v21.12 +# - is no longer provided under releases/download/v3.21.12, +# +# Use the "repo version number" (PROTOBUF_VERSION=21.12) +# when calling this script +# + +export CPP_PROTOBUF_VERSION="3.${PROTOBUF_VERSION}" cd / -wget https://github.com/google/protobuf/releases/download/v${PROTOBUF_VERSION}/protobuf-cpp-${PROTOBUF_VERSION}.tar.gz -tar zxf protobuf-cpp-${PROTOBUF_VERSION}.tar.gz --no-same-owner -cd protobuf-${PROTOBUF_VERSION} +wget https://github.com/google/protobuf/releases/download/v${PROTOBUF_VERSION}/protobuf-cpp-${CPP_PROTOBUF_VERSION}.tar.gz +tar zxf protobuf-cpp-${CPP_PROTOBUF_VERSION}.tar.gz --no-same-owner +cd protobuf-${CPP_PROTOBUF_VERSION} ./configure -make && make install +make -j $(nproc) && make install ldconfig diff --git a/exporters/otlp/src/otlp_metric_utils.cc b/exporters/otlp/src/otlp_metric_utils.cc index f3ee79e283..c570b7d827 100644 --- a/exporters/otlp/src/otlp_metric_utils.cc +++ b/exporters/otlp/src/otlp_metric_utils.cc @@ -266,7 +266,7 @@ sdk::metrics::AggregationTemporality OtlpMetricUtils::DeltaTemporalitySelector( } sdk::metrics::AggregationTemporality OtlpMetricUtils::CumulativeTemporalitySelector( - sdk::metrics::InstrumentType instrument_type) noexcept + sdk::metrics::InstrumentType /* instrument_type */) noexcept { return sdk::metrics::AggregationTemporality::kCumulative; } diff --git a/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h b/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h index 7099a44c46..da4fb2980a 100644 --- a/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h +++ b/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h @@ -30,7 +30,8 @@ class SyncMetricStorage : public MetricStorage, public SyncWritableMetricStorage SyncMetricStorage(InstrumentDescriptor instrument_descriptor, const AggregationType aggregation_type, const AttributesProcessor *attributes_processor, - nostd::shared_ptr &&exemplar_reservoir, + nostd::shared_ptr &&exemplar_reservoir + OPENTELEMETRY_MAYBE_UNUSED, const AggregationConfig *aggregation_config) : instrument_descriptor_(instrument_descriptor), attributes_hashmap_(new AttributesHashMap()), @@ -48,7 +49,9 @@ class SyncMetricStorage : public MetricStorage, public SyncWritableMetricStorage }; } - void RecordLong(int64_t value, const opentelemetry::context::Context &context) noexcept override + void RecordLong(int64_t value, + const opentelemetry::context::Context &context + OPENTELEMETRY_MAYBE_UNUSED) noexcept override { if (instrument_descriptor_.value_type_ != InstrumentValueType::kLong) { @@ -63,7 +66,8 @@ class SyncMetricStorage : public MetricStorage, public SyncWritableMetricStorage void RecordLong(int64_t value, const opentelemetry::common::KeyValueIterable &attributes, - const opentelemetry::context::Context &context) noexcept override + const opentelemetry::context::Context &context + OPENTELEMETRY_MAYBE_UNUSED) noexcept override { if (instrument_descriptor_.value_type_ != InstrumentValueType::kLong) { @@ -78,7 +82,9 @@ class SyncMetricStorage : public MetricStorage, public SyncWritableMetricStorage attributes_hashmap_->GetOrSetDefault(attr, create_default_aggregation_)->Aggregate(value); } - void RecordDouble(double value, const opentelemetry::context::Context &context) noexcept override + void RecordDouble(double value, + const opentelemetry::context::Context &context + OPENTELEMETRY_MAYBE_UNUSED) noexcept override { if (instrument_descriptor_.value_type_ != InstrumentValueType::kDouble) { @@ -93,7 +99,8 @@ class SyncMetricStorage : public MetricStorage, public SyncWritableMetricStorage void RecordDouble(double value, const opentelemetry::common::KeyValueIterable &attributes, - const opentelemetry::context::Context &context) noexcept override + const opentelemetry::context::Context &context + OPENTELEMETRY_MAYBE_UNUSED) noexcept override { #ifdef ENABLE_METRICS_EXEMPLAR_PREVIEW exemplar_reservoir_->OfferMeasurement(value, attributes, context, From 9200d0c8fb4684e5fffd2c6915c8117a747c8e2b Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Sat, 4 Feb 2023 20:32:33 -0800 Subject: [PATCH 2/5] upgrade prometheus-cpp to v1.1.0 (#1954) --- CHANGELOG.md | 2 ++ bazel/repository.bzl | 6 +++--- exporters/prometheus/test/exporter_utils_test.cc | 4 +++- third_party/prometheus-cpp | 2 +- third_party_release | 2 +- 5 files changed, 10 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b73e3f1a7f..0b2ef9554e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,8 @@ Increment the: ## [Unreleased] +* Upgrade prometheus-cpp to v1.1.0 [#1954](https://github.com/open-telemetry/opentelemetry-cpp/pull/1954) + ## [1.8.2] 2023-01-31 * Remove redundant macro check in nostd::shared_ptr [#1939](https://github.com/open-telemetry/opentelemetry-cpp/pull/1939) diff --git a/bazel/repository.bzl b/bazel/repository.bzl index 210ee20258..a3d77026c6 100644 --- a/bazel/repository.bzl +++ b/bazel/repository.bzl @@ -119,10 +119,10 @@ def opentelemetry_cpp_deps(): maybe( http_archive, name = "com_github_jupp0r_prometheus_cpp", - sha256 = "07018db604ea3e61f5078583e87c80932ea10c300d979061490ee1b7dc8e3a41", - strip_prefix = "prometheus-cpp-1.0.0", + sha256 = "397544fe91e183029120b4eebcfab24ed9ec833d15850aae78fd5db19062d13a", + strip_prefix = "prometheus-cpp-1.1.0", urls = [ - "https://github.com/jupp0r/prometheus-cpp/archive/refs/tags/v1.0.0.tar.gz", + "https://github.com/jupp0r/prometheus-cpp/archive/refs/tags/v1.1.0.tar.gz", ], ) diff --git a/exporters/prometheus/test/exporter_utils_test.cc b/exporters/prometheus/test/exporter_utils_test.cc index 25e7ad0fdd..8a14c2effa 100644 --- a/exporters/prometheus/test/exporter_utils_test.cc +++ b/exporters/prometheus/test/exporter_utils_test.cc @@ -48,11 +48,13 @@ void assert_basic(prometheus_client::MetricFamily &metric, } break; case prometheus_client::MetricType::Summary: - // Summary type not supported + // Summary and Info type not supported ASSERT_TRUE(false); break; case prometheus::MetricType::Untyped: break; + default: + break; } } diff --git a/third_party/prometheus-cpp b/third_party/prometheus-cpp index 4ea303fa66..c9ffcdda90 160000 --- a/third_party/prometheus-cpp +++ b/third_party/prometheus-cpp @@ -1 +1 @@ -Subproject commit 4ea303fa66e4c26dc4df67045fa0edf09c2f3077 +Subproject commit c9ffcdda9086ffd9e1283ea7a0276d831f3c8a8d diff --git a/third_party_release b/third_party_release index 3362b23db9..8ec284f2b9 100644 --- a/third_party_release +++ b/third_party_release @@ -19,5 +19,5 @@ googletest=release-1.12.1 ms-gsl=v3.1.0-67-g6f45293 nlohmann-json=v3.10.5 opentelemetry-proto=v0.19.0 -prometheus-cpp=v1.0.0 +prometheus-cpp=v1.1.0 vcpkg=2022.08.15 From c4c8ee053f68d052f2287dd15dffcf42cc9e98a8 Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Sun, 5 Feb 2023 10:12:09 -0800 Subject: [PATCH 3/5] Convert Prometheus Exporter to Pull MetricReader (#1953) --- CHANGELOG.md | 1 + ci/do_ci.sh | 4 +- examples/prometheus/main.cc | 13 +- exporters/prometheus/BUILD | 2 + .../exporters/prometheus/collector.h | 38 +- .../exporters/prometheus/exporter.h | 59 +-- .../exporters/prometheus/exporter_utils.h | 2 +- exporters/prometheus/src/collector.cc | 60 +-- exporters/prometheus/src/exporter.cc | 75 +--- exporters/prometheus/src/exporter_utils.cc | 145 ++++--- exporters/prometheus/test/collector_test.cc | 368 +++--------------- exporters/prometheus/test/exporter_test.cc | 137 ++----- .../prometheus/test/exporter_utils_test.cc | 45 +-- .../opentelemetry/sdk/metrics/metric_reader.h | 9 +- 14 files changed, 198 insertions(+), 760 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0b2ef9554e..b38b92b9d3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ Increment the: ## [Unreleased] +* Convert Prometheus Exporter to Pull MetricReader [#1953](https://github.com/open-telemetry/opentelemetry-cpp/pull/1953) * Upgrade prometheus-cpp to v1.1.0 [#1954](https://github.com/open-telemetry/opentelemetry-cpp/pull/1954) ## [1.8.2] 2023-01-31 diff --git a/ci/do_ci.sh b/ci/do_ci.sh index 5154618952..00f77a581b 100755 --- a/ci/do_ci.sh +++ b/ci/do_ci.sh @@ -298,7 +298,9 @@ elif [[ "$1" == "bazel.asan" ]]; then //examples/metrics_simple:metrics_ostream_example > /dev/null exit 0 elif [[ "$1" == "bazel.tsan" ]]; then - bazel $BAZEL_STARTUP_OPTIONS test --config=tsan $BAZEL_TEST_OPTIONS_ASYNC //... +# TODO - potential race condition in Civetweb server used by prometheus-cpp during shutdown +# https://github.com/civetweb/civetweb/issues/861, so removing prometheus from the test + bazel $BAZEL_STARTUP_OPTIONS test --config=tsan $BAZEL_TEST_OPTIONS_ASYNC -- //... -//exporters/prometheus/... bazel $BAZEL_STARTUP_OPTIONS run --config=tsan $BAZEL_TEST_OPTIONS_ASYNC \ //examples/metrics_simple:metrics_ostream_example > /dev/null exit 0 diff --git a/examples/prometheus/main.cc b/examples/prometheus/main.cc index 567dfd4843..7a23006b6a 100644 --- a/examples/prometheus/main.cc +++ b/examples/prometheus/main.cc @@ -34,21 +34,16 @@ void InitMetrics(const std::string &name, const std::string &addr) } std::puts("PrometheusExporter example program running ..."); - std::unique_ptr exporter{ - new metrics_exporter::PrometheusExporter(opts)}; - std::string version{"1.2.0"}; std::string schema{"https://opentelemetry.io/schemas/1.2.0"}; + std::shared_ptr prometheus_exporter( + new metrics_exporter::PrometheusExporter(opts)); + // Initialize and set the global MeterProvider - metrics_sdk::PeriodicExportingMetricReaderOptions options; - options.export_interval_millis = std::chrono::milliseconds(1000); - options.export_timeout_millis = std::chrono::milliseconds(500); - std::unique_ptr reader{ - new metrics_sdk::PeriodicExportingMetricReader(std::move(exporter), options)}; auto provider = std::shared_ptr(new metrics_sdk::MeterProvider()); auto p = std::static_pointer_cast(provider); - p->AddMetricReader(std::move(reader)); + p->AddMetricReader(prometheus_exporter); // counter view std::string counter_name = name + "_counter"; diff --git a/exporters/prometheus/BUILD b/exporters/prometheus/BUILD index 832a6f8acc..f8af28015a 100644 --- a/exporters/prometheus/BUILD +++ b/exporters/prometheus/BUILD @@ -96,6 +96,7 @@ cc_test( deps = [ ":prometheus_exporter", ":prometheus_test_helper", + "//sdk/src/metrics", "@com_google_googletest//:gtest_main", ], ) @@ -112,6 +113,7 @@ cc_test( deps = [ ":prometheus_collector", ":prometheus_test_helper", + "//sdk/src/metrics", "@com_google_googletest//:gtest_main", ], ) diff --git a/exporters/prometheus/include/opentelemetry/exporters/prometheus/collector.h b/exporters/prometheus/include/opentelemetry/exporters/prometheus/collector.h index 2e7489e983..46d270905b 100644 --- a/exporters/prometheus/include/opentelemetry/exporters/prometheus/collector.h +++ b/exporters/prometheus/include/opentelemetry/exporters/prometheus/collector.h @@ -10,6 +10,7 @@ #include #include #include "opentelemetry/exporters/prometheus/exporter_utils.h" +#include "opentelemetry/sdk/metrics/metric_reader.h" namespace prometheus_client = ::prometheus; @@ -30,7 +31,7 @@ class PrometheusCollector : public prometheus_client::Collectable * This constructor initializes the collection for metrics to export * in this class with default capacity */ - explicit PrometheusCollector(size_t max_collection_size = 2048); + explicit PrometheusCollector(sdk::metrics::MetricReader *reader); /** * Collects all metrics data from metricsToCollect collection. @@ -39,41 +40,8 @@ class PrometheusCollector : public prometheus_client::Collectable */ 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 sdk::metrics::ResourceMetrics &data); - - /** - * 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. - */ - mutable std::vector> metrics_to_collect_; - - /** - * Maximum size of the metricsToCollect collection. - */ - size_t max_collection_size_; + sdk::metrics::MetricReader *reader_; /* * Lock when operating the metricsToCollect collection diff --git a/exporters/prometheus/include/opentelemetry/exporters/prometheus/exporter.h b/exporters/prometheus/include/opentelemetry/exporters/prometheus/exporter.h index ab05fc71db..b3baebde70 100644 --- a/exporters/prometheus/include/opentelemetry/exporters/prometheus/exporter.h +++ b/exporters/prometheus/include/opentelemetry/exporters/prometheus/exporter.h @@ -12,7 +12,7 @@ #include "opentelemetry/exporters/prometheus/collector.h" #include "opentelemetry/nostd/span.h" #include "opentelemetry/sdk/common/env_variables.h" -#include "opentelemetry/sdk/metrics/push_metric_exporter.h" +#include "opentelemetry/sdk/metrics/metric_reader.h" #include "opentelemetry/version.h" /** @@ -46,7 +46,7 @@ struct PrometheusExporterOptions std::string url = GetPrometheusDefaultHttpEndpoint(); }; -class PrometheusExporter : public sdk::metrics::PushMetricExporter +class PrometheusExporter : public sdk::metrics::MetricReader { public: /** @@ -56,54 +56,12 @@ class PrometheusExporter : public sdk::metrics::PushMetricExporter */ PrometheusExporter(const PrometheusExporterOptions &options); - /** - * Get the AggregationTemporality for Prometheus exporter - * - * @return AggregationTemporality - */ sdk::metrics::AggregationTemporality GetAggregationTemporality( sdk::metrics::InstrumentType instrument_type) const noexcept override; - /** - * Exports a batch of Metric Records. - * @param records: a collection of records to export - * @return: returns a ReturnCode detailing a success, or type of failure - */ - sdk::common::ExportResult Export(const sdk::metrics::ResourceMetrics &data) noexcept override; - - /** - * Force flush the exporter. - */ - bool ForceFlush( - std::chrono::microseconds timeout = (std::chrono::microseconds::max)()) noexcept override; - - /** - * Shuts down the exporter and does cleanup. - * Since Prometheus is a pull based interface, - * we cannot serve data remaining in the intermediate - * collection to to client an HTTP request being sent, - * so we flush the data. - */ - bool Shutdown(std::chrono::microseconds timeout = std::chrono::microseconds(0)) noexcept override; - - /** - * @return: returns a shared_ptr to - * the PrometheusCollector instance - */ - std::shared_ptr &GetCollector(); - - /** - * @return: Gets the shutdown status of the exporter - */ - bool IsShutdown() const; - private: // The configuration options associated with this exporter. const PrometheusExporterOptions options_; - /** - * exporter shutdown status - */ - bool is_shutdown_; /** * Pointer to a @@ -117,16 +75,11 @@ class PrometheusExporter : public sdk::metrics::PushMetricExporter */ std::unique_ptr<::prometheus::Exposer> exposer_; - /** - * friend class for testing - */ - friend class PrometheusExporterTest; + bool OnForceFlush(std::chrono::microseconds timeout) noexcept override; - /** - * PrometheusExporter constructor with no parameters - * Used for testing only - */ - PrometheusExporter(); + bool OnShutDown(std::chrono::microseconds timeout) noexcept override; + + void OnInitialized() noexcept override; }; } // namespace metrics } // namespace exporter diff --git a/exporters/prometheus/include/opentelemetry/exporters/prometheus/exporter_utils.h b/exporters/prometheus/include/opentelemetry/exporters/prometheus/exporter_utils.h index ab5f50058c..e602c87737 100644 --- a/exporters/prometheus/include/opentelemetry/exporters/prometheus/exporter_utils.h +++ b/exporters/prometheus/include/opentelemetry/exporters/prometheus/exporter_utils.h @@ -29,7 +29,7 @@ class PrometheusExporterUtils * @return a collection of translated metrics that is acceptable by Prometheus */ static std::vector<::prometheus::MetricFamily> TranslateToPrometheus( - const std::vector> &data); + const sdk::metrics::ResourceMetrics &data); private: /** diff --git a/exporters/prometheus/src/collector.cc b/exporters/prometheus/src/collector.cc index 07dd40da98..77d4255679 100644 --- a/exporters/prometheus/src/collector.cc +++ b/exporters/prometheus/src/collector.cc @@ -16,9 +16,7 @@ namespace metrics * This constructor initializes the collection for metrics to export * in this class with default capacity */ -PrometheusCollector::PrometheusCollector(size_t max_collection_size) - : max_collection_size_(max_collection_size) -{} +PrometheusCollector::PrometheusCollector(sdk::metrics::MetricReader *reader) : reader_(reader) {} /** * Collects all metrics data from metricsToCollect collection. @@ -27,59 +25,21 @@ PrometheusCollector::PrometheusCollector(size_t max_collection_size) */ std::vector PrometheusCollector::Collect() const { - this->collection_lock_.lock(); - if (metrics_to_collect_.empty()) + if (reader_->IsShutdown()) { - this->collection_lock_.unlock(); return {}; } + collection_lock_.lock(); std::vector result; - - // copy the intermediate collection, and then clear it - std::vector> copied_data; - copied_data.swap(metrics_to_collect_); - 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 sdk::metrics::ResourceMetrics &data) -{ - collection_lock_.lock(); - if (metrics_to_collect_.size() + 1 <= max_collection_size_) - { - // We can not use initializer lists here due to broken variadic capture on GCC 4.8.5 - metrics_to_collect_.emplace_back(new sdk::metrics::ResourceMetrics(data)); - } + reader_->Collect([&result](sdk::metrics::ResourceMetrics &metric_data) { + auto prometheus_metric_data = PrometheusExporterUtils::TranslateToPrometheus(metric_data); + for (auto &data : prometheus_metric_data) + result.emplace_back(data); + return true; + }); 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_; + return result; } } // namespace metrics diff --git a/exporters/prometheus/src/exporter.cc b/exporters/prometheus/src/exporter.cc index f220ebd7d7..946d84bf91 100644 --- a/exporters/prometheus/src/exporter.cc +++ b/exporters/prometheus/src/exporter.cc @@ -14,24 +14,14 @@ namespace metrics * @param address: an address for an exposer that exposes * an HTTP endpoint for the exporter to connect to */ -PrometheusExporter::PrometheusExporter(const PrometheusExporterOptions &options) - : options_(options), is_shutdown_(false) +PrometheusExporter::PrometheusExporter(const PrometheusExporterOptions &options) : options_(options) { exposer_ = std::unique_ptr<::prometheus::Exposer>(new ::prometheus::Exposer{options_.url}); - collector_ = std::shared_ptr(new PrometheusCollector); + collector_ = std::shared_ptr(new PrometheusCollector(this)); exposer_->RegisterCollectable(collector_); } -/** - * PrometheusExporter constructor with no parameters - * Used for testing only - */ -PrometheusExporter::PrometheusExporter() : is_shutdown_(false) -{ - collector_ = std::unique_ptr(new PrometheusCollector(3)); -} - sdk::metrics::AggregationTemporality PrometheusExporter::GetAggregationTemporality( sdk::metrics::InstrumentType /* instrument_type */) const noexcept { @@ -39,71 +29,18 @@ sdk::metrics::AggregationTemporality PrometheusExporter::GetAggregationTemporali return sdk::metrics::AggregationTemporality::kCumulative; } -/** - * Exports a batch of Metric Records. - * @param records: a collection of records to export - * @return: returns a ReturnCode detailing a success, or type of failure - */ -sdk::common::ExportResult PrometheusExporter::Export( - const sdk::metrics::ResourceMetrics &data) noexcept -{ - if (is_shutdown_) - { - return sdk::common::ExportResult::kFailure; - } - else if (collector_->GetCollection().size() + data.scope_metric_data_.size() > - (size_t)collector_->GetMaxCollectionSize()) - { - return sdk::common::ExportResult::kFailureFull; - } - else if (data.scope_metric_data_.empty()) - { - return sdk::common::ExportResult::kFailureInvalidArgument; - } - else - { - collector_->AddMetricData(data); - return sdk::common::ExportResult::kSuccess; - } - return sdk::common::ExportResult::kSuccess; -} - -bool PrometheusExporter::ForceFlush(std::chrono::microseconds /* timeout */) noexcept +bool PrometheusExporter::OnForceFlush(std::chrono::microseconds /* timeout */) noexcept { return true; } -/** - * Shuts down the exporter and does cleanup. - * Since Prometheus is a pull based interface, - * we cannot serve data remaining in the intermediate - * collection to to client an HTTP request being sent, - * so we flush the data. - */ -bool PrometheusExporter::Shutdown(std::chrono::microseconds /* timeout */) noexcept +bool PrometheusExporter::OnShutDown(std::chrono::microseconds /* timeout */) noexcept { - is_shutdown_ = true; + exposer_->RemoveCollectable(collector_); return true; - - collector_->GetCollection().clear(); } -/** - * @return: returns a shared_ptr to - * the PrometheusCollector instance - */ -std::shared_ptr &PrometheusExporter::GetCollector() -{ - return collector_; -} - -/** - * @return: Gets the shutdown status of the exporter - */ -bool PrometheusExporter::IsShutdown() const -{ - return is_shutdown_; -} +void PrometheusExporter::OnInitialized() noexcept {} } // namespace metrics } // namespace exporter diff --git a/exporters/prometheus/src/exporter_utils.cc b/exporters/prometheus/src/exporter_utils.cc index 3fc71180e5..451c34ff45 100644 --- a/exporters/prometheus/src/exporter_utils.cc +++ b/exporters/prometheus/src/exporter_utils.cc @@ -28,102 +28,93 @@ namespace metrics * @return a collection of translated metrics that is acceptable by Prometheus */ std::vector PrometheusExporterUtils::TranslateToPrometheus( - const std::vector> &data) + const sdk::metrics::ResourceMetrics &data) { - if (data.empty()) - { - return {}; - } // initialize output vector std::vector output; - // iterate through the vector and set result data into it - for (const auto &r : data) + for (const auto &instrumentation_info : data.scope_metric_data_) { - for (const auto &instrumentation_info : r->scope_metric_data_) + for (const auto &metric_data : instrumentation_info.metric_data_) { - for (const auto &metric_data : instrumentation_info.metric_data_) + auto origin_name = metric_data.instrument_descriptor.name_; + auto sanitized = SanitizeNames(origin_name); + prometheus_client::MetricFamily metric_family; + metric_family.name = sanitized; + metric_family.help = metric_data.instrument_descriptor.description_; + auto time = metric_data.start_ts.time_since_epoch(); + for (const auto &point_data_attr : metric_data.point_data_attr_) { - auto origin_name = metric_data.instrument_descriptor.name_; - auto sanitized = SanitizeNames(origin_name); - prometheus_client::MetricFamily metric_family; - metric_family.name = sanitized; - metric_family.help = metric_data.instrument_descriptor.description_; - auto time = metric_data.start_ts.time_since_epoch(); - for (const auto &point_data_attr : metric_data.point_data_attr_) + auto kind = getAggregationType(point_data_attr.point_data); + bool is_monotonic = true; + if (kind == sdk::metrics::AggregationType::kSum) + { + is_monotonic = + nostd::get(point_data_attr.point_data).is_monotonic_; + } + const prometheus_client::MetricType type = TranslateType(kind, is_monotonic); + metric_family.type = type; + if (type == prometheus_client::MetricType::Histogram) // Histogram + { + auto histogram_point_data = + nostd::get(point_data_attr.point_data); + auto boundaries = histogram_point_data.boundaries_; + auto counts = histogram_point_data.counts_; + double sum = 0.0; + if (nostd::holds_alternative(histogram_point_data.sum_)) + { + sum = nostd::get(histogram_point_data.sum_); + } + else + { + sum = nostd::get(histogram_point_data.sum_); + } + SetData(std::vector{sum, (double)histogram_point_data.count_}, boundaries, counts, + point_data_attr.attributes, time, &metric_family); + } + else if (type == prometheus_client::MetricType::Gauge) { - auto kind = getAggregationType(point_data_attr.point_data); - bool is_monotonic = true; - if (kind == sdk::metrics::AggregationType::kSum) + if (nostd::holds_alternative( + point_data_attr.point_data)) { - is_monotonic = - nostd::get(point_data_attr.point_data).is_monotonic_; + auto last_value_point_data = + nostd::get(point_data_attr.point_data); + std::vector values{last_value_point_data.value_}; + SetData(values, point_data_attr.attributes, type, time, &metric_family); } - const prometheus_client::MetricType type = TranslateType(kind, is_monotonic); - metric_family.type = type; - if (type == prometheus_client::MetricType::Histogram) // Histogram + else if (nostd::holds_alternative(point_data_attr.point_data)) { - auto histogram_point_data = - nostd::get(point_data_attr.point_data); - auto boundaries = histogram_point_data.boundaries_; - auto counts = histogram_point_data.counts_; - double sum = 0.0; - if (nostd::holds_alternative(histogram_point_data.sum_)) - { - sum = nostd::get(histogram_point_data.sum_); - } - else - { - sum = nostd::get(histogram_point_data.sum_); - } - SetData(std::vector{sum, (double)histogram_point_data.count_}, boundaries, - counts, point_data_attr.attributes, time, &metric_family); + auto sum_point_data = + nostd::get(point_data_attr.point_data); + std::vector values{sum_point_data.value_}; + SetData(values, point_data_attr.attributes, type, time, &metric_family); } - else if (type == prometheus_client::MetricType::Gauge) + else + { + OTEL_INTERNAL_LOG_WARN( + "[Prometheus Exporter] TranslateToPrometheus - " + "invalid LastValuePointData type"); + } + } + else // Counter, Untyped + { + if (nostd::holds_alternative(point_data_attr.point_data)) { - if (nostd::holds_alternative( - point_data_attr.point_data)) - { - auto last_value_point_data = - nostd::get(point_data_attr.point_data); - std::vector values{last_value_point_data.value_}; - SetData(values, point_data_attr.attributes, type, time, &metric_family); - } - else if (nostd::holds_alternative( - point_data_attr.point_data)) - { - auto sum_point_data = - nostd::get(point_data_attr.point_data); - std::vector values{sum_point_data.value_}; - SetData(values, point_data_attr.attributes, type, time, &metric_family); - } - else - { - OTEL_INTERNAL_LOG_WARN( - "[Prometheus Exporter] TranslateToPrometheus - " - "invalid LastValuePointData type"); - } + auto sum_point_data = + nostd::get(point_data_attr.point_data); + std::vector values{sum_point_data.value_}; + SetData(values, point_data_attr.attributes, type, time, &metric_family); } - else // Counter, Untyped + else { - if (nostd::holds_alternative(point_data_attr.point_data)) - { - auto sum_point_data = - nostd::get(point_data_attr.point_data); - std::vector values{sum_point_data.value_}; - SetData(values, point_data_attr.attributes, type, time, &metric_family); - } - else - { - OTEL_INTERNAL_LOG_WARN( - "[Prometheus Exporter] TranslateToPrometheus - " - "invalid SumPointData type"); - } + OTEL_INTERNAL_LOG_WARN( + "[Prometheus Exporter] TranslateToPrometheus - " + "invalid SumPointData type"); } } - output.emplace_back(metric_family); } + output.emplace_back(metric_family); } } return output; diff --git a/exporters/prometheus/test/collector_test.cc b/exporters/prometheus/test/collector_test.cc index b6460761ed..d422c45c5b 100644 --- a/exporters/prometheus/test/collector_test.cc +++ b/exporters/prometheus/test/collector_test.cc @@ -1,352 +1,82 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 +#include "opentelemetry/exporters/prometheus/collector.h" +#include "opentelemetry/metrics/meter_provider.h" +#include "opentelemetry/version.h" +#include "prometheus_test_helper.h" + #include #include #include #include -#include "opentelemetry/exporters/prometheus/collector.h" -#include "opentelemetry/version.h" -#include "prometheus_test_helper.h" - using opentelemetry::exporter::metrics::PrometheusCollector; -namespace metric_api = opentelemetry::metrics; -namespace metric_sdk = opentelemetry::sdk::metrics; +using opentelemetry::sdk::metrics::ResourceMetrics; +namespace metric_api = opentelemetry::metrics; +namespace metric_sdk = opentelemetry::sdk::metrics; +namespace metric_exporter = opentelemetry::exporter::metrics; -OPENTELEMETRY_BEGIN_NAMESPACE - -// ==================== Test for addMetricsData() function ====================== - -/** - * AddMetricData() should be able to successfully add a collection - * of SumPointData. 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) +class MockMetricProducer : public opentelemetry::sdk::metrics::MetricProducer { - PrometheusCollector collector; - - // construct a collection of records with CounterAggregators and double - auto data = CreateSumPointData(); - - // add records to collection - collector.AddMetricData(data); - - // Collection size should be the same as the size - // of the records collection passed to addMetricData() - ASSERT_EQ(collector.GetCollection().size(), 1); - - // check values of records created vs records from metricsToCollect, - // accessed by getCollection() +public: + MockMetricProducer(std::chrono::microseconds sleep_ms = std::chrono::microseconds::zero()) + : sleep_ms_{sleep_ms}, data_sent_size_(0) + {} - auto &&collector_data = collector.GetCollection(); - ASSERT_EQ(collector_data.at(0)->resource_, data.resource_); - for (auto &collector_d : collector_data) + bool Collect(nostd::function_ref callback) noexcept override { - for (uint32_t instrument_idx = 0; instrument_idx < collector_d->scope_metric_data_.size(); - ++instrument_idx) - { - ASSERT_EQ(collector_d->scope_metric_data_.at(instrument_idx).scope_, - data.scope_metric_data_.at(instrument_idx).scope_); - ASSERT_EQ(collector_d->scope_metric_data_.at(instrument_idx).metric_data_, - data.scope_metric_data_.at(instrument_idx).metric_data_); - } + std::this_thread::sleep_for(sleep_ms_); + data_sent_size_++; + ResourceMetrics data = CreateSumPointData(); + callback(data); + return true; } -} - -/** - * AddMetricData() should be able to successfully add a collection - * of HistogramPointData. It checks that the cumulative - * sum of updates to the aggregator of a record before and after AddMetricData() - * is called are equal. - */ -TEST(PrometheusCollector, AddMetricDataWithHistogramSuccessfully) -{ - PrometheusCollector collector; - - // construct a collection of records with CounterAggregators and double - auto data = CreateHistogramPointData(); - - // add records to collection - collector.AddMetricData(data); - // Collection size should be the same as the size - // of the records collection passed to addMetricData() - ASSERT_EQ(collector.GetCollection().size(), 1); + size_t GetDataCount() { return data_sent_size_; } - // check values of records created vs records from metricsToCollect, - // accessed by getCollection() +private: + std::chrono::microseconds sleep_ms_; + size_t data_sent_size_; +}; - auto &&collector_data = collector.GetCollection(); - ASSERT_EQ(collector_data.at(0)->resource_, data.resource_); - for (auto &collector_d : collector_data) +class MockMetricReader : public opentelemetry::sdk::metrics::MetricReader +{ +public: + opentelemetry::sdk::metrics::AggregationTemporality GetAggregationTemporality( + opentelemetry::sdk::metrics::InstrumentType /* instrument_type */) const noexcept override { - for (uint32_t instrument_idx = 0; instrument_idx < collector_d->scope_metric_data_.size(); - ++instrument_idx) - { - ASSERT_EQ(collector_d->scope_metric_data_.at(instrument_idx).scope_, - data.scope_metric_data_.at(instrument_idx).scope_); - ASSERT_EQ(collector_d->scope_metric_data_.at(instrument_idx).metric_data_, - data.scope_metric_data_.at(instrument_idx).metric_data_); - } + // Prometheus exporter only support Cumulative + return opentelemetry::sdk::metrics::AggregationTemporality::kCumulative; } -} - -/** - * AddMetricData() should be able to successfully add a collection - * of LastValuePointData. It checks that the cumulative - * sum of updates to the aggregator of a record before and after AddMetricData() - * is called are equal. - */ -TEST(PrometheusCollector, AddMetricDataWithLastValueSuccessfully) -{ - PrometheusCollector collector; - // construct a collection of records with CounterAggregators and double - auto data = CreateLastValuePointData(); +private: + bool OnForceFlush(std::chrono::microseconds /* timeout */) noexcept override { return true; } - // add records to collection - collector.AddMetricData(data); - - // Collection size should be the same as the size - // of the records collection passed to addMetricData() - ASSERT_EQ(collector.GetCollection().size(), 1); + bool OnShutDown(std::chrono::microseconds /* timeout */) noexcept override { return true; } - // check values of records created vs records from metricsToCollect, - // accessed by getCollection() + void OnInitialized() noexcept override {} +}; - auto &&collector_data = collector.GetCollection(); - ASSERT_EQ(collector_data.at(0)->resource_, data.resource_); - for (auto &collector_d : collector_data) - { - for (uint32_t instrument_idx = 0; instrument_idx < collector_d->scope_metric_data_.size(); - ++instrument_idx) - { - ASSERT_EQ(collector_d->scope_metric_data_.at(instrument_idx).scope_, - data.scope_metric_data_.at(instrument_idx).scope_); - ASSERT_EQ(collector_d->scope_metric_data_.at(instrument_idx).metric_data_, - data.scope_metric_data_.at(instrument_idx).metric_data_); - } - } -} +// ==================== Test for addMetricsData() function ====================== /** * AddMetricData() should be able to successfully add a collection - * of DropPointData. It checks that the cumulative + * of SumPointData. It checks that the cumulative * sum of updates to the aggregator of a record before and after AddMetricData() * is called are equal. */ -TEST(PrometheusCollector, AddMetricDataWithDropSuccessfully) +TEST(PrometheusCollector, BasicTests) { - PrometheusCollector collector; - - // construct a collection of records with CounterAggregators and double - auto data = CreateDropPointData(); - - // add records to collection - collector.AddMetricData(data); + MockMetricReader *reader = new MockMetricReader(); + MockMetricProducer *producer = new MockMetricProducer(); + reader->SetMetricProducer(producer); + PrometheusCollector collector(reader); + auto data = collector.Collect(); // Collection size should be the same as the size - // of the records collection passed to addMetricData() - ASSERT_EQ(collector.GetCollection().size(), 1); - - // check values of records created vs records from metricsToCollect, - // accessed by getCollection() - - auto &&collector_data = collector.GetCollection(); - ASSERT_EQ(collector_data.at(0)->resource_, data.resource_); - for (auto &collector_d : collector_data) - { - for (uint32_t instrument_idx = 0; instrument_idx < collector_d->scope_metric_data_.size(); - ++instrument_idx) - { - ASSERT_EQ(collector_d->scope_metric_data_.at(instrument_idx).scope_, - data.scope_metric_data_.at(instrument_idx).scope_); - ASSERT_EQ(collector_d->scope_metric_data_.at(instrument_idx).metric_data_, - data.scope_metric_data_.at(instrument_idx).metric_data_); - } - } -} - -TEST(PrometheusCollector, AddMetricDataDoesNotAddWithInsufficentSpace) -{ - PrometheusCollector collector; - - int max_collection_size = collector.GetMaxCollectionSize(); - - for (int count = 1; count <= max_collection_size; ++count) - { - collector.AddMetricData(CreateSumPointData()); - ASSERT_EQ(count, collector.GetCollection().size()); - } - - // Try adding one more collection of records again to - // metricsToCollect. - collector.AddMetricData(CreateSumPointData()); - - // Check that the number of records in metricsToCollect - // has not changed. - ASSERT_EQ(collector.GetCollection().size(), max_collection_size); + // of the records collection produced by MetricProducer. + ASSERT_EQ(data.size(), 1); + delete reader; + delete producer; } - -// ==================== 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 - collector.AddMetricData(CreateSumPointData()); - collector.AddMetricData(CreateSumPointData()); - - // the collection should not be empty now - ASSERT_EQ(collector.GetCollection().size(), 2); - - // 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; - - collector.AddMetricData(CreateSumPointData()); - - // the collection should not be empty now - ASSERT_EQ(collector.GetCollection().size(), 1); - auto collected = collector.Collect(); - - ASSERT_EQ(collected.size(), 1); - - auto metric_family = collected[0]; - - // Collect function really collects a vector of MetricFamily - ASSERT_EQ(metric_family.name, "library_name"); - ASSERT_EQ(metric_family.help, "description"); - ASSERT_EQ(metric_family.type, prometheus_client::MetricType::Counter); - ASSERT_EQ(metric_family.metric.size(), 2); - ASSERT_DOUBLE_EQ(metric_family.metric[0].counter.value, 10); -} - -/** - * Concurrency Test 1: After adding data concurrently, the intermediate collection should - * contain all data from all threads. - */ -TEST(PrometheusCollector, ConcurrencyAddingRecords) -{ - PrometheusCollector collector; - - std::thread first(&PrometheusCollector::AddMetricData, std::ref(collector), CreateSumPointData()); - std::thread second(&PrometheusCollector::AddMetricData, std::ref(collector), - CreateSumPointData()); - - first.join(); - second.join(); - - ASSERT_EQ(collector.GetCollection().size(), 2); -} - -/** - * 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; - - std::thread first(&PrometheusCollector::AddMetricData, std::ref(collector), CreateSumPointData()); - std::thread second(&PrometheusCollector::AddMetricData, std::ref(collector), - CreateSumPointData()); - 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(), 2); -} - -/** - * 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 - - std::thread first(&PrometheusCollector::AddMetricData, std::ref(collector), CreateSumPointData()); - std::thread second(&PrometheusCollector::AddMetricData, std::ref(collector), - CreateSumPointData()); - 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(), 2); -} - -/** - * 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; - - // concurrently adding - std::thread first(&PrometheusCollector::AddMetricData, std::ref(collector), CreateSumPointData()); - std::thread second(&PrometheusCollector::AddMetricData, std::ref(collector), - CreateSumPointData()); - 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(), 2); -} - -OPENTELEMETRY_END_NAMESPACE diff --git a/exporters/prometheus/test/exporter_test.cc b/exporters/prometheus/test/exporter_test.cc index c115dbfa87..3bda4c3d7b 100644 --- a/exporters/prometheus/test/exporter_test.cc +++ b/exporters/prometheus/test/exporter_test.cc @@ -5,6 +5,7 @@ #include "opentelemetry/exporters/prometheus/collector.h" #include "opentelemetry/exporters/prometheus/exporter.h" +#include "opentelemetry/sdk/metrics/instruments.h" #include "opentelemetry/version.h" #include "prometheus_test_helper.h" @@ -14,36 +15,24 @@ * an exposer as an argument, and instead takes no arguments; this * private constructor is only to be used here for testing */ -OPENTELEMETRY_BEGIN_NAMESPACE -namespace exporter -{ -namespace metrics -{ -class PrometheusExporterTest // : public ::testing::Test -{ -public: - PrometheusExporter GetExporter() { return PrometheusExporter(); } -}; -} // namespace metrics -} // namespace exporter -OPENTELEMETRY_END_NAMESPACE using opentelemetry::exporter::metrics::PrometheusCollector; using opentelemetry::exporter::metrics::PrometheusExporter; -using opentelemetry::exporter::metrics::PrometheusExporterTest; -using opentelemetry::sdk::common::ExportResult; - +using opentelemetry::exporter::metrics::PrometheusExporterOptions; +using opentelemetry::sdk::metrics::AggregationTemporality; +using opentelemetry::sdk::metrics::InstrumentType; /** * When a PrometheusExporter is initialized, * isShutdown should be false. */ TEST(PrometheusExporter, InitializeConstructorIsNotShutdown) { - PrometheusExporterTest p; - PrometheusExporter exporter = p.GetExporter(); - + PrometheusExporterOptions options; + options.url = "localhost:8081"; + PrometheusExporter exporter(options); // // Asserts that the exporter is not shutdown. - ASSERT_TRUE(!exporter.IsShutdown()); + // ASSERT_TRUE(!exporter.IsShutdown()); + exporter.Shutdown(); } /** @@ -51,8 +40,8 @@ TEST(PrometheusExporter, InitializeConstructorIsNotShutdown) */ TEST(PrometheusExporter, ShutdownSetsIsShutdownToTrue) { - PrometheusExporterTest p; - PrometheusExporter exporter = p.GetExporter(); + PrometheusExporterOptions options; + PrometheusExporter exporter(options); // exporter shuold not be shutdown by default ASSERT_TRUE(!exporter.IsShutdown()); @@ -68,93 +57,23 @@ TEST(PrometheusExporter, ShutdownSetsIsShutdownToTrue) } /** - * The Export() function should return kSuccess = 0 - * when data is exported successfully. - */ -TEST(PrometheusExporter, ExportSuccessfully) -{ - PrometheusExporterTest p; - PrometheusExporter exporter = p.GetExporter(); - - auto res = exporter.Export(CreateSumPointData()); - - // result should be kSuccess = 0 - ExportResult code = ExportResult::kSuccess; - ASSERT_EQ(res, code); -} - -/** - * If the exporter is shutdown, it cannot process - * any more export requests and returns kFailure = 1. - */ -TEST(PrometheusExporter, ExporterIsShutdown) -{ - PrometheusExporterTest p; - PrometheusExporter exporter = p.GetExporter(); - - exporter.Shutdown(); - - // send export request after shutdown - auto res = exporter.Export(CreateSumPointData()); - - // result code should be kFailure = 1 - ExportResult code = ExportResult::kFailure; - ASSERT_EQ(res, code); -} - -/** - * The Export() function should return - * kFailureFull = 2 when the collection is full, - * or when the collection is not full but does not have enough - * space to hold the batch data. - */ -TEST(PrometheusExporter, CollectionNotEnoughSpace) -{ - PrometheusExporterTest p; - PrometheusExporter exporter = p.GetExporter(); - - // prepare two collections of records to export, - // one close to max size and another one that, when added - // to the first, will exceed the size of the collection - - int max_collection_size = exporter.GetCollector()->GetMaxCollectionSize(); - - // send export request to fill the - // collection in the collector - ExportResult code = ExportResult::kSuccess; - for (int count = 1; count <= max_collection_size; ++count) - { - auto res = exporter.Export(CreateSumPointData()); - ASSERT_EQ(res, code); - } - - // send export request that does not complete - // due to not enough space in the collection - auto res = exporter.Export(CreateSumPointData()); - - // the result code should be kFailureFull = 2 - code = ExportResult::kFailureFull; - ASSERT_EQ(res, code); -} - -/** - * The Export() function should return - * kFailureInvalidArgument = 3 when an empty collection - * of records is passed to the Export() function. + * The Aggregation temporality should be correctly set */ -TEST(PrometheusExporter, InvalidArgumentWhenPassedEmptyRecordCollection) +TEST(PrometheusExporter, CheckAggregationTemporality) { - PrometheusExporterTest p; - PrometheusExporter exporter = p.GetExporter(); - - // Initializes an empty colelction of records - metric_sdk::ResourceMetrics data; - - // send export request to fill the - // collection in the collector - auto res = exporter.Export(data); - - // the result code should be kFailureInvalidArgument = 3 - ExportResult code = ExportResult::kFailureInvalidArgument; - ASSERT_EQ(res, code); + PrometheusExporterOptions options; + PrometheusExporter exporter(options); + + ASSERT_EQ(exporter.GetAggregationTemporality(InstrumentType::kCounter), + AggregationTemporality::kCumulative); + ASSERT_EQ(exporter.GetAggregationTemporality(InstrumentType::kHistogram), + AggregationTemporality::kCumulative); + ASSERT_EQ(exporter.GetAggregationTemporality(InstrumentType::kObservableCounter), + AggregationTemporality::kCumulative); + ASSERT_EQ(exporter.GetAggregationTemporality(InstrumentType::kObservableGauge), + AggregationTemporality::kCumulative); + ASSERT_EQ(exporter.GetAggregationTemporality(InstrumentType::kObservableUpDownCounter), + AggregationTemporality::kCumulative); + ASSERT_EQ(exporter.GetAggregationTemporality(InstrumentType::kUpDownCounter), + AggregationTemporality::kCumulative); } diff --git a/exporters/prometheus/test/exporter_utils_test.cc b/exporters/prometheus/test/exporter_utils_test.cc index 8a14c2effa..467525ff7d 100644 --- a/exporters/prometheus/test/exporter_utils_test.cc +++ b/exporters/prometheus/test/exporter_utils_test.cc @@ -83,67 +83,44 @@ void assert_histogram(prometheus_client::MetricFamily &metric, TEST(PrometheusExporterUtils, TranslateToPrometheusEmptyInputReturnsEmptyCollection) { - auto translated = PrometheusExporterUtils::TranslateToPrometheus( - std::vector>{}); + metric_sdk::ResourceMetrics metrics_data = {}; + auto translated = PrometheusExporterUtils::TranslateToPrometheus(metrics_data); ASSERT_EQ(translated.size(), 0); } TEST(PrometheusExporterUtils, TranslateToPrometheusIntegerCounter) { - std::vector> collection; - collection.emplace_back(new metric_sdk::ResourceMetrics{CreateSumPointData()}); + metric_sdk::ResourceMetrics metrics_data = CreateSumPointData(); - auto translated = PrometheusExporterUtils::TranslateToPrometheus(collection); - ASSERT_EQ(translated.size(), collection.size()); + auto translated = PrometheusExporterUtils::TranslateToPrometheus(metrics_data); + ASSERT_EQ(translated.size(), 1); auto metric1 = translated[0]; std::vector vals = {10}; assert_basic(metric1, "library_name", "description", prometheus_client::MetricType::Counter, 1, vals); - - collection.emplace_back(new metric_sdk::ResourceMetrics{CreateSumPointData()}); - - translated = PrometheusExporterUtils::TranslateToPrometheus(collection); - ASSERT_EQ(translated.size(), collection.size()); - - auto metric2 = translated[1]; - assert_basic(metric2, "library_name", "description", prometheus_client::MetricType::Counter, 1, - vals); } TEST(PrometheusExporterUtils, TranslateToPrometheusIntegerLastValue) { - std::vector> collection; - - collection.emplace_back(new metric_sdk::ResourceMetrics{CreateLastValuePointData()}); + metric_sdk::ResourceMetrics metrics_data = CreateLastValuePointData(); - auto translated = PrometheusExporterUtils::TranslateToPrometheus(collection); - ASSERT_EQ(translated.size(), collection.size()); + auto translated = PrometheusExporterUtils::TranslateToPrometheus(metrics_data); + ASSERT_EQ(translated.size(), 1); auto metric1 = translated[0]; std::vector vals = {10}; assert_basic(metric1, "library_name", "description", prometheus_client::MetricType::Gauge, 1, vals); - - collection.emplace_back(new metric_sdk::ResourceMetrics{CreateLastValuePointData()}); - - translated = PrometheusExporterUtils::TranslateToPrometheus(collection); - ASSERT_EQ(translated.size(), collection.size()); - - auto metric2 = translated[1]; - assert_basic(metric2, "library_name", "description", prometheus_client::MetricType::Gauge, 1, - vals); } TEST(PrometheusExporterUtils, TranslateToPrometheusHistogramNormal) { - std::vector> collection; - - collection.emplace_back(new metric_sdk::ResourceMetrics{CreateHistogramPointData()}); + metric_sdk::ResourceMetrics metrics_data = CreateHistogramPointData(); - auto translated = PrometheusExporterUtils::TranslateToPrometheus(collection); - ASSERT_EQ(translated.size(), collection.size()); + auto translated = PrometheusExporterUtils::TranslateToPrometheus(metrics_data); + ASSERT_EQ(translated.size(), 1); auto metric = translated[0]; std::vector vals = {3, 900.5, 4}; diff --git a/sdk/include/opentelemetry/sdk/metrics/metric_reader.h b/sdk/include/opentelemetry/sdk/metrics/metric_reader.h index d7d6bcbd39..86101e39a0 100644 --- a/sdk/include/opentelemetry/sdk/metrics/metric_reader.h +++ b/sdk/include/opentelemetry/sdk/metrics/metric_reader.h @@ -45,7 +45,7 @@ class MetricReader InstrumentType instrument_type) const noexcept = 0; /** - * Shutdown the meter reader. + * Shutdown the metric reader. */ bool Shutdown(std::chrono::microseconds timeout = std::chrono::microseconds::max()) noexcept; @@ -54,6 +54,11 @@ class MetricReader */ bool ForceFlush(std::chrono::microseconds timeout = std::chrono::microseconds::max()) noexcept; + /** + * Return the status of Metric reader. + */ + bool IsShutdown() const noexcept; + virtual ~MetricReader() = default; private: @@ -64,8 +69,6 @@ class MetricReader virtual void OnInitialized() noexcept {} protected: - bool IsShutdown() const noexcept; - private: MetricProducer *metric_producer_; mutable opentelemetry::common::SpinLockMutex lock_; From 2bae1c68da87dd439a34a3f080da1320959e6e09 Mon Sep 17 00:00:00 2001 From: Severin Neumann Date: Mon, 6 Feb 2023 20:19:04 +0100 Subject: [PATCH 4/5] Add alpine packages to INSTALL.md (#1957) --- INSTALL.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/INSTALL.md b/INSTALL.md index cf5138dcf7..f4a0615350 100644 --- a/INSTALL.md +++ b/INSTALL.md @@ -320,6 +320,10 @@ with `vcpkg install opentelemetry-cpp` and follow the then displayed descriptions. Please see the vcpkg project for any issues regarding the packaging. +If you are using [alpine linux](https://www.alpinelinux.org/) you can install +the [opentelemetry-cpp packages](https://pkgs.alpinelinux.org/packages?name=opentelemetry-cpp-*) +with `apk add -X http://dl-cdn.alpinelinux.org/alpine/edge/testing opentelemetry-cpp-dev`. + Please note, these packages are not officially provided and maintained by OpenTelemetry C++ project, and are just listed here to consolidate all such efforts for ease of developers. From 97f0751ba9a7f29e9e541447ccf05b8a581d7a53 Mon Sep 17 00:00:00 2001 From: Tom Tan Date: Mon, 6 Feb 2023 12:51:21 -0800 Subject: [PATCH 5/5] Update copyright headers in zipkin and Jaeger exporters for consistency (#1959) --- exporters/jaeger/CMakeLists.txt | 3 +++ exporters/zipkin/CMakeLists.txt | 15 ++------------- 2 files changed, 5 insertions(+), 13 deletions(-) diff --git a/exporters/jaeger/CMakeLists.txt b/exporters/jaeger/CMakeLists.txt index 9641300124..d40ba58527 100644 --- a/exporters/jaeger/CMakeLists.txt +++ b/exporters/jaeger/CMakeLists.txt @@ -1,3 +1,6 @@ +# Copyright The OpenTelemetry Authors +# SPDX-License-Identifier: Apache-2.0 + include_directories(thrift-gen) find_package(Thrift REQUIRED) diff --git a/exporters/zipkin/CMakeLists.txt b/exporters/zipkin/CMakeLists.txt index ce093f3e55..ba74ba17b6 100644 --- a/exporters/zipkin/CMakeLists.txt +++ b/exporters/zipkin/CMakeLists.txt @@ -1,16 +1,5 @@ -# Copyright 2021, 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. +# Copyright The OpenTelemetry Authors +# SPDX-License-Identifier: Apache-2.0 include_directories(include) add_definitions(-DWITH_CURL)