KLogger API Cleanup & Ambiguity Fix #586

oshai · 2026-01-06T14:08:01Z

This PR cleans up the KLogger interface and resolves long-standing ambiguity issues involving Markers and lambdas (Issue #488).

Key Changes:

Undeprecated Marker Overloads:
Restored info(marker: Marker?, msg: () -> Any?), info(marker: Marker?, t: Throwable?, msg: () -> Any?), and their equivalents for all log levels. These are now the primary, idiomatic way to log with Markers.
Resolved Ambiguity:
Removed the = null default value from the throwable parameter in signatures like info(throwable: Throwable?, marker: Marker?, ...) to prevent compiler ambiguity and positional argument mishaps.
Removed Deprecated Methods (Breaking Change):
Deleted all deprecated methods from KLogger, including:
- Legacy SLF4J-style string overloads (e.g., info(msg: String?)).
- Deprecated property accessors (e.g., isTraceEnabled).
  The API is now strictly lambda-oriented for lazy evaluation.
Documentation:
Added missing KDoc comments to all logic methods.
Impl Cleanup:
Removed obsolete overrides from Slf4jLogger and LogbackLoggerWrapper to match the cleaner KLogger interface.

Type of methods left:

// 1. Standard message (Most common)
fun info(message: () -> Any?)

// 2. Exception + message (Very common)
fun info(throwable: Throwable?, message: () -> Any?)

// 3. Marker + message (Common for audit/security)
fun info(marker: Marker?, message: () -> Any?)

// 4. Full power (Marker + Exception + message)
fun info(marker: Marker?, throwable: Throwable?, message: () -> Any?)

Copilot

Pull request overview

This PR performs a major cleanup of the KLogger interface to resolve ambiguity issues and streamline the API towards a lambda-first approach. The changes remove all deprecated SLF4J-style methods and restore marker-based overloads as primary API methods.

Key changes:

Removed default value = null from throwable parameters in marker-accepting methods to eliminate compiler ambiguity
Undeprecated marker overloads (trace(marker: Marker?, msg: () -> Any?) and trace(marker: Marker?, t: Throwable?, msg: () -> Any?)) making them the primary API
Removed all deprecated methods including SLF4J-style string overloads and property accessors
Added KDoc comments to previously undocumented marker overload methods

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 9 comments.

File	Description
src/commonMain/kotlin/io/github/oshai/kotlinlogging/KLogger.kt	Removed default parameter values from throwable+marker methods (lines 78, 85, 92, 99, 106), undeprecated and documented marker overloads (lines 238-266), removed ~600 lines of deprecated methods
src/javaMain/kotlin/io/github/oshai/kotlinlogging/slf4j/internal/Slf4jLogger.kt	Removed deprecated SLF4J-style method implementations that were previously marked for removal
src/jvmMain/kotlin/io/github/oshai/kotlinlogging/logback/internal/LogbackLoggerWrapper.kt	Removed obsolete SLF4J-style overrides that delegated to underlying Logback logger
src/jvmLogbackTest/kotlin/io/github/oshai/kotlinlogging/logback/internal/LogbackLoggerWrapperTest.kt	Replaced deprecated SLF4J-style tests with lambda-based marker tests, removed ~170 lines of tests for removed API methods

Comments suppressed due to low confidence (1)

src/commonMain/kotlin/io/github/oshai/kotlinlogging/KLogger.kt:110

The documentation comment doesn't mention the marker parameter. Consider updating the documentation to indicate that this method also accepts a marker, for example: "Lazy add a log message with throwable and marker if isErrorEnabled is true".

  /** Lazy add a log message if isErrorEnabled is true */
  public fun error(throwable: Throwable?, marker: Marker?, message: () -> Any?): Unit =
    at(Level.ERROR, marker) {
      this.message = message.toStringSafe()
      this.cause = throwable
    }

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-06T19:06:43Z

src/commonMain/kotlin/io/github/oshai/kotlinlogging/KLogger.kt

+  /** Lazy add a log message with a marker if isDebugEnabled is true */
  public fun debug(marker: Marker?, t: Throwable?, msg: () -> Any?): Unit = debug(t, marker, msg)


The documentation comment doesn't mention the throwable parameter 't'. Consider updating the documentation to indicate that this method also accepts a throwable, for example: "Lazy add a log message with a marker and throwable if isDebugEnabled is true".

Copilot · 2026-01-06T19:06:43Z

src/commonMain/kotlin/io/github/oshai/kotlinlogging/KLogger.kt

  public fun debug(marker: Marker?, t: Throwable?, msg: () -> Any?): Unit = debug(t, marker, msg)

-  @Deprecated("use info instead", ReplaceWith("info(t, marker, msg)"))
+  /** Lazy add a log message with a marker if isInfoEnabled is true */


The documentation comment doesn't mention the throwable parameter 't'. Consider updating the documentation to indicate that this method also accepts a throwable, for example: "Lazy add a log message with a marker and throwable if isInfoEnabled is true".

Suggested change

/** Lazy add a log message with a marker if isInfoEnabled is true */

/** Lazy add a log message with a marker and throwable if isInfoEnabled is true */

Copilot · 2026-01-06T19:06:44Z

src/commonMain/kotlin/io/github/oshai/kotlinlogging/KLogger.kt

+  /** Lazy add a log message with a marker if isWarnEnabled is true */
  public fun warn(marker: Marker?, t: Throwable?, msg: () -> Any?): Unit = warn(t, marker, msg)


The documentation comment doesn't mention the throwable parameter 't'. Consider updating the documentation to indicate that this method also accepts a throwable, for example: "Lazy add a log message with a marker and throwable if isWarnEnabled is true".

Copilot · 2026-01-06T19:06:44Z

src/commonMain/kotlin/io/github/oshai/kotlinlogging/KLogger.kt

  /** Lazy add a log message if isInfoEnabled is true */
-  public fun info(throwable: Throwable? = null, marker: Marker?, message: () -> Any?): Unit =
+  public fun info(throwable: Throwable?, marker: Marker?, message: () -> Any?): Unit =
    at(Level.INFO, marker) {
      this.message = message.toStringSafe()
      this.cause = throwable
    }


The documentation comment doesn't mention the marker parameter. Consider updating the documentation to indicate that this method also accepts a marker, for example: "Lazy add a log message with throwable and marker if isInfoEnabled is true".

Copilot · 2026-01-06T19:06:44Z

src/commonMain/kotlin/io/github/oshai/kotlinlogging/KLogger.kt

  /** Lazy add a log message if isWarnEnabled is true */
-  public fun warn(throwable: Throwable? = null, marker: Marker?, message: () -> Any?): Unit =
+  public fun warn(throwable: Throwable?, marker: Marker?, message: () -> Any?): Unit =
    at(Level.WARN, marker) {
      this.message = message.toStringSafe()
      this.cause = throwable
    }


The documentation comment doesn't mention the marker parameter. Consider updating the documentation to indicate that this method also accepts a marker, for example: "Lazy add a log message with throwable and marker if isWarnEnabled is true".

Copilot · 2026-01-06T19:06:44Z

src/commonMain/kotlin/io/github/oshai/kotlinlogging/KLogger.kt

+  /** Lazy add a log message with a marker if isTraceEnabled is true */
  public fun trace(marker: Marker?, t: Throwable?, msg: () -> Any?): Unit = trace(t, marker, msg)


The documentation comment doesn't mention the throwable parameter 't'. Consider updating the documentation to indicate that this method also accepts a throwable, for example: "Lazy add a log message with a marker and throwable if isTraceEnabled is true".

Copilot · 2026-01-06T19:06:45Z

src/commonMain/kotlin/io/github/oshai/kotlinlogging/KLogger.kt

+  /** Lazy add a log message with a marker if isErrorEnabled is true */
  public fun error(marker: Marker?, t: Throwable?, msg: () -> Any?): Unit = error(t, marker, msg)


The documentation comment doesn't mention the throwable parameter 't'. Consider updating the documentation to indicate that this method also accepts a throwable, for example: "Lazy add a log message with a marker and throwable if isErrorEnabled is true".

Copilot · 2026-01-06T19:06:45Z

src/commonMain/kotlin/io/github/oshai/kotlinlogging/KLogger.kt

  /** Lazy add a log message if isTraceEnabled is true */
-  public fun trace(throwable: Throwable? = null, marker: Marker?, message: () -> Any?): Unit =
+  public fun trace(throwable: Throwable?, marker: Marker?, message: () -> Any?): Unit =
    at(Level.TRACE, marker) {
      this.message = message.toStringSafe()
      this.cause = throwable
    }


The documentation comment doesn't mention the marker parameter. Consider updating the documentation to indicate that this method also accepts a marker, for example: "Lazy add a log message with throwable and marker if isTraceEnabled is true".

Copilot · 2026-01-06T19:06:45Z

src/commonMain/kotlin/io/github/oshai/kotlinlogging/KLogger.kt

  /** Lazy add a log message if isDebugEnabled is true */
-  public fun debug(throwable: Throwable? = null, marker: Marker?, message: () -> Any?): Unit =
+  public fun debug(throwable: Throwable?, marker: Marker?, message: () -> Any?): Unit =
    at(Level.DEBUG, marker) {
      this.message = message.toStringSafe()
      this.cause = throwable
    }


The documentation comment doesn't mention the marker parameter. Consider updating the documentation to indicate that this method also accepts a marker, for example: "Lazy add a log message with throwable and marker if isDebugEnabled is true".

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-06T20:18:20Z

src/javaMain/kotlin/io/github/oshai/kotlinlogging/slf4j/internal/LocationAwareKLogger.kt

+  override fun trace(marker: Marker?, t: Throwable?, msg: () -> Any?): Unit =
    at(Level.TRACE, marker) {
-      this.message = message.toStringSafe()
-      this.cause = throwable
+      this.message = msg.toStringSafe()
+      this.cause = t
    }


The parameter names have been changed from throwable/message to t/msg for consistency with the full three-parameter signature. While this improves consistency within the method set, consider that throwable and message are more descriptive and self-documenting names. The abbreviated forms t and msg may reduce code readability, especially for developers new to the codebase.

Copilot · 2026-01-06T20:18:21Z

src/commonMain/kotlin/io/github/oshai/kotlinlogging/KLogger.kt

+  public fun trace(marker: Marker?, t: Throwable?, msg: () -> Any?): Unit =
+    at(Level.TRACE, marker) {
+      this.message = msg.toStringSafe()
+      this.cause = t
+    }


The parameter names have been abbreviated from throwable/message to t/msg. While this achieves consistency with the three-parameter signature, it reduces readability compared to the more descriptive full names. Consider using throwable and message for better self-documentation.

Copilot · 2026-01-06T20:18:21Z

src/commonMain/kotlin/io/github/oshai/kotlinlogging/KLogger.kt

+  /** Lazy add a log message with a marker and throwable if isTraceEnabled is true */
+  public fun trace(marker: Marker?, t: Throwable?, msg: () -> Any?): Unit =
+    at(Level.TRACE, marker) {
+      this.message = msg.toStringSafe()
+      this.cause = t
+    }


There's an inconsistency in parameter ordering across the API. The two-parameter method signature is info(throwable: Throwable?, message: () -> Any?) where throwable comes first, but the three-parameter method signature is info(marker: Marker?, t: Throwable?, msg: () -> Any?) where marker comes first and throwable second.

This creates a confusing API where the position of the throwable parameter changes based on whether a marker is present. For consistency, consider either:

Keeping the throwable-first ordering: info(throwable: Throwable?, marker: Marker?, message: () -> Any?)

Or changing the two-parameter signature to match marker-first: info(marker: Marker?, message: () -> Any?) and info(throwable: Throwable?, message: () -> Any?) with different names or structure

The current design violates the principle of least surprise since users will expect consistent parameter positioning.

Marker First (marker, throwable, message) makes more sense.

Here is the reasoning (the "answer") you can use to address the review:

Semantic Hierarchy: A Marker is context/metadata about the log event (similar to the Log Level), whereas the Throwable is part of the payload/content (similar to the message). It is standard design to place context parameters before data parameters:
log(Context, Data)
.
SLF4J Convention: The underlying SLF4J API (which this library is heavily inspired by and often wraps) consistently places Marker as the first argument in all its signatures. Deviating from this established convention would actually be more surprising to users familiar with the Java logging ecosystem.
Consistency with 2-arg overloads: We have
info(marker, message)
and
info(throwable, message)
. When combining them, preserving the "Context before Content" order results in
info(marker, throwable, message)
.
Conclusion: The seeming inconsistency of "Throwable first" in the 2-arg overload (
info(throwable, message)
) is simply because there is no Marker context there. Once Marker (context) is introduced, it logically takes precedence as the leading argument.

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

oshai added 3 commits January 6, 2026 16:07

align KLogger signature

f692b3a

align KLogger signature

656e8d3

align KLogger signature

462b58f

oshai changed the title ~~align KLogger signature~~ KLogger API Cleanup & Ambiguity Fix Jan 6, 2026

oshai requested a review from Copilot January 6, 2026 19:02

Copilot started reviewing on behalf of oshai January 6, 2026 19:03 View session

Copilot AI reviewed Jan 6, 2026

View reviewed changes

remove some more methods

c32b90b

oshai requested a review from Copilot January 6, 2026 20:13

Copilot started reviewing on behalf of oshai January 6, 2026 20:14 View session

Copilot AI reviewed Jan 6, 2026

View reviewed changes

remove some more methods

7f693a1

Copilot AI reviewed Jan 6, 2026

View reviewed changes

oshai requested a review from Copilot January 6, 2026 21:32

Copilot started reviewing on behalf of oshai January 6, 2026 21:32 View session

Copilot AI reviewed Jan 6, 2026

View reviewed changes

oshai added 2 commits January 7, 2026 08:47

minor changes

1ffb3a1

minor docs

de7fe8e

oshai merged commit 10f0a6f into master Jan 7, 2026
9 checks passed

oshai deleted the signatures branch January 7, 2026 23:06

		/** Lazy add a log message with a marker if isDebugEnabled is true */
		public fun debug(marker: Marker?, t: Throwable?, msg: () -> Any?): Unit = debug(t, marker, msg)

		/** Lazy add a log message with a marker if isWarnEnabled is true */
		public fun warn(marker: Marker?, t: Throwable?, msg: () -> Any?): Unit = warn(t, marker, msg)

		/** Lazy add a log message with a marker if isTraceEnabled is true */
		public fun trace(marker: Marker?, t: Throwable?, msg: () -> Any?): Unit = trace(t, marker, msg)

		/** Lazy add a log message with a marker if isErrorEnabled is true */
		public fun error(marker: Marker?, t: Throwable?, msg: () -> Any?): Unit = error(t, marker, msg)

Uh oh!

KLogger API Cleanup & Ambiguity Fix #586

KLogger API Cleanup & Ambiguity Fix #586

Conversation

oshai commented Jan 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

oshai Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

oshai commented Jan 6, 2026 •

edited

Loading