-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
LOG4J2-3472 support custom WaitStrategy configuration #824
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
LOG4J2-3472 support custom WaitStrategy configuration #824
Conversation
log4j-core/src/main/java/org/apache/logging/log4j/core/async/AsyncLoggerConfigDisruptor.java
Show resolved
Hide resolved
log4j-core/src/main/java/org/apache/logging/log4j/core/async/AsyncLoggerDisruptor.java
Outdated
Show resolved
Hide resolved
log4j-core/src/main/java/org/apache/logging/log4j/core/async/AsyncWaitStrategyFactory.java
Show resolved
Hide resolved
...-core/src/main/java/org/apache/logging/log4j/core/async/DefaultAsyncWaitStrategyFactory.java
Show resolved
Hide resolved
log4j-core/src/main/java/org/apache/logging/log4j/core/config/Configuration.java
Show resolved
Hide resolved
...pache/logging/log4j/core/async/AsyncWaitStrategyFactoryIncorrectConfigGlobalLoggersTest.java
Outdated
Show resolved
Hide resolved
...pache/logging/log4j/core/async/AsyncWaitStrategyFactoryIncorrectConfigGlobalLoggersTest.java
Outdated
Show resolved
Hide resolved
log4j-core/src/test/resources/AsyncWaitStrategyFactoryConfigGlobalLoggerTest.xml
Outdated
Show resolved
Hide resolved
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.
Hi @remkop
I scattered some comments.
@garydgregory Awesome fast feedback Gary! Super appreciated! |
@remkop |
@garydgregory Thanks for your replies! Are you okay with the PR as it is now? (The github review status still says "changes requested"...) |
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.
LGTM.
...re/src/test/java/org/apache/logging/log4j/core/async/AsyncWaitStrategyFactoryConfigTest.java
Outdated
Show resolved
Hide resolved
...j-core/src/main/java/org/apache/logging/log4j/core/async/AsyncWaitStrategyFactoryConfig.java
Outdated
Show resolved
Hide resolved
@carterkozak and @ppkarwasz Thank you for your reviews! |
Rats, it turns out that I need to add the license text to some files: log4j-core/src/main/java/org/apache/logging/log4j/core/async/AsyncWaitStrategyFactoryConfig.java I will do that when I get home later today. |
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.
LGTM
This PR implements the change discussed in https://issues.apache.org/jira/browse/LOG4J2-3472:
Allow the configuration of custom
WaitStrategy
implementations by specifying aorg.apache.logging.log4j.core.async.AsyncWaitStrategyFactory
implementation.Configuration looks similar to the below: