You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@logging.apache.org by "ppkarwasz (via GitHub)" <gi...@apache.org> on 2024/03/14 11:28:53 UTC

[PR] Add Logback's throwable-consuming semantics as an option (logging-log4j2)

ppkarwasz opened a new pull request, #2381:
URL: https://github.com/apache/logging-log4j2/pull/2381

   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
   ```
   
   In this PR two choices have been made:
   
   - the message factory is only included in `log4j-slf4j-impl` and `log4j-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 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 new `log4j-message` artifact for this.
   - no **garbage-free** version of this factory is provided. SLF4J creates temporary arrays whenever **more than 2** arguments are provided. I believe that an additional creation of a `ParameterizeMessage` instance does not make a difference.
   
   Closes #2363 
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add Logback's throwable-consuming semantics as an option (logging-log4j2)

Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz merged PR #2381:
URL: https://github.com/apache/logging-log4j2/pull/2381


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add Logback's throwable-consuming semantics as an option (logging-log4j2)

Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on code in PR #2381:
URL: https://github.com/apache/logging-log4j2/pull/2381#discussion_r1662583010


##########
src/changelog/.2.x.x/2363_add_logback_throwable_consuming_semantics.xml:
##########
@@ -1,8 +1,8 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <entry xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
-       xmlns="http://logging.apache.org/log4j/changelog"
-       xsi:schemaLocation="http://logging.apache.org/log4j/changelog https://logging.apache.org/log4j/changelog-0.1.3.xsd"
-       type="added">
+       xmlns="https://logging.apache.org/xml/ns"
+       xsi:schemaLocation="https://logging.apache.org/xml/ns https://logging.apache.org/xml/ns/log4j-changelog-0.xsd"
+       type="changed">

Review Comment:
   Nice catch! It is a copy/paste error.
   
   Fixed in 42b5ebbb41b4b23f665d395833a10c431a2a602f.



##########
src/site/antora/modules/ROOT/examples/migrate-from-logback/MigrateFromLogback.java:
##########
@@ -0,0 +1,28 @@
+package example;
+
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+
+class MigrateFromLogback {
+    private static Logger logger = LogManager.getLogger();
+
+    doLogWrong() {
+        try {
+            // do something
+            // tag::wrong[]
+        } catch (Exception e) {

Review Comment:
   Fixed in 42b5ebbb41b4b23f665d395833a10c431a2a602f.



##########
src/site/antora/modules/ROOT/pages/migrate-from-logback.adoc:
##########
@@ -106,3 +106,62 @@ To assist with migrating Logback configuration components to Log4j Core, see the
 * xref:manual/filters.adoc[]
 
 For the complete list of all Log4j configuration knobs, see xref:manual/configuration.adoc[the Configuration page].
+
+[#parameterized-logging]
+=== Parameterized logging
+
+A common mistake in parameterized logging is to add a `{}` placeholder for the exception associated with a log event:
+
+[source,java,indent=0]
+----
+include::example$migrate-from-logback/MigrateFromLogback.java[tag=wrong]
+----
+
+Log4j Core and Logback differ in the way they treat this statement:
+
+Logback::
+Logback interprets the `e` argument as throwable and removes it from the list of parameters.
+We end up with a parameterized statement with one placeholder, but zero parameters.
+The placeholder therefore remains as is:
++
+[source]
+----
+The foo process exited with and error: {}
+java.lang.RuntimeException: Message
+    at example.MigrateFromLogback.doLogWrong(MigrateFromLogback.java:10)
+...
+----
+
+Log4j Core::
+Log4j Core first looks for the parameters of the message.
+Since the format string has one placeholder, the `e` argument is interpreted as a parameter of the log message.
+The throwable associated to the log event is `null`, which results in a missing stack trace:
++
+[source]
+----
+The foo process exited with and error: java.lang.RuntimeException: Message
+----
+
+To fix this problem and obtain the same output in both backends, you should remove the placeholder from the format string:
+
+[source,java,indent=0]
+----
+include::example$migrate-from-logback/MigrateFromLogback.java[tag=right]
+----
+
+After the change, the output will look us:
+
+[source]
+----
+The foo process exited with and error.
+java.lang.RuntimeException: Message
+    at example.MigrateFromLogback.doLogWrong(MigrateFromLogback.java:10)
+...
+----
+
+[TIP]
+====

Review Comment:
   Fixed in 42b5ebbb41b4b23f665d395833a10c431a2a602f.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org