diff --git a/core.go b/core.go index bcb7660..9411d53 100644 --- a/core.go +++ b/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() diff --git a/error.go b/error.go new file mode 100644 index 0000000..5531f2e --- /dev/null +++ b/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() +} diff --git a/error_test.go b/error_test.go new file mode 100644 index 0000000..d95f7e2 --- /dev/null +++ b/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: +}