Support arguments where possible for Logback and Slf4j (#458)

* Support arguments where possible for Logback and Slf4j * Add test of json output and arguments for Logback wrapper * Use extra for version control of logstash-logback-encoder --------- Co-authored-by: Ohad Shai <[email protected]>
oshai · Nov 30, 2024 · 2bc99d7 · 2bc99d7
1 parent 1f9ecdd
commit 2bc99d7
Show file tree

Hide file tree

Showing 7 changed files with 70 additions and 10 deletions.
diff --git a/build.gradle.kts b/build.gradle.kts
@@ -134,6 +134,8 @@ kotlin {
                 implementation("org.apache.logging.log4j:log4j-slf4j2-impl:${extra["log4j_version"]}")
                 implementation("org.slf4j:slf4j-api:${extra["slf4j_version"]}")
                 implementation("ch.qos.logback:logback-classic:${extra["logback_version"]}")
+                implementation("net.logstash.logback:logstash-logback-encoder:${extra["logstash_logback_encoder_version"]}")
+
                 // our jul test just forward the logs jul -> slf4j -> log4j
                 implementation("org.slf4j:jul-to-slf4j:${extra["slf4j_version"]}")
                 implementation("org.jetbrains.kotlinx:kotlinx-coroutines-slf4j:${extra["coroutines_version"]}")

diff --git a/src/commonMain/kotlin/io/github/oshai/kotlinlogging/KLoggingEventBuilder.kt b/src/commonMain/kotlin/io/github/oshai/kotlinlogging/KLoggingEventBuilder.kt
@@ -4,6 +4,8 @@ public class KLoggingEventBuilder {
   public var message: String? = null
   public var cause: Throwable? = null
   public var payload: Map<String, Any?>? = null
+  /** Arguments passed as is to underlying impl. API stability is not guaranteed. */
+  public var arguments: Array<Any?>? = null
 
   /**
    * Internal data that is used by compiler plugin to provide additional information about the log

diff --git a/src/javaMain/kotlin/io/github/oshai/kotlinlogging/slf4j/internal/LocationAwareKLogger.kt b/src/javaMain/kotlin/io/github/oshai/kotlinlogging/slf4j/internal/LocationAwareKLogger.kt
@@ -58,6 +58,7 @@ internal class LocationAwareKLogger(override val underlyingLogger: LocationAware
     val builder = underlyingLogger.atLevel(level.toSlf4j())
     marker?.toSlf4j()?.let { builder.addMarker(it) }
     kLoggingEventBuilder.payload?.forEach { (key, value) -> builder.addKeyValue(key, value) }
+    kLoggingEventBuilder.arguments?.forEach { arg -> builder.addArgument(arg) }
     builder.setCause(kLoggingEventBuilder.cause)
     if (builder is CallerBoundaryAware) {
       builder.setCallerBoundary(fqcn)
@@ -75,7 +76,7 @@ internal class LocationAwareKLogger(override val underlyingLogger: LocationAware
       fqcn,
       level.toSlf4j().toInt(),
       kLoggingEventBuilder.message,
-      null,
+      kLoggingEventBuilder.arguments,
       kLoggingEventBuilder.cause,
     )
   }

diff --git a/src/javaMain/kotlin/io/github/oshai/kotlinlogging/slf4j/internal/LocationIgnorantKLogger.kt b/src/javaMain/kotlin/io/github/oshai/kotlinlogging/slf4j/internal/LocationIgnorantKLogger.kt
@@ -43,6 +43,7 @@ internal class LocationIgnorantKLogger(override val underlyingLogger: Logger) :
     val builder = underlyingLogger.atLevel(level.toSlf4j())
     marker?.toSlf4j()?.let { builder.addMarker(it) }
     kLoggingEventBuilder.payload?.forEach { (key, value) -> builder.addKeyValue(key, value) }
+    kLoggingEventBuilder.arguments?.forEach { arg -> builder.addArgument(arg) }
     builder.setCause(kLoggingEventBuilder.cause)
     builder.log(kLoggingEventBuilder.message)
   }
@@ -55,6 +56,7 @@ internal class LocationIgnorantKLogger(override val underlyingLogger: Logger) :
     val slf4jMarker = marker?.toSlf4j()
     val message = kLoggingEventBuilder.message
     val cause = kLoggingEventBuilder.cause
+
     when (level) {
       Level.TRACE -> underlyingLogger.trace(slf4jMarker, message, cause)
       Level.DEBUG -> underlyingLogger.debug(slf4jMarker, message, cause)

diff --git a/src/jvmMain/kotlin/io/github/oshai/kotlinlogging/logback/internal/LogbackLogEvent.kt b/src/jvmMain/kotlin/io/github/oshai/kotlinlogging/logback/internal/LogbackLogEvent.kt
@@ -18,7 +18,7 @@ public class LogbackLogEvent(
     level.toLogbackLevel(),
     kLoggingEvent.internalCompilerData?.messageTemplate ?: kLoggingEvent.message,
     kLoggingEvent.cause,
-    emptyArray(),
+    kLoggingEvent.arguments ?: emptyArray(),
   ) {
 
   override fun getFormattedMessage(): String? {

diff --git a/...jvmTest/kotlin/io/github/oshai/kotlinlogging/logback/internal/LogbackLoggerWrapperTest.kt b/...jvmTest/kotlin/io/github/oshai/kotlinlogging/logback/internal/LogbackLoggerWrapperTest.kt
@@ -8,9 +8,14 @@ import ch.qos.logback.core.OutputStreamAppender
 import io.github.oshai.kotlinlogging.KLogger
 import io.github.oshai.kotlinlogging.KotlinLogging
 import java.io.ByteArrayOutputStream
+import net.logstash.logback.argument.StructuredArguments
+import net.logstash.logback.composite.loggingevent.ArgumentsJsonProvider
+import net.logstash.logback.composite.loggingevent.LoggingEventPatternJsonProvider
+import net.logstash.logback.encoder.LoggingEventCompositeJsonEncoder
 import org.junit.jupiter.api.AfterAll
 import org.junit.jupiter.api.Assertions.*
 import org.junit.jupiter.api.BeforeAll
+import org.junit.jupiter.api.BeforeEach
 import org.junit.jupiter.api.Test
 
 class LogbackLoggerWrapperTest {
@@ -20,7 +25,9 @@ class LogbackLoggerWrapperTest {
     private lateinit var warnLogger: KLogger
     private lateinit var errorLogger: KLogger
     private lateinit var logOutputStream: ByteArrayOutputStream
+    private lateinit var jsonLogOutputStream: ByteArrayOutputStream
     private lateinit var appender: OutputStreamAppender<ILoggingEvent>
+    private lateinit var jsonAppender: OutputStreamAppender<ILoggingEvent>
     private lateinit var rootLogger: Logger
 
     @BeforeAll
@@ -36,15 +43,35 @@ class LogbackLoggerWrapperTest {
       encoder.charset = Charsets.UTF_8
       encoder.start()
 
+      val jsonEncoder = LoggingEventCompositeJsonEncoder()
+      jsonEncoder.context = loggerContext
+      val patternProvider = LoggingEventPatternJsonProvider()
+      patternProvider.context = loggerContext
+      patternProvider.pattern = """{"message": "%message"}"""
+      jsonEncoder.providers.addProvider(patternProvider)
+      val argumentsJsonProvider = ArgumentsJsonProvider()
+      argumentsJsonProvider.isIncludeStructuredArguments = true
+      argumentsJsonProvider.nonStructuredArgumentsFieldPrefix = ""
+      jsonEncoder.providers.addProvider(argumentsJsonProvider)
+      jsonEncoder.start()
+
       logOutputStream = ByteArrayOutputStream()
       appender = OutputStreamAppender<ILoggingEvent>()
       appender.context = loggerContext
       appender.encoder = encoder
       appender.outputStream = logOutputStream
       appender.start()
 
+      jsonLogOutputStream = ByteArrayOutputStream()
+      jsonAppender = OutputStreamAppender<ILoggingEvent>()
+      jsonAppender.context = loggerContext
+      jsonAppender.encoder = jsonEncoder
+      jsonAppender.outputStream = jsonLogOutputStream
+      jsonAppender.start()
+
       rootLogger = loggerContext.getLogger(Logger.ROOT_LOGGER_NAME)
       rootLogger.addAppender(appender)
+      rootLogger.addAppender(jsonAppender)
       rootLogger.level = Level.TRACE
 
       logger = KotlinLogging.logger {}
@@ -63,6 +90,12 @@ class LogbackLoggerWrapperTest {
     }
   }
 
+  @BeforeEach
+  fun resetTest() {
+    logOutputStream.reset()
+    jsonLogOutputStream.reset()
+  }
+
   @Test
   fun testLogbackLogger() {
     assertTrue(logger is LogbackLoggerWrapper)
@@ -71,19 +104,38 @@ class LogbackLoggerWrapperTest {
     logger.info { "simple logback info message" }
     warnLogger.warn { "simple logback warn message" }
     errorLogger.error { "simple logback error message" }
-    val lines =
-      logOutputStream
-        .toByteArray()
-        .toString(Charsets.UTF_8)
-        .trim()
-        .replace("\r", "\n")
-        .replace("\n\n", "\n")
-        .split("\n")
+    val lines = logOutputStream.toByteArray().toString(Charsets.UTF_8).trim().lines()
+    val jsonLines = jsonLogOutputStream.toByteArray().toString(Charsets.UTF_8).trim().lines()
     assertEquals(
       "INFO  io.github.oshai.kotlinlogging.logback.internal.LogbackLoggerWrapperTest  - simple logback info message",
       lines[0],
     )
+    assertEquals("""{"message":"simple logback info message"}""", jsonLines[0])
     assertEquals("WARN  warnLogger  - simple logback warn message", lines[1])
+    assertEquals("""{"message":"simple logback warn message"}""", jsonLines[1])
     assertEquals("ERROR errorLogger  - simple logback error message", lines[2])
+    assertEquals("""{"message":"simple logback error message"}""", jsonLines[2])
+  }
+
+  @Test
+  fun testLogbackLoggerWithArguments() {
+    logger.atInfo {
+      message = "msg"
+      arguments =
+        arrayOf(
+          StructuredArguments.keyValue("arg1", "val1"),
+          StructuredArguments.keyValue("arg2", "val2"),
+        )
+    }
+    val lines = logOutputStream.toByteArray().toString(Charsets.UTF_8).trim().lines()
+    val jsonLines = jsonLogOutputStream.toByteArray().toString(Charsets.UTF_8).trim().lines()
+
+    assertEquals(1, lines.size)
+    assertEquals(1, jsonLines.size)
+    assertEquals(
+      "INFO  io.github.oshai.kotlinlogging.logback.internal.LogbackLoggerWrapperTest  - msg",
+      lines[0],
+    )
+    assertEquals("""{"message":"msg","arg1":"val1","arg2":"val2"}""", jsonLines[0])
   }
 }
diff --git a/versions.gradle.kts b/versions.gradle.kts
@@ -4,3 +4,4 @@ extra["log4j_version"] = "2.22.0"
 extra["mockito_version"] = "4.11.0"
 extra["junit_version"] = "5.9.2"
 extra["logback_version"] = "1.5.11"
+extra["logstash_logback_encoder_version"] = "8.0"