Skip to content

Commit

Permalink
Make Logger a concrete type instead of an interface (#270)
Browse files Browse the repository at this point in the history
`zapcore.Facility` is the extension point we're offering to third-party
developers, so there's no need to make the logger an interface. In fact, an
interface will be *more* constraining, since we won't be able to add any methods
after version 1.0.

Along the way, this fixes our woes with `AddCaller` not working properly (#40).
  • Loading branch information
akshayjshah committed Feb 3, 2017
1 parent 3b92221 commit d3d9d5e
Show file tree
Hide file tree
Showing 11 changed files with 208 additions and 162 deletions.
4 changes: 2 additions & 2 deletions benchmarks/zap_sugar_bench_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,11 @@ func fakeSugarFormatArgs() (string, []interface{}) {
}

func newSugarLogger(lvl zapcore.Level, options ...zap.Option) *zap.SugaredLogger {
return zap.Sugar(zap.New(zapcore.WriterFacility(
return zap.New(zapcore.WriterFacility(
benchEncoder(),
&testutils.Discarder{},
lvl,
), options...))
), options...).Sugar()
}

func BenchmarkZapSugarDisabledLevelsWithoutFields(b *testing.B) {
Expand Down
4 changes: 2 additions & 2 deletions common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,15 @@ func opts(opts ...Option) []Option {

// Here specifically to introduce an easily-identifiable filename for testing
// stacktraces and caller skips.
func withLogger(t testing.TB, e zapcore.LevelEnabler, opts []Option, f func(Logger, *observer.ObservedLogs)) {
func withLogger(t testing.TB, e zapcore.LevelEnabler, opts []Option, f func(*Logger, *observer.ObservedLogs)) {
var logs observer.ObservedLogs
fac := observer.New(e, logs.Add, true)
log := New(fac, opts...)
f(log, &logs)
}

func withSugar(t testing.TB, e zapcore.LevelEnabler, opts []Option, f func(*SugaredLogger, *observer.ObservedLogs)) {
withLogger(t, e, opts, func(logger Logger, logs *observer.ObservedLogs) { f(Sugar(logger), logs) })
withLogger(t, e, opts, func(logger *Logger, logs *observer.ObservedLogs) { f(logger.Sugar(), logs) })
}

func runConcurrently(goroutines, iterations int, wg *sync.WaitGroup, f func()) {
Expand Down
2 changes: 1 addition & 1 deletion http_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ import (
"github.com/stretchr/testify/require"
)

func newHandler() (AtomicLevel, Logger) {
func newHandler() (AtomicLevel, *Logger) {
lvl := DynamicLevel()
// XXX should be a discard facility
fac := observer.New(lvl, func(observer.LoggedEntry) error { return nil }, false)
Expand Down
186 changes: 98 additions & 88 deletions logger.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,37 +44,11 @@ func defaultEncoderConfig() zapcore.EncoderConfig {
}
}

// A Logger enables leveled, structured logging. All methods are safe for
// A Logger provides fast, leveled, structured logging. All methods are safe for
// concurrent use.
type Logger interface {
// Create a child logger, and optionally add some context to that logger.
With(...zapcore.Field) Logger
// Add a sub-scope to the logger's name.
Named(string) Logger

// Check returns a CheckedEntry if logging a message at the specified level
// is enabled. It's a completely optional optimization; in high-performance
// applications, Check can help avoid allocating a slice to hold fields.
Check(zapcore.Level, string) *zapcore.CheckedEntry

// Log a message at the given level. Messages include any context that's
// accumulated on the logger, as well as any fields added at the log site.
//
// Calling Panic should panic and calling Fatal should terminate the process,
// even if logging at those levels is disabled.
Debug(string, ...zapcore.Field)
Info(string, ...zapcore.Field)
Warn(string, ...zapcore.Field)
Error(string, ...zapcore.Field)
DPanic(string, ...zapcore.Field)
Panic(string, ...zapcore.Field)
Fatal(string, ...zapcore.Field)

// Facility returns the destination that log entries are written to.
Facility() zapcore.Facility
}

type logger struct {
//
// If you'd prefer a slower but less verbose logger, see the SugaredLogger.
type Logger struct {
fac zapcore.Facility

development bool
Expand All @@ -90,15 +64,15 @@ type logger struct {
// New returns a new logger with sensible defaults: logging at InfoLevel,
// development mode off, errors written to standard error, and logs JSON
// encoded to standard output.
func New(fac zapcore.Facility, options ...Option) Logger {
func New(fac zapcore.Facility, options ...Option) *Logger {
if fac == nil {
fac = zapcore.WriterFacility(
zapcore.NewJSONEncoder(defaultEncoderConfig()),
os.Stdout,
InfoLevel,
)
}
log := &logger{
log := &Logger{
fac: fac,
errorOutput: zapcore.Lock(os.Stderr),
addStack: LevelEnablerFunc(func(_ zapcore.Level) bool { return false }),
Expand All @@ -109,7 +83,16 @@ func New(fac zapcore.Facility, options ...Option) Logger {
return log
}

func (log *logger) Named(s string) Logger {
// Sugar converts a Logger to a SugaredLogger.
func (log *Logger) Sugar() *SugaredLogger {
core := log.clone()
core.callerSkip += 2
return &SugaredLogger{core}
}

// Named adds a new path segment to the logger's name. Segments are joined by
// periods.
func (log *Logger) Named(s string) *Logger {
if s == "" {
return log
}
Expand All @@ -122,7 +105,9 @@ func (log *logger) Named(s string) Logger {
return l
}

func (log *logger) With(fields ...zapcore.Field) Logger {
// With creates a child logger and adds structured context to it. Fields added
// to the child don't affect the parent, and vice versa.
func (log *Logger) With(fields ...zapcore.Field) *Logger {
if len(fields) == 0 {
return log
}
Expand All @@ -131,11 +116,88 @@ func (log *logger) With(fields ...zapcore.Field) Logger {
return l
}

func (log *logger) Check(lvl zapcore.Level, msg string) *zapcore.CheckedEntry {
// Check returns a CheckedEntry if logging a message at the specified level
// is enabled. It's a completely optional optimization; in high-performance
// applications, Check can help avoid allocating a slice to hold fields.
func (log *Logger) Check(lvl zapcore.Level, msg string) *zapcore.CheckedEntry {
return log.check(lvl, msg)
}

func (log *logger) check(lvl zapcore.Level, msg string) *zapcore.CheckedEntry {
// Debug logs a message at DebugLevel. The message includes any fields passed
// at the log site, as well as any fields accumulated on the logger.
func (log *Logger) Debug(msg string, fields ...zapcore.Field) {
if ce := log.check(DebugLevel, msg); ce != nil {
ce.Write(fields...)
}
}

// Info logs a message at InfoLevel. The message includes any fields passed
// at the log site, as well as any fields accumulated on the logger.
func (log *Logger) Info(msg string, fields ...zapcore.Field) {
if ce := log.check(InfoLevel, msg); ce != nil {
ce.Write(fields...)
}
}

// Warn logs a message at WarnLevel. The message includes any fields passed
// at the log site, as well as any fields accumulated on the logger.
func (log *Logger) Warn(msg string, fields ...zapcore.Field) {
if ce := log.check(WarnLevel, msg); ce != nil {
ce.Write(fields...)
}
}

// Error logs a message at ErrorLevel. The message includes any fields passed
// at the log site, as well as any fields accumulated on the logger.
func (log *Logger) Error(msg string, fields ...zapcore.Field) {
if ce := log.check(ErrorLevel, msg); ce != nil {
ce.Write(fields...)
}
}

// DPanic logs a message at DPanicLevel. The message includes any fields passed
// at the log site, as well as any fields accumulated on the logger.
//
// If the logger is in development mode, it then panics (DPanic means
// "development panic"). This is useful for catching errors that are
// recoverable, but shouldn't ever happen.
func (log *Logger) DPanic(msg string, fields ...zapcore.Field) {
if ce := log.check(DPanicLevel, msg); ce != nil {
ce.Write(fields...)
}
}

// Panic logs a message at PanicLevel. The message includes any fields passed
// at the log site, as well as any fields accumulated on the logger.
//
// The logger then panics, even if logging at PanicLevel is disabled.
func (log *Logger) Panic(msg string, fields ...zapcore.Field) {
if ce := log.check(PanicLevel, msg); ce != nil {
ce.Write(fields...)
}
}

// Fatal logs a message at FatalLevel. The message includes any fields passed
// at the log site, as well as any fields accumulated on the logger.
//
// The logger then calls os.Exit(1), even if logging at FatalLevel is disabled.
func (log *Logger) Fatal(msg string, fields ...zapcore.Field) {
if ce := log.check(FatalLevel, msg); ce != nil {
ce.Write(fields...)
}
}

// Facility returns the destination that logs entries are written to.
func (log *Logger) Facility() zapcore.Facility {
return log.fac
}

func (log *Logger) clone() *Logger {
copy := *log
return &copy
}

func (log *Logger) check(lvl zapcore.Level, msg string) *zapcore.CheckedEntry {
// check must always be called directly by a method in the Logger interface
// (e.g., Check, Info, Fatal).
const callerSkipOffset = 2
Expand Down Expand Up @@ -188,55 +250,3 @@ func (log *logger) check(lvl zapcore.Level, msg string) *zapcore.CheckedEntry {

return ce
}

func (log *logger) Debug(msg string, fields ...zapcore.Field) {
if ce := log.check(DebugLevel, msg); ce != nil {
ce.Write(fields...)
}
}

func (log *logger) Info(msg string, fields ...zapcore.Field) {
if ce := log.check(InfoLevel, msg); ce != nil {
ce.Write(fields...)
}
}

func (log *logger) Warn(msg string, fields ...zapcore.Field) {
if ce := log.check(WarnLevel, msg); ce != nil {
ce.Write(fields...)
}
}

func (log *logger) Error(msg string, fields ...zapcore.Field) {
if ce := log.check(ErrorLevel, msg); ce != nil {
ce.Write(fields...)
}
}

func (log *logger) DPanic(msg string, fields ...zapcore.Field) {
if ce := log.check(DPanicLevel, msg); ce != nil {
ce.Write(fields...)
}
}

func (log *logger) Panic(msg string, fields ...zapcore.Field) {
if ce := log.check(PanicLevel, msg); ce != nil {
ce.Write(fields...)
}
}

func (log *logger) Fatal(msg string, fields ...zapcore.Field) {
if ce := log.check(FatalLevel, msg); ce != nil {
ce.Write(fields...)
}
}

// Facility returns the destination that logs entries are written to.
func (log *logger) Facility() zapcore.Facility {
return log.fac
}

func (log *logger) clone() *logger {
copy := *log
return &copy
}
30 changes: 15 additions & 15 deletions logger_bench_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ var _jane = &user{
CreatedAt: time.Date(1980, 1, 1, 12, 0, 0, 0, time.UTC),
}

func withBenchedLogger(b *testing.B, f func(Logger)) {
func withBenchedLogger(b *testing.B, f func(*Logger)) {
logger := New(
zapcore.WriterFacility(zapcore.NewJSONEncoder(defaultEncoderConfig()),
&testutils.Discarder{},
Expand All @@ -63,81 +63,81 @@ func withBenchedLogger(b *testing.B, f func(Logger)) {
}

func BenchmarkNoContext(b *testing.B) {
withBenchedLogger(b, func(log Logger) {
withBenchedLogger(b, func(log *Logger) {
log.Info("No context.")
})
}

func BenchmarkBoolField(b *testing.B) {
withBenchedLogger(b, func(log Logger) {
withBenchedLogger(b, func(log *Logger) {
log.Info("Boolean.", Bool("foo", true))
})
}

func BenchmarkFloat64Field(b *testing.B) {
withBenchedLogger(b, func(log Logger) {
withBenchedLogger(b, func(log *Logger) {
log.Info("Floating point.", Float64("foo", 3.14))
})
}

func BenchmarkIntField(b *testing.B) {
withBenchedLogger(b, func(log Logger) {
withBenchedLogger(b, func(log *Logger) {
log.Info("Integer.", Int("foo", 42))
})
}

func BenchmarkInt64Field(b *testing.B) {
withBenchedLogger(b, func(log Logger) {
withBenchedLogger(b, func(log *Logger) {
log.Info("64-bit integer.", Int64("foo", 42))
})
}

func BenchmarkStringField(b *testing.B) {
withBenchedLogger(b, func(log Logger) {
withBenchedLogger(b, func(log *Logger) {
log.Info("Strings.", String("foo", "bar"))
})
}

func BenchmarkStringerField(b *testing.B) {
withBenchedLogger(b, func(log Logger) {
withBenchedLogger(b, func(log *Logger) {
log.Info("Level.", Stringer("foo", InfoLevel))
})
}

func BenchmarkTimeField(b *testing.B) {
t := time.Unix(0, 0)
withBenchedLogger(b, func(log Logger) {
withBenchedLogger(b, func(log *Logger) {
log.Info("Time.", Time("foo", t))
})
}

func BenchmarkDurationField(b *testing.B) {
withBenchedLogger(b, func(log Logger) {
withBenchedLogger(b, func(log *Logger) {
log.Info("Duration", Duration("foo", time.Second))
})
}

func BenchmarkErrorField(b *testing.B) {
err := errors.New("egad")
withBenchedLogger(b, func(log Logger) {
withBenchedLogger(b, func(log *Logger) {
log.Info("Error.", Error(err))
})
}

func BenchmarkStackField(b *testing.B) {
withBenchedLogger(b, func(log Logger) {
withBenchedLogger(b, func(log *Logger) {
log.Info("Error.", Stack())
})
}

func BenchmarkObjectField(b *testing.B) {
withBenchedLogger(b, func(log Logger) {
withBenchedLogger(b, func(log *Logger) {
log.Info("Arbitrary ObjectMarshaler.", Object("user", _jane))
})
}

func BenchmarkReflectField(b *testing.B) {
withBenchedLogger(b, func(log Logger) {
withBenchedLogger(b, func(log *Logger) {
log.Info("Reflection-based serialization.", Reflect("user", _jane))
})
}
Expand All @@ -160,7 +160,7 @@ func BenchmarkAddCallerHook(b *testing.B) {
}

func Benchmark10Fields(b *testing.B) {
withBenchedLogger(b, func(log Logger) {
withBenchedLogger(b, func(log *Logger) {
log.Info("Ten fields, passed at the log site.",
Int("one", 1),
Int("two", 2),
Expand Down
Loading

0 comments on commit d3d9d5e

Please sign in to comment.