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(®istry_,
"00000000000000000000000000000000");
+ scoped_refptr<Counter> tablet_counter =
+ METRIC_tablet_test_counter.Instantiate(tablet_metric_entity);
+
+ auto tablet_metric_entity2 =
+ METRIC_ENTITY_tablet.Instantiate(®istry_,
"11111111111111111111111111111111");
+ scoped_refptr<Counter> tablet_counter2 =
+ METRIC_tablet_test_counter.Instantiate(tablet_metric_entity2);
+
+ auto table_metric_entity = METRIC_ENTITY_table.Instantiate(®istry_,
"table_name1");
+ scoped_refptr<Counter> table_counter =
METRIC_table_test_counter.Instantiate(table_metric_entity);
+
+ auto table_metric_entity2 = METRIC_ENTITY_table.Instantiate(®istry_,
"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