From fce917d848ddd6be05599995efad41eb48fe2ba8 Mon Sep 17 00:00:00 2001 From: Damiano Petrungaro Date: Sat, 10 Sep 2022 22:46:28 +0200 Subject: [PATCH] fix(sentry): use log message when capturing the event to avoid grouping unrelated errors in Sentry, we must never create a new error BREAKING CHANGE: the sentry writer has been simplified and now needs less dependencies to fully operate --- .github/release/release.json | 2 +- CHANGELOG.md | 31 +++---- cover.out.tmp | 74 --------------- sentry/sentry.go | 57 ++++++------ sentry/sentry_test.go | 172 +++++++++++++++++++++-------------- 5 files changed, 144 insertions(+), 192 deletions(-) delete mode 100644 cover.out.tmp diff --git a/.github/release/release.json b/.github/release/release.json index 4dc71a1..1447c9c 100644 --- a/.github/release/release.json +++ b/.github/release/release.json @@ -1,3 +1,3 @@ { - "version": "2.1.0" + "version": "1.11.0" } \ No newline at end of file diff --git a/CHANGELOG.md b/CHANGELOG.md index 3a8a571..1ad8889 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,51 +1,46 @@ -# [2.1.0](https://github.com/damianopetrungaro/golog/compare/v2.0.0...v2.1.0) (2022-09-10) +# [1.11.0](https://github.com/damianopetrungaro/golog/compare/v1.10.0...v1.11.0) (2022-08-22) ### Features -* **test:** add a fake logger with matcher APIs ([cbd0c72](https://github.com/damianopetrungaro/golog/commit/cbd0c72d364eb0af43b07a6967f67505dde21e8d)) +* **encoder:** Add coloured level option to text encoder ([9ae33c2](https://github.com/damianopetrungaro/golog/commit/9ae33c2c8ebc38d9a269adb86c267c7ded63af3f)) +* **encoder:** Add LevelFormatter to TextEncoder config ([f663020](https://github.com/damianopetrungaro/golog/commit/f6630206224bcd205d281185bb217be67f1eba3b)) -# [2.0.0](https://github.com/damianopetrungaro/golog/compare/v1.11.0...v2.0.0) (2022-09-10) +# [1.10.0](https://github.com/damianopetrungaro/golog/compare/v1.9.3...v1.10.0) (2022-08-04) ### Features -* add presets ([5f5dc45](https://github.com/damianopetrungaro/golog/commit/5f5dc45d07d698591a47e5e50c28bf83228853ee)) - - -### BREAKING CHANGES - -* The API for the text encoder now adds a default text encoder which adds colors +* **datadog:** add dd tracing decorator ([3101db1](https://github.com/damianopetrungaro/golog/commit/3101db1689596c59b5649141b5d7e44b353fe998)) -# [1.11.0](https://github.com/damianopetrungaro/golog/compare/v1.10.0...v1.11.0) (2022-08-22) +## [1.9.3](https://github.com/damianopetrungaro/golog/compare/v1.9.2...v1.9.3) (2022-07-28) -### Features +### Bug Fixes -* **encoder:** Add coloured level option to text encoder ([9ae33c2](https://github.com/damianopetrungaro/golog/commit/9ae33c2c8ebc38d9a269adb86c267c7ded63af3f)) -* **encoder:** Add LevelFormatter to TextEncoder config ([f663020](https://github.com/damianopetrungaro/golog/commit/f6630206224bcd205d281185bb217be67f1eba3b)) +* **http:** use underscore instead of white space in the default fields ([be87e7f](https://github.com/damianopetrungaro/golog/commit/be87e7fde3315a25f33bdddea9c55c1bac641b44)) -# [1.10.0](https://github.com/damianopetrungaro/golog/compare/v1.9.3...v1.10.0) (2022-08-04) +## [1.9.2](https://github.com/damianopetrungaro/golog/compare/v1.9.1...v1.9.2) (2022-06-30) -### Features +### Bug Fixes -* **datadog:** add dd tracing decorator ([3101db1](https://github.com/damianopetrungaro/golog/commit/3101db1689596c59b5649141b5d7e44b353fe998)) +* **sentry:** writer concurrent safe ([1519590](https://github.com/damianopetrungaro/golog/commit/15195909b0374476f1fa9075389492cb861b7c1f)) -## [1.9.3](https://github.com/damianopetrungaro/golog/compare/v1.9.2...v1.9.3) (2022-07-28) +## [1.9.1](https://github.com/damianopetrungaro/golog/compare/v1.9.0...v1.9.1) (2022-06-10) ### Bug Fixes -* **http:** use underscore instead of white space in the default fields ([be87e7f](https://github.com/damianopetrungaro/golog/commit/be87e7fde3315a25f33bdddea9c55c1bac641b44)) +* **opentelemtery:** change package name ([c8adc6f](https://github.com/damianopetrungaro/golog/commit/c8adc6f78fe65682ae5b1a5d81e879895564b607)) diff --git a/cover.out.tmp b/cover.out.tmp deleted file mode 100644 index 3b19d52..0000000 --- a/cover.out.tmp +++ /dev/null @@ -1,74 +0,0 @@ -mode: set -github.com/damianopetrungaro/golog/test/generated_mock.go:27.57,31.2 3 0 -github.com/damianopetrungaro/golog/test/generated_mock.go:34.55,36.2 1 0 -github.com/damianopetrungaro/golog/test/generated_mock.go:39.63,42.2 2 0 -github.com/damianopetrungaro/golog/test/generated_mock.go:45.78,48.2 2 0 -github.com/damianopetrungaro/golog/test/generated_mock.go:51.63,54.2 2 0 -github.com/damianopetrungaro/golog/test/generated_mock.go:57.78,60.2 2 0 -github.com/damianopetrungaro/golog/test/generated_mock.go:63.63,66.2 2 0 -github.com/damianopetrungaro/golog/test/generated_mock.go:69.78,72.2 2 0 -github.com/damianopetrungaro/golog/test/generated_mock.go:75.62,78.2 2 0 -github.com/damianopetrungaro/golog/test/generated_mock.go:81.77,84.2 2 0 -github.com/damianopetrungaro/golog/test/generated_mock.go:87.62,90.2 2 0 -github.com/damianopetrungaro/golog/test/generated_mock.go:93.77,96.2 2 0 -github.com/damianopetrungaro/golog/test/generated_mock.go:99.61,102.25 3 0 -github.com/damianopetrungaro/golog/test/generated_mock.go:105.2,107.13 3 0 -github.com/damianopetrungaro/golog/test/generated_mock.go:102.25,104.3 1 0 -github.com/damianopetrungaro/golog/test/generated_mock.go:111.74,114.2 2 0 -github.com/damianopetrungaro/golog/test/generated_mock.go:128.71,132.2 3 0 -github.com/damianopetrungaro/golog/test/generated_mock.go:135.69,137.2 1 0 -github.com/damianopetrungaro/golog/test/generated_mock.go:140.54,143.25 3 0 -github.com/damianopetrungaro/golog/test/generated_mock.go:146.2,146.35 1 0 -github.com/damianopetrungaro/golog/test/generated_mock.go:143.25,145.3 1 0 -github.com/damianopetrungaro/golog/test/generated_mock.go:150.80,153.2 2 0 -github.com/damianopetrungaro/golog/test/generated_mock.go:167.55,171.2 3 0 -github.com/damianopetrungaro/golog/test/generated_mock.go:174.53,176.2 1 0 -github.com/damianopetrungaro/golog/test/generated_mock.go:179.47,184.2 4 0 -github.com/damianopetrungaro/golog/test/generated_mock.go:187.57,190.2 2 0 -github.com/damianopetrungaro/golog/test/generated_mock.go:193.43,198.2 4 0 -github.com/damianopetrungaro/golog/test/generated_mock.go:201.56,204.2 2 0 -github.com/damianopetrungaro/golog/test/generated_mock.go:207.41,212.2 4 0 -github.com/damianopetrungaro/golog/test/generated_mock.go:215.55,218.2 2 0 -github.com/damianopetrungaro/golog/test/generated_mock.go:221.38,226.2 4 0 -github.com/damianopetrungaro/golog/test/generated_mock.go:229.57,232.2 2 0 -github.com/damianopetrungaro/golog/test/generated_mock.go:235.59,238.25 3 0 -github.com/damianopetrungaro/golog/test/generated_mock.go:241.2,243.13 3 0 -github.com/damianopetrungaro/golog/test/generated_mock.go:238.25,240.3 1 0 -github.com/damianopetrungaro/golog/test/generated_mock.go:247.73,250.2 2 0 -github.com/damianopetrungaro/golog/test/generated_mock.go:264.57,268.2 3 0 -github.com/damianopetrungaro/golog/test/generated_mock.go:271.55,273.2 1 0 -github.com/damianopetrungaro/golog/test/generated_mock.go:276.54,282.2 5 0 -github.com/damianopetrungaro/golog/test/generated_mock.go:285.72,288.2 2 0 -github.com/damianopetrungaro/golog/test/generated_mock.go:291.51,294.2 2 0 -github.com/damianopetrungaro/golog/test/generated_mock.go:297.77,300.2 2 0 -github.com/damianopetrungaro/golog/test/generated_mock.go:314.59,318.2 3 0 -github.com/damianopetrungaro/golog/test/generated_mock.go:321.57,323.2 1 0 -github.com/damianopetrungaro/golog/test/generated_mock.go:326.37,331.2 4 0 -github.com/damianopetrungaro/golog/test/generated_mock.go:334.57,337.2 2 0 -github.com/damianopetrungaro/golog/test/matcher.go:12.41,13.26 1 1 -github.com/damianopetrungaro/golog/test/matcher.go:17.2,18.9 2 1 -github.com/damianopetrungaro/golog/test/matcher.go:21.2,22.9 2 1 -github.com/damianopetrungaro/golog/test/matcher.go:26.2,26.26 1 1 -github.com/damianopetrungaro/golog/test/matcher.go:30.2,30.26 1 1 -github.com/damianopetrungaro/golog/test/matcher.go:34.2,34.26 1 1 -github.com/damianopetrungaro/golog/test/matcher.go:38.2,38.51 1 1 -github.com/damianopetrungaro/golog/test/matcher.go:13.26,15.3 1 1 -github.com/damianopetrungaro/golog/test/matcher.go:18.9,20.3 1 1 -github.com/damianopetrungaro/golog/test/matcher.go:22.9,24.3 1 1 -github.com/damianopetrungaro/golog/test/matcher.go:26.26,28.3 1 1 -github.com/damianopetrungaro/golog/test/matcher.go:30.26,32.3 1 1 -github.com/damianopetrungaro/golog/test/matcher.go:34.26,36.3 1 1 -github.com/damianopetrungaro/golog/test/matcher.go:41.48,42.26 1 1 -github.com/damianopetrungaro/golog/test/matcher.go:46.2,46.25 1 1 -github.com/damianopetrungaro/golog/test/matcher.go:51.2,51.12 1 1 -github.com/damianopetrungaro/golog/test/matcher.go:42.26,44.3 1 1 -github.com/damianopetrungaro/golog/test/matcher.go:46.25,47.37 1 1 -github.com/damianopetrungaro/golog/test/matcher.go:47.37,49.4 1 1 -github.com/damianopetrungaro/golog/test/null.go:12.35,14.2 1 1 -github.com/damianopetrungaro/golog/test/null.go:19.49,21.2 1 1 -github.com/damianopetrungaro/golog/test/null.go:25.45,27.2 1 1 -github.com/damianopetrungaro/golog/test/null.go:32.54,34.2 1 1 -github.com/damianopetrungaro/golog/test/null.go:36.41,38.2 1 1 -github.com/damianopetrungaro/golog/test/null.go:47.36,49.2 1 1 -github.com/damianopetrungaro/golog/test/null.go:52.49,57.2 3 1 -github.com/damianopetrungaro/golog/test/null.go:60.54,63.2 2 1 diff --git a/sentry/sentry.go b/sentry/sentry.go index e90408d..84204f8 100644 --- a/sentry/sentry.go +++ b/sentry/sentry.go @@ -1,9 +1,7 @@ package sentry import ( - "bytes" - "errors" - "fmt" + "context" "github.com/getsentry/sentry-go" @@ -11,43 +9,42 @@ import ( ) type Writer struct { - Encoder golog.Encoder - Hub *sentry.Hub - ErrHandler golog.ErrorHandler - DefaultLevel golog.Level - CaptureExceptionFromLevel golog.Level + Hub *sentry.Hub + DefaultLevel golog.Level } func (w *Writer) WriteEntry(e golog.Entry) { - wTo, err := w.Encoder.Encode(e) - if err != nil { - w.ErrHandler(fmt.Errorf("%w: sentry writer on encoding: %s", golog.ErrEntryNotWritten, err)) - return - } - - buf := &bytes.Buffer{} - if _, err := wTo.WriteTo(buf); err != nil { - w.ErrHandler(fmt.Errorf("%w: sentry writer on write to: %s", golog.ErrEntryNotWritten, err)) - return + ev := sentry.NewEvent() + ev.Level = toSentryLevel(e.Level()) + ev.Message = e.Message() + for _, f := range e.Fields() { + ev.Extra[f.Key()] = f.Value() } hub := w.Hub.Clone() - if e.Level() >= w.CaptureExceptionFromLevel { - hub.CaptureException(errors.New(buf.String())) - return - } + hub.CaptureEvent(ev) +} - hub.CaptureMessage(buf.String()) +func (w *Writer) Write(msg []byte) (int, error) { + e := golog.NewStdEntry(context.Background(), w.DefaultLevel, string(msg), golog.Fields{}) + w.WriteEntry(e) + return len(msg), nil } -func (w *Writer) Write(msg []byte) (int, error) { - hub := w.Hub.Clone() - if w.DefaultLevel >= w.CaptureExceptionFromLevel { - hub.CaptureException(errors.New(string(msg))) - return len(msg), nil +func toSentryLevel(lvl golog.Level) sentry.Level { + switch lvl { + case golog.DEBUG: + return sentry.LevelDebug + case golog.INFO: + return sentry.LevelInfo + case golog.WARN: + return sentry.LevelWarning + case golog.ERROR: + return sentry.LevelError + case golog.FATAL: + return sentry.LevelFatal } - hub.CaptureMessage(string(msg)) - return len(msg), nil + return sentry.LevelError } diff --git a/sentry/sentry_test.go b/sentry/sentry_test.go index 8f9b68a..cbf0092 100644 --- a/sentry/sentry_test.go +++ b/sentry/sentry_test.go @@ -1,16 +1,14 @@ -package sentry_test +package sentry import ( - "bytes" "context" - "io" + "strings" "testing" "time" "github.com/getsentry/sentry-go" "github.com/damianopetrungaro/golog" - . "github.com/damianopetrungaro/golog/sentry" ) var ( @@ -20,91 +18,127 @@ var ( ) type fakeTransport struct { - Message string - Excpetion []sentry.Exception + Level string + Message string + Extra map[string]interface{} } func (t *fakeTransport) Configure(sentry.ClientOptions) {} func (t *fakeTransport) SendEvent(ev *sentry.Event) { + t.Level = string(ev.Level) t.Message = ev.Message - t.Excpetion = ev.Exception + t.Extra = ev.Extra } func (t *fakeTransport) Flush(time.Duration) bool { return true } -// FakeEncoder used for internal testing purposes -type FakeEncoder struct { - Entry golog.Entry - ShouldFail bool - ShouldWriterTo io.WriterTo -} +func TestWriter_WriteEntry(t *testing.T) { + transport := &fakeTransport{} + + hub := getHubHelper(t, transport) + + w := &Writer{ + Hub: hub, + DefaultLevel: golog.INFO, + } + + if _, err := w.Write([]byte(`message`)); err != nil { + t.Fatalf("could not write log message: %s", err) + } + + if transport.Message != `message` { + t.Error("could not match message") + t.Errorf("got: %s", transport.Message) + t.Errorf("want: %s", debugEntry.Message()) + } + + if transport.Level != strings.ToLower(w.DefaultLevel.String()) { + t.Error("could not match level") + t.Errorf("got: %s", transport.Level) + t.Errorf("want: %s", debugEntry.Level().String()) + } + + if len(transport.Extra) > 0 { + t.Error("could not match extra") + t.Errorf("got: %v", transport.Extra) + } + + w.WriteEntry(errorEntry.With(golog.String("extra_key", "extra_value"))) + if transport.Message != errorEntry.Message() { + t.Error("could not match message") + t.Errorf("got: %s", transport.Message) + t.Errorf("want: %s", errorEntry.Message()) + } + + if transport.Level != strings.ToLower(errorEntry.Level().String()) { + t.Error("could not match level") + t.Errorf("got: %s", transport.Level) + t.Errorf("want: %s", errorEntry.Level().String()) + } + + if len(transport.Extra) != 1 { + t.Error("could not match extra") + t.Errorf("got: %v", transport.Extra) + } -func (fe *FakeEncoder) Encode(e golog.Entry) (io.WriterTo, error) { - fe.Entry = e - return fe.ShouldWriterTo, nil + if transport.Extra["extra_key"] != "extra_value" { + t.Error("could not match extra key") + t.Errorf("got: %v", transport.Extra["extra_key"]) + } } -func TestWriter(t *testing.T) { - t.Run("capture message", func(t *testing.T) { - data := []byte(`This is the data written`) - writerTo := bytes.NewBuffer(data) - transport := &fakeTransport{} - - hub := getHubHelper(t, transport) - enc := &FakeEncoder{ShouldWriterTo: writerTo} - - w := &Writer{ - Encoder: enc, - Hub: hub, - ErrHandler: golog.DefaultErrorHandler(), - DefaultLevel: golog.INFO, - CaptureExceptionFromLevel: golog.WARN, - } +func TestWriter_Write(t *testing.T) { + transport := &fakeTransport{} - w.WriteEntry(debugEntry) - if transport.Message != string(data) { - t.Error("could not match message") - t.Errorf("got: %s", transport.Message) - t.Errorf("want: %s", data) - } + hub := getHubHelper(t, transport) - if len(transport.Excpetion) > 0 { - t.Error("could not match exception") - t.Errorf("got: %v", transport.Excpetion) - } - }) - - t.Run("capture exception", func(t *testing.T) { - data := []byte(`This is the data written`) - writerTo := bytes.NewBuffer(data) - transport := &fakeTransport{} - - hub := getHubHelper(t, transport) - enc := &FakeEncoder{ShouldWriterTo: writerTo} - - w := &Writer{ - Encoder: enc, - Hub: hub, - ErrHandler: golog.DefaultErrorHandler(), - DefaultLevel: golog.INFO, - CaptureExceptionFromLevel: golog.WARN, - } + w := &Writer{ + Hub: hub, + DefaultLevel: golog.INFO, + } - w.WriteEntry(errorEntry) - if transport.Excpetion[0].Value != string(data) { - t.Error("could not match exception") - t.Errorf("got: %s", transport.Message) - t.Errorf("want: %s", data) - } + w.WriteEntry(debugEntry) + if transport.Message != debugEntry.Message() { + t.Error("could not match message") + t.Errorf("got: %s", transport.Message) + t.Errorf("want: %s", debugEntry.Message()) + } - if transport.Message != "" { - t.Error("could not match message") - t.Errorf("got: %v", transport.Excpetion) + if transport.Level != strings.ToLower(debugEntry.Level().String()) { + t.Error("could not match level") + t.Errorf("got: %s", transport.Level) + t.Errorf("want: %s", debugEntry.Level().String()) + } + + if len(transport.Extra) > 0 { + t.Error("could not match extra") + t.Errorf("got: %v", transport.Extra) + } +} + +func Test_toSentryLevel(t *testing.T) { + tests := []struct { + lvl golog.Level + want sentry.Level + }{ + {lvl: golog.DEBUG, want: sentry.LevelDebug}, + {lvl: golog.INFO, want: sentry.LevelInfo}, + {lvl: golog.WARN, want: sentry.LevelWarning}, + {lvl: golog.ERROR, want: sentry.LevelError}, + {lvl: golog.FATAL, want: sentry.LevelFatal}, + {lvl: golog.Level(99), want: sentry.LevelError}, + } + + for _, test := range tests { + if got := toSentryLevel(test.lvl); got != test.want { + t.Error("could not match level") + t.Errorf("got: %s", got) + t.Errorf("want: %s", test.want) } - }) + } } func getHubHelper(t *testing.T, transport sentry.Transport) *sentry.Hub {