[exporters/prometheus] Sanitize labels according to Prometheus spec

Fixes #2289
open-telemetry · Sep 25, 2023 · 71d4104 · 71d4104
1 parent d49ba52
commit 71d4104
Show file tree

Hide file tree

Showing 3 changed files with 102 additions and 68 deletions.
diff --git a/exporters/prometheus/include/opentelemetry/exporters/prometheus/exporter_utils.h b/exporters/prometheus/include/opentelemetry/exporters/prometheus/exporter_utils.h
@@ -32,15 +32,6 @@ class PrometheusExporterUtils
       const sdk::metrics::ResourceMetrics &data);
 
 private:
-  /**
-   * Sanitize the given metric name or label according to Prometheus rule.
-   *
-   * This function is needed because names in OpenTelemetry can contain
-   * alphanumeric characters, '_', '.', and '-', whereas in Prometheus the
-   * name should only contain alphanumeric characters and '_'.
-   */
-  static std::string SanitizeNames(std::string name);
-
   static opentelemetry::sdk::metrics::AggregationType getAggregationType(
       const opentelemetry::sdk::metrics::PointType &point_type);
 

diff --git a/exporters/prometheus/src/exporter_utils.cc b/exporters/prometheus/src/exporter_utils.cc
@@ -20,6 +20,73 @@ namespace exporter
 {
 namespace metrics
 {
+namespace
+{
+template <typename T>
+inline std::string Sanitize(std::string name, const T &valid)
+{
+  constexpr const auto replacement     = '_';
+  constexpr const auto replacement_dup = '=';
+
+  bool has_dup = false;
+  for (int i = 0; i < (int)name.size(); ++i)
+  {
+    if (valid(i, name[i]))
+    {
+      continue;
+    }
+    if (i > 0 && (name[i - 1] == replacement || name[i - 1] == replacement_dup))
+    {
+      has_dup = true;
+      name[i] = replacement_dup;
+    }
+    else
+    {
+      name[i] = replacement;
+    }
+  }
+  if (has_dup)
+  {
+    auto end = std::remove(name.begin(), name.end(), replacement_dup);
+    return std::string{name.begin(), end};
+  }
+  return name;
+}
+
+/**
+ * Sanitize the given metric label according to Prometheus rule.
+ *
+ * This function is needed because names in OpenTelemetry can contain
+ * alphanumeric characters, '_', '.', and '-', whereas in Prometheus the
+ * name should only contain alphanumeric characters and '_'.
+ */
+std::string SanitizeLabel(std::string name)
+{
+  return Sanitize(name, [](int i, char c) {
+    return (c >= 'a' && c <= 'z') ||  //
+           (c >= 'A' && c <= 'Z') ||  //
+           (c >= '0' && c <= '9' && i > 0);
+  });
+}
+
+/**
+ * Sanitize the given metric name according to Prometheus rule.
+ *
+ * This function is needed because names in OpenTelemetry can contain
+ * alphanumeric characters, '_', '.', and '-', whereas in Prometheus the
+ * name should only contain alphanumeric characters and '_'.
+ */
+std::string SanitizeName(std::string name)
+{
+  return Sanitize(name, [](int i, char c) {
+    return (c >= 'a' && c <= 'z') ||  //
+           (c >= 'A' && c <= 'Z') ||  //
+           c == ':' ||                //
+           (c >= '0' && c <= '9' && i > 0);
+  });
+}
+}  // namespace
+
 /**
  * Helper function to convert OpenTelemetry metrics data collection
  * to Prometheus metrics data collection
@@ -40,7 +107,7 @@ std::vector<prometheus_client::MetricFamily> PrometheusExporterUtils::TranslateT
     {
       auto origin_name = metric_data.instrument_descriptor.name_;
       auto unit        = metric_data.instrument_descriptor.unit_;
-      auto sanitized   = SanitizeNames(origin_name);
+      auto sanitized   = SanitizeName(origin_name);
       prometheus_client::MetricFamily metric_family;
       metric_family.name = sanitized + "_" + unit;
       metric_family.help = metric_data.instrument_descriptor.description_;
@@ -120,52 +187,6 @@ std::vector<prometheus_client::MetricFamily> PrometheusExporterUtils::TranslateT
   return output;
 }
 
-/**
- * Sanitize the given metric name or label according to Prometheus rule.
- *
- * This function is needed because names in OpenTelemetry can contain
- * alphanumeric characters, '_', '.', and '-', whereas in Prometheus the
- * name should only contain alphanumeric characters and '_'.
- */
-std::string PrometheusExporterUtils::SanitizeNames(std::string name)
-{
-  constexpr const auto replacement     = '_';
-  constexpr const auto replacement_dup = '=';
-
-  auto valid = [](int i, char c) {
-    if ((c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || c == ':' ||
-        (c >= '0' && c <= '9' && i > 0))
-    {
-      return true;
-    }
-    return false;
-  };
-
-  bool has_dup = false;
-  for (int i = 0; i < (int)name.size(); ++i)
-  {
-    if (valid(i, name[i]))
-    {
-      continue;
-    }
-    if (i > 0 && (name[i - 1] == replacement || name[i - 1] == replacement_dup))
-    {
-      has_dup = true;
-      name[i] = replacement_dup;
-    }
-    else
-    {
-      name[i] = replacement;
-    }
-  }
-  if (has_dup)
-  {
-    auto end = std::remove(name.begin(), name.end(), replacement_dup);
-    return std::string{name.begin(), end};
-  }
-  return name;
-}
-
 metric_sdk::AggregationType PrometheusExporterUtils::getAggregationType(
     const metric_sdk::PointType &point_type)
 {
@@ -271,7 +292,7 @@ void PrometheusExporterUtils::SetMetricBasic(prometheus_client::ClientMetric &me
   std::string previous_key;
   for (auto const &label : labels)
   {
-    auto sanitized = SanitizeNames(label.first);
+    auto sanitized = SanitizeLabel(label.first);
     int comparison = previous_key.compare(sanitized);
     if (metric.label.empty() || comparison < 0)  // new key
     {

diff --git a/exporters/prometheus/test/exporter_utils_test.cc b/exporters/prometheus/test/exporter_utils_test.cc
@@ -138,33 +138,55 @@ TEST(PrometheusExporterUtils, TranslateToPrometheusHistogramNormal)
   assert_histogram(metric, std::list<double>{10.1, 20.2, 30.2}, {200, 300, 400, 500});
 }
 
-class SanitizeNameTest : public ::testing::Test
+class SanitizeTest : public ::testing::Test
 {
   Resource resource_ = Resource::Create({});
   nostd::unique_ptr<InstrumentationScope> instrumentation_scope_ =
       InstrumentationScope::Create("library_name", "1.2.0");
 
 protected:
-  void CheckSanitation(const std::string &original, const std::string &sanitized)
+  void CheckSanitizeName(const std::string &original, const std::string &sanitized)
   {
-    metric_sdk::InstrumentDescriptor instrument_descriptor_{
+    metric_sdk::InstrumentDescriptor instrument_descriptor{
         original, "description", "unit", metric_sdk::InstrumentType::kCounter,
         metric_sdk::InstrumentValueType::kDouble};
     std::vector<prometheus::MetricFamily> result = PrometheusExporterUtils::TranslateToPrometheus(
         {&resource_,
-         {{instrumentation_scope_.get(), {{instrument_descriptor_, {}, {}, {}, {{{}, {}}}}}}}});
+         {{instrumentation_scope_.get(), {{instrument_descriptor, {}, {}, {}, {{{}, {}}}}}}}});
     EXPECT_EQ(result.begin()->name, sanitized + "_unit");
   }
+
+  void CheckSanitizeLabel(const std::string &original, const std::string &sanitized)
+  {
+    metric_sdk::InstrumentDescriptor instrument_descriptor{
+        "name", "description", "unit", metric_sdk::InstrumentType::kCounter,
+        metric_sdk::InstrumentValueType::kDouble};
+    std::vector<prometheus::MetricFamily> result = PrometheusExporterUtils::TranslateToPrometheus(
+        {&resource_,
+         {{instrumentation_scope_.get(),
+           {{instrument_descriptor, {}, {}, {}, {{{{original, "value"}}, {}}}}}}}});
+    EXPECT_EQ(result.begin()->metric.begin()->label.begin()->name, sanitized);
+  }
 };
 
-TEST_F(SanitizeNameTest, All)
+TEST_F(SanitizeTest, Name)
+{
+  CheckSanitizeName("name", "name");
+  CheckSanitizeName("name?", "name_");
+  CheckSanitizeName("name???", "name_");
+  CheckSanitizeName("name?__", "name_");
+  CheckSanitizeName("name?__name", "name_name");
+  CheckSanitizeName("name?__name:", "name_name:");
+}
+
+TEST_F(SanitizeTest, Label)
 {
-  CheckSanitation("name", "name");
-  CheckSanitation("name?", "name_");
-  CheckSanitation("name???", "name_");
-  CheckSanitation("name?__", "name_");
-  CheckSanitation("name?__name", "name_name");
-  CheckSanitation("name?__name:", "name_name:");
+  CheckSanitizeLabel("name", "name");
+  CheckSanitizeLabel("name?", "name_");
+  CheckSanitizeLabel("name???", "name_");
+  CheckSanitizeLabel("name?__", "name_");
+  CheckSanitizeLabel("name?__name", "name_name");
+  CheckSanitizeLabel("name?__name:", "name_name_");
 }
 
 class AttributeCollisionTest : public ::testing::Test