diff --git a/docs/apidiffs/current_vs_latest/prometheus-metrics-model.txt b/docs/apidiffs/current_vs_latest/prometheus-metrics-model.txt index d298829ca6..d514400194 100644 --- a/docs/apidiffs/current_vs_latest/prometheus-metrics-model.txt +++ b/docs/apidiffs/current_vs_latest/prometheus-metrics-model.txt @@ -1,4 +1,6 @@ Comparing source compatibility of prometheus-metrics-model-1.8.1-SNAPSHOT.jar against prometheus-metrics-model-1.8.0.jar +*** MODIFIED CLASS: PUBLIC STATIC io.prometheus.metrics.model.snapshots.CounterSnapshot$CounterDataPointSnapshot (not serializable) + === CLASS FILE FORMAT VERSION: 52.0 <- 52.0 *** MODIFIED CLASS: PUBLIC FINAL io.prometheus.metrics.model.snapshots.Labels (not serializable) === CLASS FILE FORMAT VERSION: 52.0 <- 52.0 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 1c614bdf33..8b7905571b 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. @@ -212,8 +213,13 @@ private CounterSnapshot.CounterDataPointSnapshot collect(Labels labels) { } } } - return new CounterSnapshot.CounterDataPointSnapshot( - get(), labels, latestExemplar, createdTimeMillis); + return CounterSnapshot.CounterDataPointSnapshot.builder() + .value(get()) + .labels(labels) + .exemplar(latestExemplar) + .createdTimestampMillis(createdTimeMillis) + .metricName(metricName) + .build(); } } 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 514a13d268..cde6f18e1d 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 @@ -47,8 +47,11 @@ public CounterSnapshot collect() { callback.accept( (value, labelValues) -> { dataPoints.add( - new CounterSnapshot.CounterDataPointSnapshot( - value, makeLabels(labelValues), null, 0L)); + CounterSnapshot.CounterDataPointSnapshot.builder() + .value(value) + .labels(makeLabels(labelValues)) + .metricName(metadata.getName()) + .build()); }); return new CounterSnapshot(metadata, dataPoints); } diff --git a/prometheus-metrics-core/src/test/java/io/prometheus/metrics/core/metrics/CounterWithCallbackTest.java b/prometheus-metrics-core/src/test/java/io/prometheus/metrics/core/metrics/CounterWithCallbackTest.java index 2907a8a027..61377c18f9 100644 --- a/prometheus-metrics-core/src/test/java/io/prometheus/metrics/core/metrics/CounterWithCallbackTest.java +++ b/prometheus-metrics-core/src/test/java/io/prometheus/metrics/core/metrics/CounterWithCallbackTest.java @@ -43,4 +43,17 @@ void testCounterNoCallback() { .isThrownBy( () -> CounterWithCallback.builder().name("counter").labelNames("l1", "l2").build()); } + + @Test + void testNegativeValueIncludesMetricName() { + CounterWithCallback counter = + CounterWithCallback.builder() + .name("negative_counter") + .callback(callback -> callback.call(-1.0)) + .build(); + + assertThatExceptionOfType(IllegalArgumentException.class) + .isThrownBy(counter::collect) + .withMessage("negative_counter=-1.0: counters cannot have a negative value"); + } } 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 42a06f3bbf..d2d3dfcaac 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 @@ -49,6 +49,9 @@ 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 * directly or use the Builder with {@link CounterDataPointSnapshot#builder()}. @@ -62,7 +65,7 @@ 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, false, null); } /** @@ -77,7 +80,7 @@ public CounterDataPointSnapshot( @Nullable Exemplar exemplar, long createdTimestampMillis, long scrapeTimestampMillis) { - this(value, labels, exemplar, createdTimestampMillis, scrapeTimestampMillis, false); + this(value, labels, exemplar, createdTimestampMillis, scrapeTimestampMillis, false, null); } @SuppressWarnings("this-escape") @@ -88,9 +91,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 +123,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 +144,8 @@ DataPointSnapshot escape(EscapingScheme escapingScheme) { SnapshotEscaper.escapeExemplar(exemplar, escapingScheme), getCreatedTimestampMillis(), getScrapeTimestampMillis(), - true); + true, + metricName); } public static Builder builder() { @@ -131,6 +157,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 +177,27 @@ 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, + false, + 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 af6060e65e..58f16a6b8e 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,22 @@ 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"); + } }