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.

Sign up for GitHub

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

Jump to bottom

Implementing grpclog.LoggerV2 compatible logger #538

Closed
wants to merge 3 commits into from
Closed

Implementing grpclog.LoggerV2 compatible logger #538

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0e55ec4
implementing grpclog.LoggerV2 compatible logger
Dec 19, 2017
c03c2b3
implementing grpclog.LoggerV2 compatible logger
frankgreco Dec 19, 2017
d78f25e
Merge branch 'grpclog-v2' of github.com:frankgreco/zap into grpclog-v2
frankgreco Sep 10, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
95 changes: 80 additions & 15 deletions zapgrpc/zapgrpc.go
View file
Open in desktop
Expand Up @@ -21,7 +21,10 @@
// Package zapgrpc provides a logger that is compatible with grpclog.
package zapgrpc // import "go.uber.org/zap/zapgrpc"

import "go.uber.org/zap"
import (
"go.uber.org/zap"
"go.uber.org/zap/zapcore"
)

// An Option overrides a Logger's default configuration.
type Option interface {
Expand All @@ -48,38 +51,95 @@ func WithDebug() Option {
// By default, Loggers print at zap's InfoLevel.
func NewLogger(l *zap.Logger, options ...Option) *Logger {
logger := &Logger{
log: l.Sugar(),
fatal: (*zap.SugaredLogger).Fatal,
fatalf: (*zap.SugaredLogger).Fatalf,
print: (*zap.SugaredLogger).Info,
printf: (*zap.SugaredLogger).Infof,
log: l.Sugar(),
info: (*zap.SugaredLogger).Info,
Copy link
Collaborator

Choose a reason for hiding this comment

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

is there a reason why we have to keep function pointers for each level, instead of just doing l.sugar.Info(..) etc?

Copy link
Author

Choose a reason for hiding this comment

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

@prashantv no reason. I was just trying to maintain the programming style that was previously present.

Copy link
Contributor

Choose a reason for hiding this comment

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

@prashantv this is for the WithDebug function, which changes the zap function used for the Print functions.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah, I see. In that case, aren't the only functions we need to do this for print and printf? The rest should just be normal function calls on the Logger.

Copy link
Contributor

Choose a reason for hiding this comment

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

That would work too, either/or, I was just being consistent, especially “back in the day” before this PR when there were only a few functions :-)

infof: (*zap.SugaredLogger).Infof,
warning: (*zap.SugaredLogger).Warn,
warningf: (*zap.SugaredLogger).Warnf,
err: (*zap.SugaredLogger).Error,
errf: (*zap.SugaredLogger).Errorf,
fatal: (*zap.SugaredLogger).Fatal,
fatalf: (*zap.SugaredLogger).Fatalf,
print: (*zap.SugaredLogger).Info,
printf: (*zap.SugaredLogger).Infof,
}
for _, option := range options {
option.apply(logger)
}
return logger
}

// Logger adapts zap's Logger to be compatible with grpclog.Logger.
// Logger adapts zap's Logger to be compatible with grpclog.Logger and grpclog.LoggerV2.
type Logger struct {
log *zap.SugaredLogger
fatal func(*zap.SugaredLogger, ...interface{})
fatalf func(*zap.SugaredLogger, string, ...interface{})
print func(*zap.SugaredLogger, ...interface{})
printf func(*zap.SugaredLogger, string, ...interface{})
log *zap.SugaredLogger
info func(*zap.SugaredLogger, ...interface{})
infof func(*zap.SugaredLogger, string, ...interface{})
warning func(*zap.SugaredLogger, ...interface{})
warningf func(*zap.SugaredLogger, string, ...interface{})
err func(*zap.SugaredLogger, ...interface{})
errf func(*zap.SugaredLogger, string, ...interface{})
fatal func(*zap.SugaredLogger, ...interface{})
fatalf func(*zap.SugaredLogger, string, ...interface{})
print func(*zap.SugaredLogger, ...interface{})
printf func(*zap.SugaredLogger, string, ...interface{})
}

// Fatal implements grpclog.Logger.
// Info implements grpclog.LoggerV2.
func (l *Logger) Info(args ...interface{}) {
l.info(l.log, args...)
}

// Infof implements grpclog.LoggerV2.
func (l *Logger) Infof(format string, args ...interface{}) {
l.infof(l.log, format, args...)
}

// Infoln implements grpclog.LoggerV2.
func (l *Logger) Infoln(args ...interface{}) {
l.info(l.log, args...)
}

// Warning implements grpclog.LoggerV2.
func (l *Logger) Warning(args ...interface{}) {
l.warning(l.log, args...)
}

// Warningf implements grpclog.LoggerV2.
func (l *Logger) Warningf(format string, args ...interface{}) {
l.warningf(l.log, format, args...)
}

// Warningln implements grpclog.LoggerV2.
func (l *Logger) Warningln(args ...interface{}) {
l.warning(l.log, args...)
}

// Error implements grpclog.LoggerV2.
func (l *Logger) Error(args ...interface{}) {
l.err(l.log, args...)
}

// Errorf implements grpclog.LoggerV2.
func (l *Logger) Errorf(format string, args ...interface{}) {
l.errf(l.log, format, args...)
}

// Errorln implements grpclog.LoggerV2.
func (l *Logger) Errorln(args ...interface{}) {
l.err(l.log, args...)
}

// Fatal implements grpclog.Logger and grpclog.LoggerV2.
func (l *Logger) Fatal(args ...interface{}) {
l.fatal(l.log, args...)
}

// Fatalf implements grpclog.Logger.
// Fatalf implements grpclog.Logger and grpclog.LoggerV2.
func (l *Logger) Fatalf(format string, args ...interface{}) {
l.fatalf(l.log, format, args...)
}

// Fatalln implements grpclog.Logger.
// Fatalln implements grpclog.Logger and grpclog.LoggerV2.
func (l *Logger) Fatalln(args ...interface{}) {
l.fatal(l.log, args...)
}
Expand All @@ -98,3 +158,8 @@ func (l *Logger) Printf(format string, args ...interface{}) {
func (l *Logger) Println(args ...interface{}) {
l.print(l.log, args...)
}

// V implements grpclog.LoggerV2.
func (l *Logger) V(lvl int) bool {
return l.log.Desugar().Core().Enabled(zapcore.Level(lvl))
}
54 changes: 54 additions & 0 deletions zapgrpc/zapgrpc_test.go
View file
Open in desktop
Expand Up @@ -35,7 +35,13 @@ func TestLoggerInfoExpected(t *testing.T) {
"hello",
"world",
"foo",
"hello",
"world",
"foo",
}, func(logger *Logger) {
logger.Info("hello")
logger.Infof("world")
logger.Infoln("foo")
logger.Print("hello")
logger.Printf("world")
logger.Println("foo")
Expand All @@ -62,6 +68,30 @@ func TestLoggerDebugSuppressed(t *testing.T) {
})
}

func TestLoggerWarnExpected(t *testing.T) {
checkMessages(t, zapcore.DebugLevel, nil, zapcore.WarnLevel, []string{
"hello",
"world",
"foo",
}, func(logger *Logger) {
logger.Warning("hello")
logger.Warningf("world")
logger.Warningln("foo")
})
}

func TestLoggerErrorExpected(t *testing.T) {
checkMessages(t, zapcore.DebugLevel, nil, zapcore.ErrorLevel, []string{
"hello",
"world",
"foo",
}, func(logger *Logger) {
logger.Error("hello")
logger.Errorf("world")
logger.Errorln("foo")
})
}

func TestLoggerFatalExpected(t *testing.T) {
checkMessages(t, zapcore.DebugLevel, nil, zapcore.FatalLevel, []string{
"hello",
Expand All @@ -74,6 +104,30 @@ func TestLoggerFatalExpected(t *testing.T) {
})
}

func TestLoggerTrueExpected(t *testing.T) {
checkLevel(t, zapcore.FatalLevel, true, func(logger *Logger) bool {
return logger.V(6)
})
}

func TestLoggerFalseExpected(t *testing.T) {
checkLevel(t, zapcore.FatalLevel, false, func(logger *Logger) bool {
return logger.V(0)
})
}

func checkLevel(
t testing.TB,
enab zapcore.LevelEnabler,
expectedBool bool,
f func(*Logger) bool,
) {
withLogger(enab, nil, func(logger *Logger, observedLogs *observer.ObservedLogs) {
actualBool := f(logger)
require.Equal(t, expectedBool, actualBool)
})
}

func checkMessages(
t testing.TB,
enab zapcore.LevelEnabler,
Expand Down