This is an automated email from the ASF dual-hosted git repository. mmerli pushed a commit to branch branch-4.14 in repository https://gitbox.apache.org/repos/asf/bookkeeper.git
commit 4292db8dc503d173f06ca4fe542bef5a7c581dee 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();
