context-propagation icon indicating copy to clipboard operation
context-propagation copied to clipboard

SLF4J MDC context clear() does not clear context

Open Marcono1234 opened this issue 4 years ago • 2 comments

Expected Behavior

As per documentation of Clearable, it should clear the context of the current thread:

Clearing the context removes not only an active context from the current thread, but also the potential stack of any parents.

The correct behavior would therefore probably be to clear the MDC data using MDC.clear().

Actual Behavior

If the context was obtained from MdcManager.getActiveContext() calling clear() does nothing.
If the context was created by initializing the MDC data in the current thread (initializeNewContext(...)) calling clear() only reverts to the previous state.

The way in which nl.talsmasoftware.context.ContextManagers.clearActiveContexts() currently behaves does therefore nothing because only MdcManager.MdcContext implements Clearable and an instance of it is obtained using getActiveContext().

Specifications

  • Version: 1.0.9-SNAPSHOT (d4a6283e817bf81e3c69217d6d308a779c51f555)

Marcono1234 avatar Feb 10 '21 00:02 Marcono1234

Thank you for pointing this out. I feel some reluctance of 'clearing' values that were not part of this context, so maybe reverting to the context as it was before is better than calling MDC.clear?

I guess the question is, are pre-existing MDC values part of the captured context or not? I like to leave things the way they were. Perhaps Clearable was a mistake to introduce?

Could you provide a unit-test that points out the bug so we can discuss this with a concrete example?

sjoerdtalsma avatar Feb 10 '21 06:02 sjoerdtalsma

Perhaps Clearable was a mistake to introduce?

The documentation of ContextManagers.clearActiveContexts() provides a good example use-case where Clearable can be useful:

Appropriate use includes thread management, where threads are reused by some pooling mechanism. For example, it is considered safe to clear the context when obtaining a 'fresh' thread from a thread pool (as no context expectations should exist at that point).

When you are dealing with an executor or task execution facility provided by a third party library, there might be cases where you cannot be sure that previous tasks (not submitted by yourself) are properly cleaning up the context after themselves. In such cases calling ContextManagers.clearActiveContexts() can be useful to make sure your work task is run in a clean environment. But as pointed out by #202, if the execution environment is controlled by yourself, then there is no need to call that method.


Could you provide a unit-test that points out the bug so we can discuss this with a concrete example?

MdcManagerTest actually has a test method which demonstrates this:

https://github.com/talsma-ict/context-propagation/blob/4faad7b8cddf7bafe68ad8e81ad5aa78eb788553/mdc-propagation/src/test/java/nl/talsmasoftware/context/mdc/MdcManagerTest.java#L115-L119

I would expect that after calling clearActiveContexts() the MDC has been cleared (regardless of whether SLF4J or this library is managing it).

Though to clarify, I am fine with MdcManager.getActiveContext().close() having no effect since the rationale that the active context is managed by SLF4J makes sense here (but not for clear()).

Marcono1234 avatar Feb 11 '21 23:02 Marcono1234