diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/observation/ObservationHandlerGrouping.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/observation/ObservationHandlerGrouping.java index 163186947e..8312bc986c 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/observation/ObservationHandlerGrouping.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/observation/ObservationHandlerGrouping.java @@ -16,6 +16,7 @@ package org.springframework.boot.actuate.autoconfigure.observation; +import java.util.ArrayList; import java.util.List; import io.micrometer.observation.ObservationHandler; @@ -30,6 +31,7 @@ import org.springframework.util.MultiValueMap; * Groups {@link ObservationHandler ObservationHandlers} by type. * * @author Andy Wilkinson + * @author Moritz Halbritter */ @SuppressWarnings("rawtypes") class ObservationHandlerGrouping { @@ -46,13 +48,14 @@ class ObservationHandlerGrouping { void apply(List> handlers, ObservationConfig config) { MultiValueMap, ObservationHandler> groupings = new LinkedMultiValueMap<>(); + List> handlersWithoutCategory = new ArrayList<>(); for (ObservationHandler handler : handlers) { Class category = findCategory(handler); if (category != null) { groupings.add(category, handler); } else { - config.observationHandler(handler); + handlersWithoutCategory.add(handler); } } for (Class category : this.categories) { @@ -61,6 +64,9 @@ class ObservationHandlerGrouping { config.observationHandler(new FirstMatchingCompositeObservationHandler(handlerGroup)); } } + for (ObservationHandler observationHandler : handlersWithoutCategory) { + config.observationHandler(observationHandler); + } } private Class findCategory(ObservationHandler handler) { diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/observation/ObservationAutoConfigurationTests.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/observation/ObservationAutoConfigurationTests.java index eb994e6896..8060d26890 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/observation/ObservationAutoConfigurationTests.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/observation/ObservationAutoConfigurationTests.java @@ -223,14 +223,13 @@ class ObservationAutoConfigurationTests { Observation.start("test-observation", observationRegistry).stop(); assertThat(context).doesNotHaveBean(DefaultMeterObservationHandler.class); assertThat(handlers).hasSize(2); - // Regular handlers are registered first - assertThat(handlers.get(0)).isInstanceOf(CustomObservationHandler.class); // Multiple MeterObservationHandler are wrapped in - // FirstMatchingCompositeObservationHandler, which calls only the first - // one - assertThat(handlers.get(1)).isInstanceOf(CustomMeterObservationHandler.class); - assertThat(((CustomMeterObservationHandler) handlers.get(1)).getName()) + // FirstMatchingCompositeObservationHandler, which calls only the first one + assertThat(handlers.get(0)).isInstanceOf(CustomMeterObservationHandler.class); + assertThat(((CustomMeterObservationHandler) handlers.get(0)).getName()) .isEqualTo("customMeterObservationHandler1"); + // Regular handlers are registered last + assertThat(handlers.get(1)).isInstanceOf(CustomObservationHandler.class); assertThat(context).doesNotHaveBean(DefaultMeterObservationHandler.class); assertThat(context).doesNotHaveBean(TracingAwareMeterObservationHandler.class); }); @@ -273,20 +272,18 @@ class ObservationAutoConfigurationTests { List> handlers = context.getBean(CalledHandlers.class).getCalledHandlers(); Observation.start("test-observation", observationRegistry).stop(); assertThat(handlers).hasSize(3); - // Regular handlers are registered first - assertThat(handlers.get(0)).isInstanceOf(CustomObservationHandler.class); // Multiple TracingObservationHandler are wrapped in - // FirstMatchingCompositeObservationHandler, which calls only the first - // one - assertThat(handlers.get(1)).isInstanceOf(CustomTracingObservationHandler.class); - assertThat(((CustomTracingObservationHandler) handlers.get(1)).getName()) + // FirstMatchingCompositeObservationHandler, which calls only the first one + assertThat(handlers.get(0)).isInstanceOf(CustomTracingObservationHandler.class); + assertThat(((CustomTracingObservationHandler) handlers.get(0)).getName()) .isEqualTo("customTracingHandler1"); // Multiple MeterObservationHandler are wrapped in - // FirstMatchingCompositeObservationHandler, which calls only the first - // one - assertThat(handlers.get(2)).isInstanceOf(CustomMeterObservationHandler.class); - assertThat(((CustomMeterObservationHandler) handlers.get(2)).getName()) + // FirstMatchingCompositeObservationHandler, which calls only the first one + assertThat(handlers.get(1)).isInstanceOf(CustomMeterObservationHandler.class); + assertThat(((CustomMeterObservationHandler) handlers.get(1)).getName()) .isEqualTo("customMeterObservationHandler1"); + // Regular handlers are registered last + assertThat(handlers.get(2)).isInstanceOf(CustomObservationHandler.class); assertThat(context).doesNotHaveBean(TracingAwareMeterObservationHandler.class); assertThat(context).doesNotHaveBean(DefaultMeterObservationHandler.class); }); diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/observation/ObservationHandlerGroupingTests.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/observation/ObservationHandlerGroupingTests.java new file mode 100644 index 0000000000..62ac14d092 --- /dev/null +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/observation/ObservationHandlerGroupingTests.java @@ -0,0 +1,126 @@ +/* + * 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.boot.actuate.autoconfigure.observation; + +import java.lang.reflect.Method; +import java.util.List; + +import io.micrometer.observation.Observation; +import io.micrometer.observation.Observation.Context; +import io.micrometer.observation.ObservationHandler; +import io.micrometer.observation.ObservationHandler.FirstMatchingCompositeObservationHandler; +import io.micrometer.observation.ObservationRegistry.ObservationConfig; +import org.junit.jupiter.api.Test; + +import org.springframework.util.ReflectionUtils; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Tests for {@link ObservationHandlerGrouping}. + * + * @author Moritz Halbritter + */ +class ObservationHandlerGroupingTests { + + @Test + void shouldGroupCategoriesIntoFirstMatchingHandlerAndRespectsCategoryOrder() { + ObservationHandlerGrouping grouping = new ObservationHandlerGrouping( + List.of(ObservationHandlerA.class, ObservationHandlerB.class)); + ObservationConfig config = new ObservationConfig(); + ObservationHandlerA handlerA1 = new ObservationHandlerA("a1"); + ObservationHandlerA handlerA2 = new ObservationHandlerA("a2"); + ObservationHandlerB handlerB1 = new ObservationHandlerB("b1"); + ObservationHandlerB handlerB2 = new ObservationHandlerB("b2"); + grouping.apply(List.of(handlerB1, handlerB2, handlerA1, handlerA2), config); + List> handlers = getObservationHandlers(config); + assertThat(handlers).hasSize(2); + // Category A is first + assertThat(handlers.get(0)).isInstanceOf(FirstMatchingCompositeObservationHandler.class); + FirstMatchingCompositeObservationHandler firstMatching0 = (FirstMatchingCompositeObservationHandler) handlers + .get(0); + assertThat(firstMatching0.getHandlers()).containsExactly(handlerA1, handlerA2); + // Category B is second + assertThat(handlers.get(1)).isInstanceOf(FirstMatchingCompositeObservationHandler.class); + FirstMatchingCompositeObservationHandler firstMatching1 = (FirstMatchingCompositeObservationHandler) handlers + .get(1); + assertThat(firstMatching1.getHandlers()).containsExactly(handlerB1, handlerB2); + } + + @Test + void uncategorizedHandlersShouldBeOrderedAfterCategories() { + ObservationHandlerGrouping grouping = new ObservationHandlerGrouping(ObservationHandlerA.class); + ObservationConfig config = new ObservationConfig(); + ObservationHandlerA handlerA1 = new ObservationHandlerA("a1"); + ObservationHandlerA handlerA2 = new ObservationHandlerA("a2"); + ObservationHandlerB handlerB1 = new ObservationHandlerB("b1"); + grouping.apply(List.of(handlerB1, handlerA1, handlerA2), config); + List> handlers = getObservationHandlers(config); + assertThat(handlers).hasSize(2); + // Category A is first + assertThat(handlers.get(0)).isInstanceOf(FirstMatchingCompositeObservationHandler.class); + FirstMatchingCompositeObservationHandler firstMatching0 = (FirstMatchingCompositeObservationHandler) handlers + .get(0); + // Uncategorized handlers follow + assertThat(firstMatching0.getHandlers()).containsExactly(handlerA1, handlerA2); + assertThat(handlers.get(1)).isEqualTo(handlerB1); + } + + @SuppressWarnings("unchecked") + private static List> getObservationHandlers(ObservationConfig config) { + Method method = ReflectionUtils.findMethod(ObservationConfig.class, "getObservationHandlers"); + ReflectionUtils.makeAccessible(method); + return (List>) ReflectionUtils.invokeMethod(method, config); + } + + private static class NamedObservationHandler implements ObservationHandler { + + private final String name; + + NamedObservationHandler(String name) { + this.name = name; + } + + @Override + public boolean supportsContext(Context context) { + return true; + } + + @Override + public String toString() { + return getClass().getSimpleName() + "{name='" + this.name + "'}"; + } + + } + + private static class ObservationHandlerA extends NamedObservationHandler { + + ObservationHandlerA(String name) { + super(name); + } + + } + + private static class ObservationHandlerB extends NamedObservationHandler { + + ObservationHandlerB(String name) { + super(name); + } + + } + +}