Add Logback's throwable-consuming semantics as an option#2381
Merged
Conversation
The semantics used to determine the `Throwable` attached to a log event changed in Logback 1.1.0 (in the fix to [LOGBACK-873](https://jira.qos.ch/browse/LOGBACK-873)). When there are not enough arguments to the log call to fill both the throwable field and the placeholders contained in the message pattern, Logback 1.1.0 and later prefer to fill the throwable field and leave some placeholders empty. Log4j Core does the opposite choice: it first fills all the placeholders and only fills the throwable field if there is something left. This change allows `log4j-slf4j-impl` and `log4j-slf4j2-impl` users to switch between the two behaviors by setting the property `log4j2.messageFactory` to: ``` org.apache.logging.slf4j.message.ThrowableConsumingMessageFactory ```
Closed
vy
requested changes
Jul 1, 2024
Member
vy
left a comment
There was a problem hiding this comment.
LGTM, thanks for taking care of this @ppkarwasz! 💯
I have two nits:
- Would you mind explicitly stating in all newly added Java source files something like the following, please?
This class should be identical to its clone in {@code log4j-slf4j2-impl}.
Any changes made here should be reflected to the other one, and vice versa.
- Would you mind documenting this in the following files, please?
migrate-from-logback.adocinstallation.adoc#impl-logback
…add_logback_throwable_consuming_semantics
Contributor
Author
|
@vy,
I don't think this is necessary. Every time one of
I added some documentation to |
vy
reviewed
Jul 2, 2024
src/changelog/.2.x.x/2363_add_logback_throwable_consuming_semantics.xml
Outdated
Show resolved
Hide resolved
vy
reviewed
Jul 2, 2024
src/site/antora/modules/ROOT/examples/migrate-from-logback/MigrateFromLogback.java
Outdated
Show resolved
Hide resolved
vy
reviewed
Jul 2, 2024
vy
approved these changes
Jul 2, 2024
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 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.
The semantics used to determine the
Throwableattached to a log event changed in Logback 1.1.0 (in the fix to LOGBACK-873).When there are not enough arguments to the log call to fill both the throwable field and the placeholders contained in the message pattern, Logback 1.1.0 and later prefer to fill the throwable field and leave some placeholders empty. Log4j Core does the opposite choice: it first fills all the placeholders and only fills the throwable field if there is something left.
This change allows
log4j-slf4j-implandlog4j-slf4j2-implusers to switch between the two behaviors by setting the propertylog4j2.messageFactoryto:In this PR two choices have been made:
log4j-slf4j-implandlog4j-slf4j2-impl, since it should be mostly required by Logback users during a transition period. Note that Logback's and Log4j's semantics to deal with throwables agree whenever enough arguments are provided. In a second step users can modify their code using Add recipe to clone an exception argument openrewrite/rewrite-logging-frameworks#141, so that it behaves in the same way in Logback and Log4j Core out-of-the-box. However I can also create a newlog4j-messageartifact for this.ParameterizeMessageinstance does not make a difference.Closes #2363