You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@logging.apache.org by ma...@apache.org on 2014/08/26 05:52:06 UTC
svn commit: r1620501 - in /logging/log4j/log4j2/trunk/log4j-slf4j-impl/src:
main/java/org/apache/logging/slf4j/Log4jLogger.java
main/java/org/apache/logging/slf4j/Log4jMarkerFactory.java
test/java/org/apache/logging/slf4j/LoggerTest.java
Author: mattsicker
Date: Tue Aug 26 03:52:05 2014
New Revision: 1620501
URL: http://svn.apache.org/r1620501
Log:
Add custom SLF4J marker support in log4j-slf4j-impl bridge.
- Fixes LOG4J2-793.
Modified:
logging/log4j/log4j2/trunk/log4j-slf4j-impl/src/main/java/org/apache/logging/slf4j/Log4jLogger.java
logging/log4j/log4j2/trunk/log4j-slf4j-impl/src/main/java/org/apache/logging/slf4j/Log4jMarkerFactory.java
logging/log4j/log4j2/trunk/log4j-slf4j-impl/src/test/java/org/apache/logging/slf4j/LoggerTest.java
Modified: logging/log4j/log4j2/trunk/log4j-slf4j-impl/src/main/java/org/apache/logging/slf4j/Log4jLogger.java
URL: http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/log4j-slf4j-impl/src/main/java/org/apache/logging/slf4j/Log4jLogger.java?rev=1620501&r1=1620500&r2=1620501&view=diff
==============================================================================
--- logging/log4j/log4j2/trunk/log4j-slf4j-impl/src/main/java/org/apache/logging/slf4j/Log4jLogger.java (original)
+++ logging/log4j/log4j2/trunk/log4j-slf4j-impl/src/main/java/org/apache/logging/slf4j/Log4jLogger.java Tue Aug 26 03:52:05 2014
@@ -29,6 +29,7 @@ import org.apache.logging.log4j.message.
import org.apache.logging.log4j.spi.ExtendedLogger;
import org.slf4j.Marker;
import org.slf4j.MarkerFactory;
+import org.slf4j.impl.StaticMarkerBinder;
import org.slf4j.spi.LocationAwareLogger;
/**
@@ -375,7 +376,14 @@ public class Log4jLogger implements Loca
}
private static org.apache.logging.log4j.Marker getMarker(final Marker marker) {
- return marker != null ? ((org.apache.logging.slf4j.Log4jMarker) marker).getLog4jMarker() : null;
+ if (marker == null) {
+ return null;
+ } else if (marker instanceof Log4jMarker) {
+ return ((Log4jMarker) marker).getLog4jMarker();
+ } else {
+ final Log4jMarkerFactory factory = (Log4jMarkerFactory) StaticMarkerBinder.SINGLETON.getMarkerFactory();
+ return ((Log4jMarker) factory.getMarker(marker)).getLog4jMarker();
+ }
}
@Override
Modified: logging/log4j/log4j2/trunk/log4j-slf4j-impl/src/main/java/org/apache/logging/slf4j/Log4jMarkerFactory.java
URL: http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/log4j-slf4j-impl/src/main/java/org/apache/logging/slf4j/Log4jMarkerFactory.java?rev=1620501&r1=1620500&r2=1620501&view=diff
==============================================================================
--- logging/log4j/log4j2/trunk/log4j-slf4j-impl/src/main/java/org/apache/logging/slf4j/Log4jMarkerFactory.java (original)
+++ logging/log4j/log4j2/trunk/log4j-slf4j-impl/src/main/java/org/apache/logging/slf4j/Log4jMarkerFactory.java Tue Aug 26 03:52:05 2014
@@ -16,6 +16,7 @@
*/
package org.apache.logging.slf4j;
+import java.util.Iterator;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
@@ -24,14 +25,14 @@ import org.slf4j.IMarkerFactory;
import org.slf4j.Marker;
/**
- *
+ * Log4j/SLF4J bridge to create SLF4J Markers based on name or based on existing SLF4J Markers.
*/
public class Log4jMarkerFactory implements IMarkerFactory {
private final ConcurrentMap<String, Marker> markerMap = new ConcurrentHashMap<String, Marker>();
/**
- * Return a Log4j Marker that is compatible with SLF4J.
+ * Returns a Log4j Marker that is compatible with SLF4J.
* @param name The name of the Marker.
* @return A Marker.
*/
@@ -45,12 +46,49 @@ public class Log4jMarkerFactory implemen
return marker;
}
final org.apache.logging.log4j.Marker log4jMarker = MarkerManager.getMarker(name);
- marker = new Log4jMarker(log4jMarker);
+ return addMarkerIfAbsent(name, log4jMarker);
+ }
+
+ private Marker addMarkerIfAbsent(final String name, final org.apache.logging.log4j.Marker log4jMarker) {
+ final Marker marker = new Log4jMarker(log4jMarker);
final Marker existing = markerMap.putIfAbsent(name, marker);
return existing == null ? marker : existing;
}
/**
+ * Returns a Log4j Marker converted from an existing custom SLF4J Marker.
+ * @param marker The SLF4J Marker to convert.
+ * @return A converted Log4j/SLF4J Marker.
+ * @since 2.1
+ */
+ public Marker getMarker(final Marker marker) {
+ if (marker == null) {
+ throw new IllegalArgumentException("Marker must not be null");
+ }
+ Marker m = markerMap.get(marker.getName());
+ if (m != null) {
+ return m;
+ }
+ return addMarkerIfAbsent(marker.getName(), convertMarker(marker));
+ }
+
+ private static org.apache.logging.log4j.Marker convertMarker(final Marker original) {
+ if (original == null) {
+ throw new IllegalArgumentException("Marker must not be null");
+ }
+ final org.apache.logging.log4j.Marker marker = MarkerManager.getMarker(original.getName());
+ if (original.hasReferences()) {
+ final Iterator it = original.iterator();
+ while (it.hasNext()) {
+ final Marker next = (Marker) it.next();
+ // kind of hope nobody uses cycles in their Markers. I mean, why would you do that?
+ marker.addParents(convertMarker(next));
+ }
+ }
+ return marker;
+ }
+
+ /**
* Returns true if the Marker exists.
* @param name The Marker name.
* @return true if the Marker exists, false otherwise.
Modified: logging/log4j/log4j2/trunk/log4j-slf4j-impl/src/test/java/org/apache/logging/slf4j/LoggerTest.java
URL: http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/log4j-slf4j-impl/src/test/java/org/apache/logging/slf4j/LoggerTest.java?rev=1620501&r1=1620500&r2=1620501&view=diff
==============================================================================
--- logging/log4j/log4j2/trunk/log4j-slf4j-impl/src/test/java/org/apache/logging/slf4j/LoggerTest.java (original)
+++ logging/log4j/log4j2/trunk/log4j-slf4j-impl/src/test/java/org/apache/logging/slf4j/LoggerTest.java Tue Aug 26 03:52:05 2014
@@ -18,22 +18,17 @@ package org.apache.logging.slf4j;
import java.util.List;
import java.util.Locale;
-import java.util.Map;
-import org.apache.logging.log4j.LogManager;
-import org.apache.logging.log4j.core.Appender;
-import org.apache.logging.log4j.core.LoggerContext;
-import org.apache.logging.log4j.core.config.ConfigurationFactory;
import org.apache.logging.log4j.core.util.Constants;
-import org.apache.logging.log4j.status.StatusLogger;
+import org.apache.logging.log4j.junit.InitialLoggerContext;
import org.apache.logging.log4j.test.appender.ListAppender;
-import org.junit.AfterClass;
import org.junit.Before;
-import org.junit.BeforeClass;
+import org.junit.ClassRule;
import org.junit.Test;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.slf4j.MDC;
+import org.slf4j.Marker;
import org.slf4j.ext.EventData;
import org.slf4j.ext.EventLogger;
import org.slf4j.ext.XLogger;
@@ -48,22 +43,9 @@ import static org.junit.Assert.*;
public class LoggerTest {
private static final String CONFIG = "log4j-test1.xml";
- private static LoggerContext ctx;
- @BeforeClass
- public static void setupClass() {
- System.setProperty(ConfigurationFactory.CONFIGURATION_FILE_PROPERTY, CONFIG);
- ctx = (LoggerContext) LogManager.getContext(false);
- ctx.reconfigure();
- ctx.getConfiguration();
- }
-
- @AfterClass
- public static void cleanupClass() {
- System.clearProperty(ConfigurationFactory.CONFIGURATION_FILE_PROPERTY);
- ctx.reconfigure();
- StatusLogger.getLogger().reset();
- }
+ @ClassRule
+ public static InitialLoggerContext ctx = new InitialLoggerContext(CONFIG);
Logger logger = LoggerFactory.getLogger("LoggerTest");
XLogger xlogger = XLoggerFactory.getXLogger("LoggerTest");
@@ -135,6 +117,16 @@ public class LoggerTest {
verify("List", "o.a.l.s.LoggerTest Debug message MDC{}" + Constants.LINE_SEPARATOR);
}
+ /**
+ * @issue LOG4J2-793
+ */
+ @Test
+ public void supportsCustomSLF4JMarkers() {
+ final Marker marker = new CustomFlatMarker("TEST");
+ logger.debug(marker, "Test");
+ verify("List", "o.a.l.s.LoggerTest Test MDC{}" + Constants.LINE_SEPARATOR);
+ }
+
@Test
public void testRootLogger() {
final Logger l = LoggerFactory.getLogger(Logger.ROOT_LOGGER_NAME);
@@ -168,23 +160,18 @@ public class LoggerTest {
}
private void verify(final String name, final String expected) {
- //LoggerContext ctx = (LoggerContext) LogManager.getContext(false);
- final Appender listApp = ctx.getConfiguration().getAppender(name);
+ final ListAppender listApp = ctx.getListAppender(name);
assertNotNull("Missing Appender", listApp);
- assertTrue("Not a ListAppender", listApp instanceof ListAppender);
- final List<String> events = ((ListAppender) listApp).getMessages();
+ 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);
assertEquals("Incorrect message. Expected " + expected + ". Actual " + actual, expected, actual);
- ((ListAppender) listApp).clear();
+ listApp.clear();
}
@Before
public void cleanup() {
- final Map<String, Appender> map = ctx.getConfiguration().getAppenders();
- final Appender listApp = map.get("List");
- ((ListAppender) listApp).clear();
- final Appender eventApp = map.get("EventLogger");
- ((ListAppender) eventApp).clear();
+ ctx.getListAppender("List").clear();
+ ctx.getListAppender("EventLogger").clear();
}
}
Re: svn commit: r1620501 - in /logging/log4j/log4j2/trunk/log4j-slf4j-impl/src:
main/java/org/apache/logging/slf4j/Log4jLogger.java main/java/org/apache/logging/slf4j/Log4jMarkerFactory.java
test/java/org/apache/logging/slf4j/LoggerTest.java
Posted by Matt Sicker <bo...@gmail.com>.
Actually, the log4j-to-slf4j code path uses Marker.isInstanceOf() which
itself may be susceptible to the same infinite loop. Possible bug?
On 26 August 2014 07:42, Matt Sicker <bo...@gmail.com> wrote:
> Thanks for the advice. I've updated that section. Now I think something
> similar should be done in log4j-to-slf4j, too. So, note to self.
>
>
> On 26 August 2014 00:38, Matt Sicker <bo...@gmail.com> wrote:
>
>> I started to consider something similar but didn't follow through quite
>> yet. Good idea
>>
>>
>> On Monday, 25 August 2014, Remko Popma <re...@gmail.com> wrote:
>>
>>> About your cycle comment:
>>> One option is to pass an ArrayList to Log4jMarkerFactory.convertMarker
>>> and put processed elements in it. During recursive calls you check & return
>>> when you detect that the list already contains the parameter you're about
>>> to process. If a user does have a cycle this can prevent an infinite loop.
>>>
>>> Sent from my iPhone
>>>
>>> > On 2014/08/26, at 5:52, mattsicker@apache.org wrote:
>>> >
>>> > Log4jMarkerFactory
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
>>> For additional commands, e-mail: log4j-dev-help@logging.apache.org
>>>
>>>
>>
>> --
>> Matt Sicker <bo...@gmail.com>
>>
>
>
>
> --
> Matt Sicker <bo...@gmail.com>
>
--
Matt Sicker <bo...@gmail.com>
Re: svn commit: r1620501 - in /logging/log4j/log4j2/trunk/log4j-slf4j-impl/src:
main/java/org/apache/logging/slf4j/Log4jLogger.java main/java/org/apache/logging/slf4j/Log4jMarkerFactory.java
test/java/org/apache/logging/slf4j/LoggerTest.java
Posted by Matt Sicker <bo...@gmail.com>.
Thanks for the advice. I've updated that section. Now I think something
similar should be done in log4j-to-slf4j, too. So, note to self.
On 26 August 2014 00:38, Matt Sicker <bo...@gmail.com> wrote:
> I started to consider something similar but didn't follow through quite
> yet. Good idea
>
>
> On Monday, 25 August 2014, Remko Popma <re...@gmail.com> wrote:
>
>> About your cycle comment:
>> One option is to pass an ArrayList to Log4jMarkerFactory.convertMarker
>> and put processed elements in it. During recursive calls you check & return
>> when you detect that the list already contains the parameter you're about
>> to process. If a user does have a cycle this can prevent an infinite loop.
>>
>> Sent from my iPhone
>>
>> > On 2014/08/26, at 5:52, mattsicker@apache.org wrote:
>> >
>> > Log4jMarkerFactory
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
>> For additional commands, e-mail: log4j-dev-help@logging.apache.org
>>
>>
>
> --
> Matt Sicker <bo...@gmail.com>
>
--
Matt Sicker <bo...@gmail.com>
Re: svn commit: r1620501 - in /logging/log4j/log4j2/trunk/log4j-slf4j-impl/src:
main/java/org/apache/logging/slf4j/Log4jLogger.java main/java/org/apache/logging/slf4j/Log4jMarkerFactory.java
test/java/org/apache/logging/slf4j/LoggerTest.java
Posted by Matt Sicker <bo...@gmail.com>.
I started to consider something similar but didn't follow through quite
yet. Good idea
On Monday, 25 August 2014, Remko Popma <re...@gmail.com> wrote:
> About your cycle comment:
> One option is to pass an ArrayList to Log4jMarkerFactory.convertMarker and
> put processed elements in it. During recursive calls you check & return
> when you detect that the list already contains the parameter you're about
> to process. If a user does have a cycle this can prevent an infinite loop.
>
> Sent from my iPhone
>
> > On 2014/08/26, at 5:52, mattsicker@apache.org <javascript:;> wrote:
> >
> > Log4jMarkerFactory
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
> <javascript:;>
> For additional commands, e-mail: log4j-dev-help@logging.apache.org
> <javascript:;>
>
>
--
Matt Sicker <bo...@gmail.com>
Re: svn commit: r1620501 - in /logging/log4j/log4j2/trunk/log4j-slf4j-impl/src: main/java/org/apache/logging/slf4j/Log4jLogger.java main/java/org/apache/logging/slf4j/Log4jMarkerFactory.java test/java/org/apache/logging/slf4j/LoggerTest.java
Posted by Remko Popma <re...@gmail.com>.
About your cycle comment:
One option is to pass an ArrayList to Log4jMarkerFactory.convertMarker and put processed elements in it. During recursive calls you check & return when you detect that the list already contains the parameter you're about to process. If a user does have a cycle this can prevent an infinite loop.
Sent from my iPhone
> On 2014/08/26, at 5:52, mattsicker@apache.org wrote:
>
> Log4jMarkerFactory
---------------------------------------------------------------------
To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
For additional commands, e-mail: log4j-dev-help@logging.apache.org