Skip to content

Commit 9482537

Browse files
logaretmlogaretm
authored
feat(browser): Run static beforeSendSpan for INP spans (#22877)
This fixes a `beforeSendSpan` gap for INP spans in the static trace lifecycle, without reintroducing v1 spans. INP is always sent as a v2 span. When span streaming is disabled it goes through the standalone send path, which runs `captureSpan`. That only honors a `beforeSendSpan` wrapped with `withStreamedSpan`, so a plain callback (the one static users write, typed for v1 `SpanJSON`) was silently skipped for INP. This also lets it call the `preprocessSpan` and `processSpan` hooks.
1 parent 9698d03 commit 9482537

9 files changed

Lines changed: 330 additions & 14 deletions

File tree

‎dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-before-send-span/init.js‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
import * as Sentry from '@sentry/browser';
2+
3+
window.Sentry = Sentry;
4+
5+
Sentry.init({
6+
traceLifecycle: 'static',
7+
dsn: 'https://public@dsn.ingest.sentry.io/1337',
8+
integrations: [
9+
Sentry.browserTracingIntegration({
10+
idleTimeout: 4000,
11+
enableLongTask: false,
12+
enableInp: true,
13+
instrumentPageLoad: false,
14+
instrumentNavigation: false,
15+
}),
16+
],
17+
tracesSampleRate: 1,
18+
// A plain (non-streamed) `beforeSendSpan` operates on the v1 `SpanJSON`. INP is sent as a v2 span,
19+
// so this verifies the static callback still runs and its changes are carried into the v2 span.
20+
beforeSendSpan: span => {
21+
if (span.op === 'ui.interaction.click') {
22+
span.description = 'scrubbed';
23+
span.data['custom.attribute'] = 'from-before-send-span';
24+
}
25+
26+
return span;
27+
},
28+
debug: true,
29+
});
30+
31+
const client = Sentry.getClient();
32+
33+
// Force page load transaction name to a testable value
34+
Sentry.startBrowserTracingPageLoadSpan(client, {
35+
name: 'test-url',
36+
attributes: {
37+
[Sentry.SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url',
38+
},
39+
});

‎dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-before-send-span/subject.js‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
const blockUI =
2+
(delay = 70) =>
3+
e => {
4+
const startTime = Date.now();
5+
6+
function getElasped() {
7+
const time = Date.now();
8+
return time - startTime;
9+
}
10+
11+
while (getElasped() < delay) {
12+
//
13+
}
14+
15+
e.target.classList.add('clicked');
16+
};
17+
18+
document.querySelector('[data-test-id=not-so-slow-button]').addEventListener('click', blockUI(300));
19+
document.querySelector('[data-test-id=slow-button]').addEventListener('click', blockUI(450));
20+
document.querySelector('[data-test-id=normal-button]').addEventListener('click', blockUI());

‎dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-before-send-span/template.html‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
<!doctype html>
2+
<html>
3+
<head>
4+
<meta charset="utf-8" />
5+
</head>
6+
<body>
7+
<div>Rendered Before Long Task</div>
8+
<button data-test-id="slow-button" data-sentry-element="SlowButton">Slow</button>
9+
<button data-test-id="not-so-slow-button" data-sentry-element="NotSoSlowButton">Not so slow</button>
10+
<button data-test-id="normal-button" data-sentry-element="NormalButton">Click Me</button>
11+
</body>
12+
</html>

‎dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-before-send-span/test.ts‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
import { expect } from '@playwright/test';
2+
import { sentryTest } from '../../../../utils/fixtures';
3+
import { hidePage, shouldSkipTracingTest } from '../../../../utils/helpers';
4+
import { getSpanOp, getSpansFromEnvelope, waitForStreamedSpanEnvelope } from '../../../../utils/spanUtils';
5+
6+
// This app does not enable span streaming (`traceLifecycle: 'static'`) and defines a plain, non-streamed
7+
// `beforeSendSpan` callback (operating on the v1 `SpanJSON`). INP is still emitted as a v2 span, so this
8+
// verifies the static callback runs for INP and its modifications are carried into the v2 span.
9+
10+
sentryTest('runs a non-streamed `beforeSendSpan` for the INP span', async ({ browserName, getLocalTestUrl, page }) => {
11+
const supportedBrowsers = ['chromium'];
12+
13+
if (shouldSkipTracingTest() || !supportedBrowsers.includes(browserName)) {
14+
sentryTest.skip();
15+
}
16+
17+
const url = await getLocalTestUrl({ testDir: __dirname });
18+
19+
const spanEnvelopePromise = waitForStreamedSpanEnvelope(
20+
page,
21+
env => !!getSpansFromEnvelope(env).find(s => getSpanOp(s) === 'ui.interaction.click'),
22+
);
23+
24+
await page.goto(url);
25+
26+
await page.locator('[data-test-id=normal-button]').click();
27+
await page.locator('.clicked[data-test-id=normal-button]').isVisible();
28+
29+
await page.waitForTimeout(500);
30+
31+
// Page hide to trigger INP
32+
await hidePage(page);
33+
34+
const spanEnvelope = await spanEnvelopePromise;
35+
const inpSpan = getSpansFromEnvelope(spanEnvelope).find(s => getSpanOp(s) === 'ui.interaction.click')!;
36+
37+
// The callback rewrote the name and added a custom attribute.
38+
expect(inpSpan.name).toBe('scrubbed');
39+
expect(inpSpan.attributes['custom.attribute']).toEqual({ value: 'from-before-send-span', type: 'string' });
40+
41+
// The span is still a valid v2 INP span carrying its web vital value.
42+
const inpValue = inpSpan.attributes['browser.web_vital.inp.value']?.value as number;
43+
expect(inpValue).toBeGreaterThan(0);
44+
});

‎packages/browser-utils/src/metrics/webVitalSpans.ts‎

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
1-
import type { Client, Span, SpanAttributes } from '@sentry/core';
1+
import type { Client, Integration, Span, SpanAttributes } from '@sentry/core';
22
import {
33
browserPerformanceTimeOrigin,
44
debug,
55
getActiveSpan,
6+
getClient,
67
getCurrentScope,
78
getRootSpan,
89
hasSpanStreamingEnabled,
@@ -108,6 +109,15 @@ export function _emitWebVitalSpan(options: WebVitalSpanOptions): void {
108109
attributes[`browser.web_vital.${metricName}.report_event`] = reportEvent;
109110
}
110111

112+
// A standalone span is sent as a plain v2 span without running the `processSpan` hooks (see
113+
// `captureStandaloneSpanWithStaticCallback`), so Replay can't attach the replay id itself. Set it
114+
// here, mirroring Replay's `processSpan`, so INP keeps its replay association like it did on v1.
115+
// TODO(standalone): remove once the static (transaction) trace lifecycle is dropped and INP always
116+
// streams, at which point Replay's `processSpan` runs and attaches the replay id.
117+
if (standalone) {
118+
Object.assign(attributes, getReplayAttributes());
119+
}
120+
111121
const span = startInactiveSpan({
112122
name,
113123
attributes,
@@ -122,6 +132,26 @@ export function _emitWebVitalSpan(options: WebVitalSpanOptions): void {
122132
}
123133
}
124134

135+
interface ReplayIntegration extends Integration {
136+
getReplayId: (onlyIfSampled?: boolean) => string | undefined;
137+
getRecordingMode: () => 'session' | 'buffer' | undefined;
138+
}
139+
140+
// TODO(standalone): remove once the static (transaction) trace lifecycle is dropped; Replay's
141+
// `processSpan` then attaches the replay id to the streamed INP span instead.
142+
function getReplayAttributes(): SpanAttributes {
143+
const replay = getClient()?.getIntegrationByName<ReplayIntegration>('Replay');
144+
const replayId = replay?.getReplayId(true);
145+
if (!replayId) {
146+
return {};
147+
}
148+
149+
return {
150+
'sentry.replay_id': replayId,
151+
'sentry._internal.replay_is_buffering': replay!.getRecordingMode() === 'buffer' ? true : undefined,
152+
};
153+
}
154+
125155
/**
126156
* Tracks LCP as a streamed span.
127157
*/

‎packages/browser-utils/test/metrics/webVitalSpans.test.ts‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ vi.mock('@sentry/core', async () => {
1919
browserPerformanceTimeOrigin: vi.fn(),
2020
timestampInSeconds: vi.fn(),
2121
getCurrentScope: vi.fn(),
22+
getClient: vi.fn(),
2223
startInactiveSpan: vi.fn(),
2324
getActiveSpan: vi.fn(),
2425
getRootSpan: vi.fn(),
@@ -64,6 +65,7 @@ describe('_emitWebVitalSpan', () => {
6465
vi.mocked(SentryCore.getCurrentScope).mockReturnValue(mockScope as any);
6566
vi.mocked(SentryCore.startInactiveSpan).mockReturnValue(mockSpan as any);
6667
vi.mocked(SentryCore.spanToStreamedSpanJSON).mockReturnValue({ attributes: {} } as any);
68+
vi.mocked(SentryCore.getClient).mockReturnValue({ getIntegrationByName: () => undefined } as any);
6769
});
6870

6971
afterEach(() => {
@@ -118,6 +120,71 @@ describe('_emitWebVitalSpan', () => {
118120
);
119121
});
120122

123+
it('adds the replay id to a standalone span when a replay is recording', () => {
124+
vi.mocked(SentryCore.getClient).mockReturnValue({
125+
getIntegrationByName: () => ({ getReplayId: () => 'replay-123', getRecordingMode: () => 'session' }),
126+
} as any);
127+
128+
_emitWebVitalSpan({
129+
name: 'Test',
130+
op: 'ui.interaction.click',
131+
origin: 'auto.http.browser.inp',
132+
metricName: 'inp',
133+
value: 100,
134+
startTime: 1.5,
135+
standalone: true,
136+
});
137+
138+
expect(SentryCore.startInactiveSpan).toHaveBeenCalledWith(
139+
expect.objectContaining({
140+
attributes: expect.objectContaining({
141+
'sentry.replay_id': 'replay-123',
142+
'sentry._internal.replay_is_buffering': undefined,
143+
}),
144+
}),
145+
);
146+
});
147+
148+
it('flags buffering when the replay is in buffer mode', () => {
149+
vi.mocked(SentryCore.getClient).mockReturnValue({
150+
getIntegrationByName: () => ({ getReplayId: () => 'replay-123', getRecordingMode: () => 'buffer' }),
151+
} as any);
152+
153+
_emitWebVitalSpan({
154+
name: 'Test',
155+
op: 'ui.interaction.click',
156+
origin: 'auto.http.browser.inp',
157+
metricName: 'inp',
158+
value: 100,
159+
startTime: 1.5,
160+
standalone: true,
161+
});
162+
163+
expect(SentryCore.startInactiveSpan).toHaveBeenCalledWith(
164+
expect.objectContaining({
165+
attributes: expect.objectContaining({ 'sentry._internal.replay_is_buffering': true }),
166+
}),
167+
);
168+
});
169+
170+
it('does not add a replay id to non-standalone spans', () => {
171+
vi.mocked(SentryCore.getClient).mockReturnValue({
172+
getIntegrationByName: () => ({ getReplayId: () => 'replay-123', getRecordingMode: () => 'session' }),
173+
} as any);
174+
175+
_emitWebVitalSpan({
176+
name: 'Test',
177+
op: 'ui.interaction.click',
178+
origin: 'auto.http.browser.inp',
179+
metricName: 'inp',
180+
value: 100,
181+
startTime: 1.5,
182+
});
183+
184+
const attributes = vi.mocked(SentryCore.startInactiveSpan).mock.calls[0]![0].attributes!;
185+
expect(attributes['sentry.replay_id']).toBeUndefined();
186+
});
187+
121188
it('includes pageload span id when parentSpan is a pageload span', () => {
122189
const mockPageloadSpan = createMockPageloadSpan('abc123');
123190
vi.mocked(SentryCore.spanToStreamedSpanJSON).mockReturnValue({

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

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,8 @@ import { getDynamicSamplingContextFromSpan } from './dynamicSamplingContext';
4646
import { logSpanEnd } from './logSpans';
4747
import { timedEventsToMeasurements } from './measurement';
4848
import { getSegmentSpanCaptureStrategy, type SegmentSpanCaptureConvertOptions } from './segmentSpanCaptureStrategy';
49-
import { captureSpan } from './spans/captureSpan';
49+
import { isStreamedBeforeSendSpanCallback } from './spans/beforeSendSpan';
50+
import { captureSpan, captureStandaloneSpanWithStaticCallback } from './spans/captureSpan';
5051
import { createStreamedSpanEnvelope } from './spans/envelope';
5152
import { hasSpanStreamingEnabled } from './spans/hasSpanStreamingEnabled';
5253
import {
@@ -556,6 +557,21 @@ function isStandaloneSpan(span: Span): boolean {
556557
* TODO(standalone): remove once the static (transaction) trace lifecycle is dropped.
557558
*/
558559
function sendStandaloneSpan(span: SentrySpan, client: Client): void {
560+
const { beforeSendSpan } = client.getOptions();
561+
562+
// A user who opted out of span streaming writes `beforeSendSpan` in the v1 `SpanJSON` format. That
563+
// callback never runs through `captureSpan` (which only honors streamed callbacks), so scrub the
564+
// span in its native v1 shape and convert it forward to v2, mirroring the gen_ai extraction path.
565+
// TODO(standalone): remove this branch once the static trace lifecycle is dropped.
566+
if (beforeSendSpan && !isStreamedBeforeSendSpanCallback(beforeSendSpan)) {
567+
const serializedSpan = captureStandaloneSpanWithStaticCallback(span, client, beforeSendSpan);
568+
const dsc = getDynamicSamplingContextFromSpan(span);
569+
// sendEnvelope should not throw
570+
// eslint-disable-next-line @typescript-eslint/no-floating-promises
571+
client.sendEnvelope(createStreamedSpanEnvelope([serializedSpan], dsc, client));
572+
return;
573+
}
574+
559575
const { _segmentSpan, ...serializedSpan } = captureSpan(span, client);
560576
const dsc = getDynamicSamplingContextFromSpan(_segmentSpan);
561577
// sendEnvelope should not throw

‎packages/core/src/tracing/spans/captureSpan.ts‎

Lines changed: 56 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,16 +11,18 @@ import {
1111
SEMANTIC_ATTRIBUTE_USER_IP_ADDRESS,
1212
SEMANTIC_ATTRIBUTE_USER_USERNAME,
1313
} from '../../semanticAttributes';
14-
import type { SerializedStreamedSpan, Span, StreamedSpanJSON } from '../../types/span';
14+
import type { SerializedStreamedSpan, Span, SpanAttributeValue, SpanJSON, StreamedSpanJSON } from '../../types/span';
1515
import { getCombinedScopeData } from '../../utils/scopeData';
1616
import {
1717
INTERNAL_getSegmentSpan,
1818
showSpanDropWarning,
19+
spanToJSON,
1920
spanToStreamedSpanJSON,
2021
streamedSpanJsonToSerializedSpan,
2122
} from '../../utils/spanUtils';
2223
import { getCapturedScopesOnSpan } from '../utils';
2324
import { isStreamedBeforeSendSpanCallback } from './beforeSendSpan';
25+
import { spanJsonToSerializedStreamedSpan } from './spanJsonToStreamedSpan';
2426
import { scopeContextsToSpanAttributes } from './scopeContextAttributes';
2527
import { DEFAULT_ENVIRONMENT } from '../../constants';
2628
import {
@@ -126,17 +128,18 @@ function applySdkMetadataToSegmentSpan(segmentSpanJSON: StreamedSpanJSON, client
126128
});
127129
}
128130

129-
function applyCommonSpanAttributes(
130-
spanJSON: StreamedSpanJSON,
131+
function commonSpanAttributes(
131132
serializedSegmentSpan: StreamedSpanJSON,
132133
client: Client,
133134
scopeData: ScopeData,
134-
): void {
135+
// TODO(standalone): remove this param (always include scope attributes) once the static (transaction)
136+
// trace lifecycle is dropped and standalone spans no longer need to look transaction-shaped.
137+
includeScopeAttributes = true,
138+
): RawAttributes<Record<string, unknown>> {
135139
const sdk = client.getSdkMetadata();
136140
const { release, environment } = client.getOptions();
137141

138-
// avoid overwriting any previously set attributes (from users or potentially our SDK instrumentation)
139-
safeSetSpanJSONAttributes(spanJSON, {
142+
return {
140143
[SENTRY_TRACE_LIFECYCLE]: 'stream',
141144
[SENTRY_SEGMENT_NAME]: serializedSegmentSpan.name,
142145
[SENTRY_SEGMENT_ID]: serializedSegmentSpan.span_id,
@@ -148,8 +151,54 @@ function applyCommonSpanAttributes(
148151
[SEMANTIC_ATTRIBUTE_USER_EMAIL]: scopeData.user?.email,
149152
[SEMANTIC_ATTRIBUTE_USER_IP_ADDRESS]: scopeData.user?.ip_address,
150153
[SEMANTIC_ATTRIBUTE_USER_USERNAME]: scopeData.user?.username,
151-
...scopeData.attributes,
154+
...(includeScopeAttributes ? scopeData.attributes : undefined),
155+
};
156+
}
157+
158+
function applyCommonSpanAttributes(
159+
spanJSON: StreamedSpanJSON,
160+
serializedSegmentSpan: StreamedSpanJSON,
161+
client: Client,
162+
scopeData: ScopeData,
163+
): void {
164+
// avoid overwriting any previously set attributes (from users or potentially our SDK instrumentation)
165+
safeSetSpanJSONAttributes(spanJSON, commonSpanAttributes(serializedSegmentSpan, client, scopeData));
166+
}
167+
168+
/**
169+
* Captures a standalone span whose `beforeSendSpan` callback expects the v1 {@link SpanJSON} format
170+
* (i.e. the user opted out of span streaming). The span is serialized to v1, the common attributes are
171+
* applied, the callback runs in its native format, and the result is converted forward to a serialized
172+
* v2 span. This mirrors how gen_ai spans reach the v2 span path from a static transaction (a plain
173+
* conversion, no `processSpan` hooks), so there is never a reverse v2 -> v1 conversion.
174+
*
175+
* TODO(standalone): remove once the static (transaction) trace lifecycle is dropped.
176+
*/
177+
export function captureStandaloneSpanWithStaticCallback(
178+
span: Span,
179+
client: Client,
180+
beforeSendSpan: (span: SpanJSON) => SpanJSON,
181+
): SerializedStreamedSpan {
182+
const spanJSON = spanToJSON(span);
183+
184+
const segmentSpan = INTERNAL_getSegmentSpan(span);
185+
const serializedSegmentSpan = spanToStreamedSpanJSON(segmentSpan);
186+
187+
const { isolationScope: spanIsolationScope, scope: spanScope } = getCapturedScopesOnSpan(span);
188+
const finalScopeData = getCombinedScopeData(spanIsolationScope, spanScope);
189+
190+
// Skip scope attributes: their `{ unit, value }` shape is unexpected for a static callback, and like
191+
// transactions, standalone spans don't get them.
192+
const commonAttributes = commonSpanAttributes(serializedSegmentSpan, client, finalScopeData, false);
193+
Object.entries(commonAttributes).forEach(([key, value]) => {
194+
if (value != null && !(key in spanJSON.data)) {
195+
spanJSON.data[key] = value as SpanAttributeValue;
196+
}
152197
});
198+
199+
const processedSpan = beforeSendSpan(spanJSON) || (showSpanDropWarning(), spanJSON);
200+
201+
return spanJsonToSerializedStreamedSpan(processedSpan);
153202
}
154203

155204
/**

0 commit comments

Comments
 (0)