From 34453fea52c683551166c48ef219b6d5213ddad8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emir=20Ribi=C4=87?= Date: Sat, 21 Dec 2024 23:37:02 +0100 Subject: [PATCH 1/4] simplify logrus --- logrus/logrusentry.go | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/logrus/logrusentry.go b/logrus/logrusentry.go index d95358655..0fa75426c 100644 --- a/logrus/logrusentry.go +++ b/logrus/logrusentry.go @@ -156,34 +156,40 @@ func (h *Hook) entryToEvent(l *logrus.Entry) *sentry.Event { Timestamp: l.Time, Logger: name, } + key := h.key(FieldRequest) if req, ok := s.Extra[key].(*http.Request); ok { delete(s.Extra, key) s.Request = sentry.NewRequest(req) } + if err, ok := s.Extra[logrus.ErrorKey].(error); ok { delete(s.Extra, logrus.ErrorKey) s.SetException(err, -1) } + key = h.key(FieldUser) - if user, ok := s.Extra[key].(sentry.User); ok { + switch user := s.Extra[key].(type) { + case sentry.User: delete(s.Extra, key) s.User = user - } - if user, ok := s.Extra[key].(*sentry.User); ok { + case *sentry.User: delete(s.Extra, key) s.User = *user } + key = h.key(FieldTransaction) if txn, ok := s.Extra[key].(string); ok { delete(s.Extra, key) s.Transaction = txn } + key = h.key(FieldFingerprint) if fp, ok := s.Extra[key].([]string); ok { delete(s.Extra, key) s.Fingerprint = fp } + delete(s.Extra, FieldGoVersion) delete(s.Extra, FieldMaxProcs) return s From 6dc19293766f756bc75b110830a2dead3032cddb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emir=20Ribi=C4=87?= Date: Sun, 22 Dec 2024 11:44:44 +0100 Subject: [PATCH 2/4] add SetHubProvider to sentrylogrus --- CHANGELOG.md | 20 +++++ logrus/logrusentry.go | 32 +++++--- logrus/logrusentry_test.go | 152 +++++++++---------------------------- 3 files changed, 75 insertions(+), 129 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 25f6b7237..9c7603562 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,26 @@ Add ability to override `hub` in `context` for integrations that use custom context ([#931](https://github.com/getsentry/sentry-go/pull/931)) +- Add `HubProvider` Hook for `sentrylogrus`, enabling dynamic Sentry hub allocation for each log entry or goroutine. This change enhances compatibility with Sentry's recommendation of using separate hubs per goroutine. + +To ensure a separate Sentry hub for each goroutine, configure the `HubProvider` like this: + +```go +hook, err := sentrylogrus.New(nil, sentry.ClientOptions{}) +if err != nil { + log.Fatalf("Failed to initialize Sentry hook: %v", err) +} + +// Set a custom HubProvider to generate a new hub for each goroutine or log entry +hook.SetHubProvider(func() *sentry.Hub { + client, _ := sentry.NewClient(sentry.ClientOptions{}) + return sentry.NewHub(client, sentry.NewScope()) +}) + +logrus.AddHook(hook) +``` + + ## 0.30.0 The Sentry SDK team is happy to announce the immediate availability of Sentry Go SDK v0.30.0. diff --git a/logrus/logrusentry.go b/logrus/logrusentry.go index 0fa75426c..1775ffe33 100644 --- a/logrus/logrusentry.go +++ b/logrus/logrusentry.go @@ -42,10 +42,10 @@ const ( // It is not safe to configure the hook while logging is happening. Please // perform all configuration before using it. type Hook struct { - hub *sentry.Hub - fallback FallbackFunc - keys map[string]string - levels []logrus.Level + hubProvider func() *sentry.Hub + fallback FallbackFunc + keys map[string]string + levels []logrus.Level } var _ logrus.Hook = &Hook{} @@ -66,17 +66,26 @@ func New(levels []logrus.Level, opts sentry.ClientOptions) (*Hook, error) { // NewFromClient initializes a new Logrus hook which sends logs to the provided // sentry client. func NewFromClient(levels []logrus.Level, client *sentry.Client) *Hook { - h := &Hook{ + defaultHub := sentry.NewHub(client, sentry.NewScope()) + return &Hook{ levels: levels, - hub: sentry.NewHub(client, sentry.NewScope()), - keys: make(map[string]string), + hubProvider: func() *sentry.Hub { + // Default to using the same hub if no specific provider is set + return defaultHub + }, + keys: make(map[string]string), } - return h +} + +// SetHubProvider sets a function to provide a hub for each log entry. +// This can be used to ensure separate hubs per goroutine if needed. +func (h *Hook) SetHubProvider(provider func() *sentry.Hub) { + h.hubProvider = provider } // AddTags adds tags to the hook's scope. func (h *Hook) AddTags(tags map[string]string) { - h.hub.Scope().SetTags(tags) + h.hubProvider().Scope().SetTags(tags) } // A FallbackFunc can be used to attempt to handle any errors in logging, before @@ -124,8 +133,9 @@ func (h *Hook) Levels() []logrus.Level { // Fire sends entry to Sentry. func (h *Hook) Fire(entry *logrus.Entry) error { + hub := h.hubProvider() // Use the hub provided by the HubProvider event := h.entryToEvent(entry) - if id := h.hub.CaptureEvent(event); id == nil { + if id := hub.CaptureEvent(event); id == nil { if h.fallback != nil { return h.fallback(entry) } @@ -199,5 +209,5 @@ func (h *Hook) entryToEvent(l *logrus.Entry) *sentry.Event { // 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. func (h *Hook) Flush(timeout time.Duration) bool { - return h.hub.Client().Flush(timeout) + return h.hubProvider().Client().Flush(timeout) } diff --git a/logrus/logrusentry_test.go b/logrus/logrusentry_test.go index de68e16d3..350cd4f20 100644 --- a/logrus/logrusentry_test.go +++ b/logrus/logrusentry_test.go @@ -1,7 +1,6 @@ package sentrylogrus import ( - "errors" "net/http" "net/http/httptest" "strings" @@ -10,7 +9,6 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" - pkgerr "github.com/pkg/errors" "github.com/sirupsen/logrus" "github.com/getsentry/sentry-go" @@ -33,15 +31,39 @@ func TestNew(t *testing.T) { if err != nil { t.Fatal(err) } - if id := h.hub.CaptureEvent(&sentry.Event{}); id == nil { + if id := h.hubProvider().CaptureEvent(&sentry.Event{}); id == nil { t.Error("CaptureEvent failed") } - if !h.Flush(testutils.FlushTimeout()) { + if !h.hubProvider().Client().Flush(testutils.FlushTimeout()) { t.Error("flush failed") } }) } +func TestSetHubProvider(t *testing.T) { + t.Parallel() + + h, err := New(nil, sentry.ClientOptions{}) + if err != nil { + t.Fatal(err) + } + + // Custom HubProvider to ensure separate hubs for each test + h.SetHubProvider(func() *sentry.Hub { + client, _ := sentry.NewClient(sentry.ClientOptions{}) + return sentry.NewHub(client, sentry.NewScope()) + }) + + entry := &logrus.Entry{Level: logrus.ErrorLevel} + if err := h.Fire(entry); err != nil { + t.Fatal(err) + } + + if !h.hubProvider().Client().Flush(testutils.FlushTimeout()) { + t.Error("flush failed") + } +} + func TestFire(t *testing.T) { t.Parallel() @@ -54,12 +76,13 @@ func TestFire(t *testing.T) { if err != nil { t.Fatal(err) } + err = hook.Fire(entry) if err != nil { t.Fatal(err) } - if !hook.Flush(testutils.FlushTimeout()) { + if !hook.hubProvider().Client().Flush(testutils.FlushTimeout()) { t.Error("flush failed") } } @@ -140,119 +163,6 @@ func Test_entryToEvent(t *testing.T) { Logger: "logrus", }, }, - "error": { - entry: &logrus.Entry{ - Data: map[string]any{ - logrus.ErrorKey: errors.New("things failed"), - }, - }, - want: &sentry.Event{ - Level: "fatal", - Extra: map[string]any{}, - Exception: []sentry.Exception{ - {Type: "*errors.errorString", Value: "things failed", Stacktrace: &sentry.Stacktrace{Frames: []sentry.Frame{}}}, - }, - Logger: "logrus", - }, - }, - "non-error": { - entry: &logrus.Entry{ - Data: map[string]any{ - logrus.ErrorKey: "this isn't really an error", - }, - }, - want: &sentry.Event{ - Level: "fatal", - Extra: map[string]any{ - "error": "this isn't really an error", - }, - Logger: "logrus", - }, - }, - "error with stack trace": { - entry: &logrus.Entry{ - Data: map[string]any{ - logrus.ErrorKey: pkgerr.WithStack(errors.New("failure")), - }, - }, - want: &sentry.Event{ - Level: "fatal", - Extra: map[string]any{}, - Exception: []sentry.Exception{ - { - Type: "*errors.errorString", - Value: "failure", - Mechanism: &sentry.Mechanism{ - ExceptionID: 0, - IsExceptionGroup: true, - Type: "generic", - }, - }, - { - Type: "*errors.withStack", - Value: "failure", - Stacktrace: &sentry.Stacktrace{ - Frames: []sentry.Frame{}, - }, - Mechanism: &sentry.Mechanism{ - ExceptionID: 1, - IsExceptionGroup: true, - ParentID: sentry.Pointer(0), - Type: "generic", - }, - }, - }, - Logger: "logrus", - }, - }, - "user": { - entry: &logrus.Entry{ - Data: map[string]any{ - FieldUser: sentry.User{ - ID: "bob", - }, - }, - }, - want: &sentry.Event{ - Level: "fatal", - Extra: map[string]any{}, - User: sentry.User{ - ID: "bob", - }, - Logger: "logrus", - }, - }, - "user pointer": { - entry: &logrus.Entry{ - Data: map[string]any{ - FieldUser: &sentry.User{ - ID: "alice", - }, - }, - }, - want: &sentry.Event{ - Level: "fatal", - Extra: map[string]any{}, - User: sentry.User{ - ID: "alice", - }, - Logger: "logrus", - }, - }, - "non-user": { - entry: &logrus.Entry{ - Data: map[string]any{ - FieldUser: "just say no to drugs", - }, - }, - want: &sentry.Event{ - Level: "fatal", - Extra: map[string]any{ - "user": "just say no to drugs", - }, - Logger: "logrus", - }, - }, } h, err := New(nil, sentry.ClientOptions{ @@ -262,6 +172,12 @@ func Test_entryToEvent(t *testing.T) { t.Fatal(err) } + // Custom HubProvider for test environment + h.SetHubProvider(func() *sentry.Hub { + client, _ := sentry.NewClient(sentry.ClientOptions{}) + return sentry.NewHub(client, sentry.NewScope()) + }) + for name, tt := range tests { tt := tt t.Run(name, func(t *testing.T) { From 6305e23dfc271bdc3580f46a8efff9fd9ae64676 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emir=20Ribi=C4=87?= Date: Sun, 22 Dec 2024 11:46:13 +0100 Subject: [PATCH 3/4] readd tests --- logrus/logrusentry_test.go | 116 +++++++++++++++++++++++++++++++++++++ 1 file changed, 116 insertions(+) diff --git a/logrus/logrusentry_test.go b/logrus/logrusentry_test.go index 350cd4f20..d44af1bc2 100644 --- a/logrus/logrusentry_test.go +++ b/logrus/logrusentry_test.go @@ -1,12 +1,15 @@ package sentrylogrus import ( + "errors" "net/http" "net/http/httptest" "strings" "testing" "time" + pkgerr "github.com/pkg/errors" + "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" "github.com/sirupsen/logrus" @@ -163,6 +166,119 @@ func Test_entryToEvent(t *testing.T) { Logger: "logrus", }, }, + "error": { + entry: &logrus.Entry{ + Data: map[string]any{ + logrus.ErrorKey: errors.New("things failed"), + }, + }, + want: &sentry.Event{ + Level: "fatal", + Extra: map[string]any{}, + Exception: []sentry.Exception{ + {Type: "*errors.errorString", Value: "things failed", Stacktrace: &sentry.Stacktrace{Frames: []sentry.Frame{}}}, + }, + Logger: "logrus", + }, + }, + "non-error": { + entry: &logrus.Entry{ + Data: map[string]any{ + logrus.ErrorKey: "this isn't really an error", + }, + }, + want: &sentry.Event{ + Level: "fatal", + Extra: map[string]any{ + "error": "this isn't really an error", + }, + Logger: "logrus", + }, + }, + "error with stack trace": { + entry: &logrus.Entry{ + Data: map[string]any{ + logrus.ErrorKey: pkgerr.WithStack(errors.New("failure")), + }, + }, + want: &sentry.Event{ + Level: "fatal", + Extra: map[string]any{}, + Exception: []sentry.Exception{ + { + Type: "*errors.errorString", + Value: "failure", + Mechanism: &sentry.Mechanism{ + ExceptionID: 0, + IsExceptionGroup: true, + Type: "generic", + }, + }, + { + Type: "*errors.withStack", + Value: "failure", + Stacktrace: &sentry.Stacktrace{ + Frames: []sentry.Frame{}, + }, + Mechanism: &sentry.Mechanism{ + ExceptionID: 1, + IsExceptionGroup: true, + ParentID: sentry.Pointer(0), + Type: "generic", + }, + }, + }, + Logger: "logrus", + }, + }, + "user": { + entry: &logrus.Entry{ + Data: map[string]any{ + FieldUser: sentry.User{ + ID: "bob", + }, + }, + }, + want: &sentry.Event{ + Level: "fatal", + Extra: map[string]any{}, + User: sentry.User{ + ID: "bob", + }, + Logger: "logrus", + }, + }, + "user pointer": { + entry: &logrus.Entry{ + Data: map[string]any{ + FieldUser: &sentry.User{ + ID: "alice", + }, + }, + }, + want: &sentry.Event{ + Level: "fatal", + Extra: map[string]any{}, + User: sentry.User{ + ID: "alice", + }, + Logger: "logrus", + }, + }, + "non-user": { + entry: &logrus.Entry{ + Data: map[string]any{ + FieldUser: "just say no to drugs", + }, + }, + want: &sentry.Event{ + Level: "fatal", + Extra: map[string]any{ + "user": "just say no to drugs", + }, + Logger: "logrus", + }, + }, } h, err := New(nil, sentry.ClientOptions{ From 1e902e7259b29856a95387b47d62b869b378afd5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emir=20Ribi=C4=87?= Date: Sun, 22 Dec 2024 11:48:55 +0100 Subject: [PATCH 4/4] update changelog (add pr link) --- CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9c7603562..03f73f5bc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,9 +13,9 @@ Add ability to override `hub` in `context` for integrations that use custom context ([#931](https://github.com/getsentry/sentry-go/pull/931)) -- Add `HubProvider` Hook for `sentrylogrus`, enabling dynamic Sentry hub allocation for each log entry or goroutine. This change enhances compatibility with Sentry's recommendation of using separate hubs per goroutine. +- Add `HubProvider` Hook for `sentrylogrus`, enabling dynamic Sentry hub allocation for each log entry or goroutine. ([#936](https://github.com/getsentry/sentry-go/pull/936)) -To ensure a separate Sentry hub for each goroutine, configure the `HubProvider` like this: +This change enhances compatibility with Sentry's recommendation of using separate hubs per goroutine. To ensure a separate Sentry hub for each goroutine, configure the `HubProvider` like this: ```go hook, err := sentrylogrus.New(nil, sentry.ClientOptions{})