On Thu, Sep 22, 2016 at 3:01 PM, Ralph Goers <[email protected]> wrote:
> OK - Thanks for doing that. > You bet. Gary > > Ralph > > On Sep 22, 2016, at 2:26 PM, Gary Gregory <[email protected]> wrote: > > On Thu, Sep 22, 2016 at 2:07 PM, Ralph Goers <[email protected]> > wrote: > >> Is the default to keep the existing behavior? >> > > Yes, it is. > > Gary > > >> >> Ralph >> >> On Sep 22, 2016, at 1:54 PM, Gary Gregory <[email protected]> wrote: >> >> On Thu, Sep 22, 2016 at 1:07 PM, Gary Gregory <[email protected]> >> wrote: >> >>> On Thu, Sep 22, 2016 at 12:52 PM, Ralph Goers <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 <[email protected]> >>>> 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> 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 <[email protected]> >>>>> wrote: >>>>> >>>>> I created https://issues.apache.org/jira/browse/LOG4J2-1608 >>>>> >>>>> Gary >>>>> >>>>> On Thu, Sep 22, 2016 at 8:22 AM, Matt Sicker <[email protected]> wrote: >>>>> >>>>>> Oh that explains it, thanks for the info! >>>>>> >>>>>> On 22 September 2016 at 10:21, Ralph Goers <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 <[email protected]> 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 <[email protected]> >>>>>>> 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: [email protected] | [email protected] >>>>>>>> <[email protected]> >>>>>>>> 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 >>>>>>>> Home: http://garygregory.com/ >>>>>>>> Tweet! http://twitter.com/GaryGregory >>>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> -- >>>>>>> Matt Sicker <[email protected]> >>>>>>> >>>>>>> >>>>>>> >>>>>> >>>>>> >>>>>> -- >>>>>> Matt Sicker <[email protected]> >>>>>> >>>>> >>>>> >>>>> >>>>> -- >>>>> E-Mail: [email protected] | [email protected] >>>>> <[email protected]> >>>>> 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 >>>>> Home: http://garygregory.com/ >>>>> Tweet! http://twitter.com/GaryGregory >>>>> >>>>> >>>>> >>>> >>>> >>>> -- >>>> E-Mail: [email protected] | [email protected] >>>> <[email protected]> >>>> 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 >>>> Home: http://garygregory.com/ >>>> Tweet! http://twitter.com/GaryGregory >>>> >>>> >>>> >>> >>> >>> -- >>> E-Mail: [email protected] | [email protected] >>> <[email protected]> >>> 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 >>> Home: http://garygregory.com/ >>> Tweet! http://twitter.com/GaryGregory >>> >> >> >> >> -- >> E-Mail: [email protected] | [email protected] >> <[email protected]> >> 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 >> Home: http://garygregory.com/ >> Tweet! http://twitter.com/GaryGregory >> >> >> > > > -- > E-Mail: [email protected] | [email protected] > 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 > Home: http://garygregory.com/ > Tweet! http://twitter.com/GaryGregory > > > -- E-Mail: [email protected] | [email protected] 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 Home: http://garygregory.com/ Tweet! http://twitter.com/GaryGregory
