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

verbosity **IS NOT** about logging level #451

Closed
fxrlv opened this issue Aug 17, 2021 · 2 comments
Closed

verbosity **IS NOT** about logging level #451

fxrlv opened this issue Aug 17, 2021 · 2 comments
Labels
easy-fix Easy to fix issues help wanted

Comments

@fxrlv
Copy link

fxrlv commented Aug 17, 2021

verbosity IS NOT about logging level
grpclog package logs error by calling method (*zapGrpcLoggerV2).Error*
verbosity controls how much details we log, NOT level
could you please revert this pull request?

Originally posted by @fxrlv in #366 (comment)

@central182
Copy link
Contributor

central182 commented Aug 19, 2021

+1

The following is the default implementation of the grpclog.LoggerV2 interface and it's clear that the intention is to check
whether the verbosity of the current log (l) is within (<=) the specified threshold (g.v).
https://github.com/grpc/grpc-go/blob/41e044e1c82fcf6a5801d6cbd7ecf952505eecb1/grpclog/loggerv2.go#L199-L201

It also feels more intuitive that someone setting the threshold to level 0 (not verbose at all) doesn't want to see logs of level 2 (a bit verbose), while someone setting the threshold to level 99 (very verbose) definitely wants to see everything including logs of level 2.

The (possibly fictitious) verbosity level 99 is mentioned at https://github.com/grpc/grpc-go#how-to-turn-on-logging.

@johanbrandhorst
Copy link
Collaborator

Fixed in #452

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
easy-fix Easy to fix issues help wanted
Projects
None yet
Development

No branches or pull requests

4 participants