2016-10-26 8:26 GMT+02:00 Rainer Jung <rainer.j...@kippdata.de>: > Am 25.10.2016 um 22:40 schrieb Romain Manni-Bucau: > >> 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. >> > > I was more concerned about a webapp that does not use juli, but uses > Log4J2 directly (or via SLF4J etc.). We should not disturb the behavior of > such a webapp by our Tomcat log setup. > > Yes Tomcat is not yet able to filter resources by classloader (Loader could get a config for that with defaults, we do it in tomee for slf4j and several specs for instance)
> Using a specific class loader for loading TC logging resources/config > might work (although it will have the usual problem, that logging isn't set > up correctly before the loader is in place). Setting it up as I described > it might have side effects on deployed webapps due to delegation rules. > I'll try to find time to check this. > > Should be fine, worse case the constraint would be to do the LogManager.getLogger lazily which is acceptable for bootstrap loggers (no runtime performance constraint). > 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. >> > > By "safe" I did not mean whether it will work in any case, but whether it > can disturb deployed contexts. > > Sure, that was my point too: if the factory is context aware you can delegate to the local LoggerConfig when not in tomcat common (and shared?) classloader. > > 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 > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org > For additional commands, e-mail: dev-h...@tomcat.apache.org > >
