Address Copilot review feedback: merge cross-source scope label collisions and serialize array attributes

Author: dashpole

exporters/prometheus/src/exporter_utils.cc

@@ -5,6 +5,7 @@
 #include <prometheus/metric_family.h>
 #include <prometheus/metric_type.h>
 #include <algorithm>
+#include <cmath>
 #include <cstddef>
 #include <cstdint>
 #include <functional>
@@ -103,6 +104,72 @@ std::string SanitizeLabel(std::string label_key)
   });
 }
 
+std::string EscapeJsonString(const std::string &str)
+{
+  std::string out;
+  out.reserve(str.size() + 2);
+  out.push_back('"');
+  for (unsigned char c : str)
+  {
+    switch (c)
+    {
+      case '"':
+        out.append("\\\"");
+        break;
+      case '\\':
+        out.append("\\\\");
+        break;
+      case '\b':
+        out.append("\\b");
+        break;
+      case '\f':
+        out.append("\\f");
+        break;
+      case '\n':
+        out.append("\\n");
+        break;
+      case '\r':
+        out.append("\\r");
+        break;
+      case '\t':
+        out.append("\\t");
+        break;
+      default:
+        if (c <= 0x1F)
+        {
+          constexpr char kHex[] = "0123456789abcdef";
+          unsigned int uc       = static_cast<unsigned int>(c);
+          char buf[6]           = {'\\', 'u', '0', '0', kHex[(uc >> 4U) & 0x0FU], kHex[uc & 0x0FU]};
+          out.append(buf, 6);
+        }
+        else
+        {
+          out.push_back(static_cast<char>(c));
+        }
+        break;
+    }
+  }
+  out.push_back('"');
+  return out;
+}
+
+template <typename T>
+std::string NumericVectorToJson(const std::vector<T> &vec)
+{
+  std::string result;
+  result.push_back('[');
+  for (std::size_t i = 0; i < vec.size(); ++i)
+  {
+    if (i > 0)
+    {
+      result.push_back(',');
+    }
+    result.append(std::to_string(vec[i]));
+  }
+  result.push_back(']');
+  return result;
+}
+
 }  // namespace
 
 /**
@@ -688,19 +755,41 @@ void PrometheusExporterUtils::SetMetricBasic(
     reserve_labels += 3 + scope->GetAttributes().size();
   }
   metric.label.reserve(reserve_labels);
+
+  struct LabelCollisionEntry
+  {
+    std::string original_key;
+    std::string value;
+  };
+  std::vector<std::vector<LabelCollisionEntry>> label_entries;
+  if (scope != nullptr)
+  {
+    label_entries.reserve(reserve_labels);
+  }
+
   std::string previous_key;
   for (auto const &label : labels)
   {
     auto sanitized = SanitizeLabel(label.first);
     int comparison = previous_key.compare(sanitized);
     if (metric.label.empty() || comparison < 0)  // new key
     {
-      previous_key = sanitized;
-      metric.label.push_back({sanitized, AttributeValueToString(label.second)});
+      previous_key    = sanitized;
+      std::string val = AttributeValueToString(label.second);
+      if (scope != nullptr)
+      {
+        label_entries.push_back({{label.first, val}});
+      }
+      metric.label.push_back({sanitized, std::move(val)});
     }
     else if (comparison == 0)  // key collision after sanitation
     {
-      metric.label.back().value += ";" + AttributeValueToString(label.second);
+      std::string val = AttributeValueToString(label.second);
+      if (scope != nullptr)
+      {
+        label_entries.back().push_back({label.first, val});
+      }
+      metric.label.back().value += ";" + val;
     }
     else  // order inversion introduced by sanitation
     {
@@ -715,26 +804,51 @@ void PrometheusExporterUtils::SetMetricBasic(
   {
     return;
   }
+
+  auto add_or_merge_scope_label = [&metric, &label_entries](const std::string &sanitized_key,
+                                                            const std::string &original_key,
+                                                            const std::string &value) {
+    for (std::size_t i = 0; i < metric.label.size(); ++i)
+    {
+      if (metric.label[i].name == sanitized_key)
+      {
+        auto &entries = label_entries[i];
+        entries.push_back({original_key, value});
+        std::stable_sort(entries.begin(), entries.end(),
+                         [](const LabelCollisionEntry &a, const LabelCollisionEntry &b) {
+                           return a.original_key < b.original_key;
+                         });
+        std::string merged;
+        for (std::size_t j = 0; j < entries.size(); ++j)
+        {
+          if (j > 0)
+          {
+            merged.push_back(';');
+          }
+          merged.append(entries[j].value);
+        }
+        metric.label[i].value = std::move(merged);
+        return;
+      }
+    }
+    label_entries.push_back({{original_key, value}});
+    metric.label.push_back({sanitized_key, value});
+  };
+
   const auto &scope_name = scope->GetName();
   if (!scope_name.empty())
   {
-    metric.label.emplace_back();
-    metric.label.back().name  = kScopeNameKey;
-    metric.label.back().value = scope_name;
+    add_or_merge_scope_label(kScopeNameKey, kScopeNameKey, scope_name);
   }
   const auto &scope_version = scope->GetVersion();
   if (!scope_version.empty())
   {
-    metric.label.emplace_back();
-    metric.label.back().name  = kScopeVersionKey;
-    metric.label.back().value = scope_version;
+    add_or_merge_scope_label(kScopeVersionKey, kScopeVersionKey, scope_version);
   }
   const auto &scope_schema_url = scope->GetSchemaURL();
   if (!scope_schema_url.empty())
   {
-    metric.label.emplace_back();
-    metric.label.back().name  = kScopeSchemaUrlKey;
-    metric.label.back().value = scope_schema_url;
+    add_or_merge_scope_label(kScopeSchemaUrlKey, kScopeSchemaUrlKey, scope_schema_url);
   }
   const auto &scope_attributes = scope->GetAttributes();
   if (!scope_attributes.empty())
@@ -765,17 +879,10 @@ void PrometheusExporterUtils::SetMetricBasic(
                 }
                 return *a.original_key < *b.original_key;
               });
-    for (std::size_t i = 0; i < sorted_scope_attrs.size(); ++i)
+    for (const auto &scope_attr : sorted_scope_attrs)
     {
-      if (i > 0 && sorted_scope_attrs[i].sanitized_key == metric.label.back().name)
-      {
-        metric.label.back().value += ";" + AttributeValueToString(*sorted_scope_attrs[i].value);
-      }
-      else
-      {
-        metric.label.push_back({std::move(sorted_scope_attrs[i].sanitized_key),
-                                AttributeValueToString(*sorted_scope_attrs[i].value)});
-      }
+      add_or_merge_scope_label(scope_attr.sanitized_key, *scope_attr.original_key,
+                               AttributeValueToString(*scope_attr.value));
     }
   }
 }
@@ -812,6 +919,75 @@ std::string PrometheusExporterUtils::AttributeValueToString(
   {
     result = nostd::get<std::string>(value);
   }
+  else if (nostd::holds_alternative<std::vector<bool>>(value))
+  {
+    const auto &vec = nostd::get<std::vector<bool>>(value);
+    result.push_back('[');
+    for (std::size_t i = 0; i < vec.size(); ++i)
+    {
+      if (i > 0)
+      {
+        result.push_back(',');
+      }
+      result.append(vec[i] ? "true" : "false");
+    }
+    result.push_back(']');
+  }
+  else if (nostd::holds_alternative<std::vector<int>>(value))
+  {
+    result = NumericVectorToJson(nostd::get<std::vector<int>>(value));
+  }
+  else if (nostd::holds_alternative<std::vector<int64_t>>(value))
+  {
+    result = NumericVectorToJson(nostd::get<std::vector<int64_t>>(value));
+  }
+  else if (nostd::holds_alternative<std::vector<unsigned int>>(value))
+  {
+    result = NumericVectorToJson(nostd::get<std::vector<unsigned int>>(value));
+  }
+  else if (nostd::holds_alternative<std::vector<uint64_t>>(value))
+  {
+    result = NumericVectorToJson(nostd::get<std::vector<uint64_t>>(value));
+  }
+  else if (nostd::holds_alternative<std::vector<double>>(value))
+  {
+    const auto &vec = nostd::get<std::vector<double>>(value);
+    result.push_back('[');
+    for (std::size_t i = 0; i < vec.size(); ++i)
+    {
+      if (i > 0)
+      {
+        result.push_back(',');
+      }
+      if (std::isnan(vec[i]))
+      {
+        result.append("\"NaN\"");
+      }
+      else if (std::isinf(vec[i]))
+      {
+        result.append(vec[i] < 0 ? "\"-Infinity\"" : "\"Infinity\"");
+      }
+      else
+      {
+        result.append(std::to_string(vec[i]));
+      }
+    }
+    result.push_back(']');
+  }
+  else if (nostd::holds_alternative<std::vector<std::string>>(value))
+  {
+    const auto &vec = nostd::get<std::vector<std::string>>(value);
+    result.push_back('[');
+    for (std::size_t i = 0; i < vec.size(); ++i)
+    {
+      if (i > 0)
+      {
+        result.push_back(',');
+      }
+      result.append(EscapeJsonString(vec[i]));
+    }
+    result.push_back(']');
+  }
   else
   {
     OTEL_INTERNAL_LOG_WARN(

exporters/prometheus/test/exporter_utils_test.cc

@@ -15,6 +15,7 @@
 #include "opentelemetry/common/timestamp.h"
 #include "opentelemetry/exporters/prometheus/exporter_utils.h"
 #include "opentelemetry/nostd/unique_ptr.h"
+#include "opentelemetry/sdk/common/attribute_utils.h"
 #include "opentelemetry/sdk/instrumentationscope/instrumentation_scope.h"
 #include "opentelemetry/sdk/metrics/data/metric_data.h"
 #include "opentelemetry/sdk/metrics/data/point_data.h"
@@ -641,4 +642,58 @@ TEST(PrometheusExporterUtils, InstrumentationScopeAttributeCollisionsAndReserved
   }
 }
 
+TEST(PrometheusExporterUtils, InstrumentationScopeCrossSourceCollisionsAndArrayAttributes)
+{
+  opentelemetry::sdk::instrumentationscope::InstrumentationScopeAttributes scope_attrs;
+  scope_attrs["foo"]    = std::string("scope");
+  scope_attrs["z"]      = std::string("scope_z");
+  scope_attrs["tags.a"] = std::vector<std::string>{"a", "b\"c"};
+  scope_attrs["tags_a"] = std::vector<std::string>{"d"};
+  scope_attrs["counts"] = std::vector<int>{1, 2, 3};
+  scope_attrs["flags"]  = std::vector<bool>{true, false};
+  auto scope            = InstrumentationScope::Create(
+      "library_name", "1.2.0", "https://opentelemetry.io/schemas/1.21.0", std::move(scope_attrs));
+
+  metric_sdk::SumPointData sum_point_data{};
+  sum_point_data.value_ = 5.0;
+  metric_sdk::MetricData metric_data{
+      metric_sdk::InstrumentDescriptor{"requests", "description", "1",
+                                       metric_sdk::InstrumentType::kCounter,
+                                       metric_sdk::InstrumentValueType::kDouble},
+      metric_sdk::AggregationTemporality::kCumulative, opentelemetry::common::SystemTimestamp{},
+      opentelemetry::common::SystemTimestamp{},
+      std::vector<metric_sdk::PointDataAttributes>{
+          {{{"otel_scope_foo", "point"},
+            {"otel_scope_schema_url", "https://example.com/point_schema"},
+            {"otel_scope_z", "point_z"}},
+           sum_point_data}}};
+
+  Resource resource = Resource::Create(ResourceAttributes{});
+  metric_sdk::ResourceMetrics data{
+      &resource, std::vector<metric_sdk::ScopeMetrics>{
+                     {scope.get(), std::vector<metric_sdk::MetricData>{metric_data}}}};
+
+  auto translated = PrometheusExporterUtils::TranslateToPrometheus(data, false, false);
+  ASSERT_EQ(translated.size(), 1U);
+  ASSERT_EQ(translated[0].metric.size(), 1U);
+
+  const auto &labels                                    = translated[0].metric[0].label;
+  std::vector<prometheus::ClientMetric::Label> expected = {
+      {"otel_scope_foo", "scope;point"},
+      {"otel_scope_schema_url",
+       "https://example.com/point_schema;https://opentelemetry.io/schemas/1.21.0"},
+      {"otel_scope_z", "point_z;scope_z"},
+      {"otel_scope_name", "library_name"},
+      {"otel_scope_version", "1.2.0"},
+      {"otel_scope_counts", "[1,2,3]"},
+      {"otel_scope_flags", "[true,false]"},
+      {"otel_scope_tags_a", "[\"a\",\"b\\\"c\"];[\"d\"]"}};
+  ASSERT_EQ(labels.size(), expected.size());
+  for (std::size_t i = 0; i < expected.size(); ++i)
+  {
+    EXPECT_EQ(labels[i].name, expected[i].name);
+    EXPECT_EQ(labels[i].value, expected[i].value);
+  }
+}
+
 OPENTELEMETRY_END_NAMESPACE