Centralize initialization in Provider class #2374
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
ppkarwasz added a commit that referenced this pull request Mar 13, 2024
9 tasks
vy reviewed Mar 14, 2024
log4j-api-test/src/test/java/org/apache/logging/log4j/spi/ThreadContextMapTest.java Outdated Show resolved Hide resolved
log4j-api/src/main/java/org/apache/logging/log4j/spi/DefaultThreadContextMap.java Show resolved Hide resolved
log4j-api/src/main/java/org/apache/logging/log4j/spi/DefaultThreadContextMap.java Outdated Show resolved Hide resolved
log4j-api/src/main/java/org/apache/logging/log4j/spi/Provider.java Outdated Show resolved Hide resolved
log4j-api/src/main/java/org/apache/logging/log4j/spi/Provider.java Outdated Show resolved Hide resolved
log4j-api/src/main/java/org/apache/logging/log4j/spi/Provider.java Outdated Show resolved Hide resolved
log4j-api/src/main/java/org/apache/logging/log4j/spi/Provider.java Outdated Show resolved Hide resolved
log4j-api/src/main/java/org/apache/logging/log4j/spi/Provider.java Outdated Show resolved Hide resolved
log4j-api/src/main/java/org/apache/logging/log4j/spi/Provider.java Outdated Show resolved Hide resolved
log4j-api/src/main/java/org/apache/logging/log4j/util/ProviderUtil.java Outdated Show resolved Hide resolved
ppkarwasz added a commit that referenced this pull request Mar 19, 2024
ppkarwasz added a commit that referenced this pull request Mar 19, 2024
ppkarwasz force-pushed the feature/provider-revamp branch from 2b7448f to 8c5ffb7 Compare ppkarwasz added a commit that referenced this pull request Mar 19, 2024
ppkarwasz force-pushed the feature/provider-revamp branch from 8c5ffb7 to ed4c72e Compare 
We move the code responsible for the instantiation of `LoggerContextFactory` and `ThreadContextMap` from the static entry points to the logging system (`LogManager` and `ThreadContext`) to the `Provider` class. The `Provider` class is instantiated using `ServiceLoader`, so `log4j-core` 2.x and 3.x can reimplement the initialization process according to their own rules. E.g. `log4j-core` 3.x can use the DI to create an instance of `LoggerContextFactory` and `ThreadContextMap`. The following modification were performed: * a **new** system property `log4j.provider` was introduced, * the old `log4j2.loggerContextFactory` has been deprecated and revised: if set it selects the first provider that uses the given `LoggerContextFactory`. Therefore it selects now both the context factory and thread context map implementations, * private static configuration values were removed from `ThreadContextMap` implementations, helping test parallelisation, * a distinct `NoOpThreadContextStack` implementation has been introduced.
ppkarwasz force-pushed the feature/provider-revamp branch from ed4c72e to 3cfb7b5 Compare 
| I apologize for taking so long to look at this but while working on moving context related stuff to log4j-context-data I ran into this and it makes things much more difficult as it hard-wires a bunch of classes that should be internal/private into the Provider class. I would request that those changes be undone. |
| Those internal/private classes are hardwired in Once I'll promote the
|
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit. This suggestion is invalid because no changes were made to the code. Suggestions cannot be applied while the pull request is closed. Suggestions cannot be applied while viewing a subset of changes. Only one suggestion per line can be applied in a batch. Add this suggestion to a batch that can be applied as a single commit. Applying suggestions on deleted lines is not supported. You must change the existing code in this line in order to create a valid suggestion. Outdated suggestions cannot be applied. This suggestion has been applied or marked resolved. Suggestions cannot be applied from pending reviews. Suggestions cannot be applied on multi-line comments. Suggestions cannot be applied while the pull request is queued to merge. Suggestion cannot be applied right now. Please check back later.
We move the code responsible for the instantiation of
LoggerContextFactoryandThreadContextMapfrom the static entry points to the logging system (LogManagerandThreadContext) to theProviderclass.The
Providerclass is instantiated usingServiceLoader, solog4j-core2.x and 3.x can reimplement the initialization process according to their own rules. E.g.log4j-core3.x can use the DI to create an instance ofLoggerContextFactoryandThreadContextMap.The following modification were performed:
log4j.providerwas introduced,log4j2.loggerContextFactoryhas been deprecated and revised: if set it selects the first provider that uses the givenLoggerContextFactory. Therefore it selects now both the context factory and thread context map implementations,ThreadContextMapimplementations, helping test parallelisation,NoOpThreadContextStackimplementation has been introduced.This PR only includes tests for the first two items. The remaining points should be covered by existing tests.