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 d182810abcd8 [SPARK-47575][INFRA] Implement logWarning API in structured logging framework d182810abcd8 is described below commit d182810abcd8ff6a86211b90f0b4217100546688 Author: Gengliang Wang <gengli...@apache.org> AuthorDate: Fri Mar 29 11:13:21 2024 -0700 [SPARK-47575][INFRA] Implement logWarning API in structured logging framework ### What changes were proposed in this pull request? Implement logWarning API in structured logging framework. Also, refactor the logging test suites to reduce duplicated code. ### Why are the changes needed? To enhance Apache Spark's logging system by implementing structured logging. ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? New unit tests ### Was this patch authored or co-authored using generative AI tooling? No Closes #45770 from gengliangwang/logWarning. Authored-by: Gengliang Wang <gengli...@apache.org> Signed-off-by: Gengliang Wang <gengli...@apache.org> --- .../scala/org/apache/spark/internal/Logging.scala | 14 ++++ .../apache/spark/util/PatternLoggingSuite.scala | 33 ++-------- .../apache/spark/util/StructuredLoggingSuite.scala | 74 +++++++++++++++------- 3 files changed, 72 insertions(+), 49 deletions(-) diff --git a/common/utils/src/main/scala/org/apache/spark/internal/Logging.scala b/common/utils/src/main/scala/org/apache/spark/internal/Logging.scala index 7f380a9c7887..2fed115f3dbb 100644 --- a/common/utils/src/main/scala/org/apache/spark/internal/Logging.scala +++ b/common/utils/src/main/scala/org/apache/spark/internal/Logging.scala @@ -134,6 +134,20 @@ trait Logging { if (log.isWarnEnabled) log.warn(msg) } + protected def logWarning(entry: LogEntry): Unit = { + if (log.isWarnEnabled) { + log.warn(entry.message) + entry.context.map(_.close()) + } + } + + protected def logWarning(entry: LogEntry, throwable: Throwable): Unit = { + if (log.isWarnEnabled) { + log.warn(entry.message, throwable) + entry.context.map(_.close()) + } + } + protected def logError(msg: => String): Unit = { if (log.isErrorEnabled) log.error(msg) } diff --git a/common/utils/src/test/scala/org/apache/spark/util/PatternLoggingSuite.scala b/common/utils/src/test/scala/org/apache/spark/util/PatternLoggingSuite.scala index 0c6ed89172e0..ef0aa7050b07 100644 --- a/common/utils/src/test/scala/org/apache/spark/util/PatternLoggingSuite.scala +++ b/common/utils/src/test/scala/org/apache/spark/util/PatternLoggingSuite.scala @@ -18,8 +18,7 @@ package org.apache.spark.util import org.scalatest.BeforeAndAfterAll -import org.apache.spark.internal.{Logging, MDC} -import org.apache.spark.internal.LogKey.EXECUTOR_ID +import org.apache.spark.internal.Logging class PatternLoggingSuite extends LoggingSuiteBase with BeforeAndAfterAll { @@ -29,30 +28,12 @@ class PatternLoggingSuite extends LoggingSuiteBase with BeforeAndAfterAll { override def afterAll(): Unit = Logging.enableStructuredLogging() - test("Pattern layout logging") { - val msg = "This is a log message" + override def expectedPatternForBasicMsg(level: String): String = + s""".*$level PatternLoggingSuite: This is a log message\n""" - val logOutput = captureLogOutput(() => logError(msg)) - // scalastyle:off line.size.limit - val pattern = """.*ERROR PatternLoggingSuite: This is a log message\n""".r - // scalastyle:on - assert(pattern.matches(logOutput)) - } + override def expectedPatternForMsgWithMDC(level: String): String = + s""".*$level PatternLoggingSuite: Lost executor 1.\n""" - test("Pattern layout logging with MDC") { - logError(log"Lost executor ${MDC(EXECUTOR_ID, "1")}.") - - val logOutput = captureLogOutput(() => logError(log"Lost executor ${MDC(EXECUTOR_ID, "1")}.")) - val pattern = """.*ERROR PatternLoggingSuite: Lost executor 1.\n""".r - assert(pattern.matches(logOutput)) - } - - test("Pattern layout exception logging") { - val exception = new RuntimeException("OOM") - - val logOutput = captureLogOutput(() => - logError(log"Error in executor ${MDC(EXECUTOR_ID, "1")}.", exception)) - assert(logOutput.contains("ERROR PatternLoggingSuite: Error in executor 1.")) - assert(logOutput.contains("java.lang.RuntimeException: OOM")) - } + override def expectedPatternForMsgWithMDCAndException(level: String): String = + s""".*$level PatternLoggingSuite: Error in executor 1.\njava.lang.RuntimeException: OOM\n.*""" } diff --git a/common/utils/src/test/scala/org/apache/spark/util/StructuredLoggingSuite.scala b/common/utils/src/test/scala/org/apache/spark/util/StructuredLoggingSuite.scala index eef9866a68b1..b032649170bc 100644 --- a/common/utils/src/test/scala/org/apache/spark/util/StructuredLoggingSuite.scala +++ b/common/utils/src/test/scala/org/apache/spark/util/StructuredLoggingSuite.scala @@ -21,7 +21,7 @@ import java.nio.file.Files import org.scalatest.funsuite.AnyFunSuite // scalastyle:ignore funsuite -import org.apache.spark.internal.{Logging, MDC} +import org.apache.spark.internal.{LogEntry, Logging, MDC} import org.apache.spark.internal.LogKey.EXECUTOR_ID abstract class LoggingSuiteBase extends AnyFunSuite // scalastyle:ignore funsuite @@ -45,39 +45,67 @@ abstract class LoggingSuiteBase extends AnyFunSuite // scalastyle:ignore funsuit val newContent = Files.readString(logFile.toPath) newContent.substring(content.length) } -} -class StructuredLoggingSuite extends LoggingSuiteBase { - private val className = this.getClass.getName.stripSuffix("$") - override def logFilePath: String = "target/structured.log" + def basicMsg: String = "This is a log message" + + def msgWithMDC: LogEntry = log"Lost executor ${MDC(EXECUTOR_ID, "1")}." + + def msgWithMDCAndException: LogEntry = log"Error in executor ${MDC(EXECUTOR_ID, "1")}." + + def expectedPatternForBasicMsg(level: String): String + + def expectedPatternForMsgWithMDC(level: String): String + + def expectedPatternForMsgWithMDCAndException(level: String): String test("Structured logging") { val msg = "This is a log message" - val logOutput = captureLogOutput(() => logError(msg)) - - // scalastyle:off line.size.limit - val pattern = s"""\\{"ts":"[^"]+","level":"ERROR","msg":"This is a log message","logger":"$className"}\n""".r - // scalastyle:on - assert(pattern.matches(logOutput)) + Seq( + ("ERROR", () => logError(msg)), + ("WARN", () => logWarning(msg))).foreach { case (level, logFunc) => + val logOutput = captureLogOutput(logFunc) + assert(expectedPatternForBasicMsg(level).r.matches(logOutput)) + } } test("Structured logging with MDC") { - val logOutput = captureLogOutput(() => logError(log"Lost executor ${MDC(EXECUTOR_ID, "1")}.")) - assert(logOutput.nonEmpty) - // scalastyle:off line.size.limit - val pattern1 = s"""\\{"ts":"[^"]+","level":"ERROR","msg":"Lost executor 1.","context":\\{"executor_id":"1"},"logger":"$className"}\n""".r - // scalastyle:on - assert(pattern1.matches(logOutput)) + Seq( + ("ERROR", () => logError(log"Lost executor ${MDC(EXECUTOR_ID, "1")}.")), + ("WARN", () => logWarning(log"Lost executor ${MDC(EXECUTOR_ID, "1")}."))) + .foreach { + case (level, logFunc) => + val logOutput = captureLogOutput(logFunc) + assert(expectedPatternForMsgWithMDC(level).r.matches(logOutput)) + } } test("Structured exception logging with MDC") { val exception = new RuntimeException("OOM") - val logOutput = captureLogOutput(() => - logError(log"Error in executor ${MDC(EXECUTOR_ID, "1")}.", exception)) - assert(logOutput.nonEmpty) + Seq( + ("ERROR", () => logError(log"Error in executor ${MDC(EXECUTOR_ID, "1")}.", exception)), + ("WARN", () => logWarning(log"Error in executor ${MDC(EXECUTOR_ID, "1")}.", exception))) + .foreach { + case (level, logFunc) => + val logOutput = captureLogOutput(logFunc) + assert(expectedPatternForMsgWithMDCAndException(level).r.findFirstIn(logOutput).isDefined) + } + } +} + +class StructuredLoggingSuite extends LoggingSuiteBase { + private val className = this.getClass.getName.stripSuffix("$") + override def logFilePath: String = "target/structured.log" + + override def expectedPatternForBasicMsg(level: String): String = + s"""\\{"ts":"[^"]+","level":"$level","msg":"This is a log message","logger":"$className"}\n""" + + override def expectedPatternForMsgWithMDC(level: String): String = // scalastyle:off line.size.limit - val pattern = s"""\\{"ts":"[^"]+","level":"ERROR","msg":"Error in executor 1.","context":\\{"executor_id":"1"},"exception":\\{"class":"java.lang.RuntimeException","msg":"OOM","stacktrace":.*},"logger":"$className"}\n""".r + s"""\\{"ts":"[^"]+","level":"$level","msg":"Lost executor 1.","context":\\{"executor_id":"1"},"logger":"$className"}\n""" + // scalastyle:on + + override def expectedPatternForMsgWithMDCAndException(level: String): String = + // scalastyle:off line.size.limit + s"""\\{"ts":"[^"]+","level":"$level","msg":"Error in executor 1.","context":\\{"executor_id":"1"},"exception":\\{"class":"java.lang.RuntimeException","msg":"OOM","stacktrace":.*},"logger":"$className"}\n""" // scalastyle:on - assert(pattern.matches(logOutput)) - } } --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@spark.apache.org For additional commands, e-mail: commits-h...@spark.apache.org