(spark) branch master updated: [SPARK-48059][CORE][FOLLOWUP] Fix bug for SparkLogger

[gengliang]

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 bb21861a9580 [SPARK-48059][CORE][FOLLOWUP] Fix bug for SparkLogger
bb21861a9580 is described below

commit bb21861a9580f5a5e1cdf90c723fa8f814d4916c
Author: panbingkun <panbing...@baidu.com>
AuthorDate: Wed Jun 26 00:14:21 2024 -0700

    [SPARK-48059][CORE][FOLLOWUP] Fix bug for SparkLogger
    
    ### What changes were proposed in this pull request?
    The pr is following up https://github.com/apache/spark/pull/46301, to fix 
bug for `SparkLogger`.
    
    ### Why are the changes needed?
    Fix bug for `SparkLogger`.
    
    ### Does this PR introduce _any_ user-facing change?
    No.
    
    ### How was this patch tested?
    Add new UT.
    Manually test.
    
    ### Was this patch authored or co-authored using generative AI tooling?
    No.
    
    Closes #47095 from panbingkun/SPARK-48059_FOLLOWUP.
    
    Authored-by: panbingkun <panbing...@baidu.com>
    Signed-off-by: Gengliang Wang <gengli...@apache.org>
---
 .../java/org/apache/spark/internal/SparkLogger.java |  4 ++--
 .../org/apache/spark/util/SparkLoggerSuiteBase.java | 21 +++++++++++++++++++++
 2 files changed, 23 insertions(+), 2 deletions(-)

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 32dd8f1f26b5..8c210a4fab3c 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
@@ -130,7 +130,7 @@ public class SparkLogger {
 
   public void warn(String msg, Throwable throwable, MDC... mdcs) {
     if (mdcs == null || mdcs.length == 0) {
-      slf4jLogger.warn(msg);
+      slf4jLogger.warn(msg, throwable);
     } else if (slf4jLogger.isWarnEnabled()) {
       withLogContext(msg, mdcs, throwable, mt -> slf4jLogger.warn(mt.message, 
mt.throwable));
     }
@@ -158,7 +158,7 @@ public class SparkLogger {
 
   public void info(String msg, Throwable throwable, MDC... mdcs) {
     if (mdcs == null || mdcs.length == 0) {
-      slf4jLogger.info(msg);
+      slf4jLogger.info(msg, throwable);
     } else if (slf4jLogger.isInfoEnabled()) {
       withLogContext(msg, mdcs, throwable, mt -> slf4jLogger.info(mt.message, 
mt.throwable));
     }
diff --git 
a/common/utils/src/test/java/org/apache/spark/util/SparkLoggerSuiteBase.java 
b/common/utils/src/test/java/org/apache/spark/util/SparkLoggerSuiteBase.java
index 37b5372885b8..186088ede1d0 100644
--- a/common/utils/src/test/java/org/apache/spark/util/SparkLoggerSuiteBase.java
+++ b/common/utils/src/test/java/org/apache/spark/util/SparkLoggerSuiteBase.java
@@ -81,6 +81,8 @@ public abstract class SparkLoggerSuiteBase {
     MDC.of(LogKeys.REASON$.MODULE$, "the shuffle data is too large")};
   private final String msgWithMDCs = "Lost executor {}, reason: {}";
 
+  private final MDC[] emptyMDCs = new MDC[0];
+
   private final MDC executorIDMDCValueIsNull = 
MDC.of(LogKeys.EXECUTOR_ID$.MODULE$, null);
 
   private final MDC scalaCustomLogMDC =
@@ -107,6 +109,11 @@ public abstract class SparkLoggerSuiteBase {
   // test for message (with mdcs and exception)
   abstract String expectedPatternForMsgWithMDCsAndException(Level level);
 
+  // test for message (with empty mdcs and exception)
+  String expectedPatternForMsgWithEmptyMDCsAndException(Level level) {
+    return expectedPatternForBasicMsgWithException(level);
+  }
+
   // test for message (with mdc - the value is null)
   abstract String expectedPatternForMsgWithMDCValueIsNull(Level level);
 
@@ -191,6 +198,20 @@ public abstract class SparkLoggerSuiteBase {
       checkLogOutput(pair.getLeft(), pair.getRight(), 
this::expectedPatternForMsgWithMDCs));
   }
 
+  @Test
+  public void testLoggerWithEmptyMDCsAndException() {
+    Throwable exception = new RuntimeException("OOM");
+    Runnable errorFn = () -> logger().error(basicMsg, exception, emptyMDCs);
+    Runnable warnFn = () -> logger().warn(basicMsg, exception, emptyMDCs);
+    Runnable infoFn = () -> logger().info(basicMsg, exception, emptyMDCs);
+    List.of(
+        Pair.of(Level.ERROR, errorFn),
+        Pair.of(Level.WARN, warnFn),
+        Pair.of(Level.INFO, infoFn)).forEach(pair ->
+        checkLogOutput(pair.getLeft(), pair.getRight(),
+            this::expectedPatternForMsgWithEmptyMDCsAndException));
+  }
+
   @Test
   public void testLoggerWithMDCsAndException() {
     Throwable exception = new RuntimeException("OOM");


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