Skip to content

Commit 6c82338

Browse files
mnssnmnssn
authored
Let clients of DropwizardExports handle validation errors (#1354)
A possible solution for #1353. 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 just that - by changing the access modifiers to the `fromXYZ` methods subclasses can now handle the errors. Finally, to allow for skipping of invalid metrics, we follow the example in `fromGauge` to interpret the meaning of a returned null to be that the metric should be skipped. Signed-off-by: Anders Månsson <[email protected]>
1 parent 913a7c4 commit 6c82338

File tree

5 files changed

+223
-54
lines changed

5 files changed

+223
-54
lines changed

‎prometheus-metrics-instrumentation-dropwizard/src/main/java/io/prometheus/metrics/instrumentation/dropwizard/DropwizardExports.java‎

Lines changed: 55 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import com.codahale.metrics.MetricRegistry;
1010
import com.codahale.metrics.Snapshot;
1111
import com.codahale.metrics.Timer;
12+
import io.prometheus.metrics.instrumentation.dropwizard5.InvalidMetricHandler;
1213
import io.prometheus.metrics.instrumentation.dropwizard5.labels.CustomLabelMapper;
1314
import io.prometheus.metrics.model.registry.MultiCollector;
1415
import io.prometheus.metrics.model.registry.PrometheusRegistry;
@@ -21,8 +22,10 @@
2122
import io.prometheus.metrics.model.snapshots.Quantiles;
2223
import io.prometheus.metrics.model.snapshots.SummarySnapshot;
2324
import java.util.Collections;
25+
import java.util.Map;
2426
import java.util.Optional;
2527
import java.util.concurrent.TimeUnit;
28+
import java.util.function.BiFunction;
2629
import java.util.logging.Level;
2730
import java.util.logging.Logger;
2831

@@ -32,6 +35,7 @@ public class DropwizardExports implements MultiCollector {
3235
private final MetricRegistry registry;
3336
private final MetricFilter metricFilter;
3437
private final Optional<CustomLabelMapper> labelMapper;
38+
private final InvalidMetricHandler invalidMetricHandler;
3539

3640
/**
3741
* Creates a new DropwizardExports and {@link MetricFilter#ALL}.
@@ -43,6 +47,7 @@ public DropwizardExports(MetricRegistry registry) {
4347
this.registry = registry;
4448
this.metricFilter = MetricFilter.ALL;
4549
this.labelMapper = Optional.empty();
50+
this.invalidMetricHandler = InvalidMetricHandler.ALWAYS_THROW;
4651
}
4752

4853
/**
@@ -55,6 +60,7 @@ public DropwizardExports(MetricRegistry registry, MetricFilter metricFilter) {
5560
this.registry = registry;
5661
this.metricFilter = metricFilter;
5762
this.labelMapper = Optional.empty();
63+
this.invalidMetricHandler = InvalidMetricHandler.ALWAYS_THROW;
5864
}
5965

6066
/**
@@ -67,6 +73,23 @@ public DropwizardExports(
6773
this.registry = registry;
6874
this.metricFilter = metricFilter;
6975
this.labelMapper = Optional.ofNullable(labelMapper);
76+
this.invalidMetricHandler = InvalidMetricHandler.ALWAYS_THROW;
77+
}
78+
79+
/**
80+
* @param registry a metric registry to export in prometheus.
81+
* @param metricFilter a custom metric filter.
82+
* @param labelMapper a labelMapper to use to map labels.
83+
*/
84+
private DropwizardExports(
85+
MetricRegistry registry,
86+
MetricFilter metricFilter,
87+
CustomLabelMapper labelMapper,
88+
InvalidMetricHandler invalidMetricHandler) {
89+
this.registry = registry;
90+
this.metricFilter = metricFilter;
91+
this.labelMapper = Optional.ofNullable(labelMapper);
92+
this.invalidMetricHandler = invalidMetricHandler;
7093
}
7194

7295
private static String getHelpMessage(String metricName, Metric metric) {
@@ -194,34 +217,33 @@ MetricSnapshot fromMeter(String dropwizardName, Meter meter) {
194217
@Override
195218
public MetricSnapshots collect() {
196219
MetricSnapshots.Builder metricSnapshots = MetricSnapshots.builder();
197-
198-
registry
199-
.getGauges(metricFilter)
200-
.forEach(
201-
(name, gauge) -> {
202-
MetricSnapshot snapshot = fromGauge(name, gauge);
203-
if (snapshot != null) {
204-
metricSnapshots.metricSnapshot(snapshot);
205-
}
206-
});
207-
208-
registry
209-
.getCounters(metricFilter)
210-
.forEach((name, counter) -> metricSnapshots.metricSnapshot(fromCounter(name, counter)));
211-
registry
212-
.getHistograms(metricFilter)
213-
.forEach(
214-
(name, histogram) -> metricSnapshots.metricSnapshot(fromHistogram(name, histogram)));
215-
registry
216-
.getTimers(metricFilter)
217-
.forEach((name, timer) -> metricSnapshots.metricSnapshot(fromTimer(name, timer)));
218-
registry
219-
.getMeters(metricFilter)
220-
.forEach((name, meter) -> metricSnapshots.metricSnapshot(fromMeter(name, meter)));
221-
220+
collectMetricKind(metricSnapshots, registry.getGauges(metricFilter), this::fromGauge);
221+
collectMetricKind(metricSnapshots, registry.getCounters(metricFilter), this::fromCounter);
222+
collectMetricKind(metricSnapshots, registry.getHistograms(metricFilter), this::fromHistogram);
223+
collectMetricKind(metricSnapshots, registry.getTimers(metricFilter), this::fromTimer);
224+
collectMetricKind(metricSnapshots, registry.getMeters(metricFilter), this::fromMeter);
222225
return metricSnapshots.build();
223226
}
224227

228+
private <T> void collectMetricKind(
229+
MetricSnapshots.Builder builder,
230+
Map<String, T> metric,
231+
BiFunction<String, T, MetricSnapshot> toSnapshot) {
232+
for (Map.Entry<String, T> entry : metric.entrySet()) {
233+
String metricName = entry.getKey();
234+
try {
235+
MetricSnapshot snapshot = toSnapshot.apply(metricName, entry.getValue());
236+
if (snapshot != null) {
237+
builder.metricSnapshot(snapshot);
238+
}
239+
} catch (Exception e) {
240+
if (!invalidMetricHandler.suppressException(metricName, e)) {
241+
throw e;
242+
}
243+
}
244+
}
245+
}
246+
225247
public static Builder builder() {
226248
return new Builder();
227249
}
@@ -231,9 +253,11 @@ public static class Builder {
231253
private MetricRegistry registry;
232254
private MetricFilter metricFilter;
233255
private CustomLabelMapper labelMapper;
256+
private InvalidMetricHandler invalidMetricHandler;
234257

235258
private Builder() {
236259
this.metricFilter = MetricFilter.ALL;
260+
this.invalidMetricHandler = InvalidMetricHandler.ALWAYS_THROW;
237261
}
238262

239263
public Builder dropwizardRegistry(MetricRegistry registry) {
@@ -251,15 +275,16 @@ public Builder customLabelMapper(CustomLabelMapper labelMapper) {
251275
return this;
252276
}
253277

278+
public Builder invalidMetricHandler(InvalidMetricHandler invalidMetricHandler) {
279+
this.invalidMetricHandler = invalidMetricHandler;
280+
return this;
281+
}
282+
254283
DropwizardExports build() {
255284
if (registry == null) {
256285
throw new IllegalArgumentException("MetricRegistry must be set");
257286
}
258-
if (labelMapper == null) {
259-
return new DropwizardExports(registry, metricFilter);
260-
} else {
261-
return new DropwizardExports(registry, metricFilter, labelMapper);
262-
}
287+
return new DropwizardExports(registry, metricFilter, labelMapper, invalidMetricHandler);
263288
}
264289

265290
public void register() {

‎prometheus-metrics-instrumentation-dropwizard/src/test/java/io/prometheus/metrics/instrumentation/dropwizard/DropwizardExportsTest.java‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
import com.codahale.metrics.*;
88
import io.prometheus.metrics.expositionformats.OpenMetricsTextFormatWriter;
9+
import io.prometheus.metrics.instrumentation.dropwizard5.InvalidMetricHandler;
910
import io.prometheus.metrics.model.registry.PrometheusRegistry;
1011
import io.prometheus.metrics.model.snapshots.SummarySnapshot;
1112
import java.io.ByteArrayOutputStream;
@@ -278,6 +279,56 @@ public void testThatMetricHelpUsesOriginalDropwizardName() {
278279
assertThat(convertToOpenMetricsFormat()).isEqualTo(expected);
279280
}
280281

282+
@Test
283+
void responseWhenRegistryIsEmpty() {
284+
var registry = new PrometheusRegistry();
285+
registry.register(DropwizardExports.builder().dropwizardRegistry(metricRegistry).build());
286+
assertThat(convertToOpenMetricsFormat(registry))
287+
.isEqualTo(
288+
"""
289+
# EOF
290+
""");
291+
}
292+
293+
@Test
294+
void collectInvalidMetricFails() {
295+
metricRegistry.counter("my.application.namedCounter1").inc(-10);
296+
metricRegistry.counter("my.application.namedCounter2").inc(10);
297+
var registry = new PrometheusRegistry();
298+
DropwizardExports.builder().dropwizardRegistry(metricRegistry).register(registry);
299+
assertThatThrownBy(() -> convertToOpenMetricsFormat(registry))
300+
.isInstanceOf(IllegalArgumentException.class);
301+
}
302+
303+
@Test
304+
void collectInvalidMetricPassesWhenExceptionIsIgnored() {
305+
metricRegistry.counter("my.application.namedCounter1").inc(-10);
306+
metricRegistry.counter("my.application.namedCounter2").inc(10);
307+
var registry = new PrometheusRegistry();
308+
309+
final StringBuilder buf = new StringBuilder();
310+
InvalidMetricHandler invalidMetricHandler =
311+
(name, exc) -> {
312+
buf.append("%s: %s%n".formatted(name, exc.getMessage()));
313+
return true;
314+
};
315+
316+
DropwizardExports.builder()
317+
.dropwizardRegistry(metricRegistry)
318+
.invalidMetricHandler(invalidMetricHandler)
319+
.register(registry);
320+
assertThat(convertToOpenMetricsFormat(registry))
321+
.isEqualTo(
322+
"""
323+
# TYPE my_application_namedCounter2 counter
324+
# HELP my_application_namedCounter2 Generated from Dropwizard metric import (metric=my.application.namedCounter2, type=com.codahale.metrics.Counter)
325+
my_application_namedCounter2_total 10.0
326+
# EOF
327+
""");
328+
assertThat(buf.toString())
329+
.contains("my.application.namedCounter1: -10.0: counters cannot have a negative value");
330+
}
331+
281332
private static class ExampleDoubleGauge implements Gauge<Double> {
282333
@Override
283334
public Double getValue() {

‎prometheus-metrics-instrumentation-dropwizard5/src/main/java/io/prometheus/metrics/instrumentation/dropwizard5/DropwizardExports.java‎

Lines changed: 53 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,8 @@
2424
import java.util.Collections;
2525
import java.util.Map;
2626
import java.util.Optional;
27-
import java.util.Set;
2827
import java.util.concurrent.TimeUnit;
28+
import java.util.function.BiFunction;
2929
import java.util.logging.Level;
3030
import java.util.logging.Logger;
3131

@@ -35,6 +35,7 @@ public class DropwizardExports implements MultiCollector {
3535
private final MetricRegistry registry;
3636
private final MetricFilter metricFilter;
3737
private final Optional<CustomLabelMapper> labelMapper;
38+
private final InvalidMetricHandler invalidMetricHandler;
3839

3940
/**
4041
* Creates a new DropwizardExports and {@link MetricFilter#ALL}.
@@ -46,6 +47,7 @@ public DropwizardExports(MetricRegistry registry) {
4647
this.registry = registry;
4748
this.metricFilter = MetricFilter.ALL;
4849
this.labelMapper = Optional.empty();
50+
this.invalidMetricHandler = InvalidMetricHandler.ALWAYS_THROW;
4951
}
5052

5153
/**
@@ -58,6 +60,7 @@ public DropwizardExports(MetricRegistry registry, MetricFilter metricFilter) {
5860
this.registry = registry;
5961
this.metricFilter = metricFilter;
6062
this.labelMapper = Optional.empty();
63+
this.invalidMetricHandler = InvalidMetricHandler.ALWAYS_THROW;
6164
}
6265

6366
/**
@@ -70,6 +73,23 @@ public DropwizardExports(
7073
this.registry = registry;
7174
this.metricFilter = metricFilter;
7275
this.labelMapper = Optional.ofNullable(labelMapper);
76+
this.invalidMetricHandler = InvalidMetricHandler.ALWAYS_THROW;
77+
}
78+
79+
/**
80+
* @param registry a metric registry to export in prometheus.
81+
* @param metricFilter a custom metric filter.
82+
* @param labelMapper a labelMapper to use to map labels.
83+
*/
84+
private DropwizardExports(
85+
MetricRegistry registry,
86+
MetricFilter metricFilter,
87+
CustomLabelMapper labelMapper,
88+
InvalidMetricHandler invalidMetricHandler) {
89+
this.registry = registry;
90+
this.metricFilter = metricFilter;
91+
this.labelMapper = Optional.ofNullable(labelMapper);
92+
this.invalidMetricHandler = invalidMetricHandler;
7393
}
7494

7595
private static String getHelpMessage(String metricName, Metric metric) {
@@ -197,27 +217,33 @@ MetricSnapshot fromMeter(String dropwizardName, Meter meter) {
197217
@Override
198218
public MetricSnapshots collect() {
199219
MetricSnapshots.Builder metricSnapshots = MetricSnapshots.builder();
200-
@SuppressWarnings("rawtypes")
201-
Set<Map.Entry<MetricName, Gauge>> entries = registry.getGauges(metricFilter).entrySet();
202-
for (@SuppressWarnings("rawtypes") Map.Entry<MetricName, Gauge> entry : entries) {
203-
Optional.ofNullable(fromGauge(entry.getKey().getKey(), entry.getValue()))
204-
.ifPresent(metricSnapshots::metricSnapshot);
205-
}
206-
for (Map.Entry<MetricName, Counter> entry : registry.getCounters(metricFilter).entrySet()) {
207-
metricSnapshots.metricSnapshot(fromCounter(entry.getKey().getKey(), entry.getValue()));
208-
}
209-
for (Map.Entry<MetricName, Histogram> entry : registry.getHistograms(metricFilter).entrySet()) {
210-
metricSnapshots.metricSnapshot(fromHistogram(entry.getKey().getKey(), entry.getValue()));
211-
}
212-
for (Map.Entry<MetricName, Timer> entry : registry.getTimers(metricFilter).entrySet()) {
213-
metricSnapshots.metricSnapshot(fromTimer(entry.getKey().getKey(), entry.getValue()));
214-
}
215-
for (Map.Entry<MetricName, Meter> entry : registry.getMeters(metricFilter).entrySet()) {
216-
metricSnapshots.metricSnapshot(fromMeter(entry.getKey().getKey(), entry.getValue()));
217-
}
220+
collectMetricKind(metricSnapshots, registry.getGauges(metricFilter), this::fromGauge);
221+
collectMetricKind(metricSnapshots, registry.getCounters(metricFilter), this::fromCounter);
222+
collectMetricKind(metricSnapshots, registry.getHistograms(metricFilter), this::fromHistogram);
223+
collectMetricKind(metricSnapshots, registry.getTimers(metricFilter), this::fromTimer);
224+
collectMetricKind(metricSnapshots, registry.getMeters(metricFilter), this::fromMeter);
218225
return metricSnapshots.build();
219226
}
220227

228+
private <T> void collectMetricKind(
229+
MetricSnapshots.Builder builder,
230+
Map<MetricName, T> metric,
231+
BiFunction<String, T, MetricSnapshot> toSnapshot) {
232+
for (Map.Entry<MetricName, T> entry : metric.entrySet()) {
233+
String metricName = entry.getKey().getKey();
234+
try {
235+
MetricSnapshot snapshot = toSnapshot.apply(metricName, entry.getValue());
236+
if (snapshot != null) {
237+
builder.metricSnapshot(snapshot);
238+
}
239+
} catch (Exception e) {
240+
if (!invalidMetricHandler.suppressException(metricName, e)) {
241+
throw e;
242+
}
243+
}
244+
}
245+
}
246+
221247
public static Builder builder() {
222248
return new Builder();
223249
}
@@ -227,9 +253,11 @@ public static class Builder {
227253
private MetricRegistry registry;
228254
private MetricFilter metricFilter;
229255
private CustomLabelMapper labelMapper;
256+
private InvalidMetricHandler invalidMetricHandler;
230257

231258
private Builder() {
232259
this.metricFilter = MetricFilter.ALL;
260+
this.invalidMetricHandler = InvalidMetricHandler.ALWAYS_THROW;
233261
}
234262

235263
public Builder dropwizardRegistry(MetricRegistry registry) {
@@ -247,15 +275,16 @@ public Builder customLabelMapper(CustomLabelMapper labelMapper) {
247275
return this;
248276
}
249277

278+
public Builder invalidMetricHandler(InvalidMetricHandler invalidMetricHandler) {
279+
this.invalidMetricHandler = invalidMetricHandler;
280+
return this;
281+
}
282+
250283
DropwizardExports build() {
251284
if (registry == null) {
252285
throw new IllegalArgumentException("MetricRegistry must be set");
253286
}
254-
if (labelMapper == null) {
255-
return new DropwizardExports(registry, metricFilter);
256-
} else {
257-
return new DropwizardExports(registry, metricFilter, labelMapper);
258-
}
287+
return new DropwizardExports(registry, metricFilter, labelMapper, invalidMetricHandler);
259288
}
260289

261290
public void register() {

‎prometheus-metrics-instrumentation-dropwizard5/src/main/java/io/prometheus/metrics/instrumentation/dropwizard5/InvalidMetricHandler.java‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
package io.prometheus.metrics.instrumentation.dropwizard5;
2+
3+
@FunctionalInterface
4+
public interface InvalidMetricHandler {
5+
InvalidMetricHandler ALWAYS_THROW = (metricName, exc) -> false;
6+
7+
/**
8+
* @param metricName the name of the metric that was collected.
9+
* @param exc The exception that was thrown when producing the metric snapshot.
10+
* @return true if the exception should be suppressed.
11+
*/
12+
boolean suppressException(String metricName, Exception exc);
13+
}

0 commit comments

Comments
 (0)