From 882a37fb7a84c1ff1e9a560e81945de0336d59fd Mon Sep 17 00:00:00 2001 From: Andrew Newdigate Date: Wed, 21 Oct 2020 15:58:29 +0200 Subject: [PATCH] Allow non-global sentry with state providers State providers allow the caller to specify whether or not global state should be used. This can be helpful in ensuring that tests do not have side-effects. --- errortracking/capture.go | 9 +---- errortracking/capture_context.go | 2 +- errortracking/capture_field.go | 2 +- errortracking/capture_field_test.go | 3 +- errortracking/capture_options.go | 21 +++++++++- errortracking/capture_options_test.go | 42 ++++++++++++++++++++ errortracking/capture_registry.go | 14 +++++++ errortracking/capture_request.go | 2 +- errortracking/initialization_options.go | 15 ++++++- errortracking/initialization_options_test.go | 29 +++++++++++--- errortracking/stateprovider_default.go | 25 ++++++++++++ errortracking/stateprovider_extra.go | 25 ++++++++++++ 12 files changed, 170 insertions(+), 19 deletions(-) create mode 100644 errortracking/capture_options_test.go create mode 100644 errortracking/capture_registry.go create mode 100644 errortracking/stateprovider_default.go create mode 100644 errortracking/stateprovider_extra.go diff --git a/errortracking/capture.go b/errortracking/capture.go index e1135594..e5f0cf80 100644 --- a/errortracking/capture.go +++ b/errortracking/capture.go @@ -8,12 +8,7 @@ import ( // Capture will report an error to the error reporting service func Capture(err error, opts ...CaptureOption) { - event := sentry.NewEvent() - event.Level = sentry.LevelError - - for _, v := range opts { - v(event) - } + config, event := applyCaptureOptions(opts) event.Exception = []sentry.Exception{ { @@ -23,5 +18,5 @@ func Capture(err error, opts ...CaptureOption) { }, } - sentry.CaptureEvent(event) + config.stateProvider.hub().CaptureEvent(event) } diff --git a/errortracking/capture_context.go b/errortracking/capture_context.go index fda98ed6..175bcc50 100644 --- a/errortracking/capture_context.go +++ b/errortracking/capture_context.go @@ -12,7 +12,7 @@ const sentryExtraKey = "gitlab.CorrelationID" // WithContext will extract information from the context to add to the error func WithContext(ctx context.Context) CaptureOption { - return func(event *sentry.Event) { + return func(config *captureConfig, event *sentry.Event) { correlationID := correlation.ExtractFromContext(ctx) if correlationID != "" { event.Tags[sentryExtraKey] = correlationID diff --git a/errortracking/capture_field.go b/errortracking/capture_field.go index 59ca7ec4..f03a006d 100644 --- a/errortracking/capture_field.go +++ b/errortracking/capture_field.go @@ -6,7 +6,7 @@ import ( // WithField allows to add a custom field to the error func WithField(key string, value string) CaptureOption { - return func(event *sentry.Event) { + return func(config *captureConfig, event *sentry.Event) { event.Tags[key] = value } } diff --git a/errortracking/capture_field_test.go b/errortracking/capture_field_test.go index 5a1eb268..21dcd58b 100644 --- a/errortracking/capture_field_test.go +++ b/errortracking/capture_field_test.go @@ -10,8 +10,9 @@ import ( func TestWithField(t *testing.T) { event := sentry.NewEvent() domain := "http://example.com" + config := &captureConfig{} - WithField("domain", domain)(event) + WithField("domain", domain)(config, event) require.True(t, event.Tags["domain"] == domain) } diff --git a/errortracking/capture_options.go b/errortracking/capture_options.go index 7adabc3a..fbb1b93b 100644 --- a/errortracking/capture_options.go +++ b/errortracking/capture_options.go @@ -5,4 +5,23 @@ import ( ) // CaptureOption will configure how an error is captured -type CaptureOption func(*sentry.Event) +type CaptureOption func(*captureConfig, *sentry.Event) + +type captureConfig struct { + stateProvider StateProvider +} + +func applyCaptureOptions(opts []CaptureOption) (captureConfig, *sentry.Event) { + event := sentry.NewEvent() + event.Level = sentry.LevelError + + config := captureConfig{ + stateProvider: DefaultStateProvider, + } + + for _, v := range opts { + v(&config, event) + } + + return config, event +} diff --git a/errortracking/capture_options_test.go b/errortracking/capture_options_test.go new file mode 100644 index 00000000..561c97f0 --- /dev/null +++ b/errortracking/capture_options_test.go @@ -0,0 +1,42 @@ +package errortracking + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/getsentry/sentry-go" +) + +var altStateProvider = NewStateProvider() + +func Test_applyCaptureOptions(t *testing.T) { + tests := []struct { + name string + opts []CaptureOption + wantConfig captureConfig + wantEventLevel sentry.Level + }{ + { + name: "default", + opts: []CaptureOption{}, + wantConfig: captureConfig{stateProvider: DefaultStateProvider}, + wantEventLevel: sentry.LevelError, + }, + { + name: "want-state-provider", + opts: []CaptureOption{WithCaptureStateProvider(altStateProvider)}, + wantConfig: captureConfig{stateProvider: altStateProvider}, + wantEventLevel: sentry.LevelError, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gotConfig, gotEvent := applyCaptureOptions(tt.opts) + gotEventLevel := gotEvent.Level + + require.Equalf(t, gotConfig, tt.wantConfig, "applyInitializationOptions() = %v, want %v", gotConfig, tt.wantConfig) + require.Equalf(t, gotEventLevel, tt.wantEventLevel, "applyInitializationOptions() Event.Level = %v, want %v", gotEventLevel, tt.wantEventLevel) + }) + } +} diff --git a/errortracking/capture_registry.go b/errortracking/capture_registry.go new file mode 100644 index 00000000..0c4768c2 --- /dev/null +++ b/errortracking/capture_registry.go @@ -0,0 +1,14 @@ +package errortracking + +import ( + "github.com/getsentry/sentry-go" +) + +// WithCaptureStateProvider capture to take place with an alternative registry +// this can be used with `NewExtraErrorTrackingRegistry()` to use a use a non-global +// error tracker. This is especially useful for testing purposes. +func WithCaptureStateProvider(stateProvider StateProvider) CaptureOption { + return func(config *captureConfig, event *sentry.Event) { + config.stateProvider = stateProvider + } +} diff --git a/errortracking/capture_request.go b/errortracking/capture_request.go index 3db06569..b261f270 100644 --- a/errortracking/capture_request.go +++ b/errortracking/capture_request.go @@ -10,7 +10,7 @@ import ( // WithRequest will capture details of the request along with the error func WithRequest(r *http.Request) CaptureOption { - return func(event *sentry.Event) { + return func(config *captureConfig, event *sentry.Event) { event.Request = redactRequestInfo(r) event.Request.URL = r.URL.String() event.Request.Headers["host"] = r.URL.Hostname() diff --git a/errortracking/initialization_options.go b/errortracking/initialization_options.go index 921d889e..35f81066 100644 --- a/errortracking/initialization_options.go +++ b/errortracking/initialization_options.go @@ -6,13 +6,17 @@ type initializationConfig struct { version string sentryEnvironment string loggerName string + stateProvider StateProvider } // InitializationOption will configure a correlation handler type InitializationOption func(*initializationConfig) func applyInitializationOptions(opts []InitializationOption) initializationConfig { - config := initializationConfig{} + config := initializationConfig{ + stateProvider: DefaultStateProvider, + } + for _, v := range opts { v(&config) } @@ -48,3 +52,12 @@ func WithLoggerName(loggerName string) InitializationOption { config.loggerName = loggerName } } + +// WithStateProvider configures the handle to use an alternative registry. +// This can be used with `NewExtraStateProvider()` to use a use a non-global +// error tracker. This is especially useful for testing purposes. +func WithStateProvider(stateProvider StateProvider) InitializationOption { + return func(config *initializationConfig) { + config.stateProvider = stateProvider + } +} diff --git a/errortracking/initialization_options_test.go b/errortracking/initialization_options_test.go index 1db5f269..63ab3e89 100644 --- a/errortracking/initialization_options_test.go +++ b/errortracking/initialization_options_test.go @@ -6,11 +6,28 @@ import ( "github.com/stretchr/testify/require" ) -func TestWithLoggerName(t *testing.T) { - loggerName := "test-logger" - config := initializationConfig{} +func Test_applyInitializationOptions(t *testing.T) { + tests := []struct { + name string + opts []InitializationOption + want initializationConfig + }{ + { + name: "with-logger-name", + opts: []InitializationOption{WithLoggerName("test-logger")}, + want: initializationConfig{loggerName: "test-logger", stateProvider: DefaultStateProvider}, + }, + { + name: "with-state-provider", + opts: []InitializationOption{WithStateProvider(altStateProvider)}, + want: initializationConfig{stateProvider: altStateProvider}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := applyInitializationOptions(tt.opts) - WithLoggerName(loggerName)(&config) - - require.Equal(t, config.loggerName, loggerName) + require.Equalf(t, got, tt.want, "applyInitializationOptions() = %v, want %v", got, tt.want) + }) + } } diff --git a/errortracking/stateprovider_default.go b/errortracking/stateprovider_default.go new file mode 100644 index 00000000..2ef614d2 --- /dev/null +++ b/errortracking/stateprovider_default.go @@ -0,0 +1,25 @@ +package errortracking + +import ( + "github.com/getsentry/sentry-go" +) + +// StateProvider is a container for any state held by the +// errortracker, for example global clients. By default +// DefaultStateProvider is used, but alternative registries can +// be passed via `WithRegistry` and `WithCaptureRegistry` for testing +// purposes. +type StateProvider interface { + hub() *sentry.Hub +} + +// defaultStateProvider is an implementation of StateProvider +// which uses the default sentry global state +type defaultStateProvider struct{} + +func (c *defaultStateProvider) hub() *sentry.Hub { + return sentry.CurrentHub() +} + +// DefaultStateProvider will use a global client for error handling +var DefaultStateProvider StateProvider = &defaultStateProvider{} diff --git a/errortracking/stateprovider_extra.go b/errortracking/stateprovider_extra.go new file mode 100644 index 00000000..d6b0c7f8 --- /dev/null +++ b/errortracking/stateprovider_extra.go @@ -0,0 +1,25 @@ +package errortracking + +import "github.com/getsentry/sentry-go" + +type extraStateProvider struct { + extraHub *sentry.Hub +} + +func (c *extraStateProvider) hub() *sentry.Hub { + return c.extraHub +} + +// NewStateProvider will provide a non-global state container +// for the errortracking module. This can be used in testing +// to avoid side-effects +func NewStateProvider() StateProvider { + client, err := sentry.NewClient(sentry.ClientOptions{}) + if err != nil { + panic(err) + } + + return &extraStateProvider{ + extraHub: sentry.NewHub(client, sentry.NewScope()), + } +} -- GitLab