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

alexey pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git


The following commit(s) were added to refs/heads/master by this push:
     new 036c22968 KUDU-3563 Output tablet-level metrics in Prometheus
036c22968 is described below

commit 036c22968bc8725e191259865f793efa7097d30b
Author: Ádám Bakai <[email protected]>
AuthorDate: Wed Jan 8 13:40:18 2025 +0100

    KUDU-3563 Output tablet-level metrics in Prometheus
    
    Newline characters have been removed from tablet metric entity
    descriptions because they produce invalid Prometheus format.
    
    Change-Id: Id033a1410966167d9bb24f6a44c584e73c17a4af
    Reviewed-on: http://gerrit.cloudera.org:8080/22314
    Tested-by: Alexey Serbin <[email protected]>
    Reviewed-by: Alexey Serbin <[email protected]>
---
 src/kudu/master/master-test.cc         | 18 ++++++++++
 src/kudu/tablet/tablet_metrics.cc      |  8 ++---
 src/kudu/tserver/tablet_server-test.cc | 10 ++++++
 src/kudu/util/metrics-test.cc          | 61 +++++++++++++++++++++++++++++++++-
 src/kudu/util/metrics.cc               | 40 ++++++++++------------
 src/kudu/util/test_util.cc             | 37 +++++++++++++++++++++
 src/kudu/util/test_util.h              |  3 ++
 7 files changed, 149 insertions(+), 28 deletions(-)

diff --git a/src/kudu/master/master-test.cc b/src/kudu/master/master-test.cc
index fd58a0b75..a87dbc6e9 100644
--- a/src/kudu/master/master-test.cc
+++ b/src/kudu/master/master-test.cc
@@ -3834,5 +3834,23 @@ TEST_F(MasterTest, TestMasterContentTypeHeaders) {
   }
 }
 
+// Smoke test that Prometheus Metrics output is correct.
+TEST_F(MasterTest, SmokeTestPrometheusMetrics) {
+  constexpr char kTableName[] = "testtb";
+  const Schema kTableSchema({ ColumnSchema("key", INT32),
+                             ColumnSchema("v1", UINT64),
+                             ColumnSchema("v2", STRING) },
+                            1);
+
+  ASSERT_OK(CreateTable(kTableName, kTableSchema));
+
+  EasyCurl c;
+  faststring buf;
+  ASSERT_OK(c.FetchURL(Substitute("http://$0/metrics_prometheus";,
+                                  mini_master_->bound_http_addr().ToString()),
+                       &buf));
+  CheckPrometheusOutput(buf.ToString());
+}
+
 } // namespace master
 } // namespace kudu
diff --git a/src/kudu/tablet/tablet_metrics.cc 
b/src/kudu/tablet/tablet_metrics.cc
index 783618af0..3d5a216c7 100644
--- a/src/kudu/tablet/tablet_metrics.cc
+++ b/src/kudu/tablet/tablet_metrics.cc
@@ -126,9 +126,9 @@ METRIC_DEFINE_counter(tablet, 
scanner_cells_scanned_from_disk, "Scanner Cells Sc
                       "as a raw count prior to application of predicates, 
deleted data,"
                       "or MVCC-based filtering. Thus, this is a better measure 
of actual "
                       "table cells that have been processed by scan operations 
compared "
-                      "to the Scanner Cells Returned metric.\n"
+                      "to the Scanner Cells Returned metric. "
                       "Note that this only counts data that has been flushed 
to disk, "
-                      "and does not include data read from in-memory stores. 
However, it"
+                      "and does not include data read from in-memory stores. 
However, it "
                       "includes both cache misses and cache hits.",
                       kudu::MetricLevel::kDebug);
 
@@ -138,9 +138,9 @@ METRIC_DEFINE_counter(tablet, 
scanner_bytes_scanned_from_disk, "Scanner Bytes Sc
                       "as a raw count prior to application of predicates, 
deleted data,"
                       "or MVCC-based filtering. Thus, this is a better measure 
of actual "
                       "IO that has been caused by scan operations compared "
-                      "to the Scanner Bytes Returned metric.\n"
+                      "to the Scanner Bytes Returned metric. "
                       "Note that this only counts data that has been flushed 
to disk, "
-                      "and does not include data read from in-memory stores. 
However, it"
+                      "and does not include data read from in-memory stores. 
However, it "
                       "includes both cache misses and cache hits.",
                       kudu::MetricLevel::kDebug);
 
diff --git a/src/kudu/tserver/tablet_server-test.cc 
b/src/kudu/tserver/tablet_server-test.cc
index fd11086a8..c0d5f2a97 100644
--- a/src/kudu/tserver/tablet_server-test.cc
+++ b/src/kudu/tserver/tablet_server-test.cc
@@ -4351,6 +4351,16 @@ TEST_F(TabletServerTest, ServerAttributes) {
   ASSERT_STR_CONTAINS(raw, "\"hostname\": \"" + server_hostname + "\"");
 }
 
+// Smoke test that Prometheus Metrics output is correct.
+TEST_F(TabletServerTest, SmokeTestPrometheusMetrics) {
+  EasyCurl c;
+  faststring buf;
+  ASSERT_OK(c.FetchURL(Substitute("http://$0/metrics_prometheus";,
+                                  mini_server_->bound_http_addr().ToString()),
+                       &buf));
+  CheckPrometheusOutput(buf.ToString());
+}
+
 // Test that hostname is set properly for TabletServer's Messenger.
 TEST_F(TabletServerTest, ServerHostname) {
   string server_hostname;
diff --git a/src/kudu/util/metrics-test.cc b/src/kudu/util/metrics-test.cc
index 550bfcdac..0f442dcec 100644
--- a/src/kudu/util/metrics-test.cc
+++ b/src/kudu/util/metrics-test.cc
@@ -61,6 +61,21 @@ namespace kudu {
 
 METRIC_DEFINE_entity(test_entity);
 
+METRIC_DEFINE_entity(tablet);
+METRIC_DEFINE_counter(tablet, tablet_test_counter,
+                      "Tablet-wise test counter label",
+                      kudu::MetricUnit::kBytes,
+                      "Tablet-wise test counter description.",
+                      kudu::MetricLevel::kDebug);
+
+METRIC_DEFINE_entity(table);
+METRIC_DEFINE_counter(table, table_test_counter,
+                      "Table-wise test counter label",
+                      kudu::MetricUnit::kBytes,
+                      "Table-wise test counter description.",
+                      kudu::MetricLevel::kDebug);
+
+
 class MetricsTest : public KuduTest {
  public:
   void SetUp() override {
@@ -133,6 +148,50 @@ TEST_F(MetricsTest, ResetCounter) {
   ASSERT_EQ(0, c->value());
 }
 
+TEST_F(MetricsTest, TableAndTabletPrometheusTest) {
+  // Simulate two tablets in the metric registry. Write out their metric data.
+  auto tablet_metric_entity =
+      METRIC_ENTITY_tablet.Instantiate(&registry_, 
"00000000000000000000000000000000");
+  scoped_refptr<Counter> tablet_counter =
+      METRIC_tablet_test_counter.Instantiate(tablet_metric_entity);
+
+  auto tablet_metric_entity2 =
+      METRIC_ENTITY_tablet.Instantiate(&registry_, 
"11111111111111111111111111111111");
+  scoped_refptr<Counter> tablet_counter2 =
+      METRIC_tablet_test_counter.Instantiate(tablet_metric_entity2);
+
+  auto table_metric_entity = METRIC_ENTITY_table.Instantiate(&registry_, 
"table_name1");
+  scoped_refptr<Counter> table_counter = 
METRIC_table_test_counter.Instantiate(table_metric_entity);
+
+  auto table_metric_entity2 = METRIC_ENTITY_table.Instantiate(&registry_, 
"table_name2");
+  scoped_refptr<Counter> table_counter2 =
+      METRIC_table_test_counter.Instantiate(table_metric_entity2);
+
+  ostringstream output;
+  PrometheusWriter writer(&output);
+  ASSERT_OK(registry_.WriteAsPrometheus(&writer));
+  ASSERT_EQ(
+      "# HELP kudu_table_table_name2_table_test_counter Table-wise test 
counter "
+      "description.\n"
+      "# TYPE kudu_table_table_name2_table_test_counter counter\n"
+      "kudu_table_table_name2_table_test_counter{unit_type=\"bytes\"} 0\n"
+      "# HELP kudu_tablet_00000000000000000000000000000000_tablet_test_counter 
Tablet-wise "
+      "test counter description.\n"
+      "# TYPE kudu_tablet_00000000000000000000000000000000_tablet_test_counter 
counter\n"
+      
"kudu_tablet_00000000000000000000000000000000_tablet_test_counter{unit_type=\"bytes\"}"
+      " 0\n"
+      "# HELP kudu_tablet_11111111111111111111111111111111_tablet_test_counter 
Tablet-wise "
+      "test counter description.\n"
+      "# TYPE kudu_tablet_11111111111111111111111111111111_tablet_test_counter 
counter\n"
+      
"kudu_tablet_11111111111111111111111111111111_tablet_test_counter{unit_type=\"bytes\"}"
+      " 0\n"
+      "# HELP kudu_table_table_name1_table_test_counter Table-wise test 
counter "
+      "description.\n"
+      "# TYPE kudu_table_table_name1_table_test_counter counter\n"
+      "kudu_table_table_name1_table_test_counter{unit_type=\"bytes\"} 0\n",
+      output.str());
+}
+
 TEST_F(MetricsTest, CounterPrometheusTest) {
   scoped_refptr<Counter> requests(new Counter(&METRIC_test_counter));
 
@@ -977,7 +1036,7 @@ TEST_F(MetricsTest, TestDumpJsonPrototypes) {
   int num_entities = d["entities"].Size();
   LOG(INFO) << "Parsed " << num_metrics << " metrics and " << num_entities << 
" entities";
   ASSERT_GT(num_metrics, 5);
-  ASSERT_EQ(num_entities, 2);
+  ASSERT_EQ(num_entities, 4);
 
   // Spot-check that some metrics were properly registered and that the JSON 
was properly
   // formed.
diff --git a/src/kudu/util/metrics.cc b/src/kudu/util/metrics.cc
index d94b039c6..1715004af 100644
--- a/src/kudu/util/metrics.cc
+++ b/src/kudu/util/metrics.cc
@@ -408,14 +408,6 @@ Status MetricEntity::WriteAsPrometheus(PrometheusWriter* 
writer) const {
   static const string kIdMaster = "kudu.master";
   static const string kIdTabletServer = "kudu.tabletserver";
 
-  if (strcmp(prototype_->name(), "server") != 0) {
-    // Only server-level metrics are emitted in Prometheus format as of now,
-    // non-server metric entities are currently silently skipped.
-    //
-    // TODO(KUDU-3563): output tablet-level metrics in Prometheus format as 
well
-    return Status::OK();
-  }
-
   // Empty filters result in getting all the metrics for this MetricEntity.
   //
   // TODO(aserbin): instead of hard-coding, pass MetricFilters as a parameter
@@ -431,22 +423,24 @@ Status MetricEntity::WriteAsPrometheus(PrometheusWriter* 
writer) const {
     return Status::OK();
   }
   RETURN_NOT_OK(s);
-
-  if (id_ == kIdMaster) {
-    // Prefix all master metrics with 'kudu_master_'.
-    static const string kMasterPrefix = "kudu_master_";
-    WriteMetricsPrometheus(writer, metrics, kMasterPrefix);
-    return Status::OK();
-  }
-  if (id_ == kIdTabletServer) {
-    // Prefix all tablet server metrics with 'kudu_tserver_'.
-    static const string kTabletServerPrefix = "kudu_tserver_";
-    WriteMetricsPrometheus(writer, metrics, kTabletServerPrefix);
-    return Status::OK();
+  if (strcmp(prototype_->name(), "server") == 0) {
+    if (id_ == kIdMaster) {
+      // Prefix all master metrics with 'kudu_master_'.
+      static const string kMasterPrefix = "kudu_master_";
+      WriteMetricsPrometheus(writer, metrics, kMasterPrefix);
+      return Status::OK();
+    }
+    if (id_ == kIdTabletServer) {
+      // Prefix all tablet server metrics with 'kudu_tserver_'.
+      static const string kTabletServerPrefix = "kudu_tserver_";
+      WriteMetricsPrometheus(writer, metrics, kTabletServerPrefix);
+      return Status::OK();
+    }
+    return Status::NotSupported(Substitute("$0: unexpected server-level metric 
entity", id_));
   }
-
-  return Status::NotSupported(
-      Substitute("$0: unexpected server-level metric entity", id_));
+  const string prefix = Substitute("kudu_$0_$1_", prototype_->name(), id_);
+  WriteMetricsPrometheus(writer, metrics, prefix);
+  return Status::OK();
 }
 
 Status MetricEntity::CollectTo(MergedEntityMetrics* collections,
diff --git a/src/kudu/util/test_util.cc b/src/kudu/util/test_util.cc
index 72b3465b1..84a644655 100644
--- a/src/kudu/util/test_util.cc
+++ b/src/kudu/util/test_util.cc
@@ -28,6 +28,7 @@
 #include <memory>
 #include <ostream>
 #include <string>
+#include <unordered_set>
 #include <utility>
 #include <vector>
 
@@ -60,6 +61,7 @@
 #include "kudu/util/status.h"
 #include "kudu/util/string_case.h"
 #include "kudu/util/subprocess.h"
+#include "kudu/util/test_macros.h"
 
 DEFINE_string(test_leave_files, "on_failure",
               "Whether to leave test files around after the test run. "
@@ -721,4 +723,39 @@ const unordered_map<string, string>& 
GetMasterWebserverEndpoints(const string& t
   return master_endpoints;
 }
 
+void CheckPrometheusOutput(const string& prometheus_output) {
+  vector<string> lines = strings::Split(prometheus_output, "\n", 
strings::SkipEmpty());
+  vector<vector<string>> metric_groups;
+  // Split the lines into groups. Every group contains a help line, a type 
line and
+  // then lines with the actual metric values in this order.
+  for (const auto& line : lines) {
+    if (HasPrefixString(line, "# HELP")) {
+      metric_groups.push_back({line});
+    } else if (HasPrefixString(line, "# TYPE")) {
+      metric_groups.back().push_back(line);
+    } else {
+      metric_groups.back().push_back(line);
+    }
+  }
+
+  std::unordered_set<string> metric_names;
+  for (const auto& group : metric_groups) {
+    ASSERT_GE(group.size(), 3);
+    ASSERT_STR_MATCHES(group[0], "^# HELP ");
+    ASSERT_STR_MATCHES(group[1], "^# TYPE ");
+    vector<string> help_line_split(strings::Split(group[0], " "));
+    vector<string> type_line_split(strings::Split(group[1], " "));
+    ASSERT_GE(help_line_split.size(), 3);
+    ASSERT_GE(type_line_split.size(), 3);
+    string name_from_help_line = help_line_split[2];
+    string name_from_type_line = type_line_split[2];
+    ASSERT_EQ(name_from_type_line, name_from_help_line);
+    ASSERT_TRUE(metric_names.emplace(name_from_help_line).second)
+        << "Duplicate metric: " << name_from_help_line;
+    for (int i = 2; i < group.size(); i++) {
+      ASSERT_TRUE(HasPrefixString(group[i], name_from_help_line))
+          << "Every line should start with the expected metric name: " << 
name_from_help_line;
+    }
+  }
+}
 } // namespace kudu
diff --git a/src/kudu/util/test_util.h b/src/kudu/util/test_util.h
index 33071112b..4f882e7ab 100644
--- a/src/kudu/util/test_util.h
+++ b/src/kudu/util/test_util.h
@@ -216,5 +216,8 @@ const std::unordered_map<std::string, std::string>& 
GetTServerWebserverEndpoints
 const std::unordered_map<std::string, std::string>& 
GetMasterWebserverEndpoints(
     const std::string& table_id);
 
+// Prometheus metrics output sanity check. This check is used in both the 
tablet_server-test
+// and the master-test, so it is placed here.
+void CheckPrometheusOutput(const std::string& prometheus_output);
 } // namespace kudu
 #endif

Reply via email to