Skip to content

Commit 4292db8

Browse files
hangc0276merlimat
authored andcommitted
fix prometheus metric provider bug and add test to cover label scope …
### 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 <penghui@apache.org>, Andrey Yegorov <None>, Matteo Merli <mmerli@apache.org>, Jia Zhai <zhaijia@apache.org>, Addison Higham <None>, Enrico Olivelli <eolivelli@gmail.com> 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)
1 parent 12f0f5f commit 4292db8

File tree

2 files changed

+74
-9
lines changed

2 files changed

+74
-9
lines changed

bookkeeper-stats-providers/prometheus-metrics-provider/src/main/java/org/apache/bookkeeper/stats/prometheus/PrometheusTextFormatUtil.java

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -139,25 +139,34 @@ private static void writeQuantile(Writer w, DataSketchesOpStatsLogger opStat, St
139139
w.append(name)
140140
.append("{success=\"").append(success.toString())
141141
.append("\",quantile=\"").append(Double.toString(quantile))
142-
.append("\", ");
143-
writeLabelsNoBraces(w, opStat.getLabels());
142+
.append("\"");
143+
if (!opStat.getLabels().isEmpty()) {
144+
w.append(", ");
145+
writeLabelsNoBraces(w, opStat.getLabels());
146+
}
144147
w.append("} ")
145148
.append(Double.toString(opStat.getQuantileValue(success, quantile))).append('\n');
146149
}
147150

148151
private static void writeCount(Writer w, DataSketchesOpStatsLogger opStat, String name, Boolean success)
149152
throws IOException {
150-
w.append(name).append("_count{success=\"").append(success.toString()).append("\", ");
151-
writeLabelsNoBraces(w, opStat.getLabels());
152-
w.append("\"} ")
153+
w.append(name).append("_count{success=\"").append(success.toString()).append("\"");
154+
if (!opStat.getLabels().isEmpty()) {
155+
w.append(", ");
156+
writeLabelsNoBraces(w, opStat.getLabels());
157+
}
158+
w.append("} ")
153159
.append(Long.toString(opStat.getCount(success))).append('\n');
154160
}
155161

156162
private static void writeSum(Writer w, DataSketchesOpStatsLogger opStat, String name, Boolean success)
157163
throws IOException {
158-
w.append(name).append("_sum{success=\"").append(success.toString()).append("\", ");
159-
writeLabelsNoBraces(w, opStat.getLabels());
160-
w.append("\"} ")
164+
w.append(name).append("_sum{success=\"").append(success.toString()).append("\"");
165+
if (!opStat.getLabels().isEmpty()) {
166+
w.append(", ");
167+
writeLabelsNoBraces(w, opStat.getLabels());
168+
}
169+
w.append("} ")
161170
.append(Double.toString(opStat.getSum(success))).append('\n');
162171
}
163172

bookkeeper-stats-providers/prometheus-metrics-provider/src/test/java/org/apache/bookkeeper/stats/prometheus/TestPrometheusFormatter.java

Lines changed: 57 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@
4343
*/
4444
public class TestPrometheusFormatter {
4545

46-
@Test
46+
@Test(timeout = 30000)
4747
public void testStatsOutput() throws Exception {
4848
PrometheusMetricsProvider provider = new PrometheusMetricsProvider();
4949
StatsLogger statsLogger = provider.getStatsLogger("test");
@@ -56,6 +56,12 @@ public void testStatsOutput() throws Exception {
5656
opStats.registerSuccessfulEvent(10, TimeUnit.MILLISECONDS);
5757
opStats.registerSuccessfulEvent(5, TimeUnit.MILLISECONDS);
5858

59+
OpStatsLogger opStats1 = statsLogger.scopeLabel("test_label", "test_value")
60+
.getOpStatsLogger("op_label");
61+
opStats1.registerSuccessfulEvent(10, TimeUnit.MILLISECONDS);
62+
opStats1.registerSuccessfulEvent(5, TimeUnit.MILLISECONDS);
63+
opStats1.registerFailedEvent(1, TimeUnit.MILLISECONDS);
64+
5965
provider.rotateLatencyCollection();
6066

6167
StringWriter writer = new StringWriter();
@@ -112,6 +118,52 @@ public void testStatsOutput() throws Exception {
112118
}
113119

114120
assertTrue(found);
121+
122+
// test_op_label_sum
123+
cm = (List<Metric>) metrics.get("test_op_label_sum");
124+
assertEquals(2, cm.size());
125+
m = cm.get(0);
126+
assertEquals(2, m.tags.size());
127+
assertEquals(1.0, m.value, 0.0);
128+
assertEquals("false", m.tags.get("success"));
129+
assertEquals("test_value", m.tags.get("test_label"));
130+
131+
m = cm.get(1);
132+
assertEquals(15.0, m.value, 0.0);
133+
assertEquals(2, m.tags.size());
134+
assertEquals("true", m.tags.get("success"));
135+
assertEquals("test_value", m.tags.get("test_label"));
136+
137+
// test_op_label_count
138+
cm = (List<Metric>) metrics.get("test_op_label_count");
139+
assertEquals(2, cm.size());
140+
m = cm.get(0);
141+
assertEquals(1, m.value, 0.0);
142+
assertEquals(2, m.tags.size());
143+
assertEquals("false", m.tags.get("success"));
144+
assertEquals("test_value", m.tags.get("test_label"));
145+
146+
m = cm.get(1);
147+
assertEquals(2.0, m.value, 0.0);
148+
assertEquals(2, m.tags.size());
149+
assertEquals("true", m.tags.get("success"));
150+
assertEquals("test_value", m.tags.get("test_label"));
151+
152+
// Latency
153+
cm = (List<Metric>) metrics.get("test_op_label");
154+
assertEquals(14, cm.size());
155+
156+
found = false;
157+
for (Metric mt : cm) {
158+
if ("true".equals(mt.tags.get("success"))
159+
&& "test_value".equals(mt.tags.get("test_label"))
160+
&& "1.0".equals(mt.tags.get("quantile"))) {
161+
assertEquals(10.0, mt.value, 0.0);
162+
found = true;
163+
}
164+
}
165+
166+
assertTrue(found);
115167
}
116168

117169
/**
@@ -126,6 +178,8 @@ private static Multimap<String, Metric> parseMetrics(String metrics) {
126178
// pulsar_subscriptions_count{cluster="standalone", namespace="sample/standalone/ns1",
127179
// topic="persistent://sample/standalone/ns1/test-2"} 0.0 1517945780897
128180
Pattern pattern = Pattern.compile("^(\\w+)(\\{([^\\}]+)\\})?\\s(-?[\\d\\w\\.]+)(\\s(\\d+))?$");
181+
Pattern formatPattern = Pattern.compile("^(\\w+)(\\{(\\w+=[\\\"\\.\\w]+(,\\s?\\w+=[\\\"\\.\\w]+)*)\\})?"
182+
+ "\\s(-?[\\d\\w\\.]+)(\\s(\\d+))?$");
129183
Pattern tagsPattern = Pattern.compile("(\\w+)=\"([^\"]+)\"(,\\s?)?");
130184

131185
Splitter.on("\n").split(metrics).forEach(line -> {
@@ -135,6 +189,7 @@ private static Multimap<String, Metric> parseMetrics(String metrics) {
135189

136190
System.err.println("LINE: '" + line + "'");
137191
Matcher matcher = pattern.matcher(line);
192+
Matcher formatMatcher = formatPattern.matcher(line);
138193
System.err.println("Matches: " + matcher.matches());
139194
System.err.println(matcher);
140195

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

146201
checkArgument(matcher.matches());
202+
checkArgument(formatMatcher.matches());
147203
String name = matcher.group(1);
148204

149205
Metric m = new Metric();

0 commit comments

Comments
 (0)