KAFKA-16702 Fix producer leaks in KafkaLog4jAppenderTest (#15922)

The tests testRealProducerConfigWithSyncSendShouldNotThrowException and testRealProducerConfigWithSyncSendAndNotIgnoringExceptionsShouldThrowException create real producer instances, which are leaked when the test exits. Instead, each test should be followed by a cleanup operation where the registered appender is removed and closed. Reviewers: Chia-Ping Tsai <chia7712@gmail.com>
apache · May 14, 2024 · 8ac32d6 · 8ac32d6
1 parent d59336a
commit 8ac32d6
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 3 deletions.
diff --git a/log4j-appender/src/main/java/org/apache/kafka/log4jappender/KafkaLog4jAppender.java b/log4j-appender/src/main/java/org/apache/kafka/log4jappender/KafkaLog4jAppender.java
@@ -349,8 +349,14 @@ protected Producer<byte[], byte[]> getKafkaProducer(Properties props) {
     protected void append(LoggingEvent event) {
         String message = subAppend(event);
         LogLog.debug("[" + new Date(event.getTimeStamp()) + "]" + message);
-        Future<RecordMetadata> response = producer.send(
-            new ProducerRecord<>(topic, message.getBytes(StandardCharsets.UTF_8)));
+        Future<RecordMetadata> response;
+        try {
+            response = producer.send(new ProducerRecord<>(topic, message.getBytes(StandardCharsets.UTF_8)));
+        } catch (IllegalStateException e) {
+            // The producer has been closed
+            LogLog.debug("Exception while sending to Kafka", e);
+            return;
+        }
         if (syncSend) {
             try {
                 response.get();
@@ -370,7 +376,9 @@ private String subAppend(LoggingEvent event) {
     public void close() {
         if (!this.closed) {
             this.closed = true;
-            producer.close();
+            if (producer != null) {
+                producer.close();
+            }
         }
     }
 

diff --git a/log4j-appender/src/test/java/org/apache/kafka/log4jappender/KafkaLog4jAppenderTest.java b/log4j-appender/src/test/java/org/apache/kafka/log4jappender/KafkaLog4jAppenderTest.java
@@ -24,9 +24,11 @@
 import org.apache.kafka.common.TopicPartition;
 import org.apache.kafka.common.config.ConfigException;
 import org.apache.kafka.common.config.SaslConfigs;
+import org.apache.log4j.Appender;
 import org.apache.log4j.Logger;
 import org.apache.log4j.PropertyConfigurator;
 import org.apache.log4j.helpers.LogLog;
+import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
@@ -50,6 +52,17 @@ public void setup() {
         LogLog.setInternalDebugging(true);
     }
 
+    @AfterEach
+    public void cleanup() {
+        Logger rootLogger = Logger.getRootLogger();
+        Appender appender = rootLogger.getAppender("KAFKA");
+        if (appender != null) {
+            // Tests which do not call PropertyConfigurator.configure don't create an appender to remove.
+            rootLogger.removeAppender(appender);
+            appender.close();
+        }
+    }
+
     @Test
     public void testKafkaLog4jConfigs() {
         Properties hostMissingProps = new Properties();