From b90c9f0f8d739e2c39c13480aa8f6c382ca7b222 Mon Sep 17 00:00:00 2001 From: Ioannis Kavvadias Date: Thu, 12 Dec 2024 13:43:09 +0000 Subject: [PATCH 1/2] pandaproxy: merge internal and public metrics --- src/v/pandaproxy/probe.cc | 162 ++++++++++++++++++++++---------------- src/v/pandaproxy/probe.h | 1 - 2 files changed, 93 insertions(+), 70 deletions(-) diff --git a/src/v/pandaproxy/probe.cc b/src/v/pandaproxy/probe.cc index a5dd7bb237d3..c73d027eb3d5 100644 --- a/src/v/pandaproxy/probe.cc +++ b/src/v/pandaproxy/probe.cc @@ -16,6 +16,7 @@ #include #include +#include namespace pandaproxy { @@ -26,82 +27,105 @@ probe::probe( , _group_name(group_name) , _metrics() { setup_metrics(); - setup_public_metrics(); } void probe::setup_metrics() { namespace sm = ss::metrics; - if (config::shard_local_cfg().disable_metrics()) { - return; + using is_internal = ss::bool_class; + + struct Labels { + sm::label_instance label; + std::vector agg; + sm::label status; + }; + const auto make_labels = [this](const is_internal internal) -> Labels { + const auto make_label = + [](const ss::sstring& key, const is_internal internal) { + return internal ? sm::label(key) + : metrics::make_namespaced_label(key); + }; + const auto operation_label = make_label("operation", internal); + const auto agg = internal ? std::vector{sm::shard_label} + : std::vector{ + sm::shard_label, operation_label}; + const auto status = make_label("status", internal); + return { + .label = operation_label(_path.operations.nickname), + .agg = agg, + .status = status}; + }; + + const auto internal_labels = make_labels(is_internal::yes); + const auto public_labels = make_labels(is_internal::no); + + const auto make_internal_request_latency = [this](const Labels& l) { + return sm::make_histogram( + "request_latency", + sm::description("Request latency"), + {l.label}, + [this] { + return _request_metrics.hist().internal_histogram_logform(); + }); + }; + + const auto make_public_request_latency = [this](const Labels& l) { + return sm::make_histogram( + "request_latency_seconds", + sm::description( + ssx::sformat("Internal latency of request for {}", _group_name)), + {l.label}, + [this] { + return _request_metrics.hist().public_histogram_logform(); + }); + }; + + const auto make_request_errors_total_5xx = [this](const Labels& l) { + return sm::make_counter( + "request_errors_total", + [this] { return _request_metrics._5xx_count; }, + sm::description( + ssx::sformat("Total number of {} server errors", _group_name)), + {l.label, l.status("5xx")}); + }; + + const auto make_request_errors_total_4xx = [this](const Labels& l) { + return sm::make_counter( + "request_errors_total", + [this] { return _request_metrics._4xx_count; }, + sm::description( + ssx::sformat("Total number of {} client errors", _group_name)), + {l.label, l.status("4xx")}); + }; + + const auto make_request_errors_total_3xx = [this](const Labels& l) { + return sm::make_counter( + "request_errors_total", + [this] { return _request_metrics._3xx_count; }, + sm::description( + ssx::sformat("Total number of {} redirection errors", _group_name)), + {l.label, l.status("3xx")}); + }; + + if (!config::shard_local_cfg().disable_metrics()) { + _metrics.add_group( + "pandaproxy", + {make_internal_request_latency(internal_labels)}, + {}, + internal_labels.agg); } - - auto operation_label = sm::label("operation"); - std::vector labels{ - operation_label(_path.operations.nickname)}; - - _metrics.add_group( - "pandaproxy", - {sm::make_histogram( - "request_latency", - sm::description("Request latency"), - labels, - [this] { - return _request_metrics.hist().internal_histogram_logform(); - })}, - {}, - {sm::shard_label}); -} - -void probe::setup_public_metrics() { - namespace sm = ss::metrics; - - if (config::shard_local_cfg().disable_public_metrics()) { - return; + if (!config::shard_local_cfg().disable_public_metrics()) { + _public_metrics.add_group( + _group_name, + {make_public_request_latency(public_labels) + .aggregate(public_labels.agg), + make_request_errors_total_5xx(public_labels) + .aggregate(public_labels.agg), + make_request_errors_total_4xx(public_labels) + .aggregate(public_labels.agg), + make_request_errors_total_3xx(public_labels) + .aggregate(public_labels.agg)}); } - - auto operation_label = metrics::make_namespaced_label("operation"); - auto status_label = metrics::make_namespaced_label("status"); - - std::vector labels{ - operation_label(_path.operations.nickname)}; - - auto aggregate_labels = std::vector{ - sm::shard_label, operation_label}; - - _public_metrics.add_group( - _group_name, - {sm::make_histogram( - "request_latency_seconds", - sm::description( - ssx::sformat("Internal latency of request for {}", _group_name)), - labels, - [this] { return _request_metrics.hist().public_histogram_logform(); }) - .aggregate(aggregate_labels), - - sm::make_counter( - "request_errors_total", - [this] { return _request_metrics._5xx_count; }, - sm::description( - ssx::sformat("Total number of {} server errors", _group_name)), - {operation_label(_path.operations.nickname), status_label("5xx")}) - .aggregate(aggregate_labels), - - sm::make_counter( - "request_errors_total", - [this] { return _request_metrics._4xx_count; }, - sm::description( - ssx::sformat("Total number of {} client errors", _group_name)), - {operation_label(_path.operations.nickname), status_label("4xx")}) - .aggregate(aggregate_labels), - - sm::make_counter( - "request_errors_total", - [this] { return _request_metrics._3xx_count; }, - sm::description( - ssx::sformat("Total number of {} redirection errors", _group_name)), - {operation_label(_path.operations.nickname), status_label("3xx")}) - .aggregate(aggregate_labels)}); } } // namespace pandaproxy diff --git a/src/v/pandaproxy/probe.h b/src/v/pandaproxy/probe.h index 70270b2b6708..9074dbccb4e8 100644 --- a/src/v/pandaproxy/probe.h +++ b/src/v/pandaproxy/probe.h @@ -72,7 +72,6 @@ class probe { private: void setup_metrics(); - void setup_public_metrics(); private: http_status_metric _request_metrics; From 939face46b0edc0824be83b5fa7ace01b17fe664 Mon Sep 17 00:00:00 2001 From: Ioannis Kavvadias Date: Thu, 12 Dec 2024 14:45:12 +0000 Subject: [PATCH 2/2] pandaproxy: add request_errors_total metric to interal metrics --- src/v/pandaproxy/probe.cc | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/v/pandaproxy/probe.cc b/src/v/pandaproxy/probe.cc index c73d027eb3d5..43f146198f93 100644 --- a/src/v/pandaproxy/probe.cc +++ b/src/v/pandaproxy/probe.cc @@ -110,7 +110,10 @@ void probe::setup_metrics() { if (!config::shard_local_cfg().disable_metrics()) { _metrics.add_group( "pandaproxy", - {make_internal_request_latency(internal_labels)}, + {make_internal_request_latency(internal_labels), + make_request_errors_total_5xx(internal_labels), + make_request_errors_total_4xx(internal_labels), + make_request_errors_total_3xx(internal_labels)}, {}, internal_labels.agg); }