-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
ref(core): Rename Hub
to AsyncContextStack
& remove unneeded methods
#11630
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
Merged
Merged
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
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 was deleted.
Oops, something went wrong.
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,31 @@ | ||
import type { Carrier } from './../carrier'; | ||
import { getMainCarrier, getSentryCarrier } from './../carrier'; | ||
import { getStackAsyncContextStrategy } from './stackStrategy'; | ||
import type { AsyncContextStrategy } from './types'; | ||
|
||
/** | ||
* @private Private API with no semver guarantees! | ||
* | ||
* Sets the global async context strategy | ||
*/ | ||
export function setAsyncContextStrategy(strategy: AsyncContextStrategy | undefined): void { | ||
// Get main carrier (global for every environment) | ||
const registry = getMainCarrier(); | ||
const sentry = getSentryCarrier(registry); | ||
sentry.acs = strategy; | ||
} | ||
|
||
/** | ||
* Get the current async context strategy. | ||
* If none has been setup, the default will be used. | ||
*/ | ||
export function getAsyncContextStrategy(carrier: Carrier): AsyncContextStrategy { | ||
const sentry = getSentryCarrier(carrier); | ||
|
||
if (sentry.acs) { | ||
return sentry.acs; | ||
} | ||
|
||
// Otherwise, use the default one (stack) | ||
return getStackAsyncContextStrategy(); | ||
} |
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,178 @@ | ||
import type { Client, Scope as ScopeInterface } from '@sentry/types'; | ||
import { isThenable } from '@sentry/utils'; | ||
import { getDefaultCurrentScope, getDefaultIsolationScope } from '../currentScopes'; | ||
import { Scope } from '../scope'; | ||
|
||
import { getMainCarrier, getSentryCarrier } from './../carrier'; | ||
import type { AsyncContextStrategy } from './types'; | ||
|
||
interface Layer { | ||
client?: Client; | ||
scope: ScopeInterface; | ||
} | ||
|
||
/** | ||
* This is an object that holds a stack of scopes. | ||
*/ | ||
export class AsyncContextStack { | ||
private readonly _stack: Layer[]; | ||
private _isolationScope: ScopeInterface; | ||
|
||
public constructor(scope?: ScopeInterface, isolationScope?: ScopeInterface) { | ||
let assignedScope; | ||
if (!scope) { | ||
assignedScope = new Scope(); | ||
} else { | ||
assignedScope = scope; | ||
} | ||
|
||
let assignedIsolationScope; | ||
if (!isolationScope) { | ||
assignedIsolationScope = new Scope(); | ||
} else { | ||
assignedIsolationScope = isolationScope; | ||
} | ||
|
||
this._stack = [{ scope: assignedScope }]; | ||
this._isolationScope = assignedIsolationScope; | ||
} | ||
|
||
/** | ||
* Fork a scope for the stack. | ||
*/ | ||
public withScope<T>(callback: (scope: ScopeInterface) => T): T { | ||
const scope = this._pushScope(); | ||
|
||
let maybePromiseResult: T; | ||
try { | ||
maybePromiseResult = callback(scope); | ||
} catch (e) { | ||
this._popScope(); | ||
throw e; | ||
} | ||
|
||
if (isThenable(maybePromiseResult)) { | ||
// @ts-expect-error - isThenable returns the wrong type | ||
return maybePromiseResult.then( | ||
res => { | ||
this._popScope(); | ||
return res; | ||
}, | ||
e => { | ||
this._popScope(); | ||
throw e; | ||
}, | ||
); | ||
} | ||
|
||
this._popScope(); | ||
return maybePromiseResult; | ||
} | ||
|
||
/** | ||
* Get the client of the stack. | ||
*/ | ||
public getClient<C extends Client>(): C | undefined { | ||
return this.getStackTop().client as C; | ||
} | ||
|
||
/** | ||
* Returns the scope of the top stack. | ||
*/ | ||
public getScope(): ScopeInterface { | ||
return this.getStackTop().scope; | ||
} | ||
|
||
/** | ||
* Get the isolation scope for the stack. | ||
*/ | ||
public getIsolationScope(): ScopeInterface { | ||
return this._isolationScope; | ||
} | ||
|
||
/** | ||
* Returns the scope stack for domains or the process. | ||
*/ | ||
public getStack(): Layer[] { | ||
return this._stack; | ||
} | ||
|
||
/** | ||
* Returns the topmost scope layer in the order domain > local > process. | ||
*/ | ||
public getStackTop(): Layer { | ||
return this._stack[this._stack.length - 1]; | ||
} | ||
|
||
/** | ||
* Push a scope to the stack. | ||
*/ | ||
private _pushScope(): ScopeInterface { | ||
// We want to clone the content of prev scope | ||
const scope = this.getScope().clone(); | ||
this.getStack().push({ | ||
client: this.getClient(), | ||
scope, | ||
}); | ||
return scope; | ||
} | ||
|
||
/** | ||
* Pop a scope from the stack. | ||
*/ | ||
private _popScope(): boolean { | ||
if (this.getStack().length <= 1) return false; | ||
return !!this.getStack().pop(); | ||
} | ||
} | ||
|
||
/** | ||
* Get the global async context stack. | ||
* This will be removed during the v8 cycle and is only here to make migration easier. | ||
*/ | ||
function getAsyncContextStack(): AsyncContextStack { | ||
const registry = getMainCarrier(); | ||
const sentry = getSentryCarrier(registry) as { hub?: AsyncContextStack }; | ||
|
||
// If there's no hub, or its an old API, assign a new one | ||
if (sentry.hub) { | ||
return sentry.hub; | ||
} | ||
|
||
sentry.hub = new AsyncContextStack(getDefaultCurrentScope(), getDefaultIsolationScope()); | ||
return sentry.hub; | ||
} | ||
|
||
function withScope<T>(callback: (scope: ScopeInterface) => T): T { | ||
return getAsyncContextStack().withScope(callback); | ||
} | ||
|
||
function withSetScope<T>(scope: ScopeInterface, callback: (scope: ScopeInterface) => T): T { | ||
const hub = getAsyncContextStack() as AsyncContextStack; | ||
return hub.withScope(() => { | ||
hub.getStackTop().scope = scope; | ||
return callback(scope); | ||
}); | ||
} | ||
|
||
function withIsolationScope<T>(callback: (isolationScope: ScopeInterface) => T): T { | ||
return getAsyncContextStack().withScope(() => { | ||
return callback(getAsyncContextStack().getIsolationScope()); | ||
}); | ||
} | ||
|
||
/** | ||
* Get the stack-based async context strategy. | ||
*/ | ||
export function getStackAsyncContextStrategy(): AsyncContextStrategy { | ||
return { | ||
withIsolationScope, | ||
withScope, | ||
withSetScope, | ||
withSetIsolationScope: <T>(_isolationScope: ScopeInterface, callback: (isolationScope: ScopeInterface) => T) => { | ||
return withIsolationScope(callback); | ||
}, | ||
getCurrentScope: () => getAsyncContextStack().getScope(), | ||
getIsolationScope: () => getAsyncContextStack().getIsolationScope(), | ||
}; | ||
} |
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,67 @@ | ||
import type { Scope } from '@sentry/types'; | ||
import type { | ||
startInactiveSpan, | ||
startSpan, | ||
startSpanManual, | ||
suppressTracing, | ||
withActiveSpan, | ||
} from './../tracing/trace'; | ||
import type { getActiveSpan } from './../utils/spanUtils'; | ||
|
||
/** | ||
* @private Private API with no semver guarantees! | ||
* | ||
* Strategy used to track async context. | ||
*/ | ||
export interface AsyncContextStrategy { | ||
/** | ||
* Fork the isolation scope inside of the provided callback. | ||
*/ | ||
withIsolationScope: <T>(callback: (isolationScope: Scope) => T) => T; | ||
|
||
/** | ||
* Fork the current scope inside of the provided callback. | ||
*/ | ||
withScope: <T>(callback: (isolationScope: Scope) => T) => T; | ||
|
||
/** | ||
* Set the provided scope as the current scope inside of the provided callback. | ||
*/ | ||
withSetScope: <T>(scope: Scope, callback: (scope: Scope) => T) => T; | ||
|
||
/** | ||
* Set the provided isolation as the current isolation scope inside of the provided callback. | ||
*/ | ||
withSetIsolationScope: <T>(isolationScope: Scope, callback: (isolationScope: Scope) => T) => T; | ||
|
||
/** | ||
* Get the currently active scope. | ||
*/ | ||
getCurrentScope: () => Scope; | ||
|
||
/** | ||
* Get the currently active isolation scope. | ||
*/ | ||
getIsolationScope: () => Scope; | ||
|
||
// OPTIONAL: Custom tracing methods | ||
// These are used so that we can provide OTEL-based implementations | ||
|
||
/** Start an active span. */ | ||
startSpan?: typeof startSpan; | ||
|
||
/** Start an inactive span. */ | ||
startInactiveSpan?: typeof startInactiveSpan; | ||
|
||
/** Start an active manual span. */ | ||
startSpanManual?: typeof startSpanManual; | ||
|
||
/** Get the currently active span. */ | ||
getActiveSpan?: typeof getActiveSpan; | ||
|
||
/** Make a span the active span in the context of the callback. */ | ||
withActiveSpan?: typeof withActiveSpan; | ||
|
||
/** Suppress tracing in the given callback, ensuring no spans are generated inside of it. */ | ||
suppressTracing?: typeof suppressTracing; | ||
} |
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.
can we stop referencing it by
.hub
here?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.
sadly not because some stuff like the loader depends on this 😬 @Lms24 learned this the hard way I believe?
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.
that's also the reason that this still has a
getClient()
method!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.
yes, if we remove/rename
.hub
our loader tests will fail because our loader script accesses__SENTRY__.hub.getClient()
to check if the SDK was initialized before the loader otherwise initializes the SDK (at least that's what I think it does; the script certainly makes the access). So I guess we can only remove this once we update the loader script 🤔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.
we can still do this when v8 becomes stable I guess, but it's possibly not worth it too as it may break stuff weirdly I guess that depends on internals 😅
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.
could we leave a comment about this somewhere?
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.
See #11633 👍