You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@flink.apache.org by ch...@apache.org on 2023/01/31 11:51:58 UTC
[flink] branch master updated: [FLINK-30796][metrics][slf4j] (Partially) skip metrics report
This is an automated email from the ASF dual-hosted git repository.
chesnay pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/flink.git
The following commit(s) were added to refs/heads/master by this push:
new e856bd8b5f8 [FLINK-30796][metrics][slf4j] (Partially) skip metrics report
e856bd8b5f8 is described below
commit e856bd8b5f834ad2dfd784940cc6186fde5b6c3c
Author: Chesnay Schepler <ch...@apache.org>
AuthorDate: Tue Jan 31 12:51:47 2023 +0100
[FLINK-30796][metrics][slf4j] (Partially) skip metrics report
---
.../apache/flink/metrics/slf4j/Slf4jReporter.java | 134 +++++++++++----------
.../flink/metrics/slf4j/Slf4jReporterTest.java | 68 ++++++++++-
2 files changed, 137 insertions(+), 65 deletions(-)
diff --git a/flink-metrics/flink-metrics-slf4j/src/main/java/org/apache/flink/metrics/slf4j/Slf4jReporter.java b/flink-metrics/flink-metrics-slf4j/src/main/java/org/apache/flink/metrics/slf4j/Slf4jReporter.java
index 0ad8604f0f2..fdf91b768b0 100644
--- a/flink-metrics/flink-metrics-slf4j/src/main/java/org/apache/flink/metrics/slf4j/Slf4jReporter.java
+++ b/flink-metrics/flink-metrics-slf4j/src/main/java/org/apache/flink/metrics/slf4j/Slf4jReporter.java
@@ -36,6 +36,8 @@ import org.slf4j.LoggerFactory;
import java.util.ConcurrentModificationException;
import java.util.Map;
import java.util.NoSuchElementException;
+import java.util.function.BiConsumer;
+import java.util.function.Function;
/** {@link MetricReporter} that exports {@link Metric Metrics} via SLF4J {@link Logger}. */
public class Slf4jReporter extends AbstractReporter implements Scheduled {
@@ -84,6 +86,10 @@ public class Slf4jReporter extends AbstractReporter implements Scheduled {
}
private void tryReport() {
+ if (gauges.isEmpty() && counters.isEmpty() && histograms.isEmpty() && meters.isEmpty()) {
+ LOG.info("Skipping metrics report because no metrics are registered.");
+ return;
+ }
// initialize with previous size to avoid repeated resizing of backing array
// pad the size to allow deviations in the final string, for example due to different double
// value representations
@@ -94,70 +100,38 @@ public class Slf4jReporter extends AbstractReporter implements Scheduled {
"=========================== Starting metrics report ===========================")
.append(lineSeparator);
- builder.append(lineSeparator)
- .append(
- "-- Counters -------------------------------------------------------------------")
- .append(lineSeparator);
- for (Map.Entry<Counter, String> metric : counters.entrySet()) {
- builder.append(metric.getValue())
- .append(": ")
- .append(metric.getKey().getCount())
- .append(lineSeparator);
- }
-
- builder.append(lineSeparator)
- .append(
- "-- Gauges ---------------------------------------------------------------------")
- .append(lineSeparator);
- for (Map.Entry<Gauge<?>, String> metric : gauges.entrySet()) {
- builder.append(metric.getValue())
- .append(": ")
- .append(metric.getKey().getValue())
- .append(lineSeparator);
- }
-
- builder.append(lineSeparator)
- .append(
- "-- Meters ---------------------------------------------------------------------")
- .append(lineSeparator);
- for (Map.Entry<Meter, String> metric : meters.entrySet()) {
- builder.append(metric.getValue())
- .append(": ")
- .append(metric.getKey().getRate())
- .append(lineSeparator);
- }
-
- builder.append(lineSeparator)
- .append(
- "-- Histograms -----------------------------------------------------------------")
- .append(lineSeparator);
- for (Map.Entry<Histogram, String> metric : histograms.entrySet()) {
- HistogramStatistics stats = metric.getKey().getStatistics();
- builder.append(metric.getValue())
- .append(": count=")
- .append(stats.size())
- .append(", min=")
- .append(stats.getMin())
- .append(", max=")
- .append(stats.getMax())
- .append(", mean=")
- .append(stats.getMean())
- .append(", stddev=")
- .append(stats.getStdDev())
- .append(", p50=")
- .append(stats.getQuantile(0.50))
- .append(", p75=")
- .append(stats.getQuantile(0.75))
- .append(", p95=")
- .append(stats.getQuantile(0.95))
- .append(", p98=")
- .append(stats.getQuantile(0.98))
- .append(", p99=")
- .append(stats.getQuantile(0.99))
- .append(", p999=")
- .append(stats.getQuantile(0.999))
- .append(lineSeparator);
- }
+ report(builder, "Counters", counters, Counter::getCount);
+ report(builder, "Gauges", gauges, Gauge::getValue);
+ report(builder, "Meters", meters, Meter::getRate);
+ report(
+ builder,
+ "Histograms",
+ histograms,
+ (metric, b) -> {
+ HistogramStatistics stats = metric.getStatistics();
+ b.append("count=")
+ .append(stats.size())
+ .append(", min=")
+ .append(stats.getMin())
+ .append(", max=")
+ .append(stats.getMax())
+ .append(", mean=")
+ .append(stats.getMean())
+ .append(", stddev=")
+ .append(stats.getStdDev())
+ .append(", p50=")
+ .append(stats.getQuantile(0.50))
+ .append(", p75=")
+ .append(stats.getQuantile(0.75))
+ .append(", p95=")
+ .append(stats.getQuantile(0.95))
+ .append(", p98=")
+ .append(stats.getQuantile(0.98))
+ .append(", p99=")
+ .append(stats.getQuantile(0.99))
+ .append(", p999=")
+ .append(stats.getQuantile(0.999));
+ });
builder.append(lineSeparator)
.append(
@@ -168,6 +142,38 @@ public class Slf4jReporter extends AbstractReporter implements Scheduled {
previousSize = builder.length();
}
+ private static <T extends Metric> void report(
+ StringBuilder builder,
+ String type,
+ Map<T, String> metrics,
+ Function<T, Object> valueExtractor) {
+ report(
+ builder,
+ type,
+ metrics,
+ (metric, build) -> builder.append(valueExtractor.apply(metric)));
+ }
+
+ private static <T extends Metric> void report(
+ StringBuilder builder,
+ String type,
+ Map<T, String> metrics,
+ BiConsumer<T, StringBuilder> valueExtractor) {
+ if (!metrics.isEmpty()) {
+ builder.append(lineSeparator)
+ .append("-- ")
+ .append(type)
+ .append(
+ " ---------------------------------------------------------------------")
+ .append(lineSeparator);
+ for (Map.Entry<T, String> metric : metrics.entrySet()) {
+ builder.append(metric.getValue()).append(": ");
+ valueExtractor.accept(metric.getKey(), builder);
+ builder.append(lineSeparator);
+ }
+ }
+ }
+
@Override
public String filterCharacters(String input) {
return input;
diff --git a/flink-metrics/flink-metrics-slf4j/src/test/java/org/apache/flink/metrics/slf4j/Slf4jReporterTest.java b/flink-metrics/flink-metrics-slf4j/src/test/java/org/apache/flink/metrics/slf4j/Slf4jReporterTest.java
index 56c97207f9f..b482db142d6 100644
--- a/flink-metrics/flink-metrics-slf4j/src/test/java/org/apache/flink/metrics/slf4j/Slf4jReporterTest.java
+++ b/flink-metrics/flink-metrics-slf4j/src/test/java/org/apache/flink/metrics/slf4j/Slf4jReporterTest.java
@@ -30,6 +30,7 @@ import org.apache.flink.metrics.util.TestMetricGroup;
import org.apache.flink.testutils.logging.LoggerAuditingExtension;
import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;
import org.slf4j.event.Level;
@@ -43,7 +44,7 @@ class Slf4jReporterTest {
private static char delimiter;
private static MetricGroup metricGroup;
- private static Slf4jReporter reporter;
+ private Slf4jReporter reporter;
@RegisterExtension
private final LoggerAuditingExtension testLoggerResource =
@@ -57,10 +58,75 @@ class Slf4jReporterTest {
TestMetricGroup.newBuilder()
.setMetricIdentifierFunction((s, characterFilter) -> SCOPE + delimiter + s)
.build();
+ }
+
+ @BeforeEach
+ void setUpReporter() {
reporter = new Slf4jReporter();
reporter.open(new MetricConfig());
}
+ @Test
+ void testSkipOnNoMetrics() {
+ reporter.report();
+
+ assertThat(testLoggerResource.getMessages())
+ .noneMatch(logOutput -> logOutput.contains("Starting metrics report"))
+ .anyMatch(logOutput -> logOutput.contains("Skipping metrics report"));
+ }
+
+ @Test
+ void testOnlyCounterRegistered() {
+ reporter.notifyOfAddedMetric(new SimpleCounter(), "metric", metricGroup);
+
+ reporter.report();
+
+ assertThat(testLoggerResource.getMessages())
+ .noneMatch(logOutput -> logOutput.contains("-- Meter"))
+ .noneMatch(logOutput -> logOutput.contains("-- Gauge"))
+ .noneMatch(logOutput -> logOutput.contains("-- Histogram"))
+ .anyMatch(logOutput -> logOutput.contains("-- Counter"));
+ }
+
+ @Test
+ void testOnlyMeterRegistered() {
+ reporter.notifyOfAddedMetric(new MeterView(new SimpleCounter()), "metric", metricGroup);
+
+ reporter.report();
+
+ assertThat(testLoggerResource.getMessages())
+ .noneMatch(logOutput -> logOutput.contains("-- Counter"))
+ .noneMatch(logOutput -> logOutput.contains("-- Gauge"))
+ .noneMatch(logOutput -> logOutput.contains("-- Histogram"))
+ .anyMatch(logOutput -> logOutput.contains("-- Meter"));
+ }
+
+ @Test
+ void testOnlyGaugeRegistered() {
+ reporter.notifyOfAddedMetric((Gauge<Number>) () -> 4, "metric", metricGroup);
+
+ reporter.report();
+
+ assertThat(testLoggerResource.getMessages())
+ .noneMatch(logOutput -> logOutput.contains("-- Meter"))
+ .noneMatch(logOutput -> logOutput.contains("-- Counter"))
+ .noneMatch(logOutput -> logOutput.contains("-- Histogram"))
+ .anyMatch(logOutput -> logOutput.contains("-- Gauge"));
+ }
+
+ @Test
+ void testOnlyHistogramRegistered() {
+ reporter.notifyOfAddedMetric(new TestHistogram(), "metric", metricGroup);
+
+ reporter.report();
+
+ assertThat(testLoggerResource.getMessages())
+ .noneMatch(logOutput -> logOutput.contains("-- Meter"))
+ .noneMatch(logOutput -> logOutput.contains("-- Gauge"))
+ .noneMatch(logOutput -> logOutput.contains("-- Counter"))
+ .anyMatch(logOutput -> logOutput.contains("-- Histogram"));
+ }
+
@Test
void testAddCounter() throws Exception {
String counterName = "simpleCounter";