From a45081a83b4ec728f8db42b0aa950ca2338ef756 Mon Sep 17 00:00:00 2001 From: Punya Biswal Date: Wed, 27 Sep 2023 03:04:27 -0400 Subject: [PATCH] [exporters/prometheus] Sanitize labels according to Prometheus spec (#2330) Fixes #2289 --- CHANGELOG.md | 2 + .../exporters/prometheus/exporter_utils.h | 9 -- exporters/prometheus/src/exporter_utils.cc | 129 +++++++++++------- .../prometheus/test/exporter_utils_test.cc | 44 ++++-- 4 files changed, 116 insertions(+), 68 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 38a077e7d8..7901a35732 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,8 @@ Increment the: [#2324](https://github.com/open-telemetry/opentelemetry-cpp/pull/2324) * [EXPORTER] Handle attribute key collisions caused by sanitation [#2324](https://github.com/open-telemetry/opentelemetry-cpp/pull/2326) +* [EXPORTER] Replace colons with underscores when converting to Prometheus label + [#2324](https://github.com/open-telemetry/opentelemetry-cpp/pull/2330) ## [1.11.0] 2023-08-21 diff --git a/exporters/prometheus/include/opentelemetry/exporters/prometheus/exporter_utils.h b/exporters/prometheus/include/opentelemetry/exporters/prometheus/exporter_utils.h index 312b641c3e..07d2c396c6 100644 --- 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 index 149d93290b..a840b087ff 100644 --- a/exporters/prometheus/src/exporter_utils.cc +++ b/exporters/prometheus/src/exporter_utils.cc @@ -20,6 +20,85 @@ namespace exporter { namespace metrics { +namespace +{ +/** + * Sanitize the given metric name by replacing invalid characters with _, + * ensuring that multiple consecutive _ characters are collapsed to a single _. + * + * @param valid a callable with the signature `(int pos, char ch) -> bool` that + * returns whether `ch` is valid at position `pos` in the string + * @param name the string to sanitize + */ +template +inline std::string Sanitize(std::string name, const T &valid) +{ + static_assert(std::is_convertible>::value, + "valid should be a callable with the signature " + "(int, char) -> bool"); + + 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]) && name[i] != replacement) + { + 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 key according to Prometheus rule. + * Prometheus metric label keys are required to match the following regex: + * [a-zA-Z_]([a-zA-Z0-9_])* + * and multiple consecutive _ characters must be collapsed to a single _. + */ +std::string SanitizeLabel(std::string label_key) +{ + return Sanitize(label_key, [](int i, char c) { + return (c >= 'a' && c <= 'z') || // + (c >= 'A' && c <= 'Z') || // + c == '_' || // + (c >= '0' && c <= '9' && i > 0); + }); +} + +/** + * Sanitize the given metric name according to Prometheus rule. + * Prometheus metric names are required to match the following regex: + * [a-zA-Z_:]([a-zA-Z0-9_:])* + * and multiple consecutive _ characters must be collapsed to a single _. + */ +std::string SanitizeName(std::string name) +{ + return Sanitize(name, [](int i, char c) { + return (c >= 'a' && c <= 'z') || // + (c >= 'A' && c <= 'Z') || // + c == '_' || // + c == ':' || // + (c >= '0' && c <= '9' && i > 0); + }); +} +} // namespace + /** * Helper function to convert OpenTelemetry metrics data collection * to Prometheus metrics data collection @@ -40,7 +119,7 @@ std::vector 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 +199,6 @@ std::vector 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 +304,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 index 228b63dbcf..c2a6273397 100644 --- 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{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 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 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 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