Skip to content

Commit

Permalink
Support bucket configuration for Histograms
Browse files Browse the repository at this point in the history
  • Loading branch information
lenin-jaganathan committed Aug 8, 2024
1 parent 718f3c2 commit 56238b2
Show file tree
Hide file tree
Showing 11 changed files with 192 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -218,12 +218,14 @@ default Map<String, String> headers() {
* @since 1.14.0
*/
default HistogramFlavour histogramFlavour() {
String histogramPreference = System.getenv().get("OTEL_EXPORTER_OTLP_METRICS_DEFAULT_HISTOGRAM_AGGREGATION");
if (histogramPreference == null) {
return getEnum(this, HistogramFlavour.class, "histogramFlavour")
.orElse(HistogramFlavour.EXPLICIT_BUCKET_HISTOGRAM);
}
return HistogramFlavour.fromString(histogramPreference);
return getEnum(this, HistogramFlavour.class, "histogramFlavour").orElseGet(() -> {
String histogramPreference = System.getenv()
.get("OTEL_EXPORTER_OTLP_METRICS_DEFAULT_HISTOGRAM_AGGREGATION");
if (histogramPreference != null) {
return HistogramFlavour.fromString(histogramPreference);
}
return HistogramFlavour.EXPLICIT_BUCKET_HISTOGRAM;
});
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,7 @@ protected TimeUnit getBaseTimeUnit() {
protected DistributionStatisticConfig defaultHistogramConfig() {
return DistributionStatisticConfig.builder()
.expiry(this.config.step())
.maxBucketCount(this.config.maxBucketCount())
.build()
.merge(DistributionStatisticConfig.DEFAULT);
}
Expand Down Expand Up @@ -405,10 +406,12 @@ static Histogram getHistogram(final Clock clock, final DistributionStatisticConf
minimumExpectedValue = 0.0;
}

final int maxBucketCount = distributionStatisticConfig.getMaxBucketCount() != null
? distributionStatisticConfig.getMaxBucketCount() : otlpConfig.maxBucketCount();
return otlpConfig.aggregationTemporality() == AggregationTemporality.DELTA
? new DeltaBase2ExponentialHistogram(otlpConfig.maxScale(), otlpConfig.maxBucketCount(),
? new DeltaBase2ExponentialHistogram(otlpConfig.maxScale(), maxBucketCount,
minimumExpectedValue, baseTimeUnit, clock, otlpConfig.step().toMillis())
: new CumulativeBase2ExponentialHistogram(otlpConfig.maxScale(), otlpConfig.maxBucketCount(),
: new CumulativeBase2ExponentialHistogram(otlpConfig.maxScale(), maxBucketCount,
minimumExpectedValue, baseTimeUnit);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -237,4 +237,43 @@ void invalidBaseTimeUnitShouldBeCaptured() {
assertThat(otlpConfig.validate().isValid()).isFalse();
}

@Test
void maxScaleAndMaxBucketsDefault() {
Map<String, String> properties = new HashMap<>();
properties.put("otlp.maxScale", "8");
properties.put("otlp.maxBucketCount", "80");

OtlpConfig otlpConfig = properties::get;
assertThat(otlpConfig.validate().isValid()).isTrue();
assertThat(otlpConfig.maxScale()).isSameAs(8);
assertThat(otlpConfig.maxBucketCount()).isSameAs(80);
}

@Test
void histogramPreference() {
Map<String, String> properties = new HashMap<>();
properties.put("otlp.histogramFlavour", "base2_exponential_bucket_histogram");

OtlpConfig otlpConfig = properties::get;
assertThat(otlpConfig.validate().isValid()).isTrue();
assertThat(otlpConfig.histogramFlavour()).isEqualTo(HistogramFlavour.BASE2_EXPONENTIAL_BUCKET_HISTOGRAM);
}

@Test
void histogramPreferenceConfigTakesPrecedenceOverEnvVars() throws Exception {
OtlpConfig config = k -> "base2_exponential_bucket_histogram";
withEnvironmentVariable("OTEL_EXPORTER_OTLP_METRICS_DEFAULT_HISTOGRAM_AGGREGATION", "explicit_bucket_histogram")
.execute(() -> assertThat(config.histogramFlavour())
.isEqualTo(HistogramFlavour.BASE2_EXPONENTIAL_BUCKET_HISTOGRAM));
}

@Test
void histogramPreferenceUseEnvVarWhenConfigNotSet() throws Exception {
OtlpConfig config = k -> null;
withEnvironmentVariable("OTEL_EXPORTER_OTLP_METRICS_DEFAULT_HISTOGRAM_AGGREGATION",
"base2_exponential_bucket_histogram")
.execute(() -> assertThat(config.histogramFlavour())
.isEqualTo(HistogramFlavour.BASE2_EXPONENTIAL_BUCKET_HISTOGRAM));
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,7 @@ void distributionSummary() {
void distributionSummaryWithHistogram() {
DistributionSummary size = DistributionSummary.builder("http.request.size")
.baseUnit(BaseUnits.BYTES)
.maxBucketCount(1000)
.publishPercentileHistogram()
.register(registry);
size.record(100);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,47 @@ void testGetSloWithPositiveInf() {
assertThat(OtlpMeterRegistry.getSloWithPositiveInf(distributionStatisticConfigWithInf)).hasSize(4);
}

@Test
void histogramWithCustomMaxBuckets() {
Timer timeWithExplicitBucketHistogram = Timer.builder("test.timer")
.maxBucketCount(10)
.publishPercentileHistogram(true)
.register(registry);

Timer timerWithExponentialHistogram = Timer.builder("test.timer1")
.publishPercentileHistogram(true)
.register(registryWithExponentialHistogram);
timerWithExponentialHistogram.record(Duration.ofMillis(2));
timerWithExponentialHistogram.record(Duration.ofMillis(60000)); // ~120 buckets
// with scale 3

Timer timerWithExponentialHistogramWithMaxBuckets = Timer.builder("test.timer2")
.maxBucketCount(80)
.publishPercentileHistogram(true)
.register(registryWithExponentialHistogram);
timerWithExponentialHistogramWithMaxBuckets.record(Duration.ofMillis(2));
timerWithExponentialHistogramWithMaxBuckets.record(Duration.ofMillis(60000)); // ~60
// buckets
// with
// scale
// 2

clock.add(exponentialHistogramOtlpConfig().step());
Metric metric = writeToMetric(timeWithExplicitBucketHistogram);
HistogramDataPoint dataPoint = metric.getHistogram().getDataPoints(0);
assertThat(dataPoint.getExplicitBoundsCount()).isEqualTo(10);

metric = writeToMetric(timerWithExponentialHistogram);
ExponentialHistogramDataPoint exponentialHistogramDataPoint = metric.getExponentialHistogram().getDataPoints(0);
assertThat(exponentialHistogramDataPoint.getScale()).isEqualTo(3);
assertThat(exponentialHistogramDataPoint.getPositive().getBucketCountsCount()).isLessThanOrEqualTo(160);

metric = writeToMetric(timerWithExponentialHistogramWithMaxBuckets);
exponentialHistogramDataPoint = metric.getExponentialHistogram().getDataPoints(0);
assertThat(exponentialHistogramDataPoint.getScale()).isEqualTo(2);
assertThat(exponentialHistogramDataPoint.getPositive().getBucketCountsCount()).isLessThanOrEqualTo(80);
}

@Test
abstract void testMetricsStartAndEndTime();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,18 @@ public B maximumExpectedValue(@Nullable Duration max) {
return (B) this;
}

/**
* Restricts the number of buckets/bin ranges used in histogram.
* @param maxBucketCount maximum number of buckets
* @return This builder
*/
public B maxBucketCount(@Nullable Integer maxBucketCount) {
if (maxBucketCount != null) {
this.distributionConfigBuilder.maxBucketCount(maxBucketCount);
}
return (B) this;
}

/**
* Statistics emanating from a timer like max, percentiles, and histogram counts decay
* over time to give greater weight to recent samples (exception: histogram counts are
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,18 @@ public Builder maximumExpectedValue(@Nullable Double max) {
return this;
}

/**
* Restricts the number of buckets/bin ranges used in histogram.
* @param maxBucketCount maximum number of buckets
* @return This builder
*/
public Builder maxBucketCount(@Nullable Integer maxBucketCount) {
if (maxBucketCount != null) {
this.distributionConfigBuilder.maxBucketCount(maxBucketCount);
}
return this;
}

/**
* Statistics emanating from a distribution summary like max, percentiles, and
* histogram counts decay over time to give greater weight to recent samples
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,18 @@ public Builder maximumExpectedValue(@Nullable Duration max) {
return this;
}

/**
* Restricts the number of buckets/bin ranges used in histogram.
* @param maxBucketCount maximum number of buckets
* @return This builder
*/
public Builder maxBucketCount(@Nullable Integer maxBucketCount) {
if (maxBucketCount != null) {
this.distributionConfigBuilder.maxBucketCount(maxBucketCount);
}
return this;
}

/**
* Statistics emanating from a timer like max, percentiles, and histogram counts
* decay over time to give greater weight to recent samples (exception: histogram
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -412,6 +412,11 @@ public Builder maximumExpectedValue(Duration max) {
return super.maximumExpectedValue(max);
}

@Override
public Builder maxBucketCount(Integer maxBucketCount) {
return super.maxBucketCount(maxBucketCount);
}

@Override
public Builder distributionStatisticExpiry(Duration expiry) {
return super.distributionStatisticExpiry(expiry);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import java.time.Duration;
import java.util.NavigableSet;
import java.util.TreeSet;
import java.util.stream.Collectors;
import java.util.stream.LongStream;

/**
Expand All @@ -41,6 +42,7 @@ public class DistributionStatisticConfig implements Mergeable<DistributionStatis
.percentilePrecision(1)
.minimumExpectedValue(1.0)
.maximumExpectedValue(Double.POSITIVE_INFINITY)
.maxBucketCount(Integer.MAX_VALUE)
.expiry(Duration.ofMinutes(2))
.bufferLength(3)
.build();
Expand Down Expand Up @@ -71,6 +73,9 @@ public class DistributionStatisticConfig implements Mergeable<DistributionStatis
@Nullable
private Integer bufferLength;

@Nullable
private Integer maxBucketCount;

public static Builder builder() {
return new Builder();
}
Expand All @@ -96,6 +101,7 @@ public DistributionStatisticConfig merge(DistributionStatisticConfig parent) {
this.minimumExpectedValue == null ? parent.minimumExpectedValue : this.minimumExpectedValue)
.maximumExpectedValue(
this.maximumExpectedValue == null ? parent.maximumExpectedValue : this.maximumExpectedValue)
.maxBucketCount(this.maxBucketCount == null ? parent.maxBucketCount : this.maxBucketCount)
.expiry(this.expiry == null ? parent.expiry : this.expiry)
.bufferLength(this.bufferLength == null ? parent.bufferLength : this.bufferLength)
.build();
Expand All @@ -121,6 +127,10 @@ public NavigableSet<Double> getHistogramBuckets(boolean supportsAggregablePercen
}
}

if (maxBucketCount != null && buckets.size() > maxBucketCount) {
return buckets.stream().limit(maxBucketCount).collect(Collectors.toCollection(TreeSet::new));
}

return buckets;
}

Expand Down Expand Up @@ -210,6 +220,11 @@ public Double getMaximumExpectedValueAsDouble() {
return maximumExpectedValue;
}

@Nullable
public Integer getMaxBucketCount() {
return maxBucketCount;
}

/**
* Statistics like max, percentiles, and histogram counts decay over time to give
* greater weight to recent samples (exception: histogram counts are cumulative for
Expand Down Expand Up @@ -453,6 +468,16 @@ public Builder bufferLength(@Nullable Integer bufferLength) {
return this;
}

/**
* Restricts the number of buckets/bin ranges used in histogram.
* @param maxBucketCount maximum number of buckets
* @return This builder
*/
public Builder maxBucketCount(@Nullable Integer maxBucketCount) {
config.maxBucketCount = maxBucketCount;
return this;
}

/**
* @return A new immutable distribution configuration.
*/
Expand Down Expand Up @@ -489,6 +514,10 @@ private void validate(DistributionStatisticConfig distributionStatisticConfig) {
+ ").");
}

if (config.maxBucketCount != null && config.maxBucketCount <= 0) {
rejectConfig("maxBucketCount (" + config.maxBucketCount + ") must be greater than zero");
}

if (distributionStatisticConfig.getServiceLevelObjectiveBoundaries() != null) {
for (double slo : distributionStatisticConfig.getServiceLevelObjectiveBoundaries()) {
if (slo <= 0) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,4 +75,32 @@ void expectedValueRangeValidation() {
.satisfies(cause -> assertThat(cause.getMessage()).startsWith("Invalid distribution configuration:"));
}

@Test
void maxBucketCountValidation() {
assertThatThrownBy(() -> DistributionStatisticConfig.builder().maxBucketCount(0).build())
.satisfies(cause -> assertThat(cause.getMessage()).startsWith("Invalid distribution configuration:"));
}

@Test
void bucketsSubSetOnMaxBucketCount() {
assertThat(DistributionStatisticConfig.builder()
.percentilesHistogram(true)
.build()
.merge(DistributionStatisticConfig.DEFAULT)
.getHistogramBuckets(true)).hasSize(276);
assertThat(DistributionStatisticConfig.builder()
.percentilesHistogram(true)
.maxBucketCount(10)
.build()
.merge(DistributionStatisticConfig.DEFAULT)
.getHistogramBuckets(true)).hasSize(10).containsExactly(1.0, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0, 8.0, 9.0, 10.0);
assertThat(DistributionStatisticConfig.builder()
.percentilesHistogram(true)
.minimumExpectedValue(2.0)
.maxBucketCount(10)
.build()
.merge(DistributionStatisticConfig.DEFAULT)
.getHistogramBuckets(true)).hasSize(10).containsExactly(2.0, 3.0, 4.0, 5.0, 6.0, 7.0, 8.0, 9.0, 10.0, 11.0);
}

}

0 comments on commit 56238b2

Please sign in to comment.