Skip to content

Commit

Permalink
fix prometheus metric provider bug and add test to cover label scope …
Browse files Browse the repository at this point in the history
### Motivation
After add label for prometheus metric by apache#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 apache#2718 from hangc0276/chenhang/fix_bookeeper_metric_bug and squashes the following commits:

8590704 [hangc0276] format code
a6942d4 [chenhang] fix prometheus metric provider bug and add test to cover label scope and metric format check
bb8b1e0 [Andrey Yegorov] Include gradle files into the source artifact for releases, exclude site2/**
732b6cf [Andrey Yegorov] [maven-release-plugin] prepare for next development iteration
b0d9f10 [Andrey Yegorov] [maven-release-plugin] prepare branch branch-4.14
8dc108b [Matteo Merli] y
73e22ca [Don Inghram] ISSUE2620: RocksDB log path configurable
034ef85 [Shoothzj] Fix logger member not correct; (apache#2605)
b824a60 [hangc0276] fix always select the same region set bug for RegionAwareEnsemblePlacementPolicy (apache#2658)
683ad45 [Matteo Merli] Allow to attach labels to metrics (apache#2650)
8091096 [Matteo Merli] Allow to bypass journal for writes (apache#2401)
63867a9 [Matteo Merli] Impose a memory limit on the bookie journal (apache#2710)
87579b0 [Matteo Merli] Read entry error should print lastAddConfirmed in the log (apache#2707)
  • Loading branch information
hangc0276 authored and merlimat committed May 29, 2021
1 parent 12f0f5f commit 4292db8
Show file tree
Hide file tree
Showing 2 changed files with 74 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -139,25 +139,34 @@ private static void writeQuantile(Writer w, DataSketchesOpStatsLogger opStat, St
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');
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
*/
public class TestPrometheusFormatter {

@Test
@Test(timeout = 30000)
public void testStatsOutput() throws Exception {
PrometheusMetricsProvider provider = new PrometheusMetricsProvider();
StatsLogger statsLogger = provider.getStatsLogger("test");
Expand All @@ -56,6 +56,12 @@ public void testStatsOutput() throws Exception {
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();
Expand Down Expand Up @@ -112,6 +118,52 @@ public void testStatsOutput() throws Exception {
}

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);
}

/**
Expand All @@ -126,6 +178,8 @@ private static Multimap<String, Metric> parseMetrics(String metrics) {
// 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 -> {
Expand All @@ -135,6 +189,7 @@ private static Multimap<String, Metric> parseMetrics(String metrics) {

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);

Expand All @@ -144,6 +199,7 @@ private static Multimap<String, Metric> parseMetrics(String metrics) {
}

checkArgument(matcher.matches());
checkArgument(formatMatcher.matches());
String name = matcher.group(1);

Metric m = new Metric();
Expand Down

0 comments on commit 4292db8

Please sign in to comment.