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

[question] using interceptor to populate coroutine context from listener #196

Closed
dariuszkuc opened this issue Oct 22, 2020 · 6 comments
Closed

Comments

@dariuszkuc
Copy link

Hello!

Was wondering if there is any way to populate coroutine context from the interceptor listeners, i.e. we would like to populate additional MDC elements based on the incoming request and afaik we can only access those from the listener.

class CustomServerInterceptor : ServerInterceptor {
    override fun <ReqT : Any, RespT : Any> interceptCall(call: ServerCall<ReqT, RespT>, metadata: Metadata, next: ServerCallHandler<ReqT, RespT>): ServerCall.Listener<ReqT> {
        return object: ForwardingServerCallListener.SimpleForwardingServerCallListener<ReqT>(next.startCall(call, metadata)) {
            override fun onMessage(message: ReqT) {
                // populate MDC here
                super.onMessage(message)
            }
        }
    }
}

Above doesn't work as listeners are invoked after processing all the interceptors. This means that context created by CoroutineContextServerInterceptor.interceptCall() will use snapshot of values up to that point and even if we modify the MDC in the listeners, those changes will be discarded when performing rpc call on the service (as it will start running a coroutine using the context from the interceptor).

Using workaround of creating custom coroutine context passed to the constructor (#66) also doesn't work as it appears that bindable service context will be initialized before invoking the listeners as well.

The only other workaround I can think of is to manually populate the MDC from within the RPC methods, e.g.

class HelloWorld : HelloWorldAPIGrpcKt.HelloWorldAPICoroutineImplBase() {
    override suspend fun helloWorld(request: HelloWorldRequest): HelloWorldResponse {
        MDC.put("foo", "bar")
        return withContext(MDCContext()) {
            HelloWorldApiProtoBuilders.HelloWorldResponse {
                helloWorldString = "Hi ${request.id}"
            }
        }
    }
}

is there any better way?

@Lin-Liang
Copy link

i also meet this issue

@lowasser
Copy link
Collaborator

This is precisely the function of CoroutineContextServerInterceptor: install it as an interceptor, and override coroutineContext with your desired context elements.

@dariuszkuc
Copy link
Author

@lowasser I might be missing something but how would I get access to the request message in custom context interceptor?

class CustomContextServerInterceptor(private val context: CoroutineContext = EmptyCoroutineContext) : CoroutineContextServerInterceptor() {

    override fun coroutineContext(call: ServerCall<*, *>, headers: Metadata): CoroutineContext {
        // populate MDC values based on metadata -> this works fine
        MDC.put("myKey", headers.get(Metadata.Key.of("myKey", Metadata.ASCII_STRING_MARSHALLER))
        // how do i populate MDC based on the incoming request?
        MDC.put("request", call?)
        return Dispatchers.Default + MDCContext() + context
   }
}

@mykevinjung
Copy link

@lowasser I guess you missed the point that the question is how to populate additional MDC elements based on the incoming request, i.e. the request message object, not ServerCall or Metadata.

@lowasser
Copy link
Collaborator

Yes, I missed that, my bad.

Given that, your described approach is as good as it gets at this time. There's a related issue, that's kind of skimmed over in your example code's comments:

            override fun onMessage(message: ReqT) {
                // populate MDC here
                super.onMessage(message)
            }

In this block, message has a completely unknown type. An interceptor is generally supposed to handle any method: any request type, streaming or unary requests. In the code you gave, you'd have to cast message to do anything with it.

@dariuszkuc
Copy link
Author

Thanks for confirming!

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

No branches or pull requests

4 participants