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 <[email protected]>
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 <[email protected]>
Signed-off-by: Gengliang Wang <[email protected]>
---
.../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: [email protected]
For additional commands, e-mail: [email protected]
