From c99a22d6ffcc01fa025483183427471ef951def7 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 20 Mar 2023 17:16:30 -0700 Subject: [PATCH 01/21] Bump com.gradle.enterprise from 3.12.4 to 3.12.5 (#3705) Bumps com.gradle.enterprise from 3.12.4 to 3.12.5. --- updated-dependencies: - dependency-name: com.gradle.enterprise dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- settings.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/settings.gradle b/settings.gradle index a2d2f8d0cb..13340c5773 100644 --- a/settings.gradle +++ b/settings.gradle @@ -5,7 +5,7 @@ pluginManagement { } plugins { - id 'com.gradle.enterprise' version '3.12.4' + id 'com.gradle.enterprise' version '3.12.5' id 'io.spring.ge.conventions' version '0.0.13' id 'org.gradle.toolchains.foojay-resolver-convention' version '0.4.0' } From 83b0d0d9cf21fd6432d431e0c59ff48a394f052b Mon Sep 17 00:00:00 2001 From: Matthieu Borgraeve <3082664+mborgraeve@users.noreply.github.com> Date: Thu, 23 Mar 2023 14:59:54 -0400 Subject: [PATCH 02/21] Allow custom sink for LoggingMeterRegistry (#3685) * feat: allowing the LoggingMeterRegistry to be provided with a sink instead of the default `log::info`. Signed-off-by: Matthieu Borgraeve <3082664+mborgraeve@users.noreply.github.com> * Polishing javadoc and tests --------- Signed-off-by: Matthieu Borgraeve <3082664+mborgraeve@users.noreply.github.com> Co-authored-by: Matthieu Borgraeve Co-authored-by: Jonatan Ivanov --- .../logging/LoggingMeterRegistry.java | 26 +++++++++++++++- .../logging/LoggingMeterRegistryTest.java | 31 +++++++++++++++++++ 2 files changed, 56 insertions(+), 1 deletion(-) diff --git a/micrometer-core/src/main/java/io/micrometer/core/instrument/logging/LoggingMeterRegistry.java b/micrometer-core/src/main/java/io/micrometer/core/instrument/logging/LoggingMeterRegistry.java index daa3029520..7ca1860ed0 100644 --- a/micrometer-core/src/main/java/io/micrometer/core/instrument/logging/LoggingMeterRegistry.java +++ b/micrometer-core/src/main/java/io/micrometer/core/instrument/logging/LoggingMeterRegistry.java @@ -48,6 +48,7 @@ * Logging {@link io.micrometer.core.instrument.MeterRegistry}. * * @author Jon Schneider + * @author Matthieu Borgraeve * @since 1.1.0 */ @Incubating(since = "1.1.0") @@ -65,8 +66,31 @@ public LoggingMeterRegistry() { this(LoggingRegistryConfig.DEFAULT, Clock.SYSTEM); } + /** + * Constructor allowing a custom clock and configuration. + * @param config the LoggingRegistryConfig + * @param clock the Clock + */ public LoggingMeterRegistry(LoggingRegistryConfig config, Clock clock) { - this(config, clock, new NamedThreadFactory("logging-metrics-publisher"), log::info, null); + this(config, clock, log::info); + } + + /** + * Constructor allowing custom sink instead of a default {@code log::info}. + * @param loggingSink the custom sink that will be called for each time series. + */ + public LoggingMeterRegistry(Consumer loggingSink) { + this(LoggingRegistryConfig.DEFAULT, Clock.SYSTEM, loggingSink); + } + + /** + * Constructor allowing a custom sink, clock and configuration. + * @param config the LoggingRegistryConfig + * @param clock the Clock + * @param loggingSink the custom sink that will be called for each time series. + */ + public LoggingMeterRegistry(LoggingRegistryConfig config, Clock clock, Consumer loggingSink) { + this(config, clock, new NamedThreadFactory("logging-metrics-publisher"), loggingSink, null); } private LoggingMeterRegistry(LoggingRegistryConfig config, Clock clock, ThreadFactory threadFactory, diff --git a/micrometer-core/src/test/java/io/micrometer/core/instrument/logging/LoggingMeterRegistryTest.java b/micrometer-core/src/test/java/io/micrometer/core/instrument/logging/LoggingMeterRegistryTest.java index 8e4b7daefc..172007d622 100644 --- a/micrometer-core/src/test/java/io/micrometer/core/instrument/logging/LoggingMeterRegistryTest.java +++ b/micrometer-core/src/test/java/io/micrometer/core/instrument/logging/LoggingMeterRegistryTest.java @@ -21,14 +21,18 @@ import java.util.Arrays; import java.util.Collections; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.Consumer; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.*; /** * Tests for {@link LoggingMeterRegistry}. * * @author Jon Schneider * @author Johnny Lim + * @author Matthieu Borgraeve */ class LoggingMeterRegistryTest { @@ -43,6 +47,33 @@ void defaultMeterIdPrinter() { assertThat(printer.id()).isEqualTo("my.gauage{tag-1=tag-2}"); } + @Test + void providedSinkFromConstructorShouldBeUsed() { + String expectedString = "my.gauage{tag-1=tag-2} value=1"; + Consumer mockConsumer = mock(Consumer.class); + doNothing().when(mockConsumer).accept(expectedString); + AtomicInteger gaugeValue = new AtomicInteger(1); + LoggingMeterRegistry registry = new LoggingMeterRegistry(LoggingRegistryConfig.DEFAULT, Clock.SYSTEM, + mockConsumer); + registry.gauge("my.gauage", Tags.of("tag-1", "tag-2"), gaugeValue); + + registry.publish(); + verify(mockConsumer, times(1)).accept(expectedString); + } + + @Test + void providedSinkFromConstructorShouldBeUsedWithDefaults() { + String expectedString = "my.gauage{tag-1=tag-2} value=1"; + Consumer mockConsumer = mock(Consumer.class); + doNothing().when(mockConsumer).accept(expectedString); + AtomicInteger gaugeValue = new AtomicInteger(1); + LoggingMeterRegistry registry = new LoggingMeterRegistry(mockConsumer); + registry.gauge("my.gauage", Tags.of("tag-1", "tag-2"), gaugeValue); + + registry.publish(); + verify(mockConsumer, times(1)).accept(expectedString); + } + @Test void customMeterIdPrinter() { LoggingMeterRegistry registry = LoggingMeterRegistry.builder(LoggingRegistryConfig.DEFAULT) From 1832bb1d52411a338bd174e7658c6218b0ab672c Mon Sep 17 00:00:00 2001 From: Tommy Ludwig <8924140+shakuzen@users.noreply.github.com> Date: Fri, 24 Mar 2023 17:05:42 +0900 Subject: [PATCH 03/21] Do not publish on close if already closed Checks if the registry is closed before calling publish on close. This avoids side effects of calling close multiple times. Resolves gh-3712 --- .../instrument/push/PushMeterRegistry.java | 2 +- .../push/PushMeterRegistryTest.java | 81 ++++++++++++++++++- 2 files changed, 81 insertions(+), 2 deletions(-) diff --git a/micrometer-core/src/main/java/io/micrometer/core/instrument/push/PushMeterRegistry.java b/micrometer-core/src/main/java/io/micrometer/core/instrument/push/PushMeterRegistry.java index ea3c6633d6..c40ec8e195 100644 --- a/micrometer-core/src/main/java/io/micrometer/core/instrument/push/PushMeterRegistry.java +++ b/micrometer-core/src/main/java/io/micrometer/core/instrument/push/PushMeterRegistry.java @@ -92,7 +92,7 @@ public void stop() { @Override public void close() { - if (config.enabled()) { + if (config.enabled() && !isClosed()) { publishSafely(); } stop(); diff --git a/micrometer-core/src/test/java/io/micrometer/core/instrument/push/PushMeterRegistryTest.java b/micrometer-core/src/test/java/io/micrometer/core/instrument/push/PushMeterRegistryTest.java index 7c5e51f93f..9e7e849ba7 100644 --- a/micrometer-core/src/test/java/io/micrometer/core/instrument/push/PushMeterRegistryTest.java +++ b/micrometer-core/src/test/java/io/micrometer/core/instrument/push/PushMeterRegistryTest.java @@ -15,7 +15,10 @@ */ package io.micrometer.core.instrument.push; -import io.micrometer.core.instrument.MockClock; +import io.micrometer.core.Issue; +import io.micrometer.core.instrument.*; +import io.micrometer.core.instrument.distribution.DistributionStatisticConfig; +import io.micrometer.core.instrument.distribution.pause.PauseDetector; import io.micrometer.core.instrument.step.StepMeterRegistry; import io.micrometer.core.instrument.step.StepRegistryConfig; import io.micrometer.core.instrument.util.NamedThreadFactory; @@ -26,6 +29,9 @@ import java.util.concurrent.CountDownLatch; import java.util.concurrent.ThreadFactory; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.ToDoubleFunction; +import java.util.function.ToLongFunction; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatCode; @@ -76,6 +82,79 @@ void whenUncaughtExceptionInPublish_closeRegistrySuccessful() { assertThatCode(() -> pushMeterRegistry.close()).doesNotThrowAnyException(); } + @Test + @Issue("#3712") + void publishOnlyHappensOnceWithMultipleClose() { + pushMeterRegistry = new CountingPushMeterRegistry(config, Clock.SYSTEM); + pushMeterRegistry.close(); + assertThat(((CountingPushMeterRegistry) pushMeterRegistry).publishCount.get()).isOne(); + pushMeterRegistry.close(); + assertThat(((CountingPushMeterRegistry) pushMeterRegistry).publishCount.get()).isOne(); + } + + static class CountingPushMeterRegistry extends PushMeterRegistry { + + AtomicInteger publishCount = new AtomicInteger(); + + protected CountingPushMeterRegistry(PushRegistryConfig config, Clock clock) { + super(config, clock); + } + + @Override + protected Gauge newGauge(Meter.Id id, T obj, ToDoubleFunction valueFunction) { + return null; + } + + @Override + protected Counter newCounter(Meter.Id id) { + return null; + } + + @Override + protected Timer newTimer(Meter.Id id, DistributionStatisticConfig distributionStatisticConfig, + PauseDetector pauseDetector) { + return null; + } + + @Override + protected DistributionSummary newDistributionSummary(Meter.Id id, + DistributionStatisticConfig distributionStatisticConfig, double scale) { + return null; + } + + @Override + protected Meter newMeter(Meter.Id id, Meter.Type type, Iterable measurements) { + return null; + } + + @Override + protected FunctionTimer newFunctionTimer(Meter.Id id, T obj, ToLongFunction countFunction, + ToDoubleFunction totalTimeFunction, TimeUnit totalTimeFunctionUnit) { + return null; + } + + @Override + protected FunctionCounter newFunctionCounter(Meter.Id id, T obj, ToDoubleFunction countFunction) { + return null; + } + + @Override + protected TimeUnit getBaseTimeUnit() { + return null; + } + + @Override + protected DistributionStatisticConfig defaultHistogramConfig() { + return null; + } + + @Override + protected void publish() { + publishCount.incrementAndGet(); + } + + } + static class ThrowingPushMeterRegistry extends StepMeterRegistry { final CountDownLatch countDownLatch; From 938f9954b3367f7afa61161e6f9bd47cc3cda244 Mon Sep 17 00:00:00 2001 From: Johnny Lim Date: Sat, 25 Mar 2023 17:28:45 +0900 Subject: [PATCH 04/21] Add Javadoc since to new constructors in LoggingMeterRegistry (#3716) See gh-3685 --- .../core/instrument/logging/LoggingMeterRegistry.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/micrometer-core/src/main/java/io/micrometer/core/instrument/logging/LoggingMeterRegistry.java b/micrometer-core/src/main/java/io/micrometer/core/instrument/logging/LoggingMeterRegistry.java index 7ca1860ed0..cab7455cf5 100644 --- a/micrometer-core/src/main/java/io/micrometer/core/instrument/logging/LoggingMeterRegistry.java +++ b/micrometer-core/src/main/java/io/micrometer/core/instrument/logging/LoggingMeterRegistry.java @@ -78,6 +78,7 @@ public LoggingMeterRegistry(LoggingRegistryConfig config, Clock clock) { /** * Constructor allowing custom sink instead of a default {@code log::info}. * @param loggingSink the custom sink that will be called for each time series. + * @since 1.11.0 */ public LoggingMeterRegistry(Consumer loggingSink) { this(LoggingRegistryConfig.DEFAULT, Clock.SYSTEM, loggingSink); @@ -88,6 +89,7 @@ public LoggingMeterRegistry(Consumer loggingSink) { * @param config the LoggingRegistryConfig * @param clock the Clock * @param loggingSink the custom sink that will be called for each time series. + * @since 1.11.0 */ public LoggingMeterRegistry(LoggingRegistryConfig config, Clock clock, Consumer loggingSink) { this(config, clock, new NamedThreadFactory("logging-metrics-publisher"), loggingSink, null); From 42c0bbfdb03f69b220d0a5cef8450365d1ac7547 Mon Sep 17 00:00:00 2001 From: Tommy Ludwig <8924140+shakuzen@users.noreply.github.com> Date: Mon, 27 Mar 2023 19:37:40 +0900 Subject: [PATCH 05/21] Polish compiler warnings --- .../logging/LoggingMeterRegistryTest.java | 16 ++++++++-------- .../instrument/step/StepFunctionCounterTest.java | 2 +- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/micrometer-core/src/test/java/io/micrometer/core/instrument/logging/LoggingMeterRegistryTest.java b/micrometer-core/src/test/java/io/micrometer/core/instrument/logging/LoggingMeterRegistryTest.java index 172007d622..f53c0c8336 100644 --- a/micrometer-core/src/test/java/io/micrometer/core/instrument/logging/LoggingMeterRegistryTest.java +++ b/micrometer-core/src/test/java/io/micrometer/core/instrument/logging/LoggingMeterRegistryTest.java @@ -18,10 +18,12 @@ import io.micrometer.core.instrument.*; import io.micrometer.core.instrument.binder.BaseUnits; import org.junit.jupiter.api.Test; +import org.mockito.Mockito; import java.util.Arrays; import java.util.Collections; import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicReference; import java.util.function.Consumer; import static org.assertj.core.api.Assertions.assertThat; @@ -50,28 +52,26 @@ void defaultMeterIdPrinter() { @Test void providedSinkFromConstructorShouldBeUsed() { String expectedString = "my.gauage{tag-1=tag-2} value=1"; - Consumer mockConsumer = mock(Consumer.class); - doNothing().when(mockConsumer).accept(expectedString); + AtomicReference actual = new AtomicReference<>(); AtomicInteger gaugeValue = new AtomicInteger(1); LoggingMeterRegistry registry = new LoggingMeterRegistry(LoggingRegistryConfig.DEFAULT, Clock.SYSTEM, - mockConsumer); + actual::set); registry.gauge("my.gauage", Tags.of("tag-1", "tag-2"), gaugeValue); registry.publish(); - verify(mockConsumer, times(1)).accept(expectedString); + assertThat(actual.get()).isEqualTo(expectedString); } @Test void providedSinkFromConstructorShouldBeUsedWithDefaults() { String expectedString = "my.gauage{tag-1=tag-2} value=1"; - Consumer mockConsumer = mock(Consumer.class); - doNothing().when(mockConsumer).accept(expectedString); + AtomicReference actual = new AtomicReference<>(); AtomicInteger gaugeValue = new AtomicInteger(1); - LoggingMeterRegistry registry = new LoggingMeterRegistry(mockConsumer); + LoggingMeterRegistry registry = new LoggingMeterRegistry(actual::set); registry.gauge("my.gauage", Tags.of("tag-1", "tag-2"), gaugeValue); registry.publish(); - verify(mockConsumer, times(1)).accept(expectedString); + assertThat(actual.get()).isEqualTo(expectedString); } @Test diff --git a/micrometer-core/src/test/java/io/micrometer/core/instrument/step/StepFunctionCounterTest.java b/micrometer-core/src/test/java/io/micrometer/core/instrument/step/StepFunctionCounterTest.java index 4975da64b2..d376c69ff9 100644 --- a/micrometer-core/src/test/java/io/micrometer/core/instrument/step/StepFunctionCounterTest.java +++ b/micrometer-core/src/test/java/io/micrometer/core/instrument/step/StepFunctionCounterTest.java @@ -67,7 +67,7 @@ void count() { @Test void manualRolloverPartialStep() { AtomicInteger n = new AtomicInteger(3); - StepFunctionCounter counter = (StepFunctionCounter) registry.more() + @SuppressWarnings({"rawtypes", "unchecked"}) StepFunctionCounter counter = (StepFunctionCounter) registry.more() .counter("my.counter", Tags.empty(), n); assertThat(counter.count()).isZero(); From 50d8b74345c4b2ee2f407f1b1ea4d27ec7735a99 Mon Sep 17 00:00:00 2001 From: Tommy Ludwig <8924140+shakuzen@users.noreply.github.com> Date: Mon, 27 Mar 2023 19:50:17 +0900 Subject: [PATCH 06/21] Polish and update ModifiedClassPathClassLoader Copy the latest changes from Spring Boot. Fix formatting and checkstyle issues in the previous commit. --- .../logging/LoggingMeterRegistryTest.java | 5 +- .../step/StepFunctionCounterTest.java | 3 +- .../ModifiedClassPathClassLoader.java | 117 ++++++++++++------ .../classpath/ModifiedClassPathExtension.java | 51 ++++---- .../testsupport/classpath/package-info.java | 21 ++++ 5 files changed, 127 insertions(+), 70 deletions(-) create mode 100644 micrometer-core/src/test/java/io/micrometer/core/testsupport/classpath/package-info.java diff --git a/micrometer-core/src/test/java/io/micrometer/core/instrument/logging/LoggingMeterRegistryTest.java b/micrometer-core/src/test/java/io/micrometer/core/instrument/logging/LoggingMeterRegistryTest.java index f53c0c8336..72fb2d42f2 100644 --- a/micrometer-core/src/test/java/io/micrometer/core/instrument/logging/LoggingMeterRegistryTest.java +++ b/micrometer-core/src/test/java/io/micrometer/core/instrument/logging/LoggingMeterRegistryTest.java @@ -18,16 +18,13 @@ import io.micrometer.core.instrument.*; import io.micrometer.core.instrument.binder.BaseUnits; import org.junit.jupiter.api.Test; -import org.mockito.Mockito; import java.util.Arrays; import java.util.Collections; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; -import java.util.function.Consumer; import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.Mockito.*; /** * Tests for {@link LoggingMeterRegistry}. @@ -55,7 +52,7 @@ void providedSinkFromConstructorShouldBeUsed() { AtomicReference actual = new AtomicReference<>(); AtomicInteger gaugeValue = new AtomicInteger(1); LoggingMeterRegistry registry = new LoggingMeterRegistry(LoggingRegistryConfig.DEFAULT, Clock.SYSTEM, - actual::set); + actual::set); registry.gauge("my.gauage", Tags.of("tag-1", "tag-2"), gaugeValue); registry.publish(); diff --git a/micrometer-core/src/test/java/io/micrometer/core/instrument/step/StepFunctionCounterTest.java b/micrometer-core/src/test/java/io/micrometer/core/instrument/step/StepFunctionCounterTest.java index d376c69ff9..3042c5d9d1 100644 --- a/micrometer-core/src/test/java/io/micrometer/core/instrument/step/StepFunctionCounterTest.java +++ b/micrometer-core/src/test/java/io/micrometer/core/instrument/step/StepFunctionCounterTest.java @@ -67,7 +67,8 @@ void count() { @Test void manualRolloverPartialStep() { AtomicInteger n = new AtomicInteger(3); - @SuppressWarnings({"rawtypes", "unchecked"}) StepFunctionCounter counter = (StepFunctionCounter) registry.more() + @SuppressWarnings({ "rawtypes", "unchecked" }) + StepFunctionCounter counter = (StepFunctionCounter) registry.more() .counter("my.counter", Tags.empty(), n); assertThat(counter.count()).isZero(); diff --git a/micrometer-core/src/test/java/io/micrometer/core/testsupport/classpath/ModifiedClassPathClassLoader.java b/micrometer-core/src/test/java/io/micrometer/core/testsupport/classpath/ModifiedClassPathClassLoader.java index f6189ea4d5..8bb244f4dd 100644 --- a/micrometer-core/src/test/java/io/micrometer/core/testsupport/classpath/ModifiedClassPathClassLoader.java +++ b/micrometer-core/src/test/java/io/micrometer/core/testsupport/classpath/ModifiedClassPathClassLoader.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2022 the original author or authors. + * Copyright 2012-2023 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,26 @@ package io.micrometer.core.testsupport.classpath; +import java.io.File; +import java.lang.management.ManagementFactory; +import java.lang.reflect.AnnotatedElement; +import java.lang.reflect.Method; +import java.net.URISyntaxException; +import java.net.URL; +import java.net.URLClassLoader; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.jar.Attributes; +import java.util.jar.JarFile; +import java.util.regex.Pattern; +import java.util.stream.Collectors; +import java.util.stream.Stream; + import org.apache.maven.repository.internal.MavenRepositorySystemUtils; import org.eclipse.aether.DefaultRepositorySystemSession; import org.eclipse.aether.RepositorySystem; @@ -32,23 +52,14 @@ import org.eclipse.aether.spi.connector.RepositoryConnectorFactory; import org.eclipse.aether.spi.connector.transport.TransporterFactory; import org.eclipse.aether.transport.http.HttpTransporterFactory; + import org.springframework.core.annotation.MergedAnnotation; import org.springframework.core.annotation.MergedAnnotations; import org.springframework.util.AntPathMatcher; import org.springframework.util.ConcurrentReferenceHashMap; +import org.springframework.util.ObjectUtils; import org.springframework.util.StringUtils; -import java.io.File; -import java.lang.management.ManagementFactory; -import java.net.URISyntaxException; -import java.net.URL; -import java.net.URLClassLoader; -import java.util.*; -import java.util.jar.Attributes; -import java.util.jar.JarFile; -import java.util.regex.Pattern; -import java.util.stream.Stream; - /** * Custom {@link URLClassLoader} that modifies the class path. * @@ -57,7 +68,7 @@ */ final class ModifiedClassPathClassLoader extends URLClassLoader { - private static final Map, ModifiedClassPathClassLoader> cache = new ConcurrentReferenceHashMap<>(); + private static final Map, ModifiedClassPathClassLoader> cache = new ConcurrentReferenceHashMap<>(); private static final Pattern INTELLIJ_CLASSPATH_JAR_PATTERN = Pattern.compile(".*classpath(\\d+)?\\.jar"); @@ -79,19 +90,45 @@ public Class loadClass(String name) throws ClassNotFoundException { return super.loadClass(name); } - static ModifiedClassPathClassLoader get(Class testClass) { - return cache.computeIfAbsent(testClass, ModifiedClassPathClassLoader::compute); + static ModifiedClassPathClassLoader get(Class testClass, Method testMethod, List arguments) { + Set candidates = new LinkedHashSet<>(); + candidates.add(testClass); + candidates.add(testMethod); + candidates.addAll(getAnnotatedElements(arguments.toArray())); + List annotatedElements = candidates.stream() + .filter(ModifiedClassPathClassLoader::hasAnnotation) + .collect(Collectors.toList()); + if (annotatedElements.isEmpty()) { + return null; + } + return cache.computeIfAbsent(annotatedElements, (key) -> compute(testClass.getClassLoader(), key)); } - private static ModifiedClassPathClassLoader compute(Class testClass) { - ClassLoader classLoader = testClass.getClassLoader(); - MergedAnnotations annotations = MergedAnnotations.from(testClass, - MergedAnnotations.SearchStrategy.TYPE_HIERARCHY); - if (annotations.isPresent(ForkedClassPath.class) && (annotations.isPresent(ClassPathOverrides.class) - || annotations.isPresent(ClassPathExclusions.class))) { - throw new IllegalStateException("@ForkedClassPath is redundant in combination with either " - + "@ClassPathOverrides or @ClassPathExclusions"); + private static Collection getAnnotatedElements(Object[] array) { + Set result = new LinkedHashSet<>(); + for (Object item : array) { + if (item instanceof AnnotatedElement) { + result.add((AnnotatedElement) item); + } + else if (ObjectUtils.isArray(item)) { + result.addAll(getAnnotatedElements(ObjectUtils.toObjectArray(item))); + } } + return result; + } + + private static boolean hasAnnotation(AnnotatedElement element) { + MergedAnnotations annotations = MergedAnnotations.from(element, + MergedAnnotations.SearchStrategy.TYPE_HIERARCHY); + return annotations.isPresent(ForkedClassPath.class) || annotations.isPresent(ClassPathOverrides.class) + || annotations.isPresent(ClassPathExclusions.class); + } + + private static ModifiedClassPathClassLoader compute(ClassLoader classLoader, + List annotatedClasses) { + List annotations = annotatedClasses.stream() + .map((source) -> MergedAnnotations.from(source, MergedAnnotations.SearchStrategy.TYPE_HIERARCHY)) + .toList(); return new ModifiedClassPathClassLoader(processUrls(extractUrls(classLoader), annotations), classLoader.getParent(), classLoader); } @@ -110,8 +147,7 @@ private static URL[] extractUrls(ClassLoader classLoader) { } private static Stream doExtractUrls(ClassLoader classLoader) { - if (classLoader instanceof URLClassLoader) { - URLClassLoader urlClassLoader = (URLClassLoader) classLoader; + if (classLoader instanceof URLClassLoader urlClassLoader) { return Stream.of(urlClassLoader.getURLs()); } return Stream.of(ManagementFactory.getRuntimeMXBean().getClassPath().split(File.pathSeparator)) @@ -170,9 +206,9 @@ private static Attributes getManifestMainAttributesFromUrl(URL url) throws Excep } } - private static URL[] processUrls(URL[] urls, MergedAnnotations annotations) { - ClassPathEntryFilter filter = new ClassPathEntryFilter(annotations.get(ClassPathExclusions.class)); - List additionalUrls = getAdditionalUrls(annotations.get(ClassPathOverrides.class)); + private static URL[] processUrls(URL[] urls, List annotations) { + ClassPathEntryFilter filter = new ClassPathEntryFilter(annotations); + List additionalUrls = getAdditionalUrls(annotations); List processedUrls = new ArrayList<>(additionalUrls); for (URL url : urls) { if (!filter.isExcluded(url)) { @@ -182,11 +218,15 @@ private static URL[] processUrls(URL[] urls, MergedAnnotations annotations) { return processedUrls.toArray(new URL[0]); } - private static List getAdditionalUrls(MergedAnnotation annotation) { - if (!annotation.isPresent()) { - return Collections.emptyList(); + private static List getAdditionalUrls(List annotations) { + Set urls = new LinkedHashSet<>(); + for (MergedAnnotations candidate : annotations) { + MergedAnnotation annotation = candidate.get(ClassPathOverrides.class); + if (annotation.isPresent()) { + urls.addAll(resolveCoordinates(annotation.getStringArray(MergedAnnotation.VALUE))); + } } - return resolveCoordinates(annotation.getStringArray(MergedAnnotation.VALUE)); + return urls.stream().toList(); } private static List resolveCoordinates(String[] coordinates) { @@ -238,10 +278,15 @@ private static final class ClassPathEntryFilter { private final AntPathMatcher matcher = new AntPathMatcher(); - private ClassPathEntryFilter(MergedAnnotation annotation) { - this.exclusions = annotation.getValue(MergedAnnotation.VALUE, String[].class) - .map(Arrays::asList) - .orElse(Collections.emptyList()); + private ClassPathEntryFilter(List annotations) { + Set exclusions = new LinkedHashSet<>(); + for (MergedAnnotations candidate : annotations) { + MergedAnnotation annotation = candidate.get(ClassPathExclusions.class); + if (annotation.isPresent()) { + exclusions.addAll(Arrays.asList(annotation.getStringArray(MergedAnnotation.VALUE))); + } + } + this.exclusions = exclusions.stream().toList(); } private boolean isExcluded(URL url) { diff --git a/micrometer-core/src/test/java/io/micrometer/core/testsupport/classpath/ModifiedClassPathExtension.java b/micrometer-core/src/test/java/io/micrometer/core/testsupport/classpath/ModifiedClassPathExtension.java index 5f43f87c87..c565dbd1ff 100644 --- a/micrometer-core/src/test/java/io/micrometer/core/testsupport/classpath/ModifiedClassPathExtension.java +++ b/micrometer-core/src/test/java/io/micrometer/core/testsupport/classpath/ModifiedClassPathExtension.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2021 the original author or authors. + * Copyright 2012-2023 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. @@ -28,9 +28,7 @@ import org.junit.platform.launcher.core.LauncherFactory; import org.junit.platform.launcher.listeners.SummaryGeneratingListener; import org.junit.platform.launcher.listeners.TestExecutionSummary; -import org.springframework.util.Assert; import org.springframework.util.CollectionUtils; -import org.springframework.util.ReflectionUtils; import java.lang.reflect.Method; import java.net.URLClassLoader; @@ -74,34 +72,43 @@ public void interceptAfterAllMethod(Invocation invocation, @Override public void interceptTestMethod(Invocation invocation, ReflectiveInvocationContext invocationContext, ExtensionContext extensionContext) throws Throwable { + interceptMethod(invocation, invocationContext, extensionContext); + } + + @Override + public void interceptTestTemplateMethod(Invocation invocation, + ReflectiveInvocationContext invocationContext, ExtensionContext extensionContext) throws Throwable { + interceptMethod(invocation, invocationContext, extensionContext); + } + + private void interceptMethod(Invocation invocation, ReflectiveInvocationContext invocationContext, + ExtensionContext extensionContext) throws Throwable { if (isModifiedClassPathClassLoader(extensionContext)) { invocation.proceed(); return; } - invocation.skip(); - runTestWithModifiedClassPath(invocationContext, extensionContext); - } - - private void runTestWithModifiedClassPath(ReflectiveInvocationContext invocationContext, - ExtensionContext extensionContext) throws Throwable { Class testClass = extensionContext.getRequiredTestClass(); Method testMethod = invocationContext.getExecutable(); + URLClassLoader modifiedClassLoader = ModifiedClassPathClassLoader.get(testClass, testMethod, + invocationContext.getArguments()); + if (modifiedClassLoader == null) { + invocation.proceed(); + return; + } + invocation.skip(); ClassLoader originalClassLoader = Thread.currentThread().getContextClassLoader(); - URLClassLoader modifiedClassLoader = ModifiedClassPathClassLoader.get(testClass); Thread.currentThread().setContextClassLoader(modifiedClassLoader); try { - runTest(modifiedClassLoader, testClass.getName(), testMethod.getName()); + runTest(extensionContext.getUniqueId()); } finally { Thread.currentThread().setContextClassLoader(originalClassLoader); } } - private void runTest(ClassLoader classLoader, String testClassName, String testMethodName) throws Throwable { - Class testClass = classLoader.loadClass(testClassName); - Method testMethod = findMethod(testClass, testMethodName); + private void runTest(String testId) throws Throwable { LauncherDiscoveryRequest request = LauncherDiscoveryRequestBuilder.request() - .selectors(DiscoverySelectors.selectMethod(testClass, testMethod)) + .selectors(DiscoverySelectors.selectUniqueId(testId)) .build(); Launcher launcher = LauncherFactory.create(); TestPlan testPlan = launcher.discover(request); @@ -114,20 +121,6 @@ private void runTest(ClassLoader classLoader, String testClassName, String testM } } - private Method findMethod(Class testClass, String testMethodName) { - Method method = ReflectionUtils.findMethod(testClass, testMethodName); - if (method == null) { - Method[] methods = ReflectionUtils.getUniqueDeclaredMethods(testClass); - for (Method candidate : methods) { - if (candidate.getName().equals(testMethodName)) { - return candidate; - } - } - } - Assert.state(method != null, () -> "Unable to find " + testClass + "." + testMethodName); - return method; - } - private void intercept(Invocation invocation, ExtensionContext extensionContext) throws Throwable { if (isModifiedClassPathClassLoader(extensionContext)) { invocation.proceed(); diff --git a/micrometer-core/src/test/java/io/micrometer/core/testsupport/classpath/package-info.java b/micrometer-core/src/test/java/io/micrometer/core/testsupport/classpath/package-info.java new file mode 100644 index 0000000000..1cfc2122f1 --- /dev/null +++ b/micrometer-core/src/test/java/io/micrometer/core/testsupport/classpath/package-info.java @@ -0,0 +1,21 @@ +/* + * 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. + * 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. + */ + +/** + * Custom JUnit extension to change the classpath. The code in this package was copied + * from the Spring Boot project. + */ +package io.micrometer.core.testsupport.classpath; From adc4bd9dd2d1fa89260b139f6abf91cf41d92e33 Mon Sep 17 00:00:00 2001 From: Tommy Ludwig <8924140+shakuzen@users.noreply.github.com> Date: Mon, 27 Mar 2023 21:32:28 +0900 Subject: [PATCH 07/21] Fix JDK 11 compile errors This can be rolled back if we decide to drop building with JDK 11. See gh-3718. --- .../classpath/ModifiedClassPathClassLoader.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/micrometer-core/src/test/java/io/micrometer/core/testsupport/classpath/ModifiedClassPathClassLoader.java b/micrometer-core/src/test/java/io/micrometer/core/testsupport/classpath/ModifiedClassPathClassLoader.java index 8bb244f4dd..afd69e9071 100644 --- a/micrometer-core/src/test/java/io/micrometer/core/testsupport/classpath/ModifiedClassPathClassLoader.java +++ b/micrometer-core/src/test/java/io/micrometer/core/testsupport/classpath/ModifiedClassPathClassLoader.java @@ -128,7 +128,7 @@ private static ModifiedClassPathClassLoader compute(ClassLoader classLoader, List annotatedClasses) { List annotations = annotatedClasses.stream() .map((source) -> MergedAnnotations.from(source, MergedAnnotations.SearchStrategy.TYPE_HIERARCHY)) - .toList(); + .collect(Collectors.toList()); return new ModifiedClassPathClassLoader(processUrls(extractUrls(classLoader), annotations), classLoader.getParent(), classLoader); } @@ -147,7 +147,8 @@ private static URL[] extractUrls(ClassLoader classLoader) { } private static Stream doExtractUrls(ClassLoader classLoader) { - if (classLoader instanceof URLClassLoader urlClassLoader) { + if (classLoader instanceof URLClassLoader) { + URLClassLoader urlClassLoader = (URLClassLoader) classLoader; return Stream.of(urlClassLoader.getURLs()); } return Stream.of(ManagementFactory.getRuntimeMXBean().getClassPath().split(File.pathSeparator)) @@ -226,7 +227,7 @@ private static List getAdditionalUrls(List annotations) urls.addAll(resolveCoordinates(annotation.getStringArray(MergedAnnotation.VALUE))); } } - return urls.stream().toList(); + return urls.stream().collect(Collectors.toList()); } private static List resolveCoordinates(String[] coordinates) { @@ -286,7 +287,7 @@ private ClassPathEntryFilter(List annotations) { exclusions.addAll(Arrays.asList(annotation.getStringArray(MergedAnnotation.VALUE))); } } - this.exclusions = exclusions.stream().toList(); + this.exclusions = exclusions.stream().collect(Collectors.toList()); } private boolean isExcluded(URL url) { From d6bd0bff1c44afe5ea67b0aa443e965c07483ae4 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 27 Mar 2023 16:53:06 -0700 Subject: [PATCH 08/21] Bump com.gradle.enterprise from 3.12.5 to 3.12.6 (#3719) Bumps com.gradle.enterprise from 3.12.5 to 3.12.6. --- updated-dependencies: - dependency-name: com.gradle.enterprise dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- settings.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/settings.gradle b/settings.gradle index 13340c5773..e31a1a891c 100644 --- a/settings.gradle +++ b/settings.gradle @@ -5,7 +5,7 @@ pluginManagement { } plugins { - id 'com.gradle.enterprise' version '3.12.5' + id 'com.gradle.enterprise' version '3.12.6' id 'io.spring.ge.conventions' version '0.0.13' id 'org.gradle.toolchains.foojay-resolver-convention' version '0.4.0' } From fddaf1222b7ba07dd5b7fda85a7bb91d23944ada Mon Sep 17 00:00:00 2001 From: Lenin Jaganathan <32874349+lenin-jaganathan@users.noreply.github.com> Date: Tue, 28 Mar 2023 10:54:32 +0530 Subject: [PATCH 09/21] Avoid concurrent publish calls from publishSafely (#3714) Guards calling `publish` in `publishSafely` with an `isPublishing` atomic flag. This will prevent the publish on close from happening if a scheduled publish is already in progress. It does not protect against calls to the `publish` method directly since that is implemented by children of PushMeterRegistry. This also exposes a protected `isPublishing` method that child classes can use if they want to do any conditional logic on this new flag. `StepMeterRegistry` will use this to avoid calling manualRollover on close if already publishing. Resolves gh-3711 Co-authored-by: sonatype-lift[bot] <37194012+sonatype-lift[bot]@users.noreply.github.com> Co-authored-by: Tommy Ludwig <8924140+shakuzen@users.noreply.github.com> --- .../instrument/push/PushMeterRegistry.java | 32 +++++- .../instrument/step/StepMeterRegistry.java | 10 +- .../push/PushMeterRegistryTest.java | 108 +++++++++++++++--- 3 files changed, 126 insertions(+), 24 deletions(-) diff --git a/micrometer-core/src/main/java/io/micrometer/core/instrument/push/PushMeterRegistry.java b/micrometer-core/src/main/java/io/micrometer/core/instrument/push/PushMeterRegistry.java index c5346e4092..7fe83df242 100644 --- a/micrometer-core/src/main/java/io/micrometer/core/instrument/push/PushMeterRegistry.java +++ b/micrometer-core/src/main/java/io/micrometer/core/instrument/push/PushMeterRegistry.java @@ -26,6 +26,7 @@ import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ThreadFactory; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; public abstract class PushMeterRegistry extends MeterRegistry { @@ -33,6 +34,8 @@ public abstract class PushMeterRegistry extends MeterRegistry { private final PushRegistryConfig config; + private final AtomicBoolean publishing = new AtomicBoolean(false); + @Nullable private ScheduledExecutorService scheduledExecutorService; @@ -49,15 +52,32 @@ protected PushMeterRegistry(PushRegistryConfig config, Clock clock) { /** * Catch uncaught exceptions thrown from {@link #publish()}. */ - private void publishSafely() { - try { - publish(); + // VisibleForTesting + void publishSafely() { + if (this.publishing.compareAndSet(false, true)) { + try { + publish(); + } + catch (Throwable e) { + logger.warn("Unexpected exception thrown while publishing metrics for " + getClass().getSimpleName(), + e); + } + finally { + this.publishing.set(false); + } } - catch (Throwable e) { - logger.warn("Unexpected exception thrown while publishing metrics for " + getClass().getSimpleName(), e); + else { + logger.warn("Publishing is already in progress. Skipping duplicate call to publishSafely()."); } } + /** + * Returns - true if scheduled publishing of metrics is in progress. + */ + protected boolean isPublishing() { + return publishing.get(); + } + /** * @deprecated Use {@link #start(ThreadFactory)} instead. */ @@ -92,10 +112,10 @@ public void stop() { @Override public void close() { + stop(); if (config.enabled() && !isClosed()) { publishSafely(); } - stop(); super.close(); } diff --git a/micrometer-core/src/main/java/io/micrometer/core/instrument/step/StepMeterRegistry.java b/micrometer-core/src/main/java/io/micrometer/core/instrument/step/StepMeterRegistry.java index 7c3389d784..7d8ba84fb9 100644 --- a/micrometer-core/src/main/java/io/micrometer/core/instrument/step/StepMeterRegistry.java +++ b/micrometer-core/src/main/java/io/micrometer/core/instrument/step/StepMeterRegistry.java @@ -107,10 +107,12 @@ protected DistributionStatisticConfig defaultHistogramConfig() { @Override public void close() { stop(); - getMeters().stream() - .filter(meter -> meter instanceof StepMeter) - .map(meter -> (StepMeter) meter) - .forEach(StepMeter::_manualRollover); + if (!isPublishing()) { + getMeters().stream() + .filter(StepMeter.class::isInstance) + .map(StepMeter.class::cast) + .forEach(StepMeter::_manualRollover); + } super.close(); } diff --git a/micrometer-core/src/test/java/io/micrometer/core/instrument/push/PushMeterRegistryTest.java b/micrometer-core/src/test/java/io/micrometer/core/instrument/push/PushMeterRegistryTest.java index 9e7e849ba7..72ced25cb9 100644 --- a/micrometer-core/src/test/java/io/micrometer/core/instrument/push/PushMeterRegistryTest.java +++ b/micrometer-core/src/test/java/io/micrometer/core/instrument/push/PushMeterRegistryTest.java @@ -22,17 +22,21 @@ import io.micrometer.core.instrument.step.StepMeterRegistry; import io.micrometer.core.instrument.step.StepRegistryConfig; import io.micrometer.core.instrument.util.NamedThreadFactory; -import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; import java.time.Duration; -import java.util.concurrent.CountDownLatch; -import java.util.concurrent.ThreadFactory; -import java.util.concurrent.TimeUnit; +import java.util.ArrayDeque; +import java.util.Arrays; +import java.util.Deque; +import java.util.Map; +import java.util.concurrent.*; import java.util.concurrent.atomic.AtomicInteger; import java.util.function.ToDoubleFunction; import java.util.function.ToLongFunction; +import java.util.stream.Collectors; +import static java.util.concurrent.TimeUnit.MILLISECONDS; +import static java.util.concurrent.TimeUnit.SECONDS; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatCode; @@ -62,15 +66,9 @@ public String get(String key) { CountDownLatch latch = new CountDownLatch(2); - PushMeterRegistry pushMeterRegistry = new ThrowingPushMeterRegistry(config, latch); - - @AfterEach - void cleanUp() { - pushMeterRegistry.close(); - } - @Test void whenUncaughtExceptionInPublish_taskStillScheduled() throws InterruptedException { + PushMeterRegistry pushMeterRegistry = new ThrowingPushMeterRegistry(config, latch); pushMeterRegistry.start(threadFactory); assertThat(latch.await(500, TimeUnit.MILLISECONDS)) .as("publish should continue to be scheduled even if an uncaught exception is thrown") @@ -79,17 +77,99 @@ void whenUncaughtExceptionInPublish_taskStillScheduled() throws InterruptedExcep @Test void whenUncaughtExceptionInPublish_closeRegistrySuccessful() { + PushMeterRegistry pushMeterRegistry = new ThrowingPushMeterRegistry(config, latch); assertThatCode(() -> pushMeterRegistry.close()).doesNotThrowAnyException(); } @Test @Issue("#3712") void publishOnlyHappensOnceWithMultipleClose() { - pushMeterRegistry = new CountingPushMeterRegistry(config, Clock.SYSTEM); + CountingPushMeterRegistry pushMeterRegistry = new CountingPushMeterRegistry(config, Clock.SYSTEM); pushMeterRegistry.close(); - assertThat(((CountingPushMeterRegistry) pushMeterRegistry).publishCount.get()).isOne(); + assertThat(pushMeterRegistry.publishCount.get()).isOne(); pushMeterRegistry.close(); - assertThat(((CountingPushMeterRegistry) pushMeterRegistry).publishCount.get()).isOne(); + assertThat(pushMeterRegistry.publishCount.get()).isOne(); + } + + @Test + @Issue("#3711") + void scheduledPublishOverlapWithPublishOnClose() throws InterruptedException { + MockClock clock = new MockClock(); + CyclicBarrier barrier = new CyclicBarrier(2); + OverlappingStepMeterRegistry overlappingStepMeterRegistry = new OverlappingStepMeterRegistry(config, clock, + barrier); + Counter c1 = overlappingStepMeterRegistry.counter("c1"); + Counter c2 = overlappingStepMeterRegistry.counter("c2"); + c1.increment(); + c2.increment(2.5); + clock.add(config.step()); + + // simulated scheduled publish + Thread scheduledPublishingThread = new Thread( + () -> ((PushMeterRegistry) overlappingStepMeterRegistry).publishSafely(), + "scheduledMetricsPublisherThread"); + scheduledPublishingThread.start(); + // publish on shutdown + Thread onClosePublishThread = new Thread(overlappingStepMeterRegistry::close, "shutdownHookThread"); + onClosePublishThread.start(); + scheduledPublishingThread.join(); + onClosePublishThread.join(); + + assertThat(overlappingStepMeterRegistry.publishes).as("only one publish happened").hasSize(1); + Deque firstPublishValues = overlappingStepMeterRegistry.publishes.get(0); + assertThat(firstPublishValues.pop()).isEqualTo(1); + assertThat(firstPublishValues.pop()).isEqualTo(2.5); + } + + private static class OverlappingStepMeterRegistry extends StepMeterRegistry { + + private final AtomicInteger numberOfPublish = new AtomicInteger(); + + private final Map> publishes = new ConcurrentHashMap<>(); + + private final CyclicBarrier barrier; + + public OverlappingStepMeterRegistry(StepRegistryConfig config, Clock clock, CyclicBarrier barrier) { + super(config, clock); + this.barrier = barrier; + } + + @Override + protected TimeUnit getBaseTimeUnit() { + return SECONDS; + } + + @Override + protected void publish() { + try { + barrier.await(100, MILLISECONDS); + } + catch (InterruptedException | BrokenBarrierException | TimeoutException e) { + throw new RuntimeException(e); + } + int publishIndex = numberOfPublish.getAndIncrement(); + for (Counter counter : getMeters().stream() + .filter(meter -> meter instanceof Counter) + .map(meter -> (Counter) meter) + .collect(Collectors.toSet())) { + publishes.merge(publishIndex, new ArrayDeque<>(Arrays.asList(counter.count())), (l1, l2) -> { + l1.addAll(l2); + return l1; + }); + } + } + + @Override + public void close() { + try { + barrier.await(100, MILLISECONDS); + } + catch (InterruptedException | BrokenBarrierException | TimeoutException e) { + throw new RuntimeException(e); + } + super.close(); + } + } static class CountingPushMeterRegistry extends PushMeterRegistry { From 9253792563a249a61d2987e49a26a85fa0551fee Mon Sep 17 00:00:00 2001 From: Christophe Bismuth Date: Tue, 28 Mar 2023 10:21:30 +0200 Subject: [PATCH 10/21] Add Azure Application Insights connection string support (#3715) Adds a new configuration method to AzureMonitorConfig for connectionString configuration, which is meant to supersede instrumentationKey config. `instrumentationKey` is marked as deprecated and only used by default when connectionString is not set. Users should set the instrumentationKey as part of the connectionString going forward. See https://learn.microsoft.com/en-us/azure/azure-monitor/app/sdk-connection-string Resolves gh-3710 --- .../azuremonitor/AzureMonitorConfig.java | 23 +++++++++++++- .../AzureMonitorMeterRegistry.java | 6 ++-- .../azuremonitor/AzureMonitorConfigTest.java | 2 +- ...eMonitorMeterRegistryCompatibilityKit.java | 4 +-- .../AzureMonitorMeterRegistryTest.java | 31 ++++++++++++++++--- .../boot2/samples/AzureMonitorSample.java | 2 +- .../core/samples/utils/SampleRegistries.java | 23 ++++++++++++-- 7 files changed, 77 insertions(+), 14 deletions(-) diff --git a/implementations/micrometer-registry-azure-monitor/src/main/java/io/micrometer/azuremonitor/AzureMonitorConfig.java b/implementations/micrometer-registry-azure-monitor/src/main/java/io/micrometer/azuremonitor/AzureMonitorConfig.java index 84247da5eb..428e54cb2a 100644 --- a/implementations/micrometer-registry-azure-monitor/src/main/java/io/micrometer/azuremonitor/AzureMonitorConfig.java +++ b/implementations/micrometer-registry-azure-monitor/src/main/java/io/micrometer/azuremonitor/AzureMonitorConfig.java @@ -15,6 +15,7 @@ */ package io.micrometer.azuremonitor; +import io.micrometer.common.lang.Nullable; import io.micrometer.core.instrument.config.validate.Validated; import io.micrometer.core.instrument.step.StepRegistryConfig; @@ -38,15 +39,35 @@ default String prefix() { /** * default implementation to get the instrumentation key from the config * @return Instrumentation Key + * @see Connection + * strings + * @deprecated use {@link #connectionString()} instead */ + @Deprecated default String instrumentationKey() { return getSecret(this, "instrumentationKey").get(); } + /** + * default implementation to get the connection string from the config + * @return Connection String + */ + @Nullable + default String connectionString() { + return getSecret(this, "connectionString").orElseGet(() -> { + String instrumentationKey = instrumentationKey(); + if (instrumentationKey == null) { + return null; + } + return "InstrumentationKey=" + instrumentationKey; + }); + } + @Override default Validated validate() { return checkAll(this, c -> StepRegistryConfig.validate(c), - check("instrumentationKey", AzureMonitorConfig::instrumentationKey)); + check("connectionString", AzureMonitorConfig::connectionString)); } } diff --git a/implementations/micrometer-registry-azure-monitor/src/main/java/io/micrometer/azuremonitor/AzureMonitorMeterRegistry.java b/implementations/micrometer-registry-azure-monitor/src/main/java/io/micrometer/azuremonitor/AzureMonitorMeterRegistry.java index f3a808a06b..9c04a552a2 100644 --- a/implementations/micrometer-registry-azure-monitor/src/main/java/io/micrometer/azuremonitor/AzureMonitorMeterRegistry.java +++ b/implementations/micrometer-registry-azure-monitor/src/main/java/io/micrometer/azuremonitor/AzureMonitorMeterRegistry.java @@ -63,9 +63,9 @@ private AzureMonitorMeterRegistry(AzureMonitorConfig config, Clock clock, super(config, clock); config().namingConvention(new AzureMonitorNamingConvention()); - if (StringUtils.isEmpty(telemetryConfiguration.getInstrumentationKey())) { - checkRequired("instrumentationKey", AzureMonitorConfig::instrumentationKey).apply(config).orThrow(); - telemetryConfiguration.setInstrumentationKey(config.instrumentationKey()); + if (StringUtils.isEmpty(telemetryConfiguration.getConnectionString())) { + checkRequired("connectionString", AzureMonitorConfig::connectionString).apply(config).orThrow(); + telemetryConfiguration.setConnectionString(config.connectionString()); } client = new TelemetryClient(telemetryConfiguration); diff --git a/implementations/micrometer-registry-azure-monitor/src/test/java/io/micrometer/azuremonitor/AzureMonitorConfigTest.java b/implementations/micrometer-registry-azure-monitor/src/test/java/io/micrometer/azuremonitor/AzureMonitorConfigTest.java index 1e7aa2a44b..06c5d167b8 100644 --- a/implementations/micrometer-registry-azure-monitor/src/test/java/io/micrometer/azuremonitor/AzureMonitorConfigTest.java +++ b/implementations/micrometer-registry-azure-monitor/src/test/java/io/micrometer/azuremonitor/AzureMonitorConfigTest.java @@ -30,7 +30,7 @@ class AzureMonitorConfigTest { @Test void valid() { - props.put("azuremonitor.instrumentationKey", "secret"); + props.put("azuremonitor.connectionString", "secret"); assertThat(config.validate().isValid()).isTrue(); } diff --git a/implementations/micrometer-registry-azure-monitor/src/test/java/io/micrometer/azuremonitor/AzureMonitorMeterRegistryCompatibilityKit.java b/implementations/micrometer-registry-azure-monitor/src/test/java/io/micrometer/azuremonitor/AzureMonitorMeterRegistryCompatibilityKit.java index 7d79ebaab6..8f698463df 100644 --- a/implementations/micrometer-registry-azure-monitor/src/test/java/io/micrometer/azuremonitor/AzureMonitorMeterRegistryCompatibilityKit.java +++ b/implementations/micrometer-registry-azure-monitor/src/test/java/io/micrometer/azuremonitor/AzureMonitorMeterRegistryCompatibilityKit.java @@ -25,8 +25,8 @@ class AzureMonitorMeterRegistryCompatibilityKit extends MeterRegistryCompatibili private final AzureMonitorConfig config = new AzureMonitorConfig() { @Override - public String instrumentationKey() { - return "fakeKey"; + public String connectionString() { + return "InstrumentationKey=fakeKey"; } @Override diff --git a/implementations/micrometer-registry-azure-monitor/src/test/java/io/micrometer/azuremonitor/AzureMonitorMeterRegistryTest.java b/implementations/micrometer-registry-azure-monitor/src/test/java/io/micrometer/azuremonitor/AzureMonitorMeterRegistryTest.java index 31679238d1..43c1a2ad83 100644 --- a/implementations/micrometer-registry-azure-monitor/src/test/java/io/micrometer/azuremonitor/AzureMonitorMeterRegistryTest.java +++ b/implementations/micrometer-registry-azure-monitor/src/test/java/io/micrometer/azuremonitor/AzureMonitorMeterRegistryTest.java @@ -43,9 +43,22 @@ public String get(String key) { return null; } + @Override + public String connectionString() { + return "InstrumentationKey=myInstrumentationKey1"; + } + }; + + private final AzureMonitorConfig configWithInstrumentationKey = new AzureMonitorConfig() { + @Override + public String get(String key) { + return null; + } + + @SuppressWarnings("deprecation") @Override public String instrumentationKey() { - return "myInstrumentationKey"; + return "myInstrumentationKey2"; } }; @@ -54,11 +67,21 @@ public String instrumentationKey() { private final AzureMonitorMeterRegistry registry = new AzureMonitorMeterRegistry(config, clock); @Test - void useTelemetryConfigInstrumentationKeyWhenSet() { + void useTelemetryConfigConnectionStringWhenSet() { TelemetryConfiguration telemetryConfiguration = TelemetryConfiguration.createDefault(); - telemetryConfiguration.setInstrumentationKey("fake"); + telemetryConfiguration.setConnectionString("InstrumentationKey=myInstrumentationKey1"); AzureMonitorMeterRegistry.builder(config).telemetryConfiguration(telemetryConfiguration).build(); - assertThat(telemetryConfiguration.getInstrumentationKey()).isEqualTo("fake"); + assertThat(telemetryConfiguration.getConnectionString()).isEqualTo("InstrumentationKey=myInstrumentationKey1"); + } + + @Test + void useTelemetryConfigInstrumentationKeyWhenSet() { + TelemetryConfiguration telemetryConfiguration = TelemetryConfiguration.createDefault(); + telemetryConfiguration.setInstrumentationKey("myInstrumentationKey2"); + AzureMonitorMeterRegistry.builder(configWithInstrumentationKey) + .telemetryConfiguration(telemetryConfiguration) + .build(); + assertThat(telemetryConfiguration.getInstrumentationKey()).isEqualTo("myInstrumentationKey2"); } @Test diff --git a/samples/micrometer-samples-boot2/src/main/java/io/micrometer/boot2/samples/AzureMonitorSample.java b/samples/micrometer-samples-boot2/src/main/java/io/micrometer/boot2/samples/AzureMonitorSample.java index 7e281235fb..ac9e926b42 100644 --- a/samples/micrometer-samples-boot2/src/main/java/io/micrometer/boot2/samples/AzureMonitorSample.java +++ b/samples/micrometer-samples-boot2/src/main/java/io/micrometer/boot2/samples/AzureMonitorSample.java @@ -33,7 +33,7 @@ public static void main(String[] args) { @Bean AzureMonitorMeterRegistry azureMonitorMeterRegistry(Environment environment) { - // will need an application property `azure.instrumentationKey` to be set + // will need an application property `azure.connectionString` to be set return AzureMonitorMeterRegistry.builder(environment::getProperty).build(); } diff --git a/samples/micrometer-samples-core/src/main/java/io/micrometer/core/samples/utils/SampleRegistries.java b/samples/micrometer-samples-core/src/main/java/io/micrometer/core/samples/utils/SampleRegistries.java index 74351c338c..b00bf638e2 100644 --- a/samples/micrometer-samples-core/src/main/java/io/micrometer/core/samples/utils/SampleRegistries.java +++ b/samples/micrometer-samples-core/src/main/java/io/micrometer/core/samples/utils/SampleRegistries.java @@ -410,8 +410,27 @@ public String uri() { public static AzureMonitorMeterRegistry azure(String apiKey) { return new AzureMonitorMeterRegistry(new AzureMonitorConfig() { @Override - public String instrumentationKey() { - return apiKey; + public String connectionString() { + return String.format("InstrumentationKey=%s", apiKey); + } + + @Override + public String get(String key) { + return null; + } + + @Override + public Duration step() { + return Duration.ofSeconds(10); + } + }, Clock.SYSTEM); + } + + public static AzureMonitorMeterRegistry azureWithConnectionString(String connectionString) { + return new AzureMonitorMeterRegistry(new AzureMonitorConfig() { + @Override + public String connectionString() { + return connectionString; } @Override From 55f61bda320443f40ecbea90076752d0731a264f Mon Sep 17 00:00:00 2001 From: Tommy Ludwig <8924140+shakuzen@users.noreply.github.com> Date: Tue, 28 Mar 2023 18:24:43 +0900 Subject: [PATCH 11/21] Polish connectionString support See gh-3715 --- .../azuremonitor/AzureMonitorConfig.java | 14 ++++++----- .../azuremonitor/AzureMonitorConfigTest.java | 24 ++++++++++++++++++- .../AzureMonitorMeterRegistryTest.java | 15 ++++++------ 3 files changed, 39 insertions(+), 14 deletions(-) diff --git a/implementations/micrometer-registry-azure-monitor/src/main/java/io/micrometer/azuremonitor/AzureMonitorConfig.java b/implementations/micrometer-registry-azure-monitor/src/main/java/io/micrometer/azuremonitor/AzureMonitorConfig.java index 428e54cb2a..1335abd432 100644 --- a/implementations/micrometer-registry-azure-monitor/src/main/java/io/micrometer/azuremonitor/AzureMonitorConfig.java +++ b/implementations/micrometer-registry-azure-monitor/src/main/java/io/micrometer/azuremonitor/AzureMonitorConfig.java @@ -37,21 +37,23 @@ default String prefix() { } /** - * default implementation to get the instrumentation key from the config + * Instrumentation key to use when sending metrics. * @return Instrumentation Key - * @see Connection - * strings - * @deprecated use {@link #connectionString()} instead + * @deprecated use {@link #connectionString()} instead. This method is only called as + * a fallback in the default implementation if a connectionString is not configured. */ + @Nullable @Deprecated default String instrumentationKey() { return getSecret(this, "instrumentationKey").get(); } /** - * default implementation to get the connection string from the config + * Connection string to use when configuring sending metrics. * @return Connection String + * @see Connection + * strings */ @Nullable default String connectionString() { diff --git a/implementations/micrometer-registry-azure-monitor/src/test/java/io/micrometer/azuremonitor/AzureMonitorConfigTest.java b/implementations/micrometer-registry-azure-monitor/src/test/java/io/micrometer/azuremonitor/AzureMonitorConfigTest.java index 06c5d167b8..e5dbfbd311 100644 --- a/implementations/micrometer-registry-azure-monitor/src/test/java/io/micrometer/azuremonitor/AzureMonitorConfigTest.java +++ b/implementations/micrometer-registry-azure-monitor/src/test/java/io/micrometer/azuremonitor/AzureMonitorConfigTest.java @@ -29,10 +29,32 @@ class AzureMonitorConfigTest { private final AzureMonitorConfig config = props::get; @Test - void valid() { + void validWithInstrumentationKey() { + props.put("azuremonitor.instrumentationKey", "key"); + + assertThat(config.validate().isValid()).isTrue(); + } + + @Test + void validWithConnectionString() { props.put("azuremonitor.connectionString", "secret"); assertThat(config.validate().isValid()).isTrue(); } + @Test + void connectionStringUsesInstrumentationKeyIfUnset() { + props.put("azuremonitor.instrumentationKey", "key"); + + assertThat(config.connectionString()).isEqualTo("InstrumentationKey=key"); + } + + @Test + void connectionStringIgnoresInstrumentationKeyIfSet() { + props.put("azuremonitor.instrumentationKey", "key"); + props.put("azuremonitor.connectionString", "InstrumentationKey=another"); + + assertThat(config.connectionString()).isEqualTo("InstrumentationKey=another"); + } + } diff --git a/implementations/micrometer-registry-azure-monitor/src/test/java/io/micrometer/azuremonitor/AzureMonitorMeterRegistryTest.java b/implementations/micrometer-registry-azure-monitor/src/test/java/io/micrometer/azuremonitor/AzureMonitorMeterRegistryTest.java index 43c1a2ad83..3d42de4e10 100644 --- a/implementations/micrometer-registry-azure-monitor/src/test/java/io/micrometer/azuremonitor/AzureMonitorMeterRegistryTest.java +++ b/implementations/micrometer-registry-azure-monitor/src/test/java/io/micrometer/azuremonitor/AzureMonitorMeterRegistryTest.java @@ -45,7 +45,7 @@ public String get(String key) { @Override public String connectionString() { - return "InstrumentationKey=myInstrumentationKey1"; + return "InstrumentationKey=connectionStringKey"; } }; @@ -58,7 +58,7 @@ public String get(String key) { @SuppressWarnings("deprecation") @Override public String instrumentationKey() { - return "myInstrumentationKey2"; + return "myInstrumentationKey"; } }; @@ -69,19 +69,20 @@ public String instrumentationKey() { @Test void useTelemetryConfigConnectionStringWhenSet() { TelemetryConfiguration telemetryConfiguration = TelemetryConfiguration.createDefault(); - telemetryConfiguration.setConnectionString("InstrumentationKey=myInstrumentationKey1"); + telemetryConfiguration.setConnectionString("InstrumentationKey=direct"); AzureMonitorMeterRegistry.builder(config).telemetryConfiguration(telemetryConfiguration).build(); - assertThat(telemetryConfiguration.getConnectionString()).isEqualTo("InstrumentationKey=myInstrumentationKey1"); + assertThat(telemetryConfiguration.getConnectionString()).isEqualTo("InstrumentationKey=direct"); + assertThat(telemetryConfiguration.getInstrumentationKey()).isEqualTo("direct"); } @Test - void useTelemetryConfigInstrumentationKeyWhenSet() { + void configInstrumentationKeyStillSetsTelemetryConfigInstrumentationKey() { TelemetryConfiguration telemetryConfiguration = TelemetryConfiguration.createDefault(); - telemetryConfiguration.setInstrumentationKey("myInstrumentationKey2"); AzureMonitorMeterRegistry.builder(configWithInstrumentationKey) .telemetryConfiguration(telemetryConfiguration) .build(); - assertThat(telemetryConfiguration.getInstrumentationKey()).isEqualTo("myInstrumentationKey2"); + assertThat(telemetryConfiguration.getConnectionString()).isEqualTo("InstrumentationKey=myInstrumentationKey"); + assertThat(telemetryConfiguration.getInstrumentationKey()).isEqualTo("myInstrumentationKey"); } @Test From d611c81fddb80a0d710981c6c73dce2f4797d711 Mon Sep 17 00:00:00 2001 From: Johnny Lim Date: Thu, 30 Mar 2023 15:51:34 +0900 Subject: [PATCH 12/21] Fully sync Spring Boot classpath test support (#3725) --- .../testsupport/classpath/ClassPathExclusions.java | 4 ++-- .../core/testsupport/classpath/ClassPathOverrides.java | 4 ++-- .../core/testsupport/classpath/ForkedClassPath.java | 10 +++++----- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/micrometer-core/src/test/java/io/micrometer/core/testsupport/classpath/ClassPathExclusions.java b/micrometer-core/src/test/java/io/micrometer/core/testsupport/classpath/ClassPathExclusions.java index e9b0f627d8..f6292ea5b1 100644 --- a/micrometer-core/src/test/java/io/micrometer/core/testsupport/classpath/ClassPathExclusions.java +++ b/micrometer-core/src/test/java/io/micrometer/core/testsupport/classpath/ClassPathExclusions.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2019 the original author or authors. + * Copyright 2012-2022 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. @@ -27,7 +27,7 @@ * @author Andy Wilkinson */ @Retention(RetentionPolicy.RUNTIME) -@Target(ElementType.TYPE) +@Target({ ElementType.TYPE, ElementType.METHOD }) @Documented @ExtendWith(ModifiedClassPathExtension.class) public @interface ClassPathExclusions { diff --git a/micrometer-core/src/test/java/io/micrometer/core/testsupport/classpath/ClassPathOverrides.java b/micrometer-core/src/test/java/io/micrometer/core/testsupport/classpath/ClassPathOverrides.java index eaec561d57..941566569e 100644 --- a/micrometer-core/src/test/java/io/micrometer/core/testsupport/classpath/ClassPathOverrides.java +++ b/micrometer-core/src/test/java/io/micrometer/core/testsupport/classpath/ClassPathOverrides.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2019 the original author or authors. + * Copyright 2012-2022 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. @@ -26,7 +26,7 @@ * @author Andy Wilkinson */ @Retention(RetentionPolicy.RUNTIME) -@Target(ElementType.TYPE) +@Target({ ElementType.TYPE, ElementType.METHOD }) @Documented @ExtendWith(ModifiedClassPathExtension.class) public @interface ClassPathOverrides { diff --git a/micrometer-core/src/test/java/io/micrometer/core/testsupport/classpath/ForkedClassPath.java b/micrometer-core/src/test/java/io/micrometer/core/testsupport/classpath/ForkedClassPath.java index 70f591d395..5a401a682e 100644 --- a/micrometer-core/src/test/java/io/micrometer/core/testsupport/classpath/ForkedClassPath.java +++ b/micrometer-core/src/test/java/io/micrometer/core/testsupport/classpath/ForkedClassPath.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2020 the original author or authors. + * Copyright 2012-2022 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. @@ -21,14 +21,14 @@ import java.lang.annotation.*; /** - * Annotation used to fork the classpath. This can be helpful where neither - * {@link ClassPathExclusions} or {@link ClassPathOverrides} are needed, but just a copy - * of the classpath. + * Annotation used to fork the classpath. This can be helpful when using annotations on + * parameterized tests, or where neither {@link ClassPathExclusions} or + * {@link ClassPathOverrides} are needed, but just a copy of the classpath. * * @author Christoph Dreis */ @Retention(RetentionPolicy.RUNTIME) -@Target(ElementType.TYPE) +@Target({ ElementType.TYPE, ElementType.METHOD }) @Documented @ExtendWith(ModifiedClassPathExtension.class) public @interface ForkedClassPath { From 4e64b29bc00b020ef51cab9d59ccc8df9dcf5c6b Mon Sep 17 00:00:00 2001 From: Tommy Ludwig <8924140+shakuzen@users.noreply.github.com> Date: Sat, 1 Apr 2023 03:08:55 +0900 Subject: [PATCH 13/21] Do not rollover again after closingRollover (#3730) If a registry is closed near the end of a step interval, the publish on closing may take enough time that some meters are read after the step boundary is crossed. Without this fix, that results in those meters rolling over again, which we do not want in this case. This renames manualRollover to closingRollover to signal this difference in behavior. Resolves gh-3720 --- .../core/instrument/step/StepCounter.java | 4 +- .../step/StepDistributionSummary.java | 4 +- .../instrument/step/StepFunctionCounter.java | 4 +- .../instrument/step/StepFunctionTimer.java | 4 +- .../core/instrument/step/StepMeter.java | 5 +- .../instrument/step/StepMeterRegistry.java | 2 +- .../core/instrument/step/StepTimer.java | 4 +- .../core/instrument/step/StepTuple2.java | 4 +- .../core/instrument/step/StepValue.java | 4 +- .../core/instrument/step/StepCounterTest.java | 7 ++- .../step/StepDistributionSummaryTest.java | 10 +++- .../step/StepFunctionCounterTest.java | 8 ++- .../step/StepFunctionTimerTest.java | 9 ++- .../step/StepMeterRegistryTest.java | 59 +++++++++++++++++++ .../core/instrument/step/StepTimerTest.java | 9 ++- .../core/instrument/step/StepValueTest.java | 35 ++++++++++- 16 files changed, 146 insertions(+), 26 deletions(-) diff --git a/micrometer-core/src/main/java/io/micrometer/core/instrument/step/StepCounter.java b/micrometer-core/src/main/java/io/micrometer/core/instrument/step/StepCounter.java index 6b96c7bbf4..eebe8f1fac 100644 --- a/micrometer-core/src/main/java/io/micrometer/core/instrument/step/StepCounter.java +++ b/micrometer-core/src/main/java/io/micrometer/core/instrument/step/StepCounter.java @@ -46,8 +46,8 @@ public double count() { } @Override - public void _manualRollover() { - value.manualRollover(); + public void _closingRollover() { + value.closingRollover(); } } diff --git a/micrometer-core/src/main/java/io/micrometer/core/instrument/step/StepDistributionSummary.java b/micrometer-core/src/main/java/io/micrometer/core/instrument/step/StepDistributionSummary.java index e9f86827d7..c679da9862 100644 --- a/micrometer-core/src/main/java/io/micrometer/core/instrument/step/StepDistributionSummary.java +++ b/micrometer-core/src/main/java/io/micrometer/core/instrument/step/StepDistributionSummary.java @@ -87,8 +87,8 @@ public Iterable measure() { } @Override - public void _manualRollover() { - countTotal.manualRollover(); + public void _closingRollover() { + countTotal.closingRollover(); } } diff --git a/micrometer-core/src/main/java/io/micrometer/core/instrument/step/StepFunctionCounter.java b/micrometer-core/src/main/java/io/micrometer/core/instrument/step/StepFunctionCounter.java index ab43ee8eb2..b0f612a7c4 100644 --- a/micrometer-core/src/main/java/io/micrometer/core/instrument/step/StepFunctionCounter.java +++ b/micrometer-core/src/main/java/io/micrometer/core/instrument/step/StepFunctionCounter.java @@ -51,9 +51,9 @@ public double count() { } @Override - public void _manualRollover() { + public void _closingRollover() { count(); // add any difference from last count - count.manualRollover(); + count.closingRollover(); } } diff --git a/micrometer-core/src/main/java/io/micrometer/core/instrument/step/StepFunctionTimer.java b/micrometer-core/src/main/java/io/micrometer/core/instrument/step/StepFunctionTimer.java index 12d8259cec..f19c37cb1a 100644 --- a/micrometer-core/src/main/java/io/micrometer/core/instrument/step/StepFunctionTimer.java +++ b/micrometer-core/src/main/java/io/micrometer/core/instrument/step/StepFunctionTimer.java @@ -119,9 +119,9 @@ public Type type() { } @Override - public void _manualRollover() { + public void _closingRollover() { accumulateCountAndTotal(); - countTotal.manualRollover(); + countTotal.closingRollover(); } } diff --git a/micrometer-core/src/main/java/io/micrometer/core/instrument/step/StepMeter.java b/micrometer-core/src/main/java/io/micrometer/core/instrument/step/StepMeter.java index c8d23df8fc..0dac66c3b2 100644 --- a/micrometer-core/src/main/java/io/micrometer/core/instrument/step/StepMeter.java +++ b/micrometer-core/src/main/java/io/micrometer/core/instrument/step/StepMeter.java @@ -23,8 +23,9 @@ interface StepMeter { /** * This is an internal method not meant for general use. *

- * Force a rollover of the values returned by a step meter. + * Force a rollover of the values returned by a step meter and never rollover again + * after. */ - void _manualRollover(); + void _closingRollover(); } diff --git a/micrometer-core/src/main/java/io/micrometer/core/instrument/step/StepMeterRegistry.java b/micrometer-core/src/main/java/io/micrometer/core/instrument/step/StepMeterRegistry.java index 7d8ba84fb9..66435c9d0f 100644 --- a/micrometer-core/src/main/java/io/micrometer/core/instrument/step/StepMeterRegistry.java +++ b/micrometer-core/src/main/java/io/micrometer/core/instrument/step/StepMeterRegistry.java @@ -111,7 +111,7 @@ public void close() { getMeters().stream() .filter(StepMeter.class::isInstance) .map(StepMeter.class::cast) - .forEach(StepMeter::_manualRollover); + .forEach(StepMeter::_closingRollover); } super.close(); } diff --git a/micrometer-core/src/main/java/io/micrometer/core/instrument/step/StepTimer.java b/micrometer-core/src/main/java/io/micrometer/core/instrument/step/StepTimer.java index bbe06ddf41..11dffdd8dc 100644 --- a/micrometer-core/src/main/java/io/micrometer/core/instrument/step/StepTimer.java +++ b/micrometer-core/src/main/java/io/micrometer/core/instrument/step/StepTimer.java @@ -80,8 +80,8 @@ public double max(final TimeUnit unit) { } @Override - public void _manualRollover() { - countTotal.manualRollover(); + public void _closingRollover() { + countTotal.closingRollover(); } } diff --git a/micrometer-core/src/main/java/io/micrometer/core/instrument/step/StepTuple2.java b/micrometer-core/src/main/java/io/micrometer/core/instrument/step/StepTuple2.java index fd3d4935af..b16415bb23 100644 --- a/micrometer-core/src/main/java/io/micrometer/core/instrument/step/StepTuple2.java +++ b/micrometer-core/src/main/java/io/micrometer/core/instrument/step/StepTuple2.java @@ -77,7 +77,9 @@ private void rollCount(long now) { * Intended for internal use. Rolls the values regardless of the clock or current * time. */ - void manualRollover() { + void closingRollover() { + // ensure rollover does not happen again + lastInitPos.set(Long.MAX_VALUE); t1Previous = t1Supplier.get(); t2Previous = t2Supplier.get(); } diff --git a/micrometer-core/src/main/java/io/micrometer/core/instrument/step/StepValue.java b/micrometer-core/src/main/java/io/micrometer/core/instrument/step/StepValue.java index 0fa287b01a..d848dccd8b 100644 --- a/micrometer-core/src/main/java/io/micrometer/core/instrument/step/StepValue.java +++ b/micrometer-core/src/main/java/io/micrometer/core/instrument/step/StepValue.java @@ -76,7 +76,9 @@ public V poll() { /** * internal use only; intentionally left package-private */ - void manualRollover() { + void closingRollover() { + // make sure value does not rollover again if passing a step boundary + lastInitPos.set(Long.MAX_VALUE); previous = valueSupplier().get(); } diff --git a/micrometer-core/src/test/java/io/micrometer/core/instrument/step/StepCounterTest.java b/micrometer-core/src/test/java/io/micrometer/core/instrument/step/StepCounterTest.java index 20fcb78ae3..f170429891 100644 --- a/micrometer-core/src/test/java/io/micrometer/core/instrument/step/StepCounterTest.java +++ b/micrometer-core/src/test/java/io/micrometer/core/instrument/step/StepCounterTest.java @@ -69,12 +69,15 @@ void count() { } @Test - void manualRolloverPartialStep() { + void closingRolloverPartialStep() { StepCounter counter = (StepCounter) registry.counter("my.counter"); counter.increment(2.5); assertThat(counter.count()).isZero(); - counter._manualRollover(); + counter._closingRollover(); + assertThat(counter.count()).isEqualTo(2.5); + + clock.add(config.step()); assertThat(counter.count()).isEqualTo(2.5); } diff --git a/micrometer-core/src/test/java/io/micrometer/core/instrument/step/StepDistributionSummaryTest.java b/micrometer-core/src/test/java/io/micrometer/core/instrument/step/StepDistributionSummaryTest.java index 67d43105b7..9964494241 100644 --- a/micrometer-core/src/test/java/io/micrometer/core/instrument/step/StepDistributionSummaryTest.java +++ b/micrometer-core/src/test/java/io/micrometer/core/instrument/step/StepDistributionSummaryTest.java @@ -50,7 +50,7 @@ void meanShouldWorkIfTotalNotCalled() { } @Test - void manualRolloverPartialStep() { + void closingRolloverPartialStep() { Duration stepDuration = Duration.ofMillis(10); StepDistributionSummary summary = new StepDistributionSummary(mock(Meter.Id.class), clock, DistributionStatisticConfig.builder().expiry(stepDuration).bufferLength(2).build(), 1.0, @@ -61,7 +61,13 @@ void manualRolloverPartialStep() { assertThat(summary.count()).isZero(); - summary._manualRollover(); + summary._closingRollover(); + + assertThat(summary.count()).isEqualTo(2); + assertThat(summary.totalAmount()).isEqualTo(300); + assertThat(summary.mean()).isEqualTo(150); + + clock.add(stepDuration); assertThat(summary.count()).isEqualTo(2); assertThat(summary.totalAmount()).isEqualTo(300); diff --git a/micrometer-core/src/test/java/io/micrometer/core/instrument/step/StepFunctionCounterTest.java b/micrometer-core/src/test/java/io/micrometer/core/instrument/step/StepFunctionCounterTest.java index 3042c5d9d1..61c90c835b 100644 --- a/micrometer-core/src/test/java/io/micrometer/core/instrument/step/StepFunctionCounterTest.java +++ b/micrometer-core/src/test/java/io/micrometer/core/instrument/step/StepFunctionCounterTest.java @@ -65,7 +65,7 @@ void count() { } @Test - void manualRolloverPartialStep() { + void closingRolloverPartialStep() { AtomicInteger n = new AtomicInteger(3); @SuppressWarnings({ "rawtypes", "unchecked" }) StepFunctionCounter counter = (StepFunctionCounter) registry.more() @@ -73,7 +73,11 @@ void manualRolloverPartialStep() { assertThat(counter.count()).isZero(); - counter._manualRollover(); + counter._closingRollover(); + + assertThat(counter.count()).isEqualTo(3); + + clock.add(config.step()); assertThat(counter.count()).isEqualTo(3); } diff --git a/micrometer-core/src/test/java/io/micrometer/core/instrument/step/StepFunctionTimerTest.java b/micrometer-core/src/test/java/io/micrometer/core/instrument/step/StepFunctionTimerTest.java index 6c9d6471c0..5a5b11c244 100644 --- a/micrometer-core/src/test/java/io/micrometer/core/instrument/step/StepFunctionTimerTest.java +++ b/micrometer-core/src/test/java/io/micrometer/core/instrument/step/StepFunctionTimerTest.java @@ -77,7 +77,7 @@ void meanShouldWorkIfTotalNotCalled() { } @Test - void manualRolloverPartialStep() { + void closingRolloverPartialStep() { Queue counts = new ArrayDeque<>(); counts.add(2L); counts.add(5L); @@ -95,7 +95,12 @@ void manualRolloverPartialStep() { assertThat(timer.count()).isZero(); assertThat(timer.totalTime(TimeUnit.SECONDS)).isZero(); - timer._manualRollover(); + timer._closingRollover(); + + assertThat(timer.count()).isEqualTo(2); + assertThat(timer.totalTime(TimeUnit.SECONDS)).isEqualTo(150); + + clock.add(stepDuration); assertThat(timer.count()).isEqualTo(2); assertThat(timer.totalTime(TimeUnit.SECONDS)).isEqualTo(150); diff --git a/micrometer-core/src/test/java/io/micrometer/core/instrument/step/StepMeterRegistryTest.java b/micrometer-core/src/test/java/io/micrometer/core/instrument/step/StepMeterRegistryTest.java index c2c09ab755..4aeb7b4ec2 100644 --- a/micrometer-core/src/test/java/io/micrometer/core/instrument/step/StepMeterRegistryTest.java +++ b/micrometer-core/src/test/java/io/micrometer/core/instrument/step/StepMeterRegistryTest.java @@ -15,6 +15,7 @@ */ package io.micrometer.core.instrument.step; +import io.micrometer.common.lang.Nullable; import io.micrometer.core.Issue; import io.micrometer.core.instrument.*; import org.junit.jupiter.api.Test; @@ -293,6 +294,54 @@ void finalPushHasPartialStep() { assertThat(registry.publishedFunctionTimerTotals.pop()).isEqualTo(24); } + @Issue("#3720") + @Test + void publishOnCloseCrossesStepBoundary() { + Counter counter = Counter.builder("counter").register(registry); + counter.increment(); + Timer timer = Timer.builder("timer").register(registry); + timer.record(5, MILLISECONDS); + DistributionSummary summary = DistributionSummary.builder("summary").register(registry); + summary.record(7); + FunctionCounter functionCounter = FunctionCounter.builder("counter.function", this, obj -> 15) + .register(registry); + FunctionTimer functionTimer = FunctionTimer.builder("timer.function", this, obj -> 3, obj -> 53, MILLISECONDS) + .register(registry); + + // before rollover + assertThat(counter.count()).isZero(); + assertThat(timer.count()).isZero(); + assertThat(timer.totalTime(MILLISECONDS)).isZero(); + assertThat(summary.count()).isZero(); + assertThat(summary.totalAmount()).isZero(); + assertThat(functionCounter.count()).isZero(); + assertThat(functionTimer.count()).isZero(); + assertThat(functionTimer.totalTime(MILLISECONDS)).isZero(); + + // before publishing, simulate a step boundary being crossed after forced rollover + // on close and before/during publishing + registry.setPrePublishAction(() -> clock.add(config.step())); + // force rollover and publish on close + registry.close(); + + assertThat(registry.publishedCounterCounts).hasSize(1); + assertThat(registry.publishedCounterCounts.pop()).isOne(); + assertThat(registry.publishedTimerCounts).hasSize(1); + assertThat(registry.publishedTimerCounts.pop()).isOne(); + assertThat(registry.publishedTimerSumMilliseconds).hasSize(1); + assertThat(registry.publishedTimerSumMilliseconds.pop()).isEqualTo(5.0); + assertThat(registry.publishedSummaryCounts).hasSize(1); + assertThat(registry.publishedSummaryCounts.pop()).isOne(); + assertThat(registry.publishedSummaryTotals).hasSize(1); + assertThat(registry.publishedSummaryTotals.pop()).isEqualTo(7); + assertThat(registry.publishedFunctionCounterCounts).hasSize(1); + assertThat(registry.publishedFunctionCounterCounts.pop()).isEqualTo(15); + assertThat(registry.publishedFunctionTimerCounts).hasSize(1); + assertThat(registry.publishedFunctionTimerCounts.pop()).isEqualTo(3); + assertThat(registry.publishedFunctionTimerTotals).hasSize(1); + assertThat(registry.publishedFunctionTimerTotals.pop()).isEqualTo(53); + } + private class MyStepMeterRegistry extends StepMeterRegistry { Deque publishedCounterCounts = new ArrayDeque<>(); @@ -311,12 +360,22 @@ private class MyStepMeterRegistry extends StepMeterRegistry { Deque publishedFunctionTimerTotals = new ArrayDeque<>(); + @Nullable + Runnable prePublishAction; + MyStepMeterRegistry() { super(StepMeterRegistryTest.this.config, StepMeterRegistryTest.this.clock); } + void setPrePublishAction(Runnable prePublishAction) { + this.prePublishAction = prePublishAction; + } + @Override protected void publish() { + if (prePublishAction != null) { + prePublishAction.run(); + } publishes.incrementAndGet(); getMeters().stream() .map(meter -> meter.match(null, this::publishCounter, this::publishTimer, this::publishSummary, null, diff --git a/micrometer-core/src/test/java/io/micrometer/core/instrument/step/StepTimerTest.java b/micrometer-core/src/test/java/io/micrometer/core/instrument/step/StepTimerTest.java index 95bbcf2d2b..b434f97c25 100644 --- a/micrometer-core/src/test/java/io/micrometer/core/instrument/step/StepTimerTest.java +++ b/micrometer-core/src/test/java/io/micrometer/core/instrument/step/StepTimerTest.java @@ -52,7 +52,7 @@ void meanShouldWorkIfTotalTimeNotCalled() { } @Test - void manualRolloverPartialStep() { + void closingRolloverPartialStep() { Duration stepDuration = Duration.ofMillis(10); StepTimer timer = new StepTimer(mock(Meter.Id.class), clock, DistributionStatisticConfig.builder().expiry(stepDuration).bufferLength(2).build(), @@ -63,7 +63,12 @@ void manualRolloverPartialStep() { assertThat(timer.count()).isZero(); assertThat(timer.totalTime(TimeUnit.MILLISECONDS)).isZero(); - timer._manualRollover(); + timer._closingRollover(); + + assertThat(timer.count()).isEqualTo(2); + assertThat(timer.totalTime(TimeUnit.MILLISECONDS)).isEqualTo(100); + + clock.add(stepDuration); assertThat(timer.count()).isEqualTo(2); assertThat(timer.totalTime(TimeUnit.MILLISECONDS)).isEqualTo(100); diff --git a/micrometer-core/src/test/java/io/micrometer/core/instrument/step/StepValueTest.java b/micrometer-core/src/test/java/io/micrometer/core/instrument/step/StepValueTest.java index 59a54eedc0..1785845eb1 100644 --- a/micrometer-core/src/test/java/io/micrometer/core/instrument/step/StepValueTest.java +++ b/micrometer-core/src/test/java/io/micrometer/core/instrument/step/StepValueTest.java @@ -15,10 +15,12 @@ */ package io.micrometer.core.instrument.step; +import io.micrometer.core.Issue; import io.micrometer.core.instrument.MockClock; import org.junit.jupiter.api.Test; import java.time.Duration; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicLong; import java.util.function.Supplier; @@ -69,8 +71,39 @@ public Long noValue() { aLong.set(27); assertThat(stepValue.poll()).isEqualTo(24L); - stepValue.manualRollover(); + stepValue.closingRollover(); assertThat(stepValue.poll()).isEqualTo(27L); } + @Test + @Issue("#3720") + void testManualRolloverDropsDataOnStepCompletion() { + final MockClock clock = new MockClock(); + final long stepTime = 60; + final AtomicLong aLong = new AtomicLong(10); + final StepValue stepValue = new StepValue(clock, stepTime) { + @Override + public Supplier valueSupplier() { + return () -> aLong.getAndSet(0); + } + + @Override + public Long noValue() { + return 0L; + } + }; + clock.add(Duration.ofMillis(1)); + assertThat(stepValue.poll()).isZero(); + clock.add(Duration.ofMillis(59)); + assertThat(stepValue.poll()).isEqualTo(10); + clock.add(Duration.ofMillis(stepTime - 1)); + aLong.set(5); + stepValue.closingRollover(); + assertThat(stepValue.poll()).isEqualTo(5); + clock.add(Duration.ofMillis(1)); + assertThat(stepValue.poll()).isEqualTo(5L); + clock.add(stepTime, TimeUnit.MILLISECONDS); + assertThat(stepValue.poll()).isEqualTo(5L); + } + } From fe07e980e96e80bb667f4f588f81aa7483be4c53 Mon Sep 17 00:00:00 2001 From: Johnny Lim Date: Sat, 1 Apr 2023 11:28:26 +0900 Subject: [PATCH 14/21] Polish "Avoid concurrent publish calls from publishSafely" (#3733) See gh-3714 --- .../instrument/push/PushMeterRegistry.java | 6 ++++-- .../push/PushMeterRegistryTest.java | 20 +++++++++---------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/micrometer-core/src/main/java/io/micrometer/core/instrument/push/PushMeterRegistry.java b/micrometer-core/src/main/java/io/micrometer/core/instrument/push/PushMeterRegistry.java index 7fe83df242..2c9daf729a 100644 --- a/micrometer-core/src/main/java/io/micrometer/core/instrument/push/PushMeterRegistry.java +++ b/micrometer-core/src/main/java/io/micrometer/core/instrument/push/PushMeterRegistry.java @@ -67,12 +67,14 @@ void publishSafely() { } } else { - logger.warn("Publishing is already in progress. Skipping duplicate call to publishSafely()."); + logger.warn("Publishing is already in progress. Skipping duplicate call to publish()."); } } /** - * Returns - true if scheduled publishing of metrics is in progress. + * Returns if scheduled publishing of metrics is in progress. + * @return if scheduled publishing of metrics is in progress + * @since 1.11.0 */ protected boolean isPublishing() { return publishing.get(); diff --git a/micrometer-core/src/test/java/io/micrometer/core/instrument/push/PushMeterRegistryTest.java b/micrometer-core/src/test/java/io/micrometer/core/instrument/push/PushMeterRegistryTest.java index 72ced25cb9..2ffb100ce4 100644 --- a/micrometer-core/src/test/java/io/micrometer/core/instrument/push/PushMeterRegistryTest.java +++ b/micrometer-core/src/test/java/io/micrometer/core/instrument/push/PushMeterRegistryTest.java @@ -33,7 +33,6 @@ import java.util.concurrent.atomic.AtomicInteger; import java.util.function.ToDoubleFunction; import java.util.function.ToLongFunction; -import java.util.stream.Collectors; import static java.util.concurrent.TimeUnit.MILLISECONDS; import static java.util.concurrent.TimeUnit.SECONDS; @@ -123,13 +122,13 @@ void scheduledPublishOverlapWithPublishOnClose() throws InterruptedException { private static class OverlappingStepMeterRegistry extends StepMeterRegistry { - private final AtomicInteger numberOfPublish = new AtomicInteger(); + private final AtomicInteger numberOfPublishes = new AtomicInteger(); private final Map> publishes = new ConcurrentHashMap<>(); private final CyclicBarrier barrier; - public OverlappingStepMeterRegistry(StepRegistryConfig config, Clock clock, CyclicBarrier barrier) { + OverlappingStepMeterRegistry(StepRegistryConfig config, Clock clock, CyclicBarrier barrier) { super(config, clock); this.barrier = barrier; } @@ -147,16 +146,15 @@ protected void publish() { catch (InterruptedException | BrokenBarrierException | TimeoutException e) { throw new RuntimeException(e); } - int publishIndex = numberOfPublish.getAndIncrement(); - for (Counter counter : getMeters().stream() + int publishIndex = numberOfPublishes.getAndIncrement(); + getMeters().stream() .filter(meter -> meter instanceof Counter) .map(meter -> (Counter) meter) - .collect(Collectors.toSet())) { - publishes.merge(publishIndex, new ArrayDeque<>(Arrays.asList(counter.count())), (l1, l2) -> { - l1.addAll(l2); - return l1; - }); - } + .forEach(counter -> publishes.merge(publishIndex, new ArrayDeque<>(Arrays.asList(counter.count())), + (l1, l2) -> { + l1.addAll(l2); + return l1; + })); } @Override From 5335c74c16ab3761539ffceff3cb681bcffb36fa Mon Sep 17 00:00:00 2001 From: Tommy Ludwig <8924140+shakuzen@users.noreply.github.com> Date: Mon, 3 Apr 2023 10:59:14 +0900 Subject: [PATCH 15/21] Remove snapshot repo for context-propagation and update locks We do not need to be on snapshots now, and unfortunately this made it out in a release. Resolves gh-3738 --- build.gradle | 8 -------- micrometer-core/gradle.lockfile | 2 +- micrometer-observation/gradle.lockfile | 2 +- 3 files changed, 2 insertions(+), 10 deletions(-) diff --git a/build.gradle b/build.gradle index e9c9a96803..8f4d4a33ec 100644 --- a/build.gradle +++ b/build.gradle @@ -389,14 +389,6 @@ subprojects { repositories { mavenCentral() - maven { - // TODO remove before releasing - url "https://repo.spring.io/snapshot/" - content { - // for context-propagation snapshots - includeModule 'io.micrometer', 'context-propagation' - } - } } def check = tasks.findByName('check') diff --git a/micrometer-core/gradle.lockfile b/micrometer-core/gradle.lockfile index 8805f8dd9f..8e9a07f9d0 100644 --- a/micrometer-core/gradle.lockfile +++ b/micrometer-core/gradle.lockfile @@ -115,7 +115,7 @@ io.grpc:grpc-protobuf-lite:1.49.2=java11TestCompileClasspath,java11TestImplement io.grpc:grpc-protobuf:1.49.2=java11TestCompileClasspath,java11TestImplementationDependenciesMetadata,java11TestRuntimeClasspath,testCompileClasspath,testImplementationDependenciesMetadata,testRuntimeClasspath io.grpc:grpc-stub:1.49.2=java11TestCompileClasspath,java11TestImplementationDependenciesMetadata,java11TestRuntimeClasspath,testCompileClasspath,testImplementationDependenciesMetadata,testRuntimeClasspath io.grpc:grpc-testing-proto:1.49.2=java11TestCompileClasspath,java11TestImplementationDependenciesMetadata,java11TestRuntimeClasspath,testCompileClasspath,testImplementationDependenciesMetadata,testRuntimeClasspath -io.micrometer:context-propagation:1.0.3-SNAPSHOT=compileClasspath,runtimeClasspath,testCompileClasspath,testRuntimeClasspath +io.micrometer:context-propagation:1.0.2=compileClasspath,runtimeClasspath,testCompileClasspath,testRuntimeClasspath io.netty:netty-bom:4.1.89.Final=apiDependenciesMetadata,compileClasspath,compileOnlyDependenciesMetadata,implementationDependenciesMetadata,java11ApiDependenciesMetadata,java11CompileClasspath,java11ImplementationDependenciesMetadata,java11RuntimeClasspath,java11TestCompileClasspath,java11TestImplementationDependenciesMetadata,java11TestRuntimeClasspath,runtimeClasspath,runtimeOnlyDependenciesMetadata,testCompileClasspath,testImplementationDependenciesMetadata,testRuntimeClasspath,testRuntimeOnlyDependenciesMetadata io.perfmark:perfmark-api:0.25.0=java11TestRuntimeClasspath,testRuntimeClasspath io.projectreactor:reactor-bom:2020.0.29=apiDependenciesMetadata,compileClasspath,compileOnlyDependenciesMetadata,implementationDependenciesMetadata,java11ApiDependenciesMetadata,java11CompileClasspath,java11ImplementationDependenciesMetadata,java11RuntimeClasspath,java11TestCompileClasspath,java11TestImplementationDependenciesMetadata,java11TestRuntimeClasspath,runtimeClasspath,runtimeOnlyDependenciesMetadata,testCompileClasspath,testImplementationDependenciesMetadata,testRuntimeClasspath,testRuntimeOnlyDependenciesMetadata diff --git a/micrometer-observation/gradle.lockfile b/micrometer-observation/gradle.lockfile index 62672f5955..e6c19c31f2 100644 --- a/micrometer-observation/gradle.lockfile +++ b/micrometer-observation/gradle.lockfile @@ -59,7 +59,7 @@ info.picocli:picocli:4.6.2=checkstyle info.picocli:picocli:4.6.3=spotless-1972451482,spotless-1972451484 io.github.detekt.sarif4k:sarif4k:0.2.0=spotless-1972451482,spotless-1972451484 io.github.microutils:kotlin-logging-jvm:2.1.23=spotless-1972451482,spotless-1972451484 -io.micrometer:context-propagation:1.0.3-SNAPSHOT=compileClasspath,runtimeClasspath,testCompileClasspath,testRuntimeClasspath +io.micrometer:context-propagation:1.0.2=compileClasspath,runtimeClasspath,testCompileClasspath,testRuntimeClasspath io.netty:netty-bom:4.1.89.Final=compileClasspath,runtimeClasspath,testCompileClasspath,testRuntimeClasspath io.projectreactor:reactor-bom:2020.0.29=compileClasspath,runtimeClasspath,testCompileClasspath,testRuntimeClasspath io.spring.javaformat:spring-javaformat-checkstyle:0.0.38=checkstyle From f82cdbc8afcb5382c814d14620965ceef206c913 Mon Sep 17 00:00:00 2001 From: Johnny Lim Date: Mon, 3 Apr 2023 11:14:01 +0900 Subject: [PATCH 16/21] Polish "Do not rollover again after closingRollover" (#3737) See gh-3730 --- .../main/java/io/micrometer/core/instrument/step/StepMeter.java | 2 +- .../main/java/io/micrometer/core/instrument/step/StepValue.java | 2 +- .../java/io/micrometer/core/instrument/step/StepValueTest.java | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/micrometer-core/src/main/java/io/micrometer/core/instrument/step/StepMeter.java b/micrometer-core/src/main/java/io/micrometer/core/instrument/step/StepMeter.java index 0dac66c3b2..dafdb604f6 100644 --- a/micrometer-core/src/main/java/io/micrometer/core/instrument/step/StepMeter.java +++ b/micrometer-core/src/main/java/io/micrometer/core/instrument/step/StepMeter.java @@ -23,7 +23,7 @@ interface StepMeter { /** * This is an internal method not meant for general use. *

- * Force a rollover of the values returned by a step meter and never rollover again + * Force a rollover of the values returned by a step meter and never roll over again * after. */ void _closingRollover(); diff --git a/micrometer-core/src/main/java/io/micrometer/core/instrument/step/StepValue.java b/micrometer-core/src/main/java/io/micrometer/core/instrument/step/StepValue.java index d848dccd8b..f48a4a77da 100644 --- a/micrometer-core/src/main/java/io/micrometer/core/instrument/step/StepValue.java +++ b/micrometer-core/src/main/java/io/micrometer/core/instrument/step/StepValue.java @@ -77,7 +77,7 @@ public V poll() { * internal use only; intentionally left package-private */ void closingRollover() { - // make sure value does not rollover again if passing a step boundary + // make sure value does not roll over again if passing a step boundary lastInitPos.set(Long.MAX_VALUE); previous = valueSupplier().get(); } diff --git a/micrometer-core/src/test/java/io/micrometer/core/instrument/step/StepValueTest.java b/micrometer-core/src/test/java/io/micrometer/core/instrument/step/StepValueTest.java index 1785845eb1..39972f111d 100644 --- a/micrometer-core/src/test/java/io/micrometer/core/instrument/step/StepValueTest.java +++ b/micrometer-core/src/test/java/io/micrometer/core/instrument/step/StepValueTest.java @@ -77,7 +77,7 @@ public Long noValue() { @Test @Issue("#3720") - void testManualRolloverDropsDataOnStepCompletion() { + void closingRolloverShouldNotDropDataOnStepCompletion() { final MockClock clock = new MockClock(); final long stepTime = 60; final AtomicLong aLong = new AtomicLong(10); From e78cdadcb33760eb41d2fa9a32c3fc4dc59114d7 Mon Sep 17 00:00:00 2001 From: Johnny Lim Date: Mon, 3 Apr 2023 11:14:34 +0900 Subject: [PATCH 17/21] Fix config prefix in comment in AzureMonitorSample (#3736) --- .../java/io/micrometer/boot2/samples/AzureMonitorSample.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/samples/micrometer-samples-boot2/src/main/java/io/micrometer/boot2/samples/AzureMonitorSample.java b/samples/micrometer-samples-boot2/src/main/java/io/micrometer/boot2/samples/AzureMonitorSample.java index 7e281235fb..97f6af61ec 100644 --- a/samples/micrometer-samples-boot2/src/main/java/io/micrometer/boot2/samples/AzureMonitorSample.java +++ b/samples/micrometer-samples-boot2/src/main/java/io/micrometer/boot2/samples/AzureMonitorSample.java @@ -33,7 +33,7 @@ public static void main(String[] args) { @Bean AzureMonitorMeterRegistry azureMonitorMeterRegistry(Environment environment) { - // will need an application property `azure.instrumentationKey` to be set + // will need an environment variable `azuremonitor.instrumentationKey` to be set return AzureMonitorMeterRegistry.builder(environment::getProperty).build(); } From b05903eb9904f270cdd7e7f146595877a1cabe3d Mon Sep 17 00:00:00 2001 From: Johnny Lim Date: Mon, 3 Apr 2023 11:20:03 +0900 Subject: [PATCH 18/21] Add Javadoc since to AzureMonitorConfig.connectionString() (#3735) See gh-3715 --- .../java/io/micrometer/azuremonitor/AzureMonitorConfig.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/implementations/micrometer-registry-azure-monitor/src/main/java/io/micrometer/azuremonitor/AzureMonitorConfig.java b/implementations/micrometer-registry-azure-monitor/src/main/java/io/micrometer/azuremonitor/AzureMonitorConfig.java index 1335abd432..4d45bc2488 100644 --- a/implementations/micrometer-registry-azure-monitor/src/main/java/io/micrometer/azuremonitor/AzureMonitorConfig.java +++ b/implementations/micrometer-registry-azure-monitor/src/main/java/io/micrometer/azuremonitor/AzureMonitorConfig.java @@ -39,8 +39,9 @@ default String prefix() { /** * Instrumentation key to use when sending metrics. * @return Instrumentation Key - * @deprecated use {@link #connectionString()} instead. This method is only called as - * a fallback in the default implementation if a connectionString is not configured. + * @deprecated since 1.11.0, use {@link #connectionString()} instead. This method is + * only called as a fallback in the default implementation if a connectionString is + * not configured. */ @Nullable @Deprecated @@ -54,6 +55,7 @@ default String instrumentationKey() { * @see Connection * strings + * @since 1.11.0 */ @Nullable default String connectionString() { From 79755b4a35be87b4581adb0f2b0071770a4056ff Mon Sep 17 00:00:00 2001 From: Tommy Ludwig <8924140+shakuzen@users.noreply.github.com> Date: Mon, 3 Apr 2023 12:24:29 +0900 Subject: [PATCH 19/21] Allow resolving context-propagation milestone versions The snapshot repo was removed in the `1.10.x` branch so snapshots would not be resolved. However, since `main` is trying to get the `1.1.+` version range of the context-propagation dependency, it could not resolve it. The error message was very misleading, but this resolves the issue by allowing milestone versions to be resolved. --- build.gradle | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/build.gradle b/build.gradle index b0a4066de3..6c343a9270 100644 --- a/build.gradle +++ b/build.gradle @@ -402,6 +402,14 @@ subprojects { repositories { mavenCentral() + maven { + // TODO remove before releasing GA + url "https://repo.spring.io/milestone/" + content { + // for context-propagation milestone versions + includeModule 'io.micrometer', 'context-propagation' + } + } } def check = tasks.findByName('check') From 341f5f17122e21c0ff8ef481ea3f8c5f4f5b4527 Mon Sep 17 00:00:00 2001 From: James Yuzawa Date: Mon, 3 Apr 2023 00:24:46 -0400 Subject: [PATCH 20/21] Avoid using streams in observations (#3722) There is better memory and CPU performance with plain old iteration. --- .../DefaultMeterObservationHandler.java | 26 ++++---- .../micrometer/observation/Observation.java | 25 +++++--- .../observation/ObservationHandler.java | 59 +++++++++++++------ .../observation/ObservationRegistry.java | 17 ++++-- .../observation/SimpleObservation.java | 26 ++++---- 5 files changed, 96 insertions(+), 57 deletions(-) diff --git a/micrometer-core/src/main/java/io/micrometer/core/instrument/observation/DefaultMeterObservationHandler.java b/micrometer-core/src/main/java/io/micrometer/core/instrument/observation/DefaultMeterObservationHandler.java index 76b71d7518..248191e6bb 100644 --- a/micrometer-core/src/main/java/io/micrometer/core/instrument/observation/DefaultMeterObservationHandler.java +++ b/micrometer-core/src/main/java/io/micrometer/core/instrument/observation/DefaultMeterObservationHandler.java @@ -15,10 +15,12 @@ */ package io.micrometer.core.instrument.observation; +import io.micrometer.common.KeyValue; import io.micrometer.core.instrument.*; import io.micrometer.observation.Observation; -import java.util.stream.Collectors; +import java.util.ArrayList; +import java.util.List; /** * Handler for {@link Timer.Sample} and {@link Counter}. @@ -58,11 +60,10 @@ public void onStart(Observation.Context context) { @Override public void onStop(Observation.Context context) { + List tags = createTags(context); + tags.add(Tag.of("error", getErrorValue(context))); Timer.Sample sample = context.getRequired(Timer.Sample.class); - sample.stop(Timer.builder(context.getName()) - .tags(createErrorTags(context)) - .tags(createTags(context)) - .register(this.meterRegistry)); + sample.stop(Timer.builder(context.getName()).tags(tags).register(this.meterRegistry)); LongTaskTimer.Sample longTaskSample = context.getRequired(LongTaskTimer.Sample.class); longTaskSample.stop(); @@ -76,20 +77,17 @@ public void onEvent(Observation.Event event, Observation.Context context) { .increment(); } - private Tags createErrorTags(Observation.Context context) { - return Tags.of("error", getErrorValue(context)); - } - private String getErrorValue(Observation.Context context) { Throwable error = context.getError(); return error != null ? error.getClass().getSimpleName() : "none"; } - private Tags createTags(Observation.Context context) { - return Tags.of(context.getLowCardinalityKeyValues() - .stream() - .map(tag -> Tag.of(tag.getKey(), tag.getValue())) - .collect(Collectors.toList())); + private List createTags(Observation.Context context) { + List tags = new ArrayList<>(); + for (KeyValue keyValue : context.getLowCardinalityKeyValues()) { + tags.add(Tag.of(keyValue.getKey(), keyValue.getValue())); + } + return tags; } } diff --git a/micrometer-observation/src/main/java/io/micrometer/observation/Observation.java b/micrometer-observation/src/main/java/io/micrometer/observation/Observation.java index 6eb0082746..f4a9825354 100644 --- a/micrometer-observation/src/main/java/io/micrometer/observation/Observation.java +++ b/micrometer-observation/src/main/java/io/micrometer/observation/Observation.java @@ -20,7 +20,6 @@ import io.micrometer.common.lang.NonNull; import io.micrometer.common.lang.Nullable; -import java.util.Arrays; import java.util.HashMap; import java.util.LinkedHashMap; import java.util.Map; @@ -367,7 +366,9 @@ default Observation lowCardinalityKeyValue(String key, String value) { * @return this */ default Observation lowCardinalityKeyValues(KeyValues keyValues) { - keyValues.stream().forEach(this::lowCardinalityKeyValue); + for (KeyValue keyValue : keyValues) { + lowCardinalityKeyValue(keyValue); + } return this; } @@ -400,7 +401,9 @@ default Observation highCardinalityKeyValue(String key, String value) { * @return this */ default Observation highCardinalityKeyValues(KeyValues keyValues) { - keyValues.stream().forEach(this::highCardinalityKeyValue); + for (KeyValue keyValue : keyValues) { + highCardinalityKeyValue(keyValue); + } return this; } @@ -1026,7 +1029,9 @@ public Context removeHighCardinalityKeyValue(String keyName) { * @return this context */ public Context addLowCardinalityKeyValues(KeyValues keyValues) { - keyValues.stream().forEach(this::addLowCardinalityKeyValue); + for (KeyValue keyValue : keyValues) { + addLowCardinalityKeyValue(keyValue); + } return this; } @@ -1036,7 +1041,9 @@ public Context addLowCardinalityKeyValues(KeyValues keyValues) { * @return this context */ public Context addHighCardinalityKeyValues(KeyValues keyValues) { - keyValues.stream().forEach(this::addHighCardinalityKeyValue); + for (KeyValue keyValue : keyValues) { + addHighCardinalityKeyValue(keyValue); + } return this; } @@ -1047,7 +1054,9 @@ public Context addHighCardinalityKeyValues(KeyValues keyValues) { * @since 1.10.1 */ public Context removeLowCardinalityKeyValues(String... keyNames) { - Arrays.stream(keyNames).forEach(this::removeLowCardinalityKeyValue); + for (String keyName : keyNames) { + removeLowCardinalityKeyValue(keyName); + } return this; } @@ -1058,7 +1067,9 @@ public Context removeLowCardinalityKeyValues(String... keyNames) { * @since 1.10.1 */ public Context removeHighCardinalityKeyValues(String... keyNames) { - Arrays.stream(keyNames).forEach(this::removeHighCardinalityKeyValue); + for (String keyName : keyNames) { + removeHighCardinalityKeyValue(keyName); + } return this; } diff --git a/micrometer-observation/src/main/java/io/micrometer/observation/ObservationHandler.java b/micrometer-observation/src/main/java/io/micrometer/observation/ObservationHandler.java index a064353f76..cb453bf0a3 100644 --- a/micrometer-observation/src/main/java/io/micrometer/observation/ObservationHandler.java +++ b/micrometer-observation/src/main/java/io/micrometer/observation/ObservationHandler.java @@ -16,10 +16,12 @@ package io.micrometer.observation; import java.util.Arrays; +import java.util.ArrayList; import java.util.List; import java.util.Optional; -import java.util.stream.Collectors; -import java.util.stream.Stream; +import java.util.function.Consumer; + +import io.micrometer.observation.Observation.Context; /** * Handler for an {@link Observation}. Hooks in to the lifecycle of an observation. @@ -129,9 +131,11 @@ public FirstMatchingCompositeObservationHandler(ObservationHandler> handlers) { - this.handlers = handlers.stream() - .map(handler -> (ObservationHandler) handler) - .collect(Collectors.toList()); + List> castedHandlers = new ArrayList<>(handlers.size()); + for (ObservationHandler handler : handlers) { + castedHandlers.add((ObservationHandler) handler); + } + this.handlers = castedHandlers; } @Override @@ -181,7 +185,12 @@ public boolean supportsContext(Observation.Context context) { private Optional> getFirstApplicableHandler( Observation.Context context) { - return this.handlers.stream().filter(handler -> handler.supportsContext(context)).findFirst(); + for (ObservationHandler handler : this.handlers) { + if (handler.supportsContext(context)) { + return Optional.of(handler); + } + } + return Optional.empty(); } } @@ -209,9 +218,11 @@ public AllMatchingCompositeObservationHandler(ObservationHandler> handlers) { - this.handlers = handlers.stream() - .map(handler -> (ObservationHandler) handler) - .collect(Collectors.toList()); + List> castedHandlers = new ArrayList<>(handlers.size()); + for (ObservationHandler handler : handlers) { + castedHandlers.add((ObservationHandler) handler); + } + this.handlers = castedHandlers; } @Override @@ -221,46 +232,56 @@ public List> getHandlers() { @Override public void onStart(Observation.Context context) { - getAllApplicableHandlers(context).forEach(handler -> handler.onStart(context)); + forEachApplicableHandler(context, handler -> handler.onStart(context)); } @Override public void onError(Observation.Context context) { - getAllApplicableHandlers(context).forEach(handler -> handler.onError(context)); + forEachApplicableHandler(context, handler -> handler.onError(context)); } @Override public void onEvent(Observation.Event event, Observation.Context context) { - getAllApplicableHandlers(context).forEach(handler -> handler.onEvent(event, context)); + forEachApplicableHandler(context, handler -> handler.onEvent(event, context)); } @Override public void onScopeOpened(Observation.Context context) { - getAllApplicableHandlers(context).forEach(handler -> handler.onScopeOpened(context)); + forEachApplicableHandler(context, handler -> handler.onScopeOpened(context)); } @Override public void onScopeClosed(Observation.Context context) { - getAllApplicableHandlers(context).forEach(handler -> handler.onScopeClosed(context)); + forEachApplicableHandler(context, handler -> handler.onScopeClosed(context)); } @Override public void onScopeReset(Observation.Context context) { - getAllApplicableHandlers(context).forEach(handler -> handler.onScopeReset(context)); + forEachApplicableHandler(context, handler -> handler.onScopeReset(context)); } @Override public void onStop(Observation.Context context) { - getAllApplicableHandlers(context).forEach(handler -> handler.onStop(context)); + forEachApplicableHandler(context, handler -> handler.onStop(context)); } @Override public boolean supportsContext(Observation.Context context) { - return getAllApplicableHandlers(context).findAny().isPresent(); + for (ObservationHandler handler : this.handlers) { + if (handler.supportsContext(context)) { + return true; + } + } + return false; } - private Stream> getAllApplicableHandlers(Observation.Context context) { - return this.handlers.stream().filter(handler -> handler.supportsContext(context)); + private void forEachApplicableHandler(Observation.Context context, + Consumer> action) { + for (ObservationHandler handler : this.handlers) { + if (handler.supportsContext(context)) { + action.accept(handler); + } + } } } diff --git a/micrometer-observation/src/main/java/io/micrometer/observation/ObservationRegistry.java b/micrometer-observation/src/main/java/io/micrometer/observation/ObservationRegistry.java index b8ff860a41..be95a99b8a 100644 --- a/micrometer-observation/src/main/java/io/micrometer/observation/ObservationRegistry.java +++ b/micrometer-observation/src/main/java/io/micrometer/observation/ObservationRegistry.java @@ -160,10 +160,12 @@ public ObservationConfig observationConvention(GlobalObservationConvention ob @SuppressWarnings("unchecked") ObservationConvention getObservationConvention(T context, ObservationConvention defaultConvention) { - return (ObservationConvention) this.observationConventions.stream() - .filter(convention -> convention.supportsContext(context)) - .findFirst() - .orElse(Objects.requireNonNull(defaultConvention, "Default ObservationConvention must not be null")); + for (ObservationConvention convention : this.observationConventions) { + if (convention.supportsContext(context)) { + return (ObservationConvention) convention; + } + } + return Objects.requireNonNull(defaultConvention, "Default ObservationConvention must not be null"); } /** @@ -174,7 +176,12 @@ ObservationConvention getObservationConventio * @return {@code true} when observation is enabled */ boolean isObservationEnabled(String name, @Nullable Observation.Context context) { - return this.observationPredicates.stream().allMatch(predicate -> predicate.test(name, context)); + for (ObservationPredicate predicate : this.observationPredicates) { + if (!predicate.test(name, context)) { + return false; + } + } + return true; } // package-private for minimal visibility diff --git a/micrometer-observation/src/main/java/io/micrometer/observation/SimpleObservation.java b/micrometer-observation/src/main/java/io/micrometer/observation/SimpleObservation.java index 8f236c44cf..dc5c87e3e1 100644 --- a/micrometer-observation/src/main/java/io/micrometer/observation/SimpleObservation.java +++ b/micrometer-observation/src/main/java/io/micrometer/observation/SimpleObservation.java @@ -22,7 +22,6 @@ import java.util.ArrayDeque; import java.util.Collection; import java.util.Deque; -import java.util.stream.Collectors; /** * Default implementation of {@link Observation}. @@ -83,20 +82,23 @@ private void setParentFromCurrentObservation() { @Nullable private static ObservationConvention getConventionFromConfig(ObservationRegistry registry, Context context) { - return registry.observationConfig() - .getObservationConventions() - .stream() - .filter(convention -> convention.supportsContext(context)) - .findFirst() - .orElse(null); + for (ObservationConvention convention : registry.observationConfig().getObservationConventions()) { + if (convention.supportsContext(context)) { + return convention; + } + } + return null; } private static Deque getHandlersFromConfig(ObservationRegistry registry, Context context) { - return registry.observationConfig() - .getObservationHandlers() - .stream() - .filter(handler -> handler.supportsContext(context)) - .collect(Collectors.toCollection(ArrayDeque::new)); + Collection> handlers = registry.observationConfig().getObservationHandlers(); + Deque deque = new ArrayDeque<>(handlers.size()); + for (ObservationHandler handler : handlers) { + if (handler.supportsContext(context)) { + deque.add(handler); + } + } + return deque; } @Override From 63cfcbccd57dabb9cfffe77b613626ded47803d2 Mon Sep 17 00:00:00 2001 From: Kuba Marchwicki Date: Tue, 4 Apr 2023 11:50:03 +0200 Subject: [PATCH 21/21] Add outcome tag to HttpClient metrics (#3732) Aligns the HTTP client instrumentations by consistently including an outcome tag. This can make querying these metrics easier as it groups a class of HTTP status codes into a single value that can be directly queried instead of using regex or ranges. --- ...cheHttpClientObservationDocumentation.java | 6 +++++ ...ApacheHttpClientObservationConvention.java | 9 ++++++- .../MicrometerHttpRequestExecutor.java | 6 ++++- ...cheHttpClientObservationDocumentation.java | 6 +++++ ...ApacheHttpClientObservationConvention.java | 9 ++++++- .../hc5/MicrometerHttpRequestExecutor.java | 6 ++++- .../DefaultOkHttpObservationConvention.java | 12 +++++++++- .../okhttp3/OkHttpMetricsEventListener.java | 10 ++++++++ .../OkHttpObservationDocumentation.java | 7 ++++++ ...efaultHttpClientObservationConvention.java | 8 +++++-- .../HttpClientObservationDocumentation.java | 7 ++++++ .../binder/jdk/MicrometerHttpClient.java | 8 +++++-- .../MicrometerHttpRequestExecutorTest.java | 24 ++++++++++++------- .../MicrometerHttpRequestExecutorTest.java | 24 ++++++++++++------- .../OkHttpMetricsEventListenerTest.java | 11 +++++---- .../binder/jdk/MicrometerHttpClientTests.java | 1 + ...imingInstrumentationVerificationTests.java | 10 +++++--- 17 files changed, 131 insertions(+), 33 deletions(-) diff --git a/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/httpcomponents/ApacheHttpClientObservationDocumentation.java b/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/httpcomponents/ApacheHttpClientObservationDocumentation.java index f07b2ef5b2..c212a39ea3 100644 --- a/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/httpcomponents/ApacheHttpClientObservationDocumentation.java +++ b/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/httpcomponents/ApacheHttpClientObservationDocumentation.java @@ -52,6 +52,12 @@ public String asString() { return "status"; } }, + OUTCOME { + @Override + public String asString() { + return "outcome"; + } + }, METHOD { @Override public String asString() { diff --git a/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/httpcomponents/DefaultApacheHttpClientObservationConvention.java b/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/httpcomponents/DefaultApacheHttpClientObservationConvention.java index b7db14bce3..1483ffb0a7 100644 --- a/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/httpcomponents/DefaultApacheHttpClientObservationConvention.java +++ b/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/httpcomponents/DefaultApacheHttpClientObservationConvention.java @@ -17,6 +17,7 @@ import io.micrometer.common.KeyValues; import io.micrometer.common.lang.Nullable; +import io.micrometer.core.instrument.binder.http.Outcome; import org.apache.http.HttpException; import org.apache.http.HttpRequest; import org.apache.http.HttpResponse; @@ -63,13 +64,19 @@ public KeyValues getLowCardinalityKeyValues(ApacheHttpClientContext context) { ApacheHttpClientObservationDocumentation.ApacheHttpClientKeyNames.URI .withValue(context.getUriMapper().apply(context.getCarrier())), ApacheHttpClientObservationDocumentation.ApacheHttpClientKeyNames.STATUS - .withValue(getStatusValue(context.getResponse(), context.getError()))); + .withValue(getStatusValue(context.getResponse(), context.getError())), + ApacheHttpClientObservationDocumentation.ApacheHttpClientKeyNames.OUTCOME + .withValue(getStatusOutcome(context.getResponse()).name())); if (context.shouldExportTagsForRoute()) { keyValues = keyValues.and(HttpContextUtils.generateTagStringsForRoute(context.getApacheHttpContext())); } return keyValues; } + Outcome getStatusOutcome(@Nullable HttpResponse response) { + return response != null ? Outcome.forStatus(response.getStatusLine().getStatusCode()) : Outcome.UNKNOWN; + } + String getStatusValue(@Nullable HttpResponse response, Throwable error) { if (error instanceof IOException || error instanceof HttpException || error instanceof RuntimeException) { return "IO_ERROR"; diff --git a/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/httpcomponents/MicrometerHttpRequestExecutor.java b/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/httpcomponents/MicrometerHttpRequestExecutor.java index fda9dcfa7a..58247babf5 100644 --- a/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/httpcomponents/MicrometerHttpRequestExecutor.java +++ b/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/httpcomponents/MicrometerHttpRequestExecutor.java @@ -21,6 +21,7 @@ import io.micrometer.core.instrument.Tag; import io.micrometer.core.instrument.Tags; import io.micrometer.core.instrument.Timer; +import io.micrometer.core.instrument.binder.http.Outcome; import io.micrometer.core.instrument.observation.ObservationOrTimerCompatibleInstrumentation; import io.micrometer.observation.Observation; import io.micrometer.observation.ObservationRegistry; @@ -118,11 +119,13 @@ public HttpResponse execute(HttpRequest request, HttpClientConnection conn, Http () -> new ApacheHttpClientContext(request, context, uriMapper, exportTagsForRoute), convention, DefaultApacheHttpClientObservationConvention.INSTANCE); String statusCodeOrError = "UNKNOWN"; + Outcome statusOutcome = Outcome.UNKNOWN; try { HttpResponse response = super.execute(request, conn, context); sample.setResponse(response); statusCodeOrError = DefaultApacheHttpClientObservationConvention.INSTANCE.getStatusValue(response, null); + statusOutcome = DefaultApacheHttpClientObservationConvention.INSTANCE.getStatusOutcome(response); return response; } catch (IOException | HttpException | RuntimeException e) { @@ -132,10 +135,11 @@ public HttpResponse execute(HttpRequest request, HttpClientConnection conn, Http } finally { String status = statusCodeOrError; + String outcome = statusOutcome.name(); sample.stop(METER_NAME, "Duration of Apache HttpClient request execution", () -> Tags .of("method", DefaultApacheHttpClientObservationConvention.INSTANCE.getMethodString(request), - "uri", uriMapper.apply(request), "status", status) + "uri", uriMapper.apply(request), "status", status, "outcome", outcome) .and(exportTagsForRoute ? HttpContextUtils.generateTagsForRoute(context) : Tags.empty()) .and(extraTags)); } diff --git a/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/httpcomponents/hc5/ApacheHttpClientObservationDocumentation.java b/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/httpcomponents/hc5/ApacheHttpClientObservationDocumentation.java index ae7b704a0e..9238c5fc10 100644 --- a/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/httpcomponents/hc5/ApacheHttpClientObservationDocumentation.java +++ b/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/httpcomponents/hc5/ApacheHttpClientObservationDocumentation.java @@ -48,6 +48,12 @@ public String asString() { return "status"; } }, + OUTCOME { + @Override + public String asString() { + return "outcome"; + } + }, METHOD { @Override public String asString() { diff --git a/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/httpcomponents/hc5/DefaultApacheHttpClientObservationConvention.java b/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/httpcomponents/hc5/DefaultApacheHttpClientObservationConvention.java index 788bd8b1b7..9492cb7bcf 100644 --- a/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/httpcomponents/hc5/DefaultApacheHttpClientObservationConvention.java +++ b/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/httpcomponents/hc5/DefaultApacheHttpClientObservationConvention.java @@ -17,6 +17,7 @@ import io.micrometer.common.KeyValues; import io.micrometer.common.lang.Nullable; +import io.micrometer.core.instrument.binder.http.Outcome; import org.apache.hc.core5.http.HttpException; import org.apache.hc.core5.http.HttpRequest; import org.apache.hc.core5.http.HttpResponse; @@ -59,13 +60,19 @@ public KeyValues getLowCardinalityKeyValues(ApacheHttpClientContext context) { ApacheHttpClientObservationDocumentation.ApacheHttpClientKeyNames.URI .withValue(context.getUriMapper().apply(context.getCarrier())), ApacheHttpClientObservationDocumentation.ApacheHttpClientKeyNames.STATUS - .withValue(getStatusValue(context.getResponse(), context.getError()))); + .withValue(getStatusValue(context.getResponse(), context.getError())), + ApacheHttpClientObservationDocumentation.ApacheHttpClientKeyNames.OUTCOME + .withValue(getStatusOutcome(context.getResponse()).name())); if (context.shouldExportTagsForRoute()) { keyValues = keyValues.and(HttpContextUtils.generateTagStringsForRoute(context.getApacheHttpContext())); } return keyValues; } + Outcome getStatusOutcome(@Nullable HttpResponse response) { + return response != null ? Outcome.forStatus(response.getCode()) : Outcome.UNKNOWN; + } + String getStatusValue(@Nullable HttpResponse response, Throwable error) { if (error instanceof IOException || error instanceof HttpException || error instanceof RuntimeException) { return "IO_ERROR"; diff --git a/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/httpcomponents/hc5/MicrometerHttpRequestExecutor.java b/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/httpcomponents/hc5/MicrometerHttpRequestExecutor.java index b378c63add..e7e46455cc 100644 --- a/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/httpcomponents/hc5/MicrometerHttpRequestExecutor.java +++ b/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/httpcomponents/hc5/MicrometerHttpRequestExecutor.java @@ -20,6 +20,7 @@ import io.micrometer.core.instrument.Tag; import io.micrometer.core.instrument.Tags; import io.micrometer.core.instrument.Timer; +import io.micrometer.core.instrument.binder.http.Outcome; import io.micrometer.core.instrument.observation.ObservationOrTimerCompatibleInstrumentation; import io.micrometer.observation.Observation; import io.micrometer.observation.ObservationRegistry; @@ -105,11 +106,13 @@ public ClassicHttpResponse execute(ClassicHttpRequest request, HttpClientConnect () -> new ApacheHttpClientContext(request, context, uriMapper, exportTagsForRoute), convention, DefaultApacheHttpClientObservationConvention.INSTANCE); String statusCodeOrError = "UNKNOWN"; + Outcome statusOutcome = Outcome.UNKNOWN; try { ClassicHttpResponse response = super.execute(request, conn, context); sample.setResponse(response); statusCodeOrError = DefaultApacheHttpClientObservationConvention.INSTANCE.getStatusValue(response, null); + statusOutcome = DefaultApacheHttpClientObservationConvention.INSTANCE.getStatusOutcome(response); return response; } catch (IOException | HttpException | RuntimeException e) { @@ -119,10 +122,11 @@ public ClassicHttpResponse execute(ClassicHttpRequest request, HttpClientConnect } finally { String status = statusCodeOrError; + String outcome = statusOutcome.name(); sample.stop(METER_NAME, "Duration of Apache HttpClient request execution", () -> Tags .of("method", DefaultApacheHttpClientObservationConvention.INSTANCE.getMethodString(request), - "uri", uriMapper.apply(request), "status", status) + "uri", uriMapper.apply(request), "status", status, "outcome", outcome) .and(exportTagsForRoute ? HttpContextUtils.generateTagsForRoute(context) : Tags.empty()) .and(extraTags)); } diff --git a/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/okhttp3/DefaultOkHttpObservationConvention.java b/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/okhttp3/DefaultOkHttpObservationConvention.java index 36527458f3..a4756b4235 100644 --- a/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/okhttp3/DefaultOkHttpObservationConvention.java +++ b/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/okhttp3/DefaultOkHttpObservationConvention.java @@ -22,6 +22,7 @@ import io.micrometer.common.lang.Nullable; import io.micrometer.core.instrument.Tag; import io.micrometer.core.instrument.Tags; +import io.micrometer.core.instrument.binder.http.Outcome; import okhttp3.Request; import okhttp3.Response; @@ -88,7 +89,8 @@ public KeyValues getLowCardinalityKeyValues(OkHttpContext context) { KeyValues keyValues = KeyValues .of(METHOD.withValue(requestAvailable ? request.method() : TAG_VALUE_UNKNOWN), URI.withValue(getUriTag(urlMapper, state, request)), - STATUS.withValue(getStatusMessage(state.response, state.exception))) + STATUS.withValue(getStatusMessage(state.response, state.exception)), + OUTCOME.withValue(getStatusOutcome(state.response).name())) .and(extraTags) .and(stream(contextSpecificTags.spliterator(), false) .map(contextTag -> contextTag.apply(request, state.response)) @@ -112,6 +114,14 @@ private String getUriTag(Function urlMapper, OkHttpObservationI : urlMapper.apply(request); } + private Outcome getStatusOutcome(@Nullable Response response) { + if (response == null) { + return Outcome.UNKNOWN; + } + + return Outcome.forStatus(response.code()); + } + private String getStatusMessage(@Nullable Response response, @Nullable IOException exception) { if (exception != null) { return "IO_ERROR"; diff --git a/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/okhttp3/OkHttpMetricsEventListener.java b/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/okhttp3/OkHttpMetricsEventListener.java index 0efc1a7dcc..8b1809303d 100644 --- a/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/okhttp3/OkHttpMetricsEventListener.java +++ b/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/okhttp3/OkHttpMetricsEventListener.java @@ -22,6 +22,7 @@ import io.micrometer.core.instrument.Tag; import io.micrometer.core.instrument.Tags; import io.micrometer.core.instrument.Timer; +import io.micrometer.core.instrument.binder.http.Outcome; import okhttp3.EventListener; import okhttp3.*; @@ -168,6 +169,7 @@ void time(CallState state) { Iterable tags = Tags .of("method", requestAvailable ? request.method() : TAG_VALUE_UNKNOWN, "uri", getUriTag(state, request), "status", getStatusMessage(state.response, state.exception)) + .and(getStatusOutcome(state.response).asTag()) .and(extraTags) .and(stream(contextSpecificTags.spliterator(), false) .map(contextTag -> contextTag.apply(request, state.response)) @@ -219,6 +221,14 @@ private Iterable getRequestTags(@Nullable Request request) { return Tags.empty(); } + private Outcome getStatusOutcome(@Nullable Response response) { + if (response == null) { + return Outcome.UNKNOWN; + } + + return Outcome.forStatus(response.code()); + } + private String getStatusMessage(@Nullable Response response, @Nullable IOException exception) { if (exception != null) { return "IO_ERROR"; diff --git a/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/okhttp3/OkHttpObservationDocumentation.java b/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/okhttp3/OkHttpObservationDocumentation.java index 568160ba1d..99b98676da 100644 --- a/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/okhttp3/OkHttpObservationDocumentation.java +++ b/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/okhttp3/OkHttpObservationDocumentation.java @@ -95,6 +95,13 @@ public String asString() { public String asString() { return "status"; } + }, + + OUTCOME { + @Override + public String asString() { + return "outcome"; + } } } diff --git a/micrometer-core/src/main/java11/io/micrometer/core/instrument/binder/jdk/DefaultHttpClientObservationConvention.java b/micrometer-core/src/main/java11/io/micrometer/core/instrument/binder/jdk/DefaultHttpClientObservationConvention.java index 12454dc28e..91e4b9871c 100644 --- a/micrometer-core/src/main/java11/io/micrometer/core/instrument/binder/jdk/DefaultHttpClientObservationConvention.java +++ b/micrometer-core/src/main/java11/io/micrometer/core/instrument/binder/jdk/DefaultHttpClientObservationConvention.java @@ -18,6 +18,7 @@ import io.micrometer.common.KeyValues; import io.micrometer.common.lang.NonNull; import io.micrometer.common.lang.Nullable; +import io.micrometer.core.instrument.binder.http.Outcome; import java.net.http.HttpRequest; import java.net.http.HttpResponse; @@ -47,8 +48,11 @@ public KeyValues getLowCardinalityKeyValues(HttpClientContext context) { HttpClientObservationDocumentation.LowCardinalityKeys.URI .withValue(getUriTag(httpRequest, context.getResponse(), context.getUriMapper()))); if (context.getResponse() != null) { - keyValues = keyValues.and(HttpClientObservationDocumentation.LowCardinalityKeys.STATUS - .withValue(String.valueOf(context.getResponse().statusCode()))); + keyValues = keyValues + .and(HttpClientObservationDocumentation.LowCardinalityKeys.STATUS + .withValue(String.valueOf(context.getResponse().statusCode()))) + .and(HttpClientObservationDocumentation.LowCardinalityKeys.OUTCOME + .withValue(Outcome.forStatus(context.getResponse().statusCode()).name())); } return keyValues; } diff --git a/micrometer-core/src/main/java11/io/micrometer/core/instrument/binder/jdk/HttpClientObservationDocumentation.java b/micrometer-core/src/main/java11/io/micrometer/core/instrument/binder/jdk/HttpClientObservationDocumentation.java index 38b61f8bdf..63888b5f41 100644 --- a/micrometer-core/src/main/java11/io/micrometer/core/instrument/binder/jdk/HttpClientObservationDocumentation.java +++ b/micrometer-core/src/main/java11/io/micrometer/core/instrument/binder/jdk/HttpClientObservationDocumentation.java @@ -60,6 +60,13 @@ public String asString() { } }, + OUTCOME { + @Override + public String asString() { + return "outcome"; + } + }, + /** * HTTP URI. */ diff --git a/micrometer-core/src/main/java11/io/micrometer/core/instrument/binder/jdk/MicrometerHttpClient.java b/micrometer-core/src/main/java11/io/micrometer/core/instrument/binder/jdk/MicrometerHttpClient.java index 41b5c7a8dc..37c7a7a1cb 100644 --- a/micrometer-core/src/main/java11/io/micrometer/core/instrument/binder/jdk/MicrometerHttpClient.java +++ b/micrometer-core/src/main/java11/io/micrometer/core/instrument/binder/jdk/MicrometerHttpClient.java @@ -19,6 +19,7 @@ import io.micrometer.core.instrument.MeterRegistry; import io.micrometer.core.instrument.Tag; import io.micrometer.core.instrument.Tags; +import io.micrometer.core.instrument.binder.http.Outcome; import io.micrometer.core.instrument.observation.ObservationOrTimerCompatibleInstrumentation; import io.micrometer.observation.Observation; import io.micrometer.observation.ObservationRegistry; @@ -238,8 +239,11 @@ private void stopObservationOrTimer( request.method(), HttpClientObservationDocumentation.LowCardinalityKeys.URI.asString(), DefaultHttpClientObservationConvention.INSTANCE.getUriTag(request, res, uriMapper)); if (res != null) { - tags = tags.and(Tag.of(HttpClientObservationDocumentation.LowCardinalityKeys.STATUS.asString(), - String.valueOf(res.statusCode()))); + tags = tags + .and(Tag.of(HttpClientObservationDocumentation.LowCardinalityKeys.STATUS.asString(), + String.valueOf(res.statusCode()))) + .and(Tag.of(HttpClientObservationDocumentation.LowCardinalityKeys.OUTCOME.asString(), + Outcome.forStatus(res.statusCode()).name())); } return tags; }); diff --git a/micrometer-core/src/test/java/io/micrometer/core/instrument/binder/httpcomponents/MicrometerHttpRequestExecutorTest.java b/micrometer-core/src/test/java/io/micrometer/core/instrument/binder/httpcomponents/MicrometerHttpRequestExecutorTest.java index f78b03c08e..14580a886c 100644 --- a/micrometer-core/src/test/java/io/micrometer/core/instrument/binder/httpcomponents/MicrometerHttpRequestExecutorTest.java +++ b/micrometer-core/src/test/java/io/micrometer/core/instrument/binder/httpcomponents/MicrometerHttpRequestExecutorTest.java @@ -96,12 +96,18 @@ void httpStatusCodeIsTagged(boolean configureObservationRegistry, @WiremockResol EntityUtils.consume(client.execute(new HttpGet(server.baseUrl() + "/ok")).getEntity()); EntityUtils.consume(client.execute(new HttpGet(server.baseUrl() + "/notfound")).getEntity()); EntityUtils.consume(client.execute(new HttpGet(server.baseUrl() + "/error")).getEntity()); - assertThat(registry.get(EXPECTED_METER_NAME).tags("method", "GET", "status", "200").timer().count()) - .isEqualTo(2L); - assertThat(registry.get(EXPECTED_METER_NAME).tags("method", "GET", "status", "404").timer().count()) - .isEqualTo(1L); - assertThat(registry.get(EXPECTED_METER_NAME).tags("method", "GET", "status", "500").timer().count()) - .isEqualTo(1L); + assertThat(registry.get(EXPECTED_METER_NAME) + .tags("method", "GET", "status", "200", "outcome", "SUCCESS") + .timer() + .count()).isEqualTo(2L); + assertThat(registry.get(EXPECTED_METER_NAME) + .tags("method", "GET", "status", "404", "outcome", "CLIENT_ERROR") + .timer() + .count()).isEqualTo(1L); + assertThat(registry.get(EXPECTED_METER_NAME) + .tags("method", "GET", "status", "500", "outcome", "SERVER_ERROR") + .timer() + .count()).isEqualTo(1L); } @ParameterizedTest @@ -329,8 +335,10 @@ void httpStatusCodeIsTaggedWithIoError(boolean configureObservationRegistry, assertThatThrownBy( () -> EntityUtils.consume(client.execute(new HttpGet(server.baseUrl() + "/error")).getEntity())) .isInstanceOf(ClientProtocolException.class); - assertThat(registry.get(EXPECTED_METER_NAME).tags("method", "GET", "status", "IO_ERROR").timer().count()) - .isEqualTo(1L); + assertThat(registry.get(EXPECTED_METER_NAME) + .tags("method", "GET", "status", "IO_ERROR", "outcome", "UNKNOWN") + .timer() + .count()).isEqualTo(1L); } static class CustomGlobalApacheHttpConvention extends DefaultApacheHttpClientObservationConvention diff --git a/micrometer-core/src/test/java/io/micrometer/core/instrument/binder/httpcomponents/hc5/MicrometerHttpRequestExecutorTest.java b/micrometer-core/src/test/java/io/micrometer/core/instrument/binder/httpcomponents/hc5/MicrometerHttpRequestExecutorTest.java index 5768cc7bc0..d83bb83245 100644 --- a/micrometer-core/src/test/java/io/micrometer/core/instrument/binder/httpcomponents/hc5/MicrometerHttpRequestExecutorTest.java +++ b/micrometer-core/src/test/java/io/micrometer/core/instrument/binder/httpcomponents/hc5/MicrometerHttpRequestExecutorTest.java @@ -107,12 +107,18 @@ void httpStatusCodeIsTagged(boolean configureObservationRegistry, @WiremockResol execute(client, new HttpGet(server.baseUrl() + "/ok")); execute(client, new HttpGet(server.baseUrl() + "/notfound")); execute(client, new HttpGet(server.baseUrl() + "/error")); - assertThat(registry.get(EXPECTED_METER_NAME).tags("method", "GET", "status", "200").timer().count()) - .isEqualTo(2L); - assertThat(registry.get(EXPECTED_METER_NAME).tags("method", "GET", "status", "404").timer().count()) - .isEqualTo(1L); - assertThat(registry.get(EXPECTED_METER_NAME).tags("method", "GET", "status", "500").timer().count()) - .isEqualTo(1L); + assertThat(registry.get(EXPECTED_METER_NAME) + .tags("method", "GET", "status", "200", "outcome", "SUCCESS") + .timer() + .count()).isEqualTo(2L); + assertThat(registry.get(EXPECTED_METER_NAME) + .tags("method", "GET", "status", "404", "outcome", "CLIENT_ERROR") + .timer() + .count()).isEqualTo(1L); + assertThat(registry.get(EXPECTED_METER_NAME) + .tags("method", "GET", "status", "500", "outcome", "SERVER_ERROR") + .timer() + .count()).isEqualTo(1L); } @ParameterizedTest @@ -323,8 +329,10 @@ void httpStatusCodeIsTaggedWithIoError(boolean configureObservationRegistry, CloseableHttpClient client = client(executor(false, configureObservationRegistry)); assertThatThrownBy(() -> execute(client, new HttpGet(server.baseUrl() + "/error"))) .isInstanceOf(ClientProtocolException.class); - assertThat(registry.get(EXPECTED_METER_NAME).tags("method", "GET", "status", "IO_ERROR").timer().count()) - .isEqualTo(1L); + assertThat(registry.get(EXPECTED_METER_NAME) + .tags("method", "GET", "status", "IO_ERROR", "outcome", "UNKNOWN") + .timer() + .count()).isEqualTo(1L); } static class CustomGlobalApacheHttpConvention extends DefaultApacheHttpClientObservationConvention diff --git a/micrometer-core/src/test/java/io/micrometer/core/instrument/binder/okhttp3/OkHttpMetricsEventListenerTest.java b/micrometer-core/src/test/java/io/micrometer/core/instrument/binder/okhttp3/OkHttpMetricsEventListenerTest.java index 9063f95a26..4a14a91588 100644 --- a/micrometer-core/src/test/java/io/micrometer/core/instrument/binder/okhttp3/OkHttpMetricsEventListenerTest.java +++ b/micrometer-core/src/test/java/io/micrometer/core/instrument/binder/okhttp3/OkHttpMetricsEventListenerTest.java @@ -72,8 +72,8 @@ void timeSuccessful(@WiremockResolver.Wiremock WireMockServer server) throws IOE client.newCall(request).execute().close(); assertThat(registry.get("okhttp.requests") - .tags("foo", "bar", "status", "200", "uri", URI_EXAMPLE_VALUE, "target.host", "localhost", "target.port", - String.valueOf(server.port()), "target.scheme", "http") + .tags("foo", "bar", "status", "200", "outcome", "SUCCESS", "uri", URI_EXAMPLE_VALUE, "target.host", + "localhost", "target.port", String.valueOf(server.port()), "target.scheme", "http") .timer() .count()).isEqualTo(1L); } @@ -86,8 +86,8 @@ void timeNotFound(@WiremockResolver.Wiremock WireMockServer server) throws IOExc client.newCall(request).execute().close(); assertThat(registry.get("okhttp.requests") - .tags("foo", "bar", "status", "404", "uri", "NOT_FOUND", "target.host", "localhost", "target.port", - String.valueOf(server.port()), "target.scheme", "http") + .tags("foo", "bar", "status", "404", "outcome", "CLIENT_ERROR", "uri", "NOT_FOUND", "target.host", + "localhost", "target.port", String.valueOf(server.port()), "target.scheme", "http") .timer() .count()).isEqualTo(1L); } @@ -114,7 +114,8 @@ void timeFailureDueToTimeout(@WiremockResolver.Wiremock WireMockServer server) { } assertThat(registry.get("okhttp.requests") - .tags("foo", "bar", "uri", URI_EXAMPLE_VALUE, "status", "IO_ERROR", "target.host", "localhost") + .tags("foo", "bar", "uri", URI_EXAMPLE_VALUE, "status", "IO_ERROR", "outcome", "UNKNOWN", "target.host", + "localhost") .timer() .count()).isEqualTo(1L); } diff --git a/micrometer-core/src/test/java11/io/micrometer/core/instrument/binder/jdk/MicrometerHttpClientTests.java b/micrometer-core/src/test/java11/io/micrometer/core/instrument/binder/jdk/MicrometerHttpClientTests.java index d2f5be8f77..d3a69a978b 100644 --- a/micrometer-core/src/test/java11/io/micrometer/core/instrument/binder/jdk/MicrometerHttpClientTests.java +++ b/micrometer-core/src/test/java11/io/micrometer/core/instrument/binder/jdk/MicrometerHttpClientTests.java @@ -88,6 +88,7 @@ private void thenMeterRegistryContainsHttpClientTags() { then(meterRegistry.find("http.client.requests") .tag("method", "GET") .tag("status", "200") + .tag("outcome", "SUCCESS") .tag("uri", "UNKNOWN") .timer()).isNotNull(); } diff --git a/micrometer-test/src/main/java/io/micrometer/core/instrument/HttpClientTimingInstrumentationVerificationTests.java b/micrometer-test/src/main/java/io/micrometer/core/instrument/HttpClientTimingInstrumentationVerificationTests.java index c370a47a2c..945c3e18fb 100644 --- a/micrometer-test/src/main/java/io/micrometer/core/instrument/HttpClientTimingInstrumentationVerificationTests.java +++ b/micrometer-test/src/main/java/io/micrometer/core/instrument/HttpClientTimingInstrumentationVerificationTests.java @@ -155,7 +155,7 @@ void getTemplatedPathForUri(TestType testType, WireMockRuntimeInfo wmRuntimeInfo templatedPath, "112", "5"); Timer timer = getRegistry().get(timerName()) - .tags("method", "GET", "status", "200", "uri", templatedPath) + .tags("method", "GET", "status", "200", "outcome", "SUCCESS", "uri", templatedPath) .timer(); assertThat(timer.count()).isEqualTo(1); assertThat(timer.totalTime(TimeUnit.NANOSECONDS)).isPositive(); @@ -195,7 +195,9 @@ void serverException(TestType testType, WireMockRuntimeInfo wmRuntimeInfo) { sendHttpRequest(instrumentedClient(testType), HttpMethod.GET, null, URI.create(wmRuntimeInfo.getHttpBaseUrl()), "/socks"); - Timer timer = getRegistry().get(timerName()).tags("method", "GET", "status", "500").timer(); + Timer timer = getRegistry().get(timerName()) + .tags("method", "GET", "status", "500", "outcome", "SERVER_ERROR") + .timer(); assertThat(timer.count()).isEqualTo(1); assertThat(timer.totalTime(TimeUnit.NANOSECONDS)).isPositive(); } @@ -211,7 +213,9 @@ void clientException(TestType testType, WireMockRuntimeInfo wmRuntimeInfo) { sendHttpRequest(instrumentedClient(testType), HttpMethod.POST, new byte[0], URI.create(wmRuntimeInfo.getHttpBaseUrl()), "/socks"); - Timer timer = getRegistry().get(timerName()).tags("method", "POST", "status", "400").timer(); + Timer timer = getRegistry().get(timerName()) + .tags("method", "POST", "status", "400", "outcome", "CLIENT_ERROR") + .timer(); assertThat(timer.count()).isEqualTo(1); assertThat(timer.totalTime(TimeUnit.NANOSECONDS)).isPositive(); }