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

Is this a bug? zap logging #559

Open
wrapupc opened this issue Apr 11, 2023 · 3 comments
Open

Is this a bug? zap logging #559

wrapupc opened this issue Apr 11, 2023 · 3 comments
Labels

Comments

@wrapupc
Copy link

wrapupc commented Apr 11, 2023

logger := zap.NewExample() return grpc.ChainUnaryInterceptor( logging.UnaryServerInterceptor(InterceptorZapLogger(logger)), )

I added zap into grpc interceptor, and here is what i got after a request

{"level":"info","msg":"started call","protocol":"grpc","protocol":"grpc","protocol":"grpc","protocol":"grpc","protocol":"grpc","protocol":"grpc","protocol":"grpc","protocol":"grpc"} {"level":"info","msg":"finished call","protocol":"grpc","protocol":"grpc","protocol":"grpc","protocol":"grpc","protocol":"grpc","protocol":"grpc","protocol":"grpc","protocol":"grpc","protocol":"grpc","p rotocol":"grpc","protocol":"grpc","protocol":"grpc","protocol":"grpc","protocol":"grpc","protocol":"grpc","protocol":"grpc","protocol":"grpc"}

@bwplotka
Copy link
Collaborator

Hi, can you check latest tag (https://github.com/grpc-ecosystem/go-grpc-middleware/releases/tag/v2.0.0-rc.5)? We recommend now to create your own "logging.Logger" implementation, see https://github.com/grpc-ecosystem/go-grpc-middleware/tree/main/interceptors/logging/examples/zap

Let me know if you can still repro the problem with this new approach.

@bwplotka bwplotka added the bug label Apr 13, 2023
@bwplotka
Copy link
Collaborator

I guess you actually found it in new version and fixed, nice!

#565 (review)

@marcoshuck
Copy link

marcoshuck commented Nov 3, 2023

Hi, can you check latest tag (https://github.com/grpc-ecosystem/go-grpc-middleware/releases/tag/v2.0.0-rc.5)? We recommend now to create your own "logging.Logger" implementation, see https://github.com/grpc-ecosystem/go-grpc-middleware/tree/main/interceptors/logging/examples/zap

Let me know if you can still repro the problem with this new approach.

What's the motivation behind this? I've found myself copy-pasting this code multiple times in different projects. I think that it's one of those things that consumers of the different interceptors would be very happy to have without the need to copy paste code, mostly those users that don't have very complex use cases, they just want to consume this interceptor right away.

Instead of encouraging them to create their own library to export their own logging.Logger implementation and reuse them across project, I think it would be valuable to have it here, although I'd agree that it might be zap's responsibility instead.

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

No branches or pull requests

3 participants