Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Include minimumExpectedValue in exponential histogram #5394

Merged
merged 1 commit into from
Aug 21, 2024
Merged

Include minimumExpectedValue in exponential histogram #5394

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 19 additions & 7 deletions ...ry-otlp/src/main/java/io/micrometer/registry/otlp/internal/Base2ExponentialHistogram.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,25 +73,37 @@ public abstract class Base2ExponentialHistogram implements Histogram {
* maxBucketsCount.
* @param maxBucketsCount - maximum number of buckets that can be used for
* distribution.
* @param zeroThreshold - values less than or equal to this are considered in zero
* count and recorded in the histogram. If less than 0, this is rounded to zero. In
* case of recording time, this should be in nanoseconds.
* @param minimumExpectedValue - values less than this are considered in zero count
* and not recorded in the histogram. If less than 0, this is rounded to zero. In case
* of recording time, this should be in nanoseconds.
* @param baseUnit - an Optional TimeUnit. If set to a non-null unit, the recorded
* values are converted to this unit.
*/
Base2ExponentialHistogram(int maxScale, int maxBucketsCount, double zeroThreshold, @Nullable TimeUnit baseUnit) {
Base2ExponentialHistogram(int maxScale, int maxBucketsCount, double minimumExpectedValue,
@Nullable TimeUnit baseUnit) {
this.maxScale = maxScale;
this.scale = maxScale;
this.maxBucketsCount = maxBucketsCount;
this.baseUnit = baseUnit;
// Convert the zeroThreshold to baseUnit.
this.zeroThreshold = Math.max(baseUnit != null ? TimeUtils.nanosToUnit(zeroThreshold, baseUnit) : zeroThreshold,
0.0);
this.zeroThreshold = getZeroThreshHoldFromMinExpectedValue(minimumExpectedValue, baseUnit);

this.circularCountHolder = new CircularCountHolder(maxBucketsCount);
this.base2IndexProvider = IndexProviderFactory.getIndexProviderForScale(scale);
}

/**
* Convert the minimumExpectedValue to zeroThreshold. Micrometer's
* minimumExpectedValue should be included as part of distribution whereas Exponential
* Histogram will exclude the zeroThreshold from distribution. Hence, we find the next
* smallest value from minimumExpectedValue and use it as zeroThreshold.
*/
private static double getZeroThreshHoldFromMinExpectedValue(final double minimumExpectedValue,
final @Nullable TimeUnit baseUnit) {
double minValueScaledToTime = baseUnit != null ? TimeUtils.nanosToUnit(minimumExpectedValue, baseUnit)
: minimumExpectedValue;
return Math.max(Math.nextDown(minValueScaledToTime), 0.0);
}

/**
* Returns the latest snapshot of recordings from
* {@link Base2ExponentialHistogram#takeExponentialHistogramSnapShot()} and not the
Expand Down
10 changes: 4 additions & 6 deletions ...istry-otlp/src/test/java/io/micrometer/registry/otlp/OtlpCumulativeMeterRegistryTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -564,15 +564,14 @@ void testExponentialHistogramWithTimer() {
.tags(Tags.of(meterTag))
.publishPercentileHistogram()
.register(registryWithExponentialHistogram);
timer.record(Duration.ofMillis(1));
timer.record(Duration.ofMillis(100));
timer.record(Duration.ofMillis(1000));

Metric metric = writeToMetric(timer);
assertThat(metric.getExponentialHistogram().getDataPointsCount()).isPositive();

ExponentialHistogramDataPoint exponentialHistogramDataPoint = metric.getExponentialHistogram().getDataPoints(0);
assertExponentialHistogram(metric, 3, 1101, 0.0, 1, 5);
assertExponentialHistogram(metric, 2, 1100, 0.0, 0, 5);
ExponentialHistogramDataPoint.Buckets buckets = exponentialHistogramDataPoint.getPositive();
assertThat(buckets.getOffset()).isEqualTo(212);
assertThat(buckets.getBucketCountsCount()).isEqualTo(107);
Expand All @@ -590,7 +589,7 @@ void testExponentialHistogramWithTimer() {
assertThat(exponentialHistogramDataPoint.getTimeUnixNano() - previousEndTime)
.isEqualTo(otlpConfig().step().toNanos());

assertExponentialHistogram(metric, 4, 11101, 0.0, 1, 4);
assertExponentialHistogram(metric, 3, 11100, 0.0, 0, 4);

buckets = exponentialHistogramDataPoint.getPositive();
assertThat(buckets.getOffset()).isEqualTo(106);
Expand All @@ -608,15 +607,14 @@ void testExponentialHistogramDs() {
.tags(Tags.of(meterTag))
.publishPercentileHistogram()
.register(registryWithExponentialHistogram);
ds.record(1);
ds.record(100);
ds.record(1000);

Metric metric = writeToMetric(ds);
assertThat(metric.getExponentialHistogram().getDataPointsCount()).isPositive();

ExponentialHistogramDataPoint exponentialHistogramDataPoint = metric.getExponentialHistogram().getDataPoints(0);
assertExponentialHistogram(metric, 3, 1101, 0.0, 1, 5);
assertExponentialHistogram(metric, 2, 1100, 0.0, 0, 5);
ExponentialHistogramDataPoint.Buckets buckets = exponentialHistogramDataPoint.getPositive();
assertThat(buckets.getOffset()).isEqualTo(212);
assertThat(buckets.getBucketCountsCount()).isEqualTo(107);
Expand All @@ -634,7 +632,7 @@ void testExponentialHistogramDs() {
assertThat(exponentialHistogramDataPoint.getTimeUnixNano() - previousEndTime)
.isEqualTo(otlpConfig().step().toNanos());

assertExponentialHistogram(metric, 4, 11101, 0.0, 1, 4);
assertExponentialHistogram(metric, 3, 11100, 0.0, 0, 4);

buckets = exponentialHistogramDataPoint.getPositive();
assertThat(buckets.getOffset()).isEqualTo(106);
Expand Down
6 changes: 2 additions & 4 deletions ...r-registry-otlp/src/test/java/io/micrometer/registry/otlp/OtlpDeltaMeterRegistryTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -513,7 +513,6 @@ void testExponentialHistogramWithTimer() {
.tags(Tags.of(meterTag))
.publishPercentileHistogram()
.register(registryWithExponentialHistogram);
timer.record(Duration.ofMillis(1));
timer.record(Duration.ofMillis(100));
timer.record(Duration.ofMillis(1000));

Expand All @@ -524,7 +523,7 @@ void testExponentialHistogramWithTimer() {
Metric metric = writeToMetric(timer);
assertThat(metric.getExponentialHistogram().getDataPointsCount()).isPositive();
ExponentialHistogramDataPoint exponentialHistogramDataPoint = metric.getExponentialHistogram().getDataPoints(0);
assertExponentialHistogram(metric, 3, 1101, 1000.0, 1, 5);
assertExponentialHistogram(metric, 2, 1100, 1000.0, 0, 5);
ExponentialHistogramDataPoint.Buckets buckets = exponentialHistogramDataPoint.getPositive();
assertThat(buckets.getOffset()).isEqualTo(212);
assertThat(buckets.getBucketCountsCount()).isEqualTo(107);
Expand Down Expand Up @@ -567,7 +566,6 @@ void testExponentialHistogramDs() {
.tags(Tags.of(meterTag))
.publishPercentileHistogram()
.register(registryWithExponentialHistogram);
ds.record(1);
ds.record(100);
ds.record(1000);

Expand All @@ -578,7 +576,7 @@ void testExponentialHistogramDs() {
Metric metric = writeToMetric(ds);
assertThat(metric.getExponentialHistogram().getDataPointsCount()).isPositive();
ExponentialHistogramDataPoint exponentialHistogramDataPoint = metric.getExponentialHistogram().getDataPoints(0);
assertExponentialHistogram(metric, 3, 1101, 1000.0, 1, 5);
assertExponentialHistogram(metric, 2, 1100, 1000.0, 0, 5);
ExponentialHistogramDataPoint.Buckets buckets = exponentialHistogramDataPoint.getPositive();
assertThat(buckets.getOffset()).isEqualTo(212);
assertThat(buckets.getBucketCountsCount()).isEqualTo(107);
Expand Down
9 changes: 4 additions & 5 deletions ...ometer-registry-otlp/src/test/java/io/micrometer/registry/otlp/OtlpMeterRegistryTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -308,15 +308,14 @@ void testZeroCountForExponentialHistogram() {

ExponentialHistogramDataPoint dataPoint = writeToMetric(timerWithZero1ms).getExponentialHistogram()
.getDataPoints(0);
assertThat(dataPoint.getZeroCount()).isEqualTo(2);
assertThat(dataPoint.getZeroCount()).isEqualTo(1);
assertThat(dataPoint.getCount()).isEqualTo(2);
assertThat(dataPoint.getPositive().getBucketCountsCount()).isZero();
assertThat(dataPoint.getPositive().getBucketCountsCount()).isEqualTo(1);

dataPoint = writeToMetric(timerWithZero1ns).getExponentialHistogram().getDataPoints(0);
assertThat(dataPoint.getZeroCount()).isEqualTo(1);
assertThat(dataPoint.getZeroCount()).isZero();
assertThat(dataPoint.getCount()).isEqualTo(2);
assertThat(dataPoint.getPositive().getBucketCountsCount()).isEqualTo(1);
assertThat(dataPoint.getPositive().getBucketCountsList()).isEqualTo(List.of(1L));
assertThat(dataPoint.getPositive().getBucketCountsCount()).isGreaterThan(1);
}

@Test
Expand Down
47 changes: 29 additions & 18 deletions ...tlp/src/test/java/io/micrometer/registry/otlp/internal/Base2ExponentialHistogramTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,9 @@ void testRecordTimeBased() {
// recordDouble(2).

ExponentialHistogramSnapShot currentSnapshot = base2ExponentialHistogram.getCurrentValuesSnapshot();
assertThat(currentSnapshot.zeroCount()).isEqualTo(1);
assertThat(currentSnapshot.scale()).isEqualTo(MAX_SCALE);
assertThat(getAllBucketsCountSum(currentSnapshot)).isEqualTo(1);
assertThat(base2ExponentialHistogram.getCurrentValuesSnapshot().positive().offset()).isEqualTo(1023);
assertThat(currentSnapshot.zeroCount()).isZero();
assertThat(currentSnapshot.scale()).isLessThan(MAX_SCALE);
assertThat(getAllBucketsCountSum(currentSnapshot)).isEqualTo(2);
}

@Test
Expand All @@ -86,17 +85,14 @@ void testRecordTimeBasedInSeconds() {
base2ExponentialHistogram.recordLong(Duration.ofMillis(50).toNanos());

ExponentialHistogramSnapShot currentSnapshot = base2ExponentialHistogram.getCurrentValuesSnapshot();
assertThat(currentSnapshot.zeroCount()).isEqualTo(1);
assertThat(currentSnapshot.scale()).isEqualTo(MAX_SCALE);
assertThat(getAllBucketsCountSum(currentSnapshot)).isEqualTo(1);
assertThat(base2ExponentialHistogram.getCurrentValuesSnapshot().positive().offset()).isEqualTo(-4426);
assertThat(currentSnapshot.zeroCount()).isZero();
assertThat(currentSnapshot.scale()).isLessThan(MAX_SCALE);
assertThat(getAllBucketsCountSum(currentSnapshot)).isEqualTo(2);

base2ExponentialHistogram.recordLong(Duration.ofMillis(90).toNanos());
currentSnapshot = base2ExponentialHistogram.getCurrentValuesSnapshot();
assertThat(currentSnapshot.zeroCount()).isEqualTo(1);
assertThat(currentSnapshot.scale()).isEqualTo(4);
assertThat(getAllBucketsCountSum(currentSnapshot)).isEqualTo(2);
assertThat(base2ExponentialHistogram.getCurrentValuesSnapshot().positive().offset()).isEqualTo(-70);
assertThat(currentSnapshot.scale()).isEqualTo(1);
assertThat(getAllBucketsCountSum(currentSnapshot)).isEqualTo(3);
}

@Test
Expand All @@ -106,9 +102,22 @@ void testZeroThreshHold() {
base2ExponentialHistogram.recordDouble(0.5);

ExponentialHistogramSnapShot currentSnapshot = base2ExponentialHistogram.getCurrentValuesSnapshot();
assertThat(currentSnapshot.zeroCount()).isEqualTo(3);
assertThat(currentSnapshot.zeroThreshold()).isLessThan(1).isGreaterThan(0);
assertThat(currentSnapshot.zeroCount()).isEqualTo(2);
assertThat(currentSnapshot.scale()).isEqualTo(MAX_SCALE);
assertThat(getAllBucketsCountSum(currentSnapshot)).isZero();
assertThat(getAllBucketsCountSum(currentSnapshot)).isEqualTo(1);

Base2ExponentialHistogram base2ExponentialHistogramWithZeroAsMin = new CumulativeBase2ExponentialHistogram(
MAX_SCALE, MAX_BUCKETS_COUNT, 0.0, null);
base2ExponentialHistogramWithZeroAsMin.recordDouble(0.0);
base2ExponentialHistogramWithZeroAsMin.recordDouble(Math.nextUp(0.0));

ExponentialHistogramSnapShot snapshotWithZeroAsMin = base2ExponentialHistogramWithZeroAsMin
.getCurrentValuesSnapshot();
assertThat(snapshotWithZeroAsMin.zeroThreshold()).isEqualTo(0.0);
assertThat(snapshotWithZeroAsMin.zeroCount()).isEqualTo(1);
assertThat(snapshotWithZeroAsMin.scale()).isEqualTo(MAX_SCALE);
assertThat(getAllBucketsCountSum(snapshotWithZeroAsMin)).isEqualTo(1);
}

@Test
Expand Down Expand Up @@ -229,19 +238,21 @@ void testUpscaleForNegativeScale() {

@Test
void reset() {
base2ExponentialHistogram.recordDouble(Math.nextDown(1.0));
base2ExponentialHistogram.recordDouble(1);
base2ExponentialHistogram.recordDouble(2);

ExponentialHistogramSnapShot currentSnapshot = base2ExponentialHistogram.getCurrentValuesSnapshot();
final int intialScale = currentSnapshot.scale();
assertThat(currentSnapshot.zeroCount()).isEqualTo(1);
assertThat(currentSnapshot.scale()).isEqualTo(MAX_SCALE);
assertThat(currentSnapshot.positive().offset()).isEqualTo(1023);
assertThat(getAllBucketsCountSum(currentSnapshot)).isEqualTo(1);
assertThat(currentSnapshot.scale()).isLessThan(MAX_SCALE);
assertThat(currentSnapshot.positive().offset()).isNotZero();
assertThat(getAllBucketsCountSum(currentSnapshot)).isEqualTo(2);

base2ExponentialHistogram.reset();
currentSnapshot = base2ExponentialHistogram.getCurrentValuesSnapshot();
assertThat(currentSnapshot.zeroCount()).isZero();
assertThat(currentSnapshot.scale()).isEqualTo(MAX_SCALE);
assertThat(currentSnapshot.scale()).isEqualTo(intialScale);
assertThat(currentSnapshot.positive().offset()).isZero();
assertThat(getAllBucketsCountSum(currentSnapshot)).isZero();
}
Expand Down
2 changes: 1 addition & 1 deletion ...rc/test/java/io/micrometer/registry/otlp/internal/DeltaBase2ExponentialHistogramTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ void setUp() {

@Test
void snapshotShouldBeSameForOneStep() {
deltaBase2ExponentialHistogram.recordDouble(1.0);
deltaBase2ExponentialHistogram.recordDouble(0.5);
deltaBase2ExponentialHistogram.recordDouble(2.0);

ExponentialHistogramSnapShot exponentialHistogramSnapShot = deltaBase2ExponentialHistogram
Expand Down