`trace`/`debug`/`info`/`warn`/`error` with marker deprecated though alternative is ambigious

[jensbaitingerbosch]

I will explain this bug with info(...), the same issue with debug, warn etc.

KLogger.info(Marker, ()-> Any?) is marked as deprecated withe the replacement info(null, marker, msg)

The issue is, that using this replacement is ambiguous causing the following compiler error:

Overload resolution ambiguity between candidates:
fun info(marker: Marker?, t: Throwable?, msg: () -> Any?): Unit
fun info(marker: Marker?, msg: String?, arg: Any?): Unit

Only calling the function like this log.info(marker, throwable=null, msg) satisfies the compiler. Which makes the call more complex (even with log.info(marker, null, msg) the null looks unnecessary.

Suggestion: undeprecate the function KLogger.info(Marker, ()-> Any?).

[github-actions]

Thank you for reporting an issue. See the wiki for documentation and slack for questions.

[oshai]

I understand the issue. The thing is that too many options for parameters also makes the api cumbersome and confusing.

Another option is using:

logger.atInfo(marker) {
  message = msg
}

[xjw580]

I understand the issue. The thing is that too many options for parameters also makes the api cumbersome and confusing.

Another option is using:

logger.atInfo(marker) {
  message = msg
}

Assigning a value to the message field is very cumbersome.

[dmitry-cherkas]

Not sure I understand the value/rational of keeping the following overloads deprecated, when the proposed replacement cannot be used due to the ambiguity issue described above.

  @Deprecated("use trace instead", ReplaceWith("trace(null, marker, msg)"))
  public fun trace(marker: Marker?, msg: () -> Any?): Unit = trace(null as Throwable?, marker, msg)

  @Deprecated("use debug instead", ReplaceWith("debug(null, marker, msg)"))
  public fun debug(marker: Marker?, msg: () -> Any?): Unit = debug(null as Throwable?, marker, msg)

  @Deprecated("use info instead", ReplaceWith("info(null, marker, msg)"))
  public fun info(marker: Marker?, msg: () -> Any?): Unit = info(null as Throwable?, marker, msg)

  @Deprecated("use warn instead", ReplaceWith("warn(null, marker, msg)"))
  public fun warn(marker: Marker?, msg: () -> Any?): Unit = warn(null as Throwable?, marker, msg)

  @Deprecated("use error instead", ReplaceWith("error(null, marker, msg)"))
  public fun error(marker: Marker?, msg: () -> Any?): Unit = error(null as Throwable?, marker, msg)

The only way I can use the replacement is by actually adding the cast, i.e. xxx(null as Throwable?, marker) { ... }

Or to ask it differently, is there a problem to un-deprecate those overloads?

[delbertooo]

I understand the issue. The thing is that too many options for parameters also makes the api cumbersome and confusing.

Another option is using:

logger.atInfo(marker) {
  message = msg
}

May it be valid for convenience to keep the

(marker: Marker?, msg: () -> Any?)

signature?

About the better API

I just wanted to share my thinking about that. I may be completely wrong, then forgive me.
I totally see the point in making the API less bloated. Especially for the methods without lambdas as message.
Also there are few points for having both

(marker: Marker?, t: Throwable?, msg: () -> Any?)
(throwable: Throwable? = null, marker: Marker?, message: () -> Any?)

But I am wondering if there is a strong connection between exceptions and markers. Are we using markers primarily when exceptions have occurred? I mean we always have to juggle with exceptions if we want to log with a marker.

logger.info(null, AUDIT) { "hi" }
// or
logger.info(marker = AUDIT) { "hi" }

---

Generally, I wonder, what the benefit of this may be:

(throwable: Throwable? = null, marker: Marker?, message: () -> Any?)
                      ^^^^^^^          ^^^^^^^

The first value has a default value, followed by positional arguments without defaults. You could (technically) use it like:

logger.info(marker = AUDIT) { "hi" }

but you wouldn't be able to use any positional arguments after marker = AUDIT, that may be introduced in the future. It just seems strange to have a parameter with defaults go in front of positional arguments.

In case we don't want convenience methods like (marker: Marker?, msg: () -> Any?), couldn't we just define the signature

(throwable: Throwable? = null, marker: Marker? = null, message: () -> Any?)

and also drop these methods:

(throwable: Throwable?, message: () -> Any?)

You got less methods, no positional argument after a default valued argument and still the same calling possibilities, or am I missing something?

logger.info(null, AUDIT) { "hi" }
logger.info(marker = AUDIT) { "hi" }
logger.info(throwable = null, marker = AUDIT) { "hi" }
logger.info(null) { "hi" }
logger.info(throwable = null) { "hi" }
logger.info { "hi" }