From 540468b2f0ce3729a5c5e48e56e5ebde59db4652 Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Wed, 13 Oct 2021 11:31:08 +0100 Subject: [PATCH] Fix parsing of day duration meter values Switch `MeterValue` parsing logic so that we try `Duration` before `double`. Prior to this commit, the value `1d` would result in `1.0` rather than "1 day". Fixes gh-28302 --- .../actuate/autoconfigure/metrics/MeterValue.java | 14 +++++++------- .../autoconfigure/metrics/MetricsProperties.java | 8 ++++---- .../ServiceLevelObjectiveBoundaryTests.java | 12 ++++++++++-- 3 files changed, 21 insertions(+), 13 deletions(-) diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/MeterValue.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/MeterValue.java index ea6f07d9c462..86ef699b40d3 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/MeterValue.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/MeterValue.java @@ -86,11 +86,11 @@ private Long getTimerValue() { * @return a {@link MeterValue} instance */ public static MeterValue valueOf(String value) { - Double number = safeParseDouble(value); - if (number != null) { - return new MeterValue(number); + Duration duration = safeParseDuration(value); + if (duration != null) { + return new MeterValue(duration); } - return new MeterValue(DurationStyle.detectAndParse(value)); + return new MeterValue(Double.valueOf(value)); } /** @@ -114,11 +114,11 @@ public static MeterValue valueOf(double value) { return new MeterValue(value); } - private static Double safeParseDouble(String value) { + private static Duration safeParseDuration(String value) { try { - return Double.valueOf(value); + return DurationStyle.detectAndParse(value); } - catch (NumberFormatException ex) { + catch (IllegalArgumentException ex) { return null; } } diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/MetricsProperties.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/MetricsProperties.java index ecc4e6865b69..0dc785df5b4c 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/MetricsProperties.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/MetricsProperties.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2020 the original author or authors. + * Copyright 2012-2021 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -235,21 +235,21 @@ public static class Distribution { /** * Specific service-level objective boundaries for meter IDs starting with the * specified name. The longest match wins. Counters will be published for each - * specified boundary. Values can be specified as a long or as a Duration value + * specified boundary. Values can be specified as a double or as a Duration value * (for timer meters, defaulting to ms if no unit specified). */ private final Map slo = new LinkedHashMap<>(); /** * Minimum value that meter IDs starting with the specified name are expected to - * observe. The longest match wins. Values can be specified as a long or as a + * observe. The longest match wins. Values can be specified as a double or as a * Duration value (for timer meters, defaulting to ms if no unit specified). */ private final Map minimumExpectedValue = new LinkedHashMap<>(); /** * Maximum value that meter IDs starting with the specified name are expected to - * observe. The longest match wins. Values can be specified as a long or as a + * observe. The longest match wins. Values can be specified as a double or as a * Duration value (for timer meters, defaulting to ms if no unit specified). */ private final Map maximumExpectedValue = new LinkedHashMap<>(); diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/ServiceLevelObjectiveBoundaryTests.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/ServiceLevelObjectiveBoundaryTests.java index d5511edf3b0d..8fd5eb246115 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/ServiceLevelObjectiveBoundaryTests.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/ServiceLevelObjectiveBoundaryTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2020 the original author or authors. + * Copyright 2012-2021 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,6 +16,8 @@ package org.springframework.boot.actuate.autoconfigure.metrics; +import java.time.Duration; + import io.micrometer.core.instrument.Meter.Type; import org.junit.jupiter.api.Test; @@ -42,11 +44,17 @@ void getValueForTimerWhenFromNumberStringShouldMsToNanosValue() { } @Test - void getValueForTimerWhenFromDurationStringShouldReturnDurationNanos() { + void getValueForTimerWhenFromMillisecondDurationStringShouldReturnDurationNanos() { ServiceLevelObjectiveBoundary slo = ServiceLevelObjectiveBoundary.valueOf("123ms"); assertThat(slo.getValue(Type.TIMER)).isEqualTo(123000000); } + @Test + void getValueForTimerWhenFromDaysDurationStringShouldReturnDurationNanos() { + ServiceLevelObjectiveBoundary slo = ServiceLevelObjectiveBoundary.valueOf("1d"); + assertThat(slo.getValue(Type.TIMER)).isEqualTo(Duration.ofDays(1).toNanos()); + } + @Test void getValueForDistributionSummaryWhenFromDoubleShouldReturnDoubleValue() { ServiceLevelObjectiveBoundary slo = ServiceLevelObjectiveBoundary.valueOf(123.42);