I don't really understand this. All of these ThreadContextMaps seem to do this same thing. It isn't a "save". All you are doing is getting a reference to the Map. In many of these cases the Map that was "saved" could change after the save operation since it is pointing to the same copy of the Map. Also, what is the value of "saving" a referece to the Map? I could understand if it was storing JSON as that could transmitted.

The DefaultThreadContextMap and CopyOnWriteSortedArrayThreadContextMap just return the underlying object, because they use immutable structures (an immutable map for the former and a frozen StringMap for the latter.

If you look at the implementation of GarbageFreeSortedArrayThreadContextMap it makes a copy of the map:

It would still be possible to make this garbage-free if we use a recycler for the copied map.

This bothers me a LOT. Without having some idea of the saved format calling restore could end up being a disaster.

I am trying to figure out when, how, or why I would use this. I could imagine using it to send the ThreadContextMap to a downstream service but none of the implementations actually save anything useful for that. While this could be used to save the current state so I can add more entries and then just restore the map to its previous state, this doesn't actually provide that functionality. The caller would still have to invent some place to store the stack of maps, so you would probably still need an extra ThreadLocal to do that.

The main application I had in mind for this method is ThreadLocalAccessor#getValue used in Spring Reactor.

When working with the reactive stack (cf. Spring's documentation) the processing of a web request is represented as a pipeline of CoreSubscriber objects, where each subscriber processes the output of the previous stage. The onX methods may be called on any thread, so the context data is stored in the CoreSubscriber object. As far as I know they don't use ThreadLocal.

The context propagation offered by Micrometer is also supposed to work in other stacks. For example the Netty-based Spring Webflux WebClient is supposed to:

• propagate the context data through Netty's outbound handlers,
• store it in Netty's channel before the request hits the wire,
• restore it when an answer is received,
• propagate the context data through Netty's inbound handlers.

As far as I understand the propagation is restricted to the bounds of the JVM, so the opaque object in the context is not a problem. This object serves a single purpose: supply the object to the restoreMap method of the ThreadContextMap that created it.

I have a similar use case in https://github.com/spinnaker/kork/blob/master/kork-security/src/main/java/com/netflix/spinnaker/security/AuthenticatedRequestDecorator.java where thunks are decorated to propagate contexts. Having to clear out the whole map and copy data over seems error-prone. Runtimes that don't map directly to physical threads may involve swapping out the thread context data a lot (such as WebFlux or Ktor).

@jvz If I am not mistaken you should be able to use ScopedContext for this. However, I think I must misunderstand something. Your code does:

• Copy the context map.
• Clear the context map.
• populate the context map using all the keys and values from the copy.

What exactly does this accomplish? I understand copying the map so you can restore it on the way out in case it was modified, but copying, clearing, and repopulating seems unnecessary.

FWIW, If you were using a ScopedContext anything added to the context would automatically be cleared when your run method exits.

In thinking about this I realize why I dislike this.

1. As much as you didn't like the get method of ScopedContext i really dislike adding these two methods to the ThreadContextMap interface. That is the public API and these methods are dangerous. I would also not expect the public API of an immutable map would let itself be replaced. I would suggest creating a separate interface and have the ThreadContextMap implementations implement that.
2. IMO save and restore are terrible names. They make it sound like the context is being serialized. Instead they should be getMap() and setMap() since that is what they explicitly do.
3. I just merged a commit that causes all ContextDataProviders to be treated equally. This change only applies to the ThreadContextMap. I would suggest that the interface you are creating should be equally applicable to any ContextDataProvider - if they implement the new interface. This could mean you could modify the ContextData class to have methods like

   Map<String, ?> getContextMap(String provider);

   Map<String, Map<String, ?> getContextMaps();

   void setContextMap(String provider, Map<String, ?> map);
   
   void setContextMaps(Map<String, Map<String, ?>> maps);

I don't think when saving and restoring you would want to merge the maps as when you "restore" you wouldn't get what you started with.

1. As much as you didn't like the get method of ScopedContext i really dislike adding these two methods to the ThreadContextMap interface. That is the public API and these methods are dangerous. I would also not expect the public API of an immutable map would let itself be replaced. I would suggest creating a separate interface and have the ThreadContextMap implementations implement that.

The ThreadContextMap is in the o.a.l.l.spi package, so it is not what I consider public API. Same applies to ExtendedThreadContext. This package already contains dangerous methods like LoggerContextFactory#shutdown, but I don't consider this a problem, because these are used by integrators that are supposed to know what they are doing.

2. IMO save and restore are terrible names. They make it sound like the context is being serialized. Instead they should be getMap() and setMap() since that is what they explicitly do.

I agree, the names are not great. What would you think about getContextData and setContextData?

3. I just merged a commit that causes all ContextDataProviders to be treated equally. This change only applies to the ThreadContextMap. I would suggest that the interface you are creating should be equally applicable to any ContextDataProvider - if they implement the new interface. This could mean you could modify the ContextData class to have methods like

   Map<String, ?> getContextMap(String provider);

   Map<String, Map<String, ?> getContextMaps();

   void setContextMap(String provider, Map<String, ?> map);
   
   void setContextMaps(Map<String, Map<String, ?>> maps);

I don't think when saving and restoring you would want to merge the maps as when you "restore" you wouldn't get what you started with.

I am not convinced this is a good idea. Users register custom ContextDataProviders to inject custom data into logs. They are not interested in Log4j propagating this data to other threads, because they can do it on their own. Look at log4j-context-data-2.17 for example: the propagation of the traceId and spanId is a core functionality of their library. They don't need Log4j Core to do it for them.

That is why we only need to provide integration points to propagate native Log4j sources. Even MDCContextMap should probably have no-op save and restore methods. This is also why I am opposed to having two thread locals that users need to propagate.