-
Notifications
You must be signed in to change notification settings - Fork 258
feat: add scope context API #1371
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
+298
−33
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,25 +15,23 @@ 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 | ||
| // boundClient is the client reference bound to this Scope. Clone copies the | ||
| // client reference but not the client object. Clear keeps the binding. | ||
| boundClient Client | ||
| // eventProcessors are retained by Clear and inherited by Clone. | ||
| eventProcessors []EventProcessor | ||
|
|
||
| // scopeData keeps track of all scope specific data | ||
| // scopeData contains clearable event/request/trace enrichment data. | ||
| scopeData | ||
| } | ||
|
|
||
|
|
@@ -58,7 +56,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. | ||
|
|
@@ -94,6 +92,22 @@ func (scope *Scope) AddBreadcrumb(breadcrumb *Breadcrumb, limit int) { | |
| } | ||
| } | ||
|
|
||
| // SetClient binds a new client to the scope. | ||
| func (scope *Scope) SetClient(client Client) { | ||
| scope.mu.Lock() | ||
| defer scope.mu.Unlock() | ||
|
|
||
| scope.boundClient = normalizeClient(client) | ||
| } | ||
|
|
||
| // client returns the current bound client under a lock. | ||
| func (scope *Scope) client() Client { | ||
| scope.mu.RLock() | ||
| defer scope.mu.RUnlock() | ||
|
|
||
| return scope.boundClient | ||
| } | ||
|
|
||
| // ClearBreadcrumbs clears all breadcrumbs from the current scope. | ||
| func (scope *Scope) ClearBreadcrumbs() { | ||
| scope.mu.Lock() | ||
|
|
@@ -292,8 +306,9 @@ func (scope *Scope) Clone() *Scope { | |
|
|
||
| data := scope.scopeData | ||
| return &Scope{ | ||
| scopeData: data.clone(), | ||
| boundClient: scope.boundClient, | ||
| eventProcessors: scope.eventProcessors[:len(scope.eventProcessors):len(scope.eventProcessors)], | ||
| scopeData: data.clone(), | ||
|
Comment on lines
+309
to
+311
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [question] does the reordering of I am just trying to understand why this change is being made here. |
||
| } | ||
| } | ||
|
|
||
|
|
@@ -527,6 +542,9 @@ func hubFromContexts(ctxs ...context.Context) *Hub { | |
| // This ordering ensures we always use the most contextually relevant tracing information. | ||
| // For example, if a specific span is active for an operation, we use that span's trace/span IDs | ||
| // rather than accidentally using a different span that might be set on the hub's scope. | ||
| // | ||
| // TODO: this should be removed when the span API is introduced. Currently kept for compatibility. The span | ||
| // and trace should only be resolved through context. | ||
| func resolveTrace(scope *Scope, client Client, ctxs ...context.Context) (traceID TraceID, spanID SpanID) { | ||
| client = normalizeClient(client) | ||
| var span *Span | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,70 @@ | ||
| package sentry | ||
|
|
||
| import "context" | ||
|
|
||
| type scopesContextKey struct{} | ||
|
|
||
| // globalScope is the process-wide global scope. | ||
| var globalScope = func() *Scope { | ||
| scope := NewScope() | ||
| scope.SetClient(NewNoopClient()) | ||
| return scope | ||
| }() | ||
|
|
||
| // GlobalScope returns the process-wide global scope. | ||
| func GlobalScope() *Scope { | ||
| return globalScope | ||
| } | ||
|
|
||
| func scopeFromContext(ctx context.Context) *Scope { | ||
| if ctx == nil { | ||
| return nil | ||
| } | ||
| scope, _ := ctx.Value(scopesContextKey{}).(*Scope) | ||
| return scope | ||
| } | ||
|
|
||
| // ScopeFromContext returns the isolation scope carried by ctx. If ctx does not | ||
| // carry one, ScopeFromContext returns a derived context carrying a new scope. | ||
| // Callers must propagate the returned context. | ||
| // | ||
| // If ctx already carries an isolation scope, ScopeFromContext returns the exact | ||
| // input context and the existing scope. | ||
| func ScopeFromContext(ctx context.Context) (context.Context, *Scope) { | ||
| if scope := scopeFromContext(ctx); scope != nil { | ||
| return ctx, scope | ||
| } | ||
|
|
||
| scope := NewScope() | ||
| return context.WithValue(ctx, scopesContextKey{}, scope), scope | ||
| } | ||
|
|
||
| // WithIsolation returns a context carrying an independent isolation scope. It | ||
| // clones a scope already carried by ctx or creates a fresh scope when none is | ||
| // carried. | ||
| func WithIsolation(ctx context.Context) context.Context { | ||
| scope := scopeFromContext(ctx) | ||
| if scope == nil { | ||
| scope = NewScope() | ||
| } else { | ||
| scope = scope.Clone() | ||
| } | ||
| return context.WithValue(ctx, scopesContextKey{}, scope) | ||
|
giortzisg marked this conversation as resolved.
|
||
| } | ||
|
|
||
| // WithScope invokes fn with a context carrying a temporary isolation fork and | ||
| // the forked scope. Changes made within fn do not affect the parent. | ||
| func WithScope(ctx context.Context, fn func(context.Context, *Scope)) { | ||
| ctx = WithIsolation(ctx) | ||
| fn(ctx, scopeFromContext(ctx)) | ||
| } | ||
|
giortzisg marked this conversation as resolved.
|
||
|
|
||
| // GetClient resolves the client bound to the isolation or global scope, in that order. | ||
| func GetClient(ctx context.Context) Client { | ||
| if scope := scopeFromContext(ctx); scope != nil { | ||
| if client := scope.client(); client != nil { | ||
| return normalizeClient(client) | ||
| } | ||
| } | ||
| return normalizeClient(GlobalScope().client()) | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,148 @@ | ||
| package sentry | ||
|
|
||
| import ( | ||
| "context" | ||
| "testing" | ||
| ) | ||
|
|
||
| func TestScopeFromContext(t *testing.T) { | ||
| parent := context.Background() | ||
| ctx, scope := ScopeFromContext(parent) | ||
| if ctx == parent || scope == GlobalScope() { | ||
| t.Fatal("scope miss did not create an independent scoped context") | ||
| } | ||
|
|
||
| sameCtx, sameScope := ScopeFromContext(ctx) | ||
| if sameCtx != ctx || sameScope != scope { | ||
| t.Fatal("scope hit did not return the exact context and scope") | ||
| } | ||
|
|
||
| otherCtx, otherScope := ScopeFromContext(parent) | ||
| if otherCtx == ctx || otherScope == scope { | ||
| t.Fatal("independent misses from an unscoped context aliased") | ||
| } | ||
| } | ||
|
|
||
| func TestIsolationScopeSharesDownstreamMutations(t *testing.T) { | ||
| type contextKey struct{} | ||
|
|
||
| ctx, scope := ScopeFromContext(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 := ScopeFromContext(context.Background()) | ||
| parent.SetTag("inherited", "yes") | ||
|
|
||
| firstCtx := WithIsolation(parentCtx) | ||
| secondCtx := WithIsolation(parentCtx) | ||
| _, first := ScopeFromContext(firstCtx) | ||
| _, second := ScopeFromContext(secondCtx) | ||
| first.SetTag("worker", "first") | ||
| second.SetTag("worker", "second") | ||
|
|
||
| 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 data") | ||
| } | ||
| 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") | ||
| } | ||
| } | ||
|
|
||
| func TestWithIsolationDoesNotCloneGlobalScope(t *testing.T) { | ||
| global := GlobalScope() | ||
| global.SetTag("global-only", "yes") | ||
| t.Cleanup(func() { global.RemoveTag("global-only") }) | ||
|
|
||
| ctx := WithIsolation(context.Background()) | ||
| _, scope := ScopeFromContext(ctx) | ||
| 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 TestWithScopeUsesTemporaryIsolation(t *testing.T) { | ||
| ctx, parent := ScopeFromContext(context.Background()) | ||
| parent.SetTag("inherited", "yes") | ||
|
|
||
| WithScope(ctx, func(ctx context.Context, scope *Scope) { | ||
| _, carried := ScopeFromContext(ctx) | ||
| if carried != scope || scope == parent { | ||
| t.Fatal("callback scope is not an independent carried scope") | ||
| } | ||
| if scope.tags["inherited"] != "yes" { | ||
| t.Fatal("callback scope did not inherit parent data") | ||
| } | ||
| scope.SetTag("temporary", "yes") | ||
|
|
||
| WithScope(ctx, func(ctx context.Context, nested *Scope) { | ||
| _, carried := ScopeFromContext(ctx) | ||
| if carried != nested || nested == scope { | ||
| t.Fatal("nested callback scope is not an independent carried scope") | ||
| } | ||
| if nested.tags["temporary"] != "yes" { | ||
| t.Fatal("nested callback scope did not inherit outer data") | ||
| } | ||
| nested.SetTag("nested", "yes") | ||
| }) | ||
|
|
||
| if _, ok := scope.tags["nested"]; ok { | ||
| t.Fatal("nested mutation leaked into outer callback scope") | ||
| } | ||
| }) | ||
|
|
||
| if _, ok := parent.tags["temporary"]; ok { | ||
| t.Fatal("WithScope mutation leaked into parent scope") | ||
| } | ||
| } | ||
|
|
||
| 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) | ||
| } | ||
| childClient, err := NewClient(ClientOptions{}) | ||
| if err != nil { | ||
| t.Fatal(err) | ||
| } | ||
|
|
||
| global := GlobalScope() | ||
| previousGlobal := global.client() | ||
| global.SetClient(globalClient) | ||
| t.Cleanup(func() { global.SetClient(previousGlobal) }) | ||
|
|
||
| ctx, scope := ScopeFromContext(context.Background()) | ||
| if GetClient(ctx) != globalClient { | ||
| t.Fatal("unbound isolation did not fall back to global client") | ||
| } | ||
| scope.SetClient(operationClient) | ||
|
|
||
| childCtx := WithIsolation(ctx) | ||
| _, child := ScopeFromContext(childCtx) | ||
| if GetClient(childCtx) != operationClient { | ||
| t.Fatal("child scope did not inherit client reference") | ||
| } | ||
| child.SetClient(childClient) | ||
| if GetClient(childCtx) != childClient || GetClient(ctx) != operationClient { | ||
| t.Fatal("child client binding was not independent") | ||
| } | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
l: I find this paragraph to be slightly unclear. What is meant by the "underlying data" in this context?