From 244dff5082bda3aa1b99af95c37ed6e7d118b77d Mon Sep 17 00:00:00 2001 From: Arnold Iakab Date: Tue, 7 Jun 2022 16:32:35 +0300 Subject: [PATCH 1/3] Extract limited stack trace frames from error --- maintenance/terminationlog/termlog.go | 41 ++++++++++++++++++++-- maintenance/terminationlog/termlog_test.go | 35 ++++++++++++++++++ 2 files changed, 74 insertions(+), 2 deletions(-) create mode 100644 maintenance/terminationlog/termlog_test.go diff --git a/maintenance/terminationlog/termlog.go b/maintenance/terminationlog/termlog.go index 4924854c3..6609db6aa 100644 --- a/maintenance/terminationlog/termlog.go +++ b/maintenance/terminationlog/termlog.go @@ -9,16 +9,27 @@ package terminationlog import ( "fmt" + "github.com/pkg/errors" "github.com/rs/zerolog/log" "os" ) var logFile *os.File +const StackTraceLimit = 100 + +type stackTracer interface { + StackTrace() errors.StackTrace +} + // Fatalf implements log Fatalf interface func Fatalf(format string, v ...interface{}) { if logFile != nil { - fmt.Fprintf(logFile, format, v...) + if fs, err := extractErrorFrames(v); err == nil { + fmt.Fprintf(logFile, format, fs) + } else { + fmt.Fprintf(logFile, format, v...) + } } log.Fatal().Msg(fmt.Sprintf(format, v...)) @@ -27,7 +38,11 @@ func Fatalf(format string, v ...interface{}) { // Fatal implements log Fatal interface func Fatal(v ...interface{}) { if logFile != nil { - fmt.Fprint(logFile, v...) + if fs, err := extractErrorFrames(v); err == nil { + fmt.Fprint(logFile, fs) + } else { + fmt.Fprint(logFile, v...) + } } log.Fatal().Msg(fmt.Sprint(v...)) @@ -37,3 +52,25 @@ func Fatal(v ...interface{}) { func Fatalln(v ...interface{}) { Fatal(v...) } + +func extractErrorFrames(v ...interface{}) (string, error) { + if len(v) != 1 { + return "", fmt.Errorf("value contains multiple elements") + } + + err, ok := v[0].(error) + if !ok { + return "", fmt.Errorf("value element is not an error") + } + + if str, ok := err.(stackTracer); ok { + st := str.StackTrace() + if len(st) > StackTraceLimit { + return fmt.Sprintf("%+v", st[0:StackTraceLimit]), nil + } else { + return fmt.Sprintf("%+v", st[0:]), nil + } + } else { + return "", fmt.Errorf("error does not implement stackTracer") + } +} diff --git a/maintenance/terminationlog/termlog_test.go b/maintenance/terminationlog/termlog_test.go new file mode 100644 index 000000000..c89bc63cc --- /dev/null +++ b/maintenance/terminationlog/termlog_test.go @@ -0,0 +1,35 @@ +package terminationlog + +import ( + "fmt" + "github.com/pkg/errors" + "github.com/stretchr/testify/assert" + "testing" +) + +func TestExtractErrorFrames(t *testing.T) { + + tcs := []struct { + name string + err error + shouldFail bool + }{ + { + name: "error created with github pkg", + err: errors.New("error"), + shouldFail: false, + }, + { + name: "error created with std lib", + err: fmt.Errorf("error"), + shouldFail: true, + }, + } + + for _, tc := range tcs { + t.Run(tc.name, func(t *testing.T) { + _, err := extractErrorFrames(tc.err) + assert.Equal(t, err != nil, tc.shouldFail) + }) + } +} From 695cf9174f6834fcedbf5833a7f4e7f543893680 Mon Sep 17 00:00:00 2001 From: Arnold Iakab Date: Wed, 8 Jun 2022 12:29:33 +0300 Subject: [PATCH 2/3] Limit termination log output --- maintenance/terminationlog/termlog.go | 64 +++++++++++++++------- maintenance/terminationlog/termlog_test.go | 56 +++++++++++++++++-- 2 files changed, 97 insertions(+), 23 deletions(-) diff --git a/maintenance/terminationlog/termlog.go b/maintenance/terminationlog/termlog.go index 6609db6aa..5bcf0ce70 100644 --- a/maintenance/terminationlog/termlog.go +++ b/maintenance/terminationlog/termlog.go @@ -16,7 +16,7 @@ import ( var logFile *os.File -const StackTraceLimit = 100 +const LogFileLimit = 4096 // bytes type stackTracer interface { StackTrace() errors.StackTrace @@ -25,11 +25,7 @@ type stackTracer interface { // Fatalf implements log Fatalf interface func Fatalf(format string, v ...interface{}) { if logFile != nil { - if fs, err := extractErrorFrames(v); err == nil { - fmt.Fprintf(logFile, format, fs) - } else { - fmt.Fprintf(logFile, format, v...) - } + fmt.Fprint(logFile, buildTerminationLogOutputf(format, v...)) } log.Fatal().Msg(fmt.Sprintf(format, v...)) @@ -38,11 +34,7 @@ func Fatalf(format string, v ...interface{}) { // Fatal implements log Fatal interface func Fatal(v ...interface{}) { if logFile != nil { - if fs, err := extractErrorFrames(v); err == nil { - fmt.Fprint(logFile, fs) - } else { - fmt.Fprint(logFile, v...) - } + fmt.Fprint(logFile, buildTerminationLogOutput(v...)) } log.Fatal().Msg(fmt.Sprint(v...)) @@ -53,24 +45,58 @@ func Fatalln(v ...interface{}) { Fatal(v...) } -func extractErrorFrames(v ...interface{}) (string, error) { +func buildTerminationLogOutputf(f string, v ...interface{}) string { + if res, err := extractErrorFrames(v...); err == nil { + vs := make([]interface{}, 0) + vs = append(vs, f) + vs = append(vs, res...) + return buildOutput(vs...) + } + + return buildOutput(fmt.Sprintf(f, v...)) +} + +func buildTerminationLogOutput(v ...interface{}) string { + if res, err := extractErrorFrames(v...); err == nil { + return buildOutput(res...) + } + + return buildOutput(v...) +} + +func buildOutput(v ...interface{}) string { + sb := make([]byte, 0) + for _, f := range v { + s := fmt.Sprintf("%+v\n", f) + b := []byte(s) + if len(b) <= LogFileLimit && len(sb)+len(b) <= LogFileLimit { + sb = append(sb, b...) + } else { + break + } + } + return string(sb) +} + +func extractErrorFrames(v ...interface{}) ([]interface{}, error) { if len(v) != 1 { - return "", fmt.Errorf("value contains multiple elements") + return nil, fmt.Errorf("value contains multiple elements") } err, ok := v[0].(error) if !ok { - return "", fmt.Errorf("value element is not an error") + return nil, fmt.Errorf("value element is not an error") } if str, ok := err.(stackTracer); ok { st := str.StackTrace() - if len(st) > StackTraceLimit { - return fmt.Sprintf("%+v", st[0:StackTraceLimit]), nil - } else { - return fmt.Sprintf("%+v", st[0:]), nil + vs := make([]interface{}, 0) + vs = append(vs, fmt.Sprintf("%s", err.Error())) + for _, f := range st { + vs = append(vs, f) } + return vs, nil } else { - return "", fmt.Errorf("error does not implement stackTracer") + return nil, fmt.Errorf("error does not implement stackTracer") } } diff --git a/maintenance/terminationlog/termlog_test.go b/maintenance/terminationlog/termlog_test.go index c89bc63cc..d03480977 100644 --- a/maintenance/terminationlog/termlog_test.go +++ b/maintenance/terminationlog/termlog_test.go @@ -1,10 +1,12 @@ package terminationlog import ( + "errors" "fmt" - "github.com/pkg/errors" - "github.com/stretchr/testify/assert" "testing" + + pkgerrors "github.com/pkg/errors" + "github.com/stretchr/testify/assert" ) func TestExtractErrorFrames(t *testing.T) { @@ -15,12 +17,17 @@ func TestExtractErrorFrames(t *testing.T) { shouldFail bool }{ { - name: "error created with github pkg", + name: "error created with std lib", err: errors.New("error"), + shouldFail: true, + }, + { + name: "error created with github pkg", + err: pkgerrors.New("error"), shouldFail: false, }, { - name: "error created with std lib", + name: "plain error", err: fmt.Errorf("error"), shouldFail: true, }, @@ -33,3 +40,44 @@ func TestExtractErrorFrames(t *testing.T) { }) } } + +func TestBuildTerminationLogOutput(t *testing.T) { + errMsg := "error" + + tcs := []struct { + name string + errFunc func(s string) error + outputWithStackTrace bool + }{ + { + name: "error created with github pkg", + errFunc: func(s string) error { + return pkgerrors.New(s) + }, + outputWithStackTrace: true, + }, + { + name: "error created with std lib", + errFunc: func(s string) error { + return errors.New(s) + }, + outputWithStackTrace: false, + }, + { + name: "plain error", + errFunc: func(s string) error { + return fmt.Errorf(s) + }, + outputWithStackTrace: false, + }, + } + + for _, tc := range tcs { + t.Run(tc.name, func(t *testing.T) { + out := buildTerminationLogOutput(tc.errFunc(errMsg)) + assert.Equal(t, tc.outputWithStackTrace, len(out) > len(fmt.Sprintf("%s\n", errMsg))) + out = buildTerminationLogOutputf("message: %v", tc.errFunc(errMsg)) + assert.Equal(t, tc.outputWithStackTrace, len(out) > len(fmt.Sprintf("message: %s\n", errMsg))) + }) + } +} From ac2f27841581023b2f9c41c06044708e36226fb8 Mon Sep 17 00:00:00 2001 From: Arnold Iakab Date: Wed, 8 Jun 2022 13:00:20 +0300 Subject: [PATCH 3/3] Limit termination log output --- maintenance/terminationlog/termlog.go | 64 +++-------------- maintenance/terminationlog/termlog_test.go | 84 +++------------------- 2 files changed, 17 insertions(+), 131 deletions(-) diff --git a/maintenance/terminationlog/termlog.go b/maintenance/terminationlog/termlog.go index 5bcf0ce70..0a7d62811 100644 --- a/maintenance/terminationlog/termlog.go +++ b/maintenance/terminationlog/termlog.go @@ -16,7 +16,7 @@ import ( var logFile *os.File -const LogFileLimit = 4096 // bytes +const LogFileLimit = 4096 // bytes; type stackTracer interface { StackTrace() errors.StackTrace @@ -25,7 +25,7 @@ type stackTracer interface { // Fatalf implements log Fatalf interface func Fatalf(format string, v ...interface{}) { if logFile != nil { - fmt.Fprint(logFile, buildTerminationLogOutputf(format, v...)) + fmt.Fprint(logFile, limitLogFileOutput(fmt.Sprintf(format, v...))) } log.Fatal().Msg(fmt.Sprintf(format, v...)) @@ -34,7 +34,7 @@ func Fatalf(format string, v ...interface{}) { // Fatal implements log Fatal interface func Fatal(v ...interface{}) { if logFile != nil { - fmt.Fprint(logFile, buildTerminationLogOutput(v...)) + fmt.Fprint(logFile, limitLogFileOutput(fmt.Sprint(v...))) } log.Fatal().Msg(fmt.Sprint(v...)) @@ -45,58 +45,12 @@ func Fatalln(v ...interface{}) { Fatal(v...) } -func buildTerminationLogOutputf(f string, v ...interface{}) string { - if res, err := extractErrorFrames(v...); err == nil { - vs := make([]interface{}, 0) - vs = append(vs, f) - vs = append(vs, res...) - return buildOutput(vs...) +func limitLogFileOutput(s string) string { + sb := []byte(s) + limit := len(sb) + if limit > LogFileLimit { + limit = LogFileLimit } - return buildOutput(fmt.Sprintf(f, v...)) -} - -func buildTerminationLogOutput(v ...interface{}) string { - if res, err := extractErrorFrames(v...); err == nil { - return buildOutput(res...) - } - - return buildOutput(v...) -} - -func buildOutput(v ...interface{}) string { - sb := make([]byte, 0) - for _, f := range v { - s := fmt.Sprintf("%+v\n", f) - b := []byte(s) - if len(b) <= LogFileLimit && len(sb)+len(b) <= LogFileLimit { - sb = append(sb, b...) - } else { - break - } - } - return string(sb) -} - -func extractErrorFrames(v ...interface{}) ([]interface{}, error) { - if len(v) != 1 { - return nil, fmt.Errorf("value contains multiple elements") - } - - err, ok := v[0].(error) - if !ok { - return nil, fmt.Errorf("value element is not an error") - } - - if str, ok := err.(stackTracer); ok { - st := str.StackTrace() - vs := make([]interface{}, 0) - vs = append(vs, fmt.Sprintf("%s", err.Error())) - for _, f := range st { - vs = append(vs, f) - } - return vs, nil - } else { - return nil, fmt.Errorf("error does not implement stackTracer") - } + return string(sb[:limit]) } diff --git a/maintenance/terminationlog/termlog_test.go b/maintenance/terminationlog/termlog_test.go index d03480977..eaec1b0f6 100644 --- a/maintenance/terminationlog/termlog_test.go +++ b/maintenance/terminationlog/termlog_test.go @@ -1,83 +1,15 @@ package terminationlog import ( - "errors" - "fmt" - "testing" - - pkgerrors "github.com/pkg/errors" "github.com/stretchr/testify/assert" + "testing" ) -func TestExtractErrorFrames(t *testing.T) { - - tcs := []struct { - name string - err error - shouldFail bool - }{ - { - name: "error created with std lib", - err: errors.New("error"), - shouldFail: true, - }, - { - name: "error created with github pkg", - err: pkgerrors.New("error"), - shouldFail: false, - }, - { - name: "plain error", - err: fmt.Errorf("error"), - shouldFail: true, - }, - } - - for _, tc := range tcs { - t.Run(tc.name, func(t *testing.T) { - _, err := extractErrorFrames(tc.err) - assert.Equal(t, err != nil, tc.shouldFail) - }) - } -} - -func TestBuildTerminationLogOutput(t *testing.T) { - errMsg := "error" - - tcs := []struct { - name string - errFunc func(s string) error - outputWithStackTrace bool - }{ - { - name: "error created with github pkg", - errFunc: func(s string) error { - return pkgerrors.New(s) - }, - outputWithStackTrace: true, - }, - { - name: "error created with std lib", - errFunc: func(s string) error { - return errors.New(s) - }, - outputWithStackTrace: false, - }, - { - name: "plain error", - errFunc: func(s string) error { - return fmt.Errorf(s) - }, - outputWithStackTrace: false, - }, - } - - for _, tc := range tcs { - t.Run(tc.name, func(t *testing.T) { - out := buildTerminationLogOutput(tc.errFunc(errMsg)) - assert.Equal(t, tc.outputWithStackTrace, len(out) > len(fmt.Sprintf("%s\n", errMsg))) - out = buildTerminationLogOutputf("message: %v", tc.errFunc(errMsg)) - assert.Equal(t, tc.outputWithStackTrace, len(out) > len(fmt.Sprintf("message: %s\n", errMsg))) - }) - } +func TestTrimLogFileOutput(t *testing.T) { + b := make([]byte, 5000) + assert.Len(t, limitLogFileOutput(string(b)), 4096) + b = make([]byte, 2000) + assert.Len(t, limitLogFileOutput(string(b)), 2000) + b = make([]byte, 4096) + assert.Len(t, limitLogFileOutput(string(b)), 4096) }