You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2021/03/02 07:46:53 UTC

[GitHub] [kafka] dongjinleekr opened a new pull request #10244: KAFKA-12399: Add log4j2 Appender

dongjinleekr opened a new pull request #10244:
URL: https://github.com/apache/kafka/pull/10244


   This PR implements `log4j2-appender`, a log4j2 equivalent of traditional `log4j-appender`.
   
   All `log4j-appender` configuration properties are supported, with some additions:
   
   1. `brokerList` is deprecated for the consistency of the other CLI tools. `bootstrapServers` is added instead.
   2. `requiredNumAcks` is deprecated for the consistency of `Producer`'s `Properties` instance. `acks` is added instead.
   3. A new configuration option, `producerClass`, is added. Any `Producer` implementation with the `Properties` argument is supported. The unit test itself uses this feature with `MockProducer`.
   4. In `log4j-appender`, the default values of `retries`, `requiredNumAcks`, `deliveryTimeoutMs`, `lingerMs`, and `batchSize` are redundantly defined in `Log4jAppender` implementation. Since their default values are already defined in `ProducerConfig`, `log4j2-appender` does not define their default values.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] dongjinleekr commented on pull request #10244: KAFKA-12399: Deprecate Log4J Appender

Posted by GitBox <gi...@apache.org>.
dongjinleekr commented on pull request #10244:
URL: https://github.com/apache/kafka/pull/10244#issuecomment-1073969635


   @mimaison Here is the update, with a migration guide! After some trial and error, I decided that a full comparison table would help.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] dongjinleekr commented on pull request #10244: KAFKA-12399: Deprecate Log4J Appender

Posted by GitBox <gi...@apache.org>.
dongjinleekr commented on pull request #10244:
URL: https://github.com/apache/kafka/pull/10244#issuecomment-1073969969


   @mimaison Here is the update, with a migration guide! After some trial and error, I concluded that a full comparison table would help.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] dongjinleekr commented on a change in pull request #10244: KAFKA-12399: Deprecate Log4J Appender

Posted by GitBox <gi...@apache.org>.
dongjinleekr commented on a change in pull request #10244:
URL: https://github.com/apache/kafka/pull/10244#discussion_r830463813



##########
File path: log4j-appender/src/main/java/org/apache/kafka/log4jappender/KafkaLog4jAppender.java
##########
@@ -333,6 +333,7 @@ public void activateOptions() {
         props.put(KEY_SERIALIZER_CLASS_CONFIG, ByteArraySerializer.class.getName());
         props.put(VALUE_SERIALIZER_CLASS_CONFIG, ByteArraySerializer.class.getName());
         this.producer = getKafkaProducer(props);
+        LogLog.warn("log4j-appender is deprecated and will be removed in a future release. For migration, please refer to the latest documentation.");

Review comment:
       Well, although this class will not be instantiated directly, it would be much better.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] dongjinleekr commented on a change in pull request #10244: KAFKA-12399: Deprecate Log4J Appender

Posted by GitBox <gi...@apache.org>.
dongjinleekr commented on a change in pull request #10244:
URL: https://github.com/apache/kafka/pull/10244#discussion_r830464206



##########
File path: bin/kafka-run-class.sh
##########
@@ -45,6 +45,45 @@ should_include_file() {
   fi
 }
 
+# As of present, the migration to log4j2 is not completed, so some modules are using log4j 1.x, and

Review comment:
       Since KAFKA-12399 and KAFKA-9366 will be released at once, we don't need `USE_LOG4J2_JAR` flag anymore; so removed.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] dongjinleekr commented on pull request #10244: KAFKA-12399: Add log4j2 Appender

Posted by GitBox <gi...@apache.org>.
dongjinleekr commented on pull request #10244:
URL: https://github.com/apache/kafka/pull/10244#issuecomment-788692068


   @omkreddy @huxihx Could you have a look? :smile:


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] mimaison commented on a change in pull request #10244: KAFKA-12399: Deprecate Log4J Appender

Posted by GitBox <gi...@apache.org>.
mimaison commented on a change in pull request #10244:
URL: https://github.com/apache/kafka/pull/10244#discussion_r812791855



##########
File path: log4j-appender/src/main/java/org/apache/kafka/log4jappender/KafkaLog4jAppender.java
##########
@@ -333,6 +333,7 @@ public void activateOptions() {
         props.put(KEY_SERIALIZER_CLASS_CONFIG, ByteArraySerializer.class.getName());
         props.put(VALUE_SERIALIZER_CLASS_CONFIG, ByteArraySerializer.class.getName());
         this.producer = getKafkaProducer(props);
+        LogLog.warn("log4j-appender is deprecated and will be removed in a future release. For migration, please refer to the latest documentation.");

Review comment:
       Should we also annotate the class with `@Deprecated`?

##########
File path: bin/kafka-run-class.sh
##########
@@ -45,6 +45,45 @@ should_include_file() {
   fi
 }
 
+# As of present, the migration to log4j2 is not completed, so some modules are using log4j 1.x, and

Review comment:
       We don't have similar logic in `kafka-run-class.bat`, is this on purpose?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] dongjinleekr removed a comment on pull request #10244: KAFKA-12399: Deprecate Log4J Appender

Posted by GitBox <gi...@apache.org>.
dongjinleekr removed a comment on pull request #10244:
URL: https://github.com/apache/kafka/pull/10244#issuecomment-1073969635


   @mimaison Here is the update, with a migration guide! After some trial and error, I decided that a full comparison table would help.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org