From 5dd7e2c566d025648fb19a15c6e89d2ff46f31c7 Mon Sep 17 00:00:00 2001 From: Tommy Ludwig <8924140+shakuzen@users.noreply.github.com> Date: Wed, 26 Feb 2025 16:58:42 +0900 Subject: [PATCH 1/4] Configure histogram flavor/max buckets per meter Adds new configuration methods to override the registry-wide configuration at a per Meter (name) level. This gives more fine-grain control of the configuration. --- .../micrometer/registry/otlp/OtlpConfig.java | 48 +++++++- .../OtlpCumulativeDistributionSummary.java | 11 +- .../registry/otlp/OtlpCumulativeTimer.java | 11 +- .../registry/otlp/OtlpMeterRegistry.java | 40 ++++--- .../otlp/OtlpStepDistributionSummary.java | 16 +-- .../registry/otlp/OtlpStepTimer.java | 18 +-- .../registry/otlp/OtlpConfigTest.java | 22 ++++ .../registry/otlp/OtlpMeterRegistryTest.java | 109 ++++++++++++++++++ 8 files changed, 216 insertions(+), 59 deletions(-) diff --git a/implementations/micrometer-registry-otlp/src/main/java/io/micrometer/registry/otlp/OtlpConfig.java b/implementations/micrometer-registry-otlp/src/main/java/io/micrometer/registry/otlp/OtlpConfig.java index 8b28489dc4..87ecf44053 100644 --- a/implementations/micrometer-registry-otlp/src/main/java/io/micrometer/registry/otlp/OtlpConfig.java +++ b/implementations/micrometer-registry-otlp/src/main/java/io/micrometer/registry/otlp/OtlpConfig.java @@ -15,17 +15,16 @@ */ package io.micrometer.registry.otlp; +import io.micrometer.common.util.StringUtils; import io.micrometer.core.instrument.config.InvalidConfigurationException; import io.micrometer.core.instrument.config.validate.Validated; import io.micrometer.core.instrument.push.PushRegistryConfig; import java.time.Duration; import java.net.URLDecoder; -import java.util.Arrays; -import java.util.Locale; -import java.util.Map; -import java.util.Objects; +import java.util.*; import java.util.concurrent.TimeUnit; +import java.util.function.Function; import java.util.stream.Collectors; import static io.micrometer.core.instrument.config.MeterRegistryConfigValidator.*; @@ -215,6 +214,7 @@ default Map headers() { * {@link HistogramFlavor#EXPLICIT_BUCKET_HISTOGRAM} is used for those meters. *

* @return - histogram flavor to be used + * @see #histogramFlavorPerMeter() * * @since 1.14.0 */ @@ -229,6 +229,19 @@ default HistogramFlavor histogramFlavor() { }); } + /** + * Configures the histogram flavor to use on a per-meter level. This will override the + * {@link #histogramFlavor()} configuration for matching Meters. The key is used to do + * an exact match on the Meter's name. + * @return mapping of meter name to histogram flavor + * @since 1.15.0 + * @see #histogramFlavor() + */ + default Map histogramFlavorPerMeter() { + String flavorPerMeterString = getString(this, "histogramFlavorPerMeter").orElse(null); + return propertiesStringToMap(flavorPerMeterString, HistogramFlavor::fromString); + } + /** * Max scale to use for exponential histograms, if configured. * @return maxScale @@ -242,9 +255,11 @@ default int maxScale() { /** * Maximum number of buckets to be used for exponential histograms, if configured. - * This has no effect on explicit bucket histograms. + * This has no effect on explicit bucket histograms. This can be overridden per meter + * with {@link #maxBucketsPerMeter()}. * @return - maxBuckets * @see #histogramFlavor() + * @see #maxBucketsPerMeter() * * @since 1.14.0 */ @@ -252,6 +267,19 @@ default int maxBucketCount() { return getInteger(this, "maxBucketCount").orElse(160); } + /** + * Configures the max bucket count to use on a per-meter level. This will override the + * {@link #maxBucketCount()} configuration for matching Meters. The key is used to do + * an exact match on the Meter's name. This has no effect on a meter if it does not + * have an exponential bucket histogram configured. + * @return mapping of meter name to max bucket count + * @since 1.15.0 + * @see #maxBucketCount() + */ + default Map maxBucketsPerMeter() { + return propertiesStringToMap(getString(this, "maxBucketsPerMeter").orElse(null), Integer::parseInt); + } + @Override default Validated validate() { return checkAll(this, c -> PushRegistryConfig.validate(c), checkRequired("url", OtlpConfig::url), @@ -264,4 +292,14 @@ default TimeUnit baseTimeUnit() { return getTimeUnit(this, "baseTimeUnit").orElse(TimeUnit.MILLISECONDS); } + static Map propertiesStringToMap(String propertiesString, + Function valueMapper) { + String[] keyvals = StringUtils.isBlank(propertiesString) ? new String[] {} : propertiesString.trim().split(","); + return Arrays.stream(keyvals) + .map(String::trim) + .filter(keyValue -> keyValue.length() > 2 && keyValue.indexOf('=') > 0) + .collect(Collectors.toMap(keyvalue -> keyvalue.substring(0, keyvalue.indexOf('=')).trim(), + keyvalue -> valueMapper.apply(keyvalue.substring(keyvalue.indexOf('=') + 1).trim()))); + } + } diff --git a/implementations/micrometer-registry-otlp/src/main/java/io/micrometer/registry/otlp/OtlpCumulativeDistributionSummary.java b/implementations/micrometer-registry-otlp/src/main/java/io/micrometer/registry/otlp/OtlpCumulativeDistributionSummary.java index f9e6f6e85b..a45e18e875 100644 --- a/implementations/micrometer-registry-otlp/src/main/java/io/micrometer/registry/otlp/OtlpCumulativeDistributionSummary.java +++ b/implementations/micrometer-registry-otlp/src/main/java/io/micrometer/registry/otlp/OtlpCumulativeDistributionSummary.java @@ -27,17 +27,12 @@ class OtlpCumulativeDistributionSummary extends CumulativeDistributionSummary implements StartTimeAwareMeter, OtlpHistogramSupport { - private final HistogramFlavor histogramFlavor; - private final long startTimeNanos; OtlpCumulativeDistributionSummary(Id id, Clock clock, DistributionStatisticConfig distributionStatisticConfig, - double scale, OtlpConfig otlpConfig) { - super(id, clock, distributionStatisticConfig, scale, - OtlpMeterRegistry.getHistogram(clock, distributionStatisticConfig, otlpConfig)); + double scale, Histogram histogram) { + super(id, clock, distributionStatisticConfig, scale, histogram); this.startTimeNanos = TimeUnit.MILLISECONDS.toNanos(clock.wallTime()); - this.histogramFlavor = OtlpMeterRegistry.histogramFlavor(otlpConfig.histogramFlavor(), - distributionStatisticConfig); } @Override @@ -48,7 +43,7 @@ public long getStartTimeNanos() { @Override @Nullable public ExponentialHistogramSnapShot getExponentialHistogramSnapShot() { - if (histogramFlavor == HistogramFlavor.BASE2_EXPONENTIAL_BUCKET_HISTOGRAM) { + if (histogram instanceof Base2ExponentialHistogram) { return ((Base2ExponentialHistogram) histogram).getLatestExponentialHistogramSnapshot(); } return null; diff --git a/implementations/micrometer-registry-otlp/src/main/java/io/micrometer/registry/otlp/OtlpCumulativeTimer.java b/implementations/micrometer-registry-otlp/src/main/java/io/micrometer/registry/otlp/OtlpCumulativeTimer.java index e8d321e78d..0716d3adb4 100644 --- a/implementations/micrometer-registry-otlp/src/main/java/io/micrometer/registry/otlp/OtlpCumulativeTimer.java +++ b/implementations/micrometer-registry-otlp/src/main/java/io/micrometer/registry/otlp/OtlpCumulativeTimer.java @@ -27,16 +27,11 @@ class OtlpCumulativeTimer extends CumulativeTimer implements StartTimeAwareMeter, OtlpHistogramSupport { - private final HistogramFlavor histogramFlavor; - private final long startTimeNanos; OtlpCumulativeTimer(Id id, Clock clock, DistributionStatisticConfig distributionStatisticConfig, - PauseDetector pauseDetector, TimeUnit baseTimeUnit, OtlpConfig otlpConfig) { - super(id, clock, distributionStatisticConfig, pauseDetector, baseTimeUnit, - OtlpMeterRegistry.getHistogram(clock, distributionStatisticConfig, otlpConfig, baseTimeUnit)); - this.histogramFlavor = OtlpMeterRegistry.histogramFlavor(otlpConfig.histogramFlavor(), - distributionStatisticConfig); + PauseDetector pauseDetector, TimeUnit baseTimeUnit, Histogram histogram) { + super(id, clock, distributionStatisticConfig, pauseDetector, baseTimeUnit, histogram); this.startTimeNanos = TimeUnit.MILLISECONDS.toNanos(clock.wallTime()); } @@ -48,7 +43,7 @@ public long getStartTimeNanos() { @Override @Nullable public ExponentialHistogramSnapShot getExponentialHistogramSnapShot() { - if (histogramFlavor == HistogramFlavor.BASE2_EXPONENTIAL_BUCKET_HISTOGRAM) { + if (histogram instanceof Base2ExponentialHistogram) { return ((Base2ExponentialHistogram) histogram).getLatestExponentialHistogramSnapshot(); } return null; diff --git a/implementations/micrometer-registry-otlp/src/main/java/io/micrometer/registry/otlp/OtlpMeterRegistry.java b/implementations/micrometer-registry-otlp/src/main/java/io/micrometer/registry/otlp/OtlpMeterRegistry.java index d5788cf231..d87cf35413 100644 --- a/implementations/micrometer-registry-otlp/src/main/java/io/micrometer/registry/otlp/OtlpMeterRegistry.java +++ b/implementations/micrometer-registry-otlp/src/main/java/io/micrometer/registry/otlp/OtlpMeterRegistry.java @@ -205,16 +205,19 @@ protected Timer newTimer(Meter.Id id, DistributionStatisticConfig distributionSt PauseDetector pauseDetector) { return isCumulative() ? new OtlpCumulativeTimer(id, this.clock, distributionStatisticConfig, pauseDetector, getBaseTimeUnit(), - config) - : new OtlpStepTimer(id, clock, distributionStatisticConfig, pauseDetector, getBaseTimeUnit(), config); + getHistogram(id, distributionStatisticConfig, getBaseTimeUnit())) + : new OtlpStepTimer(id, clock, pauseDetector, + getHistogram(id, distributionStatisticConfig, getBaseTimeUnit()), config); } @Override protected DistributionSummary newDistributionSummary(Meter.Id id, DistributionStatisticConfig distributionStatisticConfig, double scale) { return isCumulative() - ? new OtlpCumulativeDistributionSummary(id, this.clock, distributionStatisticConfig, scale, config) - : new OtlpStepDistributionSummary(id, clock, distributionStatisticConfig, scale, config); + ? new OtlpCumulativeDistributionSummary(id, clock, distributionStatisticConfig, scale, + getHistogram(id, distributionStatisticConfig)) + : new OtlpStepDistributionSummary(id, clock, scale, getHistogram(id, distributionStatisticConfig), + config); } @Override @@ -372,13 +375,12 @@ Iterable getResourceAttributes() { return attributes; } - static Histogram getHistogram(Clock clock, DistributionStatisticConfig distributionStatisticConfig, - OtlpConfig otlpConfig) { - return getHistogram(clock, distributionStatisticConfig, otlpConfig, null); + private Histogram getHistogram(Meter.Id id, DistributionStatisticConfig distributionStatisticConfig) { + return getHistogram(id, distributionStatisticConfig, null); } - static Histogram getHistogram(final Clock clock, final DistributionStatisticConfig distributionStatisticConfig, - final OtlpConfig otlpConfig, @Nullable final TimeUnit baseTimeUnit) { + private Histogram getHistogram(Meter.Id id, DistributionStatisticConfig distributionStatisticConfig, + @Nullable TimeUnit baseTimeUnit) { // While publishing to OTLP, we export either Histogram datapoint (Explicit // ExponentialBuckets // or Exponential) / Summary @@ -387,21 +389,21 @@ static Histogram getHistogram(final Clock clock, final DistributionStatisticConf // exporting of histograms over percentiles is preferred in OTLP. if (distributionStatisticConfig.isPublishingHistogram()) { if (HistogramFlavor.BASE2_EXPONENTIAL_BUCKET_HISTOGRAM - .equals(histogramFlavor(otlpConfig.histogramFlavor(), distributionStatisticConfig))) { + .equals(histogramFlavor(id, config, distributionStatisticConfig))) { Double minimumExpectedValue = distributionStatisticConfig.getMinimumExpectedValueAsDouble(); if (minimumExpectedValue == null) { minimumExpectedValue = 0.0; } - return otlpConfig.aggregationTemporality() == AggregationTemporality.DELTA - ? new DeltaBase2ExponentialHistogram(otlpConfig.maxScale(), otlpConfig.maxBucketCount(), - minimumExpectedValue, baseTimeUnit, clock, otlpConfig.step().toMillis()) - : new CumulativeBase2ExponentialHistogram(otlpConfig.maxScale(), otlpConfig.maxBucketCount(), + return config.aggregationTemporality() == AggregationTemporality.DELTA + ? new DeltaBase2ExponentialHistogram(config.maxScale(), getMaxBuckets(id), minimumExpectedValue, + baseTimeUnit, clock, config.step().toMillis()) + : new CumulativeBase2ExponentialHistogram(config.maxScale(), getMaxBuckets(id), minimumExpectedValue, baseTimeUnit); } Histogram explicitBucketHistogram = getExplicitBucketHistogram(clock, distributionStatisticConfig, - otlpConfig.aggregationTemporality(), otlpConfig.step().toMillis()); + config.aggregationTemporality(), config.step().toMillis()); if (explicitBucketHistogram != null) { return explicitBucketHistogram; } @@ -413,8 +415,14 @@ static Histogram getHistogram(final Clock clock, final DistributionStatisticConf return NoopHistogram.INSTANCE; } - static HistogramFlavor histogramFlavor(HistogramFlavor preferredHistogramFlavor, + private int getMaxBuckets(Meter.Id id) { + return config.maxBucketsPerMeter().getOrDefault(id.getName(), config.maxBucketCount()); + } + + private HistogramFlavor histogramFlavor(Meter.Id id, OtlpConfig otlpConfig, DistributionStatisticConfig distributionStatisticConfig) { + HistogramFlavor preferredHistogramFlavor = otlpConfig.histogramFlavorPerMeter() + .getOrDefault(id.getName(), otlpConfig.histogramFlavor()); final double[] serviceLevelObjectiveBoundaries = distributionStatisticConfig .getServiceLevelObjectiveBoundaries(); diff --git a/implementations/micrometer-registry-otlp/src/main/java/io/micrometer/registry/otlp/OtlpStepDistributionSummary.java b/implementations/micrometer-registry-otlp/src/main/java/io/micrometer/registry/otlp/OtlpStepDistributionSummary.java index 567419277b..3c6d6391d5 100644 --- a/implementations/micrometer-registry-otlp/src/main/java/io/micrometer/registry/otlp/OtlpStepDistributionSummary.java +++ b/implementations/micrometer-registry-otlp/src/main/java/io/micrometer/registry/otlp/OtlpStepDistributionSummary.java @@ -15,9 +15,10 @@ */ package io.micrometer.registry.otlp; +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.Histogram; import io.micrometer.registry.otlp.internal.Base2ExponentialHistogram; import io.micrometer.registry.otlp.internal.ExponentialHistogramSnapShot; @@ -26,8 +27,6 @@ class OtlpStepDistributionSummary extends AbstractDistributionSummary implements OtlpHistogramSupport { - private final HistogramFlavor histogramFlavor; - private final LongAdder count = new LongAdder(); private final DoubleAdder total = new DoubleAdder(); @@ -40,18 +39,14 @@ class OtlpStepDistributionSummary extends AbstractDistributionSummary implements * Create a new {@code OtlpStepDistributionSummary}. * @param id ID * @param clock clock - * @param distributionStatisticConfig distribution statistic configuration * @param scale scale * @param otlpConfig config for registry */ - OtlpStepDistributionSummary(Id id, Clock clock, DistributionStatisticConfig distributionStatisticConfig, - double scale, OtlpConfig otlpConfig) { - super(id, scale, OtlpMeterRegistry.getHistogram(clock, distributionStatisticConfig, otlpConfig)); + OtlpStepDistributionSummary(Id id, Clock clock, double scale, Histogram histogram, OtlpConfig otlpConfig) { + super(id, scale, histogram); this.countTotal = new OtlpStepTuple2<>(clock, otlpConfig.step().toMillis(), 0L, 0.0, count::sumThenReset, total::sumThenReset); this.max = new StepMax(clock, otlpConfig.step().toMillis()); - this.histogramFlavor = OtlpMeterRegistry.histogramFlavor(otlpConfig.histogramFlavor(), - distributionStatisticConfig); } @Override @@ -77,8 +72,9 @@ public double max() { } @Override + @Nullable public ExponentialHistogramSnapShot getExponentialHistogramSnapShot() { - if (histogramFlavor == HistogramFlavor.BASE2_EXPONENTIAL_BUCKET_HISTOGRAM) { + if (histogram instanceof Base2ExponentialHistogram) { return ((Base2ExponentialHistogram) histogram).getLatestExponentialHistogramSnapshot(); } return null; diff --git a/implementations/micrometer-registry-otlp/src/main/java/io/micrometer/registry/otlp/OtlpStepTimer.java b/implementations/micrometer-registry-otlp/src/main/java/io/micrometer/registry/otlp/OtlpStepTimer.java index e5f5726265..a4e5189b97 100644 --- a/implementations/micrometer-registry-otlp/src/main/java/io/micrometer/registry/otlp/OtlpStepTimer.java +++ b/implementations/micrometer-registry-otlp/src/main/java/io/micrometer/registry/otlp/OtlpStepTimer.java @@ -15,9 +15,10 @@ */ package io.micrometer.registry.otlp; +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.Histogram; import io.micrometer.core.instrument.distribution.pause.PauseDetector; import io.micrometer.core.instrument.util.TimeUtils; import io.micrometer.registry.otlp.internal.Base2ExponentialHistogram; @@ -28,8 +29,6 @@ class OtlpStepTimer extends AbstractTimer implements OtlpHistogramSupport { - private final HistogramFlavor histogramFlavor; - private final LongAdder count = new LongAdder(); private final LongAdder total = new LongAdder(); @@ -42,20 +41,14 @@ class OtlpStepTimer extends AbstractTimer implements OtlpHistogramSupport { * Create a new {@code OtlpStepTimer}. * @param id ID * @param clock clock - * @param distributionStatisticConfig distribution statistic configuration * @param pauseDetector pause detector - * @param baseTimeUnit base time unit * @param otlpConfig config of the registry */ - OtlpStepTimer(Id id, Clock clock, DistributionStatisticConfig distributionStatisticConfig, - PauseDetector pauseDetector, TimeUnit baseTimeUnit, OtlpConfig otlpConfig) { - super(id, clock, pauseDetector, otlpConfig.baseTimeUnit(), - OtlpMeterRegistry.getHistogram(clock, distributionStatisticConfig, otlpConfig, baseTimeUnit)); + OtlpStepTimer(Id id, Clock clock, PauseDetector pauseDetector, Histogram histogram, OtlpConfig otlpConfig) { + super(id, clock, pauseDetector, otlpConfig.baseTimeUnit(), histogram); countTotal = new OtlpStepTuple2<>(clock, otlpConfig.step().toMillis(), 0L, 0L, count::sumThenReset, total::sumThenReset); max = new StepMax(clock, otlpConfig.step().toMillis()); - this.histogramFlavor = OtlpMeterRegistry.histogramFlavor(otlpConfig.histogramFlavor(), - distributionStatisticConfig); } @Override @@ -99,8 +92,9 @@ else if (histogram instanceof Base2ExponentialHistogram) { } @Override + @Nullable public ExponentialHistogramSnapShot getExponentialHistogramSnapShot() { - if (histogramFlavor == HistogramFlavor.BASE2_EXPONENTIAL_BUCKET_HISTOGRAM) { + if (histogram instanceof Base2ExponentialHistogram) { return ((Base2ExponentialHistogram) histogram).getLatestExponentialHistogramSnapshot(); } return null; diff --git a/implementations/micrometer-registry-otlp/src/test/java/io/micrometer/registry/otlp/OtlpConfigTest.java b/implementations/micrometer-registry-otlp/src/test/java/io/micrometer/registry/otlp/OtlpConfigTest.java index 02c1db9536..f0efa86663 100644 --- a/implementations/micrometer-registry-otlp/src/test/java/io/micrometer/registry/otlp/OtlpConfigTest.java +++ b/implementations/micrometer-registry-otlp/src/test/java/io/micrometer/registry/otlp/OtlpConfigTest.java @@ -25,6 +25,7 @@ import java.util.concurrent.TimeUnit; import java.util.stream.Stream; +import static java.util.Map.entry; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static uk.org.webcompere.systemstubs.SystemStubs.withEnvironmentVariable; @@ -276,4 +277,25 @@ void histogramPreferenceUseEnvVarWhenConfigNotSet() throws Exception { .isEqualTo(HistogramFlavor.BASE2_EXPONENTIAL_BUCKET_HISTOGRAM)); } + @Test + void histogramFlavorPerMeter() { + Map properties = new HashMap<>(); + properties.put("otlp.histogramFlavorPerMeter", + "a.b.c=explicit_bucket_histogram ,expo =base2_exponential_bucket_histogram"); + OtlpConfig otlpConfig = properties::get; + assertThat(otlpConfig.validate().isValid()).isTrue(); + assertThat(otlpConfig.histogramFlavorPerMeter()).containsExactly( + entry("a.b.c", HistogramFlavor.EXPLICIT_BUCKET_HISTOGRAM), + entry("expo", HistogramFlavor.BASE2_EXPONENTIAL_BUCKET_HISTOGRAM)); + } + + @Test + void maxBucketsPerMeter() { + Map properties = new HashMap<>(); + properties.put("otlp.maxBucketsPerMeter", "a.b.c = 10"); + OtlpConfig otlpConfig = properties::get; + assertThat(otlpConfig.validate().isValid()).isTrue(); + assertThat(otlpConfig.maxBucketsPerMeter()).containsExactly(entry("a.b.c", 10)); + } + } diff --git a/implementations/micrometer-registry-otlp/src/test/java/io/micrometer/registry/otlp/OtlpMeterRegistryTest.java b/implementations/micrometer-registry-otlp/src/test/java/io/micrometer/registry/otlp/OtlpMeterRegistryTest.java index 01a39f309f..8c2e9fd093 100644 --- a/implementations/micrometer-registry-otlp/src/test/java/io/micrometer/registry/otlp/OtlpMeterRegistryTest.java +++ b/implementations/micrometer-registry-otlp/src/test/java/io/micrometer/registry/otlp/OtlpMeterRegistryTest.java @@ -31,6 +31,7 @@ import java.time.Duration; import java.util.*; import java.util.concurrent.TimeUnit; +import java.util.stream.IntStream; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.assertArg; @@ -407,6 +408,114 @@ void testGetSloWithPositiveInf() { @Test abstract void testMetricsStartAndEndTime(); + @Test + void perMeterHistogramFlavorOverridesGeneralHistogramFlavor() { + OtlpConfig config = new OtlpConfig() { + @Override + public String get(String key) { + return null; + } + + @Override + public AggregationTemporality aggregationTemporality() { + return otlpConfig().aggregationTemporality(); + } + + @Override + public HistogramFlavor histogramFlavor() { + return HistogramFlavor.EXPLICIT_BUCKET_HISTOGRAM; + } + + @Override + public Map histogramFlavorPerMeter() { + return Collections.singletonMap("expo", HistogramFlavor.BASE2_EXPONENTIAL_BUCKET_HISTOGRAM); + } + }; + OtlpMeterRegistry meterRegistry = new OtlpMeterRegistry(config, clock); + Timer expo = Timer.builder("expo").publishPercentileHistogram().register(meterRegistry); + Timer other = Timer.builder("other").publishPercentileHistogram().register(meterRegistry); + + assertThat(writeToMetric(expo).getDataCase().getNumber()) + .isEqualTo(Metric.DataCase.EXPONENTIAL_HISTOGRAM.getNumber()); + assertThat(writeToMetric(other).getDataCase().getNumber()).isEqualTo(Metric.DataCase.HISTOGRAM.getNumber()); + + meterRegistry.clear(); + + DistributionSummary expo2 = DistributionSummary.builder("expo") + .publishPercentileHistogram() + .register(meterRegistry); + DistributionSummary other2 = DistributionSummary.builder("other") + .publishPercentileHistogram() + .register(meterRegistry); + + assertThat(writeToMetric(expo2).getDataCase().getNumber()) + .isEqualTo(Metric.DataCase.EXPONENTIAL_HISTOGRAM.getNumber()); + assertThat(writeToMetric(other2).getDataCase().getNumber()).isEqualTo(Metric.DataCase.HISTOGRAM.getNumber()); + } + + @Test + void maxBucketsPerMeter() { + OtlpConfig config = new OtlpConfig() { + @Override + public String get(String key) { + return null; + } + + @Override + public AggregationTemporality aggregationTemporality() { + return otlpConfig().aggregationTemporality(); + } + + @Override + public HistogramFlavor histogramFlavor() { + return HistogramFlavor.BASE2_EXPONENTIAL_BUCKET_HISTOGRAM; + } + + @Override + public int maxBucketCount() { + return 56; + } + + @Override + public Map maxBucketsPerMeter() { + return Collections.singletonMap("low.variation", 15); + } + }; + OtlpMeterRegistry meterRegistry = new OtlpMeterRegistry(config, clock); + Timer lowVariation = Timer.builder("low.variation").publishPercentileHistogram().register(meterRegistry); + Timer other = Timer.builder("other").publishPercentileHistogram().register(meterRegistry); + + List.of(lowVariation, other) + .forEach(t -> IntStream.range(1, 111).forEach(i -> t.record(i, TimeUnit.MILLISECONDS))); + clock.add(config.step()); + + assertThat(writeToMetric(lowVariation).getExponentialHistogram() + .getDataPoints(0) + .getPositive() + .getBucketCountsList()).hasSize(15); + assertThat(writeToMetric(other).getExponentialHistogram().getDataPoints(0).getPositive().getBucketCountsList()) + .hasSize(56); + + meterRegistry.clear(); + + DistributionSummary lowVariation2 = DistributionSummary.builder("low.variation") + .publishPercentileHistogram() + .register(meterRegistry); + DistributionSummary other2 = DistributionSummary.builder("other") + .publishPercentileHistogram() + .register(meterRegistry); + + List.of(lowVariation2, other2).forEach(t -> IntStream.range(1, 111).forEach(t::record)); + clock.add(config.step()); + + assertThat(writeToMetric(lowVariation2).getExponentialHistogram() + .getDataPoints(0) + .getPositive() + .getBucketCountsList()).hasSize(15); + assertThat(writeToMetric(other2).getExponentialHistogram().getDataPoints(0).getPositive().getBucketCountsList()) + .hasSize(56); + } + protected Metric writeToMetric(Meter meter) { OtlpMetricConverter otlpMetricConverter = new OtlpMetricConverter(clock, otlpConfig().step(), registry.getBaseTimeUnit(), otlpConfig().aggregationTemporality(), From 04616d845eadbf2d7f361a80692f98ebc89efc33 Mon Sep 17 00:00:00 2001 From: Tommy Ludwig <8924140+shakuzen@users.noreply.github.com> Date: Mon, 10 Mar 2025 16:54:35 +0900 Subject: [PATCH 2/4] Move conversion from property to map to PropertyValidator --- .../micrometer/registry/otlp/OtlpConfig.java | 18 +---- .../config/validate/PropertyValidator.java | 37 +++++++++++ .../validate/PropertyValidatorTest.java | 66 +++++++++++++++++++ 3 files changed, 106 insertions(+), 15 deletions(-) create mode 100644 micrometer-core/src/test/java/io/micrometer/core/instrument/config/validate/PropertyValidatorTest.java diff --git a/implementations/micrometer-registry-otlp/src/main/java/io/micrometer/registry/otlp/OtlpConfig.java b/implementations/micrometer-registry-otlp/src/main/java/io/micrometer/registry/otlp/OtlpConfig.java index 87ecf44053..8592defeb3 100644 --- a/implementations/micrometer-registry-otlp/src/main/java/io/micrometer/registry/otlp/OtlpConfig.java +++ b/implementations/micrometer-registry-otlp/src/main/java/io/micrometer/registry/otlp/OtlpConfig.java @@ -15,7 +15,6 @@ */ package io.micrometer.registry.otlp; -import io.micrometer.common.util.StringUtils; import io.micrometer.core.instrument.config.InvalidConfigurationException; import io.micrometer.core.instrument.config.validate.Validated; import io.micrometer.core.instrument.push.PushRegistryConfig; @@ -24,7 +23,6 @@ import java.net.URLDecoder; import java.util.*; import java.util.concurrent.TimeUnit; -import java.util.function.Function; import java.util.stream.Collectors; import static io.micrometer.core.instrument.config.MeterRegistryConfigValidator.*; @@ -238,8 +236,8 @@ default HistogramFlavor histogramFlavor() { * @see #histogramFlavor() */ default Map histogramFlavorPerMeter() { - String flavorPerMeterString = getString(this, "histogramFlavorPerMeter").orElse(null); - return propertiesStringToMap(flavorPerMeterString, HistogramFlavor::fromString); + return getStringMap(this, "histogramFlavorPerMeter", HistogramFlavor::fromString) + .orElse(Collections.emptyMap()); } /** @@ -277,7 +275,7 @@ default int maxBucketCount() { * @see #maxBucketCount() */ default Map maxBucketsPerMeter() { - return propertiesStringToMap(getString(this, "maxBucketsPerMeter").orElse(null), Integer::parseInt); + return getStringMap(this, "maxBucketsPerMeter", Integer::parseInt).orElse(Collections.emptyMap()); } @Override @@ -292,14 +290,4 @@ default TimeUnit baseTimeUnit() { return getTimeUnit(this, "baseTimeUnit").orElse(TimeUnit.MILLISECONDS); } - static Map propertiesStringToMap(String propertiesString, - Function valueMapper) { - String[] keyvals = StringUtils.isBlank(propertiesString) ? new String[] {} : propertiesString.trim().split(","); - return Arrays.stream(keyvals) - .map(String::trim) - .filter(keyValue -> keyValue.length() > 2 && keyValue.indexOf('=') > 0) - .collect(Collectors.toMap(keyvalue -> keyvalue.substring(0, keyvalue.indexOf('=')).trim(), - keyvalue -> valueMapper.apply(keyvalue.substring(keyvalue.indexOf('=') + 1).trim()))); - } - } diff --git a/micrometer-core/src/main/java/io/micrometer/core/instrument/config/validate/PropertyValidator.java b/micrometer-core/src/main/java/io/micrometer/core/instrument/config/validate/PropertyValidator.java index f39705fe51..bedf4374eb 100644 --- a/micrometer-core/src/main/java/io/micrometer/core/instrument/config/validate/PropertyValidator.java +++ b/micrometer-core/src/main/java/io/micrometer/core/instrument/config/validate/PropertyValidator.java @@ -15,6 +15,7 @@ */ package io.micrometer.core.instrument.config.validate; +import io.micrometer.common.util.StringUtils; import io.micrometer.core.annotation.Incubating; import io.micrometer.core.instrument.config.MeterRegistryConfig; @@ -23,7 +24,9 @@ import java.net.URI; import java.time.Duration; import java.util.Arrays; +import java.util.Map; import java.util.concurrent.TimeUnit; +import java.util.function.Function; import java.util.stream.Collectors; /** @@ -141,6 +144,40 @@ public static Validated getUriString(MeterRegistryConfig config, String } } + /** + * Populate a Map from the given property value in the format of + * key1=value1,key2=value2. + * @param config config + * @param property name of the property + * @param valueMapper function mapping the value of the key-value pairs contained in + * the property value to the value stored in the Map + * @return Map with key-values from the given property + * @param type of the Map values + * @since 1.15.0 + */ + public static Validated> getStringMap(MeterRegistryConfig config, String property, + Function valueMapper) { + String prefixedProperty = prefixedProperty(config, property); + String propertyValue = config.get(prefixedProperty); + if (StringUtils.isBlank(propertyValue)) { + return Validated.valid(prefixedProperty, null); + } + String[] keyvals = propertyValue.trim().split(","); + Map map = null; + try { + map = Arrays.stream(keyvals) + .map(String::trim) + .filter(keyValue -> keyValue.length() > 2 && keyValue.indexOf('=') > 0) + .collect(Collectors.toMap(keyvalue -> keyvalue.substring(0, keyvalue.indexOf('=')).trim(), + keyvalue -> valueMapper.apply(keyvalue.substring(keyvalue.indexOf('=') + 1).trim()))); + } + catch (Exception e) { + return Validated.invalid(prefixedProperty, map, "Exception mapping value using valueMapper", + InvalidReason.MALFORMED, e); + } + return Validated.valid(prefixedProperty, map); + } + private static String prefixedProperty(MeterRegistryConfig config, String property) { return config.prefix() + '.' + property; } diff --git a/micrometer-core/src/test/java/io/micrometer/core/instrument/config/validate/PropertyValidatorTest.java b/micrometer-core/src/test/java/io/micrometer/core/instrument/config/validate/PropertyValidatorTest.java new file mode 100644 index 0000000000..ee6e0603af --- /dev/null +++ b/micrometer-core/src/test/java/io/micrometer/core/instrument/config/validate/PropertyValidatorTest.java @@ -0,0 +1,66 @@ +/* + * Copyright 2025 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.config.validate; + +import io.micrometer.core.instrument.config.MeterRegistryConfig; +import org.junit.jupiter.api.Test; + +import java.util.Collections; +import java.util.Map; +import java.util.Properties; + +import static org.assertj.core.api.Assertions.assertThat; + +class PropertyValidatorTest { + + Properties props = new Properties(); + + MeterRegistryConfig config = new MeterRegistryConfig() { + @Override + public String prefix() { + return "test"; + } + + @Override + public String get(String key) { + return props.getProperty(key); + } + }; + + @Test + void stringMapValid() { + props.put("test.map", "a=1,b=2"); + Validated> validated = PropertyValidator.getStringMap(config, "map", Integer::parseInt); + assertThat(validated.isValid()).isTrue(); + } + + @Test + void stringMapBlank() { + props.put("test.map", " "); + Validated> validated = PropertyValidator.getStringMap(config, "map", Integer::parseInt); + assertThat(validated.isValid()).isTrue(); + assertThat(validated.get()).isNull(); + assertThat(validated.orElse(Collections.emptyMap())).isEmpty(); + } + + @Test + void stringMapInvalid() { + props.setProperty("test.map", "a=1,b=c"); + Validated> validated = PropertyValidator.getStringMap(config, "map", Integer::parseInt); + assertThat(validated.isInvalid()).isTrue(); + } + +} From f00eab98d6635701254af2e145d81574bf18a78a Mon Sep 17 00:00:00 2001 From: Tommy Ludwig <8924140+shakuzen@users.noreply.github.com> Date: Mon, 10 Mar 2025 17:45:31 +0900 Subject: [PATCH 3/4] Update documentation --- docs/modules/ROOT/pages/implementations/otlp.adoc | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/docs/modules/ROOT/pages/implementations/otlp.adoc b/docs/modules/ROOT/pages/implementations/otlp.adoc index 98549724d2..cb13cd2eac 100644 --- a/docs/modules/ROOT/pages/implementations/otlp.adoc +++ b/docs/modules/ROOT/pages/implementations/otlp.adoc @@ -146,14 +146,17 @@ Micrometer `Timer` and `DistributionSummary` support configuring xref:/concepts/ | publishPercentiles and serviceLevelObjectives | Histogram |=== -__* The configuration `histogramFlavor` determines whether the OTLP DataPoint is a Histogram/Exponential Histogram.__ +__* The configuration `histogramFlavorPerMeter` and `histogramFlavor` determine whether the OTLP DataPoint is a Histogram/Exponential Histogram.__ -`OtlpMeterRegistry` supports 2 types of Histogram implementations (1.Explicit Bucket Histogram (or simply called Histogram), 2. Exponential Histogram) when `publishPercentileHistogram` is configured. The choice is chosen by setting `histogramFlavor` in `OtlpConfig` used by registry. When the implementation is exponential histogram, it also supports 2 additional properties +`OtlpMeterRegistry` supports 2 types of Histogram implementations (1.Explicit Bucket Histogram (or simply called Histogram), 2. Exponential Histogram) when `publishPercentileHistogram` is configured. +The type is determined by `histogramFlavorPerMeter` and `histogramFlavor` in `OtlpConfig` used by the registry. +When the implementation is the exponential histogram, additional configuration applies: -1. maxScale used to cap the maximum https://opentelemetry.io/docs/specs/otel/metrics/data-model/#exponential-scale[scale, window=_blank] used by the Exponential Histogram (defaults to 20). -2. maxBuckets determines the maximum number of https://opentelemetry.io/docs/specs/otel/metrics/data-model/#exponential-buckets[buckets, window=_blank] to be used for exponential histograms (defaults to 160). +1. `maxScale` used to cap the maximum https://opentelemetry.io/docs/specs/otel/metrics/data-model/#exponential-scale[scale, window=_blank] used by the Exponential Histogram (defaults to 20). +2. `maxBuckets` determines the maximum number of https://opentelemetry.io/docs/specs/otel/metrics/data-model/#exponential-buckets[buckets, window=_blank] to be used for exponential histograms (defaults to 160). +3. `maxBucketsPerMeter` overrides `maxBuckets` for specific meter names, giving more fine-grained configurability. -Since Exponential Histogram cannot have custom SLO's specified, explicit bucket histogram is used whenever `serviceLevelObjectives` are added. +Since Exponential Histogram cannot have custom SLO's specified, an explicit bucket histogram is used whenever `serviceLevelObjectives` are configured. === Configuration with Spring Boot If you use Spring Boot, you can use the https://docs.spring.io/spring-boot/docs/current/reference/htmlsingle/#actuator.metrics.customizing.per-meter-properties[per-meter properties, window=_blank] to configure this behavior. From 75be1599d86274dc4b78535fdf1dbe0a46f0c619 Mon Sep 17 00:00:00 2001 From: Tommy Ludwig <8924140+shakuzen@users.noreply.github.com> Date: Mon, 10 Mar 2025 17:52:10 +0900 Subject: [PATCH 4/4] Validate new config methods --- .../src/main/java/io/micrometer/registry/otlp/OtlpConfig.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/implementations/micrometer-registry-otlp/src/main/java/io/micrometer/registry/otlp/OtlpConfig.java b/implementations/micrometer-registry-otlp/src/main/java/io/micrometer/registry/otlp/OtlpConfig.java index 8592defeb3..f1a4e6263b 100644 --- a/implementations/micrometer-registry-otlp/src/main/java/io/micrometer/registry/otlp/OtlpConfig.java +++ b/implementations/micrometer-registry-otlp/src/main/java/io/micrometer/registry/otlp/OtlpConfig.java @@ -283,7 +283,9 @@ default Validated validate() { return checkAll(this, c -> PushRegistryConfig.validate(c), checkRequired("url", OtlpConfig::url), check("resourceAttributes", OtlpConfig::resourceAttributes), check("baseTimeUnit", OtlpConfig::baseTimeUnit), - check("aggregationTemporality", OtlpConfig::aggregationTemporality)); + check("aggregationTemporality", OtlpConfig::aggregationTemporality), + check("histogramFlavorPerMeter", OtlpConfig::histogramFlavorPerMeter), + check("maxBucketsPerMeter", OtlpConfig::maxBucketsPerMeter)); } default TimeUnit baseTimeUnit() {