This is an automated email from the ASF dual-hosted git repository.
pkarwasz pushed a commit to branch release-2.x
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
The following commit(s) were added to refs/heads/release-2.x by this push:
new 674dae6d55 [LOG4J2-3585] Fix throwable logging
674dae6d55 is described below
commit 674dae6d5596ce33473d8f4588128c155ba5d23c
Author: Piotr P. Karwasz <[email protected]>
AuthorDate: Thu Sep 1 00:31:34 2022 +0200
[LOG4J2-3585] Fix throwable logging
An inverted condition prevented throwables to be added as additional
arguments.
---
.../java/org/apache/logging/slf4j/Log4jLogger.java | 11 +++---
.../java/org/apache/logging/slf4j/LoggerTest.java | 42 +++++++++++++++++++++-
.../src/test/resources/log4j-test1.xml | 2 ++
.../java/org/apache/logging/slf4j/Log4jLogger.java | 10 +++---
.../java/org/apache/logging/slf4j/LoggerTest.java | 42 +++++++++++++++++++++-
.../src/test/resources/log4j-test1.xml | 2 ++
6 files changed, 97 insertions(+), 12 deletions(-)
diff --git
a/log4j-slf4j-impl/src/main/java/org/apache/logging/slf4j/Log4jLogger.java
b/log4j-slf4j-impl/src/main/java/org/apache/logging/slf4j/Log4jLogger.java
index 1fa808065d..9b970d4a68 100644
--- a/log4j-slf4j-impl/src/main/java/org/apache/logging/slf4j/Log4jLogger.java
+++ b/log4j-slf4j-impl/src/main/java/org/apache/logging/slf4j/Log4jLogger.java
@@ -355,7 +355,7 @@ public class Log4jLogger implements LocationAwareLogger,
Serializable {
}
@Override
- public void log(final Marker marker, final String fqcn, final int level,
final String message, final Object[] params, Throwable throwable) {
+ public void log(final Marker marker, final String fqcn, final int level,
final String message, final Object[] params, final Throwable throwable) {
final Level log4jLevel = getLevel(level);
final org.apache.logging.log4j.Marker log4jMarker = getMarker(marker);
@@ -363,17 +363,18 @@ public class Log4jLogger implements LocationAwareLogger,
Serializable {
return;
}
final Message msg;
+ final Throwable actualThrowable;
if (CONVERTER != null && eventLogger && marker != null &&
marker.contains(EVENT_MARKER)) {
msg = CONVERTER.convertEvent(message, params, throwable);
+ actualThrowable = throwable != null ? throwable :
msg.getThrowable();
} else if (params == null) {
msg = new SimpleMessage(message);
+ actualThrowable = throwable;
} else {
msg = new ParameterizedMessage(message, params, throwable);
- if (throwable != null) {
- throwable = msg.getThrowable();
- }
+ actualThrowable = throwable != null ? throwable :
msg.getThrowable();
}
- logger.logMessage(fqcn, log4jLevel, log4jMarker, msg, throwable);
+ logger.logMessage(fqcn, log4jLevel, log4jMarker, msg, actualThrowable);
}
private static org.apache.logging.log4j.Marker getMarker(final Marker
marker) {
diff --git
a/log4j-slf4j-impl/src/test/java/org/apache/logging/slf4j/LoggerTest.java
b/log4j-slf4j-impl/src/test/java/org/apache/logging/slf4j/LoggerTest.java
index 05240747d0..5a9f83729f 100644
--- a/log4j-slf4j-impl/src/test/java/org/apache/logging/slf4j/LoggerTest.java
+++ b/log4j-slf4j-impl/src/test/java/org/apache/logging/slf4j/LoggerTest.java
@@ -23,6 +23,7 @@ import static org.junit.Assert.assertTrue;
import java.util.List;
import java.util.Locale;
+import org.apache.logging.log4j.core.LogEvent;
import org.apache.logging.log4j.junit.LoggerContextRule;
import org.apache.logging.log4j.test.appender.ListAppender;
import org.apache.logging.log4j.util.Strings;
@@ -162,9 +163,38 @@ public class LoggerTest {
verify("EventLogger", "o.a.l.s.LoggerTest Transfer [Audit@18060
Amount=\"200.00\" FromAccount=\"123457\" ToAccount=\"123456\"] Transfer
Complete" + Strings.LINE_SEPARATOR);
}
- private void verify(final String name, final String expected) {
+ @Test
+ public void testThrowable() {
+ final Throwable expected = new RuntimeException();
+ logger.debug("Hello {}", expected);
+ verifyThrowable(expected);
+ logger.debug("Hello {}", (Object) expected);
+ verifyThrowable(null);
+ logger.debug("Hello", expected);
+ verifyThrowable(expected);
+ logger.debug("Hello {}! {}", "World!", expected);
+ verifyThrowable(null);
+ logger.debug("Hello {}!", "World!", expected);
+ verifyThrowable(expected);
+ final LocationAwareLogger lal = (LocationAwareLogger) logger;
+ lal.log(null, LoggerTest.class.getName(),
LocationAwareLogger.DEBUG_INT, "Hello {}", null, expected);
+ verifyThrowable(expected);
+ lal.log(null, LoggerTest.class.getName(),
LocationAwareLogger.DEBUG_INT, "Hello {}", new Object[] { expected },
+ null);
+ verifyThrowable(null);
+ lal.log(null, LoggerTest.class.getName(),
LocationAwareLogger.DEBUG_INT, "Hello {}",
+ new Object[] { "World!", expected }, null);
+ verifyThrowable(expected);
+ }
+
+ private ListAppender getAppenderByName(final String name) {
final ListAppender listApp = ctx.getListAppender(name);
assertNotNull("Missing Appender", listApp);
+ return listApp;
+ }
+
+ private void verify(final String name, final String expected) {
+ final ListAppender listApp = getAppenderByName(name);
final List<String> events = listApp.getMessages();
assertTrue("Incorrect number of messages. Expected 1 Actual " +
events.size(), events.size()== 1);
final String actual = events.get(0);
@@ -172,11 +202,21 @@ public class LoggerTest {
listApp.clear();
}
+ private void verifyThrowable(final Throwable expected) {
+ final ListAppender listApp = getAppenderByName("UnformattedList");
+ final List<LogEvent> events = listApp.getEvents();
+ assertEquals("Incorrect number of messages", 1, events.size());
+ final LogEvent actual = events.get(0);
+ assertEquals("Incorrect throwable.", expected, actual.getThrown());
+ listApp.clear();
+ }
+
@Before
@After
public void cleanup() {
MDC.clear();
ctx.getListAppender("List").clear();
+ ctx.getListAppender("UnformattedList").clear();
ctx.getListAppender("EventLogger").clear();
}
}
diff --git a/log4j-slf4j-impl/src/test/resources/log4j-test1.xml
b/log4j-slf4j-impl/src/test/resources/log4j-test1.xml
index a64bdfa905..1ba09ca893 100644
--- a/log4j-slf4j-impl/src/test/resources/log4j-test1.xml
+++ b/log4j-slf4j-impl/src/test/resources/log4j-test1.xml
@@ -20,6 +20,7 @@
<List name="List">
<PatternLayout pattern="%C{1.} %m MDC%X%n%ex{0}"/>
</List>
+ <List name="UnformattedList" />
<SLF4J name="SLF4J"/>
</Appenders>
@@ -34,6 +35,7 @@
<Root level="trace">
<AppenderRef ref="List"/>
+ <AppenderRef ref="UnformattedList"/>
</Root>
</Loggers>
diff --git
a/log4j-slf4j20-impl/src/main/java/org/apache/logging/slf4j/Log4jLogger.java
b/log4j-slf4j20-impl/src/main/java/org/apache/logging/slf4j/Log4jLogger.java
index 10ad49ca7e..fd42f361fc 100644
--- a/log4j-slf4j20-impl/src/main/java/org/apache/logging/slf4j/Log4jLogger.java
+++ b/log4j-slf4j20-impl/src/main/java/org/apache/logging/slf4j/Log4jLogger.java
@@ -349,7 +349,7 @@ public class Log4jLogger implements LocationAwareLogger,
Serializable {
}
@Override
- public void log(final Marker marker, final String fqcn, final int level,
final String message, final Object[] params, Throwable throwable) {
+ public void log(final Marker marker, final String fqcn, final int level,
final String message, final Object[] params, final Throwable throwable) {
final Level log4jLevel = getLevel(level);
final org.apache.logging.log4j.Marker log4jMarker = getMarker(marker);
@@ -357,15 +357,15 @@ public class Log4jLogger implements LocationAwareLogger,
Serializable {
return;
}
final Message msg;
+ final Throwable actualThrowable;
if (params == null) {
msg = new SimpleMessage(message);
+ actualThrowable = throwable;
} else {
msg = new ParameterizedMessage(message, params, throwable);
- if (throwable != null) {
- throwable = msg.getThrowable();
- }
+ actualThrowable = throwable != null ? throwable :
msg.getThrowable();
}
- logger.logMessage(fqcn, log4jLevel, log4jMarker, msg, throwable);
+ logger.logMessage(fqcn, log4jLevel, log4jMarker, msg, actualThrowable);
}
private org.apache.logging.log4j.Marker getMarker(final Marker marker) {
diff --git
a/log4j-slf4j20-impl/src/test/java/org/apache/logging/slf4j/LoggerTest.java
b/log4j-slf4j20-impl/src/test/java/org/apache/logging/slf4j/LoggerTest.java
index 7c8e71379f..1701bbcbc7 100644
--- a/log4j-slf4j20-impl/src/test/java/org/apache/logging/slf4j/LoggerTest.java
+++ b/log4j-slf4j20-impl/src/test/java/org/apache/logging/slf4j/LoggerTest.java
@@ -22,6 +22,7 @@ import static org.junit.Assert.assertTrue;
import java.util.List;
+import org.apache.logging.log4j.core.LogEvent;
import org.apache.logging.log4j.junit.LoggerContextRule;
import org.apache.logging.log4j.test.appender.ListAppender;
import org.apache.logging.log4j.util.Strings;
@@ -142,9 +143,38 @@ public class LoggerTest {
verify("List", "o.a.l.s.LoggerTest Hello, Log4j {} MDC{}" +
Strings.LINE_SEPARATOR);
}
- private void verify(final String name, final String expected) {
+ @Test
+ public void testThrowable() {
+ final Throwable expected = new RuntimeException();
+ logger.debug("Hello {}", expected);
+ verifyThrowable(expected);
+ logger.debug("Hello {}", (Object) expected);
+ verifyThrowable(null);
+ logger.debug("Hello", expected);
+ verifyThrowable(expected);
+ logger.debug("Hello {}! {}", "World!", expected);
+ verifyThrowable(null);
+ logger.debug("Hello {}!", "World!", expected);
+ verifyThrowable(expected);
+ final LocationAwareLogger lal = (LocationAwareLogger) logger;
+ lal.log(null, LoggerTest.class.getName(),
LocationAwareLogger.DEBUG_INT, "Hello {}", null, expected);
+ verifyThrowable(expected);
+ lal.log(null, LoggerTest.class.getName(),
LocationAwareLogger.DEBUG_INT, "Hello {}", new Object[] { expected },
+ null);
+ verifyThrowable(null);
+ lal.log(null, LoggerTest.class.getName(),
LocationAwareLogger.DEBUG_INT, "Hello {}",
+ new Object[] { "World!", expected }, null);
+ verifyThrowable(expected);
+ }
+
+ private ListAppender getAppenderByName(final String name) {
final ListAppender listApp = ctx.getListAppender(name);
assertNotNull("Missing Appender", listApp);
+ return listApp;
+ }
+
+ private void verify(final String name, final String expected) {
+ final ListAppender listApp = getAppenderByName(name);
final List<String> events = listApp.getMessages();
assertTrue("Incorrect number of messages. Expected 1 Actual " +
events.size(), events.size()== 1);
final String actual = events.get(0);
@@ -152,10 +182,20 @@ public class LoggerTest {
listApp.clear();
}
+ private void verifyThrowable(final Throwable expected) {
+ final ListAppender listApp = getAppenderByName("UnformattedList");
+ final List<LogEvent> events = listApp.getEvents();
+ assertEquals("Incorrect number of messages", 1, events.size());
+ final LogEvent actual = events.get(0);
+ assertEquals("Incorrect throwable.", expected, actual.getThrown());
+ listApp.clear();
+ }
+
@Before
@After
public void cleanup() {
MDC.clear();
ctx.getListAppender("List").clear();
+ ctx.getListAppender("UnformattedList").clear();
}
}
diff --git a/log4j-slf4j20-impl/src/test/resources/log4j-test1.xml
b/log4j-slf4j20-impl/src/test/resources/log4j-test1.xml
index 07a2be6316..2dee2c1153 100644
--- a/log4j-slf4j20-impl/src/test/resources/log4j-test1.xml
+++ b/log4j-slf4j20-impl/src/test/resources/log4j-test1.xml
@@ -17,6 +17,7 @@
<List name="List">
<PatternLayout pattern="%C{1.} %m MDC%X%n%ex{0}"/>
</List>
+ <List name="UnformattedList" />
<SLF4J name="SLF4J"/>
</Appenders>
@@ -27,6 +28,7 @@
<Root level="trace">
<AppenderRef ref="List"/>
+ <AppenderRef ref="UnformattedList"/>
</Root>
</Loggers>
