[GitHub] jmeter issue #254: Feature/bz60589-1 PR for review (logkit to slf4j / log4j2...
Github user pmouawad commented on the issue: https://github.com/apache/jmeter/pull/254 Forget about my comment on jcl --- 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 issue #254: Feature/bz60589-1 PR for review (logkit to slf4j / log4j2...
Github user pmouawad commented on the issue: https://github.com/apache/jmeter/pull/254 I'm not sure commons-logging can be dropped as it's a compile dependency of HttpClient 4.5.3: https://repo1.maven.org/maven2/org/apache/httpcomponents/httpclient/4.5.3/httpclient-4.5.3.pom --- 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 issue #254: Feature/bz60589-1 PR for review (logkit to slf4j / log4j2...
Github user woonsan commented on the issue: https://github.com/apache/jmeter/pull/254 I've fixed more: - Keep the backward compatibility in the Help / En(Dis)able DEBUG logging menu items. - Fixed errors in HttpMirrorServer (executed via mirror-server.sh or mirror-server.bat). It now uses log4j. Also, it provides log level setting through command line arguments (e.g, `-LDEBUG' or `-Ljmeter=DEBUG -Ljorphan=DEBUG'). This command line arguments for log level settings are provided to replace the old log level setting functionality through .properties file loading). It supports `-P' to set port number command line arg as well as the backward compatible the first port number arg (e.g, ./mirror-server.sh ). - Fixed a corner case when GuiLogAppender is used in log4j2.xml while JMeter application properties were not initialized properly. --- 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 issue #254: Feature/bz60589-1 PR for review (logkit to slf4j / log4j2...
Github user woonsan commented on the issue: https://github.com/apache/jmeter/pull/254 @pmouawad , thanks a lot for the quick review! Regarding NewDriver#replaceDateFormatInFileName(), here's the reason why I added that: - As LoggingManager#makeWriter() was gone (https://github.com/apache/jmeter/blob/trunk/src/jorphan/org/apache/jorphan/logging/LoggingManager.java#L174), I needed to have the same feature somewhere else to keep the backward compatibility for the log file name option with date format (http://jmeter.apache.org/usermanual/get-started.html#options) - However, the date format in the log file name option has been broken for long time in my understanding. For example, if I pass "jmeter_'MMddHHmmss'.log" option value, it simply fails because SimpleDateFormat complains about unrecognized format character, 'j', for instance. - Therefore, I rewrote the LoggingManager#makeWriter() method in NewDriver#replaceDateFormatInFileName() in order to extract string inside single quote pair and format / append the value using SimpleDateFormat, not the whole (which leads formatting failure always, resulting log file name option used literally, not evaluated with current date). --- 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 issue #254: Feature/bz60589-1 PR for review (logkit to slf4j / log4j2...
Github user pmouawad commented on the issue: https://github.com/apache/jmeter/pull/254 Hello , Thanks for this big piece of work. I made a first review, it looks good to me. There's one thing I don't clearly understand, what's the use of replaceDateFormatInFileName in NewDriver ? Thanks --- 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 issue #254: Feature/bz60589-1 PR for review (logkit to slf4j / log4j2...
Github user woonsan commented on the issue: https://github.com/apache/jmeter/pull/254 Now, I think this PR is ready for review (bugzilla: https://bz.apache.org/bugzilla/show_bug.cgi?id=60589). Please take a review. --- 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. ---