(spark) branch master updated: [SPARK-48214][INFRA] Ban import `org.slf4j.Logger` & `org.slf4j.LoggerFactory`

[ge...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

gengliang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/spark.git


The following commit(s) were added to refs/heads/master by this push:
     new dec910ba3c36 [SPARK-48214][INFRA] Ban import `org.slf4j.Logger` & `org.slf4j.LoggerFactory`
dec910ba3c36 is described below

commit dec910ba3c36e27b9cff5b5e139be82af6c799ab
Author: panbingkun <pa...@baidu.com>
AuthorDate: Wed May 15 21:50:48 2024 -0700

    [SPARK-48214][INFRA] Ban import `org.slf4j.Logger` & `org.slf4j.LoggerFactory`
    
    ### What changes were proposed in this pull request?
    The pr aims to ban import `org.slf4j.Logger` & `org.slf4j.LoggerFactory`.
    
    ### Why are the changes needed?
    After the migration of structured logs on the `java side` is completed, we need to ban import `org.slf4j.Logger` & `org.slf4j.LoggerFactory` in the code to avoid the log format that is not written as required in the future new java code.
    
    ### Does this PR introduce _any_ user-facing change?
    Yes, only for spark developers.
    
    ### How was this patch tested?
    - Manually test.
    - Pass GA.
    
    ### Was this patch authored or co-authored using generative AI tooling?
    No.
    
    Closes #46502 from panbingkun/ban_import_slf4j.
    
    Authored-by: panbingkun <pa...@baidu.com>
    Signed-off-by: Gengliang Wang <ge...@apache.org>
---
 common/kvstore/pom.xml                                         |  6 ++++++
 .../java/org/apache/spark/util/kvstore/LevelDBIterator.java    |  7 ++++---
 .../java/org/apache/spark/util/kvstore/DBIteratorSuite.java    |  2 ++
 .../java/org/apache/spark/util/kvstore/LevelDBBenchmark.java   |  2 ++
 .../java/org/apache/spark/util/kvstore/RocksDBBenchmark.java   |  2 ++
 .../apache/spark/network/util/TransportFrameDecoderSuite.java  |  2 ++
 .../spark/network/shuffle/RemoteBlockPushResolverSuite.java    |  2 ++
 .../apache/spark/network/shuffle/TestShuffleDataContext.java   |  2 ++
 .../src/main/java/org/apache/spark/internal/SparkLogger.java   |  9 ++++++---
 .../java/org/apache/spark/internal/SparkLoggerFactory.java     | 10 ++++++----
 dev/checkstyle.xml                                             |  5 +++++
 11 files changed, 39 insertions(+), 10 deletions(-)

diff --git a/common/kvstore/pom.xml b/common/kvstore/pom.xml
index 3820d1b8e395..046648e9c2ae 100644
--- a/common/kvstore/pom.xml
+++ b/common/kvstore/pom.xml
@@ -40,6 +40,12 @@
       <artifactId>spark-tags_${scala.binary.version}</artifactId>
     </dependency>
 
+    <dependency>
+      <groupId>org.apache.spark</groupId>
+      <artifactId>spark-common-utils_${scala.binary.version}</artifactId>
+      <version>${project.version}</version>
+    </dependency>
+
     <dependency>
       <groupId>com.google.guava</groupId>
       <artifactId>guava</artifactId>
diff --git a/common/kvstore/src/main/java/org/apache/spark/util/kvstore/LevelDBIterator.java b/common/kvstore/src/main/java/org/apache/spark/util/kvstore/LevelDBIterator.java
index b830e6afc617..69757fdc65d6 100644
--- a/common/kvstore/src/main/java/org/apache/spark/util/kvstore/LevelDBIterator.java
+++ b/common/kvstore/src/main/java/org/apache/spark/util/kvstore/LevelDBIterator.java
@@ -29,8 +29,9 @@ import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
 import com.google.common.base.Throwables;
 import org.iq80.leveldb.DBIterator;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
+
+import org.apache.spark.internal.SparkLogger;
+import org.apache.spark.internal.SparkLoggerFactory;
 
 class LevelDBIterator<T> implements KVStoreIterator<T> {
 
@@ -302,7 +303,7 @@ class LevelDBIterator<T> implements KVStoreIterator<T> {
   }
 
   static class ResourceCleaner implements Runnable {
-    private static final Logger LOG = LoggerFactory.getLogger(ResourceCleaner.class);
+    private static final SparkLogger LOG = SparkLoggerFactory.getLogger(ResourceCleaner.class);
 
     private final DBIterator dbIterator;
 
diff --git a/common/kvstore/src/test/java/org/apache/spark/util/kvstore/DBIteratorSuite.java b/common/kvstore/src/test/java/org/apache/spark/util/kvstore/DBIteratorSuite.java
index daedd56890a6..72c3690d1a18 100644
--- a/common/kvstore/src/test/java/org/apache/spark/util/kvstore/DBIteratorSuite.java
+++ b/common/kvstore/src/test/java/org/apache/spark/util/kvstore/DBIteratorSuite.java
@@ -32,8 +32,10 @@ import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.Test;
+// checkstyle.off: RegexpSinglelineJava
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+// checkstyle.on: RegexpSinglelineJava
 import static org.junit.jupiter.api.Assertions.*;
 
 public abstract class DBIteratorSuite {
diff --git a/common/kvstore/src/test/java/org/apache/spark/util/kvstore/LevelDBBenchmark.java b/common/kvstore/src/test/java/org/apache/spark/util/kvstore/LevelDBBenchmark.java
index 3158c18f9e1d..ff6db8fc34c9 100644
--- a/common/kvstore/src/test/java/org/apache/spark/util/kvstore/LevelDBBenchmark.java
+++ b/common/kvstore/src/test/java/org/apache/spark/util/kvstore/LevelDBBenchmark.java
@@ -34,7 +34,9 @@ import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
+// checkstyle.off: RegexpSinglelineJava
 import org.slf4j.LoggerFactory;
+// checkstyle.on: RegexpSinglelineJava
 import static org.junit.jupiter.api.Assertions.*;
 
 /**
diff --git a/common/kvstore/src/test/java/org/apache/spark/util/kvstore/RocksDBBenchmark.java b/common/kvstore/src/test/java/org/apache/spark/util/kvstore/RocksDBBenchmark.java
index e18a3c3b1c28..c1b8009e97e6 100644
--- a/common/kvstore/src/test/java/org/apache/spark/util/kvstore/RocksDBBenchmark.java
+++ b/common/kvstore/src/test/java/org/apache/spark/util/kvstore/RocksDBBenchmark.java
@@ -34,7 +34,9 @@ import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
+// checkstyle.off: RegexpSinglelineJava
 import org.slf4j.LoggerFactory;
+// checkstyle.on: RegexpSinglelineJava
 import static org.junit.jupiter.api.Assertions.*;
 
 /**
diff --git a/common/network-common/src/test/java/org/apache/spark/network/util/TransportFrameDecoderSuite.java b/common/network-common/src/test/java/org/apache/spark/network/util/TransportFrameDecoderSuite.java
index aa3891450a93..025be80c5ce4 100644
--- a/common/network-common/src/test/java/org/apache/spark/network/util/TransportFrameDecoderSuite.java
+++ b/common/network-common/src/test/java/org/apache/spark/network/util/TransportFrameDecoderSuite.java
@@ -27,8 +27,10 @@ import io.netty.buffer.Unpooled;
 import io.netty.channel.ChannelHandlerContext;
 import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.Test;
+// checkstyle.off: RegexpSinglelineJava
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+// checkstyle.on: RegexpSinglelineJava
 
 import static org.junit.jupiter.api.Assertions.*;
 import static org.mockito.Mockito.*;
diff --git a/common/network-shuffle/src/test/java/org/apache/spark/network/shuffle/RemoteBlockPushResolverSuite.java b/common/network-shuffle/src/test/java/org/apache/spark/network/shuffle/RemoteBlockPushResolverSuite.java
index fbde165fb39c..edd5e1961a50 100644
--- a/common/network-shuffle/src/test/java/org/apache/spark/network/shuffle/RemoteBlockPushResolverSuite.java
+++ b/common/network-shuffle/src/test/java/org/apache/spark/network/shuffle/RemoteBlockPushResolverSuite.java
@@ -47,8 +47,10 @@ import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.roaringbitmap.RoaringBitmap;
+// checkstyle.off: RegexpSinglelineJava
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+// checkstyle.on: RegexpSinglelineJava
 
 import static org.junit.jupiter.api.Assertions.*;
 
diff --git a/common/network-shuffle/src/test/java/org/apache/spark/network/shuffle/TestShuffleDataContext.java b/common/network-shuffle/src/test/java/org/apache/spark/network/shuffle/TestShuffleDataContext.java
index 406822466559..eeb936773aaa 100644
--- a/common/network-shuffle/src/test/java/org/apache/spark/network/shuffle/TestShuffleDataContext.java
+++ b/common/network-shuffle/src/test/java/org/apache/spark/network/shuffle/TestShuffleDataContext.java
@@ -28,8 +28,10 @@ import com.google.common.io.Closeables;
 import org.apache.spark.network.shuffle.protocol.ExecutorShuffleInfo;
 import org.apache.spark.network.util.JavaUtils;
 import org.junit.jupiter.api.Assertions;
+// checkstyle.off: RegexpSinglelineJava
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+// checkstyle.on: RegexpSinglelineJava
 
 /**
  * Manages some sort-shuffle data, including the creation
diff --git a/common/utils/src/main/java/org/apache/spark/internal/SparkLogger.java b/common/utils/src/main/java/org/apache/spark/internal/SparkLogger.java
index 20ad68641da0..bf8adb70637e 100644
--- a/common/utils/src/main/java/org/apache/spark/internal/SparkLogger.java
+++ b/common/utils/src/main/java/org/apache/spark/internal/SparkLogger.java
@@ -24,13 +24,16 @@ import java.util.function.Consumer;
 import org.apache.logging.log4j.CloseableThreadContext;
 import org.apache.logging.log4j.message.MessageFactory;
 import org.apache.logging.log4j.message.ParameterizedMessageFactory;
+// checkstyle.off: RegexpSinglelineJava
+import org.slf4j.Logger;
+// checkstyle.on: RegexpSinglelineJava
 
 public class SparkLogger {
 
   private static final MessageFactory MESSAGE_FACTORY = ParameterizedMessageFactory.INSTANCE;
-  private final org.slf4j.Logger slf4jLogger;
+  private final Logger slf4jLogger;
 
-  SparkLogger(org.slf4j.Logger slf4jLogger) {
+  SparkLogger(Logger slf4jLogger) {
     this.slf4jLogger = slf4jLogger;
   }
 
@@ -194,7 +197,7 @@ public class SparkLogger {
     }
   }
 
-  public org.slf4j.Logger getSlf4jLogger() {
+  public Logger getSlf4jLogger() {
     return slf4jLogger;
   }
 }
diff --git a/common/utils/src/main/java/org/apache/spark/internal/SparkLoggerFactory.java b/common/utils/src/main/java/org/apache/spark/internal/SparkLoggerFactory.java
index 699f43d77283..a59c00736241 100644
--- a/common/utils/src/main/java/org/apache/spark/internal/SparkLoggerFactory.java
+++ b/common/utils/src/main/java/org/apache/spark/internal/SparkLoggerFactory.java
@@ -17,15 +17,17 @@
 
 package org.apache.spark.internal;
 
+// checkstyle.off: RegexpSinglelineJava
+import org.slf4j.LoggerFactory;
+// checkstyle.on: RegexpSinglelineJava
+
 public class SparkLoggerFactory {
 
   public static SparkLogger getLogger(String name) {
-    org.slf4j.Logger slf4jLogger = org.slf4j.LoggerFactory.getLogger(name);
-    return new SparkLogger(slf4jLogger);
+    return new SparkLogger(LoggerFactory.getLogger(name));
   }
 
   public static SparkLogger getLogger(Class<?> clazz) {
-    org.slf4j.Logger slf4jLogger = org.slf4j.LoggerFactory.getLogger(clazz);
-    return new SparkLogger(slf4jLogger);
+    return new SparkLogger(LoggerFactory.getLogger(clazz));
   }
 }
diff --git a/dev/checkstyle.xml b/dev/checkstyle.xml
index 68a5cd91ef3c..c4023a84ee3c 100644
--- a/dev/checkstyle.xml
+++ b/dev/checkstyle.xml
@@ -206,5 +206,10 @@
             <property name="illegalPkgs" value="org.apache.log4j" />
             <property name="illegalPkgs" value="org.apache.commons.lang" />
         </module>
+        <!-- support structured logging -->
+        <module name="RegexpSinglelineJava">
+            <property name="format" value="org\.slf4j\.(Logger|LoggerFactory)" />
+            <property name="message" value="Please use org.apache.spark.internal.(SparkLogger|SparkLoggerFactory) instead." />
+        </module>
     </module>
 </module>


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@spark.apache.org
For additional commands, e-mail: commits-help@spark.apache.org