From 2bac3c75d196bcb8eff805c7b66a74fd0eaf7fb4 Mon Sep 17 00:00:00 2001 From: avanthakkar Date: Tue, 19 Sep 2023 13:48:43 +0530 Subject: [PATCH] exporter: add ceph_daemon labels to labeled counters as well Exporter missed adding the `ceph_daemon` or `instance_id` labels(in case if rgw metrics) to the new labeled performance counters. Fixes: https://tracker.ceph.com/issues/62874 Signed-off-by: avanthakkar (cherry picked from commit f061955f0beabcaa3dca0dbfb6a8c8e56c764a0b) --- src/exporter/DaemonMetricCollector.cc | 36 +++++++-------------------- src/exporter/DaemonMetricCollector.h | 3 +-- src/test/exporter/test_exporter.cc | 22 ++++++++-------- 3 files changed, 20 insertions(+), 41 deletions(-) diff --git a/src/exporter/DaemonMetricCollector.cc b/src/exporter/DaemonMetricCollector.cc index 464ec6fdc83d1..ebe85c3041e5a 100644 --- a/src/exporter/DaemonMetricCollector.cc +++ b/src/exporter/DaemonMetricCollector.cc @@ -147,15 +147,13 @@ void DaemonMetricCollector::dump_asok_metrics() { std::string counter_name = perf_group + "_" + counter_name_init; promethize(counter_name); - if (counters_labels.empty()) { - auto labels_and_name = get_labels_and_metric_name(daemon_name, counter_name); - if (labels_and_name.first.empty()) { - dout(1) << "Unable to parse instance_id from daemon_name: " << daemon_name << dendl; - continue; - } - labels = labels_and_name.first; - counter_name = labels_and_name.second; + auto extra_labels = get_extra_labels(daemon_name); + if (extra_labels.empty()) { + dout(1) << "Unable to parse instance_id from daemon_name: " << daemon_name << dendl; + continue; } + labels.insert(extra_labels.begin(), extra_labels.end()); + // For now this is only required for rgw multi-site metrics auto multisite_labels_and_name = add_fixed_name_metrics(counter_name); if (!multisite_labels_and_name.first.empty()) { @@ -289,12 +287,8 @@ std::string DaemonMetricCollector::asok_request(AdminSocketClient &asok, return response; } -std::pair -DaemonMetricCollector::get_labels_and_metric_name(std::string daemon_name, - std::string metric_name) { - std::string new_metric_name; +labels_t DaemonMetricCollector::get_extra_labels(std::string daemon_name) { labels_t labels; - new_metric_name = metric_name; const std::string ceph_daemon_prefix = "ceph-"; const std::string ceph_client_prefix = "client."; if (daemon_name.rfind(ceph_daemon_prefix, 0) == 0) { @@ -321,24 +315,12 @@ DaemonMetricCollector::get_labels_and_metric_name(std::string daemon_name, if (elems.size() >= 4) { labels["instance_id"] = quote(elems[3]); } else { - return std::make_pair(labels_t(), ""); + return labels_t(); } } else { labels.insert({"ceph_daemon", quote(daemon_name)}); } - if (daemon_name.find("rbd-mirror") != std::string::npos) { - std::regex re( - "^rbd_mirror_image_([^/]+)/(?:(?:([^/]+)/" - ")?)(.*)\\.(replay(?:_bytes|_latency)?)$"); - std::smatch match; - if (std::regex_search(daemon_name, match, re) == true) { - new_metric_name = "ceph_rbd_mirror_image_" + match.str(4); - labels["pool"] = quote(match.str(1)); - labels["namespace"] = quote(match.str(2)); - labels["image"] = quote(match.str(3)); - } - } - return {labels, new_metric_name}; + return labels; } // Add fixed name metrics from existing ones that have details in their names diff --git a/src/exporter/DaemonMetricCollector.h b/src/exporter/DaemonMetricCollector.h index 88e827bddae70..e906fb13a5970 100644 --- a/src/exporter/DaemonMetricCollector.h +++ b/src/exporter/DaemonMetricCollector.h @@ -34,8 +34,7 @@ class DaemonMetricCollector { public: void main(); std::string get_metrics(); - std::pair - get_labels_and_metric_name(std::string daemon_name, std::string metric_name); + labels_t get_extra_labels(std::string daemon_name); private: std::map clients; diff --git a/src/test/exporter/test_exporter.cc b/src/test/exporter/test_exporter.cc index 03b2db917603d..b607d1afffbed 100644 --- a/src/test/exporter/test_exporter.cc +++ b/src/test/exporter/test_exporter.cc @@ -670,27 +670,25 @@ TEST(Exporter, check_labels_and_metric_name) { counters_data.emplace_back("ceph-osd.0", "ceph_osd_numpg"); counters_data.emplace_back("ceph-client.rgw.foo.ceph-node-00.hrgsea.2.94739968030880", "ceph_rgw_get"); - static std::vector> labels_and_name; - labels_and_name.emplace_back(labels_t{{"ceph_daemon", "\"osd.0\""}}, "ceph_osd_numpg"); - labels_and_name.emplace_back(labels_t{{"instance_id", "\"hrgsea\""}}, "ceph_rgw_get"); + static std::vector labels_vec; + labels_vec.emplace_back(labels_t{{"ceph_daemon", "\"osd.0\""}}); + labels_vec.emplace_back(labels_t{{"instance_id", "\"hrgsea\""}}); auto counter_data_itr = counters_data.begin(); - auto labels_and_name_itr = labels_and_name.begin(); - for (; counter_data_itr != counters_data.end() && labels_and_name_itr != labels_and_name.end(); - ++counter_data_itr, ++labels_and_name_itr) { + auto labels_vec_itr = labels_vec.begin(); + for (; counter_data_itr != counters_data.end() && labels_vec_itr != labels_vec.end(); + ++counter_data_itr, ++labels_vec_itr) { std::string daemon_name = counter_data_itr->first; std::string counter_name = counter_data_itr->second; DaemonMetricCollector &collector = collector_instance(); - std::pair result = collector.get_labels_and_metric_name(daemon_name, counter_name); - ASSERT_EQ(result.first, labels_and_name_itr->first); - ASSERT_EQ(result.second, labels_and_name_itr->second); + labels_t result = collector.get_extra_labels(daemon_name); + ASSERT_EQ(result, *labels_vec_itr); } // test for fail case with daemon_name.size() < 4 std::string short_daemon_name = "ceph-client.rgw.foo"; std::string counter_name = "ceph_rgw_get"; DaemonMetricCollector &collector = collector_instance(); - std::pair fail_result = collector.get_labels_and_metric_name(short_daemon_name, counter_name); + labels_t fail_result = collector.get_extra_labels(short_daemon_name); // This is a special case, the daemon name is not of the required size for fetching instance_id. // So no labels should be added. - ASSERT_TRUE(fail_result.first.empty()); - ASSERT_TRUE(fail_result.second.empty()); + ASSERT_TRUE(fail_result.empty()); } -- 2.39.5