Skip to content

Commit ccde98b

Browse files
authored
feat: Ensure getRootSpan() does not rely on transaction (#10979)
Instead, we keep the root span as a non-enumerable property on the span. We use the already existing method for this, so this should work reasonably well. This also means that now `getRootSpan` can always return a `Span`, not `undefined` anymore, which makes types a bit easier there. Also, we now ensure to export this everywhere.
1 parent 99bcc3d commit ccde98b

File tree

47 files changed

+184
-159
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

47 files changed

+184
-159
lines changed

dev-packages/browser-integration-tests/suites/manual-client/browser-context/init.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ const client = new BrowserClient({
2929
transport: makeFetchTransport,
3030
stackParser: defaultStackParser,
3131
integrations,
32-
debug: true,
3332
});
3433

3534
const hub = new Hub(client);

dev-packages/browser-integration-tests/suites/public-api/instrumentation/setTimeoutFrozen/init.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,5 +4,6 @@ window.Sentry = Sentry;
44

55
Sentry.init({
66
dsn: 'https://[email protected]/1337',
7+
// We assert on the debug message, so we need to enable debug mode
78
debug: true,
89
});

dev-packages/browser-integration-tests/suites/public-api/startSpan/basic/test.ts

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,20 @@
11
import { expect } from '@playwright/test';
2-
import type { Event } from '@sentry/types';
32

43
import { sentryTest } from '../../../../utils/fixtures';
5-
import { getFirstSentryEnvelopeRequest, shouldSkipTracingTest } from '../../../../utils/helpers';
4+
import {
5+
envelopeRequestParser,
6+
shouldSkipTracingTest,
7+
waitForTransactionRequestOnUrl,
8+
} from '../../../../utils/helpers';
69

710
sentryTest('should send a transaction in an envelope', async ({ getLocalTestPath, page }) => {
811
if (shouldSkipTracingTest()) {
912
sentryTest.skip();
1013
}
1114

1215
const url = await getLocalTestPath({ testDir: __dirname });
13-
const transaction = await getFirstSentryEnvelopeRequest<Event>(page, url);
16+
const req = await waitForTransactionRequestOnUrl(page, url);
17+
const transaction = envelopeRequestParser(req);
1418

1519
expect(transaction.transaction).toBe('parent_span');
1620
expect(transaction.spans).toBeDefined();
@@ -22,7 +26,8 @@ sentryTest('should report finished spans as children of the root transaction', a
2226
}
2327

2428
const url = await getLocalTestPath({ testDir: __dirname });
25-
const transaction = await getFirstSentryEnvelopeRequest<Event>(page, url);
29+
const req = await waitForTransactionRequestOnUrl(page, url);
30+
const transaction = envelopeRequestParser(req);
2631

2732
expect(transaction.spans).toHaveLength(1);
2833

dev-packages/browser-integration-tests/suites/public-api/startSpan/init.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,5 +8,4 @@ Sentry.init({
88
dsn: 'https://[email protected]/1337',
99
tracesSampleRate: 1.0,
1010
normalizeDepth: 10,
11-
debug: true,
1211
});

dev-packages/browser-integration-tests/suites/replay/canvas/manualSnapshot/init.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ Sentry.init({
1212
sampleRate: 0,
1313
replaysSessionSampleRate: 1.0,
1414
replaysOnErrorSampleRate: 0.0,
15-
debug: true,
1615

1716
integrations: [window.Replay, Sentry.replayCanvasIntegration({ enableManualSnapshot: true })],
1817
});

dev-packages/browser-integration-tests/suites/replay/canvas/records/init.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ Sentry.init({
1212
sampleRate: 0,
1313
replaysSessionSampleRate: 1.0,
1414
replaysOnErrorSampleRate: 0.0,
15-
debug: true,
1615

1716
integrations: [window.Replay, Sentry.replayCanvasIntegration()],
1817
});

dev-packages/browser-integration-tests/suites/replay/canvas/withCanvasIntegrationFirst/init.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ Sentry.init({
1212
sampleRate: 0,
1313
replaysSessionSampleRate: 1.0,
1414
replaysOnErrorSampleRate: 0.0,
15-
debug: true,
1615

1716
integrations: [Sentry.replayCanvasIntegration(), window.Replay],
1817
});

dev-packages/browser-integration-tests/suites/replay/canvas/withCanvasIntegrationSecond/init.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ Sentry.init({
1212
sampleRate: 0,
1313
replaysSessionSampleRate: 1.0,
1414
replaysOnErrorSampleRate: 0.0,
15-
debug: true,
1615

1716
integrations: [window.Replay, Sentry.replayCanvasIntegration()],
1817
});

dev-packages/browser-integration-tests/suites/replay/canvas/withoutCanvasIntegration/init.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ Sentry.init({
1212
sampleRate: 0,
1313
replaysSessionSampleRate: 1.0,
1414
replaysOnErrorSampleRate: 0.0,
15-
debug: true,
1615

1716
integrations: [window.Replay],
1817
});

dev-packages/browser-integration-tests/suites/sessions/initial-scope/init.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,5 +12,4 @@ Sentry.init({
1212
username: 'user1337',
1313
},
1414
},
15-
debug: true,
1615
});

dev-packages/browser-integration-tests/suites/tracing/envelope-header-transaction-name/init.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ Sentry.init({
88
integrations: [Sentry.browserTracingIntegration()],
99
environment: 'production',
1010
tracesSampleRate: 1,
11-
debug: true,
1211
});
1312

1413
Sentry.setUser({ id: 'user123' });

dev-packages/browser-integration-tests/suites/tracing/envelope-header/init.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ Sentry.init({
88
tracePropagationTargets: [/.*/],
99
environment: 'production',
1010
tracesSampleRate: 1,
11-
debug: true,
1211
});
1312

1413
const scope = Sentry.getCurrentScope();

dev-packages/rollup-utils/plugins/bundlePlugins.mjs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,9 @@ export function makeTerserPlugin() {
124124
// For v7 backwards-compatibility we need to access txn._frozenDynamicSamplingContext
125125
// TODO (v8): Remove this reserved word
126126
'_frozenDynamicSamplingContext',
127+
// These are used to keep span relationships
128+
'_sentryRootSpan',
129+
'_sentryChildSpans',
127130
],
128131
},
129132
},

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
@@ -54,6 +54,7 @@ export {
5454
withMonitor,
5555
setMeasurement,
5656
getActiveSpan,
57+
getRootSpan,
5758
startSpan,
5859
startInactiveSpan,
5960
startSpanManual,

packages/core/src/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,8 +92,8 @@ export {
9292
spanToTraceContext,
9393
getSpanDescendants,
9494
getStatusMessage,
95+
getRootSpan,
9596
} from './utils/spanUtils';
96-
export { getRootSpan } from './utils/getRootSpan';
9797
export { applySdkMetadata } from './utils/sdkMetadata';
9898
export { DEFAULT_ENVIRONMENT } from './constants';
9999
/* 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

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;

0 commit comments

Comments
 (0)