This is an automated email from the ASF dual-hosted git repository.
mmerli pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/bookkeeper.git
The following commit(s) were added to refs/heads/master by this push:
new 61ddb9f fix prometheus metric provider bug and add test to cover
label scope …
61ddb9f is described below
commit 61ddb9f3469cc307ab39ac5d2fb946eed59c003b
Author: hangc0276 <[email protected]>
AuthorDate: Tue May 25 06:46:29 2021 +0800
fix prometheus metric provider bug and add test to cover label scope …
### Motivation
After add label for prometheus metric by
https://github.com/apache/bookkeeper/pull/2650, it will cause prometheus metric
format check failed when no label specified for a statsLogger. The metric list
as follow.
```
replication_bookkeeper_client_bookkeeper_client_bookie_watcher_NEW_ENSEMBLE_TIME{success="false",quantile="0.9999",
} NaN
```
### Modification
1. add label empty check for `PrometheusTextFormatUtil`
2. add label scope check test cover
3. add prometheus metric regex pattern check in test case
Reviewers: lipenghui <[email protected]>, Andrey Yegorov <None>, Matteo
Merli <[email protected]>, Jia Zhai <[email protected]>, Addison Higham
<None>, Enrico Olivelli <[email protected]>
This closes #2718 from hangc0276/chenhang/fix_bookeeper_metric_bug and
squashes the following commits:
8590704db [hangc0276] format code
a6942d49f [chenhang] fix prometheus metric provider bug and add test to
cover label scope and metric format check
bb8b1e0fe [Andrey Yegorov] Include gradle files into the source artifact
for releases, exclude site2/**
732b6cf2a [Andrey Yegorov] [maven-release-plugin] prepare for next
development iteration
b0d9f1058 [Andrey Yegorov] [maven-release-plugin] prepare branch branch-4.14
8dc108b86 [Matteo Merli] y
73e22cacf [Don Inghram] ISSUE2620: RocksDB log path configurable
034ef8566 [Shoothzj] Fix logger member not correct; (#2605)
b824a6029 [hangc0276] fix always select the same region set bug for
RegionAwareEnsemblePlacementPolicy (#2658)
683ad4592 [Matteo Merli] Allow to attach labels to metrics (#2650)
809109663 [Matteo Merli] Allow to bypass journal for writes (#2401)
63867a99b [Matteo Merli] Impose a memory limit on the bookie journal (#2710)
87579b0a9 [Matteo Merli] Read entry error should print lastAddConfirmed in
the log (#2707)
---
.../stats/prometheus/PrometheusTextFormatUtil.java | 25 +++++++---
.../stats/prometheus/TestPrometheusFormatter.java | 58 +++++++++++++++++++++-
2 files changed, 74 insertions(+), 9 deletions(-)
diff --git
a/bookkeeper-stats-providers/prometheus-metrics-provider/src/main/java/org/apache/bookkeeper/stats/prometheus/PrometheusTextFormatUtil.java
b/bookkeeper-stats-providers/prometheus-metrics-provider/src/main/java/org/apache/bookkeeper/stats/prometheus/PrometheusTextFormatUtil.java
index 330bcd6..1259f65 100644
---
a/bookkeeper-stats-providers/prometheus-metrics-provider/src/main/java/org/apache/bookkeeper/stats/prometheus/PrometheusTextFormatUtil.java
+++
b/bookkeeper-stats-providers/prometheus-metrics-provider/src/main/java/org/apache/bookkeeper/stats/prometheus/PrometheusTextFormatUtil.java
@@ -139,25 +139,34 @@ public class PrometheusTextFormatUtil {
w.append(name)
.append("{success=\"").append(success.toString())
.append("\",quantile=\"").append(Double.toString(quantile))
- .append("\", ");
- writeLabelsNoBraces(w, opStat.getLabels());
+ .append("\"");
+ if (!opStat.getLabels().isEmpty()) {
+ w.append(", ");
+ writeLabelsNoBraces(w, opStat.getLabels());
+ }
w.append("} ")
.append(Double.toString(opStat.getQuantileValue(success,
quantile))).append('\n');
}
private static void writeCount(Writer w, DataSketchesOpStatsLogger opStat,
String name, Boolean success)
throws IOException {
-
w.append(name).append("_count{success=\"").append(success.toString()).append("\",
");
- writeLabelsNoBraces(w, opStat.getLabels());
- w.append("\"} ")
+
w.append(name).append("_count{success=\"").append(success.toString()).append("\"");
+ if (!opStat.getLabels().isEmpty()) {
+ w.append(", ");
+ writeLabelsNoBraces(w, opStat.getLabels());
+ }
+ w.append("} ")
.append(Long.toString(opStat.getCount(success))).append('\n');
}
private static void writeSum(Writer w, DataSketchesOpStatsLogger opStat,
String name, Boolean success)
throws IOException {
-
w.append(name).append("_sum{success=\"").append(success.toString()).append("\",
");
- writeLabelsNoBraces(w, opStat.getLabels());
- w.append("\"} ")
+
w.append(name).append("_sum{success=\"").append(success.toString()).append("\"");
+ if (!opStat.getLabels().isEmpty()) {
+ w.append(", ");
+ writeLabelsNoBraces(w, opStat.getLabels());
+ }
+ w.append("} ")
.append(Double.toString(opStat.getSum(success))).append('\n');
}
diff --git
a/bookkeeper-stats-providers/prometheus-metrics-provider/src/test/java/org/apache/bookkeeper/stats/prometheus/TestPrometheusFormatter.java
b/bookkeeper-stats-providers/prometheus-metrics-provider/src/test/java/org/apache/bookkeeper/stats/prometheus/TestPrometheusFormatter.java
index f7b6ebe..9106e6a 100644
---
a/bookkeeper-stats-providers/prometheus-metrics-provider/src/test/java/org/apache/bookkeeper/stats/prometheus/TestPrometheusFormatter.java
+++
b/bookkeeper-stats-providers/prometheus-metrics-provider/src/test/java/org/apache/bookkeeper/stats/prometheus/TestPrometheusFormatter.java
@@ -43,7 +43,7 @@ import org.junit.Test;
*/
public class TestPrometheusFormatter {
- @Test
+ @Test(timeout = 30000)
public void testStatsOutput() throws Exception {
PrometheusMetricsProvider provider = new PrometheusMetricsProvider();
StatsLogger statsLogger = provider.getStatsLogger("test");
@@ -56,6 +56,12 @@ public class TestPrometheusFormatter {
opStats.registerSuccessfulEvent(10, TimeUnit.MILLISECONDS);
opStats.registerSuccessfulEvent(5, TimeUnit.MILLISECONDS);
+ OpStatsLogger opStats1 = statsLogger.scopeLabel("test_label",
"test_value")
+ .getOpStatsLogger("op_label");
+ opStats1.registerSuccessfulEvent(10, TimeUnit.MILLISECONDS);
+ opStats1.registerSuccessfulEvent(5, TimeUnit.MILLISECONDS);
+ opStats1.registerFailedEvent(1, TimeUnit.MILLISECONDS);
+
provider.rotateLatencyCollection();
StringWriter writer = new StringWriter();
@@ -112,6 +118,52 @@ public class TestPrometheusFormatter {
}
assertTrue(found);
+
+ // test_op_label_sum
+ cm = (List<Metric>) metrics.get("test_op_label_sum");
+ assertEquals(2, cm.size());
+ m = cm.get(0);
+ assertEquals(2, m.tags.size());
+ assertEquals(1.0, m.value, 0.0);
+ assertEquals("false", m.tags.get("success"));
+ assertEquals("test_value", m.tags.get("test_label"));
+
+ m = cm.get(1);
+ assertEquals(15.0, m.value, 0.0);
+ assertEquals(2, m.tags.size());
+ assertEquals("true", m.tags.get("success"));
+ assertEquals("test_value", m.tags.get("test_label"));
+
+ // test_op_label_count
+ cm = (List<Metric>) metrics.get("test_op_label_count");
+ assertEquals(2, cm.size());
+ m = cm.get(0);
+ assertEquals(1, m.value, 0.0);
+ assertEquals(2, m.tags.size());
+ assertEquals("false", m.tags.get("success"));
+ assertEquals("test_value", m.tags.get("test_label"));
+
+ m = cm.get(1);
+ assertEquals(2.0, m.value, 0.0);
+ assertEquals(2, m.tags.size());
+ assertEquals("true", m.tags.get("success"));
+ assertEquals("test_value", m.tags.get("test_label"));
+
+ // Latency
+ cm = (List<Metric>) metrics.get("test_op_label");
+ assertEquals(14, cm.size());
+
+ found = false;
+ for (Metric mt : cm) {
+ if ("true".equals(mt.tags.get("success"))
+ && "test_value".equals(mt.tags.get("test_label"))
+ && "1.0".equals(mt.tags.get("quantile"))) {
+ assertEquals(10.0, mt.value, 0.0);
+ found = true;
+ }
+ }
+
+ assertTrue(found);
}
/**
@@ -126,6 +178,8 @@ public class TestPrometheusFormatter {
// pulsar_subscriptions_count{cluster="standalone",
namespace="sample/standalone/ns1",
// topic="persistent://sample/standalone/ns1/test-2"} 0.0 1517945780897
Pattern pattern =
Pattern.compile("^(\\w+)(\\{([^\\}]+)\\})?\\s(-?[\\d\\w\\.]+)(\\s(\\d+))?$");
+ Pattern formatPattern =
Pattern.compile("^(\\w+)(\\{(\\w+=[\\\"\\.\\w]+(,\\s?\\w+=[\\\"\\.\\w]+)*)\\})?"
+ + "\\s(-?[\\d\\w\\.]+)(\\s(\\d+))?$");
Pattern tagsPattern = Pattern.compile("(\\w+)=\"([^\"]+)\"(,\\s?)?");
Splitter.on("\n").split(metrics).forEach(line -> {
@@ -135,6 +189,7 @@ public class TestPrometheusFormatter {
System.err.println("LINE: '" + line + "'");
Matcher matcher = pattern.matcher(line);
+ Matcher formatMatcher = formatPattern.matcher(line);
System.err.println("Matches: " + matcher.matches());
System.err.println(matcher);
@@ -144,6 +199,7 @@ public class TestPrometheusFormatter {
}
checkArgument(matcher.matches());
+ checkArgument(formatMatcher.matches());
String name = matcher.group(1);
Metric m = new Metric();
