You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@logging.apache.org by "zsewasdsdsd (via GitHub)" <gi...@apache.org> on 2026/10/14 13:30:31 UTC
[PR] Fix LocationAwareLogger bypassing configured MessageFactory [logging-log4j2]
zsewasdsdsd opened a new pull request, #4404:
URL: https://github.com/apache/logging-log4j2/pull/4404
## Summary
Fixes #4403.
`LocationAwareLogger.log()` in `log4j-slf4j-impl` previously constructed `SimpleMessage` and `ParameterizedMessage` directly, bypassing the logger's configured `MessageFactory`.
This change delegates message construction to the configured factory while preserving the supplied caller FQCN and explicit `Throwable` precedence.
## Changes
- Use the configured `MessageFactory` for location-aware logging.
- Handle both null and non-null parameter arrays.
- Preserve explicit exception precedence and caller information.
- Add regression tests covering custom message factories.
- Add a changelog entry for #4403.
## Verification
- 30 tests passed (29 regular tests + 1 loop-detection test).
- RED/GREEN regression verification completed.
- Spotless: passed.
- SpotBugs: 0 issues.
- Apache RAT: passed.
- Bnd Baseline: passed against 2.26.1.
- Changelog validation: passed.
- Maven install: BUILD SUCCESS (JDK 17).
--
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] Fix LocationAwareLogger bypassing configured MessageFactory [logging-log4j2]
Posted by "ramanathan1504 (via GitHub)" <gi...@apache.org>.
ramanathan1504 commented on PR #4404:
URL: https://github.com/apache/logging-log4j2/pull/4404#issuecomment-6300879448
Using the logger's `MessageFactory` in `LocationAwareLogger.log()` matches what `info()` and the other methods already do. Both new tests fail without the change.
With the default `ReusableMessageFactory` the message is never released, so it stays reserved and keeps the last params. Can we release it in a `finally` block as `AbstractLogger.logMessageSafely()` does, and fix the same code in `log4j-slf4j2-impl`'s `Log4jLogger`?
--
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] Fix LocationAwareLogger bypassing configured MessageFactory [logging-log4j2]
Posted by "ramanathan1504 (via GitHub)" <gi...@apache.org>.
ramanathan1504 commented on code in PR #4404:
URL: https://github.com/apache/logging-log4j2/pull/4404#discussion_r4330000371
##########
log4j-slf4j-impl/src/main/java/org/apache/logging/slf4j/Log4jLogger.java:
##########
@@ -23,8 +23,6 @@
import org.apache.logging.log4j.Level;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.message.Message;
Review Comment:
Needed by the `finally` below.
```suggestion
import org.apache.logging.log4j.message.Message;
import org.apache.logging.log4j.message.ReusableMessageFactory;
```
--
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] Fix LocationAwareLogger bypassing configured MessageFactory [logging-log4j2]
Posted by "ramanathan1504 (via GitHub)" <gi...@apache.org>.
ramanathan1504 commented on code in PR #4404:
URL: https://github.com/apache/logging-log4j2/pull/4404#discussion_r4330002165
##########
log4j-slf4j-impl/src/main/java/org/apache/logging/slf4j/Log4jLogger.java:
##########
@@ -363,14 +361,12 @@ public void log(
return;
}
final Message msg;
- final Throwable actualThrowable;
if (params == null) {
- msg = new SimpleMessage(message);
- actualThrowable = throwable;
+ msg = logger.getMessageFactory().newMessage(message);
} else {
- msg = new ParameterizedMessage(message, params, throwable);
- actualThrowable = throwable != null ? throwable : msg.getThrowable();
+ msg = logger.getMessageFactory().newMessage(message, params);
}
+ final Throwable actualThrowable = throwable != null ? throwable : msg.getThrowable();
logger.logMessage(fqcn, log4jLevel, log4jMarker, msg, actualThrowable);
Review Comment:
The default factory is `ReusableMessageFactory`, and `Logger.logMessage()` does not release the message. Without this the thread-local message stays reserved after the first call.
```suggestion
try {
logger.logMessage(fqcn, log4jLevel, log4jMarker, msg, actualThrowable);
} finally {
ReusableMessageFactory.release(msg);
}
```
##########
log4j-slf4j-impl/src/test/java/org/apache/logging/slf4j/LoggerTest.java:
##########
@@ -49,6 +56,77 @@ public LoggerTest(final LoggerContext context) {
this.logger = LoggerFactory.getLogger("LoggerTest");
}
+ @Test
+ void locationAwareLoggerUsesConfiguredMessageFactory() {
+ final String name = "LoggerTest.CustomMessageFactory";
+ final Log4jLogger customLogger = new Log4jLogger(
+ new Log4jMarkerFactory(), ctx.getLogger(name, StringFormatterMessageFactory.INSTANCE), name);
+
+ final ListAppender list = getAppenderByName("UnformattedList");
+ list.clear();
+
+ // Control: regular logging honors the configured message factory.
+ customLogger.info("value=%s", "normal");
+
+ assertEquals(1, list.getEvents().size());
+ assertEquals(
+ StringFormattedMessage.class,
+ list.getEvents().get(0).getMessage().getClass());
+ assertEquals("value=normal", list.getEvents().get(0).getMessage().getFormattedMessage());
+
+ list.clear();
+
+ // Regression: location-aware logging must use the same factory.
+ customLogger.log(
+ null,
+ LoggerTest.class.getName(),
+ LocationAwareLogger.INFO_INT,
+ "value=%s",
+ new Object[] {"aware"},
+ null);
+
+ assertEquals(1, list.getEvents().size());
+ assertEquals(
+ StringFormattedMessage.class,
+ list.getEvents().get(0).getMessage().getClass());
+ assertEquals("value=aware", list.getEvents().get(0).getMessage().getFormattedMessage());
+ }
+
+ @Test
+ void locationAwareLoggerUsesConfiguredMessageFactoryWithoutParameters() {
Review Comment:
No test reaches the `msg.getThrowable()` fallback or uses `ReusableMessageFactory`. Can we add one that passes a trailing `Throwable` in `params`, and one with `ReusableMessageFactory` that checks the message is released?
--
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