8000 feat: Implement new Async Context Strategy (#10647) · GingerAdonis/sentry-javascript@dc8726a · GitHub
[go: up one dir, main page]
Skip to content

Commit dc8726a

Browse files
mydeamydea
authored
feat: Implement new Async Context Strategy (getsentry#10647)
This updates the ACS to not rely on hubs (only) anymore. Now, instead of the ACS providing only `getCurrentHub` and `runWithAsyncContext`, this is changed a bit: 1. There is always a strategy, even if running in browser or similar. we just use the default (=stack) strategy in that case. 2. The ACS always returns a hub/scope/etc, not `undefined` - so the strategy must take care of falling back to the global hub itself (to make it easier to implement hub<>scope interop). The ACS defines the following methods now: * `getCurrentScope` * `getIsolationScope` * `withScope` * `withSetScope` - a variant that takes a scope and makes it the active one. I decided to make this a dedicated method on the ACS instead of overloading `withScope` because the types for that are rather tricky to repeat in strategies... * `withIsolationScope` * `withSetIsolationScope` - not we do not use this yet, but to keep the door open for us this is already required. * `getCurrentHub` --> for now, for backwards compatibility The methods themselves are pretty straightforward now! We basically always create a new hub, and decide if we want to fork the current/isolation scope based on what method has been used. We still use a hub everywhere under the hood for now.
1 parent cbf64ef commit dc8726a

File tree

80 files changed

+1215
-839
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

80 files changed

+1215
-839
lines changed

‎dev-packages/e2e-tests/test-applications/nextjs-app-dir/pages/api/async-context-edge-endpoint.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,16 +5,16 @@ export const config = {
55
};
66

77
export default async function handler() {
8-
// Without `runWithAsyncContext` and a working async context strategy the two spans created by `Sentry.startSpan()` would be nested.
8+
// Without a working async context strategy the two spans created by `Sentry.startSpan()` would be nested.
99

10-
const outerSpanPromise = Sentry.runWithAsyncContext(() => {
10+
const outerSpanPromise = Sentry.withIsolationScope(() => {
1111
return Sentry.startSpan({ name: 'outer-span' }, () => {
1212
return new Promise<void>(resolve => setTimeout(resolve, 300));
1313
});
1414
});
1515

1616
setTimeout(() => {
17-
Sentry.runWithAsyncContext(() => {
17+
Sentry.withIsolationScope(() => {
1818
return Sentry.startSpan({ name: 'inner-span' }, () => {
1919
return new Promise<void>(resolve => setTimeout(resolve, 100));
2020
});

‎dev-packages/node-integration-tests/suites/public-api/scopes/isolationScope/scenario.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ Sentry.withScope(scope => {
2121
Sentry.captureMessage('inner');
2222
});
2323

24-
Sentry.runWithAsyncContext(() => {
24+
Sentry.withIsolationScope(() => {
2525
Sentry.getIsolationScope().setExtra('ff', 'ff');
2626
Sentry.getCurrentScope().setExtra('gg', 'gg');
2727
Sentry.captureMessage('inner_async_context');

‎packages/astro/src/server/middleware.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@ import {
55
getActiveSpan,
66
getClient,
77
getCurrentScope,
8-
runWithAsyncContext,
98
startSpan,
9+
withIsolationScope,
1010
} from '@sentry/node';
1111
import type { Client, Scope, Span } from '@sentry/types';
1212
import { addNonEnumerableProperty, objectify, stripUrlQueryAndFragment } from '@sentry/utils';
@@ -74,7 +74,7 @@ export const handleRequest: (options?: MiddlewareOptions) => MiddlewareResponseH
7474
if (getActiveSpan()) {
7575
return instrumentRequest(ctx, next, handlerOptions);
7676
}
77-
return runWithAsyncContext(() => {
77+
return withIsolationScope(() => {
7878
return instrumentRequest(ctx, next, handlerOptions);
7979
});
8080
};

‎packages/astro/test/client/sdk.test.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,10 @@ import type { BrowserClient } from '@sentry/browser';
22
import { getActiveSpan } from '@sentry/browser';
33
import { browserTracingIntegration } from '@sentry/browser';
44
import * as SentryBrowser from '@sentry/browser';
5-
import { SDK_VERSION, WINDOW, getClient } from '@sentry/browser';
5+
import { SDK_VERSION, getClient } from '@sentry/browser';
66
import { vi } from 'vitest';
77

8-
import { getIsolationScope } from '@sentry/core';
8+
import { getCurrentScope, getGlobalScope, getIsolationScope } from '@sentry/core';
99
import { init } from '../../../astro/src/client/sdk';
1010

1111
const browserInit = vi.spyOn(SentryBrowser, 'init');
@@ -14,7 +14,11 @@ describe('Sentry client SDK', () => {
1414
describe('init', () => {
1515
afterEach(() => {
1616
vi.clearAllMocks();
17-
WINDOW.__SENTRY__.hub = undefined;
17+
18+
getCurrentScope().clear();
19+
getCurrentScope().setClient(undefined);
20+
getIsolationScope().clear();
21+
getGlobalScope().clear();
1822
});
1923

2024
it('adds Astro metadata to the SDK options', () => {

‎packages/astro/test/server/middleware.test.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -260,10 +260,10 @@ describe('sentryMiddleware', () => {
260260
});
261261

262262
describe('async context isolation', () => {
263-
const runWithAsyncContextSpy = vi.spyOn(SentryNode, 'runWithAsyncContext');
263+
const withIsolationScopeSpy = vi.spyOn(SentryNode, 'withIsolationScope');
264264
afterEach(() => {
265265
vi.clearAllMocks();
266-
runWithAsyncContextSpy.mockRestore();
266+
withIsolationScopeSpy.mockRestore();
267267
});
268268

269269
it('starts a new async context if no span is active', async () => {
@@ -279,7 +279,7 @@ describe('sentryMiddleware', () => {
279279
// this is fine, it's not required to pass in this test
280280
}
281281

282-
expect(runWithAsyncContextSpy).toHaveBeenCalledTimes(1);
282+
expect(withIsolationScopeSpy).toHaveBeenCalledTimes(1);
283283
});
284284

285285
it("doesn't start a new async context if a span is active", async () => {
@@ -297,7 +297,7 @@ describe('sentryMiddleware', () => {
297297
// this is fine, it's not required to pass in this test
298298
}
299299

300-
expect(runWithAsyncContextSpy).not.toHaveBeenCalled();
300+
expect(withIsolationScopeSpy).not.toHaveBeenCalled();
301301
});
302302
});
303303
});

‎packages/astro/test/server/sdk.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ describe('Sentry server SDK', () => {
1414
SentryNode.getGlobalScope().clear();
1515
SentryNode.getIsolationScope().clear();
1616
SentryNode.getCurrentScope().clear();
17+
SentryNode.getCurrentScope().setClient(undefined);
1718
});
1819

1920
it('adds Astro metadata to the SDK options', () => {

‎packages/browser/src/exports.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@ export {
4141
getGlobalScope,
4242
Hub,
4343
// eslint-disable-next-line deprecation/deprecation
44-
// eslint-disable-next-line deprecation/deprecation
4544
makeMain,
4645
setCurrentClient,
4746
Scope,

‎packages/browser/test/unit/index.test.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { InboundFilters, SDK_VERSION, getReportDialogEndpoint } from '@sentry/core';
1+
import { InboundFilters, SDK_VERSION, getGlobalScope, getIsolationScope, getReportDialogEndpoint } from '@sentry/core';
22
import type { WrappedFunction } from '@sentry/types';
33
import * as utils from '@sentry/utils';
44

@@ -39,7 +39,11 @@ describe('SentryBrowser', () => {
3939
const beforeSend = jest.fn(event => event);
4040

4141
beforeEach(() => {
42-
WINDOW.__SENTRY__ = { hub: undefined, logger: undefined, globalEventProcessors: [] };
42+
getGlobalScope().clear();
43+
getIsolationScope().clear();
44+
getCurrentScope().clear();
45+
getCurrentScope().setClient(undefined);
46+
4347
init({
4448
beforeSend,
4549
dsn,

‎packages/bun/src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ export {
4747
// eslint-disable-next-line deprecation/deprecation
4848
makeMain,
4949
setCurrentClient,
50+
// eslint-disable-next-line deprecation/deprecation
5051
runWithAsyncContext,
5152
Scope,
5253
// eslint-disable-next-line deprecation/deprecation

‎packages/bun/src/integrations/bunserver.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,9 @@ import {
77
convertIntegrationFnToClass,
88
defineIntegration,
99
getCurrentScope,
10-
runWithAsyncContext,
1110
setHttpStatus,
1211
startSpan,
12+
withIsolationScope,
1313
} from '@sentry/core';
1414
import type { IntegrationFn } from '@sentry/types';
1515
import { getSanitizedUrlString, parseUrl } from '@sentry/utils';
@@ -53,7 +53,7 @@ export function instrumentBunServe(): void {
5353
function instrumentBunServeOptions(serveOptions: Parameters<typeof Bun.serve>[0]): void {
5454
serveOptions.fetch = new Proxy(serveOptions.fetch, {
5555
apply(fetchTarget, fetchThisArg, fetchArgs: Parameters<typeof serveOptions.fetch>) {
56-
return runWithAsyncContext(() => {
56+
return withIsolationScope(() => {
5757
const request = fetchArgs[0];
5858
const upperCaseMethod = request.method.toUpperCase();
5959
if (upperCaseMethod === 'OPTIONS' || upperCaseMethod === 'HEAD') {

‎packages/core/src/asyncContext.ts

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import type { Hub, Integration } from '@sentry/types';
2+
import type { Scope } from '@sentry/types';
23
import { GLOBAL_OBJ } from '@sentry/utils';
34

45
/**
@@ -8,14 +9,39 @@ import { GLOBAL_OBJ } from '@sentry/utils';
89
*/
910
export interface AsyncContextStrategy {
1011
/**
11-
* Gets the current async context. Returns undefined if there is no current async context.
12+
* Gets the currently active hub.
1213
*/
13-
getCurrentHub: () => Hub | undefined;
14+
getCurrentHub: () => Hub;
1415

1516
/**
16-
* Runs the supplied callback in its own async context.
17+
* Fork the isolation scope inside of the provided callback.
1718
*/
18-
runWithAsyncContext<T>(callback: () => T): T;
19+
withIsolationScope: <T>(callback: (isolationScope: Scope) => T) => T;
20+
21+
/**
22+
* Fork the current scope inside of the provided callback.
23+
*/
24+
withScope: <T>(callback: (isolationScope: Scope) => T) => T;
25+
26+
/**
27+
* Set the provided scope as the current scope inside of the provided callback.
28+
*/
29+
withSetScope: <T>(scope: Scope, callback: (scope: Scope) => T) => T;
30+
31+
/**
32+
* Set the provided isolation as the current isolation scope inside of the provided callback.
33+
*/
34+
withSetIsolationScope: <T>(isolationScope: Scope, callback: (isolationScope: Scope) => T) => T;
35+
36+
/**
37+
* Get the currently active scope.
38+
*/
39+
getCurrentScope: () => Scope;
40+
41+
/**
42+
* Get the currently active isolation scope.
43+
*/
44+
getIsolationScope: () => Scope;
1945
}
2046

2147
/**

‎packages/core/src/breadcrumbs.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import type { Breadcrumb, BreadcrumbHint } from '@sentry/types';
22
import { consoleSandbox, dateTimestampInSeconds } from '@sentry/utils';
3-
import { getIsolationScope } from './currentScopes';
4-
import { getClient } from './exports';
3+
import { getClient, getIsolationScope } from './currentScopes';
54

65
/**
76
* Default maximum number of breadcrumbs added to an event. Can be overwritten

‎packages/core/src/currentScopes.ts

Lines changed: 82 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
import type { Scope } from '@sentry/types';
2-
import { getCurrentHub } from './hub';
2+
import type { Client } from '@sentry/types';
3+
import { getMainCarrier } from './asyncContext';
4+
import { getAsyncContextStrategy } from './hub';
35
import { Scope as ScopeClass } from './scope';
46

57
/**
@@ -12,17 +14,19 @@ let globalScope: Scope | undefined;
1214
* Get the currently active scope.
1315
*/
1416
export function getCurrentScope(): Scope {
15-
// eslint-disable-next-line deprecation/deprecation
16-
return getCurrentHub().getScope();
17+
const carrier = getMainCarrier();
18+
const acs = getAsyncContextStrategy(carrier);
19+
return acs.getCurrentScope();
1720
}
1821

1922
/**
2023
* Get the currently active isolation scope.
2124
* The isolation scope is active for the current exection context.
2225
*/
2326
export function getIsolationScope(): Scope {
24-
// eslint-disable-next-line deprecation/deprecation
25-
return getCurrentHub().getIsolationScope();
27+
const carrier = getMainCarrier();
28+
const acs = getAsyncContextStrategy(carrier);
29+
return acs.getIsolationScope();
2630
}
2731

2832
/**
@@ -45,3 +49,76 @@ export function getGlobalScope(): Scope {
4549
export function setGlobalScope(scope: Scope | undefined): void {
4650
globalScope = scope;
4751
}
52+
53+
/**
54+
* Creates a new scope with and executes the given operation within.
55+
* The scope is automatically removed once the operation
56+
* finishes or throws.
57+
*/
58+
export function withScope<T>(callback: (scope: Scope) => T): T;
59+
/**
60+
* Set the given scope as the active scope in the callback.
61+
*/
62+
export function withScope<T>(scope: Scope | undefined, callback: (scope: Scope) => T): T;
63+
/**
64+
* Either creates a new active scope, or sets the given scope as active scope in the given callback.
65+
*/
66+
export function withScope<T>(
67+
...rest: [callback: (scope: Scope) => T] | [scope: Scope | undefined, callback: (scope: Scope) => T]
68+
): T {
69+
const carrier = getMainCarrier();
70+
const acs = getAsyncContextStrategy(carrier);
71+
72+
// If a scope is defined, we want to make this the active scope instead of the default one
73+
if (rest.length === 2) {
74+
const [scope, callback] = rest;
75+
76+
if (!scope) {
77+
return acs.withScope(callback);
78+
}
79+
80+
return acs.withSetScope(scope, callback);
81+
}
82+
83+
return acs.withScope(rest[0]);
84+
}
85+
86+
/**
87+
* Attempts to fork the current isolation scope and the current scope based on the current async context strategy. If no
88+
* async context strategy is set, the isolation scope and the current scope will not be forked (this is currently the
89+
* case, for example, in the browser).
90+
*
91+
* Usage of this function in environments without async context strategy is discouraged and may lead to unexpected behaviour.
92+
*
93+
* This function is intended for Sentry SDK and SDK integration development. It is not recommended to be used in "normal"
94+
* applications directly because it comes with pitfalls. Use at your own risk!
95+
*
96+
* @param callback The callback in which the passed isolation scope is active. (Note: In environments without async
97+
* context strategy, the currently active isolation scope may change within execution of the callback.)
98+
* @returns The same value that `callback` returns.
99+
*/
100+
export function withIsolationScope<T>(callback: (isolationScope: Scope) => T): T {
101+
const carrier = getMainCarrier();
102+
const acs = getAsyncContextStrategy(carrier);
103+
return acs.withIsolationScope(callback);
104+
}
105+
106+
/**
107+
* Runs the supplied callback in its own async context. Async Context strategies are defined per SDK.
108+
*
109+
* @param callback The callback to run in its own async context
110+
* @param options Options to pass to the async context strategy
111+
* @returns The result of the callback
112+
*
113+
* @deprecated Use `Sentry.withScope()` instead.
114+
*/
115+
export function runWithAsyncContext<T>(callback: () => T): T {
116+
return withScope(() => callback());
117+
}
118+
119+
/**
120+
* Get the currently active client.
121+
*/
122+
export function getClient<C extends Client>(): C | undefined {
123+
return getCurrentScope().getClient<C>();
124+
}

0 commit comments

Comments
 (0)
0