go-grpc-middleware icon indicating copy to clipboard operation
go-grpc-middleware copied to clipboard

Extract zap logger from context for logging server interceptors

Open implmnt opened this issue 3 years ago • 4 comments

This PR extends the current zap logging server interceptors API. It makes possible to reuse existing logger from GRPC request context for logging user's contextual data (e.g. the user_id that was shifted before into the logging context).

implmnt avatar Nov 29 '21 18:11 implmnt

Hi Georgy. I'm not sure this is a change we want to make, could you explain the rationale for this new API? Please note that we have moved all new feature development to the v2 branch too, where we have a new structure for these sort of implementations.

johanbrandhorst avatar Nov 30 '21 02:11 johanbrandhorst

Hi Johan. Thanks for notifying me about v2 branch. Will check it too.

The reason of the new API is making possible to reuse logger that was inited in some before step.

For expample: I have an interceptor which lifts user_id to logger context and I expect to see it through all logs of whole application. The current logging interceptor allows to use the only logger that was passed through its context-less constructor and does not provide access to the request context. func UnaryServerInterceptor(logger *zap.Logger, opts ...Option) grpc.UnaryServerInterceptor {

So I think it would be better to have a more flexible API that allows extracting logger from request context with some predefined fields like user_id.

implmnt avatar Nov 30 '21 11:11 implmnt

@bwplotka do you have any thoughts on this?

johanbrandhorst avatar Dec 01 '21 03:12 johanbrandhorst

Sorry for lag!

Yes, while this is great, we have opportunity to change things on v2 - we would love to not add new features to current v1 (master)

bwplotka avatar Apr 06 '22 14:04 bwplotka