In a servlet environment, the overhead of MDC logging is minuscule compared to 
the servlet itself. Allocating that in a filter is about the best you can do 
here unless you don’t need it for all requests. In a reactive streams style web 
app, though, that might be a little heavier due to copying contexts between 
threads in the reactor pool more often than context changes in typical thread 
pools. I’d expect this to scale with the amount of MDC data you store.

Matt Sicker

> On Nov 8, 2021, at 14:04, Chris Hostetter <hoss...@fucit.org> wrote:
> 
> 
> : Personally, I wouldn’t bother. I have a standard practice of populating 
> : the ThreadContextMap from HTTP headers in a ServletFilter and then 
> : clearing the ThreadContext on the way out. See 
> 
> : 
> https://github.com/apache/logging-log4j-audit/blob/master/log4j-audit/log4j-audit-api/src/main/java/org/apache/logging/log4j/audit/rest/RequestContextFilter.java
>  
> 
> : Because the ServletContainer uses thread pools to serve requests it is 
> : highly unlikely it would stash anything in the ThreadContext before your 
> : Filter does. But I suppose if the ThreadContext is always empty the cost 
> 
> True -- but I would like to be a "good citizen" and respect the fact that 
> some "upstream" code (either the Servlet Container itself, or some other 
> Filter -- like perhaps the RequestContextFilter you linked to -- might 
> have set some values, and I want to respect them and their "proper" 
> lifecycle...
> 
> ...but...  My level of concern for this "good citizenship" is largely 
> dependent on how much it "costs" me in terms of performance.  I know that 
> my "downstream" code *will* put stuff in the ThreadContext that I need to 
> clean up (so it doesn't leak over to the next request) -- so I know at a 
> minimum I need to call `clearMap()` (or track all keys the application put 
> in the ThreadContext and `removeAll(...)` them).  But what I was really 
> trying to understand if there was some downside I wasn't seeing 
> that would explain *why* most ThreadContext client code seems to leave it 
> at that -- like the RequestContextFilter you linked to -- with a blanket 
> call `clearMap()`, instead of stashing/restoring a copy "just in case" 
> some upstream caller code is also populating the MDC.
> 
> : Filter does. But I suppose if the ThreadContext is always empty the cost 
> : of copying the map won’t be too bad and the call to isEmpty should be 
> : cheap.
> 
> And if the ThreadContext is _not_ empty? ... is there anything about how 
> it's implemented that would make the initial map copy (and later 
> `putAll(...)` call) more "epensive" from a performance standpoint then an 
> off the shelf 'new HashMap' ?
> 
> : > TL;DR: Are there any serious performance concerns to be aware of with 
> using something like...
> : > 
> : > doFilter(ServletRequest req, ServletResponse rsp, FilterChain c) {
> : >  final Map<String,String> ctx = ThreadContext.getContext();
> : >  try {
> : >    c.doFilter(req,rsp);
> : >  } finally {
> : >    ThreadContext.clearMap()
> : >    if ( ! ctz.isEmpty() ) { // typically empty
> : >      ThreadContext.putAll(ctx);
> : >    }
> : >  }
> : > }
> 
> 
> -Hoss
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: log4j-user-unsubscr...@logging.apache.org
> For additional commands, e-mail: log4j-user-h...@logging.apache.org

---------------------------------------------------------------------
To unsubscribe, e-mail: log4j-user-unsubscr...@logging.apache.org
For additional commands, e-mail: log4j-user-h...@logging.apache.org

Reply via email to