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

feat(logging): forward grpc internal logging to zap #1029

Merged
merged 3 commits into from
Sep 15, 2022

Conversation

GeorgeMac
Copy link
Contributor

@GeorgeMac GeorgeMac commented Sep 13, 2022

This continues on from a discussion in the original logrus to zap PR: #1020

This forwards all gRPC internal logging into zap.

Originally, I tried to use grpc_zap.ReplaceGrpcLoggerV2 in the first PR, which led to lots of gRPC chatter in the logs.
This was because the default zap level for all Flipt logs was INFO and gRPC has a lot of INFO chatter.

Normally, the default severity level for gRPC is ERROR. Which is why you don't see this.

My first attempt to improve this tapped into the env var GRPC_GO_LOG_SEVERITY_LEVEL.
However, this had the interesting effect of both forwarding logs to zap and printing gRPC logs in their native log format to stdout/stderr.

Instead, I have now introduced a new flipt compatible log.grpc_level option.
This option defaults to ERROR level. So that the default behaviour is as expected.
However, the grpc log level can always be increased independently up to the main flipt log-level (but not exceed).

see: FLIPT_LOG_GRPC_LEVEL

Normal behaviour is as expected:

Screenshot 2022-09-13 at 09 35 07

Increased gRPC log-level forwards to zap:

Screenshot 2022-09-13 at 09 27 40

Signed-off-by: George MacRorie me@georgemac.com

@codecov-commenter
Copy link

codecov-commenter commented Sep 14, 2022

Codecov Report

Merging #1029 (686ac8f) into main (4e1cd36) will decrease coverage by 0.12%.
The diff coverage is 50.00%.

@@            Coverage Diff             @@
##             main    #1029      +/-   ##
==========================================
- Coverage   81.41%   81.28%   -0.13%     
==========================================
  Files          17       17              
  Lines        1813     1817       +4     
==========================================
+ Hits         1476     1477       +1     
- Misses        263      265       +2     
- Partials       74       75       +1     
Impacted Files Coverage Δ
config/config.go 89.25% <50.00%> (-0.97%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Collaborator

@markphelps markphelps left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤘

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

Successfully merging this pull request may close these issues.

3 participants