From 76a119a462e9745cba24e2890fe4efcf2055a51c Mon Sep 17 00:00:00 2001 From: Steffen Siering Date: Fri, 24 Jan 2020 16:03:48 +0100 Subject: [PATCH] Split global and normal logger tests (#15700) Move logp global functions to global.go and selective.go, plus split up tests. The change also adds the `nologpglobal` build tag to global.go and global_test.go. By default all global functions are available. When compiling or running a package its test code with `-tags=nologpglobal`, then global logp functions are not available. (cherry picked from commit 4183ba365a4c3fe9b8f7ca0649c1cc7c48de0e68) --- libbeat/logp/core_test.go | 116 --------------------------------- libbeat/logp/global.go | 8 +-- libbeat/logp/global_test.go | 111 +++++++++++++++++++++++++++++++ libbeat/logp/logger.go | 10 +++ libbeat/logp/selective.go | 6 ++ libbeat/logp/selective_test.go | 54 +++++++++++++++ 6 files changed, 183 insertions(+), 122 deletions(-) create mode 100644 libbeat/logp/global_test.go create mode 100644 libbeat/logp/selective_test.go diff --git a/libbeat/logp/core_test.go b/libbeat/logp/core_test.go index b7d838097f1b..9ebe0bb38de4 100644 --- a/libbeat/logp/core_test.go +++ b/libbeat/logp/core_test.go @@ -20,7 +20,6 @@ package logp import ( "io/ioutil" golog "log" - "strings" "testing" "github.com/stretchr/testify/assert" @@ -29,8 +28,6 @@ import ( func TestLogger(t *testing.T) { exerciseLogger := func() { - Info("unnamed global logger") - log := NewLogger("example") log.Info("some message") log.Infof("some message with parameter x=%v, y=%v", 1, 2) @@ -55,78 +52,6 @@ func TestLogger(t *testing.T) { exerciseLogger() } -func TestLoggerSelectors(t *testing.T) { - if err := DevelopmentSetup(WithSelectors("good", " padded "), ToObserverOutput()); err != nil { - t.Fatal(err) - } - - assert.True(t, HasSelector("padded")) - - good := NewLogger("good") - bad := NewLogger("bad") - - good.Debug("is logged") - logs := ObserverLogs().TakeAll() - assert.Len(t, logs, 1) - - // Selectors only apply to debug level logs. - bad.Debug("not logged") - logs = ObserverLogs().TakeAll() - assert.Len(t, logs, 0) - - bad.Info("is also logged") - logs = ObserverLogs().TakeAll() - assert.Len(t, logs, 1) -} - -func TestGlobalLoggerLevel(t *testing.T) { - if err := DevelopmentSetup(ToObserverOutput()); err != nil { - t.Fatal(err) - } - - const loggerName = "tester" - - Debug(loggerName, "debug") - logs := ObserverLogs().TakeAll() - if assert.Len(t, logs, 1) { - assert.Equal(t, zap.DebugLevel, logs[0].Level) - assert.Equal(t, loggerName, logs[0].LoggerName) - assert.Equal(t, "debug", logs[0].Message) - } - - Info("info") - logs = ObserverLogs().TakeAll() - if assert.Len(t, logs, 1) { - assert.Equal(t, zap.InfoLevel, logs[0].Level) - assert.Equal(t, "", logs[0].LoggerName) - assert.Equal(t, "info", logs[0].Message) - } - - Warn("warning") - logs = ObserverLogs().TakeAll() - if assert.Len(t, logs, 1) { - assert.Equal(t, zap.WarnLevel, logs[0].Level) - assert.Equal(t, "", logs[0].LoggerName) - assert.Equal(t, "warning", logs[0].Message) - } - - Err("error") - logs = ObserverLogs().TakeAll() - if assert.Len(t, logs, 1) { - assert.Equal(t, zap.ErrorLevel, logs[0].Level) - assert.Equal(t, "", logs[0].LoggerName) - assert.Equal(t, "error", logs[0].Message) - } - - Critical("critical") - logs = ObserverLogs().TakeAll() - if assert.Len(t, logs, 1) { - assert.Equal(t, zap.ErrorLevel, logs[0].Level) - assert.Equal(t, "", logs[0].LoggerName) - assert.Equal(t, "critical", logs[0].Message) - } -} - func TestLoggerLevel(t *testing.T) { if err := DevelopmentSetup(ToObserverOutput()); err != nil { t.Fatal(err) @@ -168,47 +93,6 @@ func TestLoggerLevel(t *testing.T) { } } -func TestRecover(t *testing.T) { - const recoveryExplanation = "Something went wrong" - const cause = "unexpected condition" - - DevelopmentSetup(ToObserverOutput()) - - defer func() { - logs := ObserverLogs().TakeAll() - if assert.Len(t, logs, 1) { - log := logs[0] - assert.Equal(t, zap.ErrorLevel, log.Level) - assert.Equal(t, "logp/core_test.go", - strings.Split(log.Caller.TrimmedPath(), ":")[0]) - assert.Contains(t, log.Message, recoveryExplanation+ - ". Recovering, but please report this.") - assert.Contains(t, log.ContextMap(), "panic") - } - }() - - defer Recover(recoveryExplanation) - panic(cause) -} - -func TestHasSelector(t *testing.T) { - DevelopmentSetup(WithSelectors("*", "config")) - assert.True(t, HasSelector("config")) - assert.False(t, HasSelector("publish")) -} - -func TestIsDebug(t *testing.T) { - DevelopmentSetup() - assert.True(t, IsDebug("all")) - - DevelopmentSetup(WithSelectors("*")) - assert.True(t, IsDebug("all")) - - DevelopmentSetup(WithSelectors("only_this")) - assert.False(t, IsDebug("all")) - assert.True(t, IsDebug("only_this")) -} - func TestL(t *testing.T) { if err := DevelopmentSetup(ToObserverOutput()); err != nil { t.Fatal(err) diff --git a/libbeat/logp/global.go b/libbeat/logp/global.go index bc14a8c3a678..584379c71a51 100644 --- a/libbeat/logp/global.go +++ b/libbeat/logp/global.go @@ -15,6 +15,8 @@ // specific language governing permissions and limitations // under the License. +//+build !nologpglobal + package logp import ( @@ -31,12 +33,6 @@ func MakeDebug(selector string) func(string, ...interface{}) { } } -// HasSelector returns true if the given selector was explicitly set. -func HasSelector(selector string) bool { - _, found := loadLogger().selectors[selector] - return found -} - // IsDebug returns true if the given selector would be logged. // Deprecated: Use logp.NewLogger. func IsDebug(selector string) bool { diff --git a/libbeat/logp/global_test.go b/libbeat/logp/global_test.go new file mode 100644 index 000000000000..400dc0a34e45 --- /dev/null +++ b/libbeat/logp/global_test.go @@ -0,0 +1,111 @@ +// Licensed to Elasticsearch B.V. under one or more contributor +// license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright +// ownership. Elasticsearch B.V. licenses this file to you 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, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +//+build !nologpglobal + +package logp + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "go.uber.org/zap" +) + +func TestGlobalLoggerLevel(t *testing.T) { + if err := DevelopmentSetup(ToObserverOutput()); err != nil { + t.Fatal(err) + } + + const loggerName = "tester" + + Debug(loggerName, "debug") + logs := ObserverLogs().TakeAll() + if assert.Len(t, logs, 1) { + assert.Equal(t, zap.DebugLevel, logs[0].Level) + assert.Equal(t, loggerName, logs[0].LoggerName) + assert.Equal(t, "debug", logs[0].Message) + } + + Info("info") + logs = ObserverLogs().TakeAll() + if assert.Len(t, logs, 1) { + assert.Equal(t, zap.InfoLevel, logs[0].Level) + assert.Equal(t, "", logs[0].LoggerName) + assert.Equal(t, "info", logs[0].Message) + } + + Warn("warning") + logs = ObserverLogs().TakeAll() + if assert.Len(t, logs, 1) { + assert.Equal(t, zap.WarnLevel, logs[0].Level) + assert.Equal(t, "", logs[0].LoggerName) + assert.Equal(t, "warning", logs[0].Message) + } + + Err("error") + logs = ObserverLogs().TakeAll() + if assert.Len(t, logs, 1) { + assert.Equal(t, zap.ErrorLevel, logs[0].Level) + assert.Equal(t, "", logs[0].LoggerName) + assert.Equal(t, "error", logs[0].Message) + } + + Critical("critical") + logs = ObserverLogs().TakeAll() + if assert.Len(t, logs, 1) { + assert.Equal(t, zap.ErrorLevel, logs[0].Level) + assert.Equal(t, "", logs[0].LoggerName) + assert.Equal(t, "critical", logs[0].Message) + } +} + +func TestRecover(t *testing.T) { + const recoveryExplanation = "Something went wrong" + const cause = "unexpected condition" + + DevelopmentSetup(ToObserverOutput()) + + defer func() { + logs := ObserverLogs().TakeAll() + if assert.Len(t, logs, 1) { + log := logs[0] + assert.Equal(t, zap.ErrorLevel, log.Level) + assert.Equal(t, "logp/global_test.go", + strings.Split(log.Caller.TrimmedPath(), ":")[0]) + assert.Contains(t, log.Message, recoveryExplanation+ + ". Recovering, but please report this.") + assert.Contains(t, log.ContextMap(), "panic") + } + }() + + defer Recover(recoveryExplanation) + panic(cause) +} + +func TestIsDebug(t *testing.T) { + DevelopmentSetup() + assert.True(t, IsDebug("all")) + + DevelopmentSetup(WithSelectors("*")) + assert.True(t, IsDebug("all")) + + DevelopmentSetup(WithSelectors("only_this")) + assert.False(t, IsDebug("all")) + assert.True(t, IsDebug("only_this")) +} diff --git a/libbeat/logp/logger.go b/libbeat/logp/logger.go index f915477fc1cc..b776a6166f3c 100644 --- a/libbeat/logp/logger.go +++ b/libbeat/logp/logger.go @@ -18,6 +18,8 @@ package logp import ( + "fmt" + "go.uber.org/zap" "go.uber.org/zap/zapcore" ) @@ -203,6 +205,14 @@ func (l *Logger) DPanicw(msg string, keysAndValues ...interface{}) { l.sugar.DPanicw(msg, keysAndValues...) } +// Recover stops a panicking goroutine and logs an Error. +func (l *Logger) Recover(msg string) { + if r := recover(); r != nil { + msg := fmt.Sprintf("%s. Recovering, but please report this.", msg) + l.Error(msg, zap.Any("panic", r), zap.Stack("stack")) + } +} + // L returns an unnamed global logger. func L() *Logger { return loadLogger().logger diff --git a/libbeat/logp/selective.go b/libbeat/logp/selective.go index a5d6a7a7a002..cf73a3f7e6ba 100644 --- a/libbeat/logp/selective.go +++ b/libbeat/logp/selective.go @@ -27,6 +27,12 @@ type selectiveCore struct { core zapcore.Core } +// HasSelector returns true if the given selector was explicitly set. +func HasSelector(selector string) bool { + _, found := loadLogger().selectors[selector] + return found +} + func selectiveWrapper(core zapcore.Core, selectors map[string]struct{}) zapcore.Core { if len(selectors) == 0 { return core diff --git a/libbeat/logp/selective_test.go b/libbeat/logp/selective_test.go new file mode 100644 index 000000000000..2fd90f89ecf0 --- /dev/null +++ b/libbeat/logp/selective_test.go @@ -0,0 +1,54 @@ +// Licensed to Elasticsearch B.V. under one or more contributor +// license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright +// ownership. Elasticsearch B.V. licenses this file to you 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, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package logp + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestHasSelector(t *testing.T) { + DevelopmentSetup(WithSelectors("*", "config")) + assert.True(t, HasSelector("config")) + assert.False(t, HasSelector("publish")) +} + +func TestLoggerSelectors(t *testing.T) { + if err := DevelopmentSetup(WithSelectors("good", " padded "), ToObserverOutput()); err != nil { + t.Fatal(err) + } + + assert.True(t, HasSelector("padded")) + + good := NewLogger("good") + bad := NewLogger("bad") + + good.Debug("is logged") + logs := ObserverLogs().TakeAll() + assert.Len(t, logs, 1) + + // Selectors only apply to debug level logs. + bad.Debug("not logged") + logs = ObserverLogs().TakeAll() + assert.Len(t, logs, 0) + + bad.Info("is also logged") + logs = ObserverLogs().TakeAll() + assert.Len(t, logs, 1) +}