LOG4J-2518 LOG4J2-2433 Coroutines Support

[rocketraman]

Add support for coroutines by fixing LOG4J-2518 and LOG4J2-2433.

Suspend functions inside lambda functions are supported by making the lambdas inline, and ThreadContext support is added to integrate the thread-local ThreadContext with coroutines context.

@jvz Can you take a look please?

[jvz]

Looks like a test failure in LoggerCoroutinesTest?

[rocketraman]

Hmm, that's odd. Works for me here. Let me see what is different on the build machine.

[rocketraman]

@jvz There is an issue with this because a suspending function can't be converted to the Java-based Log4j2 Supplier without losing the suspension. I wasn't seeing it on my machine initially because the delay in the test was too small. I'm not sure if there are any good solutions here. I'll think about it a bit more, and if there are no good solutions I'll remove commit f2ca262 from the pull.

[rocketraman]

@jvz One approach is to do the message construction in the Kotlin code before delegating to log4j2. For example:

  inline fun error(supplier: () -> Any?) {
    if(delegate.isEnabled(Level.ERROR)) {
      val message = delegate.getMessageFactory<MessageFactory>().newMessage(supplier())
      delegate.logMessage(FQCN, Level.ERROR, null, message, null)
    }
  }

With this approach the supplier is invoked before the delegate, so suspending works correctly. However, it does require copying some machinery -- the isEnabled check and the message factory call -- out of the delegate. Overall though, it doesn't seem like a bad solution, and it might actually be every so slightly more performant because if that logging level is not enabled we avoid one additional frame on the stack. Thoughts?

[jvz]

If you identify an awkward API in log4j-api, we can always work on improving that upstream and later using it in version 1.1.x.

Feel free to implement that way you're describing. I'm mostly concerned about preserving caller location info where possible since some people rely on that feature (despite it causing logging to slow down by quite a bit). Now if we could use some sort of compiler feature or macros to implement the caller info, then we can safely inline a lot of things while also supporting caller info without much of a performance penalty.

[rocketraman]

Now if we could use some sort of compiler feature or macros to implement the caller info, then we can safely inline a lot of things while also supporting caller info without much of a performance penalty.

Yeah, I'm pretty sure the Kotlin compiler plugin capabilities that are upcoming will allow us to do this.

I'm mostly concerned about preserving caller location info where possible since some people rely on that feature

As per my other comment, this seems to work fine with the inlining, with the exception of the runInTrace methods. However, those are broken because ExtendedLogger does not expose running traceExit and catching with the FQCN, and was already a know issue/limitation. I had already opened https://issues.apache.org/jira/browse/LOG4J2-2275 to track this.

Feel free to implement that way you're describing.

Great, I'll work on that over the next couple of days.

[rocketraman]

As per my other comment, this seems to work fine with the inlining

Nope, my mistake, caller information does break with inlining the supplier :-(

[rocketraman]

New pull https://github.com/apache/logging-log4j-kotlin/pull/8 submitted without the inlining, until we've decided how to handle this without breaking caller location.

[jvz]

Closing this in favor of the existing implementation.