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 <ale...@apache.org>
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 <ale...@apache.org>
Reviewed-by: Marton Greber <greber...@gmail.com>
Reviewed-by: Attila Bukor <abu...@apache.org>
---
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<StringGauge> 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<FunctionGauge<string>> 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 <iostream>
+#include <tuple>
#include <utility>
#include <gflags/gflags.h>
@@ -69,13 +70,13 @@ void WriteMetricsToJson(JsonWriter* writer,
writer->EndArray();
}
-template<typename Collection>
-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(PrometheusWriter*
writer) const {
if (strcmp(prototype_->name(), "server") == 0) {
if (id_ == master_server) {
// attach kudu_master_ as prefix to metrics
- WriteMetricsToPrometheus(writer, metrics, master_prefix);
+ WriteMetricsPrometheus(writer, metrics, master_prefix);
return Status::OK();
}
if (id_ == tablet_server) {
// attach kudu_tserver_ as prefix to metrics
- WriteMetricsToPrometheus(writer, metrics, tserver_prefix);
+ WriteMetricsPrometheus(writer, metrics, tserver_prefix);
return Status::OK();
}
}
@@ -735,7 +736,8 @@ void MetricPrototype::WriteFields(JsonWriter* writer,
}
}
-void MetricPrototype::WriteFields(PrometheusWriter* writer, const string
&prefix) const {
+void MetricPrototype::WriteHelpAndType(PrometheusWriter* writer,
+ const string& prefix) const {
writer->WriteEntry(Substitute("# HELP $0$1 $2\n# TYPE $3$4 $5\n",
prefix, name(), description(),
prefix, name(), MetricType::Name(type())));
@@ -819,8 +821,7 @@ Status Gauge::WriteAsJson(JsonWriter* writer,
}
Status Gauge::WriteAsPrometheus(PrometheusWriter* writer, const string&
prefix) const {
- prototype_->WriteFields(writer, prefix);
-
+ prototype_->WriteHelpAndType(writer, prefix);
WriteValue(writer, prefix);
return Status::OK();
@@ -1031,11 +1032,9 @@ Status Counter::WriteAsJson(JsonWriter* writer,
}
Status Counter::WriteAsPrometheus(PrometheusWriter* writer, const string&
prefix) const {
- prototype_->WriteFields(writer, prefix);
-
+ prototype_->WriteHelpAndType(writer, prefix);
writer->WriteEntry(Substitute("$0$1{unit_type=\"$2\"} $3\n", prefix,
prototype_->name(),
MetricUnit::Name(prototype_->unit()),
value()));
-
return Status::OK();
}
@@ -1096,19 +1095,16 @@ Status Histogram::WriteAsJson(JsonWriter* writer,
}
Status Histogram::WriteAsPrometheus(PrometheusWriter* writer, const string&
prefix) const {
- prototype_->WriteFields(writer, prefix);
- string output = "";
- MetricJsonOptions opts;
// Snapshot is taken to preserve the consistency across metrics and
// requirements given by Prometheus. The value for the _bucket in +Inf
// quantile needs to be equal to the total _count
HistogramSnapshotPB snapshot;
- RETURN_NOT_OK(GetHistogramSnapshotPB(&snapshot, opts));
+ RETURN_NOT_OK(GetHistogramSnapshotPB(&snapshot, {}));
- output = Substitute("$0$1$2{unit_type=\"$3\", le=\"0.75\"} $4\n",
- prefix, prototype_->name(), "_bucket",
- MetricUnit::Name(prototype_->unit()),
- snapshot.percentile_75());
+ auto output = Substitute("$0$1$2{unit_type=\"$3\", le=\"0.75\"} $4\n",
+ prefix, prototype_->name(), "_bucket",
+ MetricUnit::Name(prototype_->unit()),
+ snapshot.percentile_75());
SubstituteAndAppend(&output, "$0$1$2{unit_type=\"$3\", le=\"0.95\"} $4\n",
prefix, prototype_->name(), "_bucket",
@@ -1145,6 +1141,7 @@ Status Histogram::WriteAsPrometheus(PrometheusWriter*
writer, const string& pref
MetricUnit::Name(prototype_->unit()),
snapshot.total_count());
+ prototype_->WriteHelpAndType(writer, prefix);
writer->WriteEntry(output);
return Status::OK();
diff --git a/src/kudu/util/metrics.h b/src/kudu/util/metrics.h
index e4b6b27c5..7efc7e471 100644
--- a/src/kudu/util/metrics.h
+++ b/src/kudu/util/metrics.h
@@ -583,8 +583,9 @@ class MetricPrototype {
void WriteFields(JsonWriter* writer,
const MetricJsonOptions& opts) const;
- void WriteFields(PrometheusWriter* writer, const std::string& prefix) const;
-
+ // Write TYPE and HELP meta-info for a metric in Prometheus format.
+ virtual void WriteHelpAndType(PrometheusWriter* writer,
+ const std::string& prefix) const;
protected:
explicit MetricPrototype(CtorArgs args);
virtual ~MetricPrototype() = default;
@@ -1013,6 +1014,14 @@ class GaugePrototype : public MetricPrototype {
return MetricType::kGauge;
}
+ void WriteHelpAndType(PrometheusWriter* writer,
+ const std::string& prefix) const override {
+ if constexpr (std::is_arithmetic_v<T>) {
+ // Non-arithmetic gauges aren't supported by Prometheus.
+ MetricPrototype::WriteHelpAndType(writer, prefix);
+ }
+ }
+
private:
DISALLOW_COPY_AND_ASSIGN(GaugePrototype);
};
@@ -1048,11 +1057,10 @@ class StringGauge : public Gauge {
return false;
}
void MergeFrom(const scoped_refptr<Metric>& other) override;
-
+ Status WriteAsPrometheus(PrometheusWriter* w, const std::string& prefix)
const override;
protected:
FRIEND_TEST(MetricsTest, SimpleStringGaugeForMergeTest);
- FRIEND_TEST(MetricsTest, StringGaugePrometheusTest);
- Status WriteAsPrometheus(PrometheusWriter* w, const std::string& prefix)
const override;
+ FRIEND_TEST(MetricsTest, StringGaugeForPrometheus);
void WriteValue(JsonWriter* writer) const override;
void WriteValue(PrometheusWriter* writer, const std::string& prefix) const
override;
void FillUniqueValuesUnlocked();
@@ -1092,6 +1100,30 @@ class MeanGauge : public Gauge {
DISALLOW_COPY_AND_ASSIGN(MeanGauge);
};
+// A helper function for writing a gauge's value in Prometheus format.
+template<typename T>
+void WriteValuePrometheus(PrometheusWriter* writer,
+ const std::string& prefix,
+ const char* proto_name,
+ const char* unit_name,
+ const T& value) {
+ static constexpr const char* const kFmt = "$0$1{unit_type=\"$2\"} $3\n";
+
+ if constexpr (!std::is_arithmetic_v<T>) {
+ // Non-arithmetic gauges aren't supported by Prometheus.
+ return;
+ }
+
+ // For a boolean gauge, convert false/true to 0/1 for Prometheus.
+ if constexpr (std::is_same_v<T, bool>) {
+ return writer->WriteEntry(
+ strings::Substitute(kFmt, prefix, proto_name, unit_name, value ? 1 :
0));
+ } else {
+ return writer->WriteEntry(
+ strings::Substitute(kFmt, prefix, proto_name, unit_name, value));
+ }
+}
+
// Lock-free implementation for types that are convertible to/from int64_t.
template <typename T>
class AtomicGauge : public Gauge {
@@ -1162,23 +1194,11 @@ class AtomicGauge : public Gauge {
}
void WriteValue(PrometheusWriter* writer,const std::string& prefix) const
override {
- std::string output = "";
-
- // If Boolean Gauge, convert false/true to 0/1 for Prometheus
- if constexpr (std::is_same_v<T, bool> ) {
- int check = 0;
- if (value() == true) {
- check = 1;
- } else {
- check = 0;
- }
- output = strings::Substitute("$0$1{unit_type=\"$2\"} $3\n", prefix,
prototype_->name(),
- MetricUnit::Name(prototype_->unit()),
check);
- } else {
- output = strings::Substitute("$0$1{unit_type=\"$2\"} $3\n", prefix,
prototype_->name(),
- MetricUnit::Name(prototype_->unit()),
value());
- }
- writer->WriteEntry(output);
+ return WriteValuePrometheus(writer,
+ prefix,
+ prototype_->name(),
+ MetricUnit::Name(prototype_->unit()),
+ value());
}
private:
AtomicInt<int64_t> value_;
@@ -1273,23 +1293,13 @@ class FunctionGauge : public Gauge {
}
void WriteValue(PrometheusWriter* writer, const std::string& prefix) const
override {
- std::string output = "";
- // If Boolean Gauge, convert false/true to 0/1 for Prometheus
- if constexpr (std::is_same_v<T, bool> ) {
- int check = 0;
- if (value() == true) {
- check = 1;
- } else {
- check = 0;
- }
- output = strings::Substitute("$0$1{unit_type=\"$2\"} $3\n", prefix,
prototype_->name(),
- MetricUnit::Name(prototype_->unit()),
check);
- } else {
- output = strings::Substitute("$0$1{unit_type=\"$2\"} $3\n", prefix,
prototype_->name(),
- MetricUnit::Name(prototype_->unit()),
value());
- }
- writer->WriteEntry(output);
+ return WriteValuePrometheus(writer,
+ prefix,
+ prototype_->name(),
+ MetricUnit::Name(prototype_->unit()),
+ value());
}
+
// Reset this FunctionGauge to return a specific value.
// This should be used during destruction. If you want a settable
// Gauge, use a normal Gauge instead of a FunctionGauge.
