Skip to content

Commit 3b2cbb6

Browse files
mydeamydea
committed
feat: Ensure getRootSpan() does not rely on transaction
Instead, we keep the root span as a non-enumerable property on the span.
1 parent cd5d63a commit 3b2cbb6

File tree

34 files changed

+170
-145
lines changed

34 files changed

+170
-145
lines changed

‎packages/angular/src/tracing.ts

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -289,10 +289,6 @@ export class TraceService implements OnDestroy {
289289
}
290290

291291
const rootSpan = getRootSpan(activeSpan);
292-
if (!rootSpan) {
293-
this._pageloadOngoing = false;
294-
return false;
295-
}
296292

297293
this._pageloadOngoing = spanToJSON(rootSpan).op === 'pageload';
298294
return this._pageloadOngoing;

‎packages/astro/src/index.server.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ export {
5959
linkedErrorsIntegration,
6060
setMeasurement,
6161
getActiveSpan,
62+
getRootSpan,
6263
startSpan,
6364
startInactiveSpan,
6465
startSpanManual,

‎packages/astro/src/index.types.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ export declare const startSpan: typeof clientSdk.startSpan;
3232
export declare const startInactiveSpan: typeof clientSdk.startInactiveSpan;
3333
export declare const startSpanManual: typeof clientSdk.startSpanManual;
3434
export declare const withActiveSpan: typeof clientSdk.withActiveSpan;
35+
export declare const getRootSpan: typeof clientSdk.getRootSpan;
3536
export declare const Span: clientSdk.Span;
3637

3738
export declare const metrics: typeof clientSdk.metrics & typeof serverSdk.metrics;

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ addTracingExtensions();
88

99
export {
1010
getActiveSpan,
11+
getRootSpan,
1112
startSpan,
1213
startInactiveSpan,
1314
startSpanManual,

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ addTracingExtensions();
88

99
export {
1010
getActiveSpan,
11+
getRootSpan,
1112
startSpan,
1213
startInactiveSpan,
1314
startSpanManual,

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ addTracingExtensions();
88

99
export {
1010
getActiveSpan,
11+
getRootSpan,
1112
startSpan,
1213
startInactiveSpan,
1314
startSpanManual,

‎packages/browser/src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ export type { RequestInstrumentationOptions } from '@sentry-internal/tracing';
6363
export {
6464
addTracingExtensions,
6565
getActiveSpan,
66+
getRootSpan,
6667
startSpan,
6768
startInactiveSpan,
6869
startSpanManual,

‎packages/bun/src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ export {
6464
withMonitor,
6565
setMeasurement,
6666
getActiveSpan,
67+
getRootSpan,
6768
startSpan,
6869
startInactiveSpan,
6970
startSpanManual,

‎packages/core/src/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,8 +91,8 @@ export {
9191
spanIsSampled,
9292
spanToTraceContext,
9393
getSpanDescendants,
94+
getRootSpan,
9495
} from './utils/spanUtils';
95-
export { getRootSpan } from './utils/getRootSpan';
9696
export { applySdkMetadata } from './utils/sdkMetadata';
9797
export { DEFAULT_ENVIRONMENT } from './constants';
9898
/* eslint-disable deprecation/deprecation */

‎packages/core/src/server-runtime-client.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,7 @@ import {
2424
getDynamicSamplingContextFromClient,
2525
getDynamicSamplingContextFromSpan,
2626
} from './tracing';
27-
import { getRootSpan } from './utils/getRootSpan';
28-
import { spanToTraceContext } from './utils/spanUtils';
27+
import { getRootSpan, spanToTraceContext } from './utils/spanUtils';
2928

3029
export interface ServerRuntimeClientOptions extends ClientOptions<BaseTransportOptions> {
3130
platform?: string;
@@ -256,8 +255,9 @@ export class ServerRuntimeClient<
256255
// eslint-disable-next-line deprecation/deprecation
257256
const span = scope.getSpan();
258257
if (span) {
259-
const samplingContext = getRootSpan(span) ? getDynamicSamplingContextFromSpan(span) : undefined;
260-
return [samplingContext, spanToTraceContext(span)];
258+
const rootSpan = getRootSpan(span);
259+
const samplingContext = getDynamicSamplingContextFromSpan(rootSpan);
260+
return [samplingContext, spanToTraceContext(rootSpan)];
261261
}
262262

263263
const { traceId, spanId, parentSpanId, dsc } = scope.getPropagationContext();

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,7 @@ import { dropUndefinedKeys } from '@sentry/utils';
44
import { DEFAULT_ENVIRONMENT } from '../constants';
55
import { getClient } from '../currentScopes';
66
import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '../semanticAttributes';
7-
import { getRootSpan } from '../utils/getRootSpan';
8-
import { spanIsSampled, spanToJSON } from '../utils/spanUtils';
7+
import { getRootSpan, spanIsSampled, spanToJSON } from '../utils/spanUtils';
98

109
/**
1110
* Creates a dynamic sampling context from a client.

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

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,9 @@ import {
55
} from '@sentry/utils';
66

77
import { DEBUG_BUILD } from '../debug-build';
8+
import { getRootSpan } from '../utils/spanUtils';
89
import { SPAN_STATUS_ERROR } from './spanstatus';
9-
import { getActiveTransaction } from './utils';
10+
import { getActiveSpan } from './utils';
1011

1112
let errorsInstrumented = false;
1213

@@ -29,15 +30,15 @@ export function registerErrorInstrumentation(): void {
2930
}
3031

3132
/**
32-
* If an error or unhandled promise occurs, we mark the active transaction as failed
33+
* If an error or unhandled promise occurs, we mark the active root span as failed
3334
*/
3435
function errorCallback(): void {
35-
// eslint-disable-next-line deprecation/deprecation
36-
const activeTransaction = getActiveTransaction();
37-
if (activeTransaction) {
36+
const activeSpan = getActiveSpan();
37+
const rootSpan = activeSpan && getRootSpan(activeSpan);
38+
if (rootSpan) {
3839
const message = 'internal_error';
39-
DEBUG_BUILD && logger.log(`[Tracing] Transaction: ${message} -> Global error occured`);
40-
activeTransaction.setStatus({ code: SPAN_STATUS_ERROR, message });
40+
DEBUG_BUILD && logger.log(`[Tracing] Root span: ${message} -> Global error occured`);
41+
rootSpan.setStatus({ code: SPAN_STATUS_ERROR, message });
4142
}
4243
}
4344

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

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,22 @@
1-
import type { MeasurementUnit } from '@sentry/types';
1+
import type { MeasurementUnit, Span, Transaction } from '@sentry/types';
2+
import { getRootSpan } from '../utils/spanUtils';
23

3-
import { getActiveTransaction } from './utils';
4+
import { getActiveSpan } from './utils';
45

56
/**
67
* Adds a measurement to the current active transaction.
78
*/
89
export function setMeasurement(name: string, value: number, unit: MeasurementUnit): void {
9-
// eslint-disable-next-line deprecation/deprecation
10-
const transaction = getActiveTransaction();
11-
if (transaction) {
10+
const activeSpan = getActiveSpan();
11+
const rootSpan = activeSpan && getRootSpan(activeSpan);
12+
13+
if (rootSpan && rootSpanIsTransaction(rootSpan)) {
1214
// eslint-disable-next-line deprecation/deprecation
13-
transaction.setMeasurement(name, value, unit);
15+
rootSpan.setMeasurement(name, value, unit);
1416
}
1517
}
18+
19+
function rootSpanIsTransaction(rootSpan: Span): rootSpan is Transaction {
20+
// eslint-disable-next-line deprecation/deprecation
21+
return typeof (rootSpan as Transaction).setMeasurement === 'function';
22+
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,11 @@ import { getClient } from '../currentScopes';
1717
import { DEBUG_BUILD } from '../debug-build';
1818
import { getMetricSummaryJsonForSpan } from '../metrics/metric-summary';
1919
import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '../semanticAttributes';
20-
import { getRootSpan } from '../utils/getRootSpan';
2120
import {
2221
TRACE_FLAG_NONE,
2322
TRACE_FLAG_SAMPLED,
2423
addChildSpanToSpan,
24+
getRootSpan,
2525
getStatusMessage,
2626
spanTimeInputToSeconds,
2727
spanToJSON,

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

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -50,15 +50,13 @@ export function startSpan<T>(context: StartSpanOptions, callback: (span: Span) =
5050
return handleCallbackErrors(
5151
() => callback(activeSpan),
5252
() => {
53-
// Only update the span status if it hasn't been changed yet
54-
if (activeSpan) {
55-
const { status } = spanToJSON(activeSpan);
56-
if (!status || status === 'ok') {
57-
activeSpan.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' });
58-
}
53+
// Only update the span status if it hasn't been changed yet, and the span is not yet finished
54+
const { status } = spanToJSON(activeSpan);
55+
if (activeSpan.isRecording() && (!status || status === 'ok')) {
56+
activeSpan.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' });
5957
}
6058
},
61-
() => activeSpan && activeSpan.end(),
59+
() => activeSpan.end(),
6260
);
6361
});
6462
}
@@ -97,18 +95,16 @@ export function startSpanManual<T>(context: StartSpanOptions, callback: (span: S
9795
scope.setSpan(activeSpan);
9896

9997
function finishAndSetSpan(): void {
100-
activeSpan && activeSpan.end();
98+
activeSpan.end();
10199
}
102100

103101
return handleCallbackErrors(
104102
() => callback(activeSpan, finishAndSetSpan),
105103
() => {
106104
// Only update the span status if it hasn't been changed yet, and the span is not yet finished
107-
if (activeSpan && activeSpan.isRecording()) {
108-
const { status } = spanToJSON(activeSpan);
109-
if (!status || status === 'ok') {
110-
activeSpan.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' });
111-
}
105+
const { status } = spanToJSON(activeSpan);
106+
if (activeSpan.isRecording() && (!status || status === 'ok')) {
107+
activeSpan.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' });
112108
}
113109
},
114110
);
@@ -315,6 +311,13 @@ function createChildSpanOrTransaction(
315311
});
316312
}
317313

314+
// TODO v8: Technically `startTransaction` can return undefined, which is not reflected by the types
315+
// This happens if tracing extensions have not been added
316+
// In this case, we just want to return a non-recording span
317+
if (!span) {
318+
return new SentryNonRecordingSpan();
319+
}
320+
318321
setCapturedScopesOnSpan(span, scope, isolationScope);
319322

320323
return span;

‎packages/core/src/utils/applyScopeDataToEvent.ts

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
11
import type { Breadcrumb, Event, ScopeData, Span } from '@sentry/types';
22
import { arrayify, dropUndefinedKeys } from '@sentry/utils';
33
import { getDynamicSamplingContextFromSpan } from '../tracing/dynamicSamplingContext';
4-
import { getRootSpan } from './getRootSpan';
5-
import { spanToJSON, spanToTraceContext } from './spanUtils';
4+
import { getRootSpan, spanToJSON, spanToTraceContext } from './spanUtils';
65

76
/**
87
* Applies data from the scope to the event and runs all event processors on it.
@@ -173,16 +172,14 @@ function applySdkMetadataToEvent(event: Event, sdkProcessingMetadata: ScopeData[
173172
function applySpanToEvent(event: Event, span: Span): void {
174173
event.contexts = { trace: spanToTraceContext(span), ...event.contexts };
175174
const rootSpan = getRootSpan(span);
176-
if (rootSpan) {
177-
event.sdkProcessingMetadata = {
178-
dynamicSamplingContext: getDynamicSamplingContextFromSpan(span),
179-
...event.sdkProcessingMetadata,
180-
};
181-
182-
const transactionName = spanToJSON(rootSpan).description;
183-
if (transactionName && !event.transaction) {
184-
event.transaction = transactionName;
185-
}
175+
event.sdkProcessingMetadata = {
176+
dynamicSamplingContext: getDynamicSamplingContextFromSpan(span),
177+
...event.sdkProcessingMetadata,
178+
};
179+
180+
const transactionName = spanToJSON(rootSpan).description;
181+
if (transactionName && !event.transaction) {
182+
event.transaction = transactionName;
186183
}
187184
}
188185

‎packages/core/src/utils/getRootSpan.ts

Lines changed: 0 additions & 16 deletions
This file was deleted.

‎packages/core/src/utils/spanUtils.ts

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,12 @@ import type {
77
SpanTimeInput,
88
TraceContext,
99
} from '@sentry/types';
10-
import { dropUndefinedKeys, generateSentryTraceHeader, timestampInSeconds } from '@sentry/utils';
10+
import {
11+
addNonEnumerableProperty,
12+
dropUndefinedKeys,
13+
generateSentryTraceHeader,
14+
timestampInSeconds,
15+
} from '@sentry/utils';
1116
import { getMetricSummaryJsonForSpan } from '../metrics/metric-summary';
1217
import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '../semanticAttributes';
1318
import type { SentrySpan } from '../tracing/sentrySpan';
@@ -169,19 +174,28 @@ export function getStatusMessage(status: SpanStatus | undefined): string | undef
169174
}
170175

171176
const CHILD_SPANS_FIELD = '_sentryChildSpans';
177+
const ROOT_SPAN_FIELD = '_sentryRootSpan';
172178

173179
type SpanWithPotentialChildren = Span & {
174180
[CHILD_SPANS_FIELD]?: Set<Span>;
181+
[ROOT_SPAN_FIELD]?: Span;
175182
};
176183

177184
/**
178185
* Adds an opaque child span reference to a span.
179186
*/
180187
export function addChildSpanToSpan(span: SpanWithPotentialChildren, childSpan: Span): void {
188+
// We store the root span reference on the child span
189+
// We need this for `getRootSpan()` to work
190+
const rootSpan = span[ROOT_SPAN_FIELD] || span;
191+
addNonEnumerableProperty(childSpan as SpanWithPotentialChildren, ROOT_SPAN_FIELD, rootSpan);
192+
193+
// We store a list of child spans on the parent span
194+
// We need this for `getSpanDescendants()` to work
181195
if (span[CHILD_SPANS_FIELD] && span[CHILD_SPANS_FIELD].size < 1000) {
182196
span[CHILD_SPANS_FIELD].add(childSpan);
183197
} else {
184-
span[CHILD_SPANS_FIELD] = new Set([childSpan]);
198+
addNonEnumerableProperty(span, CHILD_SPANS_FIELD, new Set([childSpan]));
185199
}
186200
}
187201

@@ -216,3 +230,10 @@ export function getSpanDescendants(span: SpanWithPotentialChildren): Span[] {
216230

217231
return Array.from(resultSet);
218232
}
233+
234+
/**
235+
* Returns the root span of a given span.
236+
*/
237+
export function getRootSpan(span: SpanWithPotentialChildren): Span {
238+
return span[ROOT_SPAN_FIELD] || span;
239+
}

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ describe('startSpan', () => {
4242
getIsolationScope().clear();
4343
getGlobalScope().clear();
4444

45-
const options = getDefaultTestClientOptions({ tracesSampleRate: 0.0 });
45+
const options = getDefaultTestClientOptions({ tracesSampleRate: 1 });
4646
client = new TestClient(options);
4747
setCurrentClient(client);
4848
client.init();
@@ -228,7 +228,7 @@ describe('startSpan', () => {
228228
expect(jsonSpan).toEqual({
229229
data: {
230230
'sentry.origin': 'auto.http.browser',
231-
'sentry.sample_rate': 0,
231+
'sentry.sample_rate': 1,
232232
'sentry.source': 'custom',
233233
},
234234
origin: 'auto.http.browser',
@@ -1022,7 +1022,7 @@ describe('startInactiveSpan', () => {
10221022

10231023
describe('continueTrace', () => {
10241024
beforeEach(() => {
1025-
const options = getDefaultTestClientOptions({ tracesSampleRate: 0.0 });
1025+
const options = getDefaultTestClientOptions({ tracesSampleRate: 1.0 });
10261026
client = new TestClient(options);
10271027
setCurrentClient(client);
10281028
client.init();
@@ -1222,7 +1222,7 @@ describe('span hooks', () => {
12221222
getIsolationScope().clear();
12231223
getGlobalScope().clear();
12241224

1225-
const options = getDefaultTestClientOptions({ tracesSampleRate: 0.0 });
1225+
const options = getDefaultTestClientOptions({ tracesSampleRate: 1.0 });
12261226
client = new TestClient(options);
12271227
setCurrentClient(client);
12281228
client.init();

0 commit comments

Comments
 (0)