[slf4j-dev] [JIRA] Updates for SLF4J-531: MDC should support stack values identified by key

[QOS.CH (JIRA)]

SLF4J / SLF4J-531 [Open]
MDC should support stack values identified by key

==============================

Here's what changed in this issue in the last few minutes.

There is 1 comment.

View or comment on issue using this link
https://jira.qos.ch/browse/SLF4J-531

==============================
 1 comment
------------------------------

Ralph Goers on 16/Apr/22 6:53 PM
Prior to any of this Log4j supported  a ThreadContext that contained a Map<String, String> per thread and a ContextStack<String> per thread. These were both included since Log4j 1.x had equivalents for both. While the Map has proven highly useful the Stack has never seemed useful to me since the requirement that the values be removed in the opposite order they were added always seemed fragile. While the implementation of the ThreadContextStack is done with a List in Log4j 2, users still have to access it as a Stack in the API, so treating it as a List would be a mistake.  The Thread Context Stack is a queue that is accessed via push(), pop(), and peek() as defined by ThreadContext.ContextStack.  The ThreadContextStack in Log4j 2 has always been a separate data structure as it was in Log4j 1 - i.e. a single stack per thread.

As for which questions we would be discussing I don't think we are discussing Q1 at all. The single ThreadContextStack has existed for years. It isn't a matter of which is better. It is a simple matter of it has to continue to exist to ensure compatibility. As for Q2, Again, I think the question is mis-framed. We are not comparing a Map<String, String> to a ThreadContextStack nor are we discussing merging the ThreadContextStack with the Map.

IIUC your use case is in support of span ids. I've never found the value of them myself (we use a request id, which should be analogous to a trace id) and session ids (that have nothing to do with an actual server session but represent a single user's login), but given this I can understand why you would want a data structure that is separate from the standard single NDC as users might be using that for unexpected stuff. That said, using the ThreadContextMap works for that so long as you are sure the key doesn't conflict with some other usage (of course, a key conflict could also arise with your use of a Map<String, Dequeue<String>> as well).

I would suggest that the appropriate way to deal with this would be to use push and pop as you have shown them. If you want the Map to contain a list instead then you should do

ThreadContextMap.add(key, "value");
and 
ThreadContextMap.getFirst(key) or getLast(key) or removeFirst(key) or removeLast(key) or remove(index).

In other words, there is value in having the MDC support various types of collections or a single value, not just Dequeues. If you want to deal with it as a Collection then use the appropriate methods. If you want to deal with it as a List then use the methods appropriate for that. So long as you mandate that there is no guarantee that the keys will be unique across data representations then you are free to implement this using a single Map or using multiple Maps.


==============================
 This message was sent by Atlassian Jira (v8.8.0#808000-sha1:e2c7e59)