Skip to content

Commit 7634a76

Browse files
authored
feat(browser): Use idle span for browser tracing (#10957)
And remove idle transaction. With this, we can afterwards remove the span recorder, and everything related to it 🎉 I tried to keep all functionality intact. Some small cleanups I made: - idle span finish reason is kept as an attribute - to keep backwards compat I finally also write it as a tag on the transaction event - idle span trimming happens automatically now (no more `trimEnd` option - we always set this to true anyhow). However, this only applies when the span is auto-ended by the idle functionality, not if manually calling `span.end()` - which is fine IMHO because that will usually not happen for users, and even so it's not the end of the world. - We now use `continueTrace` for trace propagation of the pageload span. - no more `location` on the sampling context - we talked about this before, this is just gone, and users can instead either access the attributes from the sampling context, or just do `window.location.href` themselves, which should have the same outcome.
1 parent 7c655fb commit 7634a76

File tree

21 files changed

+419
-1415
lines changed

21 files changed

+419
-1415
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
import * as Sentry from '@sentry/browser';
2+
3+
window.Sentry = Sentry;
4+
5+
Sentry.init({
6+
dsn: 'https://[email protected]/1337',
7+
integrations: [
8+
Sentry.browserTracingIntegration({
9+
// To avoid having this test run for 15s
10+
childSpanTimeout: 4000,
11+
}),
12+
],
13+
defaultIntegrations: false,
14+
tracesSampleRate: 1,
15+
});
16+
17+
const activeSpan = Sentry.getActiveSpan();
18+
if (activeSpan) {
19+
Sentry.startInactiveSpan({ name: 'pageload-child-span' });
20+
} else {
21+
setTimeout(() => {
22+
Sentry.startInactiveSpan({ name: 'pageload-child-span' });
23+
}, 200);
24+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
import { expect } from '@playwright/test';
2+
3+
import { sentryTest } from '../../../../utils/fixtures';
4+
import {
5+
envelopeRequestParser,
6+
shouldSkipTracingTest,
7+
waitForTransactionRequestOnUrl,
8+
} from '../../../../utils/helpers';
9+
10+
// This tests asserts that the pageload span will finish itself after the child span timeout if it
11+
// has a child span without adding any additional ones or finishing any of them finishing. All of the child spans that
12+
// are still running should have the status "cancelled".
13+
sentryTest('should send a pageload span terminated via child span timeout', async ({ getLocalTestUrl, page }) => {
14+
if (shouldSkipTracingTest()) {
15+
sentryTest.skip();
16+
}
17+
18+
const url = await getLocalTestUrl({ testDir: __dirname });
19+
const req = await waitForTransactionRequestOnUrl(page, url);
20+
21+
const eventData = envelopeRequestParser(req);
22+
23+
expect(eventData.contexts?.trace?.op).toBe('pageload');
24+
expect(eventData.spans?.length).toBeGreaterThanOrEqual(1);
25+
const testSpan = eventData.spans?.find(span => span.description === 'pageload-child-span');
26+
expect(testSpan).toBeDefined();
27+
expect(testSpan?.status).toBe('cancelled');
28+
});

dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/pageloadWithHeartbeatTimeout/init.js

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

dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/pageloadWithHeartbeatTimeout/test.ts

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

packages/core/src/semanticAttributes.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,3 +19,6 @@ export const SEMANTIC_ATTRIBUTE_SENTRY_OP = 'sentry.op';
1919
* Use this attribute to represent the origin of a span.
2020
*/
2121
export const SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN = 'sentry.origin';
22+
23+
/** The reason why an idle span finished. */
24+
export const SEMANTIC_ATTRIBUTE_SENTRY_IDLE_SPAN_FINISH_REASON = 'sentry.idle_span_finish_reason';

packages/core/src/tracing/hubextensions.ts

Lines changed: 0 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ import type { ClientOptions, CustomSamplingContext, Hub, TransactionContext } fr
22
import { getMainCarrier } from '../asyncContext';
33

44
import { registerErrorInstrumentation } from './errors';
5-
import { IdleTransaction } from './idletransaction';
65
import { sampleTransaction } from './sampling';
76
import { Transaction } from './transaction';
87

@@ -53,54 +52,6 @@ function _startTransaction(
5352
return transaction;
5453
}
5554

56-
/**
57-
* Create new idle transaction.
58-
*/
59-
export function startIdleTransaction(
60-
hub: Hub,
61-
transactionContext: TransactionContext,
62-
idleTimeout: number,
63-
finalTimeout: number,
64-
onScope?: boolean,
65-
customSamplingContext?: CustomSamplingContext,
66-
heartbeatInterval?: number,
67-
delayAutoFinishUntilSignal: boolean = false,
68-
): IdleTransaction {
69-
// eslint-disable-next-line deprecation/deprecation
70-
const client = hub.getClient();
71-
const options: Partial<ClientOptions> = (client && client.getOptions()) || {};
72-
73-
// eslint-disable-next-line deprecation/deprecation
74-
let transaction = new IdleTransaction(
75-
transactionContext,
76-
hub,
77-
idleTimeout,
78-
finalTimeout,
79-
heartbeatInterval,
80-
onScope,
81-
delayAutoFinishUntilSignal,
82-
);
83-
transaction = sampleTransaction(transaction, options, {
84-
name: transactionContext.name,
85-
parentSampled: transactionContext.parentSampled,
86-
transactionContext,
87-
attributes: {
88-
// eslint-disable-next-line deprecation/deprecation
89-
...transactionContext.data,
90-
...transactionContext.attributes,
91-
},
92-
...customSamplingContext,
93-
});
94-
if (transaction.isRecording()) {
95-
transaction.initSpanRecorder();
96-
}
97-
if (client) {
98-
client.emit('startTransaction', transaction);
99-
client.emit('spanStart', transaction);
100-
}
101-
return transaction;
102-
}
103-
10455
/**
10556
* Adds tracing extensions to the global hub.
10657
*/

packages/core/src/tracing/idleSpan.ts

Lines changed: 41 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
1-
import type { Span, StartSpanOptions } from '@sentry/types';
1+
import type { Span, SpanAttributes, StartSpanOptions } from '@sentry/types';
22
import { logger, timestampInSeconds } from '@sentry/utils';
33
import { getClient, getCurrentScope } from '../currentScopes';
44

55
import { DEBUG_BUILD } from '../debug-build';
6+
import { SEMANTIC_ATTRIBUTE_SENTRY_IDLE_SPAN_FINISH_REASON } from '../semanticAttributes';
67
import { hasTracingEnabled } from '../utils/hasTracingEnabled';
7-
import { getSpanDescendants, removeChildSpanFromSpan, spanToJSON } from '../utils/spanUtils';
8+
import { getSpanDescendants, removeChildSpanFromSpan, spanTimeInputToSeconds, spanToJSON } from '../utils/spanUtils';
89
import { SentryNonRecordingSpan } from './sentryNonRecordingSpan';
910
import { SPAN_STATUS_ERROR } from './spanstatus';
1011
import { startInactiveSpan } from './trace';
@@ -16,8 +17,6 @@ export const TRACING_DEFAULTS = {
1617
childSpanTimeout: 15_000,
1718
};
1819

19-
const FINISH_REASON_TAG = 'finishReason';
20-
2120
const FINISH_REASON_HEARTBEAT_FAILED = 'heartbeatFailed';
2221
const FINISH_REASON_IDLE_TIMEOUT = 'idleTimeout';
2322
const FINISH_REASON_FINAL_TIMEOUT = 'finalTimeout';
@@ -108,6 +107,36 @@ export function startIdleSpan(startSpanOptions: StartSpanOptions, options: Parti
108107
const previousActiveSpan = getActiveSpan();
109108
const span = _startIdleSpan(startSpanOptions);
110109

110+
function _endSpan(timestamp: number = timestampInSeconds()): void {
111+
// Ensure we end with the last span timestamp, if possible
112+
const spans = getSpanDescendants(span).filter(child => child !== span);
113+
114+
// If we have no spans, we just end, nothing else to do here
115+
if (!spans.length) {
116+
span.end(timestamp);
117+
return;
118+
}
119+
120+
const childEndTimestamps = spans
121+
.map(span => spanToJSON(span).timestamp)
122+
.filter(timestamp => !!timestamp) as number[];
123+
const latestSpanEndTimestamp = childEndTimestamps.length ? Math.max(...childEndTimestamps) : undefined;
124+
125+
const spanEndTimestamp = spanTimeInputToSeconds(timestamp);
126+
const spanStartTimestamp = spanToJSON(span).start_timestamp;
127+
128+
// The final endTimestamp should:
129+
// * Never be before the span start timestamp
130+
// * Be the latestSpanEndTimestamp, if there is one, and it is smaller than the passed span end timestamp
131+
// * Otherwise be the passed end timestamp
132+
const endTimestamp = Math.max(
133+
spanStartTimestamp || -Infinity,
134+
Math.min(spanEndTimestamp, latestSpanEndTimestamp || Infinity),
135+
);
136+
137+
span.end(endTimestamp);
138+
}
139+
111140
/**
112141
* Cancels the existing idle timeout, if there is one.
113142
*/
@@ -136,7 +165,7 @@ export function startIdleSpan(startSpanOptions: StartSpanOptions, options: Parti
136165
_idleTimeoutID = setTimeout(() => {
137166
if (!_finished && activities.size === 0 && _autoFinishAllowed) {
138167
_finishReason = FINISH_REASON_IDLE_TIMEOUT;
139-
span.end(endTimestamp);
168+
_endSpan(endTimestamp);
140169
}
141170
}, idleTimeout);
142171
}
@@ -149,7 +178,7 @@ export function startIdleSpan(startSpanOptions: StartSpanOptions, options: Parti
149178
_idleTimeoutID = setTimeout(() => {
150179
if (!_finished && _autoFinishAllowed) {
151180
_finishReason = FINISH_REASON_HEARTBEAT_FAILED;
152-
span.end(endTimestamp);
181+
_endSpan(endTimestamp);
153182
}
154183
}, childSpanTimeout);
155184
}
@@ -190,7 +219,7 @@ export function startIdleSpan(startSpanOptions: StartSpanOptions, options: Parti
190219
}
191220
}
192221

193-
function endIdleSpan(): void {
222+
function onIdleSpanEnded(): void {
194223
_finished = true;
195224
activities.clear();
196225

@@ -209,9 +238,9 @@ export function startIdleSpan(startSpanOptions: StartSpanOptions, options: Parti
209238
return;
210239
}
211240

212-
const attributes = spanJSON.data || {};
213-
if (spanJSON.op === 'ui.action.click' && !attributes[FINISH_REASON_TAG]) {
214-
span.setAttribute(FINISH_REASON_TAG, _finishReason);
241+
const attributes: SpanAttributes = spanJSON.data || {};
242+
if (spanJSON.op === 'ui.action.click' && !attributes[SEMANTIC_ATTRIBUTE_SENTRY_IDLE_SPAN_FINISH_REASON]) {
243+
span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_IDLE_SPAN_FINISH_REASON, _finishReason);
215244
}
216245

217246
DEBUG_BUILD &&
@@ -279,7 +308,7 @@ export function startIdleSpan(startSpanOptions: StartSpanOptions, options: Parti
279308
_popActivity(endedSpan.spanContext().spanId);
280309

281310
if (endedSpan === span) {
282-
endIdleSpan();
311+
onIdleSpanEnded();
283312
}
284313
});
285314

@@ -303,7 +332,7 @@ export function startIdleSpan(startSpanOptions: StartSpanOptions, options: Parti
303332
if (!_finished) {
304333
span.setStatus({ code: SPAN_STATUS_ERROR, message: 'deadline_exceeded' });
305334
_finishReason = FINISH_REASON_FINAL_TIMEOUT;
306-
span.end();
335+
_endSpan();
307336
}
308337
}, finalTimeout);
309338

0 commit comments

Comments
 (0)