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

Add Client interceptors for logging: logrus and zap #33

Merged
merged 2 commits into from
May 2, 2017

Conversation

mwitkow
Copy link
Member

@mwitkow mwitkow commented May 1, 2017

This adds simple client-side interceptors for logging. For client side, the default mapping mostly consists of debug-level mapping.

Changes in general:

  • added client_interceptor.go to both logrus and zap, with fairly similar and simple semantics
  • added client_interceptor_test.go to both logrus and zap, with very similar tests to the server side
  • added span.kind: server/client to both logrus and zap server and client-side interceptors to be able to differentiate the log statements.

@dackroyd @yifanzz

@codecov-io
Copy link

codecov-io commented May 1, 2017

Codecov Report

Merging #33 into master will decrease coverage by 1.48%.
The diff coverage is 62.35%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #33      +/-   ##
==========================================
- Coverage   74.43%   72.95%   -1.49%     
==========================================
  Files          30       32       +2     
  Lines         884     1050     +166     
==========================================
+ Hits          658      766     +108     
- Misses        185      241      +56     
- Partials       41       43       +2
Impacted Files Coverage Δ
logging/logrus/client_interceptors.go 100% <100%> (ø)
logging/zap/server_interceptors.go 100% <100%> (ø) ⬆️
logging/logrus/server_interceptors.go 90.76% <100%> (+3.26%) ⬆️
logging/zap/client_interceptors.go 100% <100%> (ø)
logging/zap/options.go 35.71% <33.33%> (-2.29%) ⬇️
logging/logrus/options.go 35.71% <33.33%> (-2.29%) ⬇️
logging/zap/context.go 100% <0%> (+18.18%) ⬆️
... and 1 more

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 d2c1593...e55adc9. Read the comment docs.

@@ -94,7 +95,7 @@ func (s *logrusServerSuite) TestPingError_WithCustomLevels() {
{
code: codes.Unauthenticated,
level: logrus.ErrorLevel,
msg: "Unauthenticated is overwritten to ErrorLevel with customCodeToLevel override, which probably didn't work",
msg: "Unauthenticated is overwritten to ErrorLevel with customClientCodeToLevel override, which probably didn't work",
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this read customCodeToLevel (as it did before) since this is a server interceptor test?

@mwitkow mwitkow merged commit aa1a867 into master May 2, 2017
@mwitkow mwitkow deleted the feature/client_logging branch March 31, 2018 11:47
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.

None yet

3 participants