From 928a87ce57d83e3f54b087ba85244eda635d0d09 Mon Sep 17 00:00:00 2001 From: Shrey Amin <43042747+samin36@users.noreply.github.com> Date: Mon, 8 Jan 2024 14:07:56 -0500 Subject: [PATCH 1/3] [EXPORTER] Set is_monotonic flag for Observable Counters (#2478) --- CHANGELOG.md | 2 + exporters/otlp/src/otlp_metric_utils.cc | 5 +- .../test/otlp_metrics_serialization_test.cc | 91 +++++++++++++++++++ 3 files changed, 96 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 11ba426024..e8379620fb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,8 @@ Increment the: [#2449](https://github.com/open-telemetry/opentelemetry-cpp/pull/2449) * [BUILD] Introduce CXX 20 CI pipeline for MSVC/Windows [#2450](https://github.com/open-telemetry/opentelemetry-cpp/pull/2450) +* [EXPORTER] Set `is_monotonic` flag for Observable Counters + [#2478](https://github.com/open-telemetry/opentelemetry-cpp/pull/2478) Important changes: diff --git a/exporters/otlp/src/otlp_metric_utils.cc b/exporters/otlp/src/otlp_metric_utils.cc index ed1620a56c..53e84e7966 100644 --- a/exporters/otlp/src/otlp_metric_utils.cc +++ b/exporters/otlp/src/otlp_metric_utils.cc @@ -54,8 +54,9 @@ void OtlpMetricUtils::ConvertSumMetric(const metric_sdk::MetricData &metric_data { sum->set_aggregation_temporality( GetProtoAggregationTemporality(metric_data.aggregation_temporality)); - sum->set_is_monotonic(metric_data.instrument_descriptor.type_ == - metric_sdk::InstrumentType::kCounter); + sum->set_is_monotonic( + (metric_data.instrument_descriptor.type_ == metric_sdk::InstrumentType::kCounter) || + (metric_data.instrument_descriptor.type_ == metric_sdk::InstrumentType::kObservableCounter)); auto start_ts = metric_data.start_ts.time_since_epoch().count(); auto ts = metric_data.end_ts.time_since_epoch().count(); for (auto &point_data_with_attributes : metric_data.point_data_attr_) diff --git a/exporters/otlp/test/otlp_metrics_serialization_test.cc b/exporters/otlp/test/otlp_metrics_serialization_test.cc index ac0f717b25..b945cb28fe 100644 --- a/exporters/otlp/test/otlp_metrics_serialization_test.cc +++ b/exporters/otlp/test/otlp_metrics_serialization_test.cc @@ -130,6 +130,66 @@ static metrics_sdk::MetricData CreateObservableGaugeAggregationData() return data; } +static metrics_sdk::MetricData CreateObservableCounterAggregationData() +{ + metrics_sdk::MetricData data; + data.start_ts = opentelemetry::common::SystemTimestamp(std::chrono::system_clock::now()); + metrics_sdk::InstrumentDescriptor inst_desc = { + "ObservableCounter", "test description", "test unit", + metrics_sdk::InstrumentType::kObservableCounter, metrics_sdk::InstrumentValueType::kDouble}; + metrics_sdk::SumPointData s_data_1, s_data_2; + s_data_1.value_ = 1.23; + s_data_2.value_ = 4.56; + + data.aggregation_temporality = metrics_sdk::AggregationTemporality::kCumulative; + data.end_ts = opentelemetry::common::SystemTimestamp(std::chrono::system_clock::now()); + data.instrument_descriptor = inst_desc; + metrics_sdk::PointDataAttributes point_data_attr_1, point_data_attr_2; + point_data_attr_1.attributes = {{"key1", "value1"}}; + point_data_attr_1.point_data = s_data_1; + + point_data_attr_2.attributes = {{"key2", "value2"}}; + point_data_attr_2.point_data = s_data_2; + std::vector point_data_attr; + point_data_attr.push_back(point_data_attr_1); + point_data_attr.push_back(point_data_attr_2); + data.point_data_attr_ = std::move(point_data_attr); + return data; +} + +static metrics_sdk::MetricData CreateObservableUpDownCounterAggregationData() +{ + metrics_sdk::MetricData data; + data.start_ts = opentelemetry::common::SystemTimestamp(std::chrono::system_clock::now()); + metrics_sdk::InstrumentDescriptor inst_desc = { + "ObservableUpDownCounter", "test description", "test unit", + metrics_sdk::InstrumentType::kObservableUpDownCounter, + metrics_sdk::InstrumentValueType::kDouble}; + metrics_sdk::SumPointData s_data_1, s_data_2, s_data_3; + s_data_1.value_ = 1.23; + s_data_2.value_ = 4.56; + s_data_3.value_ = 2.34; + + data.aggregation_temporality = metrics_sdk::AggregationTemporality::kCumulative; + data.end_ts = opentelemetry::common::SystemTimestamp(std::chrono::system_clock::now()); + data.instrument_descriptor = inst_desc; + metrics_sdk::PointDataAttributes point_data_attr_1, point_data_attr_2, point_data_attr_3; + point_data_attr_1.attributes = {{"key1", "value1"}}; + point_data_attr_1.point_data = s_data_1; + + point_data_attr_2.attributes = {{"key2", "value2"}}; + point_data_attr_2.point_data = s_data_2; + + point_data_attr_3.attributes = {{"key3", "value3"}}; + point_data_attr_3.point_data = s_data_3; + std::vector point_data_attr; + point_data_attr.push_back(point_data_attr_1); + point_data_attr.push_back(point_data_attr_2); + point_data_attr.push_back(point_data_attr_3); + data.point_data_attr_ = std::move(point_data_attr); + return data; +} + TEST(OtlpMetricSerializationTest, Counter) { metrics_sdk::MetricData data = CreateSumAggregationData(); @@ -191,6 +251,37 @@ TEST(OtlpMetricSerializationTest, ObservableGauge) EXPECT_EQ(1, 1); } +TEST(OtlpMetricSerializationTest, ObservableCounter) +{ + metrics_sdk::MetricData data = CreateObservableCounterAggregationData(); + opentelemetry::proto::metrics::v1::Sum sum; + otlp_exporter::OtlpMetricUtils::ConvertSumMetric(data, &sum); + EXPECT_EQ(sum.aggregation_temporality(), + proto::metrics::v1::AggregationTemporality::AGGREGATION_TEMPORALITY_CUMULATIVE); + EXPECT_EQ(sum.is_monotonic(), true); + EXPECT_EQ(sum.data_points_size(), 2); + EXPECT_EQ(sum.data_points(0).as_double(), 1.23); + EXPECT_EQ(sum.data_points(1).as_double(), 4.56); + + EXPECT_EQ(1, 1); +} + +TEST(OtlpMetricSerializationTest, ObservableUpDownCounter) +{ + metrics_sdk::MetricData data = CreateObservableUpDownCounterAggregationData(); + opentelemetry::proto::metrics::v1::Sum sum; + otlp_exporter::OtlpMetricUtils::ConvertSumMetric(data, &sum); + EXPECT_EQ(sum.aggregation_temporality(), + proto::metrics::v1::AggregationTemporality::AGGREGATION_TEMPORALITY_CUMULATIVE); + EXPECT_EQ(sum.is_monotonic(), false); + EXPECT_EQ(sum.data_points_size(), 3); + EXPECT_EQ(sum.data_points(0).as_double(), 1.23); + EXPECT_EQ(sum.data_points(1).as_double(), 4.56); + EXPECT_EQ(sum.data_points(2).as_double(), 2.34); + + EXPECT_EQ(1, 1); +} + } // namespace otlp } // namespace exporter OPENTELEMETRY_END_NAMESPACE From f084fbce22abb4fee503da15927cd0f0897429cc Mon Sep 17 00:00:00 2001 From: Tim Wojtulewicz Date: Mon, 8 Jan 2024 13:02:18 -0700 Subject: [PATCH 2/3] [EXPORTER] Rename populate_otel_scope to without_otel_scope (#2479) --- .../opentelemetry/exporters/prometheus/collector.h | 4 ++-- .../exporters/prometheus/exporter_options.h | 2 +- .../opentelemetry/exporters/prometheus/exporter_utils.h | 4 ++-- exporters/prometheus/src/collector.cc | 6 +++--- exporters/prometheus/src/exporter.cc | 2 +- exporters/prometheus/src/exporter_options.cc | 9 ++++----- exporters/prometheus/src/exporter_utils.cc | 6 +++--- exporters/prometheus/test/collector_test.cc | 2 +- 8 files changed, 17 insertions(+), 18 deletions(-) diff --git a/exporters/prometheus/include/opentelemetry/exporters/prometheus/collector.h b/exporters/prometheus/include/opentelemetry/exporters/prometheus/collector.h index ee257a5547..cb94fe7654 100644 --- a/exporters/prometheus/include/opentelemetry/exporters/prometheus/collector.h +++ b/exporters/prometheus/include/opentelemetry/exporters/prometheus/collector.h @@ -33,7 +33,7 @@ class PrometheusCollector : public prometheus_client::Collectable */ explicit PrometheusCollector(sdk::metrics::MetricReader *reader, bool populate_target_info, - bool populate_otel_scope); + bool without_otel_scope); /** * Collects all metrics data from metricsToCollect collection. @@ -45,7 +45,7 @@ class PrometheusCollector : public prometheus_client::Collectable private: sdk::metrics::MetricReader *reader_; bool populate_target_info_; - bool populate_otel_scope_; + bool without_otel_scope_; /* * Lock when operating the metricsToCollect collection diff --git a/exporters/prometheus/include/opentelemetry/exporters/prometheus/exporter_options.h b/exporters/prometheus/include/opentelemetry/exporters/prometheus/exporter_options.h index 61e7fb3de0..5d8b4932fc 100644 --- a/exporters/prometheus/include/opentelemetry/exporters/prometheus/exporter_options.h +++ b/exporters/prometheus/include/opentelemetry/exporters/prometheus/exporter_options.h @@ -27,7 +27,7 @@ struct PrometheusExporterOptions bool populate_target_info = true; // Populating otel_scope_name/otel_scope_labels attributes - bool populate_otel_scope = true; + bool without_otel_scope = false; }; } // namespace metrics diff --git a/exporters/prometheus/include/opentelemetry/exporters/prometheus/exporter_utils.h b/exporters/prometheus/include/opentelemetry/exporters/prometheus/exporter_utils.h index bab75b3e10..ccf3d03ff3 100644 --- a/exporters/prometheus/include/opentelemetry/exporters/prometheus/exporter_utils.h +++ b/exporters/prometheus/include/opentelemetry/exporters/prometheus/exporter_utils.h @@ -28,14 +28,14 @@ class PrometheusExporterUtils * * @param records a collection of metrics in OpenTelemetry * @param populate_target_info whether to populate target_info - * @param populate_otel_scope whether to populate otel_scope_name and otel_scope_version + * @param without_otel_scope whether to populate otel_scope_name and otel_scope_version * attributes * @return a collection of translated metrics that is acceptable by Prometheus */ static std::vector<::prometheus::MetricFamily> TranslateToPrometheus( const sdk::metrics::ResourceMetrics &data, bool populate_target_info = true, - bool populate_otel_scope = true); + bool without_otel_scope = false); private: /** diff --git a/exporters/prometheus/src/collector.cc b/exporters/prometheus/src/collector.cc index 9e91744624..53d880ee01 100644 --- a/exporters/prometheus/src/collector.cc +++ b/exporters/prometheus/src/collector.cc @@ -19,10 +19,10 @@ namespace metrics */ PrometheusCollector::PrometheusCollector(sdk::metrics::MetricReader *reader, bool populate_target_info, - bool populate_otel_scope) + bool without_otel_scope) : reader_(reader), populate_target_info_(populate_target_info), - populate_otel_scope_(populate_otel_scope) + without_otel_scope_(without_otel_scope) {} /** @@ -45,7 +45,7 @@ std::vector PrometheusCollector::Collect() cons reader_->Collect([&result, this](sdk::metrics::ResourceMetrics &metric_data) { auto prometheus_metric_data = PrometheusExporterUtils::TranslateToPrometheus( - metric_data, this->populate_target_info_, this->populate_otel_scope_); + metric_data, this->populate_target_info_, this->without_otel_scope_); for (auto &data : prometheus_metric_data) result.emplace_back(data); return true; diff --git a/exporters/prometheus/src/exporter.cc b/exporters/prometheus/src/exporter.cc index 07f1494000..eeab82d6bb 100644 --- a/exporters/prometheus/src/exporter.cc +++ b/exporters/prometheus/src/exporter.cc @@ -31,7 +31,7 @@ PrometheusExporter::PrometheusExporter(const PrometheusExporterOptions &options) return; } collector_ = std::shared_ptr( - new PrometheusCollector(this, options_.populate_target_info, options_.populate_otel_scope)); + new PrometheusCollector(this, options_.populate_target_info, options_.without_otel_scope)); exposer_->RegisterCollectable(collector_); } diff --git a/exporters/prometheus/src/exporter_options.cc b/exporters/prometheus/src/exporter_options.cc index 0a7814c166..f2c49f7a57 100644 --- a/exporters/prometheus/src/exporter_options.cc +++ b/exporters/prometheus/src/exporter_options.cc @@ -25,14 +25,13 @@ inline const std::string GetPrometheusDefaultHttpEndpoint() return exists ? endpoint : kPrometheusEndpointDefault; } -inline bool GetPrometheusPopulateOtelScope() +inline bool GetPrometheusWithoutOtelScope() { - constexpr char kPrometheusPopulateOtelScope[] = - "OTEL_CPP_PROMETHEUS_EXPORTER_POPULATE_OTEL_SCOPE"; + constexpr char kPrometheusWithoutOtelScope[] = "OTEL_CPP_PROMETHEUS_EXPORTER_WITHOUT_OTEL_SCOPE"; bool setting; auto exists = - opentelemetry::sdk::common::GetBoolEnvironmentVariable(kPrometheusPopulateOtelScope, setting); + opentelemetry::sdk::common::GetBoolEnvironmentVariable(kPrometheusWithoutOtelScope, setting); return exists ? setting : true; } @@ -52,7 +51,7 @@ inline bool GetPrometheusPopulateTargetInfo() PrometheusExporterOptions::PrometheusExporterOptions() : url(GetPrometheusDefaultHttpEndpoint()), populate_target_info(GetPrometheusPopulateTargetInfo()), - populate_otel_scope(GetPrometheusPopulateOtelScope()) + without_otel_scope(GetPrometheusWithoutOtelScope()) {} } // namespace metrics diff --git a/exporters/prometheus/src/exporter_utils.cc b/exporters/prometheus/src/exporter_utils.cc index 19dc1e88d6..1e0d018011 100644 --- a/exporters/prometheus/src/exporter_utils.cc +++ b/exporters/prometheus/src/exporter_utils.cc @@ -108,7 +108,7 @@ std::string SanitizeLabel(std::string label_key) std::vector PrometheusExporterUtils::TranslateToPrometheus( const sdk::metrics::ResourceMetrics &data, bool populate_target_info, - bool populate_otel_scope) + bool without_otel_scope) { // initialize output vector @@ -129,7 +129,7 @@ std::vector PrometheusExporterUtils::TranslateT { SetTarget(data, data.scope_metric_data_.begin()->metric_data_.begin()->end_ts.time_since_epoch(), - populate_otel_scope ? (*data.scope_metric_data_.begin()).scope_ : nullptr, &output); + without_otel_scope ? nullptr : (*data.scope_metric_data_.begin()).scope_, &output); } for (const auto &instrumentation_info : data.scope_metric_data_) @@ -153,7 +153,7 @@ std::vector PrometheusExporterUtils::TranslateT metric_data.instrument_descriptor.unit_, type); metric_family.type = type; const opentelemetry::sdk::instrumentationscope::InstrumentationScope *scope = - populate_otel_scope ? instrumentation_info.scope_ : nullptr; + without_otel_scope ? nullptr : instrumentation_info.scope_; for (const auto &point_data_attr : metric_data.point_data_attr_) { diff --git a/exporters/prometheus/test/collector_test.cc b/exporters/prometheus/test/collector_test.cc index 4708b80aaa..63f840bc9a 100644 --- a/exporters/prometheus/test/collector_test.cc +++ b/exporters/prometheus/test/collector_test.cc @@ -73,7 +73,7 @@ TEST(PrometheusCollector, BasicTests) MockMetricReader *reader = new MockMetricReader(); MockMetricProducer *producer = new MockMetricProducer(); reader->SetMetricProducer(producer); - PrometheusCollector collector(reader, true, true); + PrometheusCollector collector(reader, true, false); auto data = collector.Collect(); // Collection size should be the same as the size From c4f39f2be8109fd1a3e047677c09cf47954b92db Mon Sep 17 00:00:00 2001 From: WenTao Ou Date: Tue, 9 Jan 2024 04:27:46 +0800 Subject: [PATCH 3/3] [BUILD] Skip patch alias target (#2457) --- cmake/patch-imported-config.cmake | 7 +++++-- cmake/tools.cmake | 16 +++++++++++++++- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/cmake/patch-imported-config.cmake b/cmake/patch-imported-config.cmake index ec68d74099..a2d22ed398 100644 --- a/cmake/patch-imported-config.cmake +++ b/cmake/patch-imported-config.cmake @@ -21,8 +21,11 @@ if(TARGET c-ares::cares) endif() # curl targets -if(TARGET CURL::libcurl) - project_build_tools_patch_default_imported_config(CURL::libcurl) +if(TARGET CURL::libcurl + OR TARGET CURL::libcurl_static + OR TARGET CURL::libcurl_shared) + project_build_tools_patch_default_imported_config( + CURL::libcurl CURL::libcurl_static CURL::libcurl_shared) endif() # abseil targets diff --git a/cmake/tools.cmake b/cmake/tools.cmake index ee191121ca..43c1a7b43f 100644 --- a/cmake/tools.cmake +++ b/cmake/tools.cmake @@ -14,7 +14,16 @@ endmacro() if(NOT PATCH_PROTOBUF_SOURCES_OPTIONS_SET) if(MSVC) unset(PATCH_PROTOBUF_SOURCES_OPTIONS CACHE) - set(PATCH_PROTOBUF_SOURCES_OPTIONS /wd4244 /wd4251 /wd4267 /wd4309 /wd4668 /wd4946 /wd6001 /wd6244 /wd6246) + set(PATCH_PROTOBUF_SOURCES_OPTIONS + /wd4244 + /wd4251 + /wd4267 + /wd4309 + /wd4668 + /wd4946 + /wd6001 + /wd6244 + /wd6246) if(MSVC_VERSION GREATER_EQUAL 1922) # see @@ -147,6 +156,11 @@ function(project_build_tools_patch_default_imported_config) continue() endif() + get_target_property(IS_ALIAS_TARGET ${TARGET_NAME} ALIASED_TARGET) + if(IS_ALIAS_TARGET) + continue() + endif() + if(CMAKE_VERSION VERSION_LESS "3.19.0") get_target_property(TARGET_TYPE_NAME ${TARGET_NAME} TYPE) if(TARGET_TYPE_NAME STREQUAL "INTERFACE_LIBRARY")