Skip to content

Commit 1463686

Browse files
committed
Fix handling of whole numbers for min/max expected value and slo
Previously, a whole number string would be parsed into a Duration. Duration values are ignored for distribution summary meters which results in unexpected values when configuring minimum and maximum expected values and SLO boundaries. Strings like 20d are ambiguous as they can be either a duration of 20 days or a double of 20. To preserve existing behavior where 20d was interpreted as a 20-day duration, strings that contain only digits are parsed into a double which is ultimately treated as a number of milliseconds, the same unit as if it had been parsed into a Duration. Otherwise we fall back to the existing behavior of trying a Duration and then falling back to parsing a double. Note that negative values do not need to be considered. As per their javadoc, DistributionSummary and Timer implementations drop negative amounts passed to record. As a result, negative values for min and max expected values and SLO boundaries do not make sense. Fixes gh-50021
1 parent b459541 commit 1463686

3 files changed

Lines changed: 61 additions & 5 deletions

File tree

spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/MeterValue.java

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import io.micrometer.core.instrument.Meter.Type;
2323

2424
import org.springframework.boot.convert.DurationStyle;
25+
import org.springframework.util.StringUtils;
2526

2627
/**
2728
* A meter value that is used when configuring micrometer. Can be a String representation
@@ -86,13 +87,28 @@ private Long getTimerValue() {
8687
* @return a {@link MeterValue} instance
8788
*/
8889
public static MeterValue valueOf(String value) {
90+
if (onlyDigits(value)) {
91+
return new MeterValue(Double.parseDouble(value));
92+
}
8993
Duration duration = safeParseDuration(value);
9094
if (duration != null) {
9195
return new MeterValue(duration);
9296
}
9397
return new MeterValue(Double.parseDouble(value));
9498
}
9599

100+
private static boolean onlyDigits(String value) {
101+
if (!StringUtils.hasLength(value)) {
102+
return false;
103+
}
104+
for (char c : value.toCharArray()) {
105+
if (!Character.isDigit(c)) {
106+
return false;
107+
}
108+
}
109+
return true;
110+
}
111+
96112
/**
97113
* Return a new {@link MeterValue} instance for the given double value.
98114
* @param value the source value

spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/MeterValueTests.java

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,17 +35,29 @@
3535
class MeterValueTests {
3636

3737
@Test
38-
void getValueForDistributionSummaryWhenFromNumberShouldReturnDoubleValue() {
38+
void getValueForDistributionSummaryWhenFromDecimalNumberShouldReturnDoubleValue() {
3939
MeterValue meterValue = MeterValue.valueOf(123.42);
4040
assertThat(meterValue.getValue(Type.DISTRIBUTION_SUMMARY)).isEqualTo(123.42);
4141
}
4242

4343
@Test
44-
void getValueForDistributionSummaryWhenFromNumberStringShouldReturnDoubleValue() {
44+
void getValueForDistributionSummaryWhenFromWholeNumberShouldReturnDoubleValue() {
45+
MeterValue meterValue = MeterValue.valueOf(123);
46+
assertThat(meterValue.getValue(Type.DISTRIBUTION_SUMMARY)).isEqualTo(123);
47+
}
48+
49+
@Test
50+
void getValueForDistributionSummaryWhenFromDecimalNumberStringShouldReturnDoubleValue() {
4551
MeterValue meterValue = MeterValue.valueOf("123.42");
4652
assertThat(meterValue.getValue(Type.DISTRIBUTION_SUMMARY)).isEqualTo(123.42);
4753
}
4854

55+
@Test
56+
void getValueForDistributionSummaryWhenFromWholeNumberStringShouldReturnDoubleValue() {
57+
MeterValue meterValue = MeterValue.valueOf("123");
58+
assertThat(meterValue.getValue(Type.DISTRIBUTION_SUMMARY)).isEqualTo(123);
59+
}
60+
4961
@Test
5062
void getValueForDistributionSummaryWhenFromDurationStringShouldReturnNull() {
5163
MeterValue meterValue = MeterValue.valueOf("123ms");

spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/PropertiesMeterFilterTests.java

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -205,13 +205,21 @@ void configureWhenAllPercentilesSetShouldSetPercentilesToValue() {
205205
}
206206

207207
@Test
208-
void configureWhenHasSloShouldSetSloToValue() {
208+
void configureWhenTimerMeterHasSloShouldSetSloToValue() {
209209
PropertiesMeterFilter filter = new PropertiesMeterFilter(
210210
createProperties("distribution.slo.spring.boot=1,2,3"));
211211
assertThat(filter.configure(createMeterId("spring.boot"), DistributionStatisticConfig.DEFAULT)
212212
.getServiceLevelObjectiveBoundaries()).containsExactly(1000000, 2000000, 3000000);
213213
}
214214

215+
@Test
216+
void configureWhenDistributionSummaryMeterHasSloShouldSetSloToValue() {
217+
PropertiesMeterFilter filter = new PropertiesMeterFilter(createProperties("distribution.slo.example=1,2,3"));
218+
assertThat(filter
219+
.configure(createMeterId("example", Meter.Type.DISTRIBUTION_SUMMARY), DistributionStatisticConfig.DEFAULT)
220+
.getServiceLevelObjectiveBoundaries()).containsExactly(1.0, 2.0, 3.0);
221+
}
222+
215223
@Test
216224
void configureWhenHasHigherSloShouldSetPercentilesToValue() {
217225
PropertiesMeterFilter filter = new PropertiesMeterFilter(createProperties("distribution.slo.spring=1,2,3"));
@@ -228,13 +236,23 @@ void configureWhenHasHigherSloAndLowerShouldSetSloToLower() {
228236
}
229237

230238
@Test
231-
void configureWhenHasMinimumExpectedValueShouldSetMinimumExpectedToValue() {
239+
void configureWhenTimerMeterHasMinimumExpectedValueShouldSetMinimumExpectedToValue() {
232240
PropertiesMeterFilter filter = new PropertiesMeterFilter(
233241
createProperties("distribution.minimum-expected-value.spring.boot=10"));
234242
assertThat(filter.configure(createMeterId("spring.boot"), DistributionStatisticConfig.DEFAULT)
235243
.getMinimumExpectedValueAsDouble()).isEqualTo(Duration.ofMillis(10).toNanos());
236244
}
237245

246+
@Test
247+
void configureWhenDistributionSummaryMeterHasMinimumExpectedValueShouldSetMinimumExpectedToValue() {
248+
PropertiesMeterFilter filter = new PropertiesMeterFilter(
249+
createProperties("distribution.minimum-expected-value.spring.boot=10"));
250+
assertThat(filter
251+
.configure(createMeterId("spring.boot", Meter.Type.DISTRIBUTION_SUMMARY),
252+
DistributionStatisticConfig.DEFAULT)
253+
.getMinimumExpectedValueAsDouble()).isEqualTo(10);
254+
}
255+
238256
@Test
239257
void configureWhenHasHigherMinimumExpectedValueShouldSetMinimumExpectedValueToValue() {
240258
PropertiesMeterFilter filter = new PropertiesMeterFilter(
@@ -252,13 +270,23 @@ void configureWhenHasHigherMinimumExpectedValueAndLowerShouldSetMinimumExpectedV
252270
}
253271

254272
@Test
255-
void configureWhenHasMaximumExpectedValueShouldSetMaximumExpectedToValue() {
273+
void configureWhenTimerMeterHasMaximumExpectedValueShouldSetMaximumExpectedToValue() {
256274
PropertiesMeterFilter filter = new PropertiesMeterFilter(
257275
createProperties("distribution.maximum-expected-value.spring.boot=5000"));
258276
assertThat(filter.configure(createMeterId("spring.boot"), DistributionStatisticConfig.DEFAULT)
259277
.getMaximumExpectedValueAsDouble()).isEqualTo(Duration.ofMillis(5000).toNanos());
260278
}
261279

280+
@Test
281+
void configureWhenDistributionSummaryMeterHasMaximumExpectedValueShouldSetMaximumExpectedToValue() {
282+
PropertiesMeterFilter filter = new PropertiesMeterFilter(
283+
createProperties("distribution.maximum-expected-value.spring.boot=10"));
284+
assertThat(filter
285+
.configure(createMeterId("spring.boot", Meter.Type.DISTRIBUTION_SUMMARY),
286+
DistributionStatisticConfig.DEFAULT)
287+
.getMaximumExpectedValueAsDouble()).isEqualTo(10);
288+
}
289+
262290
@Test
263291
void configureWhenHasHigherMaximumExpectedValueShouldSetMaximumExpectedValueToValue() {
264292
PropertiesMeterFilter filter = new PropertiesMeterFilter(

0 commit comments

Comments
 (0)