(kudu) branch branch-1.17.x updated: [test] a small clean-up on StringGaugePrometheusTest
This is an automated email from the ASF dual-hosted git repository.
alexey pushed a commit to branch branch-1.17.x
in repository https://gitbox.apache.org/repos/asf/kudu.git
The following commit(s) were added to refs/heads/branch-1.17.x by this push:
new b18f36ccf [test] a small clean-up on StringGaugePrometheusTest
b18f36ccf is described below
commit b18f36ccf823da552ee9272fd36852fb7d74498b
Author: Alexey Serbin
AuthorDate: Thu Feb 1 17:49:09 2024 -0800
[test] a small clean-up on StringGaugePrometheusTest
I took a quick look at KUDU-3549, and a natural first step was to check
whether we have a proper test coverage for handling StringGauge in when
dumping metrics in Prometheus format. The result was this patch, adding
a couple new sub-scenarios into MetricsTest.StringGaugePrometheusTest.
While this didn't lead to discovering the root case of the reported
issue, the new sub-scenarios are still useful since they provide extra
coverage for the related functionality. I also took the liberty of
cleaning up the code a bit.
Change-Id: Ie495f3bae91facd98e9ce61ee86f34dea8b08087
Reviewed-on: http://gerrit.cloudera.org:8080/20984
Reviewed-by: Yingchun Lai
Tested-by: Alexey Serbin
(cherry picked from commit ce8ef84d9cf1c4aca8edd07920dfef274edb8df4)
Reviewed-on: http://gerrit.cloudera.org:8080/21006
---
src/kudu/util/metrics-test.cc | 76 ---
1 file changed, 43 insertions(+), 33 deletions(-)
diff --git a/src/kudu/util/metrics-test.cc b/src/kudu/util/metrics-test.cc
index 2ab2d15d3..2f3bc6a75 100644
--- a/src/kudu/util/metrics-test.cc
+++ b/src/kudu/util/metrics-test.cc
@@ -47,6 +47,7 @@
#include "kudu/util/test_macros.h"
#include "kudu/util/test_util.h"
+using std::ostringstream;
using std::string;
using std::unordered_map;
using std::unordered_set;
@@ -133,13 +134,11 @@ TEST_F(MetricsTest, ResetCounter) {
}
TEST_F(MetricsTest, CounterPrometheusTest) {
- scoped_refptr requests =
-new Counter(&METRIC_test_counter);
+ scoped_refptr requests(new Counter(&METRIC_test_counter));
- std::ostringstream output;
- string prefix;
+ ostringstream output;
PrometheusWriter writer(&output);
- ASSERT_OK(requests->WriteAsPrometheus(&writer, prefix));
+ ASSERT_OK(requests->WriteAsPrometheus(&writer, {}));
const string expected_output = "# HELP test_counter Description of test
counter\n"
"# TYPE test_counter counter\n"
@@ -205,14 +204,30 @@ TEST_F(MetricsTest, SimpleStringGaugeForMergeTest) {
TEST_F(MetricsTest, StringGaugePrometheusTest) {
scoped_refptr state =
-new StringGauge(&METRIC_test_string_gauge, "Healthy");
+ new StringGauge(&METRIC_test_string_gauge, "Healthy");
- std::ostringstream output;
- string prefix;
+ ostringstream output;
PrometheusWriter writer(&output);
- ASSERT_OK(state->WriteAsPrometheus(&writer, prefix));
+ ASSERT_OK(state->WriteAsPrometheus(&writer, {}));
// String Gauges are not representable in Prometheus, empty output is
expected
ASSERT_EQ("", output.str());
+
+ // Make sure proper methods are called when relying on method overrides.
+ {
+const Metric* g = state.get();
+ostringstream output;
+PrometheusWriter writer(&output);
+ASSERT_OK(g->WriteAsPrometheus(&writer, {}));
+// String Gauges are not representable in Prometheus, empty output is
expected
+ASSERT_EQ("", output.str());
+ }
+ {
+const Metric* m = state.get();
+ostringstream output;
+PrometheusWriter writer(&output);
+ASSERT_OK(m->WriteAsPrometheus(&writer, {}));
+ASSERT_EQ("", output.str());
+ }
}
METRIC_DEFINE_gauge_double(test_entity, test_mean_gauge, "Test mean Gauge",
@@ -268,9 +283,9 @@ TEST_F(MetricsTest, TestMeanGaugeJsonPrint) {
const double kTotalCount = 2.0;
test_meangauge->set_value(kTotalSum, kTotalCount);
- std::ostringstream out;
+ ostringstream out;
JsonWriter writer(&out, JsonWriter::PRETTY);
- CHECK_OK(entity_->WriteAsJson(&writer, MetricJsonOptions()));
+ ASSERT_OK(entity_->WriteAsJson(&writer, {}));
JsonReader reader(out.str());
ASSERT_OK(reader.Init());
@@ -294,10 +309,9 @@ TEST_F(MetricsTest, MeanGaugePrometheusTest) {
scoped_refptr average_usage =
METRIC_test_mean_gauge.InstantiateMeanGauge(entity_);
- std::ostringstream output;
- string prefix;
+ ostringstream output;
PrometheusWriter writer(&output);
- ASSERT_OK(average_usage->WriteAsPrometheus(&writer, prefix));
+ ASSERT_OK(average_usage->WriteAsPrometheus(&writer, {}));
const string expected_output = "# HELP test_mean_gauge Description of mean
Gauge\n"
"# TYPE test_mean_gauge gauge\n"
@@ -376,13 +390,12 @@ TEST_F(MetricsTest, SimpleAtomicGaugeMinTypeMergeTest) {
}
TEST_F(MetricsTest, AtomicGaugePrometheusTest) {
- scoped_refptr > mem_usage =
-METRIC_test_gauge.Instantiate(entity_, 0);
+ scoped_refptr> mem
(kudu) branch master updated: KUDU-3549 fix WriteAsPrometheus() for non-arithmetic gauges
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 6b1c1eb0c KUDU-3549 fix WriteAsPrometheus() for non-arithmetic gauges
6b1c1eb0c is described below
commit 6b1c1eb0c97a2349e0b3fa098bf40f8147b43a60
Author: Alexey Serbin
AuthorDate: Fri Feb 2 00:08:39 2024 -0800
KUDU-3549 fix WriteAsPrometheus() for non-arithmetic gauges
This patch fixes WriteAsPrometheus() implementation for string-based
gauges. The original changelist that introduced Prometheus metrics
had a proper implementation of WriteAsPrometheus() only for StringGauge,
but any FunctionGauge for a non-arithmetic type (e.g., for std::string)
would still output a string value in Prometheus text format, and that
could not be consumed by Prometheus. The patch also contains a test
scenario that would fail without the fix.
This is a follow-up to 00efc6826ac9a1f5d10750296c7357790a041fec.
Change-Id: Ib7128f52729c7f984004811153a7eecc8ffe751b
Reviewed-on: http://gerrit.cloudera.org:8080/20990
Tested-by: Alexey Serbin
Reviewed-by: Marton Greber
Reviewed-by: Attila Bukor
---
src/kudu/util/metrics-test.cc | 28 --
src/kudu/util/metrics.cc | 39 +---
src/kudu/util/metrics.h | 86 ---
3 files changed, 90 insertions(+), 63 deletions(-)
diff --git a/src/kudu/util/metrics-test.cc b/src/kudu/util/metrics-test.cc
index 2f3bc6a75..8d598ebc5 100644
--- a/src/kudu/util/metrics-test.cc
+++ b/src/kudu/util/metrics-test.cc
@@ -202,23 +202,22 @@ TEST_F(MetricsTest, SimpleStringGaugeForMergeTest) {
state_for_merge->unique_values());
}
-TEST_F(MetricsTest, StringGaugePrometheusTest) {
+TEST_F(MetricsTest, StringGaugeForPrometheus) {
scoped_refptr state =
new StringGauge(&METRIC_test_string_gauge, "Healthy");
ostringstream output;
PrometheusWriter writer(&output);
ASSERT_OK(state->WriteAsPrometheus(&writer, {}));
- // String Gauges are not representable in Prometheus, empty output is
expected
+ // String-based gauges are not consumable by Prometheus.
ASSERT_EQ("", output.str());
- // Make sure proper methods are called when relying on method overrides.
+ // Test that proper methods are called when relying on virtual overrides.
{
const Metric* g = state.get();
ostringstream output;
PrometheusWriter writer(&output);
ASSERT_OK(g->WriteAsPrometheus(&writer, {}));
-// String Gauges are not representable in Prometheus, empty output is
expected
ASSERT_EQ("", output.str());
}
{
@@ -230,6 +229,27 @@ TEST_F(MetricsTest, StringGaugePrometheusTest) {
}
}
+METRIC_DEFINE_gauge_string(test_entity,
+ string_function_gauge,
+ "String Function Gauge",
+ MetricUnit::kState,
+ "Description of string function gauge",
+ kudu::MetricLevel::kInfo);
+
+TEST_F(MetricsTest, StringFunctionGaugeForPrometheus) {
+ scoped_refptr> gauge =
+ METRIC_string_function_gauge.InstantiateFunctionGauge(
+ entity_, []() { return "string function gauge"; });
+ FunctionGaugeDetacher detacher;
+ gauge->AutoDetachToLastValue(&detacher);
+
+ ostringstream output;
+ PrometheusWriter writer(&output);
+ ASSERT_OK(gauge->WriteAsPrometheus(&writer, {}));
+ // String-based gauges are not consumable by Prometheus.
+ ASSERT_EQ("", output.str());
+}
+
METRIC_DEFINE_gauge_double(test_entity, test_mean_gauge, "Test mean Gauge",
MetricUnit::kUnits, "Description of mean Gauge",
kudu::MetricLevel::kInfo);
diff --git a/src/kudu/util/metrics.cc b/src/kudu/util/metrics.cc
index 22cf785f2..dfcea6db1 100644
--- a/src/kudu/util/metrics.cc
+++ b/src/kudu/util/metrics.cc
@@ -17,6 +17,7 @@
#include "kudu/util/metrics.h"
#include
+#include
#include
#include
@@ -69,13 +70,13 @@ void WriteMetricsToJson(JsonWriter* writer,
writer->EndArray();
}
-template
-void WriteMetricsToPrometheus(PrometheusWriter* writer,
- const Collection& metrics,
- const string& prefix) {
+void WriteMetricsPrometheus(PrometheusWriter* writer,
+const MetricEntity::MetricMap& metrics,
+const string& prefix) {
for (const auto& [name, val] : metrics) {
WARN_NOT_OK(val->WriteAsPrometheus(writer, prefix),
-Substitute("Failed to write $0 as Prometheus", name));
+Substitute("unable to write '$0' ($1) in Prometheus format",
+ name, val->prototype()->description()));
}
}
@@ -424,12 +425,12 @@ Status MetricEntity::WriteAsPrometheus(Prometheus
