diff --git a/dev-packages/browser-integration-tests/suites/tracing/setSpanActive/default/init.js b/dev-packages/browser-integration-tests/suites/tracing/setSpanActive/default/init.js new file mode 100644 index 000000000000..7c200c542c56 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/setSpanActive/default/init.js @@ -0,0 +1,8 @@ +import * as Sentry from '@sentry/browser'; + +window.Sentry = Sentry; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + tracesSampleRate: 1, +}); diff --git a/dev-packages/browser-integration-tests/suites/tracing/setSpanActive/default/subject.js b/dev-packages/browser-integration-tests/suites/tracing/setSpanActive/default/subject.js new file mode 100644 index 000000000000..0ce39588eb1b --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/setSpanActive/default/subject.js @@ -0,0 +1,14 @@ +const checkoutSpan = Sentry.startInactiveSpan({ name: 'checkout-flow' }); +Sentry.setActiveSpanInBrowser(checkoutSpan); + +Sentry.startSpan({ name: 'checkout-step-1' }, () => { + Sentry.startSpan({ name: 'checkout-step-1-1' }, () => { + // ... ` + }); +}); + +Sentry.startSpan({ name: 'checkout-step-2' }, () => { + // ... ` +}); + +checkoutSpan.end(); diff --git a/dev-packages/browser-integration-tests/suites/tracing/setSpanActive/default/test.ts b/dev-packages/browser-integration-tests/suites/tracing/setSpanActive/default/test.ts new file mode 100644 index 000000000000..080ebbfa6e84 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/setSpanActive/default/test.ts @@ -0,0 +1,35 @@ +import { expect } from '@playwright/test'; +import { sentryTest } from '../../../../utils/fixtures'; +import { envelopeRequestParser, shouldSkipTracingTest, waitForTransactionRequest } from '../../../../utils/helpers'; + +sentryTest('sets an inactive span active and adds child spans to it', async ({ getLocalTestUrl, page }) => { + if (shouldSkipTracingTest()) { + sentryTest.skip(); + } + + const req = waitForTransactionRequest(page, e => e.transaction === 'checkout-flow'); + + const url = await getLocalTestUrl({ testDir: __dirname }); + await page.goto(url); + + const checkoutEvent = envelopeRequestParser(await req); + const checkoutSpanId = checkoutEvent.contexts?.trace?.span_id; + expect(checkoutSpanId).toMatch(/[a-f0-9]{16}/); + + expect(checkoutEvent.spans).toHaveLength(3); + + const checkoutStep1 = checkoutEvent.spans?.find(s => s.description === 'checkout-step-1'); + const checkoutStep11 = checkoutEvent.spans?.find(s => s.description === 'checkout-step-1-1'); + const checkoutStep2 = checkoutEvent.spans?.find(s => s.description === 'checkout-step-2'); + + expect(checkoutStep1).toBeDefined(); + expect(checkoutStep11).toBeDefined(); + expect(checkoutStep2).toBeDefined(); + + expect(checkoutStep1?.parent_span_id).toBe(checkoutSpanId); + expect(checkoutStep2?.parent_span_id).toBe(checkoutSpanId); + + // despite 1-1 being called within 1, it's still parented to the root span + // due to this being default behaviour in browser environments + expect(checkoutStep11?.parent_span_id).toBe(checkoutSpanId); +}); diff --git a/dev-packages/browser-integration-tests/suites/tracing/setSpanActive/nested-parentAlwaysRoot/init.js b/dev-packages/browser-integration-tests/suites/tracing/setSpanActive/nested-parentAlwaysRoot/init.js new file mode 100644 index 000000000000..375a16cbf005 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/setSpanActive/nested-parentAlwaysRoot/init.js @@ -0,0 +1,9 @@ +import * as Sentry from '@sentry/browser'; + +window.Sentry = Sentry; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + tracesSampleRate: 1, + parentSpanIsAlwaysRootSpan: false, +}); diff --git a/dev-packages/browser-integration-tests/suites/tracing/setSpanActive/nested-parentAlwaysRoot/subject.js b/dev-packages/browser-integration-tests/suites/tracing/setSpanActive/nested-parentAlwaysRoot/subject.js new file mode 100644 index 000000000000..dc601cbf4d30 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/setSpanActive/nested-parentAlwaysRoot/subject.js @@ -0,0 +1,22 @@ +const checkoutSpan = Sentry.startInactiveSpan({ name: 'checkout-flow' }); +Sentry.setActiveSpanInBrowser(checkoutSpan); + +Sentry.startSpan({ name: 'checkout-step-1' }, () => {}); + +const checkoutStep2 = Sentry.startInactiveSpan({ name: 'checkout-step-2' }); +Sentry.setActiveSpanInBrowser(checkoutStep2); + +Sentry.startSpan({ name: 'checkout-step-2-1' }, () => { + // ... ` +}); +checkoutStep2.end(); + +Sentry.startSpan({ name: 'checkout-step-3' }, () => {}); + +checkoutSpan.end(); + +Sentry.startSpan({ name: 'post-checkout' }, () => { + Sentry.startSpan({ name: 'post-checkout-1' }, () => { + // ... ` + }); +}); diff --git a/dev-packages/browser-integration-tests/suites/tracing/setSpanActive/nested-parentAlwaysRoot/test.ts b/dev-packages/browser-integration-tests/suites/tracing/setSpanActive/nested-parentAlwaysRoot/test.ts new file mode 100644 index 000000000000..2270b470123b --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/setSpanActive/nested-parentAlwaysRoot/test.ts @@ -0,0 +1,57 @@ +import { expect } from '@playwright/test'; +import { sentryTest } from '../../../../utils/fixtures'; +import { envelopeRequestParser, shouldSkipTracingTest, waitForTransactionRequest } from '../../../../utils/helpers'; + +sentryTest( + 'nested calls to setActiveSpanInBrowser with parentSpanIsAlwaysRootSpan=false result in correct parenting', + async ({ getLocalTestUrl, page }) => { + if (shouldSkipTracingTest()) { + sentryTest.skip(); + } + + const req = waitForTransactionRequest(page, e => e.transaction === 'checkout-flow'); + const postCheckoutReq = waitForTransactionRequest(page, e => e.transaction === 'post-checkout'); + + const url = await getLocalTestUrl({ testDir: __dirname }); + await page.goto(url); + + const checkoutEvent = envelopeRequestParser(await req); + const postCheckoutEvent = envelopeRequestParser(await postCheckoutReq); + + const checkoutSpanId = checkoutEvent.contexts?.trace?.span_id; + const postCheckoutSpanId = postCheckoutEvent.contexts?.trace?.span_id; + + expect(checkoutSpanId).toMatch(/[a-f0-9]{16}/); + expect(postCheckoutSpanId).toMatch(/[a-f0-9]{16}/); + + expect(checkoutEvent.spans).toHaveLength(4); + expect(postCheckoutEvent.spans).toHaveLength(1); + + const checkoutStep1 = checkoutEvent.spans?.find(s => s.description === 'checkout-step-1'); + const checkoutStep2 = checkoutEvent.spans?.find(s => s.description === 'checkout-step-2'); + const checkoutStep21 = checkoutEvent.spans?.find(s => s.description === 'checkout-step-2-1'); + const checkoutStep3 = checkoutEvent.spans?.find(s => s.description === 'checkout-step-3'); + + expect(checkoutStep1).toBeDefined(); + expect(checkoutStep2).toBeDefined(); + expect(checkoutStep21).toBeDefined(); + expect(checkoutStep3).toBeDefined(); + + expect(checkoutStep1?.parent_span_id).toBe(checkoutSpanId); + expect(checkoutStep2?.parent_span_id).toBe(checkoutSpanId); + + // with parentSpanIsAlwaysRootSpan=false, 2-1 is parented to 2 because + // 2 was the active span when 2-1 was started + expect(checkoutStep21?.parent_span_id).toBe(checkoutStep2?.span_id); + + // since the parent of three is `checkoutSpan`, we correctly reset + // the active span to `checkoutSpan` after 2 ended + expect(checkoutStep3?.parent_span_id).toBe(checkoutSpanId); + + // post-checkout trace is started as a new trace because ending checkoutSpan removes the active + // span on the scope + const postCheckoutStep1 = postCheckoutEvent.spans?.find(s => s.description === 'post-checkout-1'); + expect(postCheckoutStep1).toBeDefined(); + expect(postCheckoutStep1?.parent_span_id).toBe(postCheckoutSpanId); + }, +); diff --git a/dev-packages/browser-integration-tests/suites/tracing/setSpanActive/nested/init.js b/dev-packages/browser-integration-tests/suites/tracing/setSpanActive/nested/init.js new file mode 100644 index 000000000000..7c200c542c56 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/setSpanActive/nested/init.js @@ -0,0 +1,8 @@ +import * as Sentry from '@sentry/browser'; + +window.Sentry = Sentry; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + tracesSampleRate: 1, +}); diff --git a/dev-packages/browser-integration-tests/suites/tracing/setSpanActive/nested/subject.js b/dev-packages/browser-integration-tests/suites/tracing/setSpanActive/nested/subject.js new file mode 100644 index 000000000000..dc601cbf4d30 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/setSpanActive/nested/subject.js @@ -0,0 +1,22 @@ +const checkoutSpan = Sentry.startInactiveSpan({ name: 'checkout-flow' }); +Sentry.setActiveSpanInBrowser(checkoutSpan); + +Sentry.startSpan({ name: 'checkout-step-1' }, () => {}); + +const checkoutStep2 = Sentry.startInactiveSpan({ name: 'checkout-step-2' }); +Sentry.setActiveSpanInBrowser(checkoutStep2); + +Sentry.startSpan({ name: 'checkout-step-2-1' }, () => { + // ... ` +}); +checkoutStep2.end(); + +Sentry.startSpan({ name: 'checkout-step-3' }, () => {}); + +checkoutSpan.end(); + +Sentry.startSpan({ name: 'post-checkout' }, () => { + Sentry.startSpan({ name: 'post-checkout-1' }, () => { + // ... ` + }); +}); diff --git a/dev-packages/browser-integration-tests/suites/tracing/setSpanActive/nested/test.ts b/dev-packages/browser-integration-tests/suites/tracing/setSpanActive/nested/test.ts new file mode 100644 index 000000000000..094bb0ed3dd8 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/setSpanActive/nested/test.ts @@ -0,0 +1,52 @@ +import { expect } from '@playwright/test'; +import { sentryTest } from '../../../../utils/fixtures'; +import { envelopeRequestParser, shouldSkipTracingTest, waitForTransactionRequest } from '../../../../utils/helpers'; + +sentryTest( + 'nested calls to setActiveSpanInBrowser still parent to root span by default', + async ({ getLocalTestUrl, page }) => { + if (shouldSkipTracingTest()) { + sentryTest.skip(); + } + + const req = waitForTransactionRequest(page, e => e.transaction === 'checkout-flow'); + const postCheckoutReq = waitForTransactionRequest(page, e => e.transaction === 'post-checkout'); + + const url = await getLocalTestUrl({ testDir: __dirname }); + await page.goto(url); + + const checkoutEvent = envelopeRequestParser(await req); + const postCheckoutEvent = envelopeRequestParser(await postCheckoutReq); + + const checkoutSpanId = checkoutEvent.contexts?.trace?.span_id; + const postCheckoutSpanId = postCheckoutEvent.contexts?.trace?.span_id; + + expect(checkoutSpanId).toMatch(/[a-f0-9]{16}/); + expect(postCheckoutSpanId).toMatch(/[a-f0-9]{16}/); + + expect(checkoutEvent.spans).toHaveLength(4); + expect(postCheckoutEvent.spans).toHaveLength(1); + + const checkoutStep1 = checkoutEvent.spans?.find(s => s.description === 'checkout-step-1'); + const checkoutStep2 = checkoutEvent.spans?.find(s => s.description === 'checkout-step-2'); + const checkoutStep21 = checkoutEvent.spans?.find(s => s.description === 'checkout-step-2-1'); + const checkoutStep3 = checkoutEvent.spans?.find(s => s.description === 'checkout-step-3'); + + expect(checkoutStep1).toBeDefined(); + expect(checkoutStep2).toBeDefined(); + expect(checkoutStep21).toBeDefined(); + expect(checkoutStep3).toBeDefined(); + + expect(checkoutStep1?.parent_span_id).toBe(checkoutSpanId); + expect(checkoutStep2?.parent_span_id).toBe(checkoutSpanId); + expect(checkoutStep3?.parent_span_id).toBe(checkoutSpanId); + + // despite 2-1 being called within 2 AND setting 2 as active span, it's still parented to the + // root span due to this being default behaviour in browser environments + expect(checkoutStep21?.parent_span_id).toBe(checkoutSpanId); + + const postCheckoutStep1 = postCheckoutEvent.spans?.find(s => s.description === 'post-checkout-1'); + expect(postCheckoutStep1).toBeDefined(); + expect(postCheckoutStep1?.parent_span_id).toBe(postCheckoutSpanId); + }, +); diff --git a/packages/browser/src/index.bundle.tracing.replay.feedback.ts b/packages/browser/src/index.bundle.tracing.replay.feedback.ts index fc805c82a4e5..7aa4b3ae778c 100644 --- a/packages/browser/src/index.bundle.tracing.replay.feedback.ts +++ b/packages/browser/src/index.bundle.tracing.replay.feedback.ts @@ -22,6 +22,7 @@ export { startBrowserTracingNavigationSpan, startBrowserTracingPageLoadSpan, } from './tracing/browserTracingIntegration'; +export { setActiveSpanInBrowser } from './tracing/setActiveSpan'; export { reportPageLoaded } from './tracing/reportPageLoaded'; diff --git a/packages/browser/src/index.bundle.tracing.replay.ts b/packages/browser/src/index.bundle.tracing.replay.ts index f77d2774c36e..3dc858d69cb5 100644 --- a/packages/browser/src/index.bundle.tracing.replay.ts +++ b/packages/browser/src/index.bundle.tracing.replay.ts @@ -22,8 +22,8 @@ export { startBrowserTracingNavigationSpan, startBrowserTracingPageLoadSpan, } from './tracing/browserTracingIntegration'; - export { reportPageLoaded } from './tracing/reportPageLoaded'; +export { setActiveSpanInBrowser } from './tracing/setActiveSpan'; export { feedbackIntegrationShim as feedbackAsyncIntegration, feedbackIntegrationShim as feedbackIntegration }; diff --git a/packages/browser/src/index.bundle.tracing.ts b/packages/browser/src/index.bundle.tracing.ts index c32e806f1de8..62259b92ce7e 100644 --- a/packages/browser/src/index.bundle.tracing.ts +++ b/packages/browser/src/index.bundle.tracing.ts @@ -22,6 +22,7 @@ export { startBrowserTracingNavigationSpan, startBrowserTracingPageLoadSpan, } from './tracing/browserTracingIntegration'; +export { setActiveSpanInBrowser } from './tracing/setActiveSpan'; export { reportPageLoaded } from './tracing/reportPageLoaded'; diff --git a/packages/browser/src/index.ts b/packages/browser/src/index.ts index f2a3e7dc179c..5e9924fe6da5 100644 --- a/packages/browser/src/index.ts +++ b/packages/browser/src/index.ts @@ -40,6 +40,8 @@ export { startBrowserTracingPageLoadSpan, } from './tracing/browserTracingIntegration'; export { reportPageLoaded } from './tracing/reportPageLoaded'; +export { setActiveSpanInBrowser } from './tracing/setActiveSpan'; + export type { RequestInstrumentationOptions } from './tracing/request'; export { registerSpanErrorInstrumentation, diff --git a/packages/browser/src/tracing/setActiveSpan.ts b/packages/browser/src/tracing/setActiveSpan.ts new file mode 100644 index 000000000000..5e3b537d4b6d --- /dev/null +++ b/packages/browser/src/tracing/setActiveSpan.ts @@ -0,0 +1,64 @@ +import type { Span } from '@sentry/core'; +import { _INTERNAL_setSpanForScope, getActiveSpan, getCurrentScope } from '@sentry/core'; + +/** + * Sets an inactive span active on the current scope. + * + * This is useful in browser applications, if you want to create a span that cannot be finished + * within its callback. Any spans started while the given span is active, will be children of the span. + * + * If there already was an active span on the scope prior to calling this function, it is replaced + * with the given span and restored after the span ended. Otherwise, the span will simply be + * removed, resulting in no active span on the scope. + * + * IMPORTANT: This function can ONLY be used in the browser! Calling this function in a server + * environment (for example in a server-side rendered component) will result in undefined behaviour + * and is not supported. + * You MUST call `span.end()` manually, otherwise the span will never be finished. + * + * @example + * ```js + * let checkoutSpan; + * + * on('checkoutStarted', () => { + * checkoutSpan = Sentry.startInactiveSpan({ name: 'checkout-flow' }); + * Sentry.setActiveSpanInBrowser(checkoutSpan); + * }) + * + * // during this time, any spans started will be children of `checkoutSpan`: + * Sentry.startSpan({ name: 'checkout-step-1' }, () => { + * // ... ` + * }) + * + * on('checkoutCompleted', () => { + * checkoutSpan?.end(); + * }) + * ``` + * + * @param span - the span to set active + */ +export function setActiveSpanInBrowser(span: Span): void { + const maybePreviousActiveSpan = getActiveSpan(); + + // If the span is already active, there's no need to double-patch or set it again. + // This also guards against users (for whatever reason) calling setActiveSpanInBrowser on SDK-started + // idle spans like pageload or navigation spans. These will already be handled correctly by the SDK. + // For nested situations, we have to double-patch to ensure we restore the correct previous span (see tests) + if (maybePreviousActiveSpan === span) { + return; + } + + const scope = getCurrentScope(); + + // Putting a small patch onto the span.end method to ensure we + // remove the span from the scope when it ends. + // eslint-disable-next-line @typescript-eslint/unbound-method + span.end = new Proxy(span.end, { + apply(target, thisArg, args: Parameters) { + _INTERNAL_setSpanForScope(scope, maybePreviousActiveSpan); + return Reflect.apply(target, thisArg, args); + }, + }); + + _INTERNAL_setSpanForScope(scope, span); +} diff --git a/packages/browser/test/tracing/setActiveSpan.test.ts b/packages/browser/test/tracing/setActiveSpan.test.ts new file mode 100644 index 000000000000..d3c7ea79cf67 --- /dev/null +++ b/packages/browser/test/tracing/setActiveSpan.test.ts @@ -0,0 +1,90 @@ +import { getActiveSpan, SentrySpan } from '@sentry/core'; +import { describe, expect, it } from 'vitest'; +import { setActiveSpanInBrowser } from '../../src'; + +describe('setActiveSpanInBrowser', () => { + it('sets the passed span active the current scope', () => { + const span = new SentrySpan({ name: 'test' }); + setActiveSpanInBrowser(span); + expect(getActiveSpan()).toBe(span); + + span.end(); + expect(getActiveSpan()).toBeUndefined(); + }); + + it('handles multiple calls to setActiveSpanInBrowser', () => { + const span = new SentrySpan({ name: 'test' }); + setActiveSpanInBrowser(span); + setActiveSpanInBrowser(span); + setActiveSpanInBrowser(span); + expect(getActiveSpan()).toBe(span); + + span.end(); + expect(getActiveSpan()).toBeUndefined(); + }); + + it('handles changing active span while span is running', () => { + const span = new SentrySpan({ name: 'test' }); + setActiveSpanInBrowser(span); + + expect(getActiveSpan()).toBe(span); + + const span2 = new SentrySpan({ name: 'test2' }); + setActiveSpanInBrowser(span2); + expect(getActiveSpan()).toBe(span2); + + span2.end(); + expect(getActiveSpan()).toBe(span); + + span.end(); + expect(getActiveSpan()).toBeUndefined(); + }); + + it('handles multiple span.end calls', () => { + const span = new SentrySpan({ name: 'test' }); + setActiveSpanInBrowser(span); + setActiveSpanInBrowser(span); + + expect(getActiveSpan()).toBe(span); + + const span2 = new SentrySpan({ name: 'test2' }); + setActiveSpanInBrowser(span2); + expect(getActiveSpan()).toBe(span2); + + span2.end(); + span2.end(); + span2.end(); + expect(getActiveSpan()).toBe(span); + + span.end(); + span.end(); + expect(getActiveSpan()).toBeUndefined(); + }); + + it('handles nested activation of the same span', () => { + const span1 = new SentrySpan({ name: 'test1', sampled: true }); + const span2 = new SentrySpan({ name: 'test2', sampled: true }); + expect(span1.isRecording()).toBe(true); + expect(span2.isRecording()).toBe(true); + + setActiveSpanInBrowser(span1); + expect(getActiveSpan()).toBe(span1); + + setActiveSpanInBrowser(span2); + expect(getActiveSpan()).toBe(span2); + + setActiveSpanInBrowser(span1); + expect(getActiveSpan()).toBe(span1); + + span2.end(); + expect(getActiveSpan()).toBe(span1); + expect(span2.isRecording()).toBe(false); + expect(span1.isRecording()).toBe(true); + + span1.end(); + expect(getActiveSpan()).toBeUndefined(); + + expect(span1.isRecording()).toBe(false); + expect(span2.isRecording()).toBe(false); + }); +}); diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 8a5566948f6e..e0daefd54d76 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -81,6 +81,7 @@ export { spanTimeInputToSeconds, updateSpanName, } from './utils/spanUtils'; +export { _setSpanForScope as _INTERNAL_setSpanForScope } from './utils/spanOnScope'; export { parseSampleRate } from './utils/parseSampleRate'; export { applySdkMetadata } from './utils/sdkMetadata'; export { getTraceData } from './utils/traceData';