From 54d8ed39166f53bde7a77a9f5cfce3ab9dac88c9 Mon Sep 17 00:00:00 2001 From: Avan Thakkar Date: Sat, 23 Mar 2024 03:22:53 +0530 Subject: [PATCH] exporter: fix regex for rgw sync metrics Fixes: https://tracker.ceph.com/issues/65091 Signed-off-by: Avan Thakkar (cherry picked from commit d9d8cbeed5cb92a15727795ac5f63338e93d47a3) --- src/exporter/DaemonMetricCollector.cc | 50 +++++++++++--------- src/exporter/DaemonMetricCollector.h | 11 +++-- src/test/exporter/test_exporter.cc | 68 +++++++++++++++++++++++++++ 3 files changed, 103 insertions(+), 26 deletions(-) diff --git a/src/exporter/DaemonMetricCollector.cc b/src/exporter/DaemonMetricCollector.cc index 959423343e5be..3f95605da97dd 100644 --- a/src/exporter/DaemonMetricCollector.cc +++ b/src/exporter/DaemonMetricCollector.cc @@ -32,7 +32,9 @@ void DaemonMetricCollector::request_loop(boost::asio::steady_timer &timer) { timer.async_wait([&](const boost::system::error_code &e) { std::cerr << e << std::endl; update_sockets(); - dump_asok_metrics(); + std::string dump_response; + std::string schema_response; + dump_asok_metrics(false, -1, true, dump_response, schema_response, true); auto stats_period = g_conf().get_val("exporter_stats_period"); // time to wait before sending requests again timer.expires_from_now(std::chrono::seconds(stats_period)); @@ -140,15 +142,17 @@ void DaemonMetricCollector::parse_asok_metrics( } } - - -void DaemonMetricCollector::dump_asok_metrics() { - BlockTimer timer(__FILE__, __FUNCTION__); +void DaemonMetricCollector::dump_asok_metrics(bool sort_metrics, int64_t counter_prio, + bool sockClientsPing, std::string &dump_response, + std::string &schema_response, + bool config_show_response) { + BlockTimer timer(__FILE__, __FUNCTION__); std::vector> daemon_pids; int failures = 0; - bool sort = g_conf().get_val("exporter_sort_metrics"); + bool sort; + sort = sort_metrics ? true : g_conf().get_val("exporter_sort_metrics"); if (sort) { builder = std::unique_ptr(new OrderedMetricsBuilder()); @@ -156,29 +160,32 @@ void DaemonMetricCollector::dump_asok_metrics() { builder = std::unique_ptr(new UnorderedMetricsBuilder()); } - auto prio_limit = g_conf().get_val("exporter_prio_limit"); + auto prio_limit = counter_prio >=0 ? counter_prio : g_conf().get_val("exporter_prio_limit"); for (auto &[daemon_name, sock_client] : clients) { - bool ok; - sock_client.ping(&ok); - if (!ok) { - failures++; - continue; + if (sockClientsPing) { + bool ok; + sock_client.ping(&ok); + if (!ok) { + failures++; + continue; + } } - std::string counter_dump_response = + std::string counter_dump_response = dump_response.size() > 0 ? dump_response : asok_request(sock_client, "counter dump", daemon_name); if (counter_dump_response.size() == 0) { failures++; continue; } - std::string counter_schema_response = + std::string counter_schema_response = schema_response.size() > 0 ? schema_response : asok_request(sock_client, "counter schema", daemon_name); if (counter_schema_response.size() == 0) { failures++; continue; } - try { + parse_asok_metrics(counter_dump_response, counter_schema_response, + prio_limit, daemon_name); std::string config_show = asok_request(sock_client, "config show", daemon_name); if (config_show.size() == 0) { failures++; @@ -195,8 +202,6 @@ void DaemonMetricCollector::dump_asok_metrics() { if (!pid_str.empty()) { daemon_pids.push_back({daemon_name, std::stoi(pid_str)}); } - parse_asok_metrics(counter_dump_response, counter_schema_response, - prio_limit, daemon_name); } catch (const std::invalid_argument &e) { failures++; dout(1) << "failed to handle " << daemon_name << ": " << e.what() @@ -360,13 +365,14 @@ DaemonMetricCollector::add_fixed_name_metrics(std::string metric_name) { labels_t labels; new_metric_name = metric_name; - std::regex re("^data_sync_from_(.*)\\."); - std::smatch match; - if (std::regex_search(metric_name, match, re) == true) { - new_metric_name = std::regex_replace(metric_name, re, "from_([^.]*)', 'from_zone"); + std::regex re("data_sync_from_([^_]*)"); + std::smatch match; + if (std::regex_search(metric_name, match, re)) { + new_metric_name = std::regex_replace(metric_name, re, "data_sync_from_zone"); labels["source_zone"] = quote(match.str(1)); return {labels, new_metric_name}; - } + } + return {}; } diff --git a/src/exporter/DaemonMetricCollector.h b/src/exporter/DaemonMetricCollector.h index 16caa79a20eb2..159786dd7e330 100644 --- a/src/exporter/DaemonMetricCollector.h +++ b/src/exporter/DaemonMetricCollector.h @@ -35,20 +35,23 @@ public: void main(); std::string get_metrics(); labels_t get_extra_labels(std::string daemon_name); - -private: + void dump_asok_metrics(bool sort_metrics, int64_t counter_prio, + bool sockClientsPing, std::string &dump_response, + std::string &schema_response, + bool config_show_response); std::map clients; std::string metrics; + std::pair add_fixed_name_metrics(std::string metric_name); + +private: std::mutex metrics_mutex; std::unique_ptr builder; void update_sockets(); void request_loop(boost::asio::steady_timer &timer); - void dump_asok_metrics(); void dump_asok_metric(boost::json::object perf_info, boost::json::value perf_values, std::string name, labels_t labels); - std::pair add_fixed_name_metrics(std::string metric_name); void parse_asok_metrics(std::string &counter_dump_response, std::string &counter_schema_response, int64_t prio_limit, const std::string &daemon_name); diff --git a/src/test/exporter/test_exporter.cc b/src/test/exporter/test_exporter.cc index b607d1afffbed..373eb60bce30e 100644 --- a/src/test/exporter/test_exporter.cc +++ b/src/test/exporter/test_exporter.cc @@ -1,12 +1,29 @@ +#include "common/ceph_argparse.h" +#include "common/config.h" +#include "common/config_proxy.h" +#include #include "gtest/gtest.h" +#include "common/ceph_context.h" +#include "global/global_context.h" +#include "global/global_init.h" #include "exporter/util.h" #include "exporter/DaemonMetricCollector.h" +#include #include #include #include typedef std::map labels_t; +using ::testing::DoAll; +using ::testing::Return; +using ::testing::Pointee; +using ::testing::Matcher; +using ::testing::_; +using ::testing::SetArgReferee; +using ::testing::Invoke; +using ::testing::WithArgs; +using ::testing::AtLeast; // 17.2.6's memento mori: // This data was gathered from the python implementation of the promethize method @@ -657,6 +674,23 @@ static std::vector> promethize_data = { {"rocksdb.submit_sync_latency_sum", "ceph_rocksdb_submit_sync_latency_sum"} }; +int main(int argc, char **argv) +{ + ::testing::InitGoogleTest(&argc, argv); + + auto args = argv_to_vec(argc, argv); + + auto cct = global_init(nullptr, args, CEPH_ENTITY_TYPE_CLIENT, + CODE_ENVIRONMENT_UTILITY, CINIT_FLAG_NO_MON_CONFIG); + + g_conf().set_val("exporter_sort_metrics", "true"); + g_conf().set_val("exporter_prio_limit", "5"); + common_init_finish(g_ceph_context); + + int r = RUN_ALL_TESTS(); + return r; +} + TEST(Exporter, promethize) { for (auto &test_case : promethize_data) { std::string path = test_case.first; @@ -692,3 +726,37 @@ TEST(Exporter, check_labels_and_metric_name) { // So no labels should be added. ASSERT_TRUE(fail_result.empty()); } + +TEST(Exporter, add_fixed_name_metrics) { + std::vector metrics = { + "ceph_data_sync_from_zone2-zg1-realm1_fetch_bytes", + "ceph_data_sync_from_zone2-zg1-realm1_fetch_bytes_sum", + "ceph_data_sync_from_zone2-zg1-realm1_poll_latency_sum", + }; + std::vector expected_metrics = { + "ceph_data_sync_from_zone_fetch_bytes", + "ceph_data_sync_from_zone_fetch_bytes_sum", + "ceph_data_sync_from_zone_poll_latency_sum", + }; + std::string metric_name; + std::pair new_metric; + labels_t expected_labels; + std::string expected_metric_name; + for (std::size_t index = 0; index < metrics.size(); ++index) { + std::string &metric_name = metrics[index]; + DaemonMetricCollector &collector = collector_instance(); + auto new_metric = collector.add_fixed_name_metrics(metric_name); + expected_labels = {{"source_zone", "\"zone2-zg1-realm1\""}}; + std::string expected_metric_name = expected_metrics[index]; + EXPECT_EQ(new_metric.first, expected_labels); + ASSERT_EQ(new_metric.second, expected_metric_name); + } + + metric_name = "ceph_data_sync_from_zone2_fetch_bytes_count"; + DaemonMetricCollector &collector = collector_instance(); + new_metric = collector.add_fixed_name_metrics(metric_name); + expected_labels = {{"source_zone", "\"zone2\""}}; + expected_metric_name = "ceph_data_sync_from_zone_fetch_bytes_count"; + EXPECT_EQ(new_metric.first, expected_labels); + ASSERT_TRUE(new_metric.second == expected_metric_name); +} -- 2.39.5