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

Adding Metrics family config #2121

Merged
merged 4 commits into from
Mar 20, 2024
Merged
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
45 changes: 43 additions & 2 deletions include/seastar/core/metrics_api.hh
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ namespace metrics {
SEASTAR_MODULE_EXPORT
struct relabel_config;

SEASTAR_MODULE_EXPORT
struct metric_family_config;
/*!
* \brief result of metric relabeling
*
Expand Down Expand Up @@ -330,10 +332,17 @@ using metric_metadata_fifo = std::deque<metric_info>;
*
* The meta data of a metric family compose of the
* metadata of the family, and a vector of the metadata for
* each of the metric.
* each of the metrics.
*
* The struct is used for two purposes. First, it allows iterating over all metric_families
* and all metrics related to them. Second, it only contains enabled metrics,
* making disabled metrics more efficient.
* The struct is recreated when impl._value_map changes
* Using a pointer to the family_info metadata is an efficient way to get
* from a metric_family to its metadata based on its name.
*/
struct metric_family_metadata {
metric_family_info mf;
metric_family_info& mf; //This points to impl._value_map
metric_metadata_fifo metrics;
};

Expand All @@ -358,6 +367,7 @@ class impl {
std::set<sstring> _labels;
std::vector<std::deque<metric_function>> _current_metrics;
std::vector<relabel_config> _relabel_configs;
std::vector<metric_family_config> _metric_family_configs;
public:
value_map& get_value_map() {
return _value_map;
Expand Down Expand Up @@ -398,6 +408,13 @@ public:
const std::vector<relabel_config>& get_relabel_configs() const noexcept {
return _relabel_configs;
}
const std::vector<metric_family_config>& get_metric_family_configs() const noexcept {
return _metric_family_configs;
}

void set_metric_family_configs(const std::vector<metric_family_config>& metrics_config);

void update_aggregate(metric_family_info& mf) const noexcept;
};

const value_map& get_value_map();
Expand Down Expand Up @@ -486,5 +503,29 @@ future<metric_relabeling_result> set_relabel_configs(const std::vector<relabel_c
*/
const std::vector<relabel_config>& get_relabel_configs();

/*
* \brief change the metrics family config
*
* Family config is a configuration that relates to all metrics with the same name but with different labels.
* set_metric_family_configs allows changing that configuration during run time.
* Specifically, change the label aggregation based on a metric name.
*
* The following is an example for setting the aggregate labels for the metric test_gauge_1
* and all metrics matching the regex test_gauge1.*:
*
* std::vector<sm::metric_family_config> fc(2);
* fc[0].name = "test_gauge_1";
* fc[0].aggregate_labels = { "lb" };
* fc[1].regex_name = "test_gauge1.*";
* fc[1].aggregate_labels = { "ll", "aa" };
* sm::set_metric_family_configs(fc);
*/
void set_metric_family_configs(const std::vector<metric_family_config>& metrics_config);

/*
* \brief return the current metric_family_config
* This function returns a vector of the current metrics family config
*/
const std::vector<metric_family_config>& get_metric_family_configs();
}
}
26 changes: 26 additions & 0 deletions include/seastar/core/relabel_config.hh
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,13 @@ public:
_regex = std::regex(_regex_str);
return *this;
}
bool empty() const noexcept {
return _regex_str.empty();
}

bool match(const std::string& str) const noexcept {
return !empty() && std::regex_match(str, _regex);
}
};

/*!
Expand Down Expand Up @@ -103,6 +110,25 @@ struct relabel_config {
*/
relabel_config::relabel_action relabel_config_action(const std::string& action);

/*!
* \brief metric_family_config allow changing metrics family configuration
*
* Allow changing metrics family configuration
* Supports changing the aggregation labels.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand what this sentence is saying - "changing the aggregation labels"? Are you changing labels? Or the aggregation of values with different labels?

* The metrics family can be identified by a name or by regex; name-matching is
* more efficient and should be used for a single metrics family.
*
* name - optional exact metric name
* regex_name - if set, all the metrics name that match the regular expression
* aggregate_labels - The labels to aggregate the metrics by.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still don't understand (see my comment on the commit message too) what aggregate_labels="x","y" means:
Does it mean you aggregate (sum?) the value with label x and label y and... where do you put the sum?
Or maybe it means that for each of x and y, you aggregate many different values that might have different other labels (does this include different shards?) and the result is one value for x and one value for y?
Or does it means something else?

Please explain this more clearly, because I don't have any intuitive grasp of what the concepts of "metric families" or "label aggregation" means.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After reading your test in the last patch, I think I understand better: It turns out labels have a name and a value - it's not that we have a metric with a label x - we can have one metric with the label "x=dog" and another one with "x=cat". So apparently aggregation will sum these two metrics and create one metric without (?) the label x at all. I still don't understand, though, what happens with other labels: What if we have the following four metrics:

some_metric[x=dog, z=hello] = 1
some_metric[x=cat, z=hello] = 2
some_metric[x=dog, z=hi] = 3
some_metric[x=cat, z=yo] = 4

How do we aggregate it over "x"? Do we sum all of those metrics and drop all labels? Or will we have three metrics for the three different "z" label values (i.e., just the first two metrics are summed)? This makes sense, but is it documented somewhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First, just for context, we already support aggregate by label, but currently, you have to configure it when you register the metrics, and it cannot be changed.

Aggregation works by dropping one or more labels and summing over all metrics that now have the same name and label.

Using your example, let's drop x

some_metric[ z=hello] = 1
some_metric[ z=hello] = 2
some_metric[ z=hi] = 3
some_metric[ z=yo] = 4

Now we sum and get:

some_metric[ z=hello] = 3
some_metric[ z=hi] = 3
some_metric[ z=yo] = 4

This is not new, just it's run-time configuration

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I must have been sleeping under a rock when you added it.

That being said, all of this should be documented somewhere. I also found the names of the fields and their explanations to be confusing. It says "aggregate_labels - The labels to aggregate the metrics by.". In the above example, we are not aggregating "by x" - in fact in some sense it's the opposite - we're dropping x, and aggregating the metrics by the other labels.

*
*/
struct metric_family_config {
std::string name;
relabel_config_regex regex_name = "";
std::vector<std::string> aggregate_labels;
};

SEASTAR_MODULE_EXPORT_END

}
Expand Down
34 changes: 34 additions & 0 deletions src/core/metrics.cc
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,20 @@ future<metric_relabeling_result> set_relabel_configs(const std::vector<relabel_c
const std::vector<relabel_config>& get_relabel_configs() {
return impl::get_local_impl()->get_relabel_configs();
}
void impl::impl::update_aggregate(metric_family_info& mf) const noexcept {
for (const auto& fc : _metric_family_configs) {
if (fc.name == mf.name || fc.regex_name.match(mf.name)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe you need to check regex_name.empty() here? Or match() is correct (and efficient enough) in the empty case?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assumed that it was efficient enough and the update_aggregate is a rare operation

mf.aggregate_labels = fc.aggregate_labels;
}
}
}
void set_metric_family_configs(const std::vector<metric_family_config>& family_config) {
impl::get_local_impl()->set_metric_family_configs(family_config);
}

const std::vector<metric_family_config>& get_metric_family_configs() {
return impl::get_local_impl()->get_metric_family_configs();
}

static bool apply_relabeling(const relabel_config& rc, impl::metric_info& info) {
std::stringstream s;
Expand Down Expand Up @@ -454,6 +467,7 @@ void impl::add_registration(const metric_id& id, const metric_type& type, metric
_value_map[name].info().inherit_type = type.type_name;
_value_map[name].info().name = id.full_name();
_value_map[name].info().aggregate_labels = aggregate_labels;
impl::update_aggregate(_value_map[name].info());
_value_map[name][rm->info().id.labels()] = rm;
}
dirty();
Expand Down Expand Up @@ -503,6 +517,26 @@ future<metric_relabeling_result> impl::set_relabel_configs(const std::vector<rel
}
return make_ready_future<metric_relabeling_result>(conflicts);
}

void impl::set_metric_family_configs(const std::vector<metric_family_config>& family_config) {
_metric_family_configs = family_config;
bool has_regex = false;
for (const auto& fc : family_config) {
has_regex |= !fc.regex_name.empty();
if (fc.name != "" && _value_map.find(fc.name) != _value_map.end()) {
_value_map[fc.name].info().aggregate_labels = fc.aggregate_labels;
}
}
if (has_regex) {
for (auto& [name, family] : _value_map) {
for (const auto& fc : family_config) {
if (fc.regex_name.match(name)) {
family.info().aggregate_labels = fc.aggregate_labels;
}
}
}
}
}
}

const bool metric_disabled = false;
Expand Down
63 changes: 63 additions & 0 deletions tests/unit/metrics_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,69 @@ SEASTAR_THREAD_TEST_CASE(test_relabel_add_labels) {
sm::set_relabel_configs({}).get();
}

SEASTAR_THREAD_TEST_CASE(test_metrics_family_aggregate) {
using namespace seastar::metrics;
namespace sm = seastar::metrics;
sm::metric_groups app_metrics;
sm::label lb("lb");
app_metrics.add_group("test", {
sm::make_gauge("gauge_1", sm::description("gague 1"), [] { return 1; })(lb("1")),
sm::make_gauge("gauge_1", sm::description("gague 1"), [] { return 2; })(lb("2")),
sm::make_counter("counter_1", sm::description("counter 1"), [] { return 3; })(lb("1")),
sm::make_counter("counter_1", sm::description("counter 1"), [] { return 4; })(lb("2"))
});
std::vector<sm::relabel_config> rl(2);
rl[0].source_labels = {"__name__"};
rl[0].action = sm::relabel_config::relabel_action::drop;

rl[1].source_labels = {"lb"};
rl[1].action = sm::relabel_config::relabel_action::keep;
// Dropping the lev label would cause a conflict, but not crash the system
sm::set_relabel_configs(rl).get();

std::vector<sm::metric_family_config> fc(2);
fc[0].name = "test_gauge_1";
fc[0].aggregate_labels = { "lb" };
fc[1].regex_name = "test_gauge1.*";
fc[1].aggregate_labels = { "ll", "aa" };
sm::set_metric_family_configs(fc);
seastar::foreign_ptr<seastar::metrics::impl::values_reference> values = seastar::metrics::impl::get_values();
int count = 0;
for (auto&& md : (*values->metadata)) {
if (md.mf.name == "test_gauge_1") {
BOOST_CHECK_EQUAL(md.mf.aggregate_labels.size(), 1);
BOOST_CHECK_EQUAL(md.mf.aggregate_labels[0], "lb");
} else {
BOOST_CHECK_EQUAL(md.mf.aggregate_labels.size(), 0);
}
count++;
}
BOOST_CHECK_EQUAL(count, 2);
app_metrics.add_group("test", {
sm::make_gauge("gauge1_1", sm::description("gague 1"), [] { return 1; })(lb("1")),
sm::make_gauge("gauge1_1", sm::description("gague 1"), [] { return 2; })(lb("2")),
sm::make_counter("counter1_1", sm::description("counter 1"), [] { return 3; })(lb("1")),
sm::make_counter("counter1_1", sm::description("counter 1"), [] { return 4; })(lb("2"))
});
values = seastar::metrics::impl::get_values();
count = 0;
for (auto&& md : (*values->metadata)) {
if (md.mf.name == "test_gauge_1") {
BOOST_CHECK_EQUAL(md.mf.aggregate_labels.size(), 1);
BOOST_CHECK_EQUAL(md.mf.aggregate_labels[0], "lb");
} else if (md.mf.name == "test_gauge1_1") {
BOOST_CHECK_EQUAL(md.mf.aggregate_labels.size(), 2);
BOOST_CHECK_EQUAL(md.mf.aggregate_labels[0], "ll");
} else {
BOOST_CHECK_EQUAL(md.mf.aggregate_labels.size(), 0);
}
count++;
}
BOOST_CHECK_EQUAL(count, 4);
std::vector<sm::relabel_config> rl1;
sm::set_relabel_configs(rl1).get();
}

SEASTAR_THREAD_TEST_CASE(test_relabel_drop_label_prevent_runtime_conflicts) {
using namespace seastar::metrics;
namespace sm = seastar::metrics;
Expand Down