From 89924eafffc6364ca98b1935395d9ac922fd1b7e Mon Sep 17 00:00:00 2001 From: heng zhang Date: Wed, 3 Apr 2024 06:13:59 +0800 Subject: [PATCH] fix: missing stack trace for parsing error in logrusentry (#689) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: missing stack trace for parsing error in logrusentry (#677) * add changes from RFC 0079 (parent_id, exception_id, is_exception_group) * use SetException in logrus --------- Co-authored-by: Emir Ribić --- CHANGELOG.md | 15 +++ interfaces.go | 5 +- logrus/logrusentry.go | 40 +------- logrus/logrusentry_test.go | 193 ++++++++++++------------------------- 4 files changed, 82 insertions(+), 171 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4d2670722..5951037e9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,20 @@ # Changelog +## Unreleased + +### Features + +- Accept `interface{}` for span data values ([#784](https://github.com/getsentry/sentry-go/pull/784)) +- Automatic transactions for Echo integration ([#722](https://github.com/getsentry/sentry-go/pull/722)) +- Add `http.request.method` attribute for performance span data ([#786](https://github.com/getsentry/sentry-go/pull/786)) +- Automatic transactions for Fasthttp integration ([#732](https://github.com/getsentry/sentry-go/pull/723)) +- Add `Fiber` integration ([#795](https://github.com/getsentry/sentry-go/pull/795)) +- Use `errors.Unwrap()` to create exception groups ([#792](https://github.com/getsentry/sentry-go/pull/792)) + +### Fixes + +- Fix missing stack trace for parsing error in logrusentry ([#689](https://github.com/getsentry/sentry-go/pull/689)) + ## 0.27.0 The Sentry SDK team is happy to announce the immediate availability of Sentry Go SDK v0.27.0. diff --git a/interfaces.go b/interfaces.go index a9b2f0222..0858bcef5 100644 --- a/interfaces.go +++ b/interfaces.go @@ -338,7 +338,8 @@ type Event struct { // SetException appends the unwrapped errors to the event's exception list. // // maxErrorDepth is the maximum depth of the error chain we will look -// into while unwrapping the errors. +// into while unwrapping the errors. If maxErrorDepth is -1, we will +// unwrap all errors in the chain. func (e *Event) SetException(exception error, maxErrorDepth int) { if exception == nil { return @@ -346,7 +347,7 @@ func (e *Event) SetException(exception error, maxErrorDepth int) { err := exception - for i := 0; err != nil && i < maxErrorDepth; i++ { + for i := 0; err != nil && (i < maxErrorDepth || maxErrorDepth == -1); i++ { // Add the current error to the exception slice with its details e.Exception = append(e.Exception, Exception{ Value: err.Error(), diff --git a/logrus/logrusentry.go b/logrus/logrusentry.go index b4695f0c9..f9d921651 100644 --- a/logrus/logrusentry.go +++ b/logrus/logrusentry.go @@ -6,8 +6,9 @@ import ( "net/http" "time" - sentry "github.com/getsentry/sentry-go" "github.com/sirupsen/logrus" + + sentry "github.com/getsentry/sentry-go" ) // The identifier of the Logrus SDK. @@ -160,8 +161,7 @@ func (h *Hook) entryToEvent(l *logrus.Entry) *sentry.Event { } if err, ok := s.Extra[logrus.ErrorKey].(error); ok { delete(s.Extra, logrus.ErrorKey) - ex := h.exceptions(err) - s.Exception = ex + s.SetException(err, -1) } key = h.key(FieldUser) if user, ok := s.Extra[key].(sentry.User); ok { @@ -187,40 +187,6 @@ func (h *Hook) entryToEvent(l *logrus.Entry) *sentry.Event { return s } -func (h *Hook) exceptions(err error) []sentry.Exception { - if !h.hub.Client().Options().AttachStacktrace { - return []sentry.Exception{{ - Type: "error", - Value: err.Error(), - }} - } - excs := []sentry.Exception{} - var last *sentry.Exception - for ; err != nil; err = errors.Unwrap(err) { - exc := sentry.Exception{ - Type: "error", - Value: err.Error(), - Stacktrace: sentry.ExtractStacktrace(err), - } - if last != nil && exc.Value == last.Value { - if last.Stacktrace == nil { - last.Stacktrace = exc.Stacktrace - continue - } - if exc.Stacktrace == nil { - continue - } - } - excs = append(excs, exc) - last = &excs[len(excs)-1] - } - // reverse - for i, j := 0, len(excs)-1; i < j; i, j = i+1, j-1 { - excs[i], excs[j] = excs[j], excs[i] - } - return excs -} - // Flush waits until the underlying Sentry transport sends any buffered events, // blocking for at most the given timeout. It returns false if the timeout was // reached, in which case some events may not have been sent. diff --git a/logrus/logrusentry_test.go b/logrus/logrusentry_test.go index afa532deb..1dbe6f47c 100644 --- a/logrus/logrusentry_test.go +++ b/logrus/logrusentry_test.go @@ -2,7 +2,6 @@ package sentrylogrus import ( "errors" - "fmt" "net/http" "net/http/httptest" "strings" @@ -67,74 +66,67 @@ func TestFire(t *testing.T) { func Test_entryToEvent(t *testing.T) { t.Parallel() - tests := []struct { - name string + tests := map[string]struct { entry *logrus.Entry want *sentry.Event }{ - { - name: "empty entry", + "empty entry": { entry: &logrus.Entry{}, want: &sentry.Event{ Level: "fatal", - Extra: map[string]interface{}{}, + Extra: map[string]any{}, }, }, - { - name: "data fields", + "data fields": { entry: &logrus.Entry{ - Data: map[string]interface{}{ + Data: map[string]any{ "foo": 123.4, "bar": "oink", }, }, want: &sentry.Event{ Level: "fatal", - Extra: map[string]interface{}{"bar": "oink", "foo": 123.4}, + Extra: map[string]any{"bar": "oink", "foo": 123.4}, }, }, - { - name: "info level", + "info level": { entry: &logrus.Entry{ Level: logrus.InfoLevel, }, want: &sentry.Event{ Level: "info", - Extra: map[string]interface{}{}, + Extra: map[string]any{}, }, }, - { - name: "message", + "message": { entry: &logrus.Entry{ Message: "the only thing we have to fear is fear itself", }, want: &sentry.Event{ Level: "fatal", - Extra: map[string]interface{}{}, + Extra: map[string]any{}, Message: "the only thing we have to fear is fear itself", }, }, - { - name: "timestamp", + "timestamp": { entry: &logrus.Entry{ Time: time.Unix(1, 2).UTC(), }, want: &sentry.Event{ Level: "fatal", - Extra: map[string]interface{}{}, + Extra: map[string]any{}, Timestamp: time.Unix(1, 2).UTC(), }, }, - { - name: "http request", + "http request": { entry: &logrus.Entry{ - Data: map[string]interface{}{ + Data: map[string]any{ FieldRequest: httptest.NewRequest("GET", "/", nil), }, }, want: &sentry.Event{ Level: "fatal", - Extra: map[string]interface{}{}, + Extra: map[string]any{}, Request: &sentry.Request{ URL: "http://example.com/", Method: http.MethodGet, @@ -142,54 +134,73 @@ func Test_entryToEvent(t *testing.T) { }, }, }, - { - name: "error", + "error": { entry: &logrus.Entry{ - Data: map[string]interface{}{ + Data: map[string]any{ logrus.ErrorKey: errors.New("things failed"), }, }, want: &sentry.Event{ Level: "fatal", - Extra: map[string]interface{}{}, + Extra: map[string]any{}, Exception: []sentry.Exception{ - {Type: "error", Value: "things failed"}, + {Type: "*errors.errorString", Value: "things failed", Stacktrace: &sentry.Stacktrace{Frames: []sentry.Frame{}}}, }, }, }, - { - name: "non-error", + "non-error": { entry: &logrus.Entry{ - Data: map[string]interface{}{ + Data: map[string]any{ logrus.ErrorKey: "this isn't really an error", }, }, want: &sentry.Event{ Level: "fatal", - Extra: map[string]interface{}{ + Extra: map[string]any{ "error": "this isn't really an error", }, }, }, - { - name: "error with stack trace", + "error with stack trace": { entry: &logrus.Entry{ - Data: map[string]interface{}{ + Data: map[string]any{ logrus.ErrorKey: pkgerr.WithStack(errors.New("failure")), }, }, want: &sentry.Event{ Level: "fatal", - Extra: map[string]interface{}{}, + Extra: map[string]any{}, Exception: []sentry.Exception{ - {Type: "error", Value: "failure", Stacktrace: &sentry.Stacktrace{Frames: []sentry.Frame{}}}, + { + Type: "*errors.errorString", + Value: "failure", + Mechanism: &sentry.Mechanism{ + Data: map[string]any{ + "exception_id": 0, + "is_exception_group": true, + }, + }, + }, + { + Type: "*errors.withStack", + Value: "failure", + Stacktrace: &sentry.Stacktrace{ + Frames: []sentry.Frame{}, + }, + Mechanism: &sentry.Mechanism{ + Data: map[string]any{ + "exception_id": 1, + "is_exception_group": true, + "parent_id": 0, + }, + }, + }, }, }, }, - { - name: "user", + "user": { entry: &logrus.Entry{ - Data: map[string]interface{}{ + Data: map[string]any{ FieldUser: sentry.User{ ID: "bob", }, @@ -197,16 +208,15 @@ func Test_entryToEvent(t *testing.T) { }, want: &sentry.Event{ Level: "fatal", - Extra: map[string]interface{}{}, + Extra: map[string]any{}, User: sentry.User{ ID: "bob", }, }, }, - { - name: "user pointer", + "user pointer": { entry: &logrus.Entry{ - Data: map[string]interface{}{ + Data: map[string]any{ FieldUser: &sentry.User{ ID: "alice", }, @@ -214,22 +224,21 @@ func Test_entryToEvent(t *testing.T) { }, want: &sentry.Event{ Level: "fatal", - Extra: map[string]interface{}{}, + Extra: map[string]any{}, User: sentry.User{ ID: "alice", }, }, }, - { - name: "non-user", + "non-user": { entry: &logrus.Entry{ - Data: map[string]interface{}{ + Data: map[string]any{ FieldUser: "just say no to drugs", }, }, want: &sentry.Event{ Level: "fatal", - Extra: map[string]interface{}{ + Extra: map[string]any{ "user": "just say no to drugs", }, }, @@ -243,15 +252,13 @@ func Test_entryToEvent(t *testing.T) { t.Fatal(err) } - for _, tt := range tests { + for name, tt := range tests { tt := tt - t.Run(tt.name, func(t *testing.T) { + t.Run(name, func(t *testing.T) { t.Parallel() got := h.entryToEvent(tt.entry) opts := cmp.Options{ - cmpopts.IgnoreFields(sentry.Event{}, - "sdkMetaData", - ), + cmpopts.IgnoreFields(sentry.Event{}, "sdkMetaData"), } if d := cmp.Diff(tt.want, got, opts); d != "" { t.Error(d) @@ -259,81 +266,3 @@ func Test_entryToEvent(t *testing.T) { }) } } - -func Test_exceptions(t *testing.T) { - t.Parallel() - tests := []struct { - name string - trace bool - err error - want []sentry.Exception - }{ - { - name: "std error", - trace: true, - err: errors.New("foo"), - want: []sentry.Exception{ - {Type: "error", Value: "foo"}, - }, - }, - { - name: "wrapped, no stack", - trace: true, - err: fmt.Errorf("foo: %w", errors.New("bar")), - want: []sentry.Exception{ - {Type: "error", Value: "bar"}, - {Type: "error", Value: "foo: bar"}, - }, - }, - { - name: "ignored stack", - trace: false, - err: pkgerr.New("foo"), - want: []sentry.Exception{ - {Type: "error", Value: "foo"}, - }, - }, - { - name: "stack", - trace: true, - err: pkgerr.New("foo"), - want: []sentry.Exception{ - {Type: "error", Value: "foo", Stacktrace: &sentry.Stacktrace{Frames: []sentry.Frame{}}}, - }, - }, - { - name: "multi-wrapped error", - trace: true, - err: func() error { - err := errors.New("original") - err = fmt.Errorf("fmt: %w", err) - err = pkgerr.Wrap(err, "wrap") - err = pkgerr.WithStack(err) - return fmt.Errorf("wrapped: %w", err) - }(), - want: []sentry.Exception{ - {Type: "error", Value: "original"}, - {Type: "error", Value: "fmt: original"}, - {Type: "error", Value: "wrap: fmt: original", Stacktrace: &sentry.Stacktrace{Frames: []sentry.Frame{}}}, - {Type: "error", Value: "wrap: fmt: original", Stacktrace: &sentry.Stacktrace{Frames: []sentry.Frame{}}}, - {Type: "error", Value: "wrapped: wrap: fmt: original"}, - }, - }, - } - - for _, tt := range tests { - tt := tt - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - h, err := New(nil, sentry.ClientOptions{AttachStacktrace: tt.trace}) - if err != nil { - t.Fatal(err) - } - got := h.exceptions(tt.err) - - if d := cmp.Diff(tt.want, got); d != "" { - t.Error(d) - } - }) - } -}