diff --git a/pkg/log/BUILD b/pkg/log/BUILD index 4dc76292f1..d4ce15fcea 100644 --- a/pkg/log/BUILD +++ b/pkg/log/BUILD @@ -8,12 +8,12 @@ package( go_library( name = "log", srcs = [ + "bug.go", "glog.go", "json.go", "json_k8s.go", "log.go", "rate_limited.go", - "warn_on.go", ], marshal = False, stateify = False, @@ -31,9 +31,9 @@ go_test( name = "log_test", size = "small", srcs = [ + "bug_test.go", "json_test.go", "log_test.go", - "warn_on_test.go", ], library = ":log", ) diff --git a/pkg/log/warn_on.go b/pkg/log/bug.go similarity index 54% rename from pkg/log/warn_on.go rename to pkg/log/bug.go index 552928de68..ff19103f8a 100644 --- a/pkg/log/warn_on.go +++ b/pkg/log/bug.go @@ -22,13 +22,22 @@ import ( "gvisor.dev/gvisor/pkg/sync" ) +// This file contains helper functions analogous to the Linux kernel's WARN* +// macros. Should be used for non-fatal errors that should be treated as bugs +// none the less. + const ( warnFmtStr = "WARNING: BUG on %s:%d\n" warnUnknownLineStr = "WARNING: BUG on unknown line\n" catchAllMagic = "runtime.Caller failed" ) -func warn(caller int, msg string) { +//go:noinline +func reportBugErr(caller int, err error) { + reportBug(caller+1, err.Error(), nil) +} + +func reportBug(caller int, msg string, vars []any) { var b strings.Builder if _, file, line, ok := runtime.Caller(caller); ok { b.WriteString(fmt.Sprintf(warnFmtStr, file, line)) @@ -37,7 +46,11 @@ func warn(caller int, msg string) { } b.WriteByte('\n') if len(msg) > 0 { - b.WriteString(msg) + if len(vars) > 0 { + b.WriteString(fmt.Sprintf(msg, vars...)) + } else { + b.WriteString(msg) + } b.WriteByte('\n') } TracebackAll(b.String()) @@ -50,7 +63,12 @@ var ( warnedSet map[string]struct{} ) -func warnOnce(caller int, msg string) { +//go:noinline +func reportBugErrOnce(caller int, err error) { + reportBugOnce(caller+1, err.Error(), nil) +} + +func reportBugOnce(caller int, msg string, vars []any) { var b strings.Builder if _, file, line, ok := runtime.Caller(caller); ok { key := fmt.Sprintf("%s:%d", file, line) @@ -62,7 +80,11 @@ func warnOnce(caller int, msg string) { b.WriteString(fmt.Sprintf(warnFmtStr, file, line)) b.WriteByte('\n') if len(msg) > 0 { - b.WriteString(msg) + if len(vars) > 0 { + b.WriteString(fmt.Sprintf(msg, vars...)) + } else { + b.WriteString(msg) + } b.WriteByte('\n') } @@ -79,7 +101,11 @@ func warnOnce(caller int, msg string) { b.WriteString(warnUnknownLineStr) b.WriteByte('\n') if len(msg) > 0 { - b.WriteString(msg) + if len(vars) > 0 { + b.WriteString(fmt.Sprintf(msg, vars...)) + } else { + b.WriteString(msg) + } b.WriteByte('\n') } @@ -89,94 +115,47 @@ func warnOnce(caller int, msg string) { } } -// WARN serves the same purpose as the Linux kernel's WARN macro. Use it -// for reporting abnormal bugs encountered at runtime that should be fixed. -// -// Will print out the full Go stacktrace at time of invocation. +// BugTraceback will report a bug with a traceback of all goroutines if the +// error isn't nil. Use it for reporting abnormal bugs encountered at runtime +// that should be fixed. // // Do not use this for bad user input. Errors reported by this function should // not be fatal. -func WARN(cond bool, s string, a ...any) { - if !cond { - return +func BugTraceback(err error) { + if err != nil { + reportBugErr(2, err) } - msg := fmt.Sprintf(s, a...) - warn(2, msg) } -// WARN_ON serves the same purpose as the Linux kernel's WARN_ON macro. Use it -// for reporting abnormal bugs encountered at runtime that should be fixed. -// -// Will print out the full Go stacktrace at time of invocation. -// -// Do not use this for bad user input. Errors reported by this function should -// not be fatal. -func WARN_ON(cond bool) { - if !cond { - return - } - warn(2, "") -} - -// WARN_ERR is a more Go-friendly version of the typical Linux WARN* macros. If -// the error isn't nil, it will report the error prefixed with the standard WARN -// string. Use it for reporting abnormal bugs encountered at runtime that -// should be fixed. -// -// Will print out the full Go stacktrace at time of invocation. -// -// Do not use this for bad user input. Errors reported by this function should -// not be fatal. -func WARN_ERR(err error) { - if err == nil { - return - } - warn(2, err.Error()) -} - -// WARN_ONCE serves the same purpose as the Linux kernel's WARN_ONCE macro. +// BugTracebackf will report a bug with a traceback of all goroutines. // Use it for reporting abnormal bugs encountered at runtime that should be // fixed. // -// Will print out the full Go stacktrace at time of invocation. -// // Do not use this for bad user input. Errors reported by this function should // not be fatal. -func WARN_ONCE(cond bool, s string, a ...any) { - if !cond { - return - } - msg := fmt.Sprintf(s, a...) - warnOnce(2, msg) +func BugTracebackf(s string, a ...any) { + reportBug(2, s, a) } -// WARN_ON_ONCE serves the same purpose as the Linux kernel's WARN_ON_ONCE macro. -// Use it for reporting abnormal bugs encountered at runtime that should be -// fixed. -// -// Will print out the full Go stacktrace at time of invocation. +// BugTracebackOnce will report a bug with a traceback of all goroutines if the +// error isn't nil. Use it for reporting abnormal bugs encountered at runtime +// that should be fixed. If called multiple time from same invocation, will only +// print once. // // Do not use this for bad user input. Errors reported by this function should // not be fatal. -func WARN_ON_ONCE(cond bool) { - if !cond { - return +func BugTracebackOnce(err error) { + if err != nil { + reportBugErrOnce(2, err) } - warnOnce(2, "") } -// WARN_ERR_ONCE is a more Go-friendly version of the typical Linux WARN* macros. -// If the error isn't nil, it will report the error prefixed with the standard -// WARN string. Use it for reporting abnormal bugs encountered at runtime that -// should be fixed. -// -// Will print out the full Go stacktrace at time of invocation. +// BugTracebackfOnce will report a bug with a traceback of all goroutines. +// Use it for reporting abnormal bugs encountered at runtime that should be +// fixed. If called multiple time from same invocation, will only print once. // // Do not use this for bad user input. Errors reported by this function should // not be fatal. -func WARN_ERR_ONCE(err error) { - if err == nil { - return - } - warnOnce(2, err.Error()) +func BugTracebackfOnce(s string, a ...any) { + reportBugOnce(2, s, a) } diff --git a/pkg/log/warn_on_test.go b/pkg/log/bug_test.go similarity index 50% rename from pkg/log/warn_on_test.go rename to pkg/log/bug_test.go index de755e81f6..be65535dcb 100644 --- a/pkg/log/warn_on_test.go +++ b/pkg/log/bug_test.go @@ -20,6 +20,8 @@ import ( "testing" ) +const expectFile = "pkg/log/bug_test.go" + func TestWarnOn(t *testing.T) { tw := &testWriter{} e := GoogleEmitter{Writer: &Writer{Next: tw}} @@ -33,104 +35,84 @@ func TestWarnOn(t *testing.T) { testCases := map[string]func(t *testing.T){ "testConditionControlsPrint": func(t *testing.T) { - WARN_ON(false) + BugTraceback(nil) if len(tw.lines) > 0 { - t.Errorf("WARN_ON printed when it shouldn't have") + t.Errorf("BugTraceback printed when it shouldn't have") } - WARN_ON(true) + BugTraceback(fmt.Errorf("error")) if len(tw.lines) == 0 { - t.Errorf("WARN_ON didn't print anything when it should have") + t.Errorf("BugTraceback didn't print anything when it should have") } }, "testStringFormat": func(t *testing.T) { - expectFile := "pkg/log/warn_on_test.go" - // Don't try to match the line to make this test less - // brittle to somebody accidentally sneezing on this file. - expectStr := strings.SplitN(warnFmtStr, "%", 2)[0] - - WARN_ON(true) - - if len(tw.lines) == 0 { - t.Errorf("WARN_ON didn't print anything when it should have") - } - if !strings.Contains(tw.lines[0], expectFile) { - t.Errorf("WARN_ON didn't contain expected output, expected: '%s', got: '%s'", expectFile, tw.lines[0]) - } - if !strings.Contains(tw.lines[0], expectStr) { - t.Errorf("WARN_ON didn't contain expected output, expected: '%s', got: '%s'", expectStr, tw.lines[0]) - } - }, - "testCustomFormat": func(t *testing.T) { - expectFile := "pkg/log/warn_on_test.go" expectStr1 := strings.SplitN(warnFmtStr, "%", 2)[0] expectStr2 := "This is just a test warning" - WARN(true, "This is just a test warning") + BugTracebackf("This is just a test warning: %s", "with another string") if len(tw.lines) == 0 { - t.Errorf("WARN_ON didn't print anything when it should have") + t.Errorf("BugTracebackf didn't print anything when it should have") } if !strings.Contains(tw.lines[0], expectFile) { - t.Errorf("WARN_ON didn't contain expected output, expected: '%s', got: '%s'", expectFile, tw.lines[0]) + t.Errorf("BugTracebackf didn't contain expected output, expected: '%s', got: '%s'", expectFile, tw.lines[0]) } if !strings.Contains(tw.lines[0], expectStr1) { - t.Errorf("WARN_ON didn't contain expected output, expected: '%s', got: '%s'", expectStr1, tw.lines[0]) + t.Errorf("BugTracebackf didn't contain expected output, expected: '%s', got: '%s'", expectStr1, tw.lines[0]) } if !strings.Contains(tw.lines[0], expectStr2) { - t.Errorf("WARN_ON didn't contain expected output, expected: '%s', got: '%s'", expectStr2, tw.lines[0]) + t.Errorf("BugTracebackf didn't contain expected output, expected: '%s', got: '%s'", expectStr2, tw.lines[0]) } }, "testWarnErr": func(t *testing.T) { - expectFile := "pkg/log/warn_on_test.go" expectStr1 := strings.SplitN(warnFmtStr, "%", 2)[0] expectStr2 := "My little error string" var err error - WARN_ERR(err) + BugTraceback(err) if len(tw.lines) > 0 { - t.Errorf("WARN_ON printed when it shouldn't have") + t.Errorf("BugTraceback printed when it shouldn't have") } err = fmt.Errorf("My little error string") - WARN_ERR(err) + BugTraceback(err) if len(tw.lines) == 0 { - t.Errorf("WARN_ON didn't print anything when it should have") + t.Errorf("BugTraceback didn't print anything when it should have") } if !strings.Contains(tw.lines[0], expectFile) { - t.Errorf("WARN_ON didn't contain expected output, expected: '%s', got: '%s'", expectFile, tw.lines[0]) + t.Errorf("BugTraceback didn't contain expected output, expected: '%s', got: '%s'", expectFile, tw.lines[0]) } if !strings.Contains(tw.lines[0], expectStr1) { - t.Errorf("WARN_ON didn't contain expected output, expected: '%s', got: '%s'", expectStr1, tw.lines[0]) + t.Errorf("BugTraceback didn't contain expected output, expected: '%s', got: '%s'", expectStr1, tw.lines[0]) } if !strings.Contains(tw.lines[0], expectStr2) { - t.Errorf("WARN_ON didn't contain expected output, expected: '%s', got: '%s'", expectStr2, tw.lines[0]) + t.Errorf("BugTraceback didn't contain expected output, expected: '%s', got: '%s'", expectStr2, tw.lines[0]) } }, "testWarnOnceOnlyPrintsOnce": func(t *testing.T) { testHelperFunc := func() { - WARN_ON_ONCE(true) + BugTracebackfOnce("error") } testHelperFunc() if len(tw.lines) == 0 { - t.Errorf("WarnOnOnce didn't print anything when it should have") + t.Errorf("BugTracebackfOnce didn't print anything when it should have") } tw.clear() testHelperFunc() if len(tw.lines) > 0 { - t.Errorf("WarnOnOnce printed out a warning a second time when it shouldn't have") + t.Errorf("BugTracebackfOnce printed out a warning a second time when it shouldn't have") } }, "testWarnOnceDoesntClobberOthers": func(t *testing.T) { - WARN_ON_ONCE(true) + BugTracebackfOnce("error") if len(tw.lines) == 0 { - t.Errorf("First WarnOnOnce didn't print anything when it should have") + t.Errorf("First BugTracebackfOnce didn't print anything when it should have") } tw.clear() - WARN_ON_ONCE(true) + BugTracebackfOnce("error") if len(tw.lines) == 0 { - t.Errorf("Second WarnOnOnce didn't print anything when it should have") + t.Errorf("Second BugTracebackfOnce didn't print anything when it should have") } }, }