Thanks for clarifying Justin, Clebert.
+1 to the approach Art is highlighting.
One of the problems with the log4j migration we had in AMQ5 (and as Matt has
linked) is that the test classes are coupled to the logging framework (log4j
specifically). So if there was a need to switch to say, logback, it couldn't be
done "at the drop of a hat".
What Art has written only depends on Slf4j and mockito which is a huge plus.
--
Étienne
On Wed, 4 May 2022, at 10:29 AM, Arthur Naseef wrote:
> In case the testing part is not entirely clear, after making the logger
> injectable, then use a Mock in the test; like this:
>
> Logger mockLogger = Mockito.mock(Logger.class);
> target.setLog(mockLogger);
>
> ...
>
> Mockito.verify(mockLogger).info("Started");
>
> Art
>
> On Wed, May 4, 2022 at 10:27 AM Arthur Naseef <a...@amlinv.com> wrote:
>
>> +1 on SLF4J - that's what it does: eliminate the tight coupling of code
>> generating log output from the logging implementation.
>>
>> For testing, I would just make the logger injectable, but configure a
>> default. Here's a "Live Template" for intellij that does this (minus the
>> getter and setter):
>>
>> private static final org.slf4j.Logger DEFAULT_LOGGER =
>> org.slf4j.LoggerFactory.getLogger($CLASS$.class);
>>
>>
>> private org.slf4j.Logger log = DEFAULT_LOGGER;
>>
>>
>> Cheers!
>>
>> Art
>>
>>
>> On Wed, May 4, 2022 at 8:15 AM Matt Pavlovich <mattr...@gmail.com> wrote:
>>
>>> Hey Clebert-
>>>
>>> We just did this as part of log4j2 conversion. Here is a sample test
>>> class w/ in-line appender:
>>>
>>>
>>> https://github.com/apache/activemq/blob/ae30dce4e24ce5e0467d2a3219627cbefef1f0ae/activemq-unit-tests/src/test/java/org/apache/activemq/usage/StoreUsageLimitsTest.java
>>> <
>>> https://github.com/apache/activemq/blob/ae30dce4e24ce5e0467d2a3219627cbefef1f0ae/activemq-unit-tests/src/test/java/org/apache/activemq/usage/StoreUsageLimitsTest.java
>>> >
>>>
>>> Matt Pavlovich
>>>
>>> > On May 4, 2022, at 7:37 AM, Clebert Suconic <clebert.suco...@gmail.com>
>>> wrote:
>>> >
>>> > A few questions I have:
>>> >
>>> > - One of things we do in the testsuite is to capture loggers with an
>>> > interceptor and assert the loggers were called. How would we do such a
>>> > thing? an appender? Can someone point me to a test in ActiveMQ5 doing
>>> > that?
>>> >
>>> > - how would you configure Log4J? (assuming we are using log4j with
>>> SLF4j)
>>> >
>>> > - also we have separate loggers for Auditing... I think that will be
>>> > just an entry on the log4j configuration.
>>> >
>>> > On Wed, May 4, 2022 at 8:16 AM Clebert Suconic
>>> > <clebert.suco...@gmail.com> wrote:
>>> >>
>>> >> MDC seems nice, but I don't think it's relevant here.. we need the
>>> >> Codes as part of the messages where they appear. Some users will
>>> >> create alerts for them on their log frameworks.
>>> >>
>>> >> On Wed, May 4, 2022 at 1:16 AM Christoph Läubrich <m...@laeubi-soft.de>
>>> wrote:
>>> >>>
>>> >>> SLF4J support the "Mapped Diagnostic Context"
>>> >>>
>>> >>> maybe this would be a good replacement here? you could even add more
>>> >>> context infos then and people are free to format them as they like:
>>> >>>
>>> >>> https://logback.qos.ch/manual/mdc.html
>>> >>>
>>> >>> Am 03.05.22 um 22:30 schrieb Justin Bertram:
>>> >>>> The point here is that the current logging implementation provides a
>>> simple
>>> >>>> way to associate codes with each user-facing log message and
>>> exception.
>>> >>>> This is helpful to those who may want to monitor logs for certain
>>> codes
>>> >>>> (e.g. for alerting purposes), filter some codes out, etc. In this
>>> way the
>>> >>>> logging is part of a contract with the users much like an API is a
>>> contract
>>> >>>> with developers. The codes stay consistent across versions but the
>>> content
>>> >>>> of the message may change (e.g. to provide more information, correct
>>> >>>> spelling errors, typos, etc.). This kind of facade also opens the
>>> door for
>>> >>>> fairly simple internationalization.
>>> >>>>
>>> >>>> The goals here as I see them:
>>> >>>> - Maintain the aforementioned functionality.
>>> >>>> - Ditch the dependence on JBoss Log Manager and JBoss Logging.
>>> >>>>
>>> >>>> Having a simple implementation of our own is an easy way to do this.
>>> If we
>>> >>>> decide to go this route then (and only then) we will need to decide
>>> on the
>>> >>>> underlying logging facade and implementation.
>>> >>>>
>>> >>>>
>>> >>>> Justin
>>> >>>>
>>> >>>>
>>> >>>> On Tue, May 3, 2022 at 3:17 PM Christopher Shannon <
>>> >>>> christopher.l.shan...@gmail.com> wrote:
>>> >>>>
>>> >>>>> Using SL4J makes sense to me as that is what almost everyone else
>>> uses so
>>> >>>>> it's pretty standard and easy to swap implementations
>>> >>>>>
>>> >>>>> On Mon, May 2, 2022 at 1:26 PM Justin Bertram <jbert...@apache.org>
>>> wrote:
>>> >>>>>
>>> >>>>>> I think this looks great, Clebert. The code is straightforward,
>>> and I
>>> >>>>> like
>>> >>>>>> the idea of reducing our dependencies.
>>> >>>>>>
>>> >>>>>> This is a +1 from me.
>>> >>>>>>
>>> >>>>>>
>>> >>>>>> Justin
>>> >>>>>>
>>> >>>>>> On Fri, Apr 29, 2022 at 3:43 PM Clebert Suconic <
>>> >>>>> clebert.suco...@gmail.com
>>> >>>>>>>
>>> >>>>>> wrote:
>>> >>>>>>
>>> >>>>>>> For a while, I thought it would be nice to remove jboss-logging
>>> from
>>> >>>>>>> artemis and use a generic logger. (SLF4J, Log4j, commons..
>>> whatever..
>>> >>>>>>> it's all orthogonal and transparent to this discussion, we can
>>> decide
>>> >>>>>>> that at a later point).
>>> >>>>>>>
>>> >>>>>>>
>>> >>>>>>> One of the issues we had while making the move would be the
>>> generated
>>> >>>>>>> error codes out of the Log Processor.
>>> >>>>>>>
>>> >>>>>>>
>>> >>>>>>> So, I put together a prototype here that would generate code
>>> based on
>>> >>>>>>> an interface and that could use whatever logger we choose. I will
>>> try
>>> >>>>>>> to never remove the branch for future reference:
>>> >>>>>>>
>>> >>>>>>>
>>> >>>>>>>
>>> >>>>>>>
>>> >>>>>>
>>> >>>>>
>>> https://github.com/clebertsuconic/activemq-artemis/tree/prototype-log-processor
>>> >>>>>>>
>>> >>>>>>>
>>> >>>>>>>
>>> >>>>>>> the Log processor would read the annotations and generate the
>>> code:
>>> >>>>>>>
>>> >>>>>>>
>>> >>>>>>>
>>> >>>>>>
>>> >>>>>
>>> https://github.com/clebertsuconic/activemq-artemis/blob/prototype-log-processor/artemis-log-processor/src/main/java/org/apache/activemq/artemis/logprocessor/processor/LogProcessor.java
>>> >>>>>>>
>>> >>>>>>>
>>> >>>>>>>
>>> >>>>>>>
>>> >>>>>>> A testcase here would show how such processing would work:
>>> >>>>>>>
>>> >>>>>>>
>>> >>>>>>>
>>> >>>>>>
>>> >>>>>
>>> https://github.com/clebertsuconic/activemq-artemis/blob/prototype-log-processor/artemis-log-processor/src/test/java/org/apache/activemq/i18n/test/SimpleBundleTest.java
>>> >>>>>>>
>>> >>>>>>>
>>> >>>>>>> I have added some code on the artemis-server, trying to simulate
>>> what
>>> >>>>>>> we would do in "real life":
>>> >>>>>>>
>>> >>>>>>>
>>> >>>>>>>
>>> >>>>>>>
>>> >>>>>>
>>> >>>>>
>>> https://github.com/clebertsuconic/activemq-artemis/blob/prototype-log-processor/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/ActiveMQServerNewLogger.java
>>> >>>>>>>
>>> >>>>>>>
>>> >>>>>>>
>>> >>>>>>> This test here is making a call to the NewLogger, just to show how
>>> >>>>>>> processing would work. Everything would work just like it would
>>> now:
>>> >>>>>>>
>>> >>>>>>>
>>> >>>>>>>
>>> >>>>>>
>>> >>>>>
>>> https://github.com/clebertsuconic/activemq-artemis/blob/prototype-log-processor/artemis-server/src/test/java/org/apache/activemq/artemis/core/TestSample.java
>>> >>>>>>>
>>> >>>>>>>
>>> >>>>>>>
>>> >>>>>>>
>>> >>>>>>> I would appreciate some feedback if this is a good way forward or
>>> >>>>> not...
>>> >>>>>>>
>>> >>>>>>> (please let's not diverge on what logging framework we are
>>> choosing
>>> >>>>>>> now... that's a separate discussion).
>>> >>>>>>>
>>> >>>>>>>
>>> >>>>>>>
>>> >>>>>>>
>>> >>>>>>>
>>> >>>>>>> --
>>> >>>>>>> Clebert Suconic
>>> >>>>>>>
>>> >>>>>>>
>>> >>>>>>
>>> >>>>>
>>> >>>>
>>> >>
>>> >>
>>> >>
>>> >> --
>>> >> Clebert Suconic
>>> >
>>> >
>>> >
>>> > --
>>> > Clebert Suconic
>>>
>>>
