apache / apache/logging-log4j2

Classes should no longer be specified via system properties

Open
#1,977 3 comments 1 reaction 1 assignee Claimed by @jvz View on GitHub
api configuration
Dominant language
Java
Stars
3.6k
Forks
1.7k
Avg merge
21h 30m
Merged PRs (30d)
27

Description

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 `ClassLoader` to 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 use `java.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::load` and any other implicit `ClassLoader` usage.

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 `Provider` service class which grouped some of these things together.

* `org.apache.logging.log4j.spi.LoggerContextFactory`
* `org.apache.logging.log4j.spi.ThreadContextMap`
* `org.apache.logging.log4j.spi.ThreadContextStack`
* `org.apache.logging.log4j.message.MessageFactory`
* `org.apache.logging.log4j.message.FlowMessageFactory`

There 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` - AsyncLogger-specific exception handler
* `com.lmax.disruptor.ExceptionHandler` - AsyncLoggerContext-specific exception handler
* `org.apache.logging.log4j.core.async.AsyncQueueFullPolicy`
* `org.apache.logging.log4j.core.async.AsyncWaitStrategyFactory`
* `org.apache.logging.log4j.core.util.AuthorizationProvider`
* `org.apache.logging.log4j.core.time.Clock`
* `org.apache.logging.log4j.core.config.ConfigurationFactory`
* `org.apache.logging.log4j.core.config.ReliabilityStrategy`
* `org.apache.logging.log4j.core.ContextDataInjector`
* Reflective instantiation of a `org.apache.logging.log4j.util.StringMap` for `ContextDataFactory`

## Other

There are a few standalone modules that use `LoaderUtil::loadClass` for things that can be ported to use DI directly. There are some other places still using `LoaderUtil` or `Loader` that should be cleaned up alongside all this.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.