Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(core): ignore labels with empty value #579

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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