First, I think the goals are as follows: # Goals G1. Drop dependencies on logkit, exclalibur and logkit. G2. Allow logging configuration with other popular logging framework configuration such as log4j2. log4j2 is the primary one to support. (It should be able to support logback, but that's out of scope in this story. That can be part of other new story later.) G3. Keep the existing UI functionalities. G4. Keep the backward compatibility for 3rd party plugin modules. G5. Ensure all the code use slf4j for logging (exception: usages of logkit logger interface for backward compatibility). Second, let me try to list all the existing logging related features of JMeter # Current Logging Related Features CF1. Users are able to configure log file, logging format, log levels for the combined root logger or for each category in the JMeter configuration properties file or command line arguments (`-j' for log file and `-L' for log level). CF2. Logs by libraries using Commons Logging are passed to the JMeter logging (logkit) by JMeter's Commons Logging implementation. CF3. Advanced Excalibur logging configuration is possible through `log_config' property in JMeter configuration properties. CF3. Logs Viewer UI Menu can be turned on to receive logging event and show in the UI panel (org.apache.jmeter.gui.LoggerPanel). Note this is not reading the log file directly, but receives and renders log events currently propagated by the underlying logging framework at runtime only. CF4. For libraries using log4j API directly, log4j system property is set with `log4j.conf' configuration properties by default, but the logs through log4j is not used by JMeter, it's simply appended to a separate log file. Since the goals include removing dependencies on logkit, excalibur and avalon, and introducing log4j2 logging framework, I'd like to propose the following in the feature level. # Proposal in Feature Level PF1. Users are able to configure log file, logging format, log levels, etc. for combined root logger and for each logger in a separate log4j2 configuration file, ./log4j2.xml or bin/log4j2.xml. PF2. Users are able to change the log4j2 configuration by passing the standard log4j2 system property (e.g, `-Dlog4j2.configuration=file:/var/conf/log4j2-pt1.xml'). PF3. Logs Viewer UI Menu keeps the same functionality as it does. PF4. All the logging related configuration in JMeter configuration properties will be dropped. Also, the logging related command line arguments (`-j' for log file and `-L' for log level) will be dropped. PF5. Advanced Excalibur logging configuration support will be dropped. PF6. JMeter Commons Logging provision will be dropped. Instead jcl-over-slf4j jar dependency will be provided for the same feature. PF7. log4j support through `log4j.configuration' system property and `log4j.conf' will be dropped. Instead Log4j 2's log4j-1.2-api.jar [1] will be provided for backward compatibility. # Candidate solutions - I'll describe solution ideas in comments. ----- [1] https://logging.apache.org/log4j/2.x/manual/migration.html
My solution ideas (roughly): - NewDriver.java can determine (default) log4j2 configuration file location (e.g, first ./log4j2.xml, second bin/log4j2.xml) if log4j2 configuration system property is not explicitly set by users. - Add jcl-over-slf4j jar and Log4j 2's log4j-1.2-api.jar dependencies for Commons Logging and Log4j using libraries. - Remove logkit, excalibur and avalon dependencies. - Fork logkit Logger interface with a wrapper class for slf4j logger for backward compatibility in jorphan. So we can remove logkit dependencies in maven pom as well. - Change LoggingManager only for using wrappers for slf4j. Drop logging initialization. - Remove all the logging related JMeter configuration properties and command line arguments support. - Provide logging event listener for the Logs Viewer UI feature). (This can be a log4j2 listener implementation since I don't see any listener support by slf4j itself (yet).)
(In reply to Woonsan Ko from comment #1) > - Provide logging event listener for the Logs Viewer UI feature). > (This can be a log4j2 listener implementation since I don't see any > listener support by slf4j itself (yet).) One solution is to provide a custom log4j2 Appender implementation, as the most minimal direct log4j2 (runtime) dependency, for the Logs Viewer UI. So, users are supposed to configure this custom appender and reference in to root logger to keep the same functionality. The custom appender can possibly propagate events with the logging events.
Thanks a lot for this full and great analysis ! I am ok with all points. Wait a bit for other committers feedback before starting work. Thank you Regards
I've created an initial pull request. This is not ready for merging, but only for initial review on the direction: - https://github.com/apache/jmeter/pull/254 This PR contains only the following at the moment: - Removing dependencies on logkit, excalibur and avalon. - Forking some logkit classes in jorphan. - NewDriver.java setting log4j2 configuration file location system property. - Add jcl-over-slf4j, log4j-1.2-api, jcl-over-slf4j and log4j2 jars. - Update LoggingManager to return slf4j logger wrapper. It doesn't include a custom appender for the UI and unit test fixes yet. Also requires cleanups. But it seems working at runtime with log4j2 now in this steps: $ git clone -b feature/bz60589-1 https://github.com/woonsan/jmeter.git $ cd jmeter $ ant download_jars $ ant $ cd bin $ ./jmeter Now, I see jmeter.log being added by log4j2 via logkit fork Logger (wrapping slf4j logger) / LoggingManager. Please review the PR and share your thoughts on the direction. P.S. The original logkit Logger is a class (not an interface) with final methods. So, I decided to remove the final modifier and add abstract modifier for overridable methods (for wrapper impl). And I kept unsupportable methods as NOP impl.
(In reply to Woonsan Ko from comment #4) > I've created an initial pull request. This is not ready for merging, but > only for initial review on the direction: > - https://github.com/apache/jmeter/pull/254 > > This PR contains only the following at the moment: > - Removing dependencies on logkit, excalibur and avalon. > - Forking some logkit classes in jorphan. > - NewDriver.java setting log4j2 configuration file location system property. > - Add jcl-over-slf4j, log4j-1.2-api, jcl-over-slf4j and log4j2 jars. > - Update LoggingManager to return slf4j logger wrapper. > > It doesn't include a custom appender for the UI and unit test fixes yet. > Also requires cleanups. > > But it seems working at runtime with log4j2 now in this steps: > $ git clone -b feature/bz60589-1 https://github.com/woonsan/jmeter.git > $ cd jmeter > $ ant download_jars > $ ant > $ cd bin > $ ./jmeter > Now, I see jmeter.log being added by log4j2 via logkit fork Logger (wrapping > slf4j logger) / LoggingManager. > > Please review the PR and share your thoughts on the direction. > > P.S. The original logkit Logger is a class (not an interface) with final > methods. So, I decided to remove the final modifier and add abstract > modifier for overridable methods (for wrapper impl). And I kept > unsupportable methods as NOP impl. The direction looks good to me. Thanks for your work. I suppose last step is having: - custom appender for the UI - The ability to set debug level through the menu - Fix Test Units Regards
Hi, I think the PR is now ready for review: - https://github.com/apache/jmeter/pull/254 Here's what I did with this PR in summary: - Removing dependencies on logkit, excalibur, avalon and commons-logging. - Adding slf4j and log4j dependencies. - Forking minimal logkit interfaces/classes in jorphan module. - NewDriver.java setting system properties for log4j2 configuration file location and the log file path (referenced by system property lookup in log4j2.xml files). - Migrating logging properties (in jmeter*.properties) to log4j2*.xml files with equivalent comments and examples. - Remove logkit initialization in LoggingManager and update it to return slf4j logger wrapper simply. - Fixing some unit tests accordingly as logkit LogTarget is not available in unit testing any more. - Implement LoggerPanel based on new logging mechanism for full backward compatibility. - Keep supporting -j command line option to allow log file setting. Almost full backward compatibility. 'LAST' value is dropped since this option handling happens before GUI initialized. - Keep supporting -L command line option to allow log level setting. Full backward compatibility. - Introduce -i command line option to allow to change log4j2 configuration file location by command line argument as well as the default log4j2 system property (log4j.configurationFile). - Minimal updates in getting started documentation.
(In reply to Philippe Mouawad from comment #5) > I suppose last step is having: > - custom appender for the UI It was added and configured in the default log4j2*.xml files. > - The ability to set debug level through the menu I'm not sure if we want to do this with this ticket because the UI menu to change log level looks like a new improvement, not part of migration. The PR includes backward compatible option support to change log level (`-L DEBUG' for example). I'd personally like to improve the UI menu for log level setting in a new ticket. > - Fix Test Units Yes, everything seems to be fixed. Thanks, Woonsan
(In reply to Woonsan Ko from comment #7) > (In reply to Philippe Mouawad from comment #5) > > - The ability to set debug level through the menu > > I'm not sure if we want to do this with this ticket because the UI menu to > change log level looks like a new improvement, not part of migration. > The PR includes backward compatible option support to change log level (`-L > DEBUG' for example). I'd personally like to improve the UI menu for log > level setting in a new ticket. I've created a new ticket for the UI menu to allow log level changes: - https://bz.apache.org/bugzilla/show_bug.cgi?id=60664 Woonsan
Author: pmouawad Date: Sun Feb 5 13:42:22 2017 New Revision: 1781756 URL: http://svn.apache.org/viewvc?rev=1781756&view=rev Log: Bug 60589 - Migrate LogKit to SLF4J - Drop avalon, logkit and excalibur with backward compatibility for 3rd party modules Part 1 of PR #254 Contributed by Woonsan Ko Bugzilla Id: 60589 Added: jmeter/trunk/src/core/org/apache/jmeter/gui/logging/ jmeter/trunk/src/core/org/apache/jmeter/gui/logging/GuiLogEventAppender.java (with props) jmeter/trunk/src/core/org/apache/jmeter/gui/logging/GuiLogEventBus.java (with props) jmeter/trunk/src/core/org/apache/jmeter/gui/logging/GuiLogEventListener.java (with props) jmeter/trunk/src/core/org/apache/jmeter/gui/logging/LogEventObject.java (with props) Modified: jmeter/trunk/src/core/org/apache/jmeter/JMeter.java jmeter/trunk/src/core/org/apache/jmeter/NewDriver.java jmeter/trunk/src/core/org/apache/jmeter/gui/GuiPackage.java jmeter/trunk/src/core/org/apache/jmeter/gui/LoggerPanel.java jmeter/trunk/src/core/org/apache/jmeter/gui/MainFrame.java jmeter/trunk/src/core/org/apache/jmeter/gui/action/What.java jmeter/trunk/src/core/org/apache/jmeter/samplers/SampleResult.java jmeter/trunk/src/core/org/apache/jmeter/util/JMeterUtils.java
Author: pmouawad Date: Sun Feb 5 13:43:13 2017 New Revision: 1781757 URL: http://svn.apache.org/viewvc?rev=1781757&view=rev Log: Bug 60589 - Migrate LogKit to SLF4J - Drop avalon, logkit and excalibur with backward compatibility for 3rd party modules Part 2 of PR #254 Contributed by Woonsan Ko Bugzilla Id: 60589 Added: jmeter/trunk/src/jorphan/org/apache/jorphan/logging/Slf4jLogkitLogger.java (with props) jmeter/trunk/src/jorphan/org/apache/log/ jmeter/trunk/src/jorphan/org/apache/log/ContextMap.java (with props) jmeter/trunk/src/jorphan/org/apache/log/LogEvent.java (with props) jmeter/trunk/src/jorphan/org/apache/log/LogTarget.java (with props) jmeter/trunk/src/jorphan/org/apache/log/Logger.java (with props) jmeter/trunk/src/jorphan/org/apache/log/Priority.java (with props) Modified: jmeter/trunk/src/jorphan/org/apache/jorphan/logging/LoggingManager.java
Author: pmouawad Date: Sun Feb 5 13:44:09 2017 New Revision: 1781758 URL: http://svn.apache.org/viewvc?rev=1781758&view=rev Log: Bug 60589 - Migrate LogKit to SLF4J - Drop avalon, logkit and excalibur with backward compatibility for 3rd party modules Part 3 of PR #254 Contributed by Woonsan Ko Bugzilla Id: 60589 Modified: jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/HttpMirrorServer.java
Author: pmouawad Date: Sun Feb 5 13:45:14 2017 New Revision: 1781759 URL: http://svn.apache.org/viewvc?rev=1781759&view=rev Log: Bug 60589 - Migrate LogKit to SLF4J - Drop avalon, logkit and excalibur with backward compatibility for 3rd party modules Part 4 of PR #254 Contributed by Woonsan Ko Bugzilla Id: 60589 Added: jmeter/trunk/test/src/org/apache/jmeter/gui/logging/ jmeter/trunk/test/src/org/apache/jmeter/gui/logging/TestGuiLogEventAppender.java (with props) jmeter/trunk/test/src/org/apache/jmeter/util/LogRecord.java (with props) jmeter/trunk/test/src/org/apache/jmeter/util/LogRecordingDelegatingLogger.java (with props) Modified: jmeter/trunk/test/src/org/apache/jmeter/JMeterVersionTest.java jmeter/trunk/test/src/org/apache/jmeter/samplers/TestSampleResult.java jmeter/trunk/test/src/org/apache/jorphan/reflect/TestFunctor.java jmeter/trunk/test/src/org/apache/jorphan/test/AllTests.java
Author: pmouawad Date: Sun Feb 5 13:46:09 2017 New Revision: 1781760 URL: http://svn.apache.org/viewvc?rev=1781760&view=rev Log: Bug 60589 - Migrate LogKit to SLF4J - Drop avalon, logkit and excalibur with backward compatibility for 3rd party modules Part 5 of PR #254 Contributed by Woonsan Ko Bugzilla Id: 60589 Added: jmeter/trunk/bin/testfiles/log4j2-batch.xml (with props) Removed: jmeter/trunk/bin/log4j.conf jmeter/trunk/bin/logkit.xml Modified: jmeter/trunk/bin/jmeter.properties jmeter/trunk/bin/mirror-server.cmd jmeter/trunk/bin/mirror-server.sh jmeter/trunk/bin/testfiles/jmeter-batch.properties jmeter/trunk/bin/user.properties
Author: pmouawad Date: Sun Feb 5 13:49:29 2017 New Revision: 1781761 URL: http://svn.apache.org/viewvc?rev=1781761&view=rev Log: Bug 60589 - Migrate LogKit to SLF4J - Drop avalon, logkit and excalibur with backward compatibility for 3rd party modules Part 6 of PR #254 Contributed by Woonsan Ko Bugzilla Id: 60589 Removed: jmeter/trunk/res/maven/ApacheJMeter_slf4j_logkit.pom Author: pmouawad Date: Sun Feb 5 13:51:56 2017 New Revision: 1781762 URL: http://svn.apache.org/viewvc?rev=1781762&view=rev Log: Bug 60589 - Migrate LogKit to SLF4J - Drop avalon, logkit and excalibur with backward compatibility for 3rd party modules Part 7 of PR #254 Contributed by Woonsan Ko Removed: jmeter/trunk/src/slf4j-logkit/
Author: pmouawad Date: Sun Feb 5 13:55:33 2017 New Revision: 1781763 URL: http://svn.apache.org/viewvc?rev=1781763&view=rev Log: Bug 60589 - Migrate LogKit to SLF4J - Drop avalon, logkit and excalibur with backward compatibility for 3rd party modules Part 8 of PR 254 This closes #254 Contributed by Woonsan Ko Bugzilla Id: 60589 Added: jmeter/trunk/licenses/bin/jcl-over-slf4j-1.7.22.txt (with props) jmeter/trunk/licenses/bin/slf4j-ext-1.7.22.txt (with props) Modified: jmeter/trunk/LICENSE jmeter/trunk/build.properties jmeter/trunk/build.xml jmeter/trunk/eclipse.classpath jmeter/trunk/lib/ (props changed) jmeter/trunk/lib/aareadme.txt jmeter/trunk/licenses/bin/README.txt jmeter/trunk/res/maven/ApacheJMeter_parent.pom jmeter/trunk/xdocs/changes.xml Author: pmouawad Date: Sun Feb 5 13:56:05 2017 New Revision: 1781764 URL: http://svn.apache.org/viewvc?rev=1781764&view=rev Log: Bug 60589 - Migrate LogKit to SLF4J - Drop avalon, logkit and excalibur with backward compatibility for 3rd party modules Remove mention of commons-logging in docs Bugzilla Id: 60589 Modified: jmeter/trunk/xdocs/usermanual/get-started.xml
Author: pmouawad Date: Sun Feb 5 14:09:25 2017 New Revision: 1781765 URL: http://svn.apache.org/viewvc?rev=1781765&view=rev Log: Bug 60589 - Migrate LogKit to SLF4J - Drop avalon, logkit and excalibur with backward compatibility for 3rd party modules Part 9 of PR 254 Add missing log4j2.xml under svn Contributed by Woonsan Ko Bugzilla Id: 60589 Added: jmeter/trunk/bin/log4j2.xml (with props)
This issue has been migrated to GitHub: https://github.com/apache/jmeter/issues/4226