-
Notifications
You must be signed in to change notification settings - Fork 131
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
Add support for Log4j API and SLF4J #177
Merged
Merged
Conversation
This file contains 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
The logging backends supported by the default `LogFactoryImpl` have all reached their end-of-life (except JUL and `SimpleLog`). Third-generation logging backends, such as Log4j Core, support multiple logger contexts per application and therefore can not be handled by the simplified caching mechanism in `LogFactoryImpl`. This PR introduces two new `LogFactory` implementations, `Log4j2LogFactory` and `Slf4jLogFactory`, that forward messages to the Log4j API and SLF4J. During initialization the three existing factories are checked in the following order: 1. the `Log4j2LogFactory` has highest priority, since the Log4j API can faithfully transmit messages of type `Object`, 2. the `Slf4jLogFactory` is the next choice. However, if the Log4j-to-SLF4J bridge is present we log directly through SLF4J instead of the Log4j API. 3. the legacy `LogFactoryImpl` has lowest priority.
Could you take a look at this? |
@ppkarwasz |
TY @ppkarwasz ! |
garydgregory
added a commit
that referenced
this pull request
Nov 3, 2023
This was referenced Mar 15, 2024
Closed
ctubbsii
added a commit
to ctubbsii/accumulo
that referenced
this pull request
Jul 16, 2024
* Bump commons-logging to 1.3, so it natively supports sending logs to slf4j or log4j without the log4j-jcl bridge See apache/commons-logging#177 * Fix issue described in apache#4558 (comment) regarding log4j-api being required for log4j-1.2-api in core/pom.xml * Fix some false-positive Closeable resource warnings from wrapping a Scanner with an IsolatedScanner * Suppress some deprecation warnings for scan server metadata schema * Remove log4j-1.2-api from minicluster and compactor modules, where they weren't used * Fix some incorrect `@SuppressWarnings` comments * Fix generics with the ServiceStatusCmd's Result class, which had a generic `A extends Integer` parameter, but `Integer` is final and cannot be extended * Remove some unused variables * Add a few comments in the POMs to explain why some log4j dependencies exist where they do * Add a try-with-resources block in ClientSideIteratorIT to ensure the scanner is closed when done
ctubbsii
added a commit
to ctubbsii/accumulo
that referenced
this pull request
Jul 17, 2024
* Bump commons-logging to 1.3, so it natively supports sending logs to slf4j or log4j without the log4j-jcl bridge See apache/commons-logging#177 * Fix issue described in apache#4558 (comment) regarding log4j-api being required for log4j-1.2-api in core/pom.xml by making sure bnd and log4j-api are in the compile scope but marked as provided to avoid the maven-dependency-plugin from complaining because they appear to be required only for the test, but actually are still needed transitively for log4j-1.2-api * Fix some false-positive Closeable resource warnings from wrapping a Scanner with an IsolatedScanner * Suppress some deprecation warnings for scan server metadata schema * Remove log4j-1.2-api from minicluster and compactor modules, where they weren't used * Fix some incorrect `@SuppressWarnings` comments * Fix generics with the ServiceStatusCmd's Result class, which had a generic `A extends Integer` parameter, but `Integer` is final and cannot be extended * Remove some unused variables * Add a few comments in the POMs to explain why some log4j dependencies exist where they do * Add a try-with-resources block in ClientSideIteratorIT to ensure the scanner is closed when done * Remove an unneeded exclusion on httpclient from libthrift (it's not a transitive dependency in the POM for the version of libthrift we are using, and upstream they have already migrated to newer versions of httpclient5 in the latest POM for libthrift)
ctubbsii
added a commit
to apache/accumulo
that referenced
this pull request
Jul 18, 2024
* Bump commons-logging to 1.3, so it natively supports sending logs to slf4j or log4j without the log4j-jcl bridge See apache/commons-logging#177 * Fix issue described in #4558 (comment) regarding log4j-api being required for log4j-1.2-api in core/pom.xml by making sure bnd and log4j-api are in the compile scope but marked as provided to avoid the maven-dependency-plugin from complaining because they appear to be required only for the test, but actually are still needed transitively for log4j-1.2-api * Fix some false-positive Closeable resource warnings from wrapping a Scanner with an IsolatedScanner * Suppress some deprecation warnings for scan server metadata schema * Remove log4j-1.2-api from minicluster and compactor modules, where they weren't used * Fix some incorrect `@SuppressWarnings` comments * Fix generics with the ServiceStatusCmd's Result class, which had a generic `A extends Integer` parameter, but `Integer` is final and cannot be extended * Remove some unused variables * Add a few comments in the POMs to explain why some log4j dependencies exist where they do * Add a try-with-resources block in ClientSideIteratorIT to ensure the scanner is closed when done * Remove an unneeded exclusion on httpclient from libthrift (it's not a transitive dependency in the POM for the version of libthrift we are using, and upstream they have already migrated to newer versions of httpclient5 in the latest POM for libthrift)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
The logging backends supported by the default
LogFactoryImpl
have all reached their end-of-life (except JUL andSimpleLog
). Third-generation logging backends, such as Log4j Core, support multiple logger contexts per application and therefore can not be handled by the simplified caching mechanism inLogFactoryImpl
.This PR introduces two new
LogFactory
implementations,Log4j2LogFactory
andSlf4jLogFactory
, that forward messages to the Log4j API and SLF4J respectively.During initialization the three existing factories are checked in the following order:
Log4j2LogFactory
has highest priority, since the Log4j API can faithfully transmit messages of typeObject
,Slf4jLogFactory
is the next choice. However, if the Log4j-to-SLF4J bridge is present we log directly to SLF4J instead of the Log4j API.LogFactoryImpl
has lowest priority.All the messages produced by loggers from
Log4j2LogFactory
andSlf4jLogFactory
are marked with aCOMMONS-LOGGING
marker.This PR also marks the old
Log4jLogger
class as deprecated.