Skip to content

Commit f29c58a

Browse files
mydeamydea
committed
feat(browser): Create spans as children of root span by default
You can opt out of this by setting `parentSpanIsAlwaysRootSpan=false` in your client options.
1 parent cd5d63a commit f29c58a

File tree

4 files changed

+201
-15
lines changed

4 files changed

+201
-15
lines changed

‎packages/browser/src/client.ts

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

55-
super(options);
60+
super(opts);
5661

57-
if (options.sendClientReports && WINDOW.document) {
62+
if (opts.sendClientReports && WINDOW.document) {
5863
WINDOW.document.addEventListener('visibilitychange', () => {
5964
if (WINDOW.document.visibilityState === 'hidden') {
6065
this._flushOutcomes();

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

Lines changed: 32 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,19 @@
1-
import type { Hub, Scope, Span, SpanTimeInput, StartSpanOptions, TransactionContext } from '@sentry/types';
1+
import type {
2+
ClientOptions,
3+
Hub,
4+
Scope,
5+
Span,
6+
SpanTimeInput,
7+
StartSpanOptions,
8+
TransactionContext,
9+
} from '@sentry/types';
210

311
import { dropUndefinedKeys, logger, tracingContextFromHeaders } from '@sentry/utils';
4-
import { getCurrentScope, getIsolationScope, withScope } from '../currentScopes';
12+
import { getClient, getCurrentScope, getIsolationScope, withScope } from '../currentScopes';
513

614
import { DEBUG_BUILD } from '../debug-build';
715
import { getCurrentHub } from '../hub';
16+
import { getRootSpan } from '../utils/getRootSpan';
817
import { handleCallbackErrors } from '../utils/handleCallbackErrors';
918
import { hasTracingEnabled } from '../utils/hasTracingEnabled';
1019
import { addChildSpanToSpan, spanIsSampled, spanTimeInputToSeconds, spanToJSON } from '../utils/spanUtils';
@@ -31,8 +40,7 @@ export function startSpan<T>(context: StartSpanOptions, callback: (span: Span) =
3140
return withScope(context.scope, scope => {
3241
// eslint-disable-next-line deprecation/deprecation
3342
const hub = getCurrentHub();
34-
// eslint-disable-next-line deprecation/deprecation
35-
const parentSpan = scope.getSpan() as SentrySpan | undefined;
43+
const parentSpan = getParentSpan(scope);
3644

3745
const shouldSkipSpan = context.onlyIfParent && !parentSpan;
3846
const activeSpan = shouldSkipSpan
@@ -80,8 +88,7 @@ export function startSpanManual<T>(context: StartSpanOptions, callback: (span: S
8088
return withScope(context.scope, scope => {
8189
// eslint-disable-next-line deprecation/deprecation
8290
const hub = getCurrentHub();
83-
// eslint-disable-next-line deprecation/deprecation
84-
const parentSpan = scope.getSpan() as SentrySpan | undefined;
91+
const parentSpan = getParentSpan(scope);
8592

8693
const shouldSkipSpan = context.onlyIfParent && !parentSpan;
8794
const activeSpan = shouldSkipSpan
@@ -129,19 +136,15 @@ export function startInactiveSpan(context: StartSpanOptions): Span {
129136
const spanContext = normalizeContext(context);
130137
// eslint-disable-next-line deprecation/deprecation
131138
const hub = getCurrentHub();
132-
const parentSpan = context.scope
133-
? // eslint-disable-next-line deprecation/deprecation
134-
(context.scope.getSpan() as SentrySpan | undefined)
135-
: (getActiveSpan() as SentrySpan | undefined);
139+
const scope = context.scope || getCurrentScope();
140+
const parentSpan = getParentSpan(scope);
136141

137142
const shouldSkipSpan = context.onlyIfParent && !parentSpan;
138143

139144
if (shouldSkipSpan) {
140145
return new SentryNonRecordingSpan();
141146
}
142147

143-
const scope = context.scope || getCurrentScope();
144-
145148
return createChildSpanOrTransaction(hub, {
146149
parentSpan,
147150
spanContext,
@@ -337,3 +340,20 @@ function normalizeContext(context: StartSpanOptions): TransactionContext {
337340

338341
return context;
339342
}
343+
344+
function getParentSpan(scope: Scope): SentrySpan | undefined {
345+
// eslint-disable-next-line deprecation/deprecation
346+
const span = scope.getSpan();
347+
348+
if (!span) {
349+
return undefined;
350+
}
351+
352+
const client = getClient();
353+
const options: Partial<ClientOptions> = client ? client.getOptions() : {};
354+
if (options.parentSpanIsAlwaysRootSpan) {
355+
return getRootSpan(span) as SentrySpan;
356+
}
357+
358+
return span as SentrySpan;
359+
}

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

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -449,6 +449,48 @@ describe('startSpan', () => {
449449
});
450450
});
451451

452+
describe('parentSpanIsAlwaysRootSpan', () => {
453+
it('creates a span as child of root span if parentSpanIsAlwaysRootSpan=true', () => {
454+
const options = getDefaultTestClientOptions({
455+
tracesSampleRate: 1,
456+
parentSpanIsAlwaysRootSpan: true,
457+
});
458+
client = new TestClient(options);
459+
setCurrentClient(client);
460+
client.init();
461+
462+
startSpan({ name: 'parent span' }, span => {
463+
expect(spanToJSON(span).parent_span_id).toBe(undefined);
464+
startSpan({ name: 'child span' }, childSpan => {
465+
expect(spanToJSON(childSpan).parent_span_id).toBe(span.spanContext().spanId);
466+
startSpan({ name: 'grand child span' }, grandChildSpan => {
467+
expect(spanToJSON(grandChildSpan).parent_span_id).toBe(span.spanContext().spanId);
468+
});
469+
});
470+
});
471+
});
472+
473+
it('does not creates a span as child of root span if parentSpanIsAlwaysRootSpan=false', () => {
474+
const options = getDefaultTestClientOptions({
475+
tracesSampleRate: 1,
476+
parentSpanIsAlwaysRootSpan: false,
477+
});
478+
client = new TestClient(options);
479+
setCurrentClient(client);
480+
client.init();
481+
482+
startSpan({ name: 'parent span' }, span => {
483+
expect(spanToJSON(span).parent_span_id).toBe(undefined);
484+
startSpan({ name: 'child span' }, childSpan => {
485+
expect(spanToJSON(childSpan).parent_span_id).toBe(span.spanContext().spanId);
486+
startSpan({ name: 'grand child span' }, grandChildSpan => {
487+
expect(spanToJSON(grandChildSpan).parent_span_id).toBe(childSpan.spanContext().spanId);
488+
});
489+
});
490+
});
491+
});
492+
});
493+
452494
it('samples with a tracesSampler', () => {
453495
const tracesSampler = jest.fn(() => {
454496
return true;
@@ -754,6 +796,54 @@ describe('startSpanManual', () => {
754796
});
755797
});
756798

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

1059+
describe('parentSpanIsAlwaysRootSpan', () => {
1060+
it('creates a span as child of root span if parentSpanIsAlwaysRootSpan=true', () => {
1061+
const options = getDefaultTestClientOptions({
1062+
tracesSampleRate: 1,
1063+
parentSpanIsAlwaysRootSpan: true,
1064+
});
1065+
client = new TestClient(options);
1066+
setCurrentClient(client);
1067+
client.init();
1068+
1069+
const inactiveSpan = startInactiveSpan({ name: 'inactive span' });
1070+
expect(spanToJSON(inactiveSpan).parent_span_id).toBe(undefined);
1071+
1072+
startSpan({ name: 'parent span' }, span => {
1073+
const inactiveSpan = startInactiveSpan({ name: 'inactive span' });
1074+
expect(spanToJSON(inactiveSpan).parent_span_id).toBe(span.spanContext().spanId);
1075+
1076+
startSpan({ name: 'child span' }, () => {
1077+
const inactiveSpan = startInactiveSpan({ name: 'inactive span' });
1078+
expect(spanToJSON(inactiveSpan).parent_span_id).toBe(span.spanContext().spanId);
1079+
1080+
startSpan({ name: 'grand child span' }, () => {
1081+
const inactiveSpan = startInactiveSpan({ name: 'inactive span' });
1082+
expect(spanToJSON(inactiveSpan).parent_span_id).toBe(span.spanContext().spanId);
1083+
});
1084+
});
1085+
});
1086+
});
1087+
1088+
it('does not creates a span as child of root span if parentSpanIsAlwaysRootSpan=false', () => {
1089+
const options = getDefaultTestClientOptions({
1090+
tracesSampleRate: 1,
1091+
parentSpanIsAlwaysRootSpan: false,
1092+
});
1093+
client = new TestClient(options);
1094+
setCurrentClient(client);
1095+
client.init();
1096+
1097+
const inactiveSpan = startInactiveSpan({ name: 'inactive span' });
1098+
expect(spanToJSON(inactiveSpan).parent_span_id).toBe(undefined);
1099+
1100+
startSpan({ name: 'parent span' }, span => {
1101+
const inactiveSpan = startInactiveSpan({ name: 'inactive span' });
1102+
expect(spanToJSON(inactiveSpan).parent_span_id).toBe(span.spanContext().spanId);
1103+
1104+
startSpan({ name: 'child span' }, childSpan => {
1105+
const inactiveSpan = startInactiveSpan({ name: 'inactive span' });
1106+
expect(spanToJSON(inactiveSpan).parent_span_id).toBe(childSpan.spanContext().spanId);
1107+
1108+
startSpan({ name: 'grand child span' }, grandChildSpan => {
1109+
const inactiveSpan = startInactiveSpan({ name: 'inactive span' });
1110+
expect(spanToJSON(inactiveSpan).parent_span_id).toBe(grandChildSpan.spanContext().spanId);
1111+
});
1112+
});
1113+
});
1114+
});
1115+
});
1116+
9691117
it('includes the scope at the time the span was started when finished', async () => {
9701118
const beforeSendTransaction = jest.fn(event => event);
9711119

‎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)