Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MDC attributes are not available on underlying thread #910

Open
shikhahere01 opened this issue Aug 4, 2021 · 7 comments
Open

MDC attributes are not available on underlying thread #910

shikhahere01 opened this issue Aug 4, 2021 · 7 comments

Comments

@shikhahere01
Copy link

shikhahere01 commented Aug 4, 2021

Hi,

I am using the MDCInitializer and let method, to pass the MDC context from main thread to the underlying threads when asyncronus call is made.
I see that the MDC infomation is available on
http-nio-8081-exec-3
but not available on
finagle/netty4-2-29
and
Netty 4 Timer-1
so basically when the resonse is returned which is on different thread i dont see the MDC information available

Here is how my code looks like, what could be the reason here?

public class FinagleMDCFilter extends SimpleFilter<Request, Response> {

    public FinagleMDCFilter() {
        MDCInitializer.init();
    }

    public Future<Response> apply(Request request, Service<Request, Response> service) {
        HashMap<String,String> map = (HashMap<String, String>) MDC.getCopyOfContextMap();
        Future<Response> responseFuture = MDCInitializer.let(
           map, () -> {
               Future<Response> future =  service.apply(request);
               return future;
           });
        return responseFuture;
    }
}
@yufangong
Copy link
Contributor

Hi @shikhahere01,
The MDCInitializer.let would put the context in the current scope of LocalContext, and finagle can manage it across threads per-request. Are you accessing the MDC context inside the function scope, in your case is the service.apply(request)?

Thanks!

@shikhahere
Copy link

Hi @yufangong
I was accessing it in the exceptionally block when the response is returned and also in the Resposeclassifier. Basically I have thenApply().exceptionally() , so if there is an exception there I would like to log it with MDC information. Also in Responseclassifier which is on a different thread too.

@mosesn
Copy link
Contributor

mosesn commented Aug 19, 2021

@shikhahere I'm a bit confused. Are you converting from a Twitter Util Future => Java Completable Future? I don't think that preserves MDC context by default.

@shikhahere
Copy link

Yes that is correct. How do I preserve that ?

@shikhahere
Copy link

It is ofcourse preserved on main thread. Just not in exception and timeout

@shikhahere01
Copy link
Author

@mosesn any advice?

@cacoco
Copy link
Contributor

cacoco commented Sep 8, 2021

@shikhahere01 @shikhahere as @yufangong asked and alluded to, for your code above are you accessing the MDC values from within the function scope of the MDCInitializer.let closure? If not the values will purposely not be visible outside of that closure. Also, have you taken a look at the documentation? https://twitter.github.io/finatra/user-guide/logging/mdc.html#usage as it mentions there this only works for Finagle managed contexts, that is this MDC integration won't work with Java Futures and you would need logic to copy over the MDC into the Java Future execution context. From your snippet it isn't clear exactly what constructs you're using but in the answer to @mosesn you seem to indicate you are converting and if that is the case, you will need to manually preserve MDC context during your conversion. Hope that helps. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

5 participants