[GitHub] incubator-rocketmq pull request #120: [ROCKETMQ-224] add rocketmq client log...

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/incubator-rocketmq/pull/120


---
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] incubator-rocketmq pull request #120: [ROCKETMQ-224] add rocketmq client log...

[vsair]

Github user vsair commented on a diff in the pull request:

https://github.com/apache/incubator-rocketmq/pull/120#discussion_r130556323
  
--- Diff: 
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) {
--- End diff --

why not modify the init method?


---
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] incubator-rocketmq pull request #120: [ROCKETMQ-224] add rocketmq client log...

[vongosling]

Github user vongosling commented on a diff in the pull request:

https://github.com/apache/incubator-rocketmq/pull/120#discussion_r130550009
  
--- Diff: 
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
--- End diff --

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


---
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] incubator-rocketmq pull request #120: [ROCKETMQ-224] add rocketmq client log...

[lindzh]

Github user lindzh commented on a diff in the pull request:

https://github.com/apache/incubator-rocketmq/pull/120#discussion_r130544050
  
--- Diff: 
client/src/main/java/org/apache/rocketmq/client/log/ClientLogger.java ---
@@ -27,16 +28,19 @@
 public static final String CLIENT_LOG_ROOT = "rocketmq.client.logRoot";
 public static final String CLIENT_LOG_MAXINDEX = 
"rocketmq.client.logFileMaxIndex";
 public static final String CLIENT_LOG_LEVEL = 
"rocketmq.client.logLevel";
+
 private static Logger log;
 
-static {
-log = createLogger(LoggerName.CLIENT_LOGGER_NAME);
+public static Class logClass = null;
--- End diff --

Fixed it with reflect


---
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] incubator-rocketmq pull request #120: [ROCKETMQ-224] add rocketmq client log...

[lizhanhui]

Github user lizhanhui commented on a diff in the pull request:

https://github.com/apache/incubator-rocketmq/pull/120#discussion_r130515522
  
--- Diff: 
client/src/main/java/org/apache/rocketmq/client/log/ClientLogger.java ---
@@ -27,16 +28,19 @@
 public static final String CLIENT_LOG_ROOT = "rocketmq.client.logRoot";
 public static final String CLIENT_LOG_MAXINDEX = 
"rocketmq.client.logFileMaxIndex";
 public static final String CLIENT_LOG_LEVEL = 
"rocketmq.client.logLevel";
+
 private static Logger log;
 
-static {
-log = createLogger(LoggerName.CLIENT_LOGGER_NAME);
+public static Class logClass = null;
--- End diff --

Add logClass and its getter for test purpose only?


---
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] incubator-rocketmq pull request #120: [ROCKETMQ-224] add rocketmq client log...

[lizhanhui]

Github user lizhanhui commented on a diff in the pull request:

https://github.com/apache/incubator-rocketmq/pull/120#discussion_r130515214
  
--- Diff: 
client/src/test/java/org/apache/rocketmq/client/log/ClientLogTest.java ---
@@ -0,0 +1,67 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.rocketmq.client.log;
+
+import junit.framework.Assert;
--- End diff --

We'd better use org.junit.Assert.


---
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] incubator-rocketmq pull request #120: [ROCKETMQ-224] add rocketmq client log...

[lindzh]

Github user lindzh commented on a diff in the pull request:

https://github.com/apache/incubator-rocketmq/pull/120#discussion_r124972804
  
--- Diff: 
client/src/test/java/org/apache/rocketmq/client/log/ClientLogTest.java ---
@@ -0,0 +1,30 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.rocketmq.client.log;
+
+import org.junit.Test;
+
+import java.util.Date;
+
+public class ClientLogTest {
+
+@Test
+public void testLog4j2() {
+ClientLogger.getLog().info("logg4j test " + new Date());
--- End diff --

Assert has been added.


---
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] incubator-rocketmq pull request #120: [ROCKETMQ-224] add rocketmq client log...

[zhouxinyu]

Github user zhouxinyu commented on a diff in the pull request:

https://github.com/apache/incubator-rocketmq/pull/120#discussion_r124163372
  
--- Diff: 
client/src/test/java/org/apache/rocketmq/client/log/ClientLogTest.java ---
@@ -0,0 +1,30 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.rocketmq.client.log;
+
+import org.junit.Test;
+
+import java.util.Date;
+
+public class ClientLogTest {
+
+@Test
+public void testLog4j2() {
+ClientLogger.getLog().info("logg4j test " + new Date());
--- End diff --

Any assert or thrown exception?


---
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] incubator-rocketmq pull request #120: [ROCKETMQ-224] add rocketmq client log...

[zhouxinyu]

Github user zhouxinyu commented on a diff in the pull request:

https://github.com/apache/incubator-rocketmq/pull/120#discussion_r124163160
  
--- Diff: 
client/src/main/java/org/apache/rocketmq/client/log/ClientLogger.java ---
@@ -35,16 +36,19 @@
 
 private static Logger createLogger(final String loggerName) {
 String logConfigFilePath =
-System.getProperty("rocketmq.client.log.configFile",
-System.getenv("ROCKETMQ_CLIENT_LOG_CONFIGFILE"));
+System.getProperty("rocketmq.client.log.configFile",
+System.getenv("ROCKETMQ_CLIENT_LOG_CONFIGFILE"));
--- End diff --

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


---
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] incubator-rocketmq pull request #120: [ROCKETMQ-224] add rocketmq client log...

[lindzh]

Github user lindzh commented on a diff in the pull request:

https://github.com/apache/incubator-rocketmq/pull/120#discussion_r123936939
  
--- Diff: client/pom.xml ---
@@ -37,13 +37,31 @@
 ${project.groupId}
 rocketmq-common
 
+
 
 org.slf4j
 slf4j-api
 
+
 
 org.apache.commons
 commons-lang3
 
+
+
+org.apache.logging.log4j
+log4j-core
+test
+
+
+org.apache.logging.log4j
+log4j-api
+test
+
+
+org.apache.logging.log4j
+log4j-slf4j-impl
--- End diff --

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


---
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] incubator-rocketmq pull request #120: [ROCKETMQ-224] add rocketmq client log...

[vongosling]

Github user vongosling commented on a diff in the pull request:

https://github.com/apache/incubator-rocketmq/pull/120#discussion_r123713735
  
--- Diff: client/pom.xml ---
@@ -37,13 +37,31 @@
 ${project.groupId}
 rocketmq-common
 
+
 
 org.slf4j
 slf4j-api
 
+
 
 org.apache.commons
 commons-lang3
 
+
+
+org.apache.logging.log4j
+log4j-core
+test
+
+
+org.apache.logging.log4j
+log4j-api
+test
+
+
+org.apache.logging.log4j
+log4j-slf4j-impl
--- End diff --

Could we  put log4j2 test in rocketmq example module ?


---
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] incubator-rocketmq pull request #120: [ROCKETMQ-224] add rocketmq client log...

[vongosling]

Github user vongosling commented on a diff in the pull request:

https://github.com/apache/incubator-rocketmq/pull/120#discussion_r123714561
  
--- Diff: pom.xml ---
@@ -638,6 +638,16 @@
 log4j-core
 2.7
 
+
+org.apache.logging.log4j
--- End diff --

Why could i depend log4j2 core directly in dependency management?


---
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] incubator-rocketmq pull request #120: [ROCKETMQ-224] add rocketmq client log...

[lindzh]

GitHub user lindzh reopened a pull request:

https://github.com/apache/incubator-rocketmq/pull/120

[ROCKETMQ-224] add rocketmq client log4j2 logging support

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


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

$ git pull https://github.com/lindzh/incubator-rocketmq fix_client_logger

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

https://github.com/apache/incubator-rocketmq/pull/120.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 #120


commit 4b4666a5b3279d66bbc1406ec9537b3383c0cdcc
Author: 鲁般 
Date:   2017-06-15T07:31:52Z

add rocketmq client log4j2 logging support




---
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] incubator-rocketmq pull request #120: [ROCKETMQ-224] add rocketmq client log...

[lindzh]

Github user lindzh closed the pull request at:

https://github.com/apache/incubator-rocketmq/pull/120


---
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] incubator-rocketmq pull request #120: [ROCKETMQ-224] add rocketmq client log...

[lindzh]

GitHub user lindzh opened a pull request:

https://github.com/apache/incubator-rocketmq/pull/120

[ROCKETMQ-224] add rocketmq client log4j2 logging support

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


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

$ git pull https://github.com/lindzh/incubator-rocketmq fix_client_logger

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

https://github.com/apache/incubator-rocketmq/pull/120.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 #120


commit 4b4666a5b3279d66bbc1406ec9537b3383c0cdcc
Author: 鲁般 
Date:   2017-06-15T07:31:52Z

add rocketmq client log4j2 logging support




---
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.
---