diff --git a/metrics-otel-producer/src/test/java/io/avaje/metrics/otel/producer/OtelMetricProducerTest.java b/metrics-otel-producer/src/test/java/io/avaje/metrics/otel/producer/OtelMetricProducerTest.java index 561340e..5f5babc 100644 --- a/metrics-otel-producer/src/test/java/io/avaje/metrics/otel/producer/OtelMetricProducerTest.java +++ b/metrics-otel-producer/src/test/java/io/avaje/metrics/otel/producer/OtelMetricProducerTest.java @@ -86,7 +86,7 @@ void counter_tagsConvertedAndNullEntriesSkipped() { } @Test - void timer_metricsAreCumulativeAndMaxResets() { + void timer_metricsAreCumulativeAndMaxIsShared() { var epochNanosSource = new MutableEpochNanosSource(epochNanos(Instant.parse("2026-01-01T00:00:00Z"))); var registry = Metrics.createRegistry(); var producer = new DOtelMetricProducer(registry, InstrumentationScopeInfo.create("test.scope"), 0, epochNanosSource); @@ -130,10 +130,10 @@ void timer_metricsAreCumulativeAndMaxResets() { assertThat(onlyLongSumPoint(second.get("app.service.method.count")).getValue()).isEqualTo(1); assertThat(onlyLongSumPoint(second.get("app.service.method.total")).getValue()).isEqualTo(5_000L); - assertThat(onlyLongGaugePoint(second.get("app.service.method.max")).getValue()).isEqualTo(0L); + assertThat(onlyLongGaugePoint(second.get("app.service.method.max")).getValue()).isEqualTo(5_000L); assertThat(onlyLongSumPoint(second.get("app.service.method.error.count")).getValue()).isEqualTo(1); assertThat(onlyLongSumPoint(second.get("app.service.method.error.total")).getValue()).isEqualTo(2_000L); - assertThat(onlyLongGaugePoint(second.get("app.service.method.error.max")).getValue()).isEqualTo(0L); + assertThat(onlyLongGaugePoint(second.get("app.service.method.error.max")).getValue()).isEqualTo(2_000L); timer.addEventDuration(true, 3_000_000L); timer.addEventDuration(false, 7_000_000L); @@ -142,10 +142,10 @@ void timer_metricsAreCumulativeAndMaxResets() { assertThat(onlyLongSumPoint(third.get("app.service.method.count")).getValue()).isEqualTo(2); assertThat(onlyLongSumPoint(third.get("app.service.method.total")).getValue()).isEqualTo(8_000L); - assertThat(onlyLongGaugePoint(third.get("app.service.method.max")).getValue()).isEqualTo(3_000L); + assertThat(onlyLongGaugePoint(third.get("app.service.method.max")).getValue()).isEqualTo(5_000L); assertThat(onlyLongSumPoint(third.get("app.service.method.error.count")).getValue()).isEqualTo(2); assertThat(onlyLongSumPoint(third.get("app.service.method.error.total")).getValue()).isEqualTo(9_000L); - assertThat(onlyLongGaugePoint(third.get("app.service.method.error.max")).getValue()).isEqualTo(7_000L); + assertThat(onlyLongGaugePoint(third.get("app.service.method.error.max")).getValue()).isEqualTo(2_000L); } @Test @@ -167,7 +167,7 @@ void timedThreshold_appliesToCumulativeTotal() { assertThat(onlyLongSumPoint(metrics.get("app.fast.method.count")).getValue()).isEqualTo(2); assertThat(onlyLongSumPoint(metrics.get("app.fast.method.total")).getValue()).isEqualTo(10_000L); - assertThat(onlyLongGaugePoint(metrics.get("app.fast.method.max")).getValue()).isEqualTo(9_000L); + assertThat(onlyLongGaugePoint(metrics.get("app.fast.method.max")).getValue()).isEqualTo(1_000L); } @Test diff --git a/metrics-prometheus/src/test/java/io/avaje/metrics/prometheus/PrometheusMetricsTest.java b/metrics-prometheus/src/test/java/io/avaje/metrics/prometheus/PrometheusMetricsTest.java index 69ff500..4a48b83 100644 --- a/metrics-prometheus/src/test/java/io/avaje/metrics/prometheus/PrometheusMetricsTest.java +++ b/metrics-prometheus/src/test/java/io/avaje/metrics/prometheus/PrometheusMetricsTest.java @@ -98,7 +98,7 @@ void timer_canIncludeWindowedMaxGauge() { assertThat(first).contains( "# TYPE app_service_max_seconds_max gauge\n", "app_service_max_seconds_max 0.005\n"); - assertThat(second).contains("app_service_max_seconds_max 0.0\n"); + assertThat(second).contains("app_service_max_seconds_max 0.005\n"); } @Test diff --git a/metrics/src/main/java/io/avaje/metrics/core/ValueCounter.java b/metrics/src/main/java/io/avaje/metrics/core/ValueCounter.java index 3e84116..fb87a40 100644 --- a/metrics/src/main/java/io/avaje/metrics/core/ValueCounter.java +++ b/metrics/src/main/java/io/avaje/metrics/core/ValueCounter.java @@ -8,8 +8,6 @@ import io.avaje.metrics.stats.TimerStats; import org.jspecify.annotations.Nullable; -import java.util.concurrent.atomic.LongAccumulator; - /** * Used to collect long value statistics for meter and timer metrics. *

@@ -21,16 +19,24 @@ final class ValueCounter extends BaseReportName { private final @Nullable String bucketRange; private final ValueAdder count = new ValueAdder(); private final ValueAdder total = new ValueAdder(); - private final LongAccumulator max = new LongAccumulator(Math::max, 0); + private final ValueMax max; ValueCounter(Metric.ID id) { - super(id); - this.bucketRange = null; + this(id, null, new ValueMax()); } ValueCounter(Metric.ID id, String bucketRange) { + this(id, bucketRange, new ValueMax()); + } + + ValueCounter(Metric.ID id, ValueMax max) { + this(id, null, max); + } + + private ValueCounter(Metric.ID id, @Nullable String bucketRange, ValueMax max) { super(id); this.bucketRange = bucketRange; + this.max = max; } @Override @@ -48,7 +54,7 @@ public String toString() { void add(long value) { count.add(1); total.add(value); - max.accumulate(value); + max.add(value); } Meter.@Nullable Stats collect(Metric.Visitor collector, String unit) { @@ -67,7 +73,7 @@ void add(long value) { return null; } final long totalVal = total.get(collector.collectionMode()); - final long maxVal = max.getThenReset(); + final long maxVal = max.collect(); final Metric.ID reportId = reportId(collector); return new Snapshot(reportId, count, totalVal, maxVal); } @@ -99,7 +105,7 @@ long total() { * Return the max value. */ long max() { - return max.get(); + return max.current(); } long mean() { diff --git a/metrics/src/main/java/io/avaje/metrics/core/ValueMax.java b/metrics/src/main/java/io/avaje/metrics/core/ValueMax.java new file mode 100644 index 0000000..3afe729 --- /dev/null +++ b/metrics/src/main/java/io/avaje/metrics/core/ValueMax.java @@ -0,0 +1,50 @@ +package io.avaje.metrics.core; + +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.LongAccumulator; +import java.util.function.LongSupplier; + +/** + * Accumulates a maximum value and publishes it in rolling 59-second windows. + */ +final class ValueMax { + + private static final long WINDOW_NANOS = TimeUnit.SECONDS.toNanos(59); + + private final LongSupplier nanoTime; + private final LongAccumulator value = new LongAccumulator(Math::max, 0); + private long lastResetNanos; + private volatile long published; + + ValueMax() { + this(System::nanoTime); + } + + ValueMax(LongSupplier nanoTime) { + this.nanoTime = nanoTime; + this.lastResetNanos = nanoTime.getAsLong() - 2 * WINDOW_NANOS; + } + + void add(long amount) { + value.accumulate(amount); + } + + synchronized long collect() { + long now = nanoTime.getAsLong(); + if (now - lastResetNanos >= WINDOW_NANOS) { + published = value.getThenReset(); + lastResetNanos = now; + } + return published; + } + + long current() { + return value.get(); + } + + synchronized void reset() { + value.reset(); + published = 0; + lastResetNanos = nanoTime.getAsLong() - 2 * WINDOW_NANOS; + } +} diff --git a/metrics/src/test/java/io/avaje/metrics/core/CollectAsJsonTest.java b/metrics/src/test/java/io/avaje/metrics/core/CollectAsJsonTest.java index 39c4fa5..805ca64 100644 --- a/metrics/src/test/java/io/avaje/metrics/core/CollectAsJsonTest.java +++ b/metrics/src/test/java/io/avaje/metrics/core/CollectAsJsonTest.java @@ -98,7 +98,7 @@ void collectAsJsonCumulative() { } @Test - void collectAsJsonCumulative_maxResets() { + void collectAsJsonCumulative_publishesSharedMax() { Meter meter = registry.meter("my.cumulative.meter"); meter.addEvent(40); @@ -108,10 +108,10 @@ void collectAsJsonCumulative_maxResets() { assertThat(first).contains("{\"name\":\"my.cumulative.meter\",\"count\":2,\"mean\":45,\"max\":50,\"total\":90}"); String second = registry.collectAsJson(CollectionMode.CUMULATIVE).asJson(); - assertThat(second).contains("{\"name\":\"my.cumulative.meter\",\"count\":2,\"mean\":45,\"max\":0,\"total\":90}"); + assertThat(second).contains("{\"name\":\"my.cumulative.meter\",\"count\":2,\"mean\":45,\"max\":50,\"total\":90}"); meter.addEvent(30); String third = registry.collectAsJson(CollectionMode.CUMULATIVE).asJson(); - assertThat(third).contains("{\"name\":\"my.cumulative.meter\",\"count\":3,\"mean\":40,\"max\":30,\"total\":120}"); + assertThat(third).contains("{\"name\":\"my.cumulative.meter\",\"count\":3,\"mean\":40,\"max\":50,\"total\":120}"); } } diff --git a/metrics/src/test/java/io/avaje/metrics/core/DBucketTimerTest.java b/metrics/src/test/java/io/avaje/metrics/core/DBucketTimerTest.java index f2eab18..a0e5074 100644 --- a/metrics/src/test/java/io/avaje/metrics/core/DBucketTimerTest.java +++ b/metrics/src/test/java/io/avaje/metrics/core/DBucketTimerTest.java @@ -86,19 +86,19 @@ void collectTimer_cumulative() { Timer.Stats stats2 = collectTimer(bucket, CollectionMode.CUMULATIVE); assertEquals(1, stats2.count()); assertEquals(50_000, stats2.total()); - assertEquals(0, stats2.max()); + assertEquals(50_000, stats2.max()); long fortyMillisAsNanos = TimeUnit.MILLISECONDS.toNanos(40); bucketTimedMetric.addEventDuration(true, fortyMillisAsNanos); Timer.Stats stats3 = collectTimer(bucket, CollectionMode.CUMULATIVE); assertEquals(2, stats3.count()); assertEquals(90_000, stats3.total()); - assertEquals(40_000, stats3.max()); + assertEquals(50_000, stats3.max()); Timer.Stats delta = collectTimer(bucket); assertEquals(2, delta.count()); assertEquals(90_000, delta.total()); - assertEquals(0, delta.max()); + assertEquals(50_000, delta.max()); assertThat(collect(bucket)).isEmpty(); } diff --git a/metrics/src/test/java/io/avaje/metrics/core/MeterTest.java b/metrics/src/test/java/io/avaje/metrics/core/MeterTest.java index 1ad5bb5..71939a4 100644 --- a/metrics/src/test/java/io/avaje/metrics/core/MeterTest.java +++ b/metrics/src/test/java/io/avaje/metrics/core/MeterTest.java @@ -87,20 +87,20 @@ void collectCumulative() { Meter.Stats statistics2 = (Meter.Stats) collect(metric, CollectionMode.CUMULATIVE).get(0); assertEquals(3, statistics2.count()); assertEquals(4500, statistics2.total()); - assertEquals(0, statistics2.max()); + assertEquals(2000, statistics2.max()); assertEquals(1500, statistics2.mean()); metric.addEvent(750); Meter.Stats statistics3 = (Meter.Stats) collect(metric, CollectionMode.CUMULATIVE).get(0); assertEquals(4, statistics3.count()); assertEquals(5250, statistics3.total()); - assertEquals(750, statistics3.max()); + assertEquals(2000, statistics3.max()); assertEquals(1312, statistics3.mean()); Meter.Stats delta = (Meter.Stats) collect(metric).get(0); assertEquals(4, delta.count()); assertEquals(5250, delta.total()); - assertEquals(0, delta.max()); + assertEquals(2000, delta.max()); assertEquals(0, metric.count()); assertThat(collect(metric)).isEmpty(); } diff --git a/metrics/src/test/java/io/avaje/metrics/core/TimerTest.java b/metrics/src/test/java/io/avaje/metrics/core/TimerTest.java index 64576da..b012465 100644 --- a/metrics/src/test/java/io/avaje/metrics/core/TimerTest.java +++ b/metrics/src/test/java/io/avaje/metrics/core/TimerTest.java @@ -212,10 +212,10 @@ void collectMetrics_cumulative() { assertThat(stats2).hasSize(2); assertEquals(1, ((Timer.Stats) stats2.get(0)).count()); assertEquals(5_000, ((Timer.Stats) stats2.get(0)).total()); - assertEquals(0, ((Timer.Stats) stats2.get(0)).max()); + assertEquals(5_000, ((Timer.Stats) stats2.get(0)).max()); assertEquals(1, ((Timer.Stats) stats2.get(1)).count()); assertEquals(2_000, ((Timer.Stats) stats2.get(1)).total()); - assertEquals(0, ((Timer.Stats) stats2.get(1)).max()); + assertEquals(2_000, ((Timer.Stats) stats2.get(1)).max()); metric.addEventDuration(true, TimeUnit.MILLISECONDS.toNanos(3)); metric.addEventDuration(false, TimeUnit.MILLISECONDS.toNanos(7)); @@ -223,19 +223,19 @@ void collectMetrics_cumulative() { assertThat(stats3).hasSize(2); assertEquals(2, ((Timer.Stats) stats3.get(0)).count()); assertEquals(8_000, ((Timer.Stats) stats3.get(0)).total()); - assertEquals(3_000, ((Timer.Stats) stats3.get(0)).max()); + assertEquals(5_000, ((Timer.Stats) stats3.get(0)).max()); assertEquals(2, ((Timer.Stats) stats3.get(1)).count()); assertEquals(9_000, ((Timer.Stats) stats3.get(1)).total()); - assertEquals(7_000, ((Timer.Stats) stats3.get(1)).max()); + assertEquals(2_000, ((Timer.Stats) stats3.get(1)).max()); List stats4 = registry.collectMetrics(); assertThat(stats4).hasSize(2); assertEquals(2, ((Timer.Stats) stats4.get(0)).count()); assertEquals(8_000, ((Timer.Stats) stats4.get(0)).total()); - assertEquals(0, ((Timer.Stats) stats4.get(0)).max()); + assertEquals(5_000, ((Timer.Stats) stats4.get(0)).max()); assertEquals(2, ((Timer.Stats) stats4.get(1)).count()); assertEquals(9_000, ((Timer.Stats) stats4.get(1)).total()); - assertEquals(0, ((Timer.Stats) stats4.get(1)).max()); + assertEquals(2_000, ((Timer.Stats) stats4.get(1)).max()); assertThat(registry.collectMetrics()).isEmpty(); } diff --git a/metrics/src/test/java/io/avaje/metrics/core/ValueCounterTest.java b/metrics/src/test/java/io/avaje/metrics/core/ValueCounterTest.java index eb991db..4e4829f 100644 --- a/metrics/src/test/java/io/avaje/metrics/core/ValueCounterTest.java +++ b/metrics/src/test/java/io/avaje/metrics/core/ValueCounterTest.java @@ -1,12 +1,21 @@ package io.avaje.metrics.core; import io.avaje.metrics.Metric; +import io.avaje.metrics.Meter; +import io.avaje.metrics.NamingMatch; import org.junit.jupiter.api.Test; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicLong; + +import static io.avaje.metrics.CollectionMode.CUMULATIVE; +import static io.avaje.metrics.CollectionMode.DELTA; import static org.junit.jupiter.api.Assertions.assertEquals; class ValueCounterTest { + private final AtomicLong nanoTime = new AtomicLong(); + @Test void testGetStatisticsWithNoReset() { ValueCounter counter = new ValueCounter(Metric.ID.of("junk")); @@ -58,4 +67,29 @@ void test() { assertEquals(0, counter.total()); assertEquals(0, counter.max()); } + + @Test + void collectSharesMaxAcrossCollectionModes() { + ValueCounter counter = new ValueCounter(Metric.ID.of("junk"), new ValueMax(nanoTime::get)); + counter.add(100); + counter.add(50); + + Meter.Stats cumulative = collect(counter, CUMULATIVE); + assertEquals(100, cumulative.max()); + + counter.add(200); + Meter.Stats delta = collect(counter, DELTA); + assertEquals(100, delta.max()); + + nanoTime.addAndGet(TimeUnit.SECONDS.toNanos(59)); + Meter.Stats next = collect(counter, CUMULATIVE); + assertEquals(200, next.max()); + counter.add(300); + assertEquals(200, collect(counter, DELTA).max()); + } + + private Meter.Stats collect(ValueCounter counter, io.avaje.metrics.CollectionMode mode) { + var collector = new DStatsCollector(NamingMatch.INSTANCE, mode); + return (Meter.Stats) counter.collect(collector, "units"); + } }