clean up nostd::variant access in otlp exporters. Add test coverage for mapping to proto AnyValue types

Author: dbarker

exporters/otlp/BUILD

@@ -613,6 +613,20 @@ cc_library(
     ],
 )
 
+cc_test(
+    name = "otlp_populate_attribute_utils_test",
+    srcs = ["test/otlp_populate_attribute_utils_test.cc"],
+    tags = [
+        "otlp",
+        "test",
+    ],
+    deps = [
+        ":otlp_recordable",
+        "//sdk/src/metrics",
+        "@com_google_googletest//:gtest_main",
+    ],
+)
+
 cc_test(
     name = "otlp_recordable_test",
     srcs = ["test/otlp_recordable_test.cc"],

exporters/otlp/CMakeLists.txt

@@ -863,6 +863,17 @@ if(WITH_OTLP_FILE)
 endif()
 
 if(BUILD_TESTING)
+  add_executable(otlp_populate_attribute_utils_test
+                 test/otlp_populate_attribute_utils_test.cc)
+  target_link_libraries(
+    otlp_populate_attribute_utils_test ${GTEST_BOTH_LIBRARIES}
+    ${CMAKE_THREAD_LIBS_INIT} opentelemetry_otlp_recordable
+    protobuf::libprotobuf)
+  gtest_add_tests(
+    TARGET otlp_populate_attribute_utils_test
+    TEST_PREFIX exporter.otlp.
+    TEST_LIST otlp_populate_attribute_utils_test)
+
   add_executable(otlp_recordable_test test/otlp_recordable_test.cc)
   target_link_libraries(
     otlp_recordable_test ${GTEST_BOTH_LIBRARIES} ${CMAKE_THREAD_LIBS_INIT}

exporters/otlp/src/otlp_http_client.cc

@@ -751,12 +751,14 @@ sdk::common::ExportResult OtlpHttpClient::Export(
     std::size_t max_running_requests) noexcept
 {
   auto session = createSession(message, std::move(result_callback));
-  if (opentelemetry::nostd::holds_alternative<sdk::common::ExportResult>(session))
+  if (const auto *result = opentelemetry::nostd::get_if<sdk::common::ExportResult>(&session))
   {
-    return opentelemetry::nostd::get<sdk::common::ExportResult>(session);
+    return *result;
   }
 
-  addSession(std::move(opentelemetry::nostd::get<HttpSessionData>(session)));
+  // session contains a valid HttpSessionData object
+  auto *session_data = opentelemetry::nostd::get_if<HttpSessionData>(&session);
+  addSession(std::move(*session_data));
 
   // Wait for the response to be received
   if (options_.console_debug)

exporters/otlp/src/otlp_metric_utils.cc

@@ -1,7 +1,7 @@
 // Copyright The OpenTelemetry Authors
 // SPDX-License-Identifier: Apache-2.0
 
-#include <stdint.h>
+#include <cstdint>
 #include <map>
 #include <memory>
 #include <utility>
@@ -38,6 +38,18 @@ namespace otlp
 {
 namespace metric_sdk = opentelemetry::sdk::metrics;
 
+namespace
+{
+
+struct NumberDataPointValueSetter
+{
+  proto::metrics::v1::NumberDataPoint *point;
+  void operator()(int64_t v) const noexcept { point->set_as_int(v); }
+  void operator()(double v) const noexcept { point->set_as_double(v); }
+};
+
+}  // namespace
+
 proto::metrics::v1::AggregationTemporality OtlpMetricUtils::GetProtoAggregationTemporality(
     const opentelemetry::sdk::metrics::AggregationTemporality &aggregation_temporality) noexcept
 {
@@ -89,23 +101,22 @@ void OtlpMetricUtils::ConvertSumMetric(const metric_sdk::MetricData &metric_data
       (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_)
+  for (const auto &point_data_with_attributes : metric_data.point_data_attr_)
   {
     proto::metrics::v1::NumberDataPoint *proto_sum_point_data = sum->add_data_points();
     proto_sum_point_data->set_start_time_unix_nano(start_ts);
     proto_sum_point_data->set_time_unix_nano(ts);
-    auto sum_data = nostd::get<sdk::metrics::SumPointData>(point_data_with_attributes.point_data);
-
-    if ((nostd::holds_alternative<int64_t>(sum_data.value_)))
-    {
-      proto_sum_point_data->set_as_int(nostd::get<int64_t>(sum_data.value_));
-    }
-    else
+    const auto *maybe_sum_data =
+        nostd::get_if<sdk::metrics::SumPointData>(&point_data_with_attributes.point_data);
+    if (maybe_sum_data == nullptr)
     {
-      proto_sum_point_data->set_as_double(nostd::get<double>(sum_data.value_));
+      continue;
     }
+    const auto &sum_data = *maybe_sum_data;
+
+    nostd::visit(NumberDataPointValueSetter{proto_sum_point_data}, sum_data.value_);
     // set attributes
-    for (auto &kv_attr : point_data_with_attributes.attributes)
+    for (const auto &kv_attr : point_data_with_attributes.attributes)
     {
       OtlpPopulateAttributeUtils::PopulateAttribute(proto_sum_point_data->add_attributes(),
                                                     kv_attr.first, kv_attr.second, false);
@@ -121,63 +132,46 @@ void OtlpMetricUtils::ConvertHistogramMetric(
       GetProtoAggregationTemporality(metric_data.aggregation_temporality));
   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_)
+  for (const auto &point_data_with_attributes : metric_data.point_data_attr_)
   {
     proto::metrics::v1::HistogramDataPoint *proto_histogram_point_data =
         histogram->add_data_points();
     proto_histogram_point_data->set_start_time_unix_nano(start_ts);
     proto_histogram_point_data->set_time_unix_nano(ts);
-    auto histogram_data =
-        nostd::get<sdk::metrics::HistogramPointData>(point_data_with_attributes.point_data);
-    // sum
-    if ((nostd::holds_alternative<int64_t>(histogram_data.sum_)))
-    {
-      // Use static_cast to avoid C4244 in MSVC
-      proto_histogram_point_data->set_sum(
-          static_cast<double>(nostd::get<int64_t>(histogram_data.sum_)));
-    }
-    else
+    const auto *maybe_histogram_data =
+        nostd::get_if<sdk::metrics::HistogramPointData>(&point_data_with_attributes.point_data);
+    if (maybe_histogram_data == nullptr)
     {
-      proto_histogram_point_data->set_sum(nostd::get<double>(histogram_data.sum_));
+      continue;
     }
+    const auto &histogram_data = *maybe_histogram_data;
+    // sum
+    // Use static_cast to avoid C4244 in MSVC
+    proto_histogram_point_data->set_sum(
+        nostd::visit([](auto v) { return static_cast<double>(v); }, histogram_data.sum_));
     // count
     proto_histogram_point_data->set_count(histogram_data.count_);
     if (histogram_data.record_min_max_)
     {
-      if (nostd::holds_alternative<int64_t>(histogram_data.min_))
-      {
-        // Use static_cast to avoid C4244 in MSVC
-        proto_histogram_point_data->set_min(
-            static_cast<double>(nostd::get<int64_t>(histogram_data.min_)));
-      }
-      else
-      {
-        proto_histogram_point_data->set_min(nostd::get<double>(histogram_data.min_));
-      }
-      if (nostd::holds_alternative<int64_t>(histogram_data.max_))
-      {
-        // Use static_cast to avoid C4244 in MSVC
-        proto_histogram_point_data->set_max(
-            static_cast<double>(nostd::get<int64_t>(histogram_data.max_)));
-      }
-      else
-      {
-        proto_histogram_point_data->set_max(nostd::get<double>(histogram_data.max_));
-      }
+      // Use static_cast to avoid C4244 in MSVC
+      proto_histogram_point_data->set_min(
+          nostd::visit([](auto v) { return static_cast<double>(v); }, histogram_data.min_));
+      proto_histogram_point_data->set_max(
+          nostd::visit([](auto v) { return static_cast<double>(v); }, histogram_data.max_));
     }
     // buckets
 
-    for (auto bound : histogram_data.boundaries_)
+    for (const auto &bound : histogram_data.boundaries_)
     {
       proto_histogram_point_data->add_explicit_bounds(bound);
     }
     // bucket counts
-    for (auto bucket_value : histogram_data.counts_)
+    for (const auto &bucket_value : histogram_data.counts_)
     {
       proto_histogram_point_data->add_bucket_counts(bucket_value);
     }
     // attributes
-    for (auto &kv_attr : point_data_with_attributes.attributes)
+    for (const auto &kv_attr : point_data_with_attributes.attributes)
     {
       OtlpPopulateAttributeUtils::PopulateAttribute(proto_histogram_point_data->add_attributes(),
                                                     kv_attr.first, kv_attr.second, false);
@@ -193,14 +187,20 @@ void OtlpMetricUtils::ConvertExponentialHistogramMetric(
       GetProtoAggregationTemporality(metric_data.aggregation_temporality));
   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_)
+  for (const auto &point_data_with_attributes : metric_data.point_data_attr_)
   {
     proto::metrics::v1::ExponentialHistogramDataPoint *proto_histogram_point_data =
         histogram->add_data_points();
     proto_histogram_point_data->set_start_time_unix_nano(start_ts);
     proto_histogram_point_data->set_time_unix_nano(ts);
-    auto histogram_data = nostd::get<sdk::metrics::Base2ExponentialHistogramPointData>(
-        point_data_with_attributes.point_data);
+    const auto *maybe_histogram_data =
+        nostd::get_if<sdk::metrics::Base2ExponentialHistogramPointData>(
+            &point_data_with_attributes.point_data);
+    if (maybe_histogram_data == nullptr)
+    {
+      continue;
+    }
+    const auto &histogram_data = *maybe_histogram_data;
     if (histogram_data.positive_buckets_ == nullptr && histogram_data.negative_buckets_ == nullptr)
     {
       continue;
@@ -241,7 +241,7 @@ void OtlpMetricUtils::ConvertExponentialHistogramMetric(
     proto_histogram_point_data->set_zero_count(histogram_data.zero_count_);
 
     // attributes
-    for (auto &kv_attr : point_data_with_attributes.attributes)
+    for (const auto &kv_attr : point_data_with_attributes.attributes)
     {
       OtlpPopulateAttributeUtils::PopulateAttribute(proto_histogram_point_data->add_attributes(),
                                                     kv_attr.first, kv_attr.second, false);
@@ -254,24 +254,22 @@ void OtlpMetricUtils::ConvertGaugeMetric(const opentelemetry::sdk::metrics::Metr
 {
   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_)
+  for (const auto &point_data_with_attributes : metric_data.point_data_attr_)
   {
     proto::metrics::v1::NumberDataPoint *proto_gauge_point_data = gauge->add_data_points();
     proto_gauge_point_data->set_start_time_unix_nano(start_ts);
     proto_gauge_point_data->set_time_unix_nano(ts);
-    auto gauge_data =
-        nostd::get<sdk::metrics::LastValuePointData>(point_data_with_attributes.point_data);
-
-    if ((nostd::holds_alternative<int64_t>(gauge_data.value_)))
+    const auto *maybe_gauge_data =
+        nostd::get_if<sdk::metrics::LastValuePointData>(&point_data_with_attributes.point_data);
+    if (maybe_gauge_data == nullptr)
     {
-      proto_gauge_point_data->set_as_int(nostd::get<int64_t>(gauge_data.value_));
-    }
-    else
-    {
-      proto_gauge_point_data->set_as_double(nostd::get<double>(gauge_data.value_));
+      continue;
     }
+    const auto &gauge_data = *maybe_gauge_data;
+
+    nostd::visit(NumberDataPointValueSetter{proto_gauge_point_data}, gauge_data.value_);
     // set attributes
-    for (auto &kv_attr : point_data_with_attributes.attributes)
+    for (const auto &kv_attr : point_data_with_attributes.attributes)
     {
       OtlpPopulateAttributeUtils::PopulateAttribute(proto_gauge_point_data->add_attributes(),
                                                     kv_attr.first, kv_attr.second, false);