gRPC Zap ReplaceGrpcLoggerV2

[kush-patel-hs]

We at Hootsuite were noticing noisy logs coming from grpc and we noticed it's because grpc_zap implements logger v1 interface. We need the logger v2 implemented instead, so I have opened this PR.

This is a revived and comments addressed version of #144 which was opened by @jamisonhyatt. Feel free to just merge this in after approval if it looks good!

[googlebot]

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.

---

What to do if you already signed the CLA

Individual signers

• It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

• Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
• The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
• The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

[codecov-io]

Codecov Report

Merging #221 into master will decrease coverage by 1.73%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master     #221      +/-   ##
==========================================
- Coverage   73.28%   71.55%   -1.74%     
==========================================
  Files          37       37              
  Lines        1359     1392      +33     
==========================================
  Hits          996      996              
- Misses        314      347      +33     
  Partials       49       49

Impacted Files	Coverage Δ
logging/zap/grpclogger.go	0% <0%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 27f3801...cd7353a. Read the comment docs.

[kush-patel-hs]

Other path would be instead of introducing ReplaceGrpcLoggerV2, just change ReplaceGrpcLogger internals. The interface to the user doesn't change (input is still only a zap.Logger).

[kush-patel-hs]

I signed it!

[googlebot]

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

[kush-patel-hs]

@domgreen

[kush-patel-hs]

@devnev

[domgreen]

Thanks for this @kush-patel-hs and thanks for poking us to keep up the momentum. 👍 Appreciate new contributions.

Looks good for me just a nit ... merge once @devnev is happy (will poke ;) )

[kush-patel-hs]

Thanks @domgreen! Pretty excited to get this in, will help us clear up some noisy logs from using the v1 logger implementation at Hootsuite

[domgreen]

Speaking with @devnev offline we can make this match much closer with the upstream code https://github.com/grpc/grpc-go/blob/master/grpclog/loggerv2.go and remove some of the complexity around options :)

[kush-patel-hs]

Thanks for all the time you two spent on reviewing and providing feedback so far @domgreen @devnev :) I think I've addressed all feedbacks now, let's push this through :)

[domgreen]

Thanks for the changes 👍

[kush-patel-hs]

Thanks for all the help 👍

[fproulx-dfuse]

@kush-patel-hs Hi! I tried this. My default zap.Logger instance is set to Info level and it appears that setting the severity to 2 like grpc_zap.ReplaceGrpcLoggerV2WithVerbosity(zlog, 2) does not lower the noise. Skimming through the code this severity appears ? to be ignored ? So should I create a different zap.Logger with Error level before calling ReplaceGrpcLoggerV2WithVerbosity ?

[kush-patel-hs]

@fproulx-eoscanada verbosity 2 will increase noise, the higher the verbosity number the more noise (i.e max noise is 99). Less noise is 0. Verbosity will be set to 0 if you simply do grpc_zap.ReplaceGrpcLoggerV2(zlog). You would notice the LoopyWriter logs transport closing in particular go away changing from verbosity 2 to verbosity 0. If you want even less noise than that then yes create a new zap logger with Error or Warn level before doing the replace.

[fproulx-dfuse]

@kush-patel-hs ah! great so let's set it back to 0 and try again =)
Thanks -- great contribution by Hootsuite. We love grpc and zap too =)

[kush-patel-hs]

Awesome :) let me know if you need any more help!
And thank you, appreciate it :)

[Icedroid]

Who can add logrus log with ReplaceGrpcLoggerV2?

[srp]

In case others run across this thread, you might want or need to pass a sublogger with a different level then your root logger. This can look something like:

grpc_zap.ReplaceGrpcLoggerV2(logger.WithOptions(zap.IncreaseLevel(zap.WarnLevel)))

And it appears that "increase" means more restrictive, with less log lines getting through, not "increase" as in increasing the volume of the lines being emitted.