Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
119 changes: 96 additions & 23 deletions reporter/sentry/options.go
Original file line number Diff line number Diff line change
@@ -1,37 +1,55 @@
package sentry

import (
"context"
"io"
"os"
"strings"
"time"

"github.com/getsentry/sentry-go"
uerrors "github.com/upfluence/errors"
"github.com/upfluence/errors/base"
"github.com/upfluence/errors/reporter"
)

var defaultOptions = Options{
Tags: make(map[string]string),
SentryOptions: sentry.ClientOptions{
Dsn: os.Getenv("SENTRY_DSN"),
Environment: os.Getenv("ENV"),
SendDefaultPII: true,
},
TagWhitelist: toStringMap(
[]string{reporter.RemoteIP, reporter.RemotePort, reporter.DomainKey},
),
Timeout: time.Minute,
TagBlacklist: []func(string) bool{
stringEqual(reporter.TransactionKey),
stringEqual(reporter.UserEmailKey),
stringEqual(reporter.UserIDKey),
stringEqual(reporter.HTTPRequestProtoKey),
stringEqual(reporter.HTTPRequestPathKey),
stringEqual(reporter.HTTPRequestHostKey),
stringEqual(reporter.HTTPRequestMethodKey),
stringEqual(reporter.HTTPRequestBodyKey),
stringPrefix(reporter.HTTPRequestHeaderKeyPrefix),
stringPrefix(reporter.HTTPRequestQueryValuesKeyPrefix),
},
type ErrorLevelMapper func(error) sentry.Level
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I saw that you applied the comment https://github.com/upfluence/errors/pull/18/changes#r2578258351 in the reporter.go file can you apply the comment in this file as well

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, for exported names, we had that conversation previously with @Sypheos and @karitham : #18 (comment), not sure it's a very good idea for clarity's sake to just call them LevelFuncs, for instance, unless you have a better idea?

For private constructions I don't mind


var defaultErrorLevelMappers = []ErrorLevelMapper{
ErrorIsLevel(context.DeadlineExceeded, sentry.LevelWarning),
ErrorIsLevel(context.Canceled, sentry.LevelWarning),
ErrorIsLevel(io.EOF, sentry.LevelWarning),
ErrorCauseTextContainsLevel("net/http: TLS handshake timeout", sentry.LevelWarning),
ErrorCauseTextContainsLevel("operation was canceled", sentry.LevelWarning),
ErrorCauseTextContainsLevel("EOF", sentry.LevelWarning),
}

func defaultOptions() Options {
return Options{
Tags: make(map[string]string),
SentryOptions: sentry.ClientOptions{
Dsn: os.Getenv("SENTRY_DSN"),
Environment: os.Getenv("ENV"),
SendDefaultPII: true,
},
TagWhitelist: toStringMap(
[]string{reporter.RemoteIP, reporter.RemotePort, reporter.DomainKey},
),
Timeout: time.Minute,
TagBlacklist: []func(string) bool{
stringEqual(reporter.TransactionKey),
stringEqual(reporter.UserEmailKey),
stringEqual(reporter.UserIDKey),
stringEqual(reporter.HTTPRequestProtoKey),
stringEqual(reporter.HTTPRequestPathKey),
stringEqual(reporter.HTTPRequestHostKey),
stringEqual(reporter.HTTPRequestMethodKey),
stringEqual(reporter.HTTPRequestBodyKey),
stringPrefix(reporter.HTTPRequestHeaderKeyPrefix),
stringPrefix(reporter.HTTPRequestQueryValuesKeyPrefix),
},
ErrorLevelMappers: defaultErrorLevelMappers,
}
}

func stringEqual(x string) func(string) bool {
Expand All @@ -52,6 +70,44 @@ func toStringMap(vs []string) map[string]struct{} {
return res
}

// ErrorIsLevel creates an ErrorLevelMapper of the passed level that checks if
// reported errors are the same as the given sentinel error
func ErrorIsLevel(sentinel error, level sentry.Level) ErrorLevelMapper {
return func(err error) sentry.Level {
if uerrors.Is(err, sentinel) {
return level
}

return ""
}
}

// ErrorIsOfTypeLevel creates an ErrorLevelMapper of the passed level that checks
// if reported errors are of the passed generic type
func ErrorIsOfTypeLevel[T error](level sentry.Level) ErrorLevelMapper {
return func(err error) sentry.Level {
if uerrors.IsOfType[T](err) {
return level
}

return ""
}
}

// ErrorCauseTextContainsLevel creates an ErrorLevelMapper of the passed level that checks
// if reported errors' cause's Error() text contains the passed string
func ErrorCauseTextContainsLevel(errorText string, level sentry.Level) ErrorLevelMapper {
return func(err error) sentry.Level {
rootCause := base.UnwrapAll(err).Error()

if strings.Contains(rootCause, errorText) {
return level
}

return ""
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nil error causes panic in ErrorCauseTextContainsLevel mapper

The ErrorCauseTextContainsLevel function doesn't handle nil errors gracefully. Calling base.UnwrapAll(nil).Error() will panic because UnwrapAll(nil) returns nil, and invoking .Error() on a nil error interface causes a nil pointer dereference. This is inconsistent with ErrorIsLevel and ErrorIsOfTypeLevel, which both handle nil errors safely (via uerrors.Is and uerrors.IsOfType). While current usage is protected by the nil check in buildEvent, this public API function could panic if used directly with a nil error.

Fix in Cursor Fix in Web

}

type Options struct {
Tags map[string]string

Expand All @@ -60,6 +116,8 @@ type Options struct {

TagWhitelist map[string]struct{}
TagBlacklist []func(string) bool

ErrorLevelMappers []ErrorLevelMapper
}

func (o Options) client() (*sentry.Client, error) {
Expand All @@ -74,10 +132,25 @@ func (o Options) client() (*sentry.Client, error) {

type Option func(*Options)

// WithTags allows for custom tags to be given to the Reporter
func WithTags(tags map[string]string) Option {
return func(opts *Options) {
for k, v := range tags {
opts.Tags[k] = v
}
}
}

// AppendErrorLevelMappers adds the passed funcs to the ErrorLevelMappers of the Reporter
func AppendErrorLevelMappers(funcs ...ErrorLevelMapper) Option {
return func(opts *Options) {
opts.ErrorLevelMappers = append(opts.ErrorLevelMappers, funcs...)
}
}

// ReplaceErrorLevelMappers replaces the ErrorLevelMappers of the Reporter with the passed ones
func ReplaceErrorLevelMappers(funcs []ErrorLevelMapper) Option {
return func(opts *Options) {
opts.ErrorLevelMappers = funcs
}
}
34 changes: 23 additions & 11 deletions reporter/sentry/reporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,14 @@ type Reporter struct {

tagWhitelist []func(string) bool
tagBlacklist []func(string) bool
levelMappers []ErrorLevelMapper

timeout time.Duration
}

// NewReporter creates a new Sentry reporter with the given options.
func NewReporter(os ...Option) (*Reporter, error) {
var opts = defaultOptions
var opts = defaultOptions()

for _, o := range os {
o(&opts)
Expand All @@ -58,6 +59,7 @@ func NewReporter(os ...Option) (*Reporter, error) {
},
tagBlacklist: opts.TagBlacklist,
timeout: opts.Timeout,
levelMappers: opts.ErrorLevelMappers,
}, nil
}

Expand Down Expand Up @@ -121,26 +123,26 @@ func (r *Reporter) buildEvent(err error, opts reporter.ReportOptions) *sentry.Ev
return nil
}

ts := tags.GetTags(err)
errorTags := tags.GetTags(err)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: the repository is already named errors is the error part of errorTags really needed? could it simply be tags?

Same feedback for:

  • computeErrorLevel
  • errorLevelMappers

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem here is that tags is a symbol already (the import name for that package), and I really dislike it being called ts or t. Any other suggestion that does not make it illegible? 😓

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that makes sends for errorTags, could you adjust the two others?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, can do when I next get to work on this one 👍


if ts == nil && len(opts.Tags) > 0 {
ts = make(map[string]interface{}, len(opts.Tags))
if errorTags == nil && len(opts.Tags) > 0 {
errorTags = make(map[string]interface{}, len(opts.Tags))
}

for k, v := range opts.Tags {
if _, ok := ts[k]; !ok {
ts[k] = v
if _, ok := errorTags[k]; !ok {
errorTags[k] = v
}
}

evt := sentry.NewEvent()

evt.Level = sentry.LevelError
evt.Level = r.computeLevel(err)
evt.Timestamp = time.Now()
evt.Message = err.Error()
evt.Transaction = transactionName(ts)
evt.User = buildUser(ts)
evt.Request = buildRequest(ts)
evt.Transaction = transactionName(errorTags)
evt.User = buildUser(errorTags)
evt.Request = buildRequest(errorTags)

cause := base.UnwrapAll(err)

Expand All @@ -153,7 +155,7 @@ func (r *Reporter) buildEvent(err error, opts reporter.ReportOptions) *sentry.Ev
},
}

for k, v := range ts {
for k, v := range errorTags {
r.appendTag(k, v, evt)
}

Expand All @@ -164,6 +166,16 @@ func (r *Reporter) buildEvent(err error, opts reporter.ReportOptions) *sentry.Ev
return evt
}

func (r *Reporter) computeLevel(err error) sentry.Level {
for _, errFunc := range r.levelMappers {
if level := errFunc(err); level != "" {
return level
}
}

return sentry.LevelError
}

func extractStacktrace(err error, n int) *sentry.Stacktrace {
var s sentry.Stacktrace

Expand Down
94 changes: 94 additions & 0 deletions reporter/sentry/reporter_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package sentry

import (
"context"
"io"
"regexp"
"testing"
Expand All @@ -13,6 +14,10 @@ import (
"github.com/upfluence/errors/reporter"
)

type mockError struct{}

func (*mockError) Error() string { return "mock" }

func TestBuildEvent(t *testing.T) {
for _, tt := range []struct {
name string
Expand Down Expand Up @@ -163,6 +168,95 @@ func TestBuildEvent(t *testing.T) {
assert.Equal(t, evt.Tags, map[string]string{"foo": "bar", "domain": "github.com/upfluence/errors/reporter/sentry"})
},
},
{
name: "simple sentinel error with severity func",
err: io.EOF,
modifiers: []func(*Reporter){},
evtfn: func(t *testing.T, evt *sentry.Event) {
assert.Equal(
t,
sentry.LevelWarning,
evt.Level,
)
},
},
{
name: "wrapped sentinel error with severity func",
err: errors.Wrap(context.DeadlineExceeded, "it took too long, boss"),
modifiers: []func(*Reporter){},
evtfn: func(t *testing.T, evt *sentry.Event) {
assert.Equal(
t,
sentry.LevelWarning,
evt.Level,
)
},
},
{
name: "simple text error with severity func",
err: errors.New("net/http: TLS handshake timeout with extra bells and whistles"),
modifiers: []func(*Reporter){},
evtfn: func(t *testing.T, evt *sentry.Event) {
assert.Equal(
t,
sentry.LevelWarning,
evt.Level,
)
},
},
{
name: "wrapped text error with severity func",
err: errors.Wrap(
errors.New("net/http: TLS handshake timeout with extra bells and whistles"),
"more bells and whistles",
),
modifiers: []func(*Reporter){},
evtfn: func(t *testing.T, evt *sentry.Event) {
assert.Equal(
t,
sentry.LevelWarning,
evt.Level,
)
},
},
{
name: "simple error type",
err: &mockError{},
modifiers: []func(*Reporter){
func(r *Reporter) {
r.levelMappers = append(
r.levelMappers,
ErrorIsOfTypeLevel[*mockError](sentry.LevelDebug),
)
},
},
evtfn: func(t *testing.T, evt *sentry.Event) {
assert.Equal(
t,
sentry.LevelDebug,
evt.Level,
)
},
},
{
name: "wrapped error type",
err: errors.Wrap(&mockError{}, "i am being mocked"),
modifiers: []func(*Reporter){
func(r *Reporter) {
r.levelMappers = append(
r.levelMappers,
ErrorIsOfTypeLevel[*mockError](sentry.LevelDebug),
)
},
},
evtfn: func(t *testing.T, evt *sentry.Event) {
assert.Equal(
t,
sentry.LevelDebug,
evt.Level,
)
},
},
} {
t.Run(tt.name, func(t *testing.T) {
r, err := NewReporter(tt.opts...)
Expand Down
Loading