Skip to content

Commit

Permalink
[exporters/prometheus] Handle attribute key collisions from sanitation
Browse files Browse the repository at this point in the history
Fixes #2290
  • Loading branch information
punya committed Sep 23, 2023
1 parent 41a7875 commit bdd9adf
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 8 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ Increment the:
[#2291](https://github.com/open-telemetry/opentelemetry-cpp/pull/2291)
* [EXPORTER] Remove explicit timestamps from metric points exported by Prometheus
[#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)

## [1.11.0] 2023-08-21

Expand Down
37 changes: 29 additions & 8 deletions exporters/prometheus/src/exporter_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -258,16 +258,37 @@ void PrometheusExporterUtils::SetData(std::vector<T> values,
void PrometheusExporterUtils::SetMetricBasic(prometheus_client::ClientMetric &metric,
const metric_sdk::PointAttributes &labels)
{
// auto label_pairs = ParseLabel(labels);
if (!labels.empty())
if (labels.empty())
{
metric.label.resize(labels.size());
size_t i = 0;
for (auto const &label : labels)
return;
}

// Concatenate values for keys that collide after sanitation.
// Note that attribute keys are sorted, but sanitized keys can be out-of-order.
// 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());
std::string previous_key;
for (auto const &label : labels)
{
auto sanitized = SanitizeNames(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)});
}
else if (comparison == 0) // key collision after sanitation
{
metric.label.back().value += ";" + AttributeValueToString(label.second);
}
else // order inversion introduced by sanitation
{
auto sanitized = SanitizeNames(label.first);
metric.label[i].name = sanitized;
metric.label[i++].value = AttributeValueToString(label.second);
OTEL_INTERNAL_LOG_WARN(
"[Prometheus Exporter] SetMetricBase - "
"the sort order of labels has changed because of sanitization: '"
<< label.first << "' became '" << sanitized << "' which is less than '" << previous_key
<< "'. Ignoring this label.");
}
}
}
Expand Down
38 changes: 38 additions & 0 deletions exporters/prometheus/test/exporter_utils_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -163,4 +163,42 @@ TEST(PrometheusExporterUtils, SanitizeName)
ASSERT_EQ(exporter::metrics::SanitizeNameTester::sanitize("name?__name:"), "name_name:");
}

class AttributeCollisionTest : public ::testing::Test
{
Resource resource_ = Resource::Create(ResourceAttributes{});
nostd::unique_ptr<InstrumentationScope> instrumentation_scope_ =
InstrumentationScope::Create("library_name", "1.2.0");
metric_sdk::InstrumentDescriptor instrument_descriptor_{"library_name", "description", "unit",
metric_sdk::InstrumentType::kCounter,
metric_sdk::InstrumentValueType::kDouble};

protected:
void CheckTranslation(const metric_sdk::PointAttributes &attrs,
const std::vector<prometheus::ClientMetric::Label> &expected)
{
std::vector<prometheus::MetricFamily> result = PrometheusExporterUtils::TranslateToPrometheus(
{&resource_,
{{instrumentation_scope_.get(), {{instrument_descriptor_, {}, {}, {}, {{attrs, {}}}}}}}});
EXPECT_EQ(result.begin()->metric.begin()->label, expected);
}
};

TEST_F(AttributeCollisionTest, SeparatesDistinctKeys)
{
CheckTranslation({{"foo.a", "value1"}, {"foo.b", "value2"}},
{{"foo_a", "value1"}, {"foo_b", "value2"}});
}

TEST_F(AttributeCollisionTest, JoinsCollidingKeys)
{
CheckTranslation({{"foo.a", "value1"}, {"foo_a", "value2"}}, //
{{"foo_a", "value1;value2"}});
}

TEST_F(AttributeCollisionTest, DropsInvertedKeys)
{
CheckTranslation({{"foo.a", "value1"}, {"foo.b", "value2"}, {"foo__a", "value3"}},
{{"foo_a", "value1"}, {"foo_b", "value2"}});
}

OPENTELEMETRY_END_NAMESPACE

0 comments on commit bdd9adf

Please sign in to comment.