From ac6390bde6d36b8f65924e0d193e4823df87193b Mon Sep 17 00:00:00 2001 From: David Ashpole Date: Mon, 26 Oct 2026 18:17:47 +0000 Subject: [PATCH 1/3] [EXPORTER] Prometheus exporter: fix attribute serialization, label collision, and empty point guards Fix pre-existing bugs in PrometheusExporterUtils: - Serialize array-valued OwnedAttributeValue alternatives as JSON arrays in AttributeValueToString per the OpenTelemetry attribute specification. - Merge generated otel_scope_name and otel_scope_version labels with colliding point attributes in SetMetricBasic using ';' concatenation ordered by original key. - Fix SetMetricBasic early return so metrics with empty point attributes and null resource still receive scope labels when scope is non-null. - Guard against empty point_data_attr_ in TranslateToPrometheus before accessing front(). --- CHANGELOG.md | 4 + exporters/prometheus/src/exporter_utils.cc | 213 +++++++++++++++++- .../prometheus/test/exporter_utils_test.cc | 89 ++++++++ 3 files changed, 294 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 098a96cc33..702cc2f9d3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,10 @@ Increment the: ## [Unreleased] +* [EXPORTER] Prometheus exporter: fix attribute serialization, label collision, + and empty point guards + [#4738](https://github.com/open-telemetry/opentelemetry-cpp/pull/4738) + ## [1.30.0] 2026-10-22 This release contains: diff --git a/exporters/prometheus/src/exporter_utils.cc b/exporters/prometheus/src/exporter_utils.cc index b6f817a91b..5f17dff5b3 100644 --- a/exporters/prometheus/src/exporter_utils.cc +++ b/exporters/prometheus/src/exporter_utils.cc @@ -5,6 +5,7 @@ #include #include #include +#include #include #include #include @@ -101,6 +102,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(c); + char buf[6] = {'\\', 'u', '0', '0', kHex[(uc >> 4U) & 0x0FU], kHex[uc & 0x0FU]}; + out.append(buf, 6); + } + else + { + out.push_back(static_cast(c)); + } + break; + } + } + out.push_back('"'); + return out; +} + +template +std::string NumericVectorToJson(const std::vector &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 /** @@ -142,6 +209,10 @@ std::vector PrometheusExporterUtils::TranslateT { for (const auto &metric_data : instrumentation_info.metric_data_) { + if (metric_data.point_data_attr_.empty()) + { + continue; + } prometheus_client::MetricFamily metric_family; metric_family.help = metric_data.instrument_descriptor.description_; auto front = metric_data.point_data_attr_.front(); @@ -671,7 +742,7 @@ void PrometheusExporterUtils::SetMetricBasic( const opentelemetry::sdk::instrumentationscope::InstrumentationScope *scope, const opentelemetry::sdk::resource::Resource *resource) { - if (labels.empty() && nullptr == resource) + if (labels.empty() && nullptr == scope && nullptr == resource) { return; } @@ -681,6 +752,18 @@ void PrometheusExporterUtils::SetMetricBasic( // We could sort the sanitized keys again, but this seems too expensive to do // in this hot code path. Instead, we ignore out-of-order keys and emit a warning. metric.label.reserve(labels.size() + 2); + + struct LabelCollisionEntry + { + std::string original_key; + std::string value; + }; + std::vector> label_entries; + if (scope != nullptr) + { + label_entries.reserve(labels.size() + 2); + } + std::string previous_key; for (auto const &label : labels) { @@ -688,12 +771,22 @@ void PrometheusExporterUtils::SetMetricBasic( 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 { @@ -708,19 +801,46 @@ void PrometheusExporterUtils::SetMetricBasic( { return; } - auto scope_name = scope->GetName(); + + 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 = std::move(scope_name); + add_or_merge_scope_label(kScopeNameKey, kScopeNameKey, scope_name); } - auto scope_version = scope->GetVersion(); + const auto &scope_version = scope->GetVersion(); if (!scope_version.empty()) { - metric.label.emplace_back(); - metric.label.back().name = kScopeVersionKey; - metric.label.back().value = std::move(scope_version); + add_or_merge_scope_label(kScopeVersionKey, kScopeVersionKey, scope_version); } } @@ -756,6 +876,75 @@ std::string PrometheusExporterUtils::AttributeValueToString( { result = nostd::get(value); } + else if (nostd::holds_alternative>(value)) + { + const auto &vec = nostd::get>(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>(value)) + { + result = NumericVectorToJson(nostd::get>(value)); + } + else if (nostd::holds_alternative>(value)) + { + result = NumericVectorToJson(nostd::get>(value)); + } + else if (nostd::holds_alternative>(value)) + { + result = NumericVectorToJson(nostd::get>(value)); + } + else if (nostd::holds_alternative>(value)) + { + result = NumericVectorToJson(nostd::get>(value)); + } + else if (nostd::holds_alternative>(value)) + { + const auto &vec = nostd::get>(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>(value)) + { + const auto &vec = nostd::get>(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( diff --git a/exporters/prometheus/test/exporter_utils_test.cc b/exporters/prometheus/test/exporter_utils_test.cc index 4c08a4037b..51a1344672 100644 --- a/exporters/prometheus/test/exporter_utils_test.cc +++ b/exporters/prometheus/test/exporter_utils_test.cc @@ -15,8 +15,10 @@ #include "opentelemetry/common/timestamp.h" #include "opentelemetry/exporters/prometheus/exporter_utils.h" #include "opentelemetry/nostd/unique_ptr.h" +#include "opentelemetry/nostd/variant.h" #include "opentelemetry/sdk/instrumentationscope/instrumentation_scope.h" #include "opentelemetry/sdk/metrics/data/metric_data.h" +#include "opentelemetry/sdk/metrics/data/point_data.h" #include "opentelemetry/sdk/metrics/export/metric_producer.h" #include "opentelemetry/sdk/metrics/instruments.h" #include "opentelemetry/sdk/resource/resource.h" @@ -547,4 +549,91 @@ TEST_F(AttributeCollisionTest, DropsInvertedKeys) {"otel_scope_version", "1.2.0"}}); } +TEST(PrometheusExporterUtils, EmptyPointDataAttributesAndNullResourceScopeLabels) +{ + auto scope = InstrumentationScope::Create("library_name", "1.2.0"); + + metric_sdk::MetricData empty_metric_data{ + metric_sdk::InstrumentDescriptor{"empty_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::SumPointData sum_point_data{}; + sum_point_data.value_ = 10.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{{{}, sum_point_data}}}; + + metric_sdk::ResourceMetrics data{ + nullptr, + std::vector{ + {scope.get(), std::vector{empty_metric_data, 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 expected = {{"otel_scope_name", "library_name"}, + {"otel_scope_version", "1.2.0"}}; + 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); + } +} + +TEST(PrometheusExporterUtils, CrossSourceScopeCollisionsAndArrayAttributes) +{ + auto scope = InstrumentationScope::Create("library_name", "1.2.0"); + + metric_sdk::PointAttributes point_attrs; + point_attrs["otel_scope.name"] = std::string("point_name"); + point_attrs["otel_scope_version"] = std::string("0.9.0"); + point_attrs["tags.a"] = std::vector{"a", "b\"c"}; + point_attrs["tags_a"] = std::vector{"d"}; + point_attrs["counts"] = std::vector{1, 2, 3}; + point_attrs["flags"] = std::vector{true, false}; + + 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{{std::move(point_attrs), sum_point_data}}}; + + Resource resource = Resource::Create(ResourceAttributes{}); + metric_sdk::ResourceMetrics data{ + &resource, std::vector{ + {scope.get(), std::vector{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 expected = { + {"counts", "[1,2,3]"}, + {"flags", "[true,false]"}, + {"otel_scope_name", "point_name;library_name"}, + {"otel_scope_version", "0.9.0;1.2.0"}, + {"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 From 6f08e40eb075e9954d89571f255a04fa458c6d91 Mon Sep 17 00:00:00 2001 From: David Ashpole Date: Tue, 27 Oct 2026 15:22:52 +0000 Subject: [PATCH 2/3] Address review comment: preserve double precision with locale-independent formatting --- exporters/prometheus/src/exporter_utils.cc | 23 ++++++++++++++++++- .../prometheus/test/exporter_utils_test.cc | 2 ++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/exporters/prometheus/src/exporter_utils.cc b/exporters/prometheus/src/exporter_utils.cc index 5f17dff5b3..30f5093f00 100644 --- a/exporters/prometheus/src/exporter_utils.cc +++ b/exporters/prometheus/src/exporter_utils.cc @@ -9,7 +9,9 @@ #include #include #include +#include #include +#include #include #include #include @@ -168,6 +170,25 @@ std::string NumericVectorToJson(const std::vector &vec) return result; } +std::string FormatDouble(double val) +{ + std::ostringstream ss; + ss.imbue(std::locale::classic()); + ss << std::setprecision(std::numeric_limits::digits10) << val; + std::string str = ss.str(); + std::istringstream iss(str); + iss.imbue(std::locale::classic()); + double roundtrip = 0.0; + if ((iss >> roundtrip) && roundtrip == val) + { + return str; + } + ss.str(""); + ss.clear(); + ss << std::setprecision(std::numeric_limits::max_digits10) << val; + return ss.str(); +} + } // namespace /** @@ -926,7 +947,7 @@ std::string PrometheusExporterUtils::AttributeValueToString( } else { - result.append(std::to_string(vec[i])); + result.append(FormatDouble(vec[i])); } } result.push_back(']'); diff --git a/exporters/prometheus/test/exporter_utils_test.cc b/exporters/prometheus/test/exporter_utils_test.cc index 51a1344672..415ca9e34f 100644 --- a/exporters/prometheus/test/exporter_utils_test.cc +++ b/exporters/prometheus/test/exporter_utils_test.cc @@ -600,6 +600,7 @@ TEST(PrometheusExporterUtils, CrossSourceScopeCollisionsAndArrayAttributes) point_attrs["tags.a"] = std::vector{"a", "b\"c"}; point_attrs["tags_a"] = std::vector{"d"}; point_attrs["counts"] = std::vector{1, 2, 3}; + point_attrs["doubles"] = std::vector{1e-9, 2e-9, 1.5}; point_attrs["flags"] = std::vector{true, false}; metric_sdk::SumPointData sum_point_data{}; @@ -624,6 +625,7 @@ TEST(PrometheusExporterUtils, CrossSourceScopeCollisionsAndArrayAttributes) const auto &labels = translated[0].metric[0].label; std::vector expected = { {"counts", "[1,2,3]"}, + {"doubles", "[1e-09,2e-09,1.5]"}, {"flags", "[true,false]"}, {"otel_scope_name", "point_name;library_name"}, {"otel_scope_version", "0.9.0;1.2.0"}, From a8fedef197245b0048b9ee8184b852c6c25094b6 Mon Sep 17 00:00:00 2001 From: David Ashpole Date: Tue, 27 Oct 2026 15:25:09 +0000 Subject: [PATCH 3/3] Address review comment: track collision entries only for scope label collisions --- exporters/prometheus/src/exporter_utils.cc | 50 ++++++++++++---------- 1 file changed, 27 insertions(+), 23 deletions(-) diff --git a/exporters/prometheus/src/exporter_utils.cc b/exporters/prometheus/src/exporter_utils.cc index 30f5093f00..9377a6ce66 100644 --- a/exporters/prometheus/src/exporter_utils.cc +++ b/exporters/prometheus/src/exporter_utils.cc @@ -776,14 +776,11 @@ void PrometheusExporterUtils::SetMetricBasic( struct LabelCollisionEntry { + std::size_t label_index; std::string original_key; std::string value; }; - std::vector> label_entries; - if (scope != nullptr) - { - label_entries.reserve(labels.size() + 2); - } + std::vector colliding_scope_entries; std::string previous_key; for (auto const &label : labels) @@ -794,18 +791,18 @@ void PrometheusExporterUtils::SetMetricBasic( { previous_key = sanitized; std::string val = AttributeValueToString(label.second); - if (scope != nullptr) + if (scope != nullptr && (sanitized == kScopeNameKey || sanitized == kScopeVersionKey)) { - label_entries.push_back({{label.first, val}}); + colliding_scope_entries.push_back({metric.label.size(), label.first, val}); } metric.label.push_back({sanitized, std::move(val)}); } else if (comparison == 0) // key collision after sanitation { std::string val = AttributeValueToString(label.second); - if (scope != nullptr) + if (scope != nullptr && (sanitized == kScopeNameKey || sanitized == kScopeVersionKey)) { - label_entries.back().push_back({label.first, val}); + colliding_scope_entries.push_back({metric.label.size() - 1, label.first, val}); } metric.label.back().value += ";" + val; } @@ -823,33 +820,40 @@ 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) + auto add_or_merge_scope_label = [&metric, &colliding_scope_entries]( + const std::string &sanitized_key, + const std::string &original_key, const std::string &value) { + for (const auto &entry : colliding_scope_entries) { - if (metric.label[i].name == sanitized_key) + if (metric.label[entry.label_index].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; + const std::size_t target_index = entry.label_index; + colliding_scope_entries.push_back({target_index, original_key, value}); + std::vector matched; + for (const auto &candidate : colliding_scope_entries) + { + if (candidate.label_index == target_index) + { + matched.push_back(&candidate); + } + } + std::stable_sort(matched.begin(), matched.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) + for (std::size_t j = 0; j < matched.size(); ++j) { if (j > 0) { merged.push_back(';'); } - merged.append(entries[j].value); + merged.append(matched[j]->value); } - metric.label[i].value = std::move(merged); + metric.label[target_index].value = std::move(merged); return; } } - label_entries.push_back({{original_key, value}}); metric.label.push_back({sanitized_key, value}); };