From acb786d359f0b11ff67fe2b44db6aaace992dc17 Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Sun, 25 Jun 2023 18:49:57 +0200 Subject: [PATCH] Improve logging in MicrometerObservationRegistryTestExecutionListener Prior to this commit, dependency checks in the static initialization block for MicrometerObservationRegistryTestExecutionListener resulted in an ExceptionInInitializerError which led to verbose logging in TestContextFailureHandler. This commit improves the logging for missing dependencies in MicrometerObservationRegistryTestExecutionListener by moving the dependency checks to the constructor and by throwing a NoClassDefFoundError instead of an IllegalStateException. This allows TestContextFailureHandler to log a concise DEBUG message denoting that the listener is being skipped due to missing dependencies. This commit also now checks for the presence of io.micrometer.context.ThreadLocalAccessor in addition to io.micrometer.observation.contextpropagation.ObservationThreadLocalAccessor. Furthermore, this commit now explicitly mentions the need for io.micrometer:context-propagation in the error message. The following demonstrate the generated DEBUB message when ObservationThreadLocalAccessor and ThreadLocalAccessor are missing, respectively. Skipping candidate TestExecutionListener [org.springframework.test.context.observation.MicrometerObservationRegistryTestExecutionListener] due to a missing dependency. Specify custom TestExecutionListener classes or make the default TestExecutionListener classes and their required dependencies available. Offending class: io.micrometer.observation.contextpropagation.ObservationThreadLocalAccessor. MicrometerObservationRegistryTestExecutionListener requires io.micrometer:micrometer-observation:1.10.8 or higher and io.micrometer:context-propagation:1.0.3 or higher. Skipping candidate TestExecutionListener [org.springframework.test.context.observation.MicrometerObservationRegistryTestExecutionListener] due to a missing dependency. Specify custom TestExecutionListener classes or make the default TestExecutionListener classes and their required dependencies available. Offending class: io.micrometer.context.ThreadLocalAccessor. MicrometerObservationRegistryTestExecutionListener requires io.micrometer:micrometer-observation:1.10.8 or higher and io.micrometer:context-propagation:1.0.3 or higher. Closes gh-30747 --- ...ervationRegistryTestExecutionListener.java | 48 ++++++++++++------- .../util/TestContextFailureHandler.java | 2 +- 2 files changed, 33 insertions(+), 17 deletions(-) diff --git a/spring-test/src/main/java/org/springframework/test/context/observation/MicrometerObservationRegistryTestExecutionListener.java b/spring-test/src/main/java/org/springframework/test/context/observation/MicrometerObservationRegistryTestExecutionListener.java index e408242889..6e8f698ed8 100644 --- a/spring-test/src/main/java/org/springframework/test/context/observation/MicrometerObservationRegistryTestExecutionListener.java +++ b/spring-test/src/main/java/org/springframework/test/context/observation/MicrometerObservationRegistryTestExecutionListener.java @@ -26,9 +26,7 @@ import org.apache.commons.logging.LogFactory; import org.springframework.context.ApplicationContext; import org.springframework.core.Conventions; import org.springframework.test.context.TestContext; -import org.springframework.test.context.TestExecutionListener; import org.springframework.test.context.support.AbstractTestExecutionListener; -import org.springframework.util.Assert; import org.springframework.util.ReflectionUtils; /** @@ -47,6 +45,8 @@ class MicrometerObservationRegistryTestExecutionListener extends AbstractTestExe private static final Log logger = LogFactory.getLog(MicrometerObservationRegistryTestExecutionListener.class); + private static final String THREAD_LOCAL_ACCESSOR_CLASS_NAME = "io.micrometer.context.ThreadLocalAccessor"; + private static final String OBSERVATION_THREAD_LOCAL_ACCESSOR_CLASS_NAME = "io.micrometer.observation.contextpropagation.ObservationThreadLocalAccessor"; @@ -63,25 +63,41 @@ class MicrometerObservationRegistryTestExecutionListener extends AbstractTestExe MicrometerObservationRegistryTestExecutionListener.class, "previousObservationRegistry"); - static { - // Trigger eager resolution of Micrometer Observation types during static - // initialization of this class to ensure that this listener can be properly - // skipped when SpringFactoriesLoader attempts to load it, if micrometer-observation - // is not in the classpath or if the version of ObservationThreadLocalAccessor + public MicrometerObservationRegistryTestExecutionListener() { + // Trigger eager resolution of Micrometer Observation types to ensure that + // this listener can be properly skipped when SpringFactoriesLoader attempts + // to load it -- for example, if context-propagation and micrometer-observation + // are not in the classpath or if the version of ObservationThreadLocalAccessor // present does not include the getObservationRegistry() method. - String errorMessage = - "MicrometerObservationRegistryTestExecutionListener requires micrometer-observation 1.10.8 or higher"; - Class clazz; + ClassLoader classLoader = getClass().getClassLoader(); + String errorMessage = """ + MicrometerObservationRegistryTestExecutionListener requires \ + io.micrometer:micrometer-observation:1.10.8 or higher and \ + io.micrometer:context-propagation:1.0.3 or higher."""; + try { - clazz = Class.forName(OBSERVATION_THREAD_LOCAL_ACCESSOR_CLASS_NAME, true, - TestExecutionListener.class.getClassLoader()); + Class.forName(THREAD_LOCAL_ACCESSOR_CLASS_NAME, false, classLoader); + Class clazz = Class.forName(OBSERVATION_THREAD_LOCAL_ACCESSOR_CLASS_NAME, false, classLoader); + Method method = ReflectionUtils.findMethod(clazz, "getObservationRegistry"); + if (method == null) { + // We simulate "class not found" even though it's "method not found", since + // the ClassNotFoundException will be processed in the subsequent catch-block. + throw new ClassNotFoundException(OBSERVATION_THREAD_LOCAL_ACCESSOR_CLASS_NAME); + } } catch (Throwable ex) { - throw new IllegalStateException(errorMessage, ex); + // Ensure that we throw a NoClassDefFoundError so that the exception will be properly + // handled in TestContextFailureHandler. + // If the original exception was a ClassNotFoundException or NoClassDefFoundError, + // we throw a NoClassDefFoundError with an augmented error message and omit the cause. + if (ex instanceof ClassNotFoundException || ex instanceof NoClassDefFoundError) { + throw new NoClassDefFoundError(ex.getMessage() + ". " + errorMessage); + } + // Otherwise, we throw a NoClassDefFoundError with the cause initialized. + Error error = new NoClassDefFoundError(errorMessage); + error.initCause(ex); + throw error; } - - Method method = ReflectionUtils.findMethod(clazz, "getObservationRegistry"); - Assert.state(method != null, errorMessage); } diff --git a/spring-test/src/main/java/org/springframework/test/context/util/TestContextFailureHandler.java b/spring-test/src/main/java/org/springframework/test/context/util/TestContextFailureHandler.java index cfe9e815cb..7a475e74c1 100644 --- a/spring-test/src/main/java/org/springframework/test/context/util/TestContextFailureHandler.java +++ b/spring-test/src/main/java/org/springframework/test/context/util/TestContextFailureHandler.java @@ -43,7 +43,7 @@ class TestContextFailureHandler implements FailureHandler { logger.debug(""" Skipping candidate %1$s [%2$s] due to a missing dependency. \ Specify custom %1$s classes or make the default %1$s classes \ - and their required dependencies available. Offending class: [%3$s]""" + and their required dependencies available. Offending class: %3$s""" .formatted(factoryType.getSimpleName(), factoryImplementationName, ex.getMessage())); } }