Skip to content

Commit c4883fa

Browse files
committed
rename to setActiveSpanInBrowser
1 parent f941afc commit c4883fa

File tree

8 files changed

+146
-53
lines changed

8 files changed

+146
-53
lines changed

dev-packages/browser-integration-tests/suites/tracing/setSpanActive/nested-parentAlwaysRoot/test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { sentryTest } from '../../../../utils/fixtures';
33
import { envelopeRequestParser, shouldSkipTracingTest, waitForTransactionRequest } from '../../../../utils/helpers';
44

55
sentryTest(
6-
'nested calls to setSpanActive with parentSpanIsAlwaysRootSpan=false result in correct parenting',
6+
'nested calls to setActiveSpanInBrowser with parentSpanIsAlwaysRootSpan=false result in correct parenting',
77
async ({ getLocalTestUrl, page }) => {
88
if (shouldSkipTracingTest()) {
99
sentryTest.skip();

dev-packages/browser-integration-tests/suites/tracing/setSpanActive/nested/test.ts

Lines changed: 48 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -2,48 +2,51 @@ import { expect } from '@playwright/test';
22
import { sentryTest } from '../../../../utils/fixtures';
33
import { envelopeRequestParser, shouldSkipTracingTest, waitForTransactionRequest } from '../../../../utils/helpers';
44

5-
sentryTest('nested calls to setSpanActive still parent to root span by default', async ({ getLocalTestUrl, page }) => {
6-
if (shouldSkipTracingTest()) {
7-
sentryTest.skip();
8-
}
9-
10-
const req = waitForTransactionRequest(page, e => e.transaction === 'checkout-flow');
11-
const postCheckoutReq = waitForTransactionRequest(page, e => e.transaction === 'post-checkout');
12-
13-
const url = await getLocalTestUrl({ testDir: __dirname });
14-
await page.goto(url);
15-
16-
const checkoutEvent = envelopeRequestParser(await req);
17-
const postCheckoutEvent = envelopeRequestParser(await postCheckoutReq);
18-
19-
const checkoutSpanId = checkoutEvent.contexts?.trace?.span_id;
20-
const postCheckoutSpanId = postCheckoutEvent.contexts?.trace?.span_id;
21-
22-
expect(checkoutSpanId).toMatch(/[a-f0-9]{16}/);
23-
expect(postCheckoutSpanId).toMatch(/[a-f0-9]{16}/);
24-
25-
expect(checkoutEvent.spans).toHaveLength(4);
26-
expect(postCheckoutEvent.spans).toHaveLength(1);
27-
28-
const checkoutStep1 = checkoutEvent.spans?.find(s => s.description === 'checkout-step-1');
29-
const checkoutStep2 = checkoutEvent.spans?.find(s => s.description === 'checkout-step-2');
30-
const checkoutStep21 = checkoutEvent.spans?.find(s => s.description === 'checkout-step-2-1');
31-
const checkoutStep3 = checkoutEvent.spans?.find(s => s.description === 'checkout-step-3');
32-
33-
expect(checkoutStep1).toBeDefined();
34-
expect(checkoutStep2).toBeDefined();
35-
expect(checkoutStep21).toBeDefined();
36-
expect(checkoutStep3).toBeDefined();
37-
38-
expect(checkoutStep1?.parent_span_id).toBe(checkoutSpanId);
39-
expect(checkoutStep2?.parent_span_id).toBe(checkoutSpanId);
40-
expect(checkoutStep3?.parent_span_id).toBe(checkoutSpanId);
41-
42-
// despite 2-1 being called within 2 AND setting 2 as active span, it's still parented to the
43-
// root span due to this being default behaviour in browser environments
44-
expect(checkoutStep21?.parent_span_id).toBe(checkoutSpanId);
45-
46-
const postCheckoutStep1 = postCheckoutEvent.spans?.find(s => s.description === 'post-checkout-1');
47-
expect(postCheckoutStep1).toBeDefined();
48-
expect(postCheckoutStep1?.parent_span_id).toBe(postCheckoutSpanId);
49-
});
5+
sentryTest(
6+
'nested calls to setActiveSpanInBrowser still parent to root span by default',
7+
async ({ getLocalTestUrl, page }) => {
8+
if (shouldSkipTracingTest()) {
9+
sentryTest.skip();
10+
}
11+
12+
const req = waitForTransactionRequest(page, e => e.transaction === 'checkout-flow');
13+
const postCheckoutReq = waitForTransactionRequest(page, e => e.transaction === 'post-checkout');
14+
15+
const url = await getLocalTestUrl({ testDir: __dirname });
16+
await page.goto(url);
17+
18+
const checkoutEvent = envelopeRequestParser(await req);
19+
const postCheckoutEvent = envelopeRequestParser(await postCheckoutReq);
20+
21+
const checkoutSpanId = checkoutEvent.contexts?.trace?.span_id;
22+
const postCheckoutSpanId = postCheckoutEvent.contexts?.trace?.span_id;
23+
24+
expect(checkoutSpanId).toMatch(/[a-f0-9]{16}/);
25+
expect(postCheckoutSpanId).toMatch(/[a-f0-9]{16}/);
26+
27+
expect(checkoutEvent.spans).toHaveLength(4);
28+
expect(postCheckoutEvent.spans).toHaveLength(1);
29+
30+
const checkoutStep1 = checkoutEvent.spans?.find(s => s.description === 'checkout-step-1');
31+
const checkoutStep2 = checkoutEvent.spans?.find(s => s.description === 'checkout-step-2');
32+
const checkoutStep21 = checkoutEvent.spans?.find(s => s.description === 'checkout-step-2-1');
33+
const checkoutStep3 = checkoutEvent.spans?.find(s => s.description === 'checkout-step-3');
34+
35+
expect(checkoutStep1).toBeDefined();
36+
expect(checkoutStep2).toBeDefined();
37+
expect(checkoutStep21).toBeDefined();
38+
expect(checkoutStep3).toBeDefined();
39+
40+
expect(checkoutStep1?.parent_span_id).toBe(checkoutSpanId);
41+
expect(checkoutStep2?.parent_span_id).toBe(checkoutSpanId);
42+
expect(checkoutStep3?.parent_span_id).toBe(checkoutSpanId);
43+
44+
// despite 2-1 being called within 2 AND setting 2 as active span, it's still parented to the
45+
// root span due to this being default behaviour in browser environments
46+
expect(checkoutStep21?.parent_span_id).toBe(checkoutSpanId);
47+
48+
const postCheckoutStep1 = postCheckoutEvent.spans?.find(s => s.description === 'post-checkout-1');
49+
expect(postCheckoutStep1).toBeDefined();
50+
expect(postCheckoutStep1?.parent_span_id).toBe(postCheckoutSpanId);
51+
},
52+
);

packages/browser/src/index.bundle.tracing.replay.feedback.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ export {
2222
startBrowserTracingNavigationSpan,
2323
startBrowserTracingPageLoadSpan,
2424
} from './tracing/browserTracingIntegration';
25-
export { setSpanActive } from './tracing/setSpanActive';
25+
export { setActiveSpanInBrowser } from './tracing/setActiveSpanInBrowser';
2626

2727
export { reportPageLoaded } from './tracing/reportPageLoaded';
2828

packages/browser/src/index.bundle.tracing.replay.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ export {
2323
startBrowserTracingPageLoadSpan,
2424
} from './tracing/browserTracingIntegration';
2525
export { reportPageLoaded } from './tracing/reportPageLoaded';
26-
export { setSpanActive } from './tracing/setSpanActive';
26+
export { setActiveSpanInBrowser } from './tracing/setActiveSpanInBrowser';
2727

2828
export { feedbackIntegrationShim as feedbackAsyncIntegration, feedbackIntegrationShim as feedbackIntegration };
2929

packages/browser/src/index.bundle.tracing.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ export {
2222
startBrowserTracingNavigationSpan,
2323
startBrowserTracingPageLoadSpan,
2424
} from './tracing/browserTracingIntegration';
25-
export { setSpanActive } from './tracing/setSpanActive';
25+
export { setActiveSpanInBrowser } from './tracing/setActiveSpanInBrowser';
2626

2727
export { reportPageLoaded } from './tracing/reportPageLoaded';
2828

packages/browser/src/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ export {
4040
startBrowserTracingPageLoadSpan,
4141
} from './tracing/browserTracingIntegration';
4242
export { reportPageLoaded } from './tracing/reportPageLoaded';
43-
export { setSpanActive } from './tracing/setSpanActive';
43+
export { setActiveSpanInBrowser } from './tracing/setActiveSpanInBrowser';
4444

4545
export type { RequestInstrumentationOptions } from './tracing/request';
4646
export {

packages/browser/src/tracing/setSpanActive.ts renamed to packages/browser/src/tracing/setActiveSpan.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import { _INTERNAL_setSpanForScope, getActiveSpan, getCurrentScope } from '@sent
2222
*
2323
* on('checkoutStarted', () => {
2424
* checkoutSpan = Sentry.startInactiveSpan({ name: 'checkout-flow' });
25-
* Sentry.setSpanActive(checkoutSpan);
25+
* Sentry.setActiveSpanInBrowser(checkoutSpan);
2626
* })
2727
*
2828
* // during this time, any spans started will be children of `checkoutSpan`:
@@ -37,11 +37,11 @@ import { _INTERNAL_setSpanForScope, getActiveSpan, getCurrentScope } from '@sent
3737
*
3838
* @param span - the span to set active
3939
*/
40-
export function setSpanActive(span: Span): void {
40+
export function setActiveSpanInBrowser(span: Span): void {
4141
const maybePreviousActiveSpan = getActiveSpan();
4242

4343
// If the span is already active, there's no need to double-patch or set it again.
44-
// This also guards against users (for whatever reason) calling setSpanActive on SDK-started
44+
// This also guards against users (for whatever reason) calling setActiveSpanInBrowser on SDK-started
4545
// idle spans like pageload or navigation spans. These will already be handled correctly by the SDK.
4646
// For nested situations, we have to double-patch to ensure we restore the correct previous span (see tests)
4747
if (maybePreviousActiveSpan === span) {
Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
import { getActiveSpan, SentrySpan } from '@sentry/core';
2+
import { describe, expect, it } from 'vitest';
3+
import { setActiveSpanInBrowser } from '../../src';
4+
5+
describe('setActiveSpanInBrowser', () => {
6+
it('sets the passed span active the current scope', () => {
7+
const span = new SentrySpan({ name: 'test' });
8+
setActiveSpanInBrowser(span);
9+
expect(getActiveSpan()).toBe(span);
10+
11+
span.end();
12+
expect(getActiveSpan()).toBeUndefined();
13+
});
14+
15+
it('handles multiple calls to setActiveSpanInBrowser', () => {
16+
const span = new SentrySpan({ name: 'test' });
17+
setActiveSpanInBrowser(span);
18+
setActiveSpanInBrowser(span);
19+
setActiveSpanInBrowser(span);
20+
expect(getActiveSpan()).toBe(span);
21+
22+
span.end();
23+
expect(getActiveSpan()).toBeUndefined();
24+
});
25+
26+
it('handles changing active span while span is running', () => {
27+
const span = new SentrySpan({ name: 'test' });
28+
setActiveSpanInBrowser(span);
29+
30+
expect(getActiveSpan()).toBe(span);
31+
32+
const span2 = new SentrySpan({ name: 'test2' });
33+
setActiveSpanInBrowser(span2);
34+
expect(getActiveSpan()).toBe(span2);
35+
36+
span2.end();
37+
expect(getActiveSpan()).toBe(span);
38+
39+
span.end();
40+
expect(getActiveSpan()).toBeUndefined();
41+
});
42+
43+
it('handles multiple span.end calls', () => {
44+
const span = new SentrySpan({ name: 'test' });
45+
setActiveSpanInBrowser(span);
46+
setActiveSpanInBrowser(span);
47+
48+
expect(getActiveSpan()).toBe(span);
49+
50+
const span2 = new SentrySpan({ name: 'test2' });
51+
setActiveSpanInBrowser(span2);
52+
expect(getActiveSpan()).toBe(span2);
53+
54+
span2.end();
55+
span2.end();
56+
span2.end();
57+
expect(getActiveSpan()).toBe(span);
58+
59+
span.end();
60+
span.end();
61+
expect(getActiveSpan()).toBeUndefined();
62+
});
63+
64+
it('handles nested activation of the same span', () => {
65+
const span1 = new SentrySpan({ name: 'test1', sampled: true });
66+
const span2 = new SentrySpan({ name: 'test2', sampled: true });
67+
expect(span1.isRecording()).toBe(true);
68+
expect(span2.isRecording()).toBe(true);
69+
70+
setActiveSpanInBrowser(span1);
71+
expect(getActiveSpan()).toBe(span1);
72+
73+
setActiveSpanInBrowser(span2);
74+
expect(getActiveSpan()).toBe(span2);
75+
76+
setActiveSpanInBrowser(span1);
77+
expect(getActiveSpan()).toBe(span1);
78+
79+
span2.end();
80+
expect(getActiveSpan()).toBe(span1);
81+
expect(span2.isRecording()).toBe(false);
82+
expect(span1.isRecording()).toBe(true);
83+
84+
span1.end();
85+
expect(getActiveSpan()).toBeUndefined();
86+
87+
expect(span1.isRecording()).toBe(false);
88+
expect(span2.isRecording()).toBe(false);
89+
});
90+
});

0 commit comments

Comments
 (0)