From 014f248c03de62f7b21f9e73f0f7e8715ff84ba6 Mon Sep 17 00:00:00 2001 From: Harsh Srivastava Date: Sun, 19 Jul 2026 22:00:16 +0530 Subject: [PATCH] Include counter metric name in negative value errors When a CounterSnapshot data point has a negative value, the exception message can now include the metric name (and labels) so large scrapes are easier to diagnose. Counter and CounterWithCallback pass the name at collect time; callers such as Micrometer can set it via the builder. Fixes #1090 Signed-off-by: Harsh Srivastava --- .../metrics/core/metrics/Counter.java | 7 +- .../core/metrics/CounterWithCallback.java | 2 +- .../model/snapshots/CounterSnapshot.java | 86 +++++++++++++++++-- .../model/snapshots/CounterSnapshotTest.java | 21 +++++ 4 files changed, 107 insertions(+), 9 deletions(-) diff --git a/prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/Counter.java b/prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/Counter.java index 1c614bdf3..5548f0c41 100644 --- a/prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/Counter.java +++ b/prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/Counter.java @@ -92,8 +92,9 @@ public CounterSnapshot collect() { @Override protected CounterSnapshot collect(List labels, List metricData) { List data = new ArrayList<>(labels.size()); + String metricName = metadata.getName(); for (int i = 0; i < labels.size(); i++) { - data.add(metricData.get(i).collect(labels.get(i))); + data.add(metricData.get(i).collect(labels.get(i), metricName)); } return new CounterSnapshot(metadata, data); } @@ -199,7 +200,7 @@ private void validateAndAdd(double amount) { doubleValue.add(amount); } - private CounterSnapshot.CounterDataPointSnapshot collect(Labels labels) { + private CounterSnapshot.CounterDataPointSnapshot collect(Labels labels, String metricName) { // Read the exemplar first. Otherwise, there is a race condition where you might // see an Exemplar for a value that's not counted yet. // If there are multiple Exemplars (by default it's just one), use the newest. @@ -213,7 +214,7 @@ private CounterSnapshot.CounterDataPointSnapshot collect(Labels labels) { } } return new CounterSnapshot.CounterDataPointSnapshot( - get(), labels, latestExemplar, createdTimeMillis); + get(), labels, latestExemplar, createdTimeMillis, metricName); } } diff --git a/prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/CounterWithCallback.java b/prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/CounterWithCallback.java index 514a13d26..2ea4b27fd 100644 --- a/prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/CounterWithCallback.java +++ b/prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/CounterWithCallback.java @@ -48,7 +48,7 @@ public CounterSnapshot collect() { (value, labelValues) -> { dataPoints.add( new CounterSnapshot.CounterDataPointSnapshot( - value, makeLabels(labelValues), null, 0L)); + value, makeLabels(labelValues), null, 0L, metadata.getName())); }); return new CounterSnapshot(metadata, dataPoints); } diff --git a/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/snapshots/CounterSnapshot.java b/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/snapshots/CounterSnapshot.java index 42a06f3bb..ff2866668 100644 --- a/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/snapshots/CounterSnapshot.java +++ b/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/snapshots/CounterSnapshot.java @@ -48,6 +48,8 @@ public static class CounterDataPointSnapshot extends DataPointSnapshot { private final double value; @Nullable private final Exemplar exemplar; + /** Optional metric name used only in validation error messages. */ + @Nullable private final String metricName; /** * To create a new {@link CounterDataPointSnapshot}, you can either call the constructor @@ -62,7 +64,20 @@ public static class CounterDataPointSnapshot extends DataPointSnapshot { */ public CounterDataPointSnapshot( double value, Labels labels, @Nullable Exemplar exemplar, long createdTimestampMillis) { - this(value, labels, exemplar, createdTimestampMillis, 0); + this(value, labels, exemplar, createdTimestampMillis, 0, null); + } + + /** + * Same as {@link #CounterDataPointSnapshot(double, Labels, Exemplar, long)} with an optional + * metric name included in validation error messages when the value is negative. + */ + public CounterDataPointSnapshot( + double value, + Labels labels, + @Nullable Exemplar exemplar, + long createdTimestampMillis, + @Nullable String metricName) { + this(value, labels, exemplar, createdTimestampMillis, 0, metricName); } /** @@ -77,7 +92,30 @@ public CounterDataPointSnapshot( @Nullable Exemplar exemplar, long createdTimestampMillis, long scrapeTimestampMillis) { - this(value, labels, exemplar, createdTimestampMillis, scrapeTimestampMillis, false); + this(value, labels, exemplar, createdTimestampMillis, scrapeTimestampMillis, false, null); + } + + /** + * Constructor with scrape timestamp and optional metric name for validation messages. + * + * @see #CounterDataPointSnapshot(double, Labels, Exemplar, long, long) + */ + @SuppressWarnings("this-escape") + public CounterDataPointSnapshot( + double value, + Labels labels, + @Nullable Exemplar exemplar, + long createdTimestampMillis, + long scrapeTimestampMillis, + @Nullable String metricName) { + this( + value, + labels, + exemplar, + createdTimestampMillis, + scrapeTimestampMillis, + false, + metricName); } @SuppressWarnings("this-escape") @@ -88,9 +126,22 @@ public CounterDataPointSnapshot( long createdTimestampMillis, long scrapeTimestampMillis, boolean internal) { + this(value, labels, exemplar, createdTimestampMillis, scrapeTimestampMillis, internal, null); + } + + @SuppressWarnings("this-escape") + private CounterDataPointSnapshot( + double value, + Labels labels, + @Nullable Exemplar exemplar, + long createdTimestampMillis, + long scrapeTimestampMillis, + boolean internal, + @Nullable String metricName) { super(labels, createdTimestampMillis, scrapeTimestampMillis, internal); this.value = value; this.exemplar = exemplar; + this.metricName = metricName; if (!internal) { validate(); } @@ -107,7 +158,16 @@ public Exemplar getExemplar() { protected void validate() { if (value < 0.0) { - throw new IllegalArgumentException(value + ": counters cannot have a negative value"); + StringBuilder message = new StringBuilder(); + if (metricName != null && !metricName.isEmpty()) { + message.append(metricName).append('='); + } + message.append(value).append(": counters cannot have a negative value"); + Labels labels = getLabels(); + if (labels != null && !labels.isEmpty()) { + message.append(" (labels=").append(labels).append(')'); + } + throw new IllegalArgumentException(message.toString()); } } @@ -119,7 +179,8 @@ DataPointSnapshot escape(EscapingScheme escapingScheme) { SnapshotEscaper.escapeExemplar(exemplar, escapingScheme), getCreatedTimestampMillis(), getScrapeTimestampMillis(), - true); + true, + metricName); } public static Builder builder() { @@ -131,6 +192,7 @@ public static class Builder extends DataPointSnapshot.Builder { @Nullable private Exemplar exemplar = null; @Nullable private Double value = null; private long createdTimestampMillis = 0L; + @Nullable private String metricName = null; private Builder() {} @@ -150,12 +212,26 @@ public Builder createdTimestampMillis(long createdTimestampMillis) { return this; } + /** + * Optional metric name included in the exception message when {@link #value(double)} is + * negative. Does not change the snapshot identity. + */ + public Builder metricName(@Nullable String metricName) { + this.metricName = metricName; + return this; + } + public CounterDataPointSnapshot build() { if (value == null) { throw new IllegalArgumentException("Missing required field: value is null."); } return new CounterDataPointSnapshot( - value, labels, exemplar, createdTimestampMillis, scrapeTimestampMillis); + value, + labels, + exemplar, + createdTimestampMillis, + scrapeTimestampMillis, + metricName); } @Override diff --git a/prometheus-metrics-model/src/test/java/io/prometheus/metrics/model/snapshots/CounterSnapshotTest.java b/prometheus-metrics-model/src/test/java/io/prometheus/metrics/model/snapshots/CounterSnapshotTest.java index af6060e65..68be65fef 100644 --- a/prometheus-metrics-model/src/test/java/io/prometheus/metrics/model/snapshots/CounterSnapshotTest.java +++ b/prometheus-metrics-model/src/test/java/io/prometheus/metrics/model/snapshots/CounterSnapshotTest.java @@ -130,4 +130,25 @@ void testDataImmutable() { iterator.next(); assertThatExceptionOfType(UnsupportedOperationException.class).isThrownBy(iterator::remove); } + + @Test + void testNegativeValueIncludesMetricNameInMessage() { + assertThatExceptionOfType(IllegalArgumentException.class) + .isThrownBy( + () -> + CounterDataPointSnapshot.builder() + .metricName("http_requests") + .value(-2.0) + .build()) + .withMessageContaining("http_requests") + .withMessageContaining("-2.0") + .withMessageContaining("counters cannot have a negative value"); + } + + @Test + void testNegativeValueWithoutMetricNameKeepsLegacyMessage() { + assertThatExceptionOfType(IllegalArgumentException.class) + .isThrownBy(() -> CounterDataPointSnapshot.builder().value(-1.0).build()) + .withMessage("-1.0: counters cannot have a negative value"); + } }