[slf4j-dev] Missing Trowables in EventRecordingLogger

[Wessel van Norel]

Hi Ceki,

Thanks for your quick follow up. My comments will be below as well.

On Sun, Aug 11, 2019 at 10:26 PM Ceki Gülcü <ceki at qos.ch> wrote:

>
> Hi Wessel,
>
> Comments below.
>
> On 09/08/2019 01:09, Wessel van Norel wrote:
> > I've not yet created the test cases for the EventRecordingLogger, I'll
> > do that in case it's indeed unintentional that it's loosing the
> > Throwables. I've added two tests for the changes I made to the
> > MessageFormatter. While I was at it I also applied some improvements
> > IntelliJ suggested.
>
> Very good. In general, it is preferable if the more meaningful changes
> are separate from more cosmetic ones even if both are welcome.
>

Sure no problem. Do you want a separate JIRA Ticket for it?

> You can see my changes on my fork:
> >
> >
> https://github.com/qos-ch/slf4j/compare/master...delgurth:eventrecordinglogger?expand=1
>
> I have made some comments there.
>

>From those remarks:

>  Instead, an analogous check can be placed in recordEvent() avoiding
repetition.

Sounds like a good plan. But I'll have to think about how to deal with the
2 object argument calls. Since if just make an two element array out of it
and then have to make it into a one element array and a throwable that's a
bit wasteful, isn't it?


> In any case, I think the unit test as well the change to the
> org.slf4j.event.EventRecodingLogger.recordEvent method are warranted. I
> propose to discuss changes to MessageFormatter class separately.
>

Ok. I'll create a Jira ticket about the MessageFormatter. The stacktrace as
last parameter is great for users of slf4j, but for the API implementation
it's a bit of a pain. Do you have a performance benchmark around slf4j or
specific the messageformatter (since the javadoc says the slf4j version is
10 times faster then the version provided by java itself)? I found a
recently created SLF4J JMH project:
https://github.com/wsargent/slf4j-benchmark but I'm not sure if that
project is doing the proper benchmarking.

Also can you please create a Jira ticket describing the bug in
> EventRecodingLogger.recordEvent?
>

Will do.


> Many thanks,
>
> --
> Ceki
>

You're most welcome.

Kind regards,
Wessel

_______________________________________________
> slf4j-dev mailing list
> slf4j-dev at qos.ch
> http://mailman.qos.ch/mailman/listinfo/slf4j-dev
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.qos.ch/pipermail/slf4j-dev/attachments/20190812/6c298b8f/attachment.html>