Add an integration with Log4J 2's ThreadContext

[MariusVolkhart]

Log4J 2 has a ThreadContext, which works the same way as SLF4J's MDC. Using the ThreadContext directly with coroutines breaks, but the same approach for an integration that exists for SLF4J can be used for Log4J.

The tests are copied from the SLF4J project, and are only modified to also include verification of stack state, since ThreadContext contains both a Map and a Stack.

[elizarov]

You should also add this new integration module to settings.gradle file so that it becomes part of the project.

[elizarov]

Nit: the line should end with dot, previous one should end with semicolon.

[elizarov]

The above is a misleading and fragile example. In case of any suspension in the outer code or inside of withContext the behavior of this code will change, as the thread context will get restore to the state that is captured in the coroutine.

This is true for any pattern like this:

ThreadContext.put("kotlin", "rocks")
// <-- adding suspension here breaks it
withContext(Log4JThreadContext()) { ... }

What I suggest is to design a convenient API to specify additional parameters right in the Log4JThreadContext, so that you can write the above code in a less-fragile way:

withContext(Log4JThreadContext(mapOf("kotlin" to "rockts"))) { ... }

(or something similar)

[elizarov]

Do we really need this class in the public API or in implementation for that matter? How about storing both mdc and ndc directly in Log4JThreadContext instance and cutting this middle-man out?

[MariusVolkhart]

@elizarov Thank you very much for the feedback. I feel I gained a much better understanding of coroutines from it, and appreciate you taking the time to leave it.

I've applied the changes you suggested, and will summarize them below.

• Dropped support for the logging context stack. It now only supports the Map. Conversations from the Log4J 2 maintainers suggested that using the stack is not really encouraged. This matches the behavior I've seen in other applications.
• Renamed Log4JThreadContext -> DiagnosticContext as I think it is unlikely for developers to have multiple conflicting classes of the same name, and I wanted to get away from thinking about threads.
• Added the MutableDiagnosticContext type, for cases when the state for the duration of the coroutine context should be modified. The API for this matches closely with that of Log4J's CloseableThreadContext, which rolls-back the state when close() is called.
• Added the immutableDiagnosticContext() function to create an optimized instance of DiagnosticContext for cases when the state should be captured when the coroutine context starts, but will not be modified.

[t1mwillis]

@MariusVolkhart Any updates on this integration?

[MariusVolkhart]

@t1mwillis What sort of an update are you looking for? I believe this code to be working, but I've only had limited ability to deploy it into production.

@elizarov would you please take another look at this?

[MariusVolkhart]

@t1mwillis It has recently occurred to me that it may be possible to implement the ThreadContextMap interface in such a way that the CoroutineContext is used as the storage container, rather than a ThreadLocal map as is used by the default ThreadContextMap subclasses. The API would probably look pretty similar, but there would likely be one round of copying things from the CoroutineContext to theThreadLocal and back saved.

I haven't had the chance to investigate this yet.

[t1mwillis]

@MariusVolkhart my use case was simply having a structured way to write a tracking id from the parent thread to a coroutine. At present I am grabbing this tracking id from the parent thread and then setting it in the coroutine using threadContext.put('trackingid', parentThreadTrackingId). This works but seems unnecessary