Skip to content

Commit 3a2a722

Browse files
mydeamydea
committed
fix(browser): Ensure explicit parentSpan is considered
Even if `parentSpanIsAlwaysRootSpan=true` is configured.
1 parent bc6725c commit 3a2a722

File tree

2 files changed

+130
-4
lines changed

2 files changed

+130
-4
lines changed

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

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ export function startSpan<T>(options: StartSpanOptions, callback: (span: Span) =
5858

5959
return wrapper(() => {
6060
const scope = getCurrentScope();
61-
const parentSpan = getParentSpan(scope);
61+
const parentSpan = getParentSpan(scope, customParentSpan);
6262

6363
const shouldSkipSpan = options.onlyIfParent && !parentSpan;
6464
const activeSpan = shouldSkipSpan
@@ -116,7 +116,7 @@ export function startSpanManual<T>(options: StartSpanOptions, callback: (span: S
116116

117117
return wrapper(() => {
118118
const scope = getCurrentScope();
119-
const parentSpan = getParentSpan(scope);
119+
const parentSpan = getParentSpan(scope, customParentSpan);
120120

121121
const shouldSkipSpan = options.onlyIfParent && !parentSpan;
122122
const activeSpan = shouldSkipSpan
@@ -176,7 +176,7 @@ export function startInactiveSpan(options: StartSpanOptions): Span {
176176

177177
return wrapper(() => {
178178
const scope = getCurrentScope();
179-
const parentSpan = getParentSpan(scope);
179+
const parentSpan = getParentSpan(scope, customParentSpan);
180180

181181
const shouldSkipSpan = options.onlyIfParent && !parentSpan;
182182

@@ -489,7 +489,17 @@ function _startChildSpan(parentSpan: Span, scope: Scope, spanArguments: SentrySp
489489
return childSpan;
490490
}
491491

492-
function getParentSpan(scope: Scope): SentrySpan | undefined {
492+
function getParentSpan(scope: Scope, customParentSpan: Span | null | undefined): SentrySpan | undefined {
493+
// always use the passed in span directly
494+
if (customParentSpan) {
495+
return customParentSpan as SentrySpan;
496+
}
497+
498+
// This is different from `undefined` as it means the user explicitly wants no parent span
499+
if (customParentSpan === null) {
500+
return undefined;
501+
}
502+
493503
const span = _getSpanForScope(scope) as SentrySpan | undefined;
494504

495505
if (!span) {

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

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -620,6 +620,44 @@ describe('startSpan', () => {
620620
});
621621
});
622622
});
623+
624+
it('explicit parentSpan takes precedence over parentSpanIsAlwaysRootSpan=true', () => {
625+
const options = getDefaultTestClientOptions({
626+
tracesSampleRate: 1,
627+
parentSpanIsAlwaysRootSpan: true,
628+
});
629+
client = new TestClient(options);
630+
setCurrentClient(client);
631+
client.init();
632+
633+
const parentSpan = startInactiveSpan({ name: 'parent span' });
634+
635+
startSpan({ name: 'parent span' }, () => {
636+
startSpan({ name: 'child span' }, () => {
637+
startSpan({ name: 'grand child span', parentSpan }, grandChildSpan => {
638+
expect(spanToJSON(grandChildSpan).parent_span_id).toBe(parentSpan.spanContext().spanId);
639+
});
640+
});
641+
});
642+
});
643+
644+
it('explicit parentSpan=null takes precedence over parentSpanIsAlwaysRootSpan=true', () => {
645+
const options = getDefaultTestClientOptions({
646+
tracesSampleRate: 1,
647+
parentSpanIsAlwaysRootSpan: true,
648+
});
649+
client = new TestClient(options);
650+
setCurrentClient(client);
651+
client.init();
652+
653+
startSpan({ name: 'parent span' }, () => {
654+
startSpan({ name: 'child span' }, () => {
655+
startSpan({ name: 'grand child span', parentSpan: null }, grandChildSpan => {
656+
expect(spanToJSON(grandChildSpan).parent_span_id).toBe(undefined);
657+
});
658+
});
659+
});
660+
});
623661
});
624662

625663
it('samples with a tracesSampler', () => {
@@ -1174,6 +1212,46 @@ describe('startSpanManual', () => {
11741212
span.end();
11751213
});
11761214
});
1215+
1216+
it('explicit parentSpan takes precedence over parentSpanIsAlwaysRootSpan=true', () => {
1217+
const options = getDefaultTestClientOptions({
1218+
tracesSampleRate: 1,
1219+
parentSpanIsAlwaysRootSpan: true,
1220+
});
1221+
client = new TestClient(options);
1222+
setCurrentClient(client);
1223+
client.init();
1224+
1225+
const parentSpan = startInactiveSpan({ name: 'parent span' });
1226+
1227+
startSpan({ name: 'parent span' }, () => {
1228+
startSpan({ name: 'child span' }, () => {
1229+
startSpanManual({ name: 'grand child span', parentSpan }, grandChildSpan => {
1230+
expect(spanToJSON(grandChildSpan).parent_span_id).toBe(parentSpan.spanContext().spanId);
1231+
grandChildSpan.end();
1232+
});
1233+
});
1234+
});
1235+
});
1236+
1237+
it('explicit parentSpan=null takes precedence over parentSpanIsAlwaysRootSpan=true', () => {
1238+
const options = getDefaultTestClientOptions({
1239+
tracesSampleRate: 1,
1240+
parentSpanIsAlwaysRootSpan: true,
1241+
});
1242+
client = new TestClient(options);
1243+
setCurrentClient(client);
1244+
client.init();
1245+
1246+
startSpan({ name: 'parent span' }, () => {
1247+
startSpan({ name: 'child span' }, () => {
1248+
startSpanManual({ name: 'grand child span', parentSpan: null }, grandChildSpan => {
1249+
expect(spanToJSON(grandChildSpan).parent_span_id).toBe(undefined);
1250+
grandChildSpan.end();
1251+
});
1252+
});
1253+
});
1254+
});
11771255
});
11781256

11791257
it('sets a child span reference on the parent span', () => {
@@ -1543,6 +1621,44 @@ describe('startInactiveSpan', () => {
15431621
});
15441622
});
15451623
});
1624+
1625+
it('explicit parentSpan takes precedence over parentSpanIsAlwaysRootSpan=true', () => {
1626+
const options = getDefaultTestClientOptions({
1627+
tracesSampleRate: 1,
1628+
parentSpanIsAlwaysRootSpan: true,
1629+
});
1630+
client = new TestClient(options);
1631+
setCurrentClient(client);
1632+
client.init();
1633+
1634+
const parentSpan = startInactiveSpan({ name: 'parent span' });
1635+
1636+
startSpan({ name: 'parent span' }, () => {
1637+
startSpan({ name: 'child span' }, () => {
1638+
const grandChildSpan = startInactiveSpan({ name: 'grand child span', parentSpan });
1639+
expect(spanToJSON(grandChildSpan).parent_span_id).toBe(parentSpan.spanContext().spanId);
1640+
grandChildSpan.end();
1641+
});
1642+
});
1643+
});
1644+
1645+
it('explicit parentSpan=null takes precedence over parentSpanIsAlwaysRootSpan=true', () => {
1646+
const options = getDefaultTestClientOptions({
1647+
tracesSampleRate: 1,
1648+
parentSpanIsAlwaysRootSpan: true,
1649+
});
1650+
client = new TestClient(options);
1651+
setCurrentClient(client);
1652+
client.init();
1653+
1654+
startSpan({ name: 'parent span' }, () => {
1655+
startSpan({ name: 'child span' }, () => {
1656+
const grandChildSpan = startInactiveSpan({ name: 'grand child span', parentSpan: null });
1657+
expect(spanToJSON(grandChildSpan).parent_span_id).toBe(undefined);
1658+
grandChildSpan.end();
1659+
});
1660+
});
1661+
});
15461662
});
15471663

15481664
it('includes the scope at the time the span was started when finished', async () => {

0 commit comments

Comments
 (0)