Skip to content

Commit

Permalink
change from logger stacktrace to error stacktrace
Browse files Browse the repository at this point in the history
  • Loading branch information
@fredcarle
fredcarle committed Sep 13, 2022
1 parent 98bad91 commit 2ddb043
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 18 deletions.
36 changes: 30 additions & 6 deletions logging/logger.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ package logging

import (
"context"
"fmt"
stdlog "log"
"os"
"sync"
Expand Down Expand Up @@ -65,6 +66,7 @@ func (l *logger) Error(ctx context.Context, message string, keyvals ...KV) {
func (l *logger) ErrorE(ctx context.Context, message string, err error, keyvals ...KV) {
kvs := keyvals
kvs = append(kvs, NewKV("Error", err))
kvs = withStackTrace(err, kvs)

l.syncLock.RLock()
defer l.syncLock.RUnlock()
Expand All @@ -82,6 +84,7 @@ func (l *logger) Fatal(ctx context.Context, message string, keyvals ...KV) {
func (l *logger) FatalE(ctx context.Context, message string, err error, keyvals ...KV) {
kvs := keyvals
kvs = append(kvs, NewKV("Error", err))
kvs = withStackTrace(err, kvs)

l.syncLock.RLock()
defer l.syncLock.RUnlock()
Expand All @@ -108,11 +111,14 @@ func (l *logger) FeedbackError(ctx context.Context, message string, keyvals ...K
}

func (l *logger) FeedbackErrorE(ctx context.Context, message string, err error, keyvals ...KV) {
l.ErrorE(ctx, message, err, keyvals...)
l.ErrorE(ctx, message, err, withStackTrace(err, keyvals)...)
l.syncLock.RLock()
defer l.syncLock.RUnlock()
if l.consoleLogger != nil {
l.consoleLogger.Println(message)
if stack, hasStack := getStackTrace(err); hasStack {
l.consoleLogger.Println(stack)
}
}
}

Expand All @@ -126,11 +132,14 @@ func (l *logger) FeedbackFatal(ctx context.Context, message string, keyvals ...K
}

func (l *logger) FeedbackFatalE(ctx context.Context, message string, err error, keyvals ...KV) {
l.FatalE(ctx, message, err, keyvals...)
l.FatalE(ctx, message, err, withStackTrace(err, keyvals)...)
l.syncLock.RLock()
defer l.syncLock.RUnlock()
if l.consoleLogger != nil {
l.consoleLogger.Println(message)
if stack, hasStack := getStackTrace(err); hasStack {
l.consoleLogger.Println(stack)
}
}
}

Expand Down Expand Up @@ -171,6 +180,25 @@ func (l *logger) ApplyConfig(config Config) {
}
}

func withStackTrace(err error, keyvals []KV) []KV {
if stack, hasStack := getStackTrace(err); hasStack {
return append(keyvals, NewKV("stacktrace", stack))
}

return keyvals
}

func getStackTrace(err error) (string, bool) {
configMutex.RLock()
defer configMutex.RUnlock()

if cachedConfig.EnableStackTrace.EnableStackTrace {
return fmt.Sprintf("%+v", err), true
}

return "", false
}

func buildZapLogger(name string, config Config) (*zap.Logger, error) {
const (
encodingTypeConsole string = "console"
Expand All @@ -188,10 +216,6 @@ func buildZapLogger(name string, config Config) (*zap.Logger, error) {
defaultConfig.Level = zap.NewAtomicLevelAt(zapcore.Level(config.Level.LogLevel))
}

if config.EnableStackTrace.HasValue {
defaultConfig.DisableStacktrace = !config.EnableStackTrace.EnableStackTrace
}

if config.DisableColor.HasValue && config.DisableColor.DisableColor {
defaultConfig.EncoderConfig.EncodeLevel = zapcore.CapitalLevelEncoder
}
Expand Down
29 changes: 17 additions & 12 deletions logging/logging_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,8 @@ func TestLogWritesFatalMessageWithStackTraceToLogAndKillsProcessGivenStackTraceE
assert.Equal(t, logMessage, logLines[0]["msg"])
assert.Equal(t, "FATAL", logLines[0]["level"])
assert.Equal(t, "TestLogName", logLines[0]["logger"])
assert.Contains(t, logLines[0], "stacktrace")
// no stacktrace will be present since no error was sent to the logger.
assert.NotContains(t, logLines[0], "stacktrace")
// caller is disabled by default
assert.NotContains(t, logLines[0], "logging_test.go")
}
Expand Down Expand Up @@ -202,35 +203,35 @@ type LogLevelTestCase struct {
func logDebug(l Logger, c context.Context, m string) { l.Debug(c, m) }
func logInfo(l Logger, c context.Context, m string) { l.Info(c, m) }
func logError(l Logger, c context.Context, m string) { l.Error(c, m) }
func logErrorE(l Logger, c context.Context, m string) { l.ErrorE(c, m, fmt.Errorf("test error")) }
func logErrorE(l Logger, c context.Context, m string) { l.ErrorE(c, m, errors.New("test error")) }

func getLogLevelTestCase() []LogLevelTestCase {
return []LogLevelTestCase{
{Debug, logDebug, "DEBUG", false, false, true},
{Debug, logDebug, "DEBUG", false, false, false},
{Debug, logInfo, "INFO", false, false, false},
{Debug, logError, "ERROR", false, false, false},
{Debug, logError, "ERROR", true, true, false},
{Debug, logError, "ERROR", true, false, false},
{Debug, logErrorE, "ERROR", false, false, false},
{Debug, logErrorE, "ERROR", true, true, false},
{Info, logDebug, "", false, false, false},
{Info, logInfo, "INFO", false, false, true},
{Info, logInfo, "INFO", false, false, false},
{Info, logError, "ERROR", false, false, false},
{Info, logError, "ERROR", true, true, false},
{Info, logError, "ERROR", true, false, false},
{Info, logErrorE, "ERROR", false, false, false},
{Info, logErrorE, "ERROR", true, true, false},
{Warn, logDebug, "", false, false, false},
{Warn, logInfo, "", false, false, false},
{Warn, logError, "ERROR", false, false, false},
{Warn, logError, "ERROR", true, true, false},
{Warn, logError, "ERROR", true, false, false},
{Warn, logErrorE, "ERROR", false, false, false},
{Warn, logErrorE, "ERROR", true, true, false},
{Error, logDebug, "", false, false, false},
{Error, logInfo, "", false, false, false},
{Error, logError, "ERROR", false, false, true},
{Error, logError, "ERROR", false, false, false},
{Error, logError, "ERROR", true, true, false},
{Error, logError, "ERROR", true, false, false},
{Error, logErrorE, "ERROR", false, false, false},
{Error, logErrorE, "ERROR", true, true, false},
{Fatal, logDebug, "", false, false, true},
Expand Down Expand Up @@ -525,31 +526,31 @@ func TestLogWritesMessagesToLogGivenUpdatedLogPath(t *testing.T) {
func logFeedbackInfo(l Logger, c context.Context, m string) { l.FeedbackInfo(c, m) }
func logFeedbackError(l Logger, c context.Context, m string) { l.FeedbackError(c, m) }
func logFeedbackErrorE(l Logger, c context.Context, m string) {
l.FeedbackErrorE(c, m, fmt.Errorf("test error"))
l.FeedbackErrorE(c, m, errors.New("test error"))
}

func getFeedbackLogLevelTestCase() []LogLevelTestCase {
return []LogLevelTestCase{
{Debug, logFeedbackInfo, "INFO", false, false, false},
{Debug, logFeedbackError, "ERROR", false, false, false},
{Debug, logFeedbackError, "ERROR", true, true, false},
{Debug, logFeedbackError, "ERROR", true, false, false},
{Debug, logFeedbackErrorE, "ERROR", false, false, false},
{Debug, logFeedbackErrorE, "ERROR", true, true, false},
{Info, logFeedbackInfo, "INFO", false, false, true},
{Info, logFeedbackInfo, "INFO", false, false, false},
{Info, logFeedbackError, "ERROR", false, false, false},
{Info, logFeedbackError, "ERROR", true, true, false},
{Info, logFeedbackError, "ERROR", true, false, false},
{Info, logFeedbackErrorE, "ERROR", false, false, false},
{Info, logFeedbackErrorE, "ERROR", true, true, false},
{Warn, logFeedbackInfo, "", false, false, false},
{Warn, logFeedbackError, "ERROR", false, false, false},
{Warn, logFeedbackError, "ERROR", true, true, false},
{Warn, logFeedbackError, "ERROR", true, false, false},
{Warn, logFeedbackErrorE, "ERROR", false, false, false},
{Warn, logFeedbackErrorE, "ERROR", true, true, false},
{Error, logFeedbackInfo, "", false, false, false},
{Error, logFeedbackError, "ERROR", false, false, true},
{Error, logFeedbackError, "ERROR", false, false, false},
{Error, logFeedbackError, "ERROR", true, true, false},
{Error, logFeedbackError, "ERROR", true, false, false},
{Error, logFeedbackErrorE, "ERROR", false, false, false},
{Error, logFeedbackErrorE, "ERROR", true, true, false},
{Fatal, logFeedbackInfo, "", false, false, false},
Expand Down Expand Up @@ -596,7 +597,11 @@ func TestLogWritesMessagesToFeedbackLog(t *testing.T) {
assert.Equal(t, tc.WithCaller, hasCaller)
}

assert.Equal(t, logMessage+"\n", b.String())
if tc.ExpectStackTrace {
assert.Contains(t, b.String(), logMessage+"\ntest error. Stack:")
} else {
assert.Equal(t, logMessage+"\n", b.String())
}

clearRegistry("TestLogName")
}
Expand Down

0 comments on commit 2ddb043

Please sign in to comment.