[ROCKETMQ-224] add rocketmq client log4j2 logging support #120

lindzh · 2017-06-15T07:43:30Z

When using RocketMQ client,we can only using logback or log4j for logging. If we using log4j2,there is no client log.

coveralls · 2017-06-15T08:14:01Z

Coverage decreased (-0.1%) to 38.464% when pulling 4b4666a on lindzh:fix_client_logger into 0c5e53d on apache:master.

coveralls · 2017-06-15T08:14:01Z

Coverage decreased (-0.1%) to 38.464% when pulling 4b4666a on lindzh:fix_client_logger into 0c5e53d on apache:master.

coveralls · 2017-06-15T08:38:53Z

Coverage increased (+0.7%) to 39.271% when pulling 4b4666a on lindzh:fix_client_logger into 0c5e53d on apache:master.

coveralls · 2017-06-15T08:38:53Z

Coverage increased (+0.7%) to 39.271% when pulling 4b4666a on lindzh:fix_client_logger into 0c5e53d on apache:master.

coveralls · 2017-06-15T08:38:53Z

Coverage increased (+0.7%) to 39.271% when pulling 4b4666a on lindzh:fix_client_logger into 0c5e53d on apache:master.

vongosling · 2017-06-23T09:58:43Z

client/pom.xml

+        </dependency>
+        <dependency>
+            <groupId>org.apache.logging.log4j</groupId>
+            <artifactId>log4j-slf4j-impl</artifactId>


Could we put log4j2 test in rocketmq example module ?

As log4j is widely used in example,add this test in example will make it confused to choose log moudle.

zhouxinyu · 2017-06-27T02:03:19Z

client/src/main/java/org/apache/rocketmq/client/log/ClientLogger.java

-            System.getProperty("rocketmq.client.log.configFile",
-                System.getenv("ROCKETMQ_CLIENT_LOG_CONFIGFILE"));
+                System.getProperty("rocketmq.client.log.configFile",
+                        System.getenv("ROCKETMQ_CLIENT_LOG_CONFIGFILE"));


Did you use the right code style? http://rocketmq.apache.org/docs/code-guidelines/

zhouxinyu · 2017-06-27T02:05:38Z

client/src/test/java/org/apache/rocketmq/client/log/ClientLogTest.java

+
+    @Test
+    public void testLog4j2() {
+        ClientLogger.getLog().info("logg4j test " + new Date());


Any assert or thrown exception?

Assert has been added.

coveralls · 2017-06-30T08:13:02Z

Coverage increased (+0.6%) to 39.189% when pulling 6ab4ad4 on lindzh:fix_client_logger into 0c5e53d on apache:master.

coveralls · 2017-06-30T08:13:02Z

Coverage increased (+0.6%) to 39.189% when pulling 6ab4ad4 on lindzh:fix_client_logger into 0c5e53d on apache:master.

coveralls · 2017-06-30T08:13:02Z

Coverage increased (+0.6%) to 39.189% when pulling 6ab4ad4 on lindzh:fix_client_logger into 0c5e53d on apache:master.

coveralls · 2017-07-28T08:35:06Z

Coverage increased (+0.6%) to 39.21% when pulling 04e5a1e on lindzh:fix_client_logger into 0c5e53d on apache:master.

coveralls · 2017-07-28T08:35:06Z

Coverage increased (+0.6%) to 39.21% when pulling 04e5a1e on lindzh:fix_client_logger into 0c5e53d on apache:master.

coveralls · 2017-07-28T08:35:06Z

Coverage increased (+0.6%) to 39.21% when pulling 04e5a1e on lindzh:fix_client_logger into 0c5e53d on apache:master.

lizhanhui · 2017-08-01T03:59:33Z

client/src/test/java/org/apache/rocketmq/client/log/ClientLogTest.java

+
+package org.apache.rocketmq.client.log;
+
+import junit.framework.Assert;


We'd better use org.junit.Assert.

lizhanhui · 2017-08-01T04:03:50Z

client/src/main/java/org/apache/rocketmq/client/log/ClientLogger.java

    private static Logger log;

-    static {
-        log = createLogger(LoggerName.CLIENT_LOGGER_NAME);
+    public static Class logClass = null;


Add logClass and its getter for test purpose only?

Fixed it with reflect

vongosling · 2017-08-01T08:40:12Z

client/src/test/java/org/apache/rocketmq/client/log/ClientLogTest.java

@@ -32,15 +33,19 @@
        LOG_DIR = System.getProperty(CLIENT_LOG_ROOT, "${user.home}/logs/rocketmqlogs");
    }

+    // FIXME: Workarond for concret implementation for slf4j, is there any better solution for all slf4j implementations in one class ? 2017/8/1


we'd better remember this task and fix it later.

vongosling · 2017-08-01T08:44:55Z

Now, LGTM. this pr also fix the rocketmq's logger appender bug when using any concrete implementation, no matter log4j, log4j2 or logback.

coveralls · 2017-08-01T08:58:30Z

Coverage increased (+0.2%) to 38.771% when pulling 0ea6994 on lindzh:fix_client_logger into 0c5e53d on apache:master.

coveralls · 2017-08-01T08:58:30Z

Coverage increased (+0.2%) to 38.771% when pulling 0ea6994 on lindzh:fix_client_logger into 0c5e53d on apache:master.

coveralls · 2017-08-01T08:58:30Z

Coverage increased (+0.2%) to 38.771% when pulling 0ea6994 on lindzh:fix_client_logger into 0c5e53d on apache:master.

vsair · 2017-08-01T09:10:27Z

client/src/main/java/org/apache/rocketmq/client/log/ClientLogger.java

@@ -99,7 +111,12 @@ private static Logger createLogger(final String loggerName) {
    }

    public static Logger getLog() {
-        return log;
+        if (log == null) {


why modify the init method?

Issue apache#120 update brokerStatus See merge request !12

Author: 鲁般 <dezhi.ldz@alibaba-inc.com> Closes apache#120 from lindzh/fix_client_logger.

add rocketmq client log4j2 logging support

4b4666a

lindzh closed this Jun 15, 2017

lindzh reopened this Jun 15, 2017

vongosling reviewed Jun 23, 2017

View reviewed changes

zhouxinyu reviewed Jun 27, 2017

View reviewed changes

lindzh added 3 commits June 30, 2017 14:30

fix log4j2 client log test case

96bfcdd

fix style

fcc42ee

fix read line

6ab4ad4

zhouxinyu approved these changes Jul 19, 2017

View reviewed changes

fix logger and appender null

04e5a1e

lizhanhui reviewed Aug 1, 2017

View reviewed changes

fix client logger static field and test case

0ea6994

vongosling reviewed Aug 1, 2017

View reviewed changes

vsair reviewed Aug 1, 2017

View reviewed changes

asfgit closed this in 98bd032 Aug 1, 2017

lizhanhui pushed a commit to lizhanhui/rocketmq that referenced this pull request Jun 25, 2019

Issue apache#120 update brokerStatus

1ef6f3d

lizhanhui pushed a commit to lizhanhui/rocketmq that referenced this pull request Jun 25, 2019

Merge branch 'shijia_0919' into 'master'

e3f3cdd

Issue apache#120 update brokerStatus See merge request !12

JiaMingLiu93 pushed a commit to JiaMingLiu93/rocketmq that referenced this pull request May 28, 2020

Author: lindzh <linsony0@163.com>

ec2319b

Author: 鲁般 <dezhi.ldz@alibaba-inc.com> Closes apache#120 from lindzh/fix_client_logger.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ROCKETMQ-224] add rocketmq client log4j2 logging support #120

[ROCKETMQ-224] add rocketmq client log4j2 logging support #120

lindzh commented Jun 15, 2017

coveralls commented Jun 15, 2017

coveralls commented Jun 15, 2017

coveralls commented Jun 15, 2017

coveralls commented Jun 15, 2017

coveralls commented Jun 15, 2017

vongosling Jun 23, 2017

lindzh Jun 26, 2017

zhouxinyu Jun 27, 2017

zhouxinyu Jun 27, 2017

lindzh Jun 30, 2017

coveralls commented Jun 30, 2017

coveralls commented Jun 30, 2017

coveralls commented Jun 30, 2017

coveralls commented Jul 28, 2017

coveralls commented Jul 28, 2017

coveralls commented Jul 28, 2017

lizhanhui Aug 1, 2017

lizhanhui Aug 1, 2017

lindzh Aug 1, 2017

vongosling Aug 1, 2017

vongosling commented Aug 1, 2017

coveralls commented Aug 1, 2017

coveralls commented Aug 1, 2017

coveralls commented Aug 1, 2017

vsair Aug 1, 2017 •

edited


		package org.apache.rocketmq.client.log;

		import junit.framework.Assert;

[ROCKETMQ-224] add rocketmq client log4j2 logging support #120

[ROCKETMQ-224] add rocketmq client log4j2 logging support #120

Conversation

lindzh commented Jun 15, 2017

coveralls commented Jun 15, 2017

coveralls commented Jun 15, 2017

coveralls commented Jun 15, 2017

coveralls commented Jun 15, 2017

coveralls commented Jun 15, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Jun 30, 2017

coveralls commented Jun 30, 2017

coveralls commented Jun 30, 2017

coveralls commented Jul 28, 2017

coveralls commented Jul 28, 2017

coveralls commented Jul 28, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vongosling commented Aug 1, 2017

coveralls commented Aug 1, 2017

coveralls commented Aug 1, 2017

coveralls commented Aug 1, 2017

vsair Aug 1, 2017 • edited

Choose a reason for hiding this comment

vsair Aug 1, 2017 •

edited