From 6a87f399419dbbb6166e6c9c6552ad731b52b670 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anders=20M=C3=A5nsson?= Date: Tue, 13 May 2025 15:45:54 +0200 Subject: [PATCH] Let DropwizardExports users deal with snapshot validation errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In the current form, if e.g. a Counter contains invalid data the entire `collect` calls fails with InvalidArgumentException. In large applications, it may be preferable to gracefully degrade the available metrics and report the failure via e.g. logging. This change allows that by letting the user add a exception handler. This allows the user to inspect the exception, and the metric name it was associated with, and decide if the exception should be suppressed or not. Metrics for which exceptions were suppressed will be absent from the collection. Signed-off-by: Anders Månsson --- .../dropwizard/DropwizardExports.java | 85 ++++++++++++------- .../dropwizard/DropwizardExportsTest.java | 51 +++++++++++ .../dropwizard5/DropwizardExports.java | 77 +++++++++++------ .../dropwizard5/InvalidMetricHandler.java | 13 +++ .../dropwizard5/DropwizardExportsTest.java | 51 +++++++++++ 5 files changed, 223 insertions(+), 54 deletions(-) create mode 100644 prometheus-metrics-instrumentation-dropwizard5/src/main/java/io/prometheus/metrics/instrumentation/dropwizard5/InvalidMetricHandler.java diff --git a/prometheus-metrics-instrumentation-dropwizard/src/main/java/io/prometheus/metrics/instrumentation/dropwizard/DropwizardExports.java b/prometheus-metrics-instrumentation-dropwizard/src/main/java/io/prometheus/metrics/instrumentation/dropwizard/DropwizardExports.java index dff657683..dd656fcd7 100644 --- a/prometheus-metrics-instrumentation-dropwizard/src/main/java/io/prometheus/metrics/instrumentation/dropwizard/DropwizardExports.java +++ b/prometheus-metrics-instrumentation-dropwizard/src/main/java/io/prometheus/metrics/instrumentation/dropwizard/DropwizardExports.java @@ -9,6 +9,7 @@ import com.codahale.metrics.MetricRegistry; import com.codahale.metrics.Snapshot; import com.codahale.metrics.Timer; +import io.prometheus.metrics.instrumentation.dropwizard5.InvalidMetricHandler; import io.prometheus.metrics.instrumentation.dropwizard5.labels.CustomLabelMapper; import io.prometheus.metrics.model.registry.MultiCollector; import io.prometheus.metrics.model.registry.PrometheusRegistry; @@ -21,8 +22,10 @@ import io.prometheus.metrics.model.snapshots.Quantiles; import io.prometheus.metrics.model.snapshots.SummarySnapshot; import java.util.Collections; +import java.util.Map; import java.util.Optional; import java.util.concurrent.TimeUnit; +import java.util.function.BiFunction; import java.util.logging.Level; import java.util.logging.Logger; @@ -32,6 +35,7 @@ public class DropwizardExports implements MultiCollector { private final MetricRegistry registry; private final MetricFilter metricFilter; private final Optional labelMapper; + private final InvalidMetricHandler invalidMetricHandler; /** * Creates a new DropwizardExports and {@link MetricFilter#ALL}. @@ -43,6 +47,7 @@ public DropwizardExports(MetricRegistry registry) { this.registry = registry; this.metricFilter = MetricFilter.ALL; this.labelMapper = Optional.empty(); + this.invalidMetricHandler = InvalidMetricHandler.ALWAYS_THROW; } /** @@ -55,6 +60,7 @@ public DropwizardExports(MetricRegistry registry, MetricFilter metricFilter) { this.registry = registry; this.metricFilter = metricFilter; this.labelMapper = Optional.empty(); + this.invalidMetricHandler = InvalidMetricHandler.ALWAYS_THROW; } /** @@ -67,6 +73,23 @@ public DropwizardExports( this.registry = registry; this.metricFilter = metricFilter; this.labelMapper = Optional.ofNullable(labelMapper); + this.invalidMetricHandler = InvalidMetricHandler.ALWAYS_THROW; + } + + /** + * @param registry a metric registry to export in prometheus. + * @param metricFilter a custom metric filter. + * @param labelMapper a labelMapper to use to map labels. + */ + private DropwizardExports( + MetricRegistry registry, + MetricFilter metricFilter, + CustomLabelMapper labelMapper, + InvalidMetricHandler invalidMetricHandler) { + this.registry = registry; + this.metricFilter = metricFilter; + this.labelMapper = Optional.ofNullable(labelMapper); + this.invalidMetricHandler = invalidMetricHandler; } private static String getHelpMessage(String metricName, Metric metric) { @@ -194,34 +217,33 @@ MetricSnapshot fromMeter(String dropwizardName, Meter meter) { @Override public MetricSnapshots collect() { MetricSnapshots.Builder metricSnapshots = MetricSnapshots.builder(); - - registry - .getGauges(metricFilter) - .forEach( - (name, gauge) -> { - MetricSnapshot snapshot = fromGauge(name, gauge); - if (snapshot != null) { - metricSnapshots.metricSnapshot(snapshot); - } - }); - - registry - .getCounters(metricFilter) - .forEach((name, counter) -> metricSnapshots.metricSnapshot(fromCounter(name, counter))); - registry - .getHistograms(metricFilter) - .forEach( - (name, histogram) -> metricSnapshots.metricSnapshot(fromHistogram(name, histogram))); - registry - .getTimers(metricFilter) - .forEach((name, timer) -> metricSnapshots.metricSnapshot(fromTimer(name, timer))); - registry - .getMeters(metricFilter) - .forEach((name, meter) -> metricSnapshots.metricSnapshot(fromMeter(name, meter))); - + collectMetricKind(metricSnapshots, registry.getGauges(metricFilter), this::fromGauge); + collectMetricKind(metricSnapshots, registry.getCounters(metricFilter), this::fromCounter); + collectMetricKind(metricSnapshots, registry.getHistograms(metricFilter), this::fromHistogram); + collectMetricKind(metricSnapshots, registry.getTimers(metricFilter), this::fromTimer); + collectMetricKind(metricSnapshots, registry.getMeters(metricFilter), this::fromMeter); return metricSnapshots.build(); } + private void collectMetricKind( + MetricSnapshots.Builder builder, + Map metric, + BiFunction toSnapshot) { + for (Map.Entry entry : metric.entrySet()) { + String metricName = entry.getKey(); + try { + MetricSnapshot snapshot = toSnapshot.apply(metricName, entry.getValue()); + if (snapshot != null) { + builder.metricSnapshot(snapshot); + } + } catch (Exception e) { + if (!invalidMetricHandler.suppressException(metricName, e)) { + throw e; + } + } + } + } + public static Builder builder() { return new Builder(); } @@ -231,9 +253,11 @@ public static class Builder { private MetricRegistry registry; private MetricFilter metricFilter; private CustomLabelMapper labelMapper; + private InvalidMetricHandler invalidMetricHandler; private Builder() { this.metricFilter = MetricFilter.ALL; + this.invalidMetricHandler = InvalidMetricHandler.ALWAYS_THROW; } public Builder dropwizardRegistry(MetricRegistry registry) { @@ -251,15 +275,16 @@ public Builder customLabelMapper(CustomLabelMapper labelMapper) { return this; } + public Builder invalidMetricHandler(InvalidMetricHandler invalidMetricHandler) { + this.invalidMetricHandler = invalidMetricHandler; + return this; + } + DropwizardExports build() { if (registry == null) { throw new IllegalArgumentException("MetricRegistry must be set"); } - if (labelMapper == null) { - return new DropwizardExports(registry, metricFilter); - } else { - return new DropwizardExports(registry, metricFilter, labelMapper); - } + return new DropwizardExports(registry, metricFilter, labelMapper, invalidMetricHandler); } public void register() { diff --git a/prometheus-metrics-instrumentation-dropwizard/src/test/java/io/prometheus/metrics/instrumentation/dropwizard/DropwizardExportsTest.java b/prometheus-metrics-instrumentation-dropwizard/src/test/java/io/prometheus/metrics/instrumentation/dropwizard/DropwizardExportsTest.java index 79480e4e3..7625771b0 100644 --- a/prometheus-metrics-instrumentation-dropwizard/src/test/java/io/prometheus/metrics/instrumentation/dropwizard/DropwizardExportsTest.java +++ b/prometheus-metrics-instrumentation-dropwizard/src/test/java/io/prometheus/metrics/instrumentation/dropwizard/DropwizardExportsTest.java @@ -6,6 +6,7 @@ import com.codahale.metrics.*; import io.prometheus.metrics.expositionformats.OpenMetricsTextFormatWriter; +import io.prometheus.metrics.instrumentation.dropwizard5.InvalidMetricHandler; import io.prometheus.metrics.model.registry.PrometheusRegistry; import io.prometheus.metrics.model.snapshots.SummarySnapshot; import java.io.ByteArrayOutputStream; @@ -278,6 +279,56 @@ public void testThatMetricHelpUsesOriginalDropwizardName() { assertThat(convertToOpenMetricsFormat()).isEqualTo(expected); } + @Test + void responseWhenRegistryIsEmpty() { + var registry = new PrometheusRegistry(); + registry.register(DropwizardExports.builder().dropwizardRegistry(metricRegistry).build()); + assertThat(convertToOpenMetricsFormat(registry)) + .isEqualTo( + """ +# EOF +"""); + } + + @Test + void collectInvalidMetricFails() { + metricRegistry.counter("my.application.namedCounter1").inc(-10); + metricRegistry.counter("my.application.namedCounter2").inc(10); + var registry = new PrometheusRegistry(); + DropwizardExports.builder().dropwizardRegistry(metricRegistry).register(registry); + assertThatThrownBy(() -> convertToOpenMetricsFormat(registry)) + .isInstanceOf(IllegalArgumentException.class); + } + + @Test + void collectInvalidMetricPassesWhenExceptionIsIgnored() { + metricRegistry.counter("my.application.namedCounter1").inc(-10); + metricRegistry.counter("my.application.namedCounter2").inc(10); + var registry = new PrometheusRegistry(); + + final StringBuilder buf = new StringBuilder(); + InvalidMetricHandler invalidMetricHandler = + (name, exc) -> { + buf.append("%s: %s%n".formatted(name, exc.getMessage())); + return true; + }; + + DropwizardExports.builder() + .dropwizardRegistry(metricRegistry) + .invalidMetricHandler(invalidMetricHandler) + .register(registry); + assertThat(convertToOpenMetricsFormat(registry)) + .isEqualTo( + """ +# TYPE my_application_namedCounter2 counter +# HELP my_application_namedCounter2 Generated from Dropwizard metric import (metric=my.application.namedCounter2, type=com.codahale.metrics.Counter) +my_application_namedCounter2_total 10.0 +# EOF +"""); + assertThat(buf.toString()) + .contains("my.application.namedCounter1: -10.0: counters cannot have a negative value"); + } + private static class ExampleDoubleGauge implements Gauge { @Override public Double getValue() { diff --git a/prometheus-metrics-instrumentation-dropwizard5/src/main/java/io/prometheus/metrics/instrumentation/dropwizard5/DropwizardExports.java b/prometheus-metrics-instrumentation-dropwizard5/src/main/java/io/prometheus/metrics/instrumentation/dropwizard5/DropwizardExports.java index c050871ce..e6631188d 100644 --- a/prometheus-metrics-instrumentation-dropwizard5/src/main/java/io/prometheus/metrics/instrumentation/dropwizard5/DropwizardExports.java +++ b/prometheus-metrics-instrumentation-dropwizard5/src/main/java/io/prometheus/metrics/instrumentation/dropwizard5/DropwizardExports.java @@ -24,8 +24,8 @@ import java.util.Collections; import java.util.Map; import java.util.Optional; -import java.util.Set; import java.util.concurrent.TimeUnit; +import java.util.function.BiFunction; import java.util.logging.Level; import java.util.logging.Logger; @@ -35,6 +35,7 @@ public class DropwizardExports implements MultiCollector { private final MetricRegistry registry; private final MetricFilter metricFilter; private final Optional labelMapper; + private final InvalidMetricHandler invalidMetricHandler; /** * Creates a new DropwizardExports and {@link MetricFilter#ALL}. @@ -46,6 +47,7 @@ public DropwizardExports(MetricRegistry registry) { this.registry = registry; this.metricFilter = MetricFilter.ALL; this.labelMapper = Optional.empty(); + this.invalidMetricHandler = InvalidMetricHandler.ALWAYS_THROW; } /** @@ -58,6 +60,7 @@ public DropwizardExports(MetricRegistry registry, MetricFilter metricFilter) { this.registry = registry; this.metricFilter = metricFilter; this.labelMapper = Optional.empty(); + this.invalidMetricHandler = InvalidMetricHandler.ALWAYS_THROW; } /** @@ -70,6 +73,23 @@ public DropwizardExports( this.registry = registry; this.metricFilter = metricFilter; this.labelMapper = Optional.ofNullable(labelMapper); + this.invalidMetricHandler = InvalidMetricHandler.ALWAYS_THROW; + } + + /** + * @param registry a metric registry to export in prometheus. + * @param metricFilter a custom metric filter. + * @param labelMapper a labelMapper to use to map labels. + */ + private DropwizardExports( + MetricRegistry registry, + MetricFilter metricFilter, + CustomLabelMapper labelMapper, + InvalidMetricHandler invalidMetricHandler) { + this.registry = registry; + this.metricFilter = metricFilter; + this.labelMapper = Optional.ofNullable(labelMapper); + this.invalidMetricHandler = invalidMetricHandler; } private static String getHelpMessage(String metricName, Metric metric) { @@ -197,27 +217,33 @@ MetricSnapshot fromMeter(String dropwizardName, Meter meter) { @Override public MetricSnapshots collect() { MetricSnapshots.Builder metricSnapshots = MetricSnapshots.builder(); - @SuppressWarnings("rawtypes") - Set> entries = registry.getGauges(metricFilter).entrySet(); - for (@SuppressWarnings("rawtypes") Map.Entry entry : entries) { - Optional.ofNullable(fromGauge(entry.getKey().getKey(), entry.getValue())) - .ifPresent(metricSnapshots::metricSnapshot); - } - for (Map.Entry entry : registry.getCounters(metricFilter).entrySet()) { - metricSnapshots.metricSnapshot(fromCounter(entry.getKey().getKey(), entry.getValue())); - } - for (Map.Entry entry : registry.getHistograms(metricFilter).entrySet()) { - metricSnapshots.metricSnapshot(fromHistogram(entry.getKey().getKey(), entry.getValue())); - } - for (Map.Entry entry : registry.getTimers(metricFilter).entrySet()) { - metricSnapshots.metricSnapshot(fromTimer(entry.getKey().getKey(), entry.getValue())); - } - for (Map.Entry entry : registry.getMeters(metricFilter).entrySet()) { - metricSnapshots.metricSnapshot(fromMeter(entry.getKey().getKey(), entry.getValue())); - } + collectMetricKind(metricSnapshots, registry.getGauges(metricFilter), this::fromGauge); + collectMetricKind(metricSnapshots, registry.getCounters(metricFilter), this::fromCounter); + collectMetricKind(metricSnapshots, registry.getHistograms(metricFilter), this::fromHistogram); + collectMetricKind(metricSnapshots, registry.getTimers(metricFilter), this::fromTimer); + collectMetricKind(metricSnapshots, registry.getMeters(metricFilter), this::fromMeter); return metricSnapshots.build(); } + private void collectMetricKind( + MetricSnapshots.Builder builder, + Map metric, + BiFunction toSnapshot) { + for (Map.Entry entry : metric.entrySet()) { + String metricName = entry.getKey().getKey(); + try { + MetricSnapshot snapshot = toSnapshot.apply(metricName, entry.getValue()); + if (snapshot != null) { + builder.metricSnapshot(snapshot); + } + } catch (Exception e) { + if (!invalidMetricHandler.suppressException(metricName, e)) { + throw e; + } + } + } + } + public static Builder builder() { return new Builder(); } @@ -227,9 +253,11 @@ public static class Builder { private MetricRegistry registry; private MetricFilter metricFilter; private CustomLabelMapper labelMapper; + private InvalidMetricHandler invalidMetricHandler; private Builder() { this.metricFilter = MetricFilter.ALL; + this.invalidMetricHandler = InvalidMetricHandler.ALWAYS_THROW; } public Builder dropwizardRegistry(MetricRegistry registry) { @@ -247,15 +275,16 @@ public Builder customLabelMapper(CustomLabelMapper labelMapper) { return this; } + public Builder invalidMetricHandler(InvalidMetricHandler invalidMetricHandler) { + this.invalidMetricHandler = invalidMetricHandler; + return this; + } + DropwizardExports build() { if (registry == null) { throw new IllegalArgumentException("MetricRegistry must be set"); } - if (labelMapper == null) { - return new DropwizardExports(registry, metricFilter); - } else { - return new DropwizardExports(registry, metricFilter, labelMapper); - } + return new DropwizardExports(registry, metricFilter, labelMapper, invalidMetricHandler); } public void register() { diff --git a/prometheus-metrics-instrumentation-dropwizard5/src/main/java/io/prometheus/metrics/instrumentation/dropwizard5/InvalidMetricHandler.java b/prometheus-metrics-instrumentation-dropwizard5/src/main/java/io/prometheus/metrics/instrumentation/dropwizard5/InvalidMetricHandler.java new file mode 100644 index 000000000..bac1aa0af --- /dev/null +++ b/prometheus-metrics-instrumentation-dropwizard5/src/main/java/io/prometheus/metrics/instrumentation/dropwizard5/InvalidMetricHandler.java @@ -0,0 +1,13 @@ +package io.prometheus.metrics.instrumentation.dropwizard5; + +@FunctionalInterface +public interface InvalidMetricHandler { + InvalidMetricHandler ALWAYS_THROW = (metricName, exc) -> false; + + /** + * @param metricName the name of the metric that was collected. + * @param exc The exception that was thrown when producing the metric snapshot. + * @return true if the exception should be suppressed. + */ + boolean suppressException(String metricName, Exception exc); +} diff --git a/prometheus-metrics-instrumentation-dropwizard5/src/test/java/io/prometheus/metrics/instrumentation/dropwizard5/DropwizardExportsTest.java b/prometheus-metrics-instrumentation-dropwizard5/src/test/java/io/prometheus/metrics/instrumentation/dropwizard5/DropwizardExportsTest.java index b80e37db3..8108a21b7 100644 --- a/prometheus-metrics-instrumentation-dropwizard5/src/test/java/io/prometheus/metrics/instrumentation/dropwizard5/DropwizardExportsTest.java +++ b/prometheus-metrics-instrumentation-dropwizard5/src/test/java/io/prometheus/metrics/instrumentation/dropwizard5/DropwizardExportsTest.java @@ -1,6 +1,7 @@ package io.prometheus.metrics.instrumentation.dropwizard5; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.assertj.core.data.Offset.offset; import io.dropwizard.metrics5.*; @@ -286,6 +287,56 @@ public void testThatMetricHelpUsesOriginalDropwizardName() { assertThat(convertToOpenMetricsFormat()).isEqualTo(expected); } + @Test + void responseWhenRegistryIsEmpty() { + var registry = new PrometheusRegistry(); + registry.register(DropwizardExports.builder().dropwizardRegistry(metricRegistry).build()); + assertThat(convertToOpenMetricsFormat(registry)) + .isEqualTo( + """ +# EOF +"""); + } + + @Test + void collectInvalidMetricFails() { + metricRegistry.counter("my.application.namedCounter1").inc(-10); + metricRegistry.counter("my.application.namedCounter2").inc(10); + var registry = new PrometheusRegistry(); + DropwizardExports.builder().dropwizardRegistry(metricRegistry).register(registry); + assertThatThrownBy(() -> convertToOpenMetricsFormat(registry)) + .isInstanceOf(IllegalArgumentException.class); + } + + @Test + void collectInvalidMetricPassesWhenExceptionIsIgnored() { + metricRegistry.counter("my.application.namedCounter1").inc(-10); + metricRegistry.counter("my.application.namedCounter2").inc(10); + var registry = new PrometheusRegistry(); + + final StringBuilder buf = new StringBuilder(); + InvalidMetricHandler invalidMetricHandler = + (name, exc) -> { + buf.append("%s: %s%n".formatted(name, exc.getMessage())); + return true; + }; + + DropwizardExports.builder() + .dropwizardRegistry(metricRegistry) + .invalidMetricHandler(invalidMetricHandler) + .register(registry); + assertThat(convertToOpenMetricsFormat(registry)) + .isEqualTo( + """ +# TYPE my_application_namedCounter2 counter +# HELP my_application_namedCounter2 Generated from Dropwizard metric import (metric=my.application.namedCounter2, type=io.dropwizard.metrics5.Counter) +my_application_namedCounter2_total 10.0 +# EOF +"""); + assertThat(buf.toString()) + .contains("my.application.namedCounter1: -10.0: counters cannot have a negative value"); + } + private static class ExampleDoubleGauge implements Gauge { @Override public Double getValue() {