Merge pull request #4826 from tinyspeck/log-error-stacks-optional

make the logging of stacks in errors an opt-in behavior

go/vt/vterrors/errors_test.go

@@ -172,15 +172,16 @@ func TestStackFormat(t *testing.T) {
 	err := outer()
 	got := fmt.Sprintf("%v", err)
 
-	assertStringContains(t, got, "innerMost")
-	assertStringContains(t, got, "middle")
-	assertStringContains(t, got, "outer")
-}
+	assertContains(t, got, "innerMost", false)
+	assertContains(t, got, "middle", false)
+	assertContains(t, got, "outer", false)
 
-func assertStringContains(t *testing.T, s, substring string) {
-	if !strings.Contains(s, substring) {
-		t.Errorf("string did not contain `%v`: \n %v", substring, s)
-	}
+	LogErrStacks = true
+	defer func() { LogErrStacks = false }()
+	got = fmt.Sprintf("%v", err)
+	assertContains(t, got, "innerMost", true)
+	assertContains(t, got, "middle", true)
+	assertContains(t, got, "outer", true)
 }
 
 // errors.New, etc values are not expected to be compared by value
@@ -256,18 +257,29 @@ func TestWrapping(t *testing.T) {
 	err1 := Errorf(vtrpcpb.Code_UNAVAILABLE, "foo")
 	err2 := Wrapf(err1, "bar")
 	err3 := Wrapf(err2, "baz")
+	errorWithoutStack := fmt.Sprintf("%v", err3)
+
+	LogErrStacks = true
 	errorWithStack := fmt.Sprintf("%v", err3)
+	LogErrStacks = false
 
 	assertEquals(t, err3.Error(), "baz: bar: foo")
-	assertContains(t, errorWithStack, "foo")
-	assertContains(t, errorWithStack, "bar")
-	assertContains(t, errorWithStack, "baz")
-	assertContains(t, errorWithStack, "TestWrapping")
+	assertContains(t, errorWithoutStack, "foo", true)
+	assertContains(t, errorWithoutStack, "bar", true)
+	assertContains(t, errorWithoutStack, "baz", true)
+	assertContains(t, errorWithoutStack, "TestWrapping", false)
+
+	assertContains(t, errorWithStack, "foo", true)
+	assertContains(t, errorWithStack, "bar", true)
+	assertContains(t, errorWithStack, "baz", true)
+	assertContains(t, errorWithStack, "TestWrapping", true)
+
 }
 
-func assertContains(t *testing.T, s, substring string) {
-	if !strings.Contains(s, substring) {
-		t.Fatalf("expected string that contains [%s] but got [%s]", substring, s)
+func assertContains(t *testing.T, s, substring string, contains bool) {
+	t.Helper()
+	if doesContain := strings.Contains(s, substring); doesContain != contains {
+		t.Errorf("string `%v` contains `%v`: %v, want %v", s, substring, doesContain, contains)
 	}
 }
 
@@ -275,4 +287,4 @@ func assertEquals(t *testing.T, a, b interface{}) {
 	if a != b {
 		t.Fatalf("expected [%s] to be equal to [%s]", a, b)
 	}
-}
+}

go/vt/vterrors/vterrors.go

@@ -70,6 +70,7 @@
 package vterrors
 
 import (
+	"flag"
 	"fmt"
 	"io"
 
@@ -77,6 +78,14 @@ import (
 	vtrpcpb "vitess.io/vitess/go/vt/proto/vtrpc"
 )
 
+// LogErrStacks controls whether or not printing errors includes the
+// embedded stack trace in the output.
+var LogErrStacks bool
+
+func init() {
+	flag.BoolVar(&LogErrStacks, "LogErrStacks", false, "log stack traces in errors")
+}
+
 // New returns an error with the supplied message.
 // New also records the stack trace at the point it was called.
 func New(code vtrpcpb.Code, message string) error {
@@ -122,7 +131,9 @@ func (f *fundamental) Format(s fmt.State, verb rune) {
 	case 'v':
 		panicIfError(io.WriteString(s, "Code: "+f.code.String()+"\n"))
 		panicIfError(io.WriteString(s, f.msg+"\n"))
-		f.stack.Format(s, verb)
+		if LogErrStacks {
+			f.stack.Format(s, verb)
+		}
 		return
 	case 's':
 		panicIfError(io.WriteString(s, f.msg))
@@ -198,7 +209,9 @@ func (w *wrapping) Format(s fmt.State, verb rune) {
 	if rune('v') == verb {
 		panicIfError(fmt.Fprintf(s, "%v\n", w.Cause()))
 		panicIfError(io.WriteString(s, w.msg))
-		w.stack.Format(s, verb)
+		if LogErrStacks {
+			w.stack.Format(s, verb)
+		}
 		return
 	}