From cbe4c486bbfa4c6d973037c1f989a2f6c34e0e31 Mon Sep 17 00:00:00 2001 From: Sung Yoon Whang Date: Mon, 11 Jul 2022 18:47:34 -0700 Subject: [PATCH] Refactor to reuse Log method This refactors the implementation of the Log method to reuse it in the other level variants of the method (i.e. `Debug`, `Info`, etc.). To do this, a skip argument was added to `check` to allow its callers to specify how many frames to skip in the log, and a private `log` method was added which calls this `check` method with the appropriate frame skip count. --- logger.go | 53 ++++++++++++++++++++++------------------------------- sugar.go | 4 ++-- 2 files changed, 24 insertions(+), 33 deletions(-) diff --git a/logger.go b/logger.go index 33c9f2c8c..9664cfda4 100644 --- a/logger.go +++ b/logger.go @@ -187,47 +187,46 @@ func (log *Logger) With(fields ...Field) *Logger { // 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) + return log.check(lvl, msg, 0) +} + +func (log *Logger) log(lvl zapcore.Level, msg string, skip int, fields ...Field) { + if ce := log.check(lvl, msg, skip); ce != nil { + ce.Write(fields...) + } } // Log logs a message at the specified level. The message includes any fields // passed at the log site, as well as any fields accumulated on the logger. func (log *Logger) Log(lvl zapcore.Level, msg string, fields ...Field) { - if ce := log.check(lvl, msg); ce != nil { - ce.Write(fields...) - } + // Skip 2 frames in Log and all Debug/Info/... methods + // below for the public method as well as the private log + // method. + log.log(lvl, msg, 2, fields...) } // 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 ...Field) { - if ce := log.check(DebugLevel, msg); ce != nil { - ce.Write(fields...) - } + log.log(DebugLevel, msg, 2, 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 ...Field) { - if ce := log.check(InfoLevel, msg); ce != nil { - ce.Write(fields...) - } + log.log(InfoLevel, msg, 2, 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 ...Field) { - if ce := log.check(WarnLevel, msg); ce != nil { - ce.Write(fields...) - } + log.log(WarnLevel, msg, 2, 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 ...Field) { - if ce := log.check(ErrorLevel, msg); ce != nil { - ce.Write(fields...) - } + log.log(ErrorLevel, msg, 2, fields...) } // DPanic logs a message at DPanicLevel. The message includes any fields @@ -237,9 +236,7 @@ func (log *Logger) Error(msg string, fields ...Field) { // "development panic"). This is useful for catching errors that are // recoverable, but shouldn't ever happen. func (log *Logger) DPanic(msg string, fields ...Field) { - if ce := log.check(DPanicLevel, msg); ce != nil { - ce.Write(fields...) - } + log.log(DPanicLevel, msg, 2, fields...) } // Panic logs a message at PanicLevel. The message includes any fields passed @@ -247,9 +244,7 @@ func (log *Logger) DPanic(msg string, fields ...Field) { // // The logger then panics, even if logging at PanicLevel is disabled. func (log *Logger) Panic(msg string, fields ...Field) { - if ce := log.check(PanicLevel, msg); ce != nil { - ce.Write(fields...) - } + log.log(PanicLevel, msg, 2, fields...) } // Fatal logs a message at FatalLevel. The message includes any fields passed @@ -258,9 +253,7 @@ func (log *Logger) Panic(msg string, fields ...Field) { // The logger then calls os.Exit(1), even if logging at FatalLevel is // disabled. func (log *Logger) Fatal(msg string, fields ...Field) { - if ce := log.check(FatalLevel, msg); ce != nil { - ce.Write(fields...) - } + log.log(FatalLevel, msg, 2, fields...) } // Sync calls the underlying Core's Sync method, flushing any buffered log @@ -279,12 +272,10 @@ func (log *Logger) clone() *Logger { return © } -func (log *Logger) check(lvl zapcore.Level, msg string) *zapcore.CheckedEntry { - // Logger.check must always be called directly by a method in the - // Logger interface (e.g., Check, Info, Fatal). - // This skips Logger.check and the Info/Fatal/Check/etc. method that - // called it. - const callerSkipOffset = 2 +func (log *Logger) check(lvl zapcore.Level, msg string, skip int) *zapcore.CheckedEntry { + // This skips Logger.check method on top of any additional + // skips from the caller. + callerSkipOffset := skip + 1 // Check the level first to reduce the cost of disabled log calls. // Since Panic and higher may exit, we skip the optimization for those levels. diff --git a/sugar.go b/sugar.go index cb5c6dd97..f6d522ae8 100644 --- a/sugar.go +++ b/sugar.go @@ -279,7 +279,7 @@ func (s *SugaredLogger) log(lvl zapcore.Level, template string, fmtArgs []interf } msg := getMessage(template, fmtArgs) - if ce := s.base.Check(lvl, msg); ce != nil { + if ce := s.base.check(lvl, msg, 0); ce != nil { ce.Write(s.sweetenFields(context)...) } } @@ -291,7 +291,7 @@ func (s *SugaredLogger) logln(lvl zapcore.Level, template string, fmtArgs []inte } msg := getMessageln(fmtArgs) - if ce := s.base.Check(lvl, msg); ce != nil { + if ce := s.base.check(lvl, msg, 0); ce != nil { ce.Write(s.sweetenFields(context)...) } }