Skip to content

Commit

Permalink
fix(core): Ignore labels with empty value
Browse files Browse the repository at this point in the history
The Data Mpdel spec states:
> A label with an empty label value is considered
> equivalent to a label that does not exist.
  • Loading branch information
gjasny committed Apr 6, 2022
1 parent bac0722 commit 651092f
Show file tree
Hide file tree
Showing 3 changed files with 84 additions and 9 deletions.
8 changes: 4 additions & 4 deletions core/include/prometheus/family.h
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ class PROMETHEUS_CPP_CORE_EXPORT Family : public Collectable {
/// metric.
/// \throw std::runtime_exception on invalid metric or label names.
Family(const std::string& name, const std::string& help,
const Labels& constant_labels);
Labels constant_labels);

/// \brief Add a new dimensional data.
///
Expand All @@ -108,7 +108,7 @@ class PROMETHEUS_CPP_CORE_EXPORT Family : public Collectable {
/// labels already exists - the already existing dimensional data.
/// \throw std::runtime_exception on invalid label names.
template <typename... Args>
T& Add(const Labels& labels, Args&&... args) {
T& Add(Labels labels, Args&&... args) {
return Add(labels, std::make_unique<T>(args...));
}

Expand All @@ -121,7 +121,7 @@ class PROMETHEUS_CPP_CORE_EXPORT Family : public Collectable {
/// \brief Returns true if the dimensional data with the given labels exist
///
/// \param labels A set of key-value pairs (= labels) of the dimensional data.
bool Has(const Labels& labels) const;
bool Has(Labels labels) const;

/// \brief Returns the name for this family.
///
Expand Down Expand Up @@ -149,7 +149,7 @@ class PROMETHEUS_CPP_CORE_EXPORT Family : public Collectable {
mutable std::mutex mutex_;

ClientMetric CollectMetric(const Labels& labels, T* metric) const;
T& Add(const Labels& labels, std::unique_ptr<T> object);
T& Add(Labels labels, std::unique_ptr<T> object);
};

} // namespace prometheus
46 changes: 41 additions & 5 deletions core/src/family.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,42 @@
#include "prometheus/summary.h"

namespace prometheus {
namespace {

template <class Key, class T, class Compare, class Alloc, class Pred>
void erase_if(std::map<Key, T, Compare, Alloc>& c, Pred pred) {
for (auto i = c.begin(), last = c.end(); i != last;)
if (pred(*i)) {
i = c.erase(i);
} else {
++i;
}
}

auto empty_label_value = [](const Labels::value_type& label) {
return label.second.empty();
};

// A label with an empty label value is considered equivalent to a label that
// does not exist.
void filter_labels(Labels& labels) {
// with C++20 use std::erase_if
erase_if(labels, empty_label_value);
}

Labels filter_and_return_labels(Labels labels) {
filter_labels(labels);
return labels;
}

} // namespace

template <typename T>
Family<T>::Family(const std::string& name, const std::string& help,
const Labels& constant_labels)
: name_(name), help_(help), constant_labels_(constant_labels) {
Labels constant_labels)
: name_(name),
help_(help),
constant_labels_(filter_and_return_labels(std::move(constant_labels))) {
if (!CheckMetricName(name_)) {
throw std::invalid_argument("Invalid metric name");
}
Expand All @@ -31,7 +62,9 @@ Family<T>::Family(const std::string& name, const std::string& help,
}

template <typename T>
T& Family<T>::Add(const Labels& labels, std::unique_ptr<T> object) {
T& Family<T>::Add(Labels labels, std::unique_ptr<T> object) {
filter_labels(labels);

std::lock_guard<std::mutex> lock{mutex_};

auto insert_result =
Expand Down Expand Up @@ -70,9 +103,12 @@ void Family<T>::Remove(T* metric) {
}

template <typename T>
bool Family<T>::Has(const Labels& labels) const {
bool Family<T>::Has(Labels labels) const {
filter_labels(labels);

std::lock_guard<std::mutex> lock{mutex_};
return metrics_.count(labels) != 0u;
auto count = metrics_.count(labels);
return count != 0u;
}

template <typename T>
Expand Down
39 changes: 39 additions & 0 deletions core/tests/family_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -139,5 +139,44 @@ TEST(FamilyTest, reject_summary_with_quantile_label) {
EXPECT_ANY_THROW(family.Add(labels, quantiles));
}

TEST(FamilyTest, ignore_constant_labels_with_empty_value) {
auto labels = Labels{{"foo", "bar"}, {"label", ""}};
auto expected_labels = Labels{{"foo", "bar"}};

Family<Counter> family{"total_requests", "All Requests", labels};
EXPECT_EQ(expected_labels, family.GetConstantLabels());
}

TEST(FamilyTest, ignore_labels_with_empty_value_on_add) {
auto labels = Labels{{"foo", "bar"}, {"label", ""}};
auto expected_labels = Labels{{"foo", "bar"}};

Family<Counter> family{"total_requests", "All Requests", {}};
family.Add(labels);

EXPECT_TRUE(family.Has(expected_labels));
}

TEST(FamilyTest, ignore_labels_with_empty_value_for_has) {
auto labels = Labels{{"foo", "bar"}};
auto accepted_labels = Labels{{"foo", "bar"}, {"label", ""}};

Family<Counter> family{"total_requests", "All Requests", {}};
family.Add(labels);

EXPECT_TRUE(family.Has(accepted_labels));
}

TEST(FamilyTest, ignore_labels_with_empty_value_and_merge) {
auto labelsA = Labels{{"foo", "bar"}};
auto labelsB = Labels{{"foo", "bar"}, {"label", ""}};

Family<Counter> family{"total_requests", "All Requests", {}};
auto& a = family.Add(labelsA);
auto& b = family.Add(labelsB);

EXPECT_EQ(&a, &b);
}

} // namespace
} // namespace prometheus

0 comments on commit 651092f

Please sign in to comment.