DiagnosticLoggingAdapter is not thread-safe

[jrudolph]

This is as documented and AFAICS you have to opt-in explicitly to get one. However, that might not deter people from assuming that LoggingAdapters can be shared (as it is commonly true).

See akka/akka-http#3974 for a case where we would need an immutable one instead.

[alexklibisz]

Do you have any guidance on how this might be implemented? I'm guessing probably an actor-based implementation, but perhaps you had something else in mind (synchronized blocks, etc.)?

[alexklibisz]

After reading through the existing implementations, mostly Logging.scala, it looks like there is precedent for using @volatile and ReentrantGuard to manage private mutable members (see trait LoggingBus). So maybe we can do something along those lines that to add some methods that add/remove keys and protect the private var _mdc .

But, that doesn't make DiagnosticLoggingAdapter immutable. So perhaps to make it immutable, we can add some methods that set/remove keys and return a new DiagnosticLoggingAdapter? This still tastes a bit funny though, as I don't really see much precedent for that in the current logging, and it also seems like we could do this same kind of pattern in akka/akka-http#3974 without any changes in akka/akka.

I don't feel particularly sold on either of the approaches I mentioned above, but if I think of another one or find a reason to think either of these is right, I'll give it a pass. In any case, if you can, lmk what you're thinking here @jrudolph .

[patriknw]

I think it would be a too big change to make DiagnosticLoggingAdapter thread safe. For applications I would recommend using slf4j directly, which is also what we have in the Typed APIs. For libraries, like Akka HTTP, there is hopefully other solutions even though they might not be convenient to implement.

DiagnosticLoggingAdapter is about MDC. A typical thread safe implementation of that would be to use thread locals but that has other caveats and surprises with async programming models.

[alexklibisz]

Thanks for the feedback @patriknw . Do you have any recommendations on how to proceed in akka/akka-http#3974.