jmeter-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andrey Pokhilko <a...@ya.ru>
Subject Re: Want to help with migrating from Apache LogKit to SLF4J
Date Tue, 10 Jan 2017 19:06:00 GMT
On 01/10/2017 09:57 PM, sebb wrote:
> On 9 January 2017 at 18:29, Woonsan Ko <woonsan@apache.org> wrote:
>> On Fri, Jan 6, 2017 at 3:23 PM, Philippe Mouawad
>> <philippe.mouawad@gmail.com> wrote:
>>> On Wednesday, January 4, 2017, sebb <sebbaz@gmail.com> wrote:
>>>
>>>> On 3 January 2017 at 20:59, Woonsan Ko <woonsan@apache.org <javascript:;>>
>>>> wrote:
>>>>> On Tue, Jan 3, 2017 at 2:32 PM, Felix Schumacher
>>>>> <felix.schumacher@internetallee.de <javascript:;>> wrote:
>>>>>> Am 02.01.2017 um 21:31 schrieb Philippe Mouawad:
>>>>>>> On Monday, January 2, 2017, Woonsan Ko <woonsan@apache.org
>>>> <javascript:;>> wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> I'd like to help with migrating from Apache LogKit to SLF4J
[1], and
>>>>>>>> so I've been reading the current logging implementation with
logkit,
>>>>>>>> avalon-framework and excalibur-logger.
>>>>>>> Thanks for your proposal
>>>>>> +1
>>>>>>>
>>>>>>>>  From my understanding, maybe we can take the following approach:
>>>>>>>> - Since SLF4J API doesn't provide a logging implementation
or binding
>>>>>>>> by itself, we need to choose one at least such as log4j2
[2] or
>>>>>>>> logback. [3] IMHO, log4j2 binding provided by Apache Logging
services
>>>>>>>> project could be a good default binding option.
>>>>>>>
>>>>>>> +1
>>>>>> Well, I would start with what we have, which is a working binding
for
>>>> SLF4J.
>>>>> It seems more important to migrate each logger usages to use slf4j
>>>>> logger in each class than replacing logging framework in the first
>>>>> step. So, we can keep the current logkit binding in the first step.
>>>>> That prioritization makes sense to me.
>>>>>
>>>>>>>
>>>>>>>> - By the default binding such as log4j2, I mean JMeter should
be
>>>>>>>> bundled with log4j2 library and its binding library as well
as a
>>>>>>>> default configuration file (e.g, log4j2.xml), by default.
It seems
>>>>>>>> neater to separate the logging configuration file from
>>>>>>>> jmeter.properties with simply following its default auto-resolving
>>>>>>>> conventions such as log4j2.xml [4] or logback.xml [5] to
me.
>>>>>>> +1
>>>>>> I am +-0 to any decision right now.
>>>>> This can be revisited with separate ticket after the first step done.
>>>>>
>>>>>>>
>>>>>>>> - It seems like each Logger instance is created as a static
member by
>>>>>>>> `LoggingManager.getLoggerForXXX()' method(s). I suppose all
of those
>>>>>>>> should be replaced by simply using slf4j logger factory as
done in
>>>>>>>> AbstractSampleConsumer.java.
>>>>>>> Yes
>>>>>>>
>>>>>>>> - It might be even better if each logging line is more optimized
by
>>>>>>>> taking advantage of slf4j logging methods (e.g, message format
>>>>>>>> arguments and throwable argument).
>>>>>>> Yes
>>>>>> Plus, if we use formatted messages with arguments, the need for if
>>>>>> (log-is-enabled) statements might be gone for simple cases.
>>>>> Yes.
>>>>>
>>>>>>>> - After all migrated, the old logkit and some other related
unused
>>>>>>>> libraries should be gone.
>>>>>>>
>>>>>>> A possible option to avoid breaking too many existing third party
>>>> plugins
>>>>>>> would be to embed in source code current logkit logger factory
and
>>>> logger
>>>>>>> so that it delegates to slf4j.
>>>>>>> We would drop avalon, logkit and  excalibur jars.
>>>>> Good point. This can be revisited in the later step.
>>>>>
>>>>>>>
>>>>>>>
>>>>>>>> Am I in the right track? Any advice or thoughts?
>>>>>>>
>>>>>>> please wait for other team members to answer before starting
code.
>>>>>>> Give a week .
>>>>>> I would start slowly and contribute drop by drop first, to see if
you
>>>> are
>>>>>> going in the right direction.
>>>>> You're right.
>>>>> Maybe we can split the steps (with separate bz tickets) like the
>>>>> following (ordered by priority):
>>>>> Step 1: Replace logkit loggers with slf4j ones with keeping the
>>>>> current logkit binding solution.
>>>>> Step 2: Optimize logging statements. e.g, message format args,
>>>>> throwable args, unnecessary if-enabled-logging in simple ones, etc.
>> I've created two tickets for the Step 1 and 2:
>> - https://bz.apache.org/bugzilla/show_bug.cgi?id=60564
>> - https://bz.apache.org/bugzilla/show_bug.cgi?id=60565
>>
>> Each looks straightforward. I'd create separate PRs for each bz ticket.
>> But, there's one thing tricky: some classes have public constructor or
>> methods requiring logkit Logger, such as:
>> - o.a.jmeter.engine.ClientJMeterEngine.tidyRMI(Logger)
>> - o.a.jmeter.util.BeanShellInterpreter.BeanShellInterpreter(String, Logger)
>> - o.a.jmeter.protocol.jms.Utils.close(MessageConsumer, Logger)
>> - o.a.jmeter.protocol.jms.Utils.close(Session, Logger)
>> - o.a.jmeter.protocol.jms.Utils.close(Connection, Logger)
>> - o.a.jmeter.protocol.jms.Utils.close(MessageProducer, Logger)
>>
>> I think we have the following options for those:
>> (a) Don't change those for backward compatibility, but we need a
>> wrapper to wrap slf4j logger as logkit logger.
>> (b) Change those to use slf4j logger, breaking bc.
>> (c) or sometimes (a) and sometimes (b)?
>>
>> What do you think? (a) seems safer to me..
> It's obviously safer, but AFAICT it negates the point of migrating?
>
> I'm not sure JMeter has ever promised BC for all classes.
> AFAICR these are mainly or entirely classes that are internal to
> JMeter, with the possible exception of jms.Utils.
> Is it likely that any 3rd party code would need to use these classes directly?
>From my knowledge, no plugins use these specific classes/methods listed
above. FYI Typical logging approach in 3rd party plugins is limited to:
import
org.apache.jorphan.logging.LoggingManager;                                               
                                                                                         
  

import org.apache.log.Logger;
private static final Logger log = LoggingManager.getLoggerForClass();

>>>>> Step 3: Drop avalon, logkit and excalibur with backward compatibility
>>>>> for 3rd party modules. (This step would require discussions about
>>>>> which logging framework/configuration can be used/changed later.)
>>>> I still think it's unnecessary.
>>> I don't share your point.
>>> I think we need to remove the old attic dependencies and use a more up to
>>> date framework.
>>> Besides some like log4j2 have really important perf improvements.
>>> This will also let us reduce code size.
>>>
>>>
>>>> However, the most important aspect is that users are able to use the
>>>> logging system to debug problems.
>>>> This means that there needs to be updated documentation on how to use
>>>> the configuration options.
>>>>
>>>> I would start with the user-facing items.
>>>> i.e documentation
>>> +1
>>>
>>>> and any user-configuration such as menu options.
>>> +0 what exact menu item ? the one that allows settings log level on element
>>> ?
>>>
>>>> Getting the documentation done is critical to the process.
>>>> Doing it first should help tease out any so far unforseen issues.
>>> +1
>> I will try to figure out what's to be done from end users' perspective
>> with some draft possibly.
>> At the moment, it seems to have a menu (Option / Log Viewer) only in
>> UI which probably reads the configuration for where to load the logs
>> from. We will need to figure out how to keep this feature without any
>> problem at least if we use log4j2 later, for instance. Anyway, I think
>> we can consider this after the step 1 and 2 are done.
>> Other UI improvement (e.g, setting log configuration, level, etc) seem
>> to be a separate enhancement to me, not necessarily part of this slf4j
>> migration.
>>
>>>
>>> I'd also suggest short to medium PRs to avoid conflict if we take some time
>>> to integrate them.
>> Yes. I'd ask for reviews with the first PR for a smaller set (e.g,
>> src/protocol/java/**) first in each bz ticket. If okay, continue with
>> the next PR(s) that can include the rest.
>>
>> Regards,
>>
>> Woonsan
>>
>>>>> Regards,
>>>>>
>>>>> Woonsan
>>>>>
>>>>>> Regards,
>>>>>>  Felix
>>>>>>
>>>>>>>> Kind regards,
>>>>>>>>
>>>>>>>> Woonsan
>>>>>>>>
>>>>>>>> [1]
>>>>>>>> https://helpwanted.apache.org/task.html?
>>>> ad91cbf270c711a1c6aa0e67180309
>>>>>>>> d282c81e02
>>>>>>>> [2] https://logging.apache.org/log4j/2.0/log4j-slf4j-impl/index.html
>>>>>>>> [3] http://www.slf4j.org/manual.html
>>>>>>>> [4] https://logging.apache.org/log4j/2.x/manual/
>>>>>>>> configuration.html#Automatic_Configuration
>>>>>>>> [5] http://logback.qos.ch/manual/configuration.html#auto_
>>>> configuration
>>>
>>> --
>>> Cordialement.
>>> Philippe Mouawad.


Mime
View raw message