Better error stacktraces for Cloud Errors

core.go

@@ -15,6 +15,10 @@ type driverConfig struct {
 
 	// ServiceName is added as `ServiceContext()` to all logs when set
 	ServiceName string
+
+	// Correct stack traces for errors logged with zap.Error() so that
+	// they get formatted correctly in stackdriver
+	SkipFmtStackTraces bool
 }
 
 // Core is a zapdriver specific core wrapped around the default zap core. It
@@ -50,6 +54,14 @@ func ReportAllErrors(report bool) func(*core) {
 	}
 }
 
+// zapdriver core option to enable outputting stack traces compatible with
+// stackdriver when set to true
+func SkipFmtStackTraces(skipFmt bool) func(*core) {
+	return func(c *core) {
+		c.config.SkipFmtStackTraces = skipFmt
+	}
+}
+
 // zapdriver core option to add `ServiceContext()` to all logs with `name` as
 // service name
 func ServiceName(name string) func(*core) {
@@ -135,11 +147,38 @@ func (c *core) Write(ent zapcore.Entry, fields []zapcore.Field) error {
 		}
 	}
 
+	if !c.config.SkipFmtStackTraces {
+		// only improve the stacktrace if the error is reported to stackdriver
+		reported, errorField := reportedError(fields)
+		if reported && errorField != nil {
+			// remove stackdriver-incompatible zap stack trace
+			ent.Stack = ""
+			errorField.Key = "exception"
+			errorField.Interface = stackdriverFmtError{errorField.Interface.(error)}
+		}
+	}
+
 	c.tempLabels.reset()
 
 	return c.Core.Write(ent, fields)
 }
 
+func reportedError(fields []zapcore.Field) (reported bool, field *zapcore.Field) {
+	var errorField int = -1
+	for i, field := range fields {
+		if field.Key == contextKey {
+			reported = true
+		}
+		if field.Type == zapcore.ErrorType {
+			errorField = i
+		}
+	}
+	if errorField >= 0 {
+		return reported, &fields[errorField]
+	}
+	return reported, nil
+}
+
 // Sync flushes buffered logs (if any).
 func (c *core) Sync() error {
 	return c.Core.Sync()

error.go

@@ -0,0 +1,45 @@
+package zapdriver
+
+import (
+	"bytes"
+	"fmt"
+	"runtime"
+
+	"github.com/pkg/errors"
+)
+
+type stackdriverFmtError struct{ err error }
+
+type stackTracer interface {
+	StackTrace() errors.StackTrace
+}
+
+// see https://github.com/googleapis/google-cloud-go/issues/1084#issuecomment-474565019
+// this is a hack to get stackdriver to parse the stacktrace
+func (e stackdriverFmtError) Error() string {
+	if e.err == nil {
+		return ""
+	}
+	stackTrace, ok := errors.Cause(e.err).(stackTracer)
+	if !ok {
+		stackTrace, ok = e.err.(stackTracer)
+	}
+	if ok {
+		buf := bytes.NewBufferString(e.err.Error())
+		// routine id and state aren't available in pure go, so we hard-coded these
+		// required for stackdrivers runtime.Stack() format parsing
+		buf.WriteString("\n\ngoroutine 1 [running]:")
+		for _, frame := range stackTrace.StackTrace() {
+			buf.WriteByte('\n')
+
+			pc := uintptr(frame) - 1
+			fn := runtime.FuncForPC(pc)
+			if fn != nil {
+				file, line := fn.FileLine(pc)
+				buf.WriteString(fmt.Sprintf("%s()\n\t%s:%d +%#x", fn.Name(), file, line, fn.Entry()))
+			}
+		}
+		return buf.String()
+	}
+	return e.err.Error()
+}

error_test.go

@@ -0,0 +1,61 @@
+package zapdriver
+
+import (
+	"os"
+	"regexp"
+	"runtime"
+	"strings"
+	"testing"
+
+	"github.com/pkg/errors"
+	"github.com/stretchr/testify/assert"
+	"go.uber.org/zap"
+)
+
+type fakeErr struct{}
+
+// manually set the frames to allow asserting stacktraces
+func (fakeErr) StackTrace() errors.StackTrace {
+	pc1, _, _, _ := runtime.Caller(0)
+	pc2, _, _, _ := runtime.Caller(0)
+	return []errors.Frame{
+		errors.Frame(pc1),
+		errors.Frame(pc2),
+	}
+}
+func (fakeErr) Error() string {
+	return "fake error: underlying error"
+}
+
+func TestFmtStack(t *testing.T) {
+	stacktrace := stackdriverFmtError{fakeErr{}}.Error()
+	assert.Equal(t, `fake error: underlying error
+
+goroutine 1 [running]:
+github.com/blendle/zapdriver.fakeErr.StackTrace()
+	/error_test.go:18 +0x1337
+github.com/blendle/zapdriver.fakeErr.StackTrace()
+	/error_test.go:19 +0x1337`, makeStackTraceStable(stacktrace))
+}
+
+// cleanup local paths & local function pointers
+func makeStackTraceStable(str string) string {
+	re := regexp.MustCompile(`(?m)^\t.+(\/\S+:\d+) \+0x.+$`)
+	str = re.ReplaceAllString(str, "\t${1} +0x1337")
+	dir, _ := os.Getwd()
+	str = strings.ReplaceAll(str, dir, "")
+	return str
+}
+
+func ExampleSkipFmtStackTraces() {
+	logger, _ := NewProduction()
+	logger.Error("with exception", zap.Error(errors.New("internal error")), ErrorReport(runtime.Caller(0)))
+
+	logger, _ = NewProduction(WrapCore(ServiceName("service"), ReportAllErrors(true)))
+	logger.Error("with exception", zap.Error(errors.New("internal error")))
+
+	logger, _ = NewProduction(WrapCore(ServiceName("service"), SkipFmtStackTraces(true)))
+	logger.Error("without exception", zap.Error(errors.New("internal error")))
+
+	// Output:
+}