From f135c0f30aac8d6b8dc0a003a6c5867913f5cd60 Mon Sep 17 00:00:00 2001 From: Carter Kozak Date: Tue, 3 Jul 2018 11:13:34 -0400 Subject: [PATCH] LOG4J2-2312 LOG4J2-2341 Fix jackson layout with async loggers AsyncLoggerContextSelector RingBufferLogEvents were not properly handled by the jackson layout. Currently the jackson layout implementation requires Log4jLogEvent instances, so we must convert all other LogEvent implementations into these. This closes #187 --- log4j-layout-jackson-json/pom.xml | 5 +++ .../jackson/json/layout/JsonLayoutTest.java | 37 +++++++++++++++++++ .../log4j/jackson/AbstractJacksonLayout.java | 4 +- .../jackson/layout/AbstractJacksonLayout.java | 4 +- src/changes/changes.xml | 6 +++ 5 files changed, 52 insertions(+), 4 deletions(-) diff --git a/log4j-layout-jackson-json/pom.xml b/log4j-layout-jackson-json/pom.xml index 962f4ac2ed1..6c971680268 100644 --- a/log4j-layout-jackson-json/pom.xml +++ b/log4j-layout-jackson-json/pom.xml @@ -55,6 +55,11 @@ test-jar ${project.version} + + com.lmax + disruptor + test + diff --git a/log4j-layout-jackson-json/src/test/java/org/apache/logging/log4j/jackson/json/layout/JsonLayoutTest.java b/log4j-layout-jackson-json/src/test/java/org/apache/logging/log4j/jackson/json/layout/JsonLayoutTest.java index 6aa5f730e1f..d477d2391dd 100644 --- a/log4j-layout-jackson-json/src/test/java/org/apache/logging/log4j/jackson/json/layout/JsonLayoutTest.java +++ b/log4j-layout-jackson-json/src/test/java/org/apache/logging/log4j/jackson/json/layout/JsonLayoutTest.java @@ -16,6 +16,8 @@ */ package org.apache.logging.log4j.jackson.json.layout; +import static org.hamcrest.CoreMatchers.containsString; +import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNull; @@ -33,12 +35,15 @@ import org.apache.logging.log4j.core.BasicConfigurationFactory; import org.apache.logging.log4j.core.Logger; import org.apache.logging.log4j.core.LoggerContext; +import org.apache.logging.log4j.core.async.RingBufferLogEvent; import org.apache.logging.log4j.core.config.Configuration; import org.apache.logging.log4j.core.config.ConfigurationFactory; import org.apache.logging.log4j.core.impl.Log4jLogEvent; import org.apache.logging.log4j.core.impl.MutableLogEvent; import org.apache.logging.log4j.core.layout.LogEventFixtures; import org.apache.logging.log4j.core.lookup.JavaLookup; +import org.apache.logging.log4j.core.time.internal.DummyNanoClock; +import org.apache.logging.log4j.core.time.internal.SystemClock; import org.apache.logging.log4j.core.util.KeyValuePair; import org.apache.logging.log4j.jackson.AbstractJacksonLayout; import org.apache.logging.log4j.jackson.json.Log4jJsonObjectMapper; @@ -49,6 +54,7 @@ import org.apache.logging.log4j.message.SimpleMessage; import org.apache.logging.log4j.spi.AbstractLogger; import org.apache.logging.log4j.test.appender.ListAppender; +import org.apache.logging.log4j.util.SortedArrayStringMap; import org.apache.logging.log4j.util.Strings; import org.junit.AfterClass; import org.junit.Assert; @@ -505,6 +511,37 @@ public void testReusableLayoutMessageWithCurlyBraces() throws Exception { } } + // Test for LOG4J2-2312 LOG4J2-2341 + @Test + public void testLayoutRingBufferEventReusableMessageWithCurlyBraces() throws Exception { + final boolean propertiesAsList = false; + final AbstractJacksonLayout layout = JsonLayout.newBuilder() + .setLocationInfo(false) + .setProperties(false) + .setPropertiesAsList(propertiesAsList) + .setComplete(false) + .setCompact(true) + .setEventEol(false) + .setCharset(StandardCharsets.UTF_8) + .setIncludeStacktrace(true) + .build(); + Message message = ReusableMessageFactory.INSTANCE.newMessage("Testing {}", new TestObj()); + try { + RingBufferLogEvent ringBufferEvent = new RingBufferLogEvent(); + ringBufferEvent.setValues( + null, "a.B", null, "f.q.c.n", Level.DEBUG, message, + null, new SortedArrayStringMap(), ThreadContext.EMPTY_STACK, 1L, + "threadName", 1, null, new SystemClock(), new DummyNanoClock()); + final String str = layout.toSerializable(ringBufferEvent); + final String expectedMessage = "Testing " + TestObj.TO_STRING_VALUE; + assertThat(str, containsString("\"message\":\"" + expectedMessage + '"')); + final Log4jLogEvent actual = new Log4jJsonObjectMapper(propertiesAsList, true, false, false).readValue(str, Log4jLogEvent.class); + assertEquals(expectedMessage, actual.getMessage().getFormattedMessage()); + } finally { + ReusableMessageFactory.release(message); + } + } + static class TestObj { static final String TO_STRING_VALUE = "This is my toString {} with curly braces"; @Override diff --git a/log4j-layout-jackson/src/main/java/org/apache/logging/log4j/jackson/AbstractJacksonLayout.java b/log4j-layout-jackson/src/main/java/org/apache/logging/log4j/jackson/AbstractJacksonLayout.java index da66b9d3b0e..2e35b01cea9 100644 --- a/log4j-layout-jackson/src/main/java/org/apache/logging/log4j/jackson/AbstractJacksonLayout.java +++ b/log4j-layout-jackson/src/main/java/org/apache/logging/log4j/jackson/AbstractJacksonLayout.java @@ -26,7 +26,7 @@ import org.apache.logging.log4j.core.config.Configuration; import org.apache.logging.log4j.core.config.plugins.PluginBuilderAttribute; import org.apache.logging.log4j.core.config.plugins.PluginElement; -import org.apache.logging.log4j.core.impl.MutableLogEvent; +import org.apache.logging.log4j.core.impl.Log4jLogEvent; import org.apache.logging.log4j.core.layout.AbstractStringLayout; import org.apache.logging.log4j.core.lookup.StrSubstitutor; import org.apache.logging.log4j.core.util.KeyValuePair; @@ -219,7 +219,7 @@ private static LogEvent convertMutableToLog4jEvent(final LogEvent event) { // TODO Jackson-based layouts have certain filters set up for Log4jLogEvent. // TODO Need to set up the same filters for MutableLogEvent but don't know how... // This is a workaround. - return event instanceof MutableLogEvent ? ((MutableLogEvent) event).createMemento() : event; + return event instanceof Log4jLogEvent ? event : Log4jLogEvent.createMemento(event); } private static ResolvableKeyValuePair[] prepareAdditionalFields(final Configuration config, final KeyValuePair[] additionalFields) { diff --git a/log4j-layout-jackson/src/main/java/org/apache/logging/log4j/jackson/layout/AbstractJacksonLayout.java b/log4j-layout-jackson/src/main/java/org/apache/logging/log4j/jackson/layout/AbstractJacksonLayout.java index a468c1e2990..175a470fe0d 100644 --- a/log4j-layout-jackson/src/main/java/org/apache/logging/log4j/jackson/layout/AbstractJacksonLayout.java +++ b/log4j-layout-jackson/src/main/java/org/apache/logging/log4j/jackson/layout/AbstractJacksonLayout.java @@ -44,7 +44,7 @@ import org.apache.logging.log4j.core.config.Configuration; import org.apache.logging.log4j.core.config.plugins.PluginBuilderAttribute; import org.apache.logging.log4j.core.config.plugins.PluginElement; -import org.apache.logging.log4j.core.impl.MutableLogEvent; +import org.apache.logging.log4j.core.impl.Log4jLogEvent; import org.apache.logging.log4j.core.layout.AbstractStringLayout; import org.apache.logging.log4j.core.lookup.StrSubstitutor; import org.apache.logging.log4j.core.util.KeyValuePair; @@ -245,7 +245,7 @@ private static LogEvent convertMutableToLog4jEvent(final LogEvent event) { // TODO Jackson-based layouts have certain filters set up for Log4jLogEvent. // TODO Need to set up the same filters for MutableLogEvent but don't know how... // This is a workaround. - return event instanceof MutableLogEvent ? ((MutableLogEvent) event).createMemento() : event; + return event instanceof Log4jLogEvent ? event : Log4jLogEvent.createMemento(event); } private static ResolvableKeyValuePair[] prepareAdditionalFields(final Configuration config, final KeyValuePair[] additionalFields) { diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 439fdc64cdf..da134c918da 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -197,6 +197,9 @@ Fixed a memory leak in which ReusableObjectMessage would hold a reference to the most recently logged object. + + Jackson layouts used with AsyncLoggerContextSelector output the expected format rather than only a json string of the message text. + @@ -304,6 +307,9 @@ Fixed a memory leak in which ReusableObjectMessage would hold a reference to the most recently logged object. + + Jackson layouts used with AsyncLoggerContextSelector output the expected format rather than only a json string of the message text. +