From 3cb3f0de567e5bd1a5281187961561b9eb724ab8 Mon Sep 17 00:00:00 2001 From: nolouch Date: Wed, 30 Jan 2019 17:36:40 +0800 Subject: [PATCH 1/5] add global function Signed-off-by: nolouch --- global.go | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) create mode 100644 global.go diff --git a/global.go b/global.go new file mode 100644 index 0000000..a033906 --- /dev/null +++ b/global.go @@ -0,0 +1,51 @@ +// Copyright 2019 PingCAP, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// See the License for the specific language governing permissions and +// limitations under the License. + +package log + +import "go.uber.org/zap" + +// 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 Info(msg string, fields ...zap.Field) { + L().Info(msg, 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 Warn(msg string, fields ...zap.Field) { + L().Warn(msg, 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 Error(msg string, fields ...zap.Field) { + L().Error(msg, 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 Panic(msg string, fields ...zap.Field) { + L().Panic(msg, 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 Fatal(msg string, fields ...zap.Field) { + L().Fatal(msg, fields...) +} From b3184ef911abb3d9a5e2efa196341e53d03717d2 Mon Sep 17 00:00:00 2001 From: nolouch Date: Thu, 31 Jan 2019 12:27:21 +0800 Subject: [PATCH 2/5] address comments Signed-off-by: nolouch --- config.go | 7 +++++++ global.go | 12 +++++++++++- log.go | 37 ++++++++++++++++++++++++++----------- 3 files changed, 44 insertions(+), 12 deletions(-) diff --git a/config.go b/config.go index fe33ea8..d948c71 100644 --- a/config.go +++ b/config.go @@ -66,6 +66,13 @@ type Config struct { Sampling *zap.SamplingConfig `toml:"sampling" json:"sampling"` } +// ZapRecord records some information about zap. +type ZapRecord struct { + Core zapcore.Core + Syncer zapcore.WriteSyncer + Level zap.AtomicLevel +} + func newZapTextEncoder(cfg *Config) zapcore.Encoder { cc := zapcore.EncoderConfig{ // Keys can be anything except the empty string. diff --git a/global.go b/global.go index a033906..f109324 100644 --- a/global.go +++ b/global.go @@ -13,7 +13,10 @@ package log -import "go.uber.org/zap" +import ( + "go.uber.org/zap" + "go.uber.org/zap/zapcore" +) // 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. @@ -49,3 +52,10 @@ func Panic(msg string, fields ...zap.Field) { func Fatal(msg string, fields ...zap.Field) { L().Fatal(msg, fields...) } + +// SetLevel alters the logging level. +func SetLevel(l zapcore.Level) { + _globalMu.Lock() + defer _globalMu.Unlock() + _globalR.Level.SetLevel(l) +} diff --git a/log.go b/log.go index 38d5eba..acf0984 100644 --- a/log.go +++ b/log.go @@ -24,7 +24,7 @@ import ( ) // InitLogger initializes a zap logger. -func InitLogger(cfg *Config) (*zap.Logger, zapcore.WriteSyncer, error) { +func InitLogger(cfg *Config) (*zap.Logger, *ZapRecord, error) { var output zapcore.WriteSyncer if len(cfg.File.Filename) > 0 { lg, err := initFileLog(&cfg.File) @@ -47,8 +47,12 @@ func InitLogger(cfg *Config) (*zap.Logger, zapcore.WriteSyncer, error) { } core := zapcore.NewCore(newZapTextEncoder(cfg), output, level) lg := zap.New(core, cfg.buildOptions(output)...) - - return lg, output, nil + r := &ZapRecord{ + Core: core, + Syncer: output, + Level: level, + } + return lg, r, nil } // initFileLog initializes file based logging options. @@ -72,16 +76,16 @@ func initFileLog(cfg *FileLogConfig) (*lumberjack.Logger, error) { }, nil } -func newStdLogger() *zap.Logger { +func newStdLogger() (*zap.Logger, *ZapRecord) { conf := &Config{Level: "info", File: FileLogConfig{}} - lg, _, _ := InitLogger(conf) - return lg + lg, r, _ := InitLogger(conf) + return lg, r } var ( - _globalMu sync.RWMutex - _globalL = newStdLogger() - _globalS = _globalL.Sugar() + _globalMu sync.RWMutex + _globalL, _globalR = newStdLogger() + _globalS = _globalL.Sugar() ) // L returns the global Logger, which can be reconfigured with ReplaceGlobals. @@ -104,11 +108,22 @@ func S() *zap.SugaredLogger { // ReplaceGlobals replaces the global Logger and SugaredLogger, and returns a // function to restore the original values. It's safe for concurrent use. -func ReplaceGlobals(logger *zap.Logger) func() { +func ReplaceGlobals(logger *zap.Logger, record *ZapRecord) func() { _globalMu.Lock() prev := _globalL + prer := _globalR _globalL = logger _globalS = logger.Sugar() + _globalR = record _globalMu.Unlock() - return func() { ReplaceGlobals(prev) } + return func() { ReplaceGlobals(prev, prer) } +} + +// Sync flushes any buffered log entries. +func Sync() error { + err := L().Sync() + if err != nil { + return err + } + return S().Sync() } From 156c4044ecb24c2b1f763b94e235f97f5ab5317f Mon Sep 17 00:00:00 2001 From: nolouch Date: Mon, 11 Feb 2019 10:35:21 +0800 Subject: [PATCH 3/5] address comments --- global.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/global.go b/global.go index f109324..3debaab 100644 --- a/global.go +++ b/global.go @@ -59,3 +59,10 @@ func SetLevel(l zapcore.Level) { defer _globalMu.Unlock() _globalR.Level.SetLevel(l) } + +// GetLevel gets the logging level. +func GetLevel() zapcore.Level { + _globalMu.RLock() + defer _globalMu.RUnlock() + return _globalR.Level.Level() +} From 6e5c1a1a9e89ddf77d8c3ef58241aa5afcde4d92 Mon Sep 17 00:00:00 2001 From: nolouch Date: Mon, 11 Feb 2019 11:32:12 +0800 Subject: [PATCH 4/5] remove mutex --- config.go | 4 ++-- global.go | 8 ++------ log.go | 33 ++++++++++----------------------- 3 files changed, 14 insertions(+), 31 deletions(-) diff --git a/config.go b/config.go index d948c71..b568973 100644 --- a/config.go +++ b/config.go @@ -66,8 +66,8 @@ type Config struct { Sampling *zap.SamplingConfig `toml:"sampling" json:"sampling"` } -// ZapRecord records some information about zap. -type ZapRecord struct { +// ZapProperties records some information about zap. +type ZapProperties struct { Core zapcore.Core Syncer zapcore.WriteSyncer Level zap.AtomicLevel diff --git a/global.go b/global.go index 3debaab..bbaa647 100644 --- a/global.go +++ b/global.go @@ -55,14 +55,10 @@ func Fatal(msg string, fields ...zap.Field) { // SetLevel alters the logging level. func SetLevel(l zapcore.Level) { - _globalMu.Lock() - defer _globalMu.Unlock() - _globalR.Level.SetLevel(l) + _globalP.Level.SetLevel(l) } // GetLevel gets the logging level. func GetLevel() zapcore.Level { - _globalMu.RLock() - defer _globalMu.RUnlock() - return _globalR.Level.Level() + return _globalP.Level.Level() } diff --git a/log.go b/log.go index acf0984..5ded710 100644 --- a/log.go +++ b/log.go @@ -16,7 +16,6 @@ package log import ( "errors" "os" - "sync" "go.uber.org/zap" "go.uber.org/zap/zapcore" @@ -24,7 +23,7 @@ import ( ) // InitLogger initializes a zap logger. -func InitLogger(cfg *Config) (*zap.Logger, *ZapRecord, error) { +func InitLogger(cfg *Config) (*zap.Logger, *ZapProperties, error) { var output zapcore.WriteSyncer if len(cfg.File.Filename) > 0 { lg, err := initFileLog(&cfg.File) @@ -47,7 +46,7 @@ func InitLogger(cfg *Config) (*zap.Logger, *ZapRecord, error) { } core := zapcore.NewCore(newZapTextEncoder(cfg), output, level) lg := zap.New(core, cfg.buildOptions(output)...) - r := &ZapRecord{ + r := &ZapProperties{ Core: core, Syncer: output, Level: level, @@ -76,47 +75,35 @@ func initFileLog(cfg *FileLogConfig) (*lumberjack.Logger, error) { }, nil } -func newStdLogger() (*zap.Logger, *ZapRecord) { +func newStdLogger() (*zap.Logger, *ZapProperties) { conf := &Config{Level: "info", File: FileLogConfig{}} lg, r, _ := InitLogger(conf) return lg, r } var ( - _globalMu sync.RWMutex - _globalL, _globalR = newStdLogger() + _globalL, _globalP = newStdLogger() _globalS = _globalL.Sugar() ) // L returns the global Logger, which can be reconfigured with ReplaceGlobals. // It's safe for concurrent use. func L() *zap.Logger { - _globalMu.RLock() - l := _globalL - _globalMu.RUnlock() - return l + return _globalL } // S returns the global SugaredLogger, which can be reconfigured with // ReplaceGlobals. It's safe for concurrent use. func S() *zap.SugaredLogger { - _globalMu.RLock() - s := _globalS - _globalMu.RUnlock() - return s + return _globalS } -// ReplaceGlobals replaces the global Logger and SugaredLogger, and returns a -// function to restore the original values. It's safe for concurrent use. -func ReplaceGlobals(logger *zap.Logger, record *ZapRecord) func() { - _globalMu.Lock() - prev := _globalL - prer := _globalR +// ReplaceGlobals replaces the global Logger and SugaredLogger. +// It's unsafe for concurrent use. +func ReplaceGlobals(logger *zap.Logger, props *ZapProperties) { _globalL = logger _globalS = logger.Sugar() - _globalR = record - _globalMu.Unlock() - return func() { ReplaceGlobals(prev, prer) } + _globalP = props } // Sync flushes any buffered log entries. From 93552f0fb9a5888b600e1a89ca6dcd0e26b31855 Mon Sep 17 00:00:00 2001 From: nolouch Date: Wed, 13 Feb 2019 14:11:57 +0800 Subject: [PATCH 5/5] address comments Signed-off-by: nolouch --- .gitignore | 13 +------------ global.go | 16 +++++++++++----- log.go | 5 +++-- log_test.go | 33 +++++++++++++++++++++++++++++++++ zap_log_test.go | 7 +++++++ 5 files changed, 55 insertions(+), 19 deletions(-) create mode 100644 log_test.go diff --git a/.gitignore b/.gitignore index f1c181e..22d0d82 100644 --- a/.gitignore +++ b/.gitignore @@ -1,12 +1 @@ -# Binaries for programs and plugins -*.exe -*.exe~ -*.dll -*.so -*.dylib - -# Test binary, build with `go test -c` -*.test - -# Output of the go coverage tool, specifically when used with LiteIDE -*.out +vendor diff --git a/global.go b/global.go index bbaa647..5d00254 100644 --- a/global.go +++ b/global.go @@ -18,22 +18,28 @@ import ( "go.uber.org/zap/zapcore" ) +// 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 Debug(msg string, fields ...zap.Field) { + L().WithOptions(zap.AddCallerSkip(1)).Debug(msg, 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 Info(msg string, fields ...zap.Field) { - L().Info(msg, fields...) + L().WithOptions(zap.AddCallerSkip(1)).Info(msg, 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 Warn(msg string, fields ...zap.Field) { - L().Warn(msg, fields...) + L().WithOptions(zap.AddCallerSkip(1)).Warn(msg, 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 Error(msg string, fields ...zap.Field) { - L().Error(msg, fields...) + L().WithOptions(zap.AddCallerSkip(1)).Error(msg, fields...) } // Panic logs a message at PanicLevel. The message includes any fields passed @@ -41,7 +47,7 @@ func Error(msg string, fields ...zap.Field) { // // The logger then panics, even if logging at PanicLevel is disabled. func Panic(msg string, fields ...zap.Field) { - L().Panic(msg, fields...) + L().WithOptions(zap.AddCallerSkip(1)).Panic(msg, fields...) } // Fatal logs a message at FatalLevel. The message includes any fields passed @@ -50,7 +56,7 @@ func Panic(msg string, fields ...zap.Field) { // The logger then calls os.Exit(1), even if logging at FatalLevel is // disabled. func Fatal(msg string, fields ...zap.Field) { - L().Fatal(msg, fields...) + L().WithOptions(zap.AddCallerSkip(1)).Fatal(msg, fields...) } // SetLevel alters the logging level. diff --git a/log.go b/log.go index 5ded710..fba88b2 100644 --- a/log.go +++ b/log.go @@ -23,7 +23,7 @@ import ( ) // InitLogger initializes a zap logger. -func InitLogger(cfg *Config) (*zap.Logger, *ZapProperties, error) { +func InitLogger(cfg *Config, opts ...zap.Option) (*zap.Logger, *ZapProperties, error) { var output zapcore.WriteSyncer if len(cfg.File.Filename) > 0 { lg, err := initFileLog(&cfg.File) @@ -45,7 +45,8 @@ func InitLogger(cfg *Config) (*zap.Logger, *ZapProperties, error) { return nil, nil, err } core := zapcore.NewCore(newZapTextEncoder(cfg), output, level) - lg := zap.New(core, cfg.buildOptions(output)...) + opts = append(opts, cfg.buildOptions(output)...) + lg := zap.New(core, opts...) r := &ZapProperties{ Core: core, Syncer: output, diff --git a/log_test.go b/log_test.go new file mode 100644 index 0000000..f2ec42c --- /dev/null +++ b/log_test.go @@ -0,0 +1,33 @@ +// Copyright 2019 PingCAP, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// See the License for the specific language governing permissions and +// limitations under the License. + +package log + +import ( + . "github.com/pingcap/check" +) + +var _ = Suite(&testLogSuite{}) + +type testLogSuite struct{} + +func (t *testLogSuite) TestExport(c *C) { + conf := &Config{Level: "debug", File: FileLogConfig{}, DisableTimestamp: true} + lg := newZapTestLogger(conf, c) + ReplaceGlobals(lg.Logger, nil) + Info("Testing") + Debug("Testing") + Warn("Testing") + Error("Testing") + lg.AssertContains("log_test.go:") +} diff --git a/zap_log_test.go b/zap_log_test.go index ac7b0d2..bf37eb8 100644 --- a/zap_log_test.go +++ b/zap_log_test.go @@ -20,6 +20,7 @@ import ( "math" "net" "os" + "strings" "testing" "time" "unsafe" @@ -70,6 +71,12 @@ func (v *verifyLogger) AssertMessage(msg ...string) { } } +func (v *verifyLogger) AssertContains(substr string) { + for _, m := range v.w.messages { + v.w.c.Assert(strings.Contains(m, substr), IsTrue) + } +} + func newZapTestLogger(cfg *Config, c *C) verifyLogger { writer := newTestingWriter(c) opt := cfg.buildOptions(writer)