From 42f15a5fd54b8a6e99b593202bbccf311e7e09e1 Mon Sep 17 00:00:00 2001 From: Kefu Chai Date: Thu, 9 May 2024 15:55:09 +0800 Subject: [PATCH] prometheus: revert the condition for enabling aggregation in d7e16bbca0, we added the query time configuration for disabling aggregation, but the condition for disabling aggregation was wrong. before that change, we always enable the aggregation, but after that change, the aggregation is disabled when the value of query parameter of "__aggregate__" is not "false". in other words, - if this query parameter is not set, the aggregation is disabled. this changes the existing behavior. - if this query parameter's value is "false", the aggregation is enabeld. this is not the expected behavior. so, in this change - change the parameter from `disable_aggregation` to `enable_aggregation`. as the positive condition is always easier to understand. - and update the handling accodingly. Fixes d7e16bbca0d81e2d3b43886c932c242eeffb6e96 Signed-off-by: Kefu Chai Closes #2232 --- src/core/prometheus.cc | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/core/prometheus.cc b/src/core/prometheus.cc index 87fd93d148..e183e5d30f 100644 --- a/src/core/prometheus.cc +++ b/src/core/prometheus.cc @@ -735,15 +735,15 @@ std::string get_value_as_string(std::stringstream& s, const mi::metric_value& va return value_str; } -future<> write_text_representation(output_stream& out, const config& ctx, const metric_family_range& m, bool show_help, bool disable_aggregation, std::function filter) { - return seastar::async([&ctx, &out, &m, show_help, disable_aggregation, filter] () mutable { +future<> write_text_representation(output_stream& out, const config& ctx, const metric_family_range& m, bool show_help, bool enable_aggregation, std::function filter) { + return seastar::async([&ctx, &out, &m, show_help, enable_aggregation, filter] () mutable { bool found = false; std::stringstream s; for (metric_family& metric_family : m) { auto name = ctx.prefix + "_" + metric_family.name(); found = false; metric_aggregate_by_labels aggregated_values(metric_family.metadata().aggregate_labels); - bool should_aggregate = !disable_aggregation && !metric_family.metadata().aggregate_labels.empty(); + bool should_aggregate = enable_aggregation && !metric_family.metadata().aggregate_labels.empty(); metric_family.foreach_metric([&s, &out, &ctx, &found, &name, &metric_family, &aggregated_values, should_aggregate, show_help, &filter](const mi::metric_value& value, const mi::metric_info& value_info) mutable { s.clear(); s.str(""); @@ -788,12 +788,12 @@ future<> write_text_representation(output_stream& out, const config& ctx, }); } -future<> write_protobuf_representation(output_stream& out, const config& ctx, metric_family_range& m, bool disable_aggregation, std::function filter) { - return do_for_each(m, [&ctx, &out, disable_aggregation, filter=std::move(filter)](metric_family& metric_family) mutable { +future<> write_protobuf_representation(output_stream& out, const config& ctx, metric_family_range& m, bool enable_aggregation, std::function filter) { + return do_for_each(m, [&ctx, &out, enable_aggregation, filter=std::move(filter)](metric_family& metric_family) mutable { std::string s; google::protobuf::io::StringOutputStream os(&s); metric_aggregate_by_labels aggregated_values(metric_family.metadata().aggregate_labels); - bool should_aggregate = !disable_aggregation && !metric_family.metadata().aggregate_labels.empty(); + bool should_aggregate = enable_aggregation && !metric_family.metadata().aggregate_labels.empty(); auto& name = metric_family.name(); pm::MetricFamily mtf; bool empty_metric = true; @@ -896,16 +896,16 @@ class metrics_handler : public httpd::handler_base { sstring metric_family_name = req->get_query_param("__name__"); bool prefix = trim_asterisk(metric_family_name); bool show_help = req->get_query_param("__help__") != "false"; - bool disable_aggregation = req->get_query_param("__aggregate__") != "false"; + bool enable_aggregation = req->get_query_param("__aggregate__") != "false"; std::function filter = make_filter(*req); - rep->write_body(is_protobuf_format ? "proto" : "txt", [this, is_protobuf_format, metric_family_name, prefix, show_help, disable_aggregation, filter] (output_stream&& s) { + rep->write_body(is_protobuf_format ? "proto" : "txt", [this, is_protobuf_format, metric_family_name, prefix, show_help, enable_aggregation, filter] (output_stream&& s) { return do_with(metrics_families_per_shard(), output_stream(std::move(s)), - [this, is_protobuf_format, prefix, &metric_family_name, show_help, disable_aggregation, filter] (metrics_families_per_shard& families, output_stream& s) mutable { - return get_map_value(families).then([&s, &families, this, is_protobuf_format, prefix, &metric_family_name, show_help, disable_aggregation, filter]() mutable { + [this, is_protobuf_format, prefix, &metric_family_name, show_help, enable_aggregation, filter] (metrics_families_per_shard& families, output_stream& s) mutable { + return get_map_value(families).then([&s, &families, this, is_protobuf_format, prefix, &metric_family_name, show_help, enable_aggregation, filter]() mutable { return do_with(get_range(families, metric_family_name, prefix), - [&s, this, is_protobuf_format, show_help, disable_aggregation, filter](metric_family_range& m) { - return (is_protobuf_format) ? write_protobuf_representation(s, _ctx, m, disable_aggregation, filter) : - write_text_representation(s, _ctx, m, show_help, disable_aggregation, filter); + [&s, this, is_protobuf_format, show_help, enable_aggregation, filter](metric_family_range& m) { + return (is_protobuf_format) ? write_protobuf_representation(s, _ctx, m, enable_aggregation, filter) : + write_text_representation(s, _ctx, m, show_help, enable_aggregation, filter); }); }).finally([&s] () mutable { return s.close();