Re: Approach for Dropping LogKit in favor of Apache Log4J2 + SLF4J

2016-03-28 Thread Antonio Gomes Rodrigues 
Hi

+1 to remove all old/deprecated/not maintained framework
If it's not made, technical debt will grow and in X years it will be more
difficult (like migrate from ant to maven)

Of course if I can help, I will do it

Antonio

2016-03-28 15:33 GMT+02:00 sebb :

> On 27 March 2016 at 20:00, Philippe Mouawad 
> wrote:
> > Hello,
> > Just for information , within fix of :
> > https://bz.apache.org/bugzilla/show_bug.cgi?id=59240
> >
> > I introduced an slf4j adapter for Logkit.
> > It was not for this subject particularly but more because some very
> > interesting 3rd party logs were dropped by Nop impl (ph-css in this case
> > but it's for every 3rd party logging through slf4j).
> >
> > But thinking about it, a way to move forward is to replace now
> everywhere:
> >  private static final Logger LOG = LoggingManager.getLoggerForClass();
> >
> > by :
> >   private static final Logger LOG = LoggerFactory.getLogger
> > (.class);
>
> No; that is error-prone, tedious to do and unnecessary.
>
> If we really are going to drop the Avalon logger then the obvious way
> to do this is to create a new version of getLoggerForClass() that
> returns the correct Logger.
>
> But as I recall no one has addressed the issues I raised regarding
> loss of functionality.
>
>
> > Regards
> > Philippe
> >
> >
> >
> > On Tue, Jan 5, 2016 at 11:38 AM, sebb  wrote:
> >
> >> On 3 January 2016 at 15:40, Philippe Mouawad <
> philippe.moua...@gmail.com>
> >> wrote:
> >> > Hello,
> >> > I am writing this message to have your opinion on the approach we
> should
> >> > follow.
> >>
> >> This assumes that we are agreed on replacing logging.
> >> I still think this is unnecessary work, and there is a lot to do.
> >>
> >> In any case, I think the work needs to be done in a branch so we can
> >> see how it will work in practise.
> >>
> >> > Current situation:
> >> >
> >> >- log configuration is done in jmeter.properties through:
> >> >   - log_format
> >> >   - log_format_type
> >> >   - log_level.[package_name].[classname]=[PRIORITY_LEVEL]
> >> >   - log_file.[category]=[filename]
> >> >   - log_file=jmeter.log
> >> >   - log_config=logkit.xml
> >> >
> >> >
> >> > I see 2 ways to proceed in implementation:
> >> >
> >> > COMMON PART:
> >> >
> >> >- We keep LoggingManager as is for third party plugins
> >> >- We create a LogTarget implementation that uses SLF4J to make all
> >> >current logging still work.
> >> >- Only this implementation will still use LogKit
> >> >- In next version of JMeter will will drop definitely
> >> >LogKit+Excalibur+Avalon
> >> >
> >> >
> >> > *APPROACH 1:*
> >> > Recode what currently exists for LogKit for Log4j2:
> >> >
> >> >- We create a new class to initialize logging based on Log4j2 and
> >> follow
> >> >one of these approaches:
> >> >   - https://logging.apache.org/log4j/2.x/manual/customconfig.html
> >> >- From my understanding , we should create a custom
> >> >CustomConfigurationFactory that takes Properties as Constructor
> >> passed in
> >> >JMeterUtils#initLogging
> >> >
> >> >
> >> >
> >> > *APPROACH 2:*
> >> > We drop what currently exists and just initialize it simply with a
> Log4j2
> >> > configuration file, we only keep:
> >> >
> >> >- log_file=jmeter.log as it is used by -j command line option
> >> >- log_config=log4j2.xml to configure the logging
> >>
> >> This appears to ignore one of the most useful parts of the config
> >> which allows one to configure logging separately for different
> >> packages and classes.
> >> It is also used by the JMeter menu item Help/Enable|Disable debug
> >>
> >> >
> >> > My opinion is that we should stick to standard mechanism of Log4j2
> that
> >> is
> >> > largelly documented and known and reduce as much as possible any
> custom
> >> > configuration in JMeter.
> >>
> >> Where is it documented how to enable/disable debug for a specific class?
> >>
> >> > This of course breaks backward compatibility but version numbering
> will
> >> be
> >> > explicit about it as long as the release notes.
> >> > Regards
> >> > Philippe M.
> >> > @philmdot
> >>
> >
> >
> >
> > --
> > Cordialement.
> > Philippe Mouawad.
>

Re: Approach for Dropping LogKit in favor of Apache Log4J2 + SLF4J

2016-03-28 Thread sebb 
On 27 March 2016 at 20:00, Philippe Mouawad  wrote:
> Hello,
> Just for information , within fix of :
> https://bz.apache.org/bugzilla/show_bug.cgi?id=59240
>
> I introduced an slf4j adapter for Logkit.
> It was not for this subject particularly but more because some very
> interesting 3rd party logs were dropped by Nop impl (ph-css in this case
> but it's for every 3rd party logging through slf4j).
>
> But thinking about it, a way to move forward is to replace now everywhere:
>  private static final Logger LOG = LoggingManager.getLoggerForClass();
>
> by :
>   private static final Logger LOG = LoggerFactory.getLogger
> (.class);

No; that is error-prone, tedious to do and unnecessary.

If we really are going to drop the Avalon logger then the obvious way
to do this is to create a new version of getLoggerForClass() that
returns the correct Logger.

But as I recall no one has addressed the issues I raised regarding
loss of functionality.


> Regards
> Philippe
>
>
>
> On Tue, Jan 5, 2016 at 11:38 AM, sebb  wrote:
>
>> On 3 January 2016 at 15:40, Philippe Mouawad 
>> wrote:
>> > Hello,
>> > I am writing this message to have your opinion on the approach we should
>> > follow.
>>
>> This assumes that we are agreed on replacing logging.
>> I still think this is unnecessary work, and there is a lot to do.
>>
>> In any case, I think the work needs to be done in a branch so we can
>> see how it will work in practise.
>>
>> > Current situation:
>> >
>> >- log configuration is done in jmeter.properties through:
>> >   - log_format
>> >   - log_format_type
>> >   - log_level.[package_name].[classname]=[PRIORITY_LEVEL]
>> >   - log_file.[category]=[filename]
>> >   - log_file=jmeter.log
>> >   - log_config=logkit.xml
>> >
>> >
>> > I see 2 ways to proceed in implementation:
>> >
>> > COMMON PART:
>> >
>> >- We keep LoggingManager as is for third party plugins
>> >- We create a LogTarget implementation that uses SLF4J to make all
>> >current logging still work.
>> >- Only this implementation will still use LogKit
>> >- In next version of JMeter will will drop definitely
>> >LogKit+Excalibur+Avalon
>> >
>> >
>> > *APPROACH 1:*
>> > Recode what currently exists for LogKit for Log4j2:
>> >
>> >- We create a new class to initialize logging based on Log4j2 and
>> follow
>> >one of these approaches:
>> >   - https://logging.apache.org/log4j/2.x/manual/customconfig.html
>> >- From my understanding , we should create a custom
>> >CustomConfigurationFactory that takes Properties as Constructor
>> passed in
>> >JMeterUtils#initLogging
>> >
>> >
>> >
>> > *APPROACH 2:*
>> > We drop what currently exists and just initialize it simply with a Log4j2
>> > configuration file, we only keep:
>> >
>> >- log_file=jmeter.log as it is used by -j command line option
>> >- log_config=log4j2.xml to configure the logging
>>
>> This appears to ignore one of the most useful parts of the config
>> which allows one to configure logging separately for different
>> packages and classes.
>> It is also used by the JMeter menu item Help/Enable|Disable debug
>>
>> >
>> > My opinion is that we should stick to standard mechanism of Log4j2 that
>> is
>> > largelly documented and known and reduce as much as possible any custom
>> > configuration in JMeter.
>>
>> Where is it documented how to enable/disable debug for a specific class?
>>
>> > This of course breaks backward compatibility but version numbering will
>> be
>> > explicit about it as long as the release notes.
>> > Regards
>> > Philippe M.
>> > @philmdot
>>
>
>
>
> --
> Cordialement.
> Philippe Mouawad.

[GitHub] jmeter pull request: Used multi-catch, enhanced for loops, contain...

2016-03-28 Thread ham1 
Github user ham1 commented on a diff in the pull request:

https://github.com/apache/jmeter/pull/174#discussion_r57560546
  
--- Diff: test/src/org/apache/jmeter/resources/PackageTest.java ---
@@ -136,9 +136,9 @@ private int readRF(String res, List l) throws 
Exception {
  * parameters and check if there is a { in the output. 
A bit
  * crude, but should be enough for now.
  */
-if (val.indexOf("{0}") > 0 && val.indexOf('\'') > 0) {
+if (val.contains("{0}") && val.contains("'")) {
--- End diff --

This change isn't exactly the same as the existing code, however it matches 
the comment, I think the > instead of >= in the original was a mistake.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] jmeter pull request: Used multi-catch, enhanced for loops, contain...

2016-03-28 Thread ham1 
GitHub user ham1 opened a pull request:

https://github.com/apache/jmeter/pull/174

Used multi-catch, enhanced for loops, contains and minor formatting



You can merge this pull request into a Git repository by running:

$ git pull https://github.com/ham1/jmeter java_7

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/jmeter/pull/174.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #174


commit e5bd5a3a4173ee0f4b08c153895f7c0651ef0162
Author: Graham Russell 
Date:   2016-03-21T22:49:45Z

Used multi-catch, enhanced for loops and minor tidy ups while in the files




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---