Skip to content

Commit e4ec09e

Browse files
mydeamydea
authored
feat(browser): Create spans as children of root span by default (#10986)
You can opt out of this by setting `parentSpanIsAlwaysRootSpan=false` in your client options. This means that in browser, any span will always be added to the active root span, not the active span. This way, we should be able to avoid problems with execution contexts etc. Closes #10944
1 parent 15d6cb1 commit e4ec09e

File tree

6 files changed

+204
-46
lines changed

6 files changed

+204
-46
lines changed

‎packages/browser/src/client.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,12 +52,17 @@ export class BrowserClient extends BaseClient<BrowserClientOptions> {
5252
* @param options Configuration options for this SDK.
5353
*/
5454
public constructor(options: BrowserClientOptions) {
55+
const opts = {
56+
// We default this to true, as it is the safer scenario
57+
parentSpanIsAlwaysRootSpan: true,
58+
...options,
59+
};
5560
const sdkSource = WINDOW.SENTRY_SDK_SOURCE || getSDKSource();
56-
applySdkMetadata(options, 'browser', ['browser'], sdkSource);
61+
applySdkMetadata(opts, 'browser', ['browser'], sdkSource);
5762

58-
super(options);
63+
super(opts);
5964

60-
if (options.sendClientReports && WINDOW.document) {
65+
if (opts.sendClientReports && WINDOW.document) {
6166
WINDOW.document.addEventListener('visibilitychange', () => {
6267
if (WINDOW.document.visibilityState === 'hidden') {
6368
this._flushOutcomes();

‎packages/core/src/tracing/trace.ts

Lines changed: 22 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import type { ClientOptions, Scope, SentrySpanArguments, Span, SpanTimeInput, StartSpanOptions } from '@sentry/types';
2-
32
import { propagationContextFromHeaders } from '@sentry/utils';
43
import type { AsyncContextStrategy } from '../asyncContext';
54
import { getMainCarrier } from '../asyncContext';
@@ -11,13 +10,7 @@ import { SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE
1110
import { handleCallbackErrors } from '../utils/handleCallbackErrors';
1211
import { hasTracingEnabled } from '../utils/hasTracingEnabled';
1312
import { _getSpanForScope, _setSpanForScope } from '../utils/spanOnScope';
14-
import {
15-
addChildSpanToSpan,
16-
getActiveSpan,
17-
spanIsSampled,
18-
spanTimeInputToSeconds,
19-
spanToJSON,
20-
} from '../utils/spanUtils';
13+
import { addChildSpanToSpan, getRootSpan, spanIsSampled, spanTimeInputToSeconds, spanToJSON } from '../utils/spanUtils';
2114
import { freezeDscOnSpan, getDynamicSamplingContextFromSpan } from './dynamicSamplingContext';
2215
import { logSpanStart } from './logSpans';
2316
import { sampleSpan } from './sampling';
@@ -47,7 +40,7 @@ export function startSpan<T>(context: StartSpanOptions, callback: (span: Span) =
4740
const spanContext = normalizeContext(context);
4841

4942
return withScope(context.scope, scope => {
50-
const parentSpan = _getSpanForScope(scope) as SentrySpan | undefined;
43+
const parentSpan = getParentSpan(scope);
5144

5245
const shouldSkipSpan = context.onlyIfParent && !parentSpan;
5346
const activeSpan = shouldSkipSpan
@@ -94,7 +87,7 @@ export function startSpanManual<T>(context: StartSpanOptions, callback: (span: S
9487
const spanContext = normalizeContext(context);
9588

9689
return withScope(context.scope, scope => {
97-
const parentSpan = _getSpanForScope(scope) as SentrySpan | undefined;
90+
const parentSpan = getParentSpan(scope);
9891

9992
const shouldSkipSpan = context.onlyIfParent && !parentSpan;
10093
const activeSpan = shouldSkipSpan
@@ -141,18 +134,16 @@ export function startInactiveSpan(context: StartSpanOptions): Span {
141134
}
142135

143136
const spanContext = normalizeContext(context);
144-
const parentSpan = context.scope
145-
? (_getSpanForScope(context.scope) as SentrySpan | undefined)
146-
: (getActiveSpan() as SentrySpan | undefined);
137+
138+
const scope = context.scope || getCurrentScope();
139+
const parentSpan = getParentSpan(scope);
147140

148141
const shouldSkipSpan = context.onlyIfParent && !parentSpan;
149142

150143
if (shouldSkipSpan) {
151144
return new SentryNonRecordingSpan();
152145
}
153146

154-
const scope = context.scope || getCurrentScope();
155-
156147
return createChildSpanOrTransaction({
157148
parentSpan,
158149
spanContext,
@@ -381,3 +372,19 @@ function _startChildSpan(parentSpan: Span, scope: Scope, spanArguments: SentrySp
381372

382373
return childSpan;
383374
}
375+
376+
function getParentSpan(scope: Scope): SentrySpan | undefined {
377+
const span = _getSpanForScope(scope) as SentrySpan | undefined;
378+
379+
if (!span) {
380+
return undefined;
381+
}
382+
383+
const client = getClient();
384+
const options: Partial<ClientOptions> = client ? client.getOptions() : {};
385+
if (options.parentSpanIsAlwaysRootSpan) {
386+
return getRootSpan(span) as SentrySpan;
387+
}
388+
389+
return span;
390+
}

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

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -417,6 +417,48 @@ describe('startSpan', () => {
417417
});
418418
});
419419

420+
describe('parentSpanIsAlwaysRootSpan', () => {
421+
it('creates a span as child of root span if parentSpanIsAlwaysRootSpan=true', () => {
422+
const options = getDefaultTestClientOptions({
423+
tracesSampleRate: 1,
424+
parentSpanIsAlwaysRootSpan: true,
425+
});
426+
client = new TestClient(options);
427+
setCurrentClient(client);
428+
client.init();
429+
430+
startSpan({ name: 'parent span' }, span => {
431+
expect(spanToJSON(span).parent_span_id).toBe(undefined);
432+
startSpan({ name: 'child span' }, childSpan => {
433+
expect(spanToJSON(childSpan).parent_span_id).toBe(span.spanContext().spanId);
434+
startSpan({ name: 'grand child span' }, grandChildSpan => {
435+
expect(spanToJSON(grandChildSpan).parent_span_id).toBe(span.spanContext().spanId);
436+
});
437+
});
438+
});
439+
});
440+
441+
it('does not creates a span as child of root span if parentSpanIsAlwaysRootSpan=false', () => {
442+
const options = getDefaultTestClientOptions({
443+
tracesSampleRate: 1,
444+
parentSpanIsAlwaysRootSpan: false,
445+
});
446+
client = new TestClient(options);
447+
setCurrentClient(client);
448+
client.init();
449+
450+
startSpan({ name: 'parent span' }, span => {
451+
expect(spanToJSON(span).parent_span_id).toBe(undefined);
452+
startSpan({ name: 'child span' }, childSpan => {
453+
expect(spanToJSON(childSpan).parent_span_id).toBe(span.spanContext().spanId);
454+
startSpan({ name: 'grand child span' }, grandChildSpan => {
455+
expect(spanToJSON(grandChildSpan).parent_span_id).toBe(childSpan.spanContext().spanId);
456+
});
457+
});
458+
});
459+
});
460+
});
461+
420462
it('samples with a tracesSampler', () => {
421463
const tracesSampler = jest.fn(() => {
422464
return true;
@@ -751,6 +793,54 @@ describe('startSpanManual', () => {
751793
});
752794
});
753795

796+
describe('parentSpanIsAlwaysRootSpan', () => {
797+
it('creates a span as child of root span if parentSpanIsAlwaysRootSpan=true', () => {
798+
const options = getDefaultTestClientOptions({
799+
tracesSampleRate: 1,
800+
parentSpanIsAlwaysRootSpan: true,
801+
});
802+
client = new TestClient(options);
803+
setCurrentClient(client);
804+
client.init();
805+
806+
startSpanManual({ name: 'parent span' }, span => {
807+
expect(spanToJSON(span).parent_span_id).toBe(undefined);
808+
startSpanManual({ name: 'child span' }, childSpan => {
809+
expect(spanToJSON(childSpan).parent_span_id).toBe(span.spanContext().spanId);
810+
startSpanManual({ name: 'grand child span' }, grandChildSpan => {
811+
expect(spanToJSON(grandChildSpan).parent_span_id).toBe(span.spanContext().spanId);
812+
grandChildSpan.end();
813+
});
814+
childSpan.end();
815+
});
816+
span.end();
817+
});
818+
});
819+
820+
it('does not creates a span as child of root span if parentSpanIsAlwaysRootSpan=false', () => {
821+
const options = getDefaultTestClientOptions({
822+
tracesSampleRate: 1,
823+
parentSpanIsAlwaysRootSpan: false,
824+
});
825+
client = new TestClient(options);
826+
setCurrentClient(client);
827+
client.init();
828+
829+
startSpanManual({ name: 'parent span' }, span => {
830+
expect(spanToJSON(span).parent_span_id).toBe(undefined);
831+
startSpanManual({ name: 'child span' }, childSpan => {
832+
expect(spanToJSON(childSpan).parent_span_id).toBe(span.spanContext().spanId);
833+
startSpanManual({ name: 'grand child span' }, grandChildSpan => {
834+
expect(spanToJSON(grandChildSpan).parent_span_id).toBe(childSpan.spanContext().spanId);
835+
grandChildSpan.end();
836+
});
837+
childSpan.end();
838+
});
839+
span.end();
840+
});
841+
});
842+
});
843+
754844
it('sets a child span reference on the parent span', () => {
755845
expect.assertions(1);
756846
startSpan({ name: 'outer' }, (outerSpan: any) => {
@@ -995,6 +1085,64 @@ describe('startInactiveSpan', () => {
9951085
});
9961086
});
9971087

1088+
describe('parentSpanIsAlwaysRootSpan', () => {
1089+
it('creates a span as child of root span if parentSpanIsAlwaysRootSpan=true', () => {
1090+
const options = getDefaultTestClientOptions({
1091+
tracesSampleRate: 1,
1092+
parentSpanIsAlwaysRootSpan: true,
1093+
});
1094+
client = new TestClient(options);
1095+
setCurrentClient(client);
1096+
client.init();
1097+
1098+
const inactiveSpan = startInactiveSpan({ name: 'inactive span' });
1099+
expect(spanToJSON(inactiveSpan).parent_span_id).toBe(undefined);
1100+
1101+
startSpan({ name: 'parent span' }, span => {
1102+
const inactiveSpan = startInactiveSpan({ name: 'inactive span' });
1103+
expect(spanToJSON(inactiveSpan).parent_span_id).toBe(span.spanContext().spanId);
1104+
1105+
startSpan({ name: 'child span' }, () => {
1106+
const inactiveSpan = startInactiveSpan({ name: 'inactive span' });
1107+
expect(spanToJSON(inactiveSpan).parent_span_id).toBe(span.spanContext().spanId);
1108+
1109+
startSpan({ name: 'grand child span' }, () => {
1110+
const inactiveSpan = startInactiveSpan({ name: 'inactive span' });
1111+
expect(spanToJSON(inactiveSpan).parent_span_id).toBe(span.spanContext().spanId);
1112+
});
1113+
});
1114+
});
1115+
});
1116+
1117+
it('does not creates a span as child of root span if parentSpanIsAlwaysRootSpan=false', () => {
1118+
const options = getDefaultTestClientOptions({
1119+
tracesSampleRate: 1,
1120+
parentSpanIsAlwaysRootSpan: false,
1121+
});
1122+
client = new TestClient(options);
1123+
setCurrentClient(client);
1124+
client.init();
1125+
1126+
const inactiveSpan = startInactiveSpan({ name: 'inactive span' });
1127+
expect(spanToJSON(inactiveSpan).parent_span_id).toBe(undefined);
1128+
1129+
startSpan({ name: 'parent span' }, span => {
1130+
const inactiveSpan = startInactiveSpan({ name: 'inactive span' });
1131+
expect(spanToJSON(inactiveSpan).parent_span_id).toBe(span.spanContext().spanId);
1132+
1133+
startSpan({ name: 'child span' }, childSpan => {
1134+
const inactiveSpan = startInactiveSpan({ name: 'inactive span' });
1135+
expect(spanToJSON(inactiveSpan).parent_span_id).toBe(childSpan.spanContext().spanId);
1136+
1137+
startSpan({ name: 'grand child span' }, grandChildSpan => {
1138+
const inactiveSpan = startInactiveSpan({ name: 'inactive span' });
1139+
expect(spanToJSON(inactiveSpan).parent_span_id).toBe(grandChildSpan.spanContext().spanId);
1140+
});
1141+
});
1142+
});
1143+
});
1144+
});
1145+
9981146
it('includes the scope at the time the span was started when finished', async () => {
9991147
const beforeSendTransaction = jest.fn(event => event);
10001148

‎packages/svelte/src/performance.ts

Lines changed: 10 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import type { Span } from '@sentry/types';
33
import { afterUpdate, beforeUpdate, onMount } from 'svelte';
44
import { current_component } from 'svelte/internal';
55

6-
import { getRootSpan, startInactiveSpan, withActiveSpan } from '@sentry/core';
6+
import { startInactiveSpan } from '@sentry/core';
77
import { DEFAULT_COMPONENT_NAME, UI_SVELTE_INIT, UI_SVELTE_UPDATE } from './constants';
88
import type { TrackComponentOptions } from './types';
99

@@ -34,17 +34,16 @@ export function trackComponent(options?: TrackComponentOptions): void {
3434
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
3535
const componentName = `<${customComponentName || current_component.constructor.name || DEFAULT_COMPONENT_NAME}>`;
3636

37-
let initSpan: Span | undefined = undefined;
3837
if (mergedOptions.trackInit) {
39-
initSpan = recordInitSpan(componentName);
38+
recordInitSpan(componentName);
4039
}
4140

4241
if (mergedOptions.trackUpdates) {
43-
recordUpdateSpans(componentName, initSpan);
42+
recordUpdateSpans(componentName);
4443
}
4544
}
4645

47-
function recordInitSpan(componentName: string): Span | undefined {
46+
function recordInitSpan(componentName: string): void {
4847
const initSpan = startInactiveSpan({
4948
onlyIfParent: true,
5049
op: UI_SVELTE_INIT,
@@ -55,35 +54,21 @@ function recordInitSpan(componentName: string): Span | undefined {
5554
onMount(() => {
5655
initSpan.end();
5756
});
58-
59-
return initSpan;
6057
}
6158

62-
function recordUpdateSpans(componentName: string, initSpan?: Span): void {
59+
function recordUpdateSpans(componentName: string): void {
6360
let updateSpan: Span | undefined;
6461
beforeUpdate(() => {
65-
// We need to get the active transaction again because the initial one could
66-
// already be finished or there is currently no transaction going on.
62+
// If there is no active span, we skip
6763
const activeSpan = getActiveSpan();
6864
if (!activeSpan) {
6965
return;
7066
}
7167

72-
// If we are initializing the component when the update span is started, we start it as child
73-
// of the init span. Else, we start it as a child of the transaction.
74-
const parentSpan =
75-
initSpan && initSpan.isRecording() && getRootSpan(initSpan) === getRootSpan(activeSpan)
76-
? initSpan
77-
: getRootSpan(activeSpan);
78-
79-
if (!parentSpan) return;
80-
81-
updateSpan = withActiveSpan(parentSpan, () => {
82-
return startInactiveSpan({
83-
op: UI_SVELTE_UPDATE,
84-
name: componentName,
85-
attributes: { [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.ui.svelte' },
86-
});
68+
updateSpan = startInactiveSpan({
69+
op: UI_SVELTE_UPDATE,
70+
name: componentName,
71+
attributes: { [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.ui.svelte' },
8772
});
8873
});
8974

‎packages/svelte/test/performance.test.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ describe('Sentry.trackComponent()', () => {
3333
});
3434
});
3535

36-
it('creates nested init and update spans on component initialization', async () => {
36+
it('creates init and update spans on component initialization', async () => {
3737
startSpan({ name: 'outer' }, span => {
3838
expect(span).toBeDefined();
3939
render(DummyComponent, { props: { options: {} } });
@@ -73,7 +73,7 @@ describe('Sentry.trackComponent()', () => {
7373
description: '<Dummy$>',
7474
op: 'ui.svelte.update',
7575
origin: 'auto.ui.svelte',
76-
parent_span_id: initSpanId,
76+
parent_span_id: rootSpanId,
7777
span_id: expect.any(String),
7878
start_timestamp: expect.any(Number),
7979
timestamp: expect.any(Number),
@@ -128,7 +128,7 @@ describe('Sentry.trackComponent()', () => {
128128
description: '<Dummy$>',
129129
op: 'ui.svelte.update',
130130
origin: 'auto.ui.svelte',
131-
parent_span_id: initSpanId,
131+
parent_span_id: rootSpanId,
132132
span_id: expect.any(String),
133133
start_timestamp: expect.any(Number),
134134
timestamp: expect.any(Number),

‎packages/types/src/options.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,19 @@ export interface ClientOptions<TO extends BaseTransportOptions = BaseTransportOp
9494
*/
9595
enableTracing?: boolean;
9696

97+
/**
98+
* If this is enabled, any spans started will always have their parent be the active root span,
99+
* if there is any active span.
100+
*
101+
* This is necessary because in some environments (e.g. browser),
102+
* we cannot guarantee an accurate active span.
103+
* Because we cannot properly isolate execution environments,
104+
* you may get wrong results when using e.g. nested `startSpan()` calls.
105+
*
106+
* To solve this, in these environments we'll by default enable this option.
107+
*/
108+
parentSpanIsAlwaysRootSpan?: boolean;
109+
97110
/**
98111
* Initial data to populate scope.
99112
*/

0 commit comments

Comments
 (0)