Skip to content

Commit 8329231

Browse files
committed
feat(core): Allow to pass scope to startSpan APIs
Also allow to pass this to `withScope()`, which actually aligns with OTEL as well, where you can also do that. If you pass a scope there, it will use this instead of forking a new one.
1 parent 118b5c7 commit 8329231

File tree

7 files changed

+200
-43
lines changed

7 files changed

+200
-43
lines changed

packages/core/src/exports.ts

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import type {
1212
FinishedCheckIn,
1313
MonitorConfig,
1414
Primitive,
15+
Scope as ScopeInterface,
1516
Severity,
1617
SeverityLevel,
1718
TransactionContext,
@@ -164,11 +165,33 @@ export function setUser(user: User | null): ReturnType<Hub['setUser']> {
164165
* pushScope();
165166
* callback();
166167
* popScope();
167-
*
168-
* @param callback that will be enclosed into push/popScope.
169168
*/
170-
export function withScope<T>(callback: (scope: Scope) => T): T {
171-
return getCurrentHub().withScope(callback);
169+
export function withScope<T>(callback: (scope: Scope) => T): T;
170+
/**
171+
* Set the given scope as the active scope in the callback.
172+
*/
173+
export function withScope<T>(scope: ScopeInterface | undefined, callback: (scope: Scope) => T): T;
174+
/**
175+
* Either creates a new active scope, or sets the given scope as active scope in the given callback.
176+
*/
177+
export function withScope<T>(
178+
...rest: [callback: (scope: Scope) => T] | [scope: ScopeInterface | undefined, callback: (scope: Scope) => T]
179+
): T {
180+
// If a scope is defined, we want to make this the active scope instead of the default one
181+
if (rest.length === 2) {
182+
const [scope, callback] = rest;
183+
if (!scope) {
184+
return getCurrentHub().withScope(callback);
185+
}
186+
187+
const hub = getCurrentHub();
188+
return hub.withScope(() => {
189+
hub.getStackTop().scope = scope as Scope;
190+
return callback(scope as Scope);
191+
});
192+
}
193+
194+
return getCurrentHub().withScope(rest[0]);
172195
}
173196

174197
/**

packages/core/src/tracing/trace.ts

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import type { Span, SpanTimeInput, TransactionContext } from '@sentry/types';
1+
import type { Scope, Span, SpanTimeInput, TransactionContext } from '@sentry/types';
22
import { dropUndefinedKeys, logger, tracingContextFromHeaders } from '@sentry/utils';
33

44
import { DEBUG_BUILD } from '../debug-build';
@@ -12,6 +12,9 @@ import { spanTimeInputToSeconds } from '../utils/spanUtils';
1212
interface StartSpanOptions extends TransactionContext {
1313
/** A manually specified start time for the created `Span` object. */
1414
startTime?: SpanTimeInput;
15+
16+
/** If defined, start this span off this scope instead off the current scope. */
17+
scope?: Scope;
1518
}
1619

1720
/**
@@ -74,9 +77,10 @@ export function trace<T>(
7477
export function startSpan<T>(context: StartSpanOptions, callback: (span: Span | undefined) => T): T {
7578
const ctx = normalizeContext(context);
7679

77-
return withScope(scope => {
80+
return withScope(context.scope, scope => {
7881
const hub = getCurrentHub();
79-
const parentSpan = scope.getSpan();
82+
const scopeForSpan = context.scope || scope;
83+
const parentSpan = scopeForSpan.getSpan();
8084

8185
const activeSpan = createChildSpanOrTransaction(hub, parentSpan, ctx);
8286
scope.setSpan(activeSpan);
@@ -116,7 +120,7 @@ export function startSpanManual<T>(
116120
): T {
117121
const ctx = normalizeContext(context);
118122

119-
return withScope(scope => {
123+
return withScope(context.scope, scope => {
120124
const hub = getCurrentHub();
121125
const parentSpan = scope.getSpan();
122126

@@ -156,7 +160,7 @@ export function startInactiveSpan(context: StartSpanOptions): Span | undefined {
156160

157161
const ctx = normalizeContext(context);
158162
const hub = getCurrentHub();
159-
const parentSpan = getActiveSpan();
163+
const parentSpan = context.scope ? context.scope.getSpan() : getActiveSpan();
160164
return parentSpan
161165
? parentSpan.startChild(ctx)
162166
: // eslint-disable-next-line deprecation/deprecation

packages/core/test/lib/exports.test.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,4 +124,28 @@ describe('withScope', () => {
124124

125125
expect(getCurrentScope()).toBe(scope1);
126126
});
127+
128+
it('allows to pass a custom scope', () => {
129+
const scope1 = getCurrentScope();
130+
scope1.setExtra('x1', 'x1');
131+
132+
const customScope = new Scope();
133+
customScope.setExtra('x2', 'x2');
134+
135+
withScope(customScope, scope2 => {
136+
expect(scope2).not.toBe(scope1);
137+
expect(scope2).toBe(customScope);
138+
expect(getCurrentScope()).toBe(scope2);
139+
expect(scope2['_extra']).toEqual({ x2: 'x2' });
140+
});
141+
142+
withScope(customScope, scope3 => {
143+
expect(scope3).not.toBe(scope1);
144+
expect(scope3).toBe(customScope);
145+
expect(getCurrentScope()).toBe(scope3);
146+
expect(scope3['_extra']).toEqual({ x2: 'x2' });
147+
});
148+
149+
expect(getCurrentScope()).toBe(scope1);
150+
});
127151
});

packages/core/test/lib/tracing/trace.test.ts

Lines changed: 90 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
1-
import type { Span } from '@sentry/types';
21
import { Hub, addTracingExtensions, getCurrentScope, makeMain } from '../../../src';
3-
import { continueTrace, startInactiveSpan, startSpan, startSpanManual } from '../../../src/tracing';
2+
import { Scope } from '../../../src/scope';
3+
import { Span, continueTrace, startInactiveSpan, startSpan, startSpanManual } from '../../../src/tracing';
44
import { TestClient, getDefaultTestClientOptions } from '../../mocks/client';
55

66
beforeAll(() => {
@@ -81,18 +81,6 @@ describe('startSpan', () => {
8181
expect(ref.status).toEqual(isError ? 'internal_error' : undefined);
8282
});
8383

84-
it('creates & finishes span', async () => {
85-
let _span: Span | undefined;
86-
startSpan({ name: 'GET users/[id]' }, span => {
87-
expect(span).toBeDefined();
88-
expect(span?.endTimestamp).toBeUndefined();
89-
_span = span;
90-
});
91-
92-
expect(_span).toBeDefined();
93-
expect(_span?.endTimestamp).toBeDefined();
94-
});
95-
9684
it('allows traceparent information to be overriden', async () => {
9785
let ref: any = undefined;
9886
client.on('finishTransaction', transaction => {
@@ -160,14 +148,6 @@ describe('startSpan', () => {
160148
expect(ref.spanRecorder.spans[1].status).toEqual(isError ? 'internal_error' : undefined);
161149
});
162150

163-
it('allows to pass a `startTime`', () => {
164-
const start = startSpan({ name: 'outer', startTime: [1234, 0] }, span => {
165-
return span?.startTimestamp;
166-
});
167-
168-
expect(start).toEqual(1234);
169-
});
170-
171151
it('allows for span to be mutated', async () => {
172152
let ref: any = undefined;
173153
client.on('finishTransaction', transaction => {
@@ -189,18 +169,57 @@ describe('startSpan', () => {
189169
expect(ref.spanRecorder.spans).toHaveLength(2);
190170
expect(ref.spanRecorder.spans[1].op).toEqual('db.query');
191171
});
172+
});
192173

193-
it('forks the scope', () => {
194-
const initialScope = getCurrentScope();
174+
it('creates & finishes span', async () => {
175+
let _span: Span | undefined;
176+
startSpan({ name: 'GET users/[id]' }, span => {
177+
expect(span).toBeDefined();
178+
expect(span?.endTimestamp).toBeUndefined();
179+
_span = span as Span;
180+
});
195181

196-
startSpan({ name: 'GET users/[id]' }, span => {
197-
expect(getCurrentScope()).not.toBe(initialScope);
198-
expect(getCurrentScope().getSpan()).toBe(span);
199-
});
182+
expect(_span).toBeDefined();
183+
expect(_span?.endTimestamp).toBeDefined();
184+
});
200185

201-
expect(getCurrentScope()).toBe(initialScope);
202-
expect(initialScope.getSpan()).toBe(undefined);
186+
it('allows to pass a `startTime`', () => {
187+
const start = startSpan({ name: 'outer', startTime: [1234, 0] }, span => {
188+
return span?.startTimestamp;
203189
});
190+
191+
expect(start).toEqual(1234);
192+
});
193+
194+
it('forks the scope', () => {
195+
const initialScope = getCurrentScope();
196+
197+
startSpan({ name: 'GET users/[id]' }, span => {
198+
expect(getCurrentScope()).not.toBe(initialScope);
199+
expect(getCurrentScope().getSpan()).toBe(span);
200+
});
201+
202+
expect(getCurrentScope()).toBe(initialScope);
203+
expect(initialScope.getSpan()).toBe(undefined);
204+
});
205+
206+
it('allows to pass a scope', () => {
207+
const initialScope = getCurrentScope();
208+
209+
const manualScope = new Scope();
210+
const parentSpan = new Span({ spanId: 'parent-span-id' });
211+
manualScope.setSpan(parentSpan);
212+
213+
startSpan({ name: 'GET users/[id]', scope: manualScope }, span => {
214+
expect(getCurrentScope()).not.toBe(initialScope);
215+
expect(getCurrentScope()).toBe(manualScope);
216+
expect(getCurrentScope().getSpan()).toBe(span);
217+
218+
expect(span?.parentSpanId).toBe('parent-span-id');
219+
});
220+
221+
expect(getCurrentScope()).toBe(initialScope);
222+
expect(initialScope.getSpan()).toBe(undefined);
204223
});
205224
});
206225

@@ -231,6 +250,29 @@ describe('startSpanManual', () => {
231250
expect(initialScope.getSpan()).toBe(undefined);
232251
});
233252

253+
it('allows to pass a scope', () => {
254+
const initialScope = getCurrentScope();
255+
256+
const manualScope = new Scope();
257+
const parentSpan = new Span({ spanId: 'parent-span-id' });
258+
manualScope.setSpan(parentSpan);
259+
260+
startSpanManual({ name: 'GET users/[id]', scope: manualScope }, (span, finish) => {
261+
expect(getCurrentScope()).not.toBe(initialScope);
262+
expect(getCurrentScope()).toBe(manualScope);
263+
expect(getCurrentScope().getSpan()).toBe(span);
264+
expect(span?.parentSpanId).toBe('parent-span-id');
265+
266+
finish();
267+
268+
// Is still the active span
269+
expect(getCurrentScope().getSpan()).toBe(span);
270+
});
271+
272+
expect(getCurrentScope()).toBe(initialScope);
273+
expect(initialScope.getSpan()).toBe(undefined);
274+
});
275+
234276
it('allows to pass a `startTime`', () => {
235277
const start = startSpanManual({ name: 'outer', startTime: [1234, 0] }, span => {
236278
span?.end();
@@ -266,6 +308,24 @@ describe('startInactiveSpan', () => {
266308
expect(initialScope.getSpan()).toBeUndefined();
267309
});
268310

311+
it('allows to pass a scope', () => {
312+
const initialScope = getCurrentScope();
313+
314+
const manualScope = new Scope();
315+
const parentSpan = new Span({ spanId: 'parent-span-id' });
316+
manualScope.setSpan(parentSpan);
317+
318+
const span = startInactiveSpan({ name: 'GET users/[id]', scope: manualScope });
319+
320+
expect(span).toBeDefined();
321+
expect(span?.parentSpanId).toBe('parent-span-id');
322+
expect(initialScope.getSpan()).toBeUndefined();
323+
324+
span?.end();
325+
326+
expect(initialScope.getSpan()).toBeUndefined();
327+
});
328+
269329
it('allows to pass a `startTime`', () => {
270330
const span = startInactiveSpan({ name: 'outer', startTime: [1234, 0] });
271331
expect(span?.startTimestamp).toEqual(1234);

packages/node-experimental/src/sdk/api.ts

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import type {
1818
User,
1919
} from '@sentry/types';
2020
import { GLOBAL_OBJ, consoleSandbox, dateTimestampInSeconds } from '@sentry/utils';
21-
import { getScopesFromContext, setScopesOnContext } from '../utils/contextData';
21+
import { getContextFromScope, getScopesFromContext, setScopesOnContext } from '../utils/contextData';
2222

2323
import type { ExclusiveEventHintOrCaptureContext } from '../utils/prepareEvent';
2424
import { parseEventHintOrCaptureContext } from '../utils/prepareEvent';
@@ -29,9 +29,39 @@ export { getCurrentScope, getGlobalScope, getIsolationScope, getClient };
2929
export { setCurrentScope, setIsolationScope } from './scope';
3030

3131
/**
32-
* Fork a scope from the current scope, and make it the current scope in the given callback
32+
* Creates a new scope with and executes the given operation within.
33+
* The scope is automatically removed once the operation
34+
* finishes or throws.
35+
*
36+
* This is essentially a convenience function for:
37+
*
38+
* pushScope();
39+
* callback();
40+
* popScope();
3341
*/
34-
export function withScope<T>(callback: (scope: Scope) => T): T {
42+
export function withScope<T>(callback: (scope: Scope) => T): T;
43+
/**
44+
* Set the given scope as the active scope in the callback.
45+
*/
46+
export function withScope<T>(scope: Scope | undefined, callback: (scope: Scope) => T): T;
47+
/**
48+
* Either creates a new active scope, or sets the given scope as active scope in the given callback.
49+
*/
50+
export function withScope<T>(
51+
...rest: [callback: (scope: Scope) => T] | [scope: Scope | undefined, callback: (scope: Scope) => T]
52+
): T {
53+
// If a scope is defined, we want to make this the active scope instead of the default one
54+
if (rest.length === 2) {
55+
const [scope, callback] = rest;
56+
if (!scope) {
57+
return context.with(context.active(), () => callback(getCurrentScope()));
58+
}
59+
60+
const ctx = getContextFromScope(scope);
61+
return context.with(ctx || context.active(), () => callback(getCurrentScope()));
62+
}
63+
64+
const callback = rest[0];
3565
return context.with(context.active(), () => callback(getCurrentScope()));
3666
}
3767

packages/node-experimental/src/utils/contextData.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,13 @@
11
import type { Context } from '@opentelemetry/api';
22
import { createContextKey } from '@opentelemetry/api';
3+
import type { Scope } from '@sentry/types';
34

45
import type { CurrentScopes } from '../sdk/types';
56

67
export const SENTRY_SCOPES_CONTEXT_KEY = createContextKey('sentry_scopes');
78

9+
const SCOPE_CONTEXT_MAP = new WeakMap<Scope, Context>();
10+
811
/**
912
* Try to get the current scopes from the given OTEL context.
1013
* This requires a Context Manager that was wrapped with getWrappedContextManager.
@@ -18,5 +21,17 @@ export function getScopesFromContext(context: Context): CurrentScopes | undefine
1821
* This will return a forked context with the Propagation Context set.
1922
*/
2023
export function setScopesOnContext(context: Context, scopes: CurrentScopes): Context {
24+
// So we can look up the context from the scope later
25+
SCOPE_CONTEXT_MAP.set(scopes.scope, context);
26+
SCOPE_CONTEXT_MAP.set(scopes.isolationScope, context);
27+
2128
return context.setValue(SENTRY_SCOPES_CONTEXT_KEY, scopes);
2229
}
30+
31+
/**
32+
* Get the context related to a scope.
33+
* TODO v8: Use this for the `trace` functions.
34+
* */
35+
export function getContextFromScope(scope: Scope): Context | undefined {
36+
return SCOPE_CONTEXT_MAP.get(scope);
37+
}

packages/opentelemetry/src/types.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import type { Attributes, Span as WriteableSpan, SpanKind, TimeInput, Tracer } from '@opentelemetry/api';
22
import type { BasicTracerProvider, ReadableSpan, Span } from '@opentelemetry/sdk-trace-base';
3-
import type { SpanOrigin, TransactionMetadata, TransactionSource } from '@sentry/types';
3+
import type { Scope, SpanOrigin, TransactionMetadata, TransactionSource } from '@sentry/types';
44

55
export interface OpenTelemetryClient {
66
tracer: Tracer;
@@ -13,6 +13,7 @@ export interface OpenTelemetrySpanContext {
1313
metadata?: Partial<TransactionMetadata>;
1414
origin?: SpanOrigin;
1515
source?: TransactionSource;
16+
scope?: Scope;
1617

1718
// Base SpanOptions we support
1819
attributes?: Attributes;

0 commit comments

Comments
 (0)