logging-log4j2
logging-log4j2 copied to clipboard
apache
Add support for stack valued MDC
Support for SFL4J stack valued MDC entries is emulated by adding JSON-ish arrays as values in ThreadContext.
I don't expect performance to be an issue in this feature.
Please add a README to this that essentially says that SLF4J 2.0 is in alpha status, so is not considered to be production ready. Accordingly, users may experience breakages with future SLF4J 2.0 releases. Log4j will attempt to resolve those in new releases potentially at the cost of being incompatible with prior SLF4J 2.0 releases.
I can't see how we are seriously talking about adding JSON code to log4j-api when we can't even agree to add a set level API. Surely a logging a API and JSON should not be tied. What am I missing?
I think stacked value thread-context could be useful, but I don't think it's a good idea to encode the data into strings. I haven't been following along the design discussion, and I'm sorry for showing up late in the game! Is there any reason we couldn't or shouldn't support such a feature directly in the log4j-api? We already have a thread context map (equivalent to mdc) and a thread context stack (which operates on string values rather than key/value pairs), a combination of the two seems natural.
@carterkozak, you are certainly not late in the game, since the game hasn't started yet.
I provided a non-invasive and non-optimized implementation, because personally I don't believe in the feature's popularity. Since the opinions clearly diverge, I propose to follow up the discussion on the dev mailing list. I believe that @ceki also follows it and he will be able to provide his input.
Background: As we all know Log4j 1.x supports an MDC and NDC. All this time I thought SLF4J did too but it seems it has only supported the MDC. Recently it seems that Ceki has tried to add support for things like the W3C Trace Context and the Open Tracing API. Rather than introduce the NDC to SLF4J he decided a Map of queues would be better. I really can't argue with the idea that it is better to have a Map of queues rather than a single queue. I've always found the NDC to be problematic. To be honest, I would have preferred that instead of a Map of queues that instead this be looked at adding support for Lists of elements to the existing ThreadContextMap. That said, lists and queues have a lot in common so one could easily imagine it being used to simply contain a set of elements in a key in the ThreadContextMap.
In any case, to support SLF4J 2.0 we have to support whatever Ceki adds whether we like it or not. In discussing this with Piotr we wanted to avoid affecting the Log4j 2 API. That means we need a way to make the existing data structures work. The only way we could come up with was converting the list to a String since that is all the ThreadContext supports. As Piotr says, we would prefer the bridge not depend on Log4j core. In addition, I am going to require JsonReader in Log4j API in 3.0 for https://cwiki.apache.org/confluence/display/LOGGING/Properties+Enhancement. JsonReader is simple and isn't like including Jackson or Gson and is less complicated than some of the other utility classes we have in Log4j API. Because it provides functionality required at the API level I see no problem with including it. While JsonWriter is more complicated than JsonReader it isn't by much.
Of course, we could simple add the support to the API but as with Piotr I don't think a list of stacks will be much more popular than the NDC. But allowing the ThreadContextMap to also contain collections as well as String values could be.