search

src-d / lookout-sdk

SDK for lookout analyzers
Apache License 2.0
4 stars 11 forks source link

Change Go interceptors order #85

Closed carlosms closed 5 years ago

carlosms commented 5 years ago

Part of #84.

While migrating lookout I noticed that we can't use pb.AddFields inside a client interceptor. This is because pb.DialContextWithInterceptors was executing Ctxlog*ClientInterceptor before the user provided ones. With the change in this PR the interceptors are called in this order:

user client interceptors -> lookout-sdk client Ctxlog interceptor -> ...gRPC call...
-> lookout-sdk server Ctxlog interceptor -> user server interceptors

Instead of the previous one:

lookout-sdk client Ctxlog interceptor -> user client interceptors -> ...gRPC call...
-> lookout-sdk server Ctxlog interceptor -> user server interceptors

@se7entyse7en can you please check if we can have a similar problem in the python API?

se7entyse7en commented 5 years ago

I was thinking that actually the Wrapped context AFAIR is actually not available at all in the interceptors as the api exposes a different data structure. If we want to enable to user to handle log fields inside the interceptors also for the Python SDK I think that we need to open an issue and investigate how we can approach it.

se7entyse7en commented 5 years ago

I had a look and it is always possible to handle log fields inside client interceptors as it is handled differently. But it's not documented how to do it, it's just used for client logger interceptors.