Open jvz opened 10 months ago
jvz
commented
9 months ago So far, I've managed to migrate most of these things. I had to disable some tests for now, though, as they significantly abuse internal details of the legacy way of doing things in order to work.
jvz
commented
9 months ago Although some of them might work properly after I try to figure out yet another architecture for the JUnit 5 extensions since splitting them up into multiple annotations doesn't seem to work too well for some reason.
jvz
commented
8 months ago Extracted https://github.com/apache/logging-log4j2/issues/2204 from work done in previous attempt at this issue.
Over time, we collected many places in the codebase that accept an override by specifying a class name through a system property. These typically worked by guessing at an appropriate
ClassLoaderto use to load the class and then using reflection to instantiate it. This pattern is problematic in module systems like the Java module system and OSGi due to stronger encapsulation guarantees. As we've already implemented for plugins, anything else remaining that customizes class selection based on system properties should be updated to usejava.util.ServiceLoader(when customizing in log4j-api) or specified in bundle classes installed via a service loader (which is also where some of the old system properties customizations are currently supported and can be simplified).In general, the goal of this is to eliminate the use of
LoaderUtil::loadand any other implicitClassLoaderusage.For some concrete examples, as I've worked on this, I've found the following interfaces that can be specified by system properties which are generally being refactored into what I said above.
Log4j API
The following interfaces are being loaded through service loader provider classes. These are similar in a way to the previous iteration of the
Providerservice class which grouped some of these things together.org.apache.logging.log4j.spi.LoggerContextFactoryorg.apache.logging.log4j.spi.ThreadContextMaporg.apache.logging.log4j.spi.ThreadContextStackorg.apache.logging.log4j.message.MessageFactoryorg.apache.logging.log4j.message.FlowMessageFactoryThere are some other service loader classes in the API, but they're either new or were already using service loading.
Log4j Core
The following interfaces either allow for overriding a default using a system property or are only configured through a system property. These should be updated to use bindings (or service loaders if that's not possible).
com.lmax.disruptor.ExceptionHandler<AsyncLoggerConfigDisruptor.Log4jEventWrapper>- AsyncLogger-specific exception handlercom.lmax.disruptor.ExceptionHandler<RingBufferLogEvent>- AsyncLoggerContext-specific exception handlerorg.apache.logging.log4j.core.async.AsyncQueueFullPolicyorg.apache.logging.log4j.core.async.AsyncWaitStrategyFactoryorg.apache.logging.log4j.core.util.AuthorizationProviderorg.apache.logging.log4j.core.time.Clockorg.apache.logging.log4j.core.config.ConfigurationFactoryorg.apache.logging.log4j.core.config.ReliabilityStrategyorg.apache.logging.log4j.core.ContextDataInjectororg.apache.logging.log4j.util.StringMapforContextDataFactoryOther
There are a few standalone modules that use
LoaderUtil::loadClassfor things that can be ported to use DI directly. There are some other places still usingLoaderUtilorLoaderthat should be cleaned up alongside all this.