You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by la...@apache.org on 2020/08/15 14:59:38 UTC

[kudu] 10/23: [collector] fix metrics filter and merge url for version 1.11

This is an automated email from the ASF dual-hosted git repository.

laiyingchun pushed a commit to tag kudu-1.12.0-mdh1.0.0-4c2c075-centos-release
in repository https://gitbox.apache.org/repos/asf/kudu.git

commit 82b0fb4d3526fd0ff08125290d6325293e291fd9
Author: Yingchun Lai <40...@qq.com>
AuthorDate: Tue Dec 31 11:50:04 2019 +0800

    [collector] fix metrics filter and merge url for version 1.11
---
 src/kudu/collector/metrics_collector-test.cc | 21 ++++++++++-----------
 src/kudu/collector/metrics_collector.cc      | 19 +------------------
 src/kudu/collector/metrics_collector.h       |  1 -
 3 files changed, 11 insertions(+), 30 deletions(-)

diff --git a/src/kudu/collector/metrics_collector-test.cc b/src/kudu/collector/metrics_collector-test.cc
index 4a3e050..19c00cf 100644
--- a/src/kudu/collector/metrics_collector-test.cc
+++ b/src/kudu/collector/metrics_collector-test.cc
@@ -41,7 +41,6 @@ DECLARE_bool(collector_request_merged_metrics);
 DECLARE_string(collector_attributes);
 DECLARE_string(collector_cluster_level_metrics);
 DECLARE_string(collector_metrics);
-DECLARE_string(collector_table_names);
 DECLARE_string(collector_metrics_types_for_test);
 
 using std::map;
@@ -735,12 +734,11 @@ TEST(TestMetricsCollector, TestInitFilters) {
   ASSERT_EQ(collector->attributes_filter_, expect_attributes_filter);
 }
 
-#define CHECK_URL_PARAMETERS(metrics, request_merged, attributes, table_names, expect_url)        \
+#define CHECK_URL_PARAMETERS(metrics, request_merged, attributes, expect_url)                     \
 do {                                                                                              \
   FLAGS_collector_metrics = metrics;                                                              \
   FLAGS_collector_request_merged_metrics = request_merged;                                        \
   FLAGS_collector_attributes = attributes;                                                        \
-  FLAGS_collector_table_names = table_names;                                                      \
   auto collector = BuildCollector();                                                              \
   ASSERT_OK(collector->InitFilters());                                                            \
   ASSERT_OK(collector->InitMetricsUrlParameters());                                               \
@@ -748,17 +746,18 @@ do {
 } while (false)
 
 TEST(TestMetricsCollector, TestInitMetricsUrlParameters) {
-  CHECK_URL_PARAMETERS("", true, "", "",
-      "/metrics?compact=1&origin=false&merge=true");
-  CHECK_URL_PARAMETERS("m1,m2,m3", true, "", "",
-      "/metrics?compact=1&metrics=m1,m2,m3&origin=false&merge=true");
+  CHECK_URL_PARAMETERS("", true, "",
+      "/metrics?compact=1&merge_rules=tablet|table|table_name");
+  CHECK_URL_PARAMETERS("m1,m2,m3", true, "",
+      "/metrics?compact=1&metrics=m1,m2,m3&merge_rules=tablet|table|table_name");
   // TODO(yingchun): now FLAGS_collector_request_merged_metrics must be true
   //CHECK_URL_PARAMETERS("", false, "", "",
   //    "/metrics?compact=1");
-  CHECK_URL_PARAMETERS("", true, "attr1:a1,a2;attr2:a3", "",
-      "/metrics?compact=1&origin=false&merge=true&attributes=attr2,a3,attr1,a1,attr1,a2,");
-  CHECK_URL_PARAMETERS("", true, "", "t1,t2,t3",
-      "/metrics?compact=1&origin=false&merge=true&table_names=t1,t2,t3");
+  CHECK_URL_PARAMETERS("", true, "attr1:a1,a2;attr2:a3",
+      "/metrics?compact=1&merge_rules=tablet|table|table_name"
+      "&attributes=attr2,a3,attr1,a1,attr1,a2,");
+  CHECK_URL_PARAMETERS("", true, "",
+      "/metrics?compact=1&merge_rules=tablet|table|table_name");
 }
 
 TEST(TestMetricsCollector, TestInitClusterLevelMetrics) {
diff --git a/src/kudu/collector/metrics_collector.cc b/src/kudu/collector/metrics_collector.cc
index e5adfcb..688ea70 100644
--- a/src/kudu/collector/metrics_collector.cc
+++ b/src/kudu/collector/metrics_collector.cc
@@ -67,8 +67,6 @@ DEFINE_string(collector_metrics_types_for_test, "",
               "Only for test, used to initialize metric_types_by_entity_type_");
 DEFINE_bool(collector_request_merged_metrics, true,
             "Whether to request merged metrics and exclude unmerged metrics from server");
-DEFINE_string(collector_table_names, "",
-              "Table names to collect (comma-separated list of table names)");
 
 DECLARE_string(collector_cluster_name);
 DECLARE_int32(collector_interval_sec);
@@ -104,10 +102,8 @@ MetricsCollector::~MetricsCollector() {
 Status MetricsCollector::Init() {
   CHECK(!initialized_);
 
-  RETURN_NOT_OK(ValidateTableFilter(FLAGS_collector_attributes, FLAGS_collector_table_names));
   RETURN_NOT_OK(InitMetrics());
   RETURN_NOT_OK(InitFilters());
-  CHECK(attributes_filter_.empty());  // TODO(yingchun) disable now
   RETURN_NOT_OK(InitMetricsUrlParameters());
   RETURN_NOT_OK(InitClusterLevelMetrics());
 
@@ -178,15 +174,6 @@ Status MetricsCollector::UpdateThreadPool(int32_t thread_count) {
   return Status::OK();
 }
 
-Status MetricsCollector::ValidateTableFilter(const string& attribute_filter,
-                                             const string& /*table_filter*/) {
-  if (attribute_filter.empty()) {
-    return Status::OK();
-  }
-
-  return Status::InvalidArgument("attribute filter is not supported now");
-}
-
 Status MetricsCollector::InitMetrics() {
   string resp;
   if (PREDICT_TRUE(FLAGS_collector_metrics_types_for_test.empty())) {
@@ -266,7 +253,7 @@ Status MetricsCollector::InitMetricsUrlParameters() {
     metric_url_parameters_ += "&metrics=" + FLAGS_collector_metrics;
   }
   if (FLAGS_collector_request_merged_metrics) {
-    metric_url_parameters_ += "&origin=false&merge=true";
+    metric_url_parameters_ += "&merge_rules=tablet|table|table_name";
   } else {
     LOG(FATAL) << "Non-merge mode is not supported now, you should set "
                   "FLAGS_collector_request_merged_metrics to true if you "
@@ -282,10 +269,6 @@ Status MetricsCollector::InitMetricsUrlParameters() {
       metric_url_parameters_ += Substitute("$0,$1,", attribute_filter.first, value);
     }
   }
-  // TODO(yingchun) This is supported since internal version 1.8.0
-  if (!FLAGS_collector_table_names.empty()) {
-    metric_url_parameters_ += "&table_names=" + FLAGS_collector_table_names;
-  }
   return Status::OK();
 }
 
diff --git a/src/kudu/collector/metrics_collector.h b/src/kudu/collector/metrics_collector.h
index 1435e52..05673ef 100644
--- a/src/kudu/collector/metrics_collector.h
+++ b/src/kudu/collector/metrics_collector.h
@@ -92,7 +92,6 @@ class MetricsCollector : public RefCounted<MetricsCollector> {
 
   typedef std::unordered_map<std::string, std::string> MetricTypes;
 
-  Status ValidateTableFilter(const std::string& attribute_filter, const std::string& table_filter);
   Status InitMetrics();
   static Status ExtractMetricTypes(const JsonReader& r,
                                    const rapidjson::Value* entity,