apache / apache/logging-log4j2
Consider removing accessors from `MDC <-> ThreadContext` bridges
- Dominant language
- Java
- Stars
- 3.6k
- Forks
- 1.7k
- Avg merge
- 21h 30m
- Merged PRs (30d)
- 27
Description
Currently we have two bridges between `MDC` and `ThreadContext`:
- [`MDCContextMap`](https://github.com/apache/logging-log4j2/blob/2.x/log4j-to-slf4j/src/main/java/org/apache/logging/slf4j/MDCContextMap.java) in `log4j-to-slf4j` that forwards `ThreadContext` calls to `MDC`,
- [`Log4jMDCAdapter`](https://github.com/apache/logging-log4j2/blob/2.x/log4j-slf4j2-impl/src/main/java/org/apache/logging/slf4j/Log4jMDCAdapter.java) in `log4j-slf4j2-impl` and `log4j-slf4j-impl` that forwards `MDC` calls to `ThreadContext`.
While the **mutator** methods of these bridges are required by user code, the **accessors** are basically useless:
- Logback does not call any `ThreadContext` accessor,
- Log4j Core does not call any `MDC` accessor.
The accessors are only used by third-party integrators like the [Context Propagation Library](https://github.com/micrometer-metrics/context-propagation) (see [`Slf4jThreadLocalAccessor`](https://github.com/micrometer-metrics/context-propagation/blob/main/context-propagation/src/main/java/io/micrometer/context/integration/Slf4jThreadLocalAccessor.java) for example).
In view of a future integration with `context-propagation` I would propose to:
- modify `MDCContextMap` accessors to always return `null` in the `get` and `getImmutableMapOrNull` and return a new `HashMap` in the `getCopy` method. These values are allowed by the [`ThreadContextMap`](https://logging.apache.org/log4j/2.x/javadoc/log4j-api/org/apache/logging/log4j/spi/ThreadContextMap.html) contract,
- modify `Log4jMDCAdapter` accessors to always return `null`. This value is allowed by the [`MDCAdapter`](https://www.slf4j.org/api/org/slf4j/MDC.html) contract.
These changes will guarantee that **at least** one of `MDC#getCopyOfContextMap` and `ThreadContextMap#getImmutableMapOrNull` will return `null`. According to the semantics of the `context-propagation` project, `null` means "don't propagate" and will prevent the propagation of `MDC` for Log4j Core users and the propagation of `ThreadContext` for Logback users.
**Remark**: we can also introduce a `log4j.threadContext.map.bridgeAccessors` property in Log4j 3 and `log4j2.threadContextMapBridgeAccessors` property in Log4j 2 to allow users that use `MDC` and `ThreadContext` for non-logging purposes to restore the previous behavior.
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.