tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Rainer Jung <rainer.j...@kippdata.de>
Subject Re: TC 8.5 and Log4J2 via Juli: Wrong location info
Date Wed, 26 Oct 2016 06:26:56 GMT
Am 25.10.2016 um 22:40 schrieb Romain Manni-Bucau:
> 2016-10-25 22:35 GMT+02:00 Rainer Jung <rainer.jung@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.jung@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.

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.

>> 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.

>> 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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Mime
View raw message