-
Notifications
You must be signed in to change notification settings - Fork 26.4k
feat: add DEBUG-level logs for SPI extension loading in ExtensionLoader #16231
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 3.3
Are you sure you want to change the base?
Changes from 9 commits
fb8aa51
d662f03
86e4be4
6c0e7f8
813d269
ada8706
bb02b48
f559f66
d7efe33
b065dbe
17dccac
bd73870
317b0cf
b9138f3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -70,12 +70,20 @@ | |
| import org.apache.dubbo.common.lang.Prioritized; | ||
| import org.apache.dubbo.common.url.component.ServiceConfigURL; | ||
| import org.apache.dubbo.rpc.model.ApplicationModel; | ||
| import org.apache.dubbo.rpc.model.FrameworkModel; | ||
|
|
||
| import java.util.Collections; | ||
| import java.util.HashSet; | ||
| import java.util.List; | ||
| import java.util.Set; | ||
|
|
||
| import java.util.concurrent.CopyOnWriteArrayList; | ||
|
|
||
| import org.apache.logging.log4j.core.LogEvent; | ||
| import org.apache.logging.log4j.core.LoggerContext; | ||
| import org.apache.logging.log4j.core.appender.AbstractAppender; | ||
| import org.apache.logging.log4j.core.config.Configuration; | ||
| import org.apache.logging.log4j.core.config.LoggerConfig; | ||
| import org.apache.logging.log4j.core.layout.PatternLayout; | ||
| import org.junit.jupiter.api.Assertions; | ||
| import org.junit.jupiter.api.Test; | ||
|
|
||
|
|
@@ -192,6 +200,23 @@ void test_getExtension_WithWrapper() { | |
| assertEquals(echoCount2 + 1, Ext6Wrapper2.echoCount.get()); | ||
| } | ||
|
|
||
| @Test | ||
| void test_getExtension_logsDebugWhenExtensionCreated() { | ||
| try (ExtensionLoaderTestContext<WrappedExt> testContext = createExtensionLoaderTestContext(WrappedExt.class); | ||
| LogCollector logCollector = LogCollector.attach(ExtensionLoader.class)) { | ||
| WrappedExt impl1 = testContext.extensionLoader.getExtension("impl1"); | ||
|
||
|
|
||
| assertNotNull(impl1); | ||
| assertTrue(logCollector.contains("Loaded extension instance, type=" + WrappedExt.class.getName())); | ||
| assertTrue(logCollector.contains("name=impl1")); | ||
| assertTrue(logCollector.contains("instanceClass=" + Ext6Wrapper1.class.getName()) | ||
| || logCollector.contains("instanceClass=" + Ext6Wrapper2.class.getName())); | ||
| assertTrue(logCollector.contains("wrapperClasses=[")); | ||
| assertTrue(logCollector.contains(Ext6Wrapper1.class.getName())); | ||
| assertTrue(logCollector.contains(Ext6Wrapper2.class.getName())); | ||
|
uuuyuqi marked this conversation as resolved.
Outdated
|
||
| } | ||
| } | ||
|
|
||
| @Test | ||
| void test_getExtension_withWrapperAnnotation() { | ||
| WrappedExt impl3 = getExtensionLoader(WrappedExt.class).getExtension("impl3"); | ||
|
|
@@ -882,4 +907,95 @@ public int getPriority() { | |
| return MAX_PRIORITY; | ||
| } | ||
| } | ||
|
|
||
| private <T> ExtensionLoaderTestContext<T> createExtensionLoaderTestContext(Class<T> type) { | ||
| FrameworkModel frameworkModel = new FrameworkModel(); | ||
| ApplicationModel applicationModel = frameworkModel.newApplication(); | ||
| return new ExtensionLoaderTestContext<>( | ||
| frameworkModel, applicationModel.getExtensionDirector().getExtensionLoader(type)); | ||
| } | ||
|
|
||
| private static final class ExtensionLoaderTestContext<T> implements AutoCloseable { | ||
| private final FrameworkModel frameworkModel; | ||
| private final ExtensionLoader<T> extensionLoader; | ||
|
|
||
| private ExtensionLoaderTestContext(FrameworkModel frameworkModel, ExtensionLoader<T> extensionLoader) { | ||
| this.frameworkModel = frameworkModel; | ||
| this.extensionLoader = extensionLoader; | ||
| } | ||
|
|
||
| @Override | ||
| public void close() { | ||
| frameworkModel.destroy(); | ||
| } | ||
| } | ||
|
|
||
| private static final class LogCollector implements AutoCloseable { | ||
| private final LoggerContext context; | ||
| private final Configuration configuration; | ||
| private final String loggerName; | ||
| private final LoggerConfig loggerConfig; | ||
| private final TestAppender appender; | ||
| private final LoggerConfig preLoggerConfig; | ||
|
|
||
| private LogCollector( | ||
| LoggerContext context, | ||
| Configuration configuration, | ||
| String loggerName, | ||
| LoggerConfig loggerConfig, | ||
| TestAppender appender, | ||
| LoggerConfig preLoggerConfig) { | ||
| this.context = context; | ||
| this.configuration = configuration; | ||
| this.loggerName = loggerName; | ||
| this.loggerConfig = loggerConfig; | ||
| this.appender = appender; | ||
| this.preLoggerConfig = preLoggerConfig; | ||
| } | ||
|
|
||
| static LogCollector attach(Class<?> loggerType) { | ||
| LoggerContext context = LoggerContext.getContext(false); | ||
| Configuration configuration = context.getConfiguration(); | ||
| String loggerName = loggerType.getName(); | ||
| TestAppender appender = new TestAppender("test-appender-" + loggerType.getSimpleName()); | ||
| appender.start(); | ||
| configuration.addAppender(appender); | ||
|
|
||
| LoggerConfig preLoggerConfig = configuration.getLoggers().get(loggerName); | ||
| LoggerConfig loggerConfig = new LoggerConfig(loggerName, org.apache.logging.log4j.Level.DEBUG, false); | ||
| loggerConfig.addAppender(appender, org.apache.logging.log4j.Level.DEBUG, null); | ||
| configuration.addLogger(loggerName, loggerConfig); | ||
| context.updateLoggers(); | ||
| return new LogCollector(context, configuration, loggerName, loggerConfig, appender, preLoggerConfig); | ||
| } | ||
|
|
||
| boolean contains(String expected) { | ||
| return appender.messages.stream().anyMatch(message -> message.contains(expected)); | ||
| } | ||
|
|
||
| @Override | ||
| public void close() { | ||
| loggerConfig.removeAppender(appender.getName()); | ||
| configuration.removeLogger(loggerName); | ||
| appender.stop(); | ||
| configuration.getAppenders().remove(appender.getName()); | ||
| if (preLoggerConfig != null) { | ||
| configuration.addLogger(loggerName, preLoggerConfig); | ||
| } | ||
| context.updateLoggers(); | ||
| } | ||
| } | ||
|
|
||
| private static final class TestAppender extends AbstractAppender { | ||
| private final List<String> messages = new CopyOnWriteArrayList<>(); | ||
|
|
||
| private TestAppender(String name) { | ||
| super(name, null, PatternLayout.newBuilder().withPattern("%m").build(), false, null); | ||
| } | ||
|
|
||
| @Override | ||
| public void append(LogEvent event) { | ||
| messages.add(event.toImmutable().getMessage().getFormattedMessage()); | ||
| } | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At this point, startNanos might be null, for example, if the application dynamically modifies the log level at runtime. This is quite common in online troubleshooting scenarios. To be on the safe side, it's probably best to assign an initial value of null or -1.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Capturing
boolean debug = logger.isDebugEnabled();at the beginning of methods and only computing/using timing when debug is true.