logging-log4j-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ralph Goers <ralph.go...@dslextreme.com>
Subject Re: org.apache.logging.log4j.web.appender.ServletAppender.append(LogEvent)
Date Thu, 22 Sep 2016 22:01:20 GMT
OK - Thanks for doing that.

Ralph

> On Sep 22, 2016, at 2:26 PM, Gary Gregory <garydgregory@gmail.com> wrote:
> 
> On Thu, Sep 22, 2016 at 2:07 PM, Ralph Goers <ralph.goers@dslextreme.com <mailto:ralph.goers@dslextreme.com>>
wrote:
> Is the default to keep the existing behavior?
> 
> Yes, it is. 
> 
> Gary
>  
> 
> Ralph
> 
>> On Sep 22, 2016, at 1:54 PM, Gary Gregory <garydgregory@gmail.com <mailto:garydgregory@gmail.com>>
wrote:
>> 
>> On Thu, Sep 22, 2016 at 1:07 PM, Gary Gregory <garydgregory@gmail.com <mailto:garydgregory@gmail.com>>
wrote:
>> On Thu, Sep 22, 2016 at 12:52 PM, Ralph Goers <ralph.goers@dslextreme.com <mailto:ralph.goers@dslextreme.com>>
wrote:
>> Because Log4j users are far more inclined to want to use the formatting we provide?
 What your change does is effectively disallow users from using our exception formatting when
logging to the servletContext. 
>> 
>> No it does not. Now, if the user does nothing, for example uses "%m%n", the exception
is still added to the messages as you pointed out. What you do get is double logging and I'll
add an option to only use the servlet context API with the Throwable if a new attribute is
set.
>> 
>> I added a Builder to the ServletAppender, deprecated the factory method, and added
a new option:
>> 
>>         /**
>>          * Logs with {@link ServletContext#log(String, Throwable)} if true and with
{@link ServletContext#log(String)} if false.
>>          * 
>>          * @return whether to log a Throwable with the servlet context.
>>          */
>>         public boolean isLogThrowables() {
>>             return logThrowables;
>>         }
>> 
>> Gary 
>> 
>> Gary
>>  
>> 
>> Also, this breaks the current behavior.
>> 
>> You really need to rethink this and provide an option or something on the Appender
to allow the user to control this.
>> 
>> Please revert what you have committed so it doesn’t go out if I can get to doing
the release tonight. 
>> 
>> Ralph
>> 
>>> On Sep 22, 2016, at 11:42 AM, Gary Gregory <garydgregory@gmail.com <mailto:garydgregory@gmail.com>>
wrote:
>>> 
>>> Since ServletContext is an interface, who knows what interesting work an implementation
will do with the Throwables. 
>>> 
>>> Another way to look at it is: Why should we hide the actual Throwable from the
ServletContext?
>>> 
>>> Gary
>>> 
>>> On Thu, Sep 22, 2016 at 10:23 AM, Ralph Goers <ralph.goers@dslextreme.com
<mailto:ralph.goers@dslextreme.com>> wrote:
>>> Yes, and them immediately committed it.  I am -1 on this commit until I get an
explanation as to why what we currently do isn’t better.
>>> 
>>> Ralph
>>> 
>>>> On Sep 22, 2016, at 9:29 AM, Gary Gregory <garydgregory@gmail.com <mailto:garydgregory@gmail.com>>
wrote:
>>>> 
>>>> I created https://issues.apache.org/jira/browse/LOG4J2-1608 <https://issues.apache.org/jira/browse/LOG4J2-1608>
>>>> 
>>>> Gary
>>>> 
>>>> On Thu, Sep 22, 2016 at 8:22 AM, Matt Sicker <boards@gmail.com <mailto:boards@gmail.com>>
wrote:
>>>> Oh that explains it, thanks for the info!
>>>> 
>>>> On 22 September 2016 at 10:21, Ralph Goers <ralph.goers@dslextreme.com
<mailto:ralph.goers@dslextreme.com>> wrote:
>>>> PatternLayout defaults to implicitly add %ex or one of its variations to
your pattern if you don’t specify it. To disable exception logging you have to use %noex.
>>>> 
>>>> Ralph
>>>> 
>>>>> On Sep 22, 2016, at 6:55 AM, Matt Sicker <boards@gmail.com <mailto:boards@gmail.com>>
wrote:
>>>>> 
>>>>> I usually don't even include %exception in my pattern layouts for some
reason, probably because of the double logging. So I'd go with (1) as well.
>>>>> 
>>>>> On 22 September 2016 at 02:59, Gary Gregory <garydgregory@gmail.com
<mailto:garydgregory@gmail.com>> wrote:
>>>>> Hi All,
>>>>> 
>>>>> The method org.apache.logging.log4j.web.appender.ServletAppender.append(LogEvent)
is defined as:
>>>>> 
>>>>>     @Override
>>>>>     public void append(final LogEvent event) {
>>>>>         servletContext.log(((AbstractStringLayout) getLayout()).toSerializable(event));
>>>>>     }
>>>>> 
>>>>> Instead of:
>>>>> 
>>>>>     @Override
>>>>>     public void append(final LogEvent event) {
>>>>>         servletContext.log(((AbstractStringLayout) getLayout()).toSerializable(event),
event.getThrown());
>>>>>     }
>>>>> 
>>>>> Which does not give the best information we have to the servlet context
logging.
>>>>> 
>>>>> The tricky part is that to avoid logging the exception twice like in
our test org.apache.logging.log4j.web.ServletAppenderTest. To avoid the double logging, we
could (1) document not using a %exception in the pattern layout.
>>>>> 
>>>>> That or (yikes) (2) provide a variation of the toSerializable(event)
like toSerializable(event, false), where the boolean is an ignoreException parameter. It seems
there are plenty of places where exceptions are treated specially already, this would be another.
>>>>> 
>>>>> I like (1) better because it is simpler.
>>>>> 
>>>>> Thoughts?
>>>>> 
>>>>> Thank you,
>>>>> Gary
>>>>> 
>>>>> 
>>>>> -- 
>>>>> E-Mail: garydgregory@gmail.com <mailto:garydgregory@gmail.com>
| ggregory@apache.org  <mailto:ggregory@apache.org>
>>>>> Java Persistence with Hibernate, Second Edition <http://www.manning.com/bauer3/>
>>>>> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
>>>>> Spring Batch in Action <http://www.manning.com/templier/>
>>>>> Blog: http://garygregory.wordpress.com <http://garygregory.wordpress.com/>

>>>>> Home: http://garygregory.com/ <http://garygregory.com/>
>>>>> Tweet! http://twitter.com/GaryGregory <http://twitter.com/GaryGregory>
>>>>> 
>>>>> 
>>>>> -- 
>>>>> Matt Sicker <boards@gmail.com <mailto:boards@gmail.com>>
>>>> 
>>>> 
>>>> 
>>>> 
>>>> -- 
>>>> Matt Sicker <boards@gmail.com <mailto:boards@gmail.com>>
>>>> 
>>>> 
>>>> 
>>>> -- 
>>>> E-Mail: garydgregory@gmail.com <mailto:garydgregory@gmail.com> | ggregory@apache.org 
<mailto:ggregory@apache.org>
>>>> Java Persistence with Hibernate, Second Edition <http://www.manning.com/bauer3/>
>>>> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
>>>> Spring Batch in Action <http://www.manning.com/templier/>
>>>> Blog: http://garygregory.wordpress.com <http://garygregory.wordpress.com/>

>>>> Home: http://garygregory.com/ <http://garygregory.com/>
>>>> Tweet! http://twitter.com/GaryGregory <http://twitter.com/GaryGregory>
>>> 
>>> 
>>> 
>>> -- 
>>> E-Mail: garydgregory@gmail.com <mailto:garydgregory@gmail.com> | ggregory@apache.org 
<mailto:ggregory@apache.org>
>>> Java Persistence with Hibernate, Second Edition <http://www.manning.com/bauer3/>
>>> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
>>> Spring Batch in Action <http://www.manning.com/templier/>
>>> Blog: http://garygregory.wordpress.com <http://garygregory.wordpress.com/>

>>> Home: http://garygregory.com/ <http://garygregory.com/>
>>> Tweet! http://twitter.com/GaryGregory <http://twitter.com/GaryGregory>
>> 
>> 
>> 
>> -- 
>> E-Mail: garydgregory@gmail.com <mailto:garydgregory@gmail.com> | ggregory@apache.org 
<mailto:ggregory@apache.org>
>> Java Persistence with Hibernate, Second Edition <http://www.manning.com/bauer3/>
>> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
>> Spring Batch in Action <http://www.manning.com/templier/>
>> Blog: http://garygregory.wordpress.com <http://garygregory.wordpress.com/>

>> Home: http://garygregory.com/ <http://garygregory.com/>
>> Tweet! http://twitter.com/GaryGregory <http://twitter.com/GaryGregory>
>> 
>> 
>> -- 
>> E-Mail: garydgregory@gmail.com <mailto:garydgregory@gmail.com> | ggregory@apache.org 
<mailto:ggregory@apache.org>
>> Java Persistence with Hibernate, Second Edition <http://www.manning.com/bauer3/>
>> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
>> Spring Batch in Action <http://www.manning.com/templier/>
>> Blog: http://garygregory.wordpress.com <http://garygregory.wordpress.com/>

>> Home: http://garygregory.com/ <http://garygregory.com/>
>> Tweet! http://twitter.com/GaryGregory <http://twitter.com/GaryGregory>
> 
> 
> 
> -- 
> E-Mail: garydgregory@gmail.com <mailto:garydgregory@gmail.com> | ggregory@apache.org
 <mailto:ggregory@apache.org>
> Java Persistence with Hibernate, Second Edition <http://www.manning.com/bauer3/>
> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
> Spring Batch in Action <http://www.manning.com/templier/>
> Blog: http://garygregory.wordpress.com <http://garygregory.wordpress.com/> 
> Home: http://garygregory.com/ <http://garygregory.com/>
> Tweet! http://twitter.com/GaryGregory <http://twitter.com/GaryGregory>

Mime
View raw message