diff --git a/scope.go b/scope.go index 9a6bd524c..1caa4e9b3 100644 --- a/scope.go +++ b/scope.go @@ -15,23 +15,22 @@ import ( "github.com/getsentry/sentry-go/report" ) -// Scope holds contextual data for the current scope. +// Scope holds contextual data for an operation. // -// The scope is an object that can cloned efficiently and stores data that is -// locally relevant to an event. For instance the scope will hold recorded -// breadcrumbs and similar information. +// The scope is an object that can be cloned efficiently and stores data that is +// locally relevant to an event. It also holds the client and event processor in +// which the scope data should be applied to. // -// The scope can be interacted with in two ways. First, the scope is routinely -// updated with information by functions such as AddBreadcrumb which will modify -// the current scope. Second, the current scope can be configured through the -// ConfigureScope function or Hub method of the same name. -// -// The scope is meant to be modified but not inspected directly. When preparing -// an event for reporting, the current client adds information from the current -// scope into the event. +// Clearing or cloning the scope only affects the underlying data. To set a new +// client or event processor, SetClient or AddEventProcessor should be used. type Scope struct { - mu sync.RWMutex + mu sync.RWMutex + // clientOverride is an explicit client binding set with SetClient. Having + // no override defaults to the global scope client. + clientOverride *Client + // eventProcessors are retained by Clear and inherited by Clone. eventProcessors []EventProcessor + lastEventID EventID // scopeData keeps track of all scope specific data scopeData @@ -58,7 +57,7 @@ type scopeData struct { } propagationContext PropagationContext - span *Span + span *Span // TODO: this should be removed when the span API is introduced. Currently kept for compatibility. } // NewScope creates a new Scope. @@ -66,6 +65,13 @@ func NewScope() *Scope { return &Scope{scopeData: newScopeData()} } +// newScopeWithClient creates a Scope with an explicit client override. +func newScopeWithClient(client *Client) *Scope { + scope := NewScope() + scope.SetClient(client) + return scope +} + func newScopeData() scopeData { return scopeData{ attributes: make(map[string]attribute.Value), @@ -94,6 +100,54 @@ func (scope *Scope) AddBreadcrumb(breadcrumb *Breadcrumb, limit int) { } } +// SetClient sets an explicit client override on the scope. Passing nil clears +// the override so client resolution falls back to GlobalScope. +func (scope *Scope) SetClient(client *Client) { + scope.mu.Lock() + defer scope.mu.Unlock() + + scope.clientOverride = normalizeClient(client) +} + +func (scope *Scope) clientOverrideSnapshot() *Client { + scope.mu.RLock() + defer scope.mu.RUnlock() + + return scope.clientOverride +} + +// Client returns the first enabled client in the scope chain. +func (scope *Scope) Client() *Client { + if scope != nil { + if client := normalizeClient(scope.clientOverrideSnapshot()); client.IsEnabled() { + return client + } + } + + global := GlobalScope() + if scope != global { + if client := normalizeClient(global.clientOverrideSnapshot()); client.IsEnabled() { + return client + } + } + return NewNoopClient() +} + +func (scope *Scope) setLastEventID(id EventID) { + scope.mu.Lock() + defer scope.mu.Unlock() + + scope.lastEventID = id +} + +// LastEventID returns the last event ID associated with this scope. +func (scope *Scope) LastEventID() EventID { + scope.mu.RLock() + defer scope.mu.RUnlock() + + return scope.lastEventID +} + // ClearBreadcrumbs clears all breadcrumbs from the current scope. func (scope *Scope) ClearBreadcrumbs() { scope.mu.Lock() @@ -294,6 +348,8 @@ func (scope *Scope) Clone() *Scope { return &Scope{ scopeData: data.clone(), eventProcessors: scope.eventProcessors[:len(scope.eventProcessors):len(scope.eventProcessors)], + clientOverride: scope.clientOverride, + lastEventID: scope.lastEventID, } } diff --git a/scope_concurrency_test.go b/scope_concurrency_test.go index 4bec89068..26d8f63f3 100644 --- a/scope_concurrency_test.go +++ b/scope_concurrency_test.go @@ -1,6 +1,7 @@ package sentry_test import ( + "context" "fmt" "net/http/httptest" "sync" @@ -48,6 +49,48 @@ func TestConcurrentScopeUsage(_ *testing.T) { wg.Wait() } +func TestConcurrentSharedIsolation(_ *testing.T) { + ctx, scope := sentry.WithIsolation(context.Background()) + var wg sync.WaitGroup + + for i := 0; i < 20; i++ { + wg.Add(1) + go func(x int) { + defer wg.Done() + scope.SetTag(fmt.Sprintf("tag-%d", x), "value") + scope.SetUser(sentry.User{ID: fmt.Sprint(x)}) + scope.SetContext(fmt.Sprintf("context-%d", x), sentry.Context{"value": x}) + scope.SetAttributes(attribute.Int("value", x)) + scope.AddBreadcrumb(&sentry.Breadcrumb{Message: fmt.Sprint(x)}, 100) + shared := sentry.ScopeFromContext(ctx) + shared.Clone() + }(i) + } + + wg.Wait() +} + +func TestConcurrentSharedIsolationClearAndClone(_ *testing.T) { + _, scope := sentry.WithIsolation(context.Background()) + var wg sync.WaitGroup + + for i := 0; i < 20; i++ { + wg.Add(1) + go func(x int) { + defer wg.Done() + for j := 0; j < 20; j++ { + scope.SetTag(fmt.Sprintf("tag-%d", x), fmt.Sprint(j)) + scope.Clone() + if j%5 == 0 { + scope.Clear() + } + } + }(i) + } + + wg.Wait() +} + func touchScope(scope *sentry.Scope, x int) { scope.SetTag("foo", "bar") scope.SetContext("foo", sentry.Context{"foo": "bar"}) diff --git a/scope_context.go b/scope_context.go new file mode 100644 index 000000000..48ebd1354 --- /dev/null +++ b/scope_context.go @@ -0,0 +1,65 @@ +package sentry + +import "context" + +type scopeContextKey struct{} + +// globalScope is the process-wide global scope. +var globalScope = newScopeWithClient(NewNoopClient()) + +// GlobalScope returns the process-wide global scope. +func GlobalScope() *Scope { + return globalScope +} + +// ScopeFromContext returns the scope carried by ctx, or nil when ctx +// does not carry one. +func ScopeFromContext(ctx context.Context) *Scope { + if ctx == nil { + return nil + } + scope, _ := ctx.Value(scopeContextKey{}).(*Scope) + return scope +} + +func contextWithScope(ctx context.Context, scope *Scope) context.Context { + return context.WithValue(ctx, scopeContextKey{}, scope) +} + +// WithIsolation returns a derived context and an independent scope. +// It clones a carried scope or creates an empty scope when ctx does +// not carry one. +func WithIsolation(ctx context.Context) (context.Context, *Scope) { + parent := ScopeFromContext(ctx) + var scope *Scope + if parent == nil { + scope = NewScope() + } else { + scope = parent.Clone() + scope.SetPropagationContext(NewPropagationContext()) + scope.SetSpan(nil) + } + return contextWithScope(ctx, scope), scope +} + +// WithScopeContext invokes fn with a derived context carrying a cloned scope. +func WithScopeContext(ctx context.Context, fn func(context.Context, *Scope)) { // TODO: should remove WithScope when hub is removed. + if fn == nil { + return + } + + parent := ScopeFromContext(ctx) + if parent == nil { + parent = NewScope() + } + scope := parent.Clone() + fn(contextWithScope(ctx, scope), scope) +} + +// GetClient returns the first enabled client for ctx. +func GetClient(ctx context.Context) *Client { + if scope := ScopeFromContext(ctx); scope != nil { + return scope.Client() + } + return GlobalScope().Client() +} diff --git a/scope_context_test.go b/scope_context_test.go new file mode 100644 index 000000000..dd1d96bfd --- /dev/null +++ b/scope_context_test.go @@ -0,0 +1,175 @@ +package sentry + +import ( + "context" + "testing" +) + +func TestScopeFromContext(t *testing.T) { + if scope := ScopeFromContext(context.Background()); scope != nil { + t.Fatalf("ScopeFromContext returned %p for an unscoped context", scope) + } + + ctx, scope := WithIsolation(context.Background()) + if got := ScopeFromContext(ctx); got != scope { + t.Fatalf("ScopeFromContext returned %p, want %p", got, scope) + } +} + +func TestIsolationScopeSharesDownstreamMutations(t *testing.T) { + type contextKey struct{} + + ctx, scope := WithIsolation(context.Background()) + child := context.WithValue(ctx, contextKey{}, "value") + childScope := ScopeFromContext(child) + childScope.SetUser(User{ID: "123"}) + + if scope.user.ID != "123" { + t.Fatal("downstream mutation was not visible to the boundary owner") + } +} + +func TestWithIsolationCreatesIndependentBoundaries(t *testing.T) { + parentCtx, parent := WithIsolation(context.Background()) + parent.SetTag("inherited", "yes") + parentPropagation := parent.propagationContextSnapshot() + + firstCtx, first := WithIsolation(parentCtx) + secondCtx, second := WithIsolation(parentCtx) + first.SetTag("worker", "first") + second.SetTag("worker", "second") + + if ScopeFromContext(firstCtx) != first || ScopeFromContext(secondCtx) != second { + t.Fatal("derived contexts do not carry their returned isolation scopes") + } + if first == parent || second == parent || first == second { + t.Fatal("isolation boundaries alias") + } + if first.tags["inherited"] != "yes" || second.tags["inherited"] != "yes" { + t.Fatal("isolation boundaries did not inherit parent enrichment") + } + if _, ok := parent.tags["worker"]; ok { + t.Fatal("child mutation leaked into parent") + } + if first.tags["worker"] != "first" || second.tags["worker"] != "second" { + t.Fatal("sibling isolation mutations leaked") + } + if first.propagationContext.TraceID == parentPropagation.TraceID || + second.propagationContext.TraceID == parentPropagation.TraceID || + first.propagationContext.TraceID == second.propagationContext.TraceID { + t.Fatal("isolation boundaries share propagation trace IDs") + } +} + +func TestWithIsolationDoesNotCloneGlobalScope(t *testing.T) { + global := GlobalScope() + global.SetTag("global-only", "yes") + t.Cleanup(func() { global.RemoveTag("global-only") }) + + ctx, scope := WithIsolation(context.Background()) + if ScopeFromContext(ctx) != scope { + t.Fatal("derived context does not carry the returned scope") + } + if scope == global { + t.Fatal("isolation scope aliases global scope") + } + if _, ok := scope.tags["global-only"]; ok { + t.Fatal("isolation scope cloned global data") + } +} + +func TestWithScopeContextCreatesTemporaryFork(t *testing.T) { + ctx, parent := WithIsolation(context.Background()) + parent.SetTag("parent", "yes") + propagation := parent.propagationContextSnapshot() + + WithScopeContext(ctx, func(childCtx context.Context, child *Scope) { + if ScopeFromContext(childCtx) != child { + t.Fatal("callback context does not carry callback scope") + } + if child == parent || child.tags["parent"] != "yes" { + t.Fatal("callback scope did not inherit an independent enrichment copy") + } + if child.propagationContext.TraceID != propagation.TraceID { + t.Fatal("temporary scope fork did not continue propagation") + } + child.SetTag("temporary", "yes") + }) + + if _, ok := parent.tags["temporary"]; ok { + t.Fatal("temporary mutation leaked into parent") + } +} + +func TestScopeClientResolution(t *testing.T) { + globalClient, err := NewClient(ClientOptions{}) + if err != nil { + t.Fatal(err) + } + operationClient, err := NewClient(ClientOptions{}) + if err != nil { + t.Fatal(err) + } + otherGlobalClient, err := NewClient(ClientOptions{}) + if err != nil { + t.Fatal(err) + } + + global := GlobalScope() + previousGlobal := global.clientOverrideSnapshot() + global.SetClient(globalClient) + t.Cleanup(func() { global.SetClient(previousGlobal) }) + + standalone := NewScope() + if standalone.clientOverrideSnapshot() != nil { + t.Fatal("NewScope has an unexpected client override") + } + if standalone.Client() != globalClient { + t.Fatal("standalone scope did not resolve the global client") + } + + ctx, scope := WithIsolation(context.Background()) + if GetClient(ctx) != globalClient || scope.Client() != globalClient || scope.clientOverrideSnapshot() != nil { + t.Fatal("new operation scope did not inherit the global client dynamically") + } + + global.SetClient(otherGlobalClient) + if GetClient(ctx) != otherGlobalClient || scope.Client() != otherGlobalClient { + t.Fatal("existing operation scope did not observe the updated global client") + } + + scope.SetClient(NewNoopClient()) + if GetClient(ctx) != otherGlobalClient || scope.Client() != otherGlobalClient { + t.Fatal("disabled operation client did not fall back to the enabled global client") + } + + scope.SetClient(operationClient) + if GetClient(ctx) != operationClient || scope.Client() != operationClient { + t.Fatal("enabled operation client did not override the global client") + } + + childCtx, child := WithIsolation(ctx) + if GetClient(childCtx) != operationClient { + t.Fatal("child isolation did not inherit the explicit client override") + } + child.SetClient(nil) + if GetClient(childCtx) != otherGlobalClient || GetClient(ctx) != operationClient { + t.Fatal("clearing the child override did not fall back to global independently") + } +} + +func TestScopeLastEventIDSurvivesCloneAndClear(t *testing.T) { + scope := NewScope() + id := EventID("0123456789abcdef0123456789abcdef") + scope.setLastEventID(id) + + clone := scope.Clone() + scope.Clear() + + if got := scope.LastEventID(); got != id { + t.Fatalf("LastEventID after Clear = %q, want %q", got, id) + } + if got := clone.LastEventID(); got != id { + t.Fatalf("clone LastEventID = %q, want %q", got, id) + } +} diff --git a/sentry.go b/sentry.go index b315c3711..94782d2c7 100644 --- a/sentry.go +++ b/sentry.go @@ -24,6 +24,7 @@ func Init(options ClientOptions) error { return err } hub.BindClient(client) + GlobalScope().SetClient(client) return nil }