From 03a227a2e27d3935573beb5ec6de4fb9f80ea909 Mon Sep 17 00:00:00 2001 From: Lenin Jaganathan <32874349+lenin-jaganathan@users.noreply.github.com> Date: Mon, 12 Jun 2023 15:56:32 +0530 Subject: [PATCH] Align DeltaHistogram in SignalFx registry with count and total (#3799) Count and total for Step Meters have been made to roll over at the start of the step. SignalFx's DeltaHistogram however did not align to this, resulting in a histogram that does not match with the count/total values as it should. This makes those align and generally updates the implementation to align better with the behavior provided to StepMeterRegistry implementations. Closes gh-3774 --- .../CumulativeHistogramConfigUtil.java | 25 ++++-- .../signalfx/DeltaHistogramCounts.java | 41 --------- .../signalfx/SignalfxDistributionSummary.java | 84 ++++++++----------- .../io/micrometer/signalfx/SignalfxTimer.java | 84 ++++++++----------- .../signalfx/DeltaHistogramCountsTest.java | 45 ---------- .../signalfx/SignalFxMeterRegistryTest.java | 60 +++++++++++++ .../step/StepDistributionSummary.java | 9 +- .../core/instrument/step/StepTimer.java | 10 ++- .../step/PollingAwareMockStepClock.java | 82 ++++++++++++++++++ .../core/instrument/step/package-info.java | 16 ++++ 10 files changed, 260 insertions(+), 196 deletions(-) delete mode 100644 implementations/micrometer-registry-signalfx/src/main/java/io/micrometer/signalfx/DeltaHistogramCounts.java delete mode 100644 implementations/micrometer-registry-signalfx/src/test/java/io/micrometer/signalfx/DeltaHistogramCountsTest.java create mode 100644 micrometer-test/src/main/java/io/micrometer/core/instrument/step/PollingAwareMockStepClock.java create mode 100644 micrometer-test/src/main/java/io/micrometer/core/instrument/step/package-info.java diff --git a/implementations/micrometer-registry-signalfx/src/main/java/io/micrometer/signalfx/CumulativeHistogramConfigUtil.java b/implementations/micrometer-registry-signalfx/src/main/java/io/micrometer/signalfx/CumulativeHistogramConfigUtil.java index 44de0db871..0e99ce47a6 100644 --- a/implementations/micrometer-registry-signalfx/src/main/java/io/micrometer/signalfx/CumulativeHistogramConfigUtil.java +++ b/implementations/micrometer-registry-signalfx/src/main/java/io/micrometer/signalfx/CumulativeHistogramConfigUtil.java @@ -28,17 +28,14 @@ */ final class CumulativeHistogramConfigUtil { - static DistributionStatisticConfig updateConfig(DistributionStatisticConfig distributionStatisticConfig) { + private static final double[] EMPTY_SLO = new double[0]; + + static DistributionStatisticConfig updateConfig(DistributionStatisticConfig distributionStatisticConfig, + boolean isDelta) { double[] sloBoundaries = distributionStatisticConfig.getServiceLevelObjectiveBoundaries(); if (sloBoundaries == null || sloBoundaries.length == 0) { return distributionStatisticConfig; } - double[] newSloBoundaries = sloBoundaries; - // Add the +Inf bucket since the "count" resets every export. - if (!isPositiveInf(sloBoundaries[sloBoundaries.length - 1])) { - newSloBoundaries = Arrays.copyOf(sloBoundaries, sloBoundaries.length + 1); - newSloBoundaries[newSloBoundaries.length - 1] = Double.MAX_VALUE; - } return DistributionStatisticConfig.builder() // Set the expiration duration for the histogram counts to be effectively @@ -46,11 +43,23 @@ static DistributionStatisticConfig updateConfig(DistributionStatisticConfig dist // Without this, the counts are reset every expiry duration. .expiry(Duration.ofNanos(Long.MAX_VALUE)) // effectively infinite .bufferLength(1) - .serviceLevelObjectives(newSloBoundaries) + // If delta Histograms are enabled, empty the slo's and use + // StepBucketHistogram. + .serviceLevelObjectives(isDelta ? EMPTY_SLO : addPositiveInfBucket(sloBoundaries)) .build() .merge(distributionStatisticConfig); } + static double[] addPositiveInfBucket(double[] sloBoundaries) { + double[] newSloBoundaries = sloBoundaries; + // Add the +Inf bucket since the "count" resets every export. + if (!isPositiveInf(sloBoundaries[sloBoundaries.length - 1])) { + newSloBoundaries = Arrays.copyOf(sloBoundaries, sloBoundaries.length + 1); + newSloBoundaries[newSloBoundaries.length - 1] = Double.MAX_VALUE; + } + return newSloBoundaries; + } + private static boolean isPositiveInf(double bucket) { return bucket == Double.POSITIVE_INFINITY || bucket == Double.MAX_VALUE || (long) bucket == Long.MAX_VALUE; } diff --git a/implementations/micrometer-registry-signalfx/src/main/java/io/micrometer/signalfx/DeltaHistogramCounts.java b/implementations/micrometer-registry-signalfx/src/main/java/io/micrometer/signalfx/DeltaHistogramCounts.java deleted file mode 100644 index b0ab70d6f0..0000000000 --- a/implementations/micrometer-registry-signalfx/src/main/java/io/micrometer/signalfx/DeltaHistogramCounts.java +++ /dev/null @@ -1,41 +0,0 @@ -/* - * Copyright 2022 VMware, Inc. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package io.micrometer.signalfx; - -import io.micrometer.common.lang.Nullable; -import io.micrometer.core.instrument.distribution.CountAtBucket; - -final class DeltaHistogramCounts { - - @Nullable - private CountAtBucket[] lastHistogramCounts; - - CountAtBucket[] calculate(CountAtBucket[] currentHistogramCounts) { - if (lastHistogramCounts == null || lastHistogramCounts.length == 0) { - lastHistogramCounts = currentHistogramCounts; - return currentHistogramCounts; - } - - CountAtBucket[] retHistogramCounts = new CountAtBucket[currentHistogramCounts.length]; - for (int i = 0; i < currentHistogramCounts.length; i++) { - retHistogramCounts[i] = new CountAtBucket(currentHistogramCounts[i].bucket(), - currentHistogramCounts[i].count() - lastHistogramCounts[i].count()); - } - lastHistogramCounts = currentHistogramCounts; - return retHistogramCounts; - } - -} diff --git a/implementations/micrometer-registry-signalfx/src/main/java/io/micrometer/signalfx/SignalfxDistributionSummary.java b/implementations/micrometer-registry-signalfx/src/main/java/io/micrometer/signalfx/SignalfxDistributionSummary.java index 91bcabe05c..b92f3d8e94 100644 --- a/implementations/micrometer-registry-signalfx/src/main/java/io/micrometer/signalfx/SignalfxDistributionSummary.java +++ b/implementations/micrometer-registry-signalfx/src/main/java/io/micrometer/signalfx/SignalfxDistributionSummary.java @@ -16,88 +16,72 @@ package io.micrometer.signalfx; import io.micrometer.common.lang.Nullable; -import io.micrometer.core.instrument.AbstractDistributionSummary; import io.micrometer.core.instrument.Clock; import io.micrometer.core.instrument.distribution.DistributionStatisticConfig; import io.micrometer.core.instrument.distribution.HistogramSnapshot; -import io.micrometer.core.instrument.distribution.TimeWindowMax; -import io.micrometer.core.instrument.step.StepTuple2; - -import java.util.concurrent.atomic.DoubleAdder; -import java.util.concurrent.atomic.LongAdder; +import io.micrometer.core.instrument.distribution.StepBucketHistogram; +import io.micrometer.core.instrument.step.StepDistributionSummary; /** - * This class is mostly the same as - * {@link io.micrometer.core.instrument.step.StepDistributionSummary}, with one notable - * difference: the {@link DistributionStatisticConfig} is modified before being passed to - * the super class constructor - that forces the histogram generated by this meter to be - * cumulative. + * A StepDistributionSummary which provides support for multiple flavours of Histograms to + * be recorded based on {@link SignalFxConfig#publishCumulativeHistogram()} and + * {@link SignalFxConfig#publishDeltaHistogram()}. * * @author Bogdan Drutu * @author Mateusz Rzeszutek + * @author Lenin Jaganathan */ -final class SignalfxDistributionSummary extends AbstractDistributionSummary { - - private final LongAdder count = new LongAdder(); - - private final DoubleAdder total = new DoubleAdder(); - - private final StepTuple2 countTotal; - - private final TimeWindowMax max; +final class SignalfxDistributionSummary extends StepDistributionSummary { @Nullable - private final DeltaHistogramCounts deltaHistogramCounts; + private final StepBucketHistogram stepBucketHistogram; SignalfxDistributionSummary(Id id, Clock clock, DistributionStatisticConfig distributionStatisticConfig, double scale, long stepMillis, boolean isDelta) { - super(id, clock, CumulativeHistogramConfigUtil.updateConfig(distributionStatisticConfig), scale, false); - this.countTotal = new StepTuple2<>(clock, stepMillis, 0L, 0.0, count::sumThenReset, total::sumThenReset); - max = new TimeWindowMax(clock, distributionStatisticConfig); - if (distributionStatisticConfig.isPublishingHistogram() && isDelta) { - deltaHistogramCounts = new DeltaHistogramCounts(); + super(id, clock, distributionStatisticConfig, scale, stepMillis, defaultHistogram(clock, + CumulativeHistogramConfigUtil.updateConfig(distributionStatisticConfig, isDelta), false)); + + double[] slo = distributionStatisticConfig.getServiceLevelObjectiveBoundaries(); + if (slo != null && slo.length > 0 && isDelta) { + stepBucketHistogram = new StepBucketHistogram(clock, stepMillis, + DistributionStatisticConfig.builder() + .serviceLevelObjectives(CumulativeHistogramConfigUtil.addPositiveInfBucket(slo)) + .build() + .merge(distributionStatisticConfig), + false, true); } else { - deltaHistogramCounts = null; + stepBucketHistogram = null; } } @Override protected void recordNonNegative(double amount) { - count.increment(); - total.add(amount); - max.record(amount); + if (stepBucketHistogram != null) { + stepBucketHistogram.recordDouble(amount); + } + super.recordNonNegative(amount); } @Override public long count() { - return countTotal.poll1(); - } - - @Override - public double totalAmount() { - return countTotal.poll2(); - } - - @Override - public double max() { - return max.poll(); + if (stepBucketHistogram != null) { + // Force the stepBucketHistogram to be aligned to step by calling count. This + // ensures that all + // values exported by the Timer are measured for the same step. + stepBucketHistogram.poll(); + } + return super.count(); } @Override public HistogramSnapshot takeSnapshot() { HistogramSnapshot currentSnapshot = super.takeSnapshot(); - if (deltaHistogramCounts == null) { + if (stepBucketHistogram == null) { return currentSnapshot; } - return new HistogramSnapshot(currentSnapshot.count(), // Already delta in sfx - // implementation. - currentSnapshot.total(), // Already delta in sfx implementation. - currentSnapshot.max(), // Max cannot be calculated as delta, keep the - // current. - currentSnapshot.percentileValues(), // No changes to the percentile - // values. - deltaHistogramCounts.calculate(currentSnapshot.histogramCounts()), currentSnapshot::outputSummary); + return new HistogramSnapshot(currentSnapshot.count(), currentSnapshot.total(), currentSnapshot.max(), + currentSnapshot.percentileValues(), stepBucketHistogram.poll(), currentSnapshot::outputSummary); } } diff --git a/implementations/micrometer-registry-signalfx/src/main/java/io/micrometer/signalfx/SignalfxTimer.java b/implementations/micrometer-registry-signalfx/src/main/java/io/micrometer/signalfx/SignalfxTimer.java index b2386d86c2..cfa65a3863 100644 --- a/implementations/micrometer-registry-signalfx/src/main/java/io/micrometer/signalfx/SignalfxTimer.java +++ b/implementations/micrometer-registry-signalfx/src/main/java/io/micrometer/signalfx/SignalfxTimer.java @@ -16,91 +16,75 @@ package io.micrometer.signalfx; import io.micrometer.common.lang.Nullable; -import io.micrometer.core.instrument.AbstractTimer; import io.micrometer.core.instrument.Clock; import io.micrometer.core.instrument.distribution.DistributionStatisticConfig; import io.micrometer.core.instrument.distribution.HistogramSnapshot; -import io.micrometer.core.instrument.distribution.TimeWindowMax; +import io.micrometer.core.instrument.distribution.StepBucketHistogram; import io.micrometer.core.instrument.distribution.pause.PauseDetector; -import io.micrometer.core.instrument.step.StepTuple2; -import io.micrometer.core.instrument.util.TimeUtils; +import io.micrometer.core.instrument.step.StepTimer; import java.util.concurrent.TimeUnit; -import java.util.concurrent.atomic.LongAdder; /** - * This class is mostly the same as {@link io.micrometer.core.instrument.step.StepTimer}, - * with one notable difference: the {@link DistributionStatisticConfig} is modified before - * being passed to the super class constructor - that forces the histogram generated by - * this meter to be cumulative. + * A StepTimer which provides support for multiple flavours of Histograms to be recorded + * based on {@link SignalFxConfig#publishCumulativeHistogram()} and + * {@link SignalFxConfig#publishDeltaHistogram()}. * * @author Bogdan Drutu * @author Mateusz Rzeszutek + * @author Lenin Jaganathan */ -final class SignalfxTimer extends AbstractTimer { - - private final LongAdder count = new LongAdder(); - - private final LongAdder total = new LongAdder(); - - private final StepTuple2 countTotal; - - private final TimeWindowMax max; +final class SignalfxTimer extends StepTimer { @Nullable - private final DeltaHistogramCounts deltaHistogramCounts; + private final StepBucketHistogram stepBucketHistogram; SignalfxTimer(Id id, Clock clock, DistributionStatisticConfig distributionStatisticConfig, PauseDetector pauseDetector, TimeUnit baseTimeUnit, long stepMillis, boolean isDelta) { - super(id, clock, CumulativeHistogramConfigUtil.updateConfig(distributionStatisticConfig), pauseDetector, - baseTimeUnit, false); - countTotal = new StepTuple2<>(clock, stepMillis, 0L, 0L, count::sumThenReset, total::sumThenReset); - max = new TimeWindowMax(clock, distributionStatisticConfig); - if (distributionStatisticConfig.isPublishingHistogram() && isDelta) { - deltaHistogramCounts = new DeltaHistogramCounts(); + super(id, clock, distributionStatisticConfig, pauseDetector, baseTimeUnit, stepMillis, defaultHistogram(clock, + CumulativeHistogramConfigUtil.updateConfig(distributionStatisticConfig, isDelta), false)); + + double[] slo = distributionStatisticConfig.getServiceLevelObjectiveBoundaries(); + if (slo != null && slo.length > 0 && isDelta) { + stepBucketHistogram = new StepBucketHistogram(clock, stepMillis, + DistributionStatisticConfig.builder() + .serviceLevelObjectives(CumulativeHistogramConfigUtil.addPositiveInfBucket(slo)) + .build() + .merge(distributionStatisticConfig), + false, true); } else { - deltaHistogramCounts = null; + stepBucketHistogram = null; } } @Override protected void recordNonNegative(long amount, TimeUnit unit) { - final long nanoAmount = (long) TimeUtils.convert(amount, unit, TimeUnit.NANOSECONDS); - count.increment(); - total.add(nanoAmount); - max.record(amount, unit); + if (stepBucketHistogram != null) { + stepBucketHistogram.recordLong(TimeUnit.NANOSECONDS.convert(amount, unit)); + } + super.recordNonNegative(amount, unit); } @Override public long count() { - return countTotal.poll1(); - } - - @Override - public double totalTime(TimeUnit unit) { - return TimeUtils.nanosToUnit(countTotal.poll2(), unit); - } - - @Override - public double max(TimeUnit unit) { - return max.poll(unit); + if (stepBucketHistogram != null) { + // Force the stepBucketHistogram to be aligned to step by calling count. This + // ensures that all + // values exported by the Timer are measured for the same step. + stepBucketHistogram.poll(); + } + return super.count(); } @Override public HistogramSnapshot takeSnapshot() { HistogramSnapshot currentSnapshot = super.takeSnapshot(); - if (deltaHistogramCounts == null) { + if (stepBucketHistogram == null) { return currentSnapshot; } - return new HistogramSnapshot(currentSnapshot.count(), // Already delta in sfx - // implementation - currentSnapshot.total(), // Already delta in sfx implementation - currentSnapshot.max(), // Max cannot be calculated as delta, keep the - // current. - currentSnapshot.percentileValues(), // No changes to the percentile - // values. - deltaHistogramCounts.calculate(currentSnapshot.histogramCounts()), currentSnapshot::outputSummary); + return new HistogramSnapshot(currentSnapshot.count(), currentSnapshot.total(), currentSnapshot.max(), + currentSnapshot.percentileValues(), stepBucketHistogram.poll(), currentSnapshot::outputSummary); } } diff --git a/implementations/micrometer-registry-signalfx/src/test/java/io/micrometer/signalfx/DeltaHistogramCountsTest.java b/implementations/micrometer-registry-signalfx/src/test/java/io/micrometer/signalfx/DeltaHistogramCountsTest.java deleted file mode 100644 index 25af71365f..0000000000 --- a/implementations/micrometer-registry-signalfx/src/test/java/io/micrometer/signalfx/DeltaHistogramCountsTest.java +++ /dev/null @@ -1,45 +0,0 @@ -/* - * Copyright 2022 VMware, Inc. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package io.micrometer.signalfx; - -import io.micrometer.core.instrument.distribution.CountAtBucket; -import org.junit.jupiter.api.Test; - -import static org.assertj.core.api.Assertions.assertThat; - -class DeltaHistogramCountsTest { - - @Test - void empty() { - DeltaHistogramCounts deltaHistogramCounts = new DeltaHistogramCounts(); - assertThat(deltaHistogramCounts.calculate(new CountAtBucket[] {})).isEmpty(); - assertThat(deltaHistogramCounts.calculate(new CountAtBucket[] {})).isEmpty(); - } - - @Test - void nonEmpty() { - DeltaHistogramCounts deltaHistogramCounts = new DeltaHistogramCounts(); - CountAtBucket[] first = new CountAtBucket[] { new CountAtBucket(1.0, 0), new CountAtBucket(5.0, 1), - new CountAtBucket(Double.MAX_VALUE, 1) }; - assertThat(deltaHistogramCounts.calculate(first)).isEqualTo(first); - CountAtBucket[] second = new CountAtBucket[] { new CountAtBucket(1.0, 0), new CountAtBucket(5.0, 2), - new CountAtBucket(Double.MAX_VALUE, 3) }; - assertThat(deltaHistogramCounts.calculate(second)).isEqualTo(new CountAtBucket[] { new CountAtBucket(1.0, 0), - new CountAtBucket(5.0, 1), new CountAtBucket(Double.MAX_VALUE, 2) }); - } - -} diff --git a/implementations/micrometer-registry-signalfx/src/test/java/io/micrometer/signalfx/SignalFxMeterRegistryTest.java b/implementations/micrometer-registry-signalfx/src/test/java/io/micrometer/signalfx/SignalFxMeterRegistryTest.java index 47814e2d34..5d1f6f5f56 100644 --- a/implementations/micrometer-registry-signalfx/src/test/java/io/micrometer/signalfx/SignalFxMeterRegistryTest.java +++ b/implementations/micrometer-registry-signalfx/src/test/java/io/micrometer/signalfx/SignalFxMeterRegistryTest.java @@ -16,10 +16,12 @@ package io.micrometer.signalfx; import com.signalfx.metrics.protobuf.SignalFxProtocolBuffers; +import io.micrometer.core.Issue; import io.micrometer.core.instrument.DistributionSummary; import io.micrometer.core.instrument.MockClock; import io.micrometer.core.instrument.Timer; import io.micrometer.core.instrument.distribution.CountAtBucket; +import io.micrometer.core.instrument.step.PollingAwareMockStepClock; import io.micrometer.core.instrument.util.DoubleFormat; import org.assertj.core.api.Condition; import org.assertj.core.util.Arrays; @@ -521,6 +523,64 @@ void shouldNotExportCumulativeHistogramDataByDefault_DistributionSummary() { registry.close(); } + @Test + @Issue("3774") + void deltaHistogramCountsShouldMatchWithTimerCount() { + PollingAwareMockStepClock mockClock = new PollingAwareMockStepClock(cumulativeDeltaConfig); + SignalFxMeterRegistry registry = new SignalFxMeterRegistry(cumulativeDeltaConfig, mockClock); + Duration[] buckets = new Duration[] { Duration.ofMillis(10), Duration.ofMillis(50), Duration.ofMillis(100) }; + Timer timer = Timer.builder("my.timer").serviceLevelObjectives(buckets).register(registry); + + timer.record(5, TimeUnit.MILLISECONDS); + timer.record(20, TimeUnit.MILLISECONDS); + timer.record(175, TimeUnit.MILLISECONDS); + timer.record(2, TimeUnit.MILLISECONDS); + + // Advance time, so we are in the "next" step where currently recorded values will + // be reported. + mockClock.add(config.step(), registry); + mockClock.add(config.step().dividedBy(2), registry); + // Recording for next step has started. But these values should be reported only + // in then next step. + timer.record(5, TimeUnit.MILLISECONDS); + timer.record(25, TimeUnit.MILLISECONDS); + timer.record(75, TimeUnit.MILLISECONDS); + timer.record(125, TimeUnit.MILLISECONDS); + timer.record(500, TimeUnit.MILLISECONDS); + + // Current is not elapsed, so only the values recorded before start of this step + // should be reflected yet. + // Assert that data recorded for previous step is available. + List dataPoints = getDataPoints(registry, mockClock.wallTime()); + assertThat(dataPoints).hasSize(8) + .has(gaugePoint("my.timer.avg", 0.0505), atIndex(0)) + .has(counterPoint("my.timer.count", 4), atIndex(1)) + .has(allOf(counterPoint("my.timer.histogram", 4), bucket("+Inf")), atIndex(2)) + .has(allOf(counterPoint("my.timer.histogram", 2), bucket(buckets[0])), atIndex(3)) + .has(allOf(counterPoint("my.timer.histogram", 3), bucket(buckets[1])), atIndex(4)) + .has(allOf(counterPoint("my.timer.histogram", 3), bucket(buckets[2])), atIndex(5)) + .has(gaugePoint("my.timer.max", 0.5), atIndex(6)) + .has(counterPoint("my.timer.totalTime", 0.202), atIndex(7)); + + // Advance time, so we are in the "next" step where currently recorded values will + // be reported. + mockClock.add(config.step(), registry); + + dataPoints = getDataPoints(registry, mockClock.wallTime()); + // Assert that data recorded for previous step is available. + assertThat(dataPoints).hasSize(8) + .has(gaugePoint("my.timer.avg", 0.146), atIndex(0)) + .has(counterPoint("my.timer.count", 5), atIndex(1)) + .has(allOf(counterPoint("my.timer.histogram", 5), bucket("+Inf")), atIndex(2)) + .has(allOf(counterPoint("my.timer.histogram", 1), bucket(buckets[0])), atIndex(3)) + .has(allOf(counterPoint("my.timer.histogram", 2), bucket(buckets[1])), atIndex(4)) + .has(allOf(counterPoint("my.timer.histogram", 3), bucket(buckets[2])), atIndex(5)) + .has(gaugePoint("my.timer.max", 0.5), atIndex(6)) + .has(counterPoint("my.timer.totalTime", 0.73), atIndex(7)); + + registry.close(); + } + private static List getDataPoints(SignalFxMeterRegistry registry, long timestamp) { return registry.getMeters() diff --git a/micrometer-core/src/main/java/io/micrometer/core/instrument/step/StepDistributionSummary.java b/micrometer-core/src/main/java/io/micrometer/core/instrument/step/StepDistributionSummary.java index bea3eead37..736b4ec8d2 100644 --- a/micrometer-core/src/main/java/io/micrometer/core/instrument/step/StepDistributionSummary.java +++ b/micrometer-core/src/main/java/io/micrometer/core/instrument/step/StepDistributionSummary.java @@ -20,6 +20,7 @@ import io.micrometer.core.instrument.Measurement; import io.micrometer.core.instrument.Statistic; import io.micrometer.core.instrument.distribution.DistributionStatisticConfig; +import io.micrometer.core.instrument.distribution.Histogram; import io.micrometer.core.instrument.distribution.TimeWindowMax; import java.util.Arrays; @@ -53,7 +54,13 @@ public class StepDistributionSummary extends AbstractDistributionSummary impleme */ public StepDistributionSummary(Id id, Clock clock, DistributionStatisticConfig distributionStatisticConfig, double scale, long stepMillis, boolean supportsAggregablePercentiles) { - super(id, clock, distributionStatisticConfig, scale, supportsAggregablePercentiles); + this(id, clock, distributionStatisticConfig, scale, stepMillis, + defaultHistogram(clock, distributionStatisticConfig, supportsAggregablePercentiles)); + } + + protected StepDistributionSummary(Id id, Clock clock, DistributionStatisticConfig distributionStatisticConfig, + double scale, long stepMillis, Histogram histogram) { + super(id, scale, histogram); this.countTotal = new StepTuple2<>(clock, stepMillis, 0L, 0.0, count::sumThenReset, total::sumThenReset); this.max = new TimeWindowMax(clock, distributionStatisticConfig); } diff --git a/micrometer-core/src/main/java/io/micrometer/core/instrument/step/StepTimer.java b/micrometer-core/src/main/java/io/micrometer/core/instrument/step/StepTimer.java index 6761e65ef7..36d0dbd7a6 100644 --- a/micrometer-core/src/main/java/io/micrometer/core/instrument/step/StepTimer.java +++ b/micrometer-core/src/main/java/io/micrometer/core/instrument/step/StepTimer.java @@ -18,6 +18,7 @@ import io.micrometer.core.instrument.AbstractTimer; import io.micrometer.core.instrument.Clock; import io.micrometer.core.instrument.distribution.DistributionStatisticConfig; +import io.micrometer.core.instrument.distribution.Histogram; import io.micrometer.core.instrument.distribution.TimeWindowMax; import io.micrometer.core.instrument.distribution.pause.PauseDetector; import io.micrometer.core.instrument.util.TimeUtils; @@ -51,7 +52,14 @@ public class StepTimer extends AbstractTimer implements StepMeter { public StepTimer(final Id id, final Clock clock, final DistributionStatisticConfig distributionStatisticConfig, final PauseDetector pauseDetector, final TimeUnit baseTimeUnit, final long stepDurationMillis, final boolean supportsAggregablePercentiles) { - super(id, clock, distributionStatisticConfig, pauseDetector, baseTimeUnit, supportsAggregablePercentiles); + this(id, clock, distributionStatisticConfig, pauseDetector, baseTimeUnit, stepDurationMillis, + defaultHistogram(clock, distributionStatisticConfig, supportsAggregablePercentiles)); + } + + protected StepTimer(final Id id, final Clock clock, final DistributionStatisticConfig distributionStatisticConfig, + final PauseDetector pauseDetector, final TimeUnit baseTimeUnit, final long stepDurationMillis, + Histogram histogram) { + super(id, clock, pauseDetector, baseTimeUnit, histogram); countTotal = new StepTuple2<>(clock, stepDurationMillis, 0L, 0L, count::sumThenReset, total::sumThenReset); max = new TimeWindowMax(clock, distributionStatisticConfig); } diff --git a/micrometer-test/src/main/java/io/micrometer/core/instrument/step/PollingAwareMockStepClock.java b/micrometer-test/src/main/java/io/micrometer/core/instrument/step/PollingAwareMockStepClock.java new file mode 100644 index 0000000000..459f8da22e --- /dev/null +++ b/micrometer-test/src/main/java/io/micrometer/core/instrument/step/PollingAwareMockStepClock.java @@ -0,0 +1,82 @@ +/* + * Copyright 2023 VMware, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.micrometer.core.instrument.step; + +import static java.util.concurrent.TimeUnit.MILLISECONDS; + +import java.time.Duration; +import java.util.concurrent.TimeUnit; + +import io.micrometer.core.annotation.Incubating; +import io.micrometer.core.instrument.Clock; +import io.micrometer.core.instrument.util.TimeUtils; + +/** + * A clock meant to be used for testing {@link StepMeterRegistry}. This clock does the + * {@link StepMeterRegistry#pollMetersToRollover()} whenever the step is crossed thus + * simulating the expected behaviour of step meters. + */ +@Incubating(since = "1.11.1") +public class PollingAwareMockStepClock implements Clock { + + private final Duration step; + + private long timeNanos = (long) TimeUtils.millisToUnit(1, TimeUnit.NANOSECONDS); + + public PollingAwareMockStepClock(final StepRegistryConfig stepRegistryConfig) { + this.step = stepRegistryConfig.step(); + } + + @Override + public long wallTime() { + return MILLISECONDS.convert(timeNanos, TimeUnit.NANOSECONDS); + } + + @Override + public long monotonicTime() { + return timeNanos; + } + + /** + * Advances clock by the duration specified and does + * {@link StepMeterRegistry#pollMetersToRollover()} if necessary. + * @param duration - amount to increment by. + * @param stepMeterRegistry - {@link StepMeterRegistry} to which clock is tied to. + * @return current time after adding step. + */ + public long add(Duration duration, StepMeterRegistry stepMeterRegistry) { + addTimeWithRolloverOnStepStart(duration, stepMeterRegistry); + return timeNanos; + } + + private void addTimeWithRolloverOnStepStart(Duration timeToAdd, StepMeterRegistry stepMeterRegistry) { + while (timeToAdd.toMillis() >= step.toMillis()) { + long boundaryForNextStep = ((timeNanos / step.toMillis()) + 1) * step.toMillis(); + Duration timeToNextStep = Duration.ofMillis(boundaryForNextStep - timeNanos); + if (timeToAdd.toMillis() >= timeToNextStep.toMillis()) { + timeToAdd = timeToAdd.minus(timeToNextStep); + addTimeToClock(timeToNextStep); + stepMeterRegistry.pollMetersToRollover(); + } + } + addTimeToClock(timeToAdd); + } + + private void addTimeToClock(Duration duration) { + timeNanos += duration.toNanos(); + } + +} diff --git a/micrometer-test/src/main/java/io/micrometer/core/instrument/step/package-info.java b/micrometer-test/src/main/java/io/micrometer/core/instrument/step/package-info.java new file mode 100644 index 0000000000..f5dd3edeae --- /dev/null +++ b/micrometer-test/src/main/java/io/micrometer/core/instrument/step/package-info.java @@ -0,0 +1,16 @@ +/* + * Copyright 2023 VMware, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.micrometer.core.instrument.step;