diff --git a/packages/core/src/tracing/trace.ts b/packages/core/src/tracing/trace.ts index 34fa94bec95d..eabdb778fb10 100644 --- a/packages/core/src/tracing/trace.ts +++ b/packages/core/src/tracing/trace.ts @@ -96,7 +96,7 @@ export function startSpan(options: StartSpanOptions, callback: (span: Span) = /** * Similar to `Sentry.startSpan`. Wraps a function with a transaction/span, but does not finish the span - * after the function is done automatically. You'll have to call `span.end()` manually. + * after the function is done automatically. Use `span.end()` to end the span. * * The created span is the active span and will be used as parent by other spans created inside the function * and can be accessed via `Sentry.getActiveSpan()`, as long as the function is executed while the scope is active. @@ -111,9 +111,11 @@ export function startSpanManual(options: StartSpanOptions, callback: (span: S } const spanArguments = parseSentrySpanArguments(options); - const { forceTransaction, parentSpan: customParentSpan } = options; + const { forceTransaction, parentSpan: customParentSpan, scope: customScope } = options; + + const customForkedScope = customScope?.clone(); - return withScope(options.scope, () => { + return withScope(customForkedScope, () => { // If `options.parentSpan` is defined, we want to wrap the callback in `withActiveSpan` const wrapper = getActiveSpanWrapper(customParentSpan); @@ -133,12 +135,12 @@ export function startSpanManual(options: StartSpanOptions, callback: (span: S _setSpanForScope(scope, activeSpan); - function finishAndSetSpan(): void { - activeSpan.end(); - } - return handleCallbackErrors( - () => callback(activeSpan, finishAndSetSpan), + // We pass the `finish` function to the callback, so the user can finish the span manually + // this is mainly here for historic purposes because previously, we instructed users to call + // `finish` instead of `span.end()` to also clean up the scope. Nowadays, calling `span.end()` + // or `finish` has the same effect and we simply leave it here to avoid breaking user code. + () => callback(activeSpan, () => activeSpan.end()), () => { // Only update the span status if it hasn't been changed yet, and the span is not yet finished const { status } = spanToJSON(activeSpan); diff --git a/packages/core/test/lib/tracing/trace.test.ts b/packages/core/test/lib/tracing/trace.test.ts index 83b875edb59b..db54e1c08ddf 100644 --- a/packages/core/test/lib/tracing/trace.test.ts +++ b/packages/core/test/lib/tracing/trace.test.ts @@ -779,27 +779,100 @@ describe('startSpanManual', () => { expect(getActiveSpan()).toBe(undefined); }); - it('allows to pass a scope', () => { - const initialScope = getCurrentScope(); + describe('starts a span on the fork of a custom scope if passed', () => { + it('with parent span', () => { + const initialScope = getCurrentScope(); - const manualScope = initialScope.clone(); - const parentSpan = new SentrySpan({ spanId: 'parent-span-id', sampled: true }); - _setSpanForScope(manualScope, parentSpan); + const customScope = initialScope.clone(); + customScope.setTag('dogs', 'great'); - startSpanManual({ name: 'GET users/[id]', scope: manualScope }, span => { - expect(getCurrentScope()).not.toBe(initialScope); - expect(getCurrentScope()).toBe(manualScope); - expect(getActiveSpan()).toBe(span); - expect(spanToJSON(span).parent_span_id).toBe('parent-span-id'); + const parentSpan = new SentrySpan({ spanId: 'parent-span-id', sampled: true }); + _setSpanForScope(customScope, parentSpan); - span.end(); + startSpanManual({ name: 'GET users/[id]', scope: customScope }, span => { + // current scope is forked from the customScope + expect(getCurrentScope()).not.toBe(initialScope); + expect(getCurrentScope()).not.toBe(customScope); + expect(spanToJSON(span).parent_span_id).toBe('parent-span-id'); - // Is still the active span - expect(getActiveSpan()).toBe(span); + // span is active span + expect(getActiveSpan()).toBe(span); + + span.end(); + + // span is still the active span (weird but it is what it is) + expect(getActiveSpan()).toBe(span); + + getCurrentScope().setTag('cats', 'great'); + customScope.setTag('bears', 'great'); + + expect(getCurrentScope().getScopeData().tags).toEqual({ dogs: 'great', cats: 'great' }); + expect(customScope.getScopeData().tags).toEqual({ dogs: 'great', bears: 'great' }); + }); + + expect(getCurrentScope()).toBe(initialScope); + expect(getActiveSpan()).toBe(undefined); + + startSpanManual({ name: 'POST users/[id]', scope: customScope }, (span, finish) => { + // current scope is forked from the customScope + expect(getCurrentScope()).not.toBe(initialScope); + expect(getCurrentScope()).not.toBe(customScope); + expect(spanToJSON(span).parent_span_id).toBe('parent-span-id'); + + // scope data modification from customScope in previous callback is persisted + expect(getCurrentScope().getScopeData().tags).toEqual({ dogs: 'great', bears: 'great' }); + + // span is active span + expect(getActiveSpan()).toBe(span); + + // calling finish() or span.end() has the same effect + finish(); + + // using finish() resets the scope correctly + expect(getActiveSpan()).toBe(span); + }); + + expect(getCurrentScope()).toBe(initialScope); + expect(getActiveSpan()).toBe(undefined); }); - expect(getCurrentScope()).toBe(initialScope); - expect(getActiveSpan()).toBe(undefined); + it('without parent span', () => { + const initialScope = getCurrentScope(); + const manualScope = initialScope.clone(); + + startSpanManual({ name: 'GET users/[id]', scope: manualScope }, span => { + // current scope is forked from the customScope + expect(getCurrentScope()).not.toBe(initialScope); + expect(getCurrentScope()).not.toBe(manualScope); + expect(getCurrentScope()).toEqual(manualScope); + + // span is active span and a root span + expect(getActiveSpan()).toBe(span); + expect(getRootSpan(span)).toBe(span); + + span.end(); + + expect(getActiveSpan()).toBe(span); + }); + + startSpanManual({ name: 'POST users/[id]', scope: manualScope }, (span, finish) => { + expect(getCurrentScope()).not.toBe(initialScope); + expect(getCurrentScope()).not.toBe(manualScope); + expect(getCurrentScope()).toEqual(manualScope); + + // second span is active span and its own root span + expect(getActiveSpan()).toBe(span); + expect(getRootSpan(span)).toBe(span); + + finish(); + + // calling finish() or span.end() has the same effect + expect(getActiveSpan()).toBe(span); + }); + + expect(getCurrentScope()).toBe(initialScope); + expect(getActiveSpan()).toBe(undefined); + }); }); it('allows to pass a parentSpan', () => {