Is the default to keep the existing behavior? Ralph
> On Sep 22, 2016, at 1:54 PM, Gary Gregory <garydgreg...@gmail.com> wrote: > > On Thu, Sep 22, 2016 at 1:07 PM, Gary Gregory <garydgreg...@gmail.com > <mailto:garydgreg...@gmail.com>> wrote: > On Thu, Sep 22, 2016 at 12:52 PM, Ralph Goers <ralph.go...@dslextreme.com > <mailto:ralph.go...@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 <garydgreg...@gmail.com >> <mailto:garydgreg...@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.go...@dslextreme.com >> <mailto:ralph.go...@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 <garydgreg...@gmail.com >>> <mailto:garydgreg...@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 <boa...@gmail.com >>> <mailto:boa...@gmail.com>> wrote: >>> Oh that explains it, thanks for the info! >>> >>> On 22 September 2016 at 10:21, Ralph Goers <ralph.go...@dslextreme.com >>> <mailto:ralph.go...@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 <boa...@gmail.com >>>> <mailto:boa...@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 <garydgreg...@gmail.com >>>> <mailto:garydgreg...@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: garydgreg...@gmail.com <mailto:garydgreg...@gmail.com> | >>>> ggreg...@apache.org <mailto:ggreg...@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 <boa...@gmail.com <mailto:boa...@gmail.com>> >>> >>> >>> >>> >>> -- >>> Matt Sicker <boa...@gmail.com <mailto:boa...@gmail.com>> >>> >>> >>> >>> -- >>> E-Mail: garydgreg...@gmail.com <mailto:garydgreg...@gmail.com> | >>> ggreg...@apache.org <mailto:ggreg...@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: garydgreg...@gmail.com <mailto:garydgreg...@gmail.com> | >> ggreg...@apache.org <mailto:ggreg...@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: garydgreg...@gmail.com <mailto:garydgreg...@gmail.com> | > ggreg...@apache.org <mailto:ggreg...@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: garydgreg...@gmail.com <mailto:garydgreg...@gmail.com> | > ggreg...@apache.org <mailto:ggreg...@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>
