diff --git a/global.go b/global.go index cef613347..2b8a10c3e 100644 --- a/global.go +++ b/global.go @@ -24,31 +24,47 @@ import ( "bytes" "log" "os" + "sync" ) var ( - // L is a global Logger. It defaults to a no-op implementation but can be - // replaced using ReplaceGlobals. - // - // Both L and S are unsynchronized, so replacing them while they're in - // use isn't safe. - L = NewNop() - // S is a global SugaredLogger, similar to L. It also defaults to a no-op - // implementation. - S = L.Sugar() + _globalMu sync.RWMutex + _globalL = NewNop() + _globalS = _globalL.Sugar() ) -// ReplaceGlobals replaces the global Logger L and the global SugaredLogger S, -// and returns a function to restore the original values. +// L returns the global Logger, which can be reconfigured with ReplaceGlobals. // -// Note that replacing the global loggers isn't safe while they're being used; -// in practice, this means that only the owner of the application's main -// function should use this method. +// It's safe for concurrent use. +func L() *Logger { + _globalMu.RLock() + l := _globalL + _globalMu.RUnlock() + return l +} + +// S returns the global SugaredLogger, which can be reconfigured with +// ReplaceGlobals. +// +// It's safe for concurrent use. +func S() *SugaredLogger { + _globalMu.RLock() + s := _globalS + _globalMu.RUnlock() + return s +} + +// ReplaceGlobals replaces the global Logger and the SugaredLogger, and returns +// a function to restore the original values. +// +// It's safe for concurrent use. func ReplaceGlobals(logger *Logger) func() { - prev := *L - L = logger - S = logger.Sugar() - return func() { ReplaceGlobals(&prev) } + _globalMu.Lock() + prev := _globalL + _globalL = logger + _globalS = logger.Sugar() + _globalMu.Unlock() + return func() { ReplaceGlobals(prev) } } // RedirectStdLog redirects output from the standard library's "log" package to diff --git a/global_test.go b/global_test.go index d6e0713bd..f14f17533 100644 --- a/global_test.go +++ b/global_test.go @@ -22,28 +22,32 @@ package zap import ( "log" + "sync" "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" + "time" "go.uber.org/zap/internal/observer" + "go.uber.org/zap/testutils" "go.uber.org/zap/zapcore" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.uber.org/atomic" ) func TestReplaceGlobals(t *testing.T) { - initialL := *L - initialS := *S + initialL := *L() + initialS := *S() withLogger(t, DebugLevel, nil, func(l *Logger, logs *observer.ObservedLogs) { - L.Info("no-op") - S.Info("no-op") + L().Info("no-op") + S().Info("no-op") assert.Equal(t, 0, logs.Len(), "Expected initial logs to go to default no-op global.") defer ReplaceGlobals(l)() - L.Info("captured") - S.Info("captured") + L().Info("captured") + S().Info("captured") expected := observer.LoggedEntry{ Entry: zapcore.Entry{Message: "captured"}, Context: []zapcore.Field{}, @@ -56,8 +60,36 @@ func TestReplaceGlobals(t *testing.T) { ) }) - assert.Equal(t, initialL, *L, "Expected func returned from ReplaceGlobals to restore initial L.") - assert.Equal(t, initialS, *S, "Expected func returned from ReplaceGlobals to restore initial S.") + assert.Equal(t, initialL, *L(), "Expected func returned from ReplaceGlobals to restore initial L.") + assert.Equal(t, initialS, *S(), "Expected func returned from ReplaceGlobals to restore initial S.") +} + +func TestGlobalsConcurrentUse(t *testing.T) { + var ( + stop atomic.Bool + wg sync.WaitGroup + ) + + for i := 0; i < 100; i++ { + wg.Add(2) + go func() { + for !stop.Load() { + ReplaceGlobals(NewNop()) + } + wg.Done() + }() + go func() { + for !stop.Load() { + L().With(Int("foo", 42)).Named("main").WithOptions(Development()).Info("") + S().Info("") + } + wg.Done() + }() + } + + testutils.Sleep(100 * time.Millisecond) + stop.Toggle() + wg.Wait() } func TestRedirectStdLog(t *testing.T) {