-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
feat(browser): Run static beforeSendSpan for INP spans #22877
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
3a52992
d9d9eee
f3d1a02
2293bfc
ded6329
758dbb4
fc7c722
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| import * as Sentry from '@sentry/browser'; | ||
|
|
||
| window.Sentry = Sentry; | ||
|
|
||
| Sentry.init({ | ||
| traceLifecycle: 'static', | ||
| dsn: 'https://public@dsn.ingest.sentry.io/1337', | ||
| integrations: [ | ||
| Sentry.browserTracingIntegration({ | ||
| idleTimeout: 4000, | ||
| enableLongTask: false, | ||
| enableInp: true, | ||
| instrumentPageLoad: false, | ||
| instrumentNavigation: false, | ||
| }), | ||
| ], | ||
| tracesSampleRate: 1, | ||
| // A plain (non-streamed) `beforeSendSpan` operates on the v1 `SpanJSON`. INP is sent as a v2 span, | ||
| // so this verifies the static callback still runs and its changes are carried into the v2 span. | ||
| beforeSendSpan: span => { | ||
| if (span.op === 'ui.interaction.click') { | ||
| span.description = 'scrubbed'; | ||
| span.data['custom.attribute'] = 'from-before-send-span'; | ||
| } | ||
|
|
||
| return span; | ||
| }, | ||
| debug: true, | ||
| }); | ||
|
|
||
| const client = Sentry.getClient(); | ||
|
|
||
| // Force page load transaction name to a testable value | ||
| Sentry.startBrowserTracingPageLoadSpan(client, { | ||
| name: 'test-url', | ||
| attributes: { | ||
| [Sentry.SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', | ||
| }, | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| const blockUI = | ||
| (delay = 70) => | ||
| e => { | ||
| const startTime = Date.now(); | ||
|
|
||
| function getElasped() { | ||
| const time = Date.now(); | ||
| return time - startTime; | ||
| } | ||
|
|
||
| while (getElasped() < delay) { | ||
| // | ||
| } | ||
|
|
||
| e.target.classList.add('clicked'); | ||
| }; | ||
|
|
||
| document.querySelector('[data-test-id=not-so-slow-button]').addEventListener('click', blockUI(300)); | ||
| document.querySelector('[data-test-id=slow-button]').addEventListener('click', blockUI(450)); | ||
| document.querySelector('[data-test-id=normal-button]').addEventListener('click', blockUI()); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| <!doctype html> | ||
| <html> | ||
| <head> | ||
| <meta charset="utf-8" /> | ||
| </head> | ||
| <body> | ||
| <div>Rendered Before Long Task</div> | ||
| <button data-test-id="slow-button" data-sentry-element="SlowButton">Slow</button> | ||
| <button data-test-id="not-so-slow-button" data-sentry-element="NotSoSlowButton">Not so slow</button> | ||
| <button data-test-id="normal-button" data-sentry-element="NormalButton">Click Me</button> | ||
| </body> | ||
| </html> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| import { expect } from '@playwright/test'; | ||
| import { sentryTest } from '../../../../utils/fixtures'; | ||
| import { hidePage, shouldSkipTracingTest } from '../../../../utils/helpers'; | ||
| import { getSpanOp, getSpansFromEnvelope, waitForStreamedSpanEnvelope } from '../../../../utils/spanUtils'; | ||
|
|
||
| // This app does not enable span streaming (`traceLifecycle: 'static'`) and defines a plain, non-streamed | ||
| // `beforeSendSpan` callback (operating on the v1 `SpanJSON`). INP is still emitted as a v2 span, so this | ||
| // verifies the static callback runs for INP and its modifications are carried into the v2 span. | ||
|
|
||
| sentryTest('runs a non-streamed `beforeSendSpan` for the INP span', async ({ browserName, getLocalTestUrl, page }) => { | ||
| const supportedBrowsers = ['chromium']; | ||
|
|
||
| if (shouldSkipTracingTest() || !supportedBrowsers.includes(browserName)) { | ||
| sentryTest.skip(); | ||
| } | ||
|
|
||
| const url = await getLocalTestUrl({ testDir: __dirname }); | ||
|
|
||
| const spanEnvelopePromise = waitForStreamedSpanEnvelope( | ||
| page, | ||
| env => !!getSpansFromEnvelope(env).find(s => getSpanOp(s) === 'ui.interaction.click'), | ||
| ); | ||
|
|
||
| await page.goto(url); | ||
|
|
||
| await page.locator('[data-test-id=normal-button]').click(); | ||
| await page.locator('.clicked[data-test-id=normal-button]').isVisible(); | ||
|
|
||
| await page.waitForTimeout(500); | ||
|
|
||
| // Page hide to trigger INP | ||
| await hidePage(page); | ||
|
|
||
| const spanEnvelope = await spanEnvelopePromise; | ||
| const inpSpan = getSpansFromEnvelope(spanEnvelope).find(s => getSpanOp(s) === 'ui.interaction.click')!; | ||
|
|
||
| // The callback rewrote the name and added a custom attribute. | ||
| expect(inpSpan.name).toBe('scrubbed'); | ||
| expect(inpSpan.attributes['custom.attribute']).toEqual({ value: 'from-before-send-span', type: 'string' }); | ||
|
|
||
| // The span is still a valid v2 INP span carrying its web vital value. | ||
| const inpValue = inpSpan.attributes['browser.web_vital.inp.value']?.value as number; | ||
| expect(inpValue).toBeGreaterThan(0); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,9 @@ | ||
| import type { Client, Span, SpanAttributes } from '@sentry/core'; | ||
| import type { Client, Integration, Span, SpanAttributes } from '@sentry/core'; | ||
| import { | ||
| browserPerformanceTimeOrigin, | ||
| debug, | ||
| getActiveSpan, | ||
| getClient, | ||
| getCurrentScope, | ||
| getRootSpan, | ||
| hasSpanStreamingEnabled, | ||
|
|
@@ -108,6 +109,15 @@ export function _emitWebVitalSpan(options: WebVitalSpanOptions): void { | |
| attributes[`browser.web_vital.${metricName}.report_event`] = reportEvent; | ||
| } | ||
|
|
||
| // A standalone span is sent as a plain v2 span without running the `processSpan` hooks (see | ||
| // `captureStandaloneSpanWithStaticCallback`), so Replay can't attach the replay id itself. Set it | ||
| // here, mirroring Replay's `processSpan`, so INP keeps its replay association like it did on v1. | ||
| // TODO(standalone): remove once the static (transaction) trace lifecycle is dropped and INP always | ||
| // streams, at which point Replay's `processSpan` runs and attaches the replay id. | ||
| if (standalone) { | ||
| Object.assign(attributes, getReplayAttributes()); | ||
| } | ||
|
|
||
| const span = startInactiveSpan({ | ||
| name, | ||
| attributes, | ||
|
|
@@ -122,6 +132,26 @@ export function _emitWebVitalSpan(options: WebVitalSpanOptions): void { | |
| } | ||
| } | ||
|
|
||
| interface ReplayIntegration extends Integration { | ||
| getReplayId: (onlyIfSampled?: boolean) => string | undefined; | ||
| getRecordingMode: () => 'session' | 'buffer' | undefined; | ||
| } | ||
|
|
||
| // TODO(standalone): remove once the static (transaction) trace lifecycle is dropped; Replay's | ||
| // `processSpan` then attaches the replay id to the streamed INP span instead. | ||
| function getReplayAttributes(): SpanAttributes { | ||
| const replay = getClient()?.getIntegrationByName<ReplayIntegration>('Replay'); | ||
| const replayId = replay?.getReplayId(true); | ||
| if (!replayId) { | ||
| return {}; | ||
| } | ||
|
|
||
| return { | ||
| 'sentry.replay_id': replayId, | ||
| 'sentry._internal.replay_is_buffering': replay!.getRecordingMode() === 'buffer' ? true : undefined, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Non-null assertion lacks safety commentLow Severity
Triggered by project rule: PR Review Guidelines for Cursor Bot Reviewed by Cursor Bugbot for commit fc7c722. Configure here. |
||
| }; | ||
| } | ||
|
|
||
| /** | ||
| * Tracks LCP as a streamed span. | ||
| */ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,16 +11,18 @@ import { | |
| SEMANTIC_ATTRIBUTE_USER_IP_ADDRESS, | ||
| SEMANTIC_ATTRIBUTE_USER_USERNAME, | ||
| } from '../../semanticAttributes'; | ||
| import type { SerializedStreamedSpan, Span, StreamedSpanJSON } from '../../types/span'; | ||
| import type { SerializedStreamedSpan, Span, SpanAttributeValue, SpanJSON, StreamedSpanJSON } from '../../types/span'; | ||
| import { getCombinedScopeData } from '../../utils/scopeData'; | ||
| import { | ||
| INTERNAL_getSegmentSpan, | ||
| showSpanDropWarning, | ||
| spanToJSON, | ||
| spanToStreamedSpanJSON, | ||
| streamedSpanJsonToSerializedSpan, | ||
| } from '../../utils/spanUtils'; | ||
| import { getCapturedScopesOnSpan } from '../utils'; | ||
| import { isStreamedBeforeSendSpanCallback } from './beforeSendSpan'; | ||
| import { spanJsonToSerializedStreamedSpan } from './spanJsonToStreamedSpan'; | ||
| import { scopeContextsToSpanAttributes } from './scopeContextAttributes'; | ||
| import { DEFAULT_ENVIRONMENT } from '../../constants'; | ||
| import { | ||
|
|
@@ -126,17 +128,18 @@ function applySdkMetadataToSegmentSpan(segmentSpanJSON: StreamedSpanJSON, client | |
| }); | ||
| } | ||
|
|
||
| function applyCommonSpanAttributes( | ||
| spanJSON: StreamedSpanJSON, | ||
| function commonSpanAttributes( | ||
| serializedSegmentSpan: StreamedSpanJSON, | ||
| client: Client, | ||
| scopeData: ScopeData, | ||
| ): void { | ||
| // TODO(standalone): remove this param (always include scope attributes) once the static (transaction) | ||
| // trace lifecycle is dropped and standalone spans no longer need to look transaction-shaped. | ||
| includeScopeAttributes = true, | ||
| ): RawAttributes<Record<string, unknown>> { | ||
| const sdk = client.getSdkMetadata(); | ||
| const { release, environment } = client.getOptions(); | ||
|
|
||
| // avoid overwriting any previously set attributes (from users or potentially our SDK instrumentation) | ||
| safeSetSpanJSONAttributes(spanJSON, { | ||
| return { | ||
| [SENTRY_TRACE_LIFECYCLE]: 'stream', | ||
| [SENTRY_SEGMENT_NAME]: serializedSegmentSpan.name, | ||
| [SENTRY_SEGMENT_ID]: serializedSegmentSpan.span_id, | ||
|
|
@@ -148,8 +151,54 @@ function applyCommonSpanAttributes( | |
| [SEMANTIC_ATTRIBUTE_USER_EMAIL]: scopeData.user?.email, | ||
| [SEMANTIC_ATTRIBUTE_USER_IP_ADDRESS]: scopeData.user?.ip_address, | ||
| [SEMANTIC_ATTRIBUTE_USER_USERNAME]: scopeData.user?.username, | ||
| ...scopeData.attributes, | ||
| ...(includeScopeAttributes ? scopeData.attributes : undefined), | ||
| }; | ||
| } | ||
|
|
||
| function applyCommonSpanAttributes( | ||
| spanJSON: StreamedSpanJSON, | ||
| serializedSegmentSpan: StreamedSpanJSON, | ||
| client: Client, | ||
| scopeData: ScopeData, | ||
| ): void { | ||
| // avoid overwriting any previously set attributes (from users or potentially our SDK instrumentation) | ||
| safeSetSpanJSONAttributes(spanJSON, commonSpanAttributes(serializedSegmentSpan, client, scopeData)); | ||
| } | ||
|
|
||
| /** | ||
| * Captures a standalone span whose `beforeSendSpan` callback expects the v1 {@link SpanJSON} format | ||
| * (i.e. the user opted out of span streaming). The span is serialized to v1, the common attributes are | ||
| * applied, the callback runs in its native format, and the result is converted forward to a serialized | ||
| * v2 span. This mirrors how gen_ai spans reach the v2 span path from a static transaction (a plain | ||
| * conversion, no `processSpan` hooks), so there is never a reverse v2 -> v1 conversion. | ||
| * | ||
| * TODO(standalone): remove once the static (transaction) trace lifecycle is dropped. | ||
| */ | ||
| export function captureStandaloneSpanWithStaticCallback( | ||
| span: Span, | ||
| client: Client, | ||
| beforeSendSpan: (span: SpanJSON) => SpanJSON, | ||
| ): SerializedStreamedSpan { | ||
| const spanJSON = spanToJSON(span); | ||
|
|
||
| const segmentSpan = INTERNAL_getSegmentSpan(span); | ||
| const serializedSegmentSpan = spanToStreamedSpanJSON(segmentSpan); | ||
|
|
||
| const { isolationScope: spanIsolationScope, scope: spanScope } = getCapturedScopesOnSpan(span); | ||
| const finalScopeData = getCombinedScopeData(spanIsolationScope, spanScope); | ||
|
|
||
| // Skip scope attributes: their `{ unit, value }` shape is unexpected for a static callback, and like | ||
| // transactions, standalone spans don't get them. | ||
| const commonAttributes = commonSpanAttributes(serializedSegmentSpan, client, finalScopeData, false); | ||
| Object.entries(commonAttributes).forEach(([key, value]) => { | ||
| if (value != null && !(key in spanJSON.data)) { | ||
| spanJSON.data[key] = value as SpanAttributeValue; | ||
| } | ||
| }); | ||
|
|
||
| const processedSpan = beforeSendSpan(spanJSON) || (showSpanDropWarning(), spanJSON); | ||
|
|
||
| return spanJsonToSerializedStreamedSpan(processedSpan); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Callback op changes not preservedMedium Severity
Reviewed by Cursor Bugbot for commit fc7c722. Configure here.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. shut up, we know |
||
| } | ||
|
|
||
| /** | ||
|
|
||


Uh oh!
There was an error while loading. Please reload this page.