2016-10-25 22:35 GMT+02:00 Rainer Jung <rainer.j...@kippdata.de>: > Am 25.10.2016 um 17:07 schrieb Romain Manni-Bucau: > >> This has the issue but the fix is easy and 100% belonging to the Log impl >> jar: >> http://svn.apache.org/repos/asf/openwebbeans/microwave/trunk >> /microwave-core/src/main/java/org/apache/microwave/logging/ >> log4j2/MicrowaveLogEventFactory.java >> and >> http://svn.apache.org/repos/asf/openwebbeans/microwave/trunk >> /microwave-core/src/main/resources/log4j2.component.properties >> >> >> Romain Manni-Bucau >> @rmannibucau <https://twitter.com/rmannibucau> | Blog >> <https://blog-rmannibucau.rhcloud.com> | Old Wordpress Blog >> <http://rmannibucau.wordpress.com> | Github < >> https://github.com/rmannibucau> | >> LinkedIn <https://www.linkedin.com/in/rmannibucau> | Tomitriber >> <http://www.tomitribe.com> | JavaEE Factory >> <https://javaeefactory-rmannibucau.rhcloud.com> >> >> 2016-10-25 16:38 GMT+02:00 Rainer Jung <rainer.j...@kippdata.de>: >> >> Am 25.10.2016 um 15:33 schrieb Romain Manni-Bucau: >>> >>> Hi guys, >>>> >>>> since now tomcat has Log API as a SPI doing 2 is easy ( >>>> http://svn.apache.org/repos/asf/openwebbeans/microwave/trunk >>>> /microwave-core/src/main/java/org/apache/microwave/logging/ >>>> tomcat/Log4j2Log.java) >>>> and >>>> just a drop-in jar setup so not sure it needs to be in tomcat default >>>> delivery. >>>> >>>> >>> I tried to plug in Log4j2Log.java using SPI. It gets used, but it doesn't >>> fix the problem. Now the location info points to Log4j2Log instead of >>> DirectJDKLog. The Problem of one indirection layer to much above Log4Js >>> has't changed. >>> >>> I think the jul log bridge removes one frame, which is the jul loger, >>> ending up taking the DirectJDKLog which vcalls the jul logger as its >>> location. Using your approach does no longer use jul, so log4j2 doesn't >>> re >>> move a frame and again takes the wrong class Log4j2Log as its location >>> info. >>> >>> I think delegating is not enough, you actually need to implement the >>> target API or extend a target implementation. >>> >>> Did you check the location info using the SPI approach? >>> >>> Regards, >>> >>> Rainer >>> >> > Thanks for that hint. Actually I hadn't noticed this trick to overwrite > the getSource() logic. > > I'm not sure though, how dangerous this is due to possible effects on > Log4J2 in some webapp. IMHO the alternative LogEvent and LogEventFactory > impl must be made available for Tomcat use already in the CLASSPATH. The > server loader could already be too late. That means it is also visible to > any webapp. And activating it is done either via a system property, or a > resource named log4j2.component.properties which contains the property > > Log4jLogEventFactory=org.apache.juli.JuliLogEventFactory > > Again this resource IMHO must reside in the CLASSPATH. So both ways of > activating the alternative LogEvent impl also activate it for any Log4J2 in > any webapp. > > True or not, nothing prevents to run it in a logger classloader to isolate from webapp. That said if you want tomcat to log with log4j2 you likely also want the webapps to do the same IMO. Also tomcat has a classloader context selector so config can be per webapp or at least hook is there for that.
> Although the code of the LogEvent looks compatible with the standard one, > this is only true as long as we consider Log4jLogEvent being the standard > one. Unfortunately the Log4J2 class LoggerConfig can use > DefaultLogEventFactory (which indeed uses Log4jLogEvent) but also > ReusableLogEventFactory, which uses thread-local MutableLogEvent. > > So I'm not sure how safe this replacement is. > > the properties config enforces it. It can get more config love but think it already works not bad. > For anyone who wants to experiment here's what I did: > > - create file log4j2.component.properties with the single line: > > Log4jLogEventFactory=org.apache.juli.JuliLogEventFactory > > - create class JuliLogEventFactory.java very similar to the one Romain > created. I'll paste it inline further down. > > - Put the class and the properties file into some new jar (tomcat-juli.jar > should also work) > > - Add the new jar and from Log4J2 log4j-api.jar, log4j-core.jar and > log4j-jul.jar to the CLASSPATH > > - Add a Log4J2 config (I have added its location to CLASSPATH) > > - Set > > LOGGING_MANAGER="-Djava.util.logging.manager=org.apache.logg > ing.log4j.jul.LogManager" > > As a result the logged location info now works although the jul bridge is > being used. But as said above, I'm not so sure about dangers for webapp > provided Log4J2. Note that not using the jul bridge by Romain's SPI idea > still needs a very similar location info fix. So we can split the > discussion about how to fix location info from the discussion whether we > prefer the jul bridge or a tomcat provided Log4J2 logger via SPI. > > For the sake of completeness, here's the call stack we need to handle to > find the location info when using the Log4J2 jul bridge: > > ... > org.apache.logging.log4j.core.Logger.logMessage(Logger.java:147) > org.apache.logging.log4j.spi.ExtendedLoggerWrapper.logMessag > e(ExtendedLoggerWrapper.java:217) > org.apache.logging.log4j.spi.AbstractLogger.logMessage(Abstr > actLogger.java:1993) > org.apache.logging.log4j.spi.AbstractLogger.logIfEnabled(Abs > tractLogger.java:1852) > org.apache.logging.log4j.jul.WrappedLogger.log(WrappedLogger.java:50) > org.apache.logging.log4j.jul.ApiLogger.log(ApiLogger.java:112) > org.apache.logging.log4j.jul.ApiLogger.logp(ApiLogger.java:132) > org.apache.juli.logging.DirectJDKLog.log(DirectJDKLog.java:179) > org.apache.juli.logging.DirectJDKLog.info(DirectJDKLog.java:122) > -> here comes the wanted location > ... > > Regards, > > Rainer > > Class org.apache.juli.JuliLogEventFactory: > > /* > * Licensed to the Apache Software Foundation (ASF) under one > * or more contributor license agreements. See the NOTICE file > * distributed with this work for additional information > * regarding copyright ownership. The ASF licenses this file > * to you under the Apache License, Version 2.0 (the > * "License"); you may not use this file except in compliance > * with the License. You may obtain a copy of the License at > * > * http://www.apache.org/licenses/LICENSE-2.0 > * > * Unless required by applicable law or agreed to in writing, > * software distributed under the License is distributed on an > * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY > * KIND, either express or implied. See the License for the > * specific language governing permissions and limitations > * under the License. > */ > package org.apache.juli; > > import org.apache.logging.log4j.Level; > import org.apache.logging.log4j.Marker; > import org.apache.logging.log4j.core.LogEvent; > import org.apache.logging.log4j.core.config.Property; > import org.apache.logging.log4j.core.impl.DefaultLogEventFactory; > import org.apache.logging.log4j.core.impl.Log4jLogEvent; > import org.apache.logging.log4j.message.Message; > > import java.util.List; > > public class JuliLogEventFactory extends DefaultLogEventFactory { > @Override > public LogEvent createEvent(final String loggerName, final Marker > marker, final String fqcn, > final Level level, final Message data, > final List<Property> properties, > final Throwable t) { > return new JuliLog4jLogEvent(loggerName, marker, fqcn, level, > data, properties, t); > } > > public static class JuliLog4jLogEvent extends Log4jLogEvent { > private static final String juliLoggerName = > "org.apache.juli.logging.DirectJDKLog"; > > private StackTraceElement source = null; > > public JuliLog4jLogEvent(final String loggerName, final Marker > marker, final String fqcn, final Level level, > final Message data, final List<Property> > properties, final Throwable t) { > super(loggerName, marker, fqcn, level, data, properties, t); > } > > /** > * Returns the StackTraceElement for the caller. This will be the > entry that occurs right > * before the first occurrence of FQCN as a class name. > * @return the StackTraceElement for the caller. > */ > @Override > public StackTraceElement getSource() { > if (source != null) { > return source; > } > String loggerFqcn = getLoggerFqcn(); > if (loggerFqcn == null || !isIncludeLocation()) { > return null; > } > source = calcLocation(loggerFqcn); > return source; > } > > public static StackTraceElement calcLocation(final String > fqcnOfLogger) { > if (fqcnOfLogger == null) { > return null; > } > // LOG4J2-1029 new Throwable().getStackTrace is faster than > Thread.currentThread().getStackTrace(). > final StackTraceElement[] stackTrace = new > Throwable().getStackTrace(); > StackTraceElement last = null; > for (int i = stackTrace.length - 1; i > 0; i--) { > final String className = stackTrace[i].getClassName(); > if (fqcnOfLogger.equals(className) || > juliLoggerName.equals(className)) { > return last; > } > last = stackTrace[i]; > } > return null; > > } > } > } > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org > For additional commands, e-mail: dev-h...@tomcat.apache.org > >
