feat(browser): Emit web vitals as streamed spans#19827
feat(browser): Emit web vitals as streamed spans#19827logaretm wants to merge 13 commits intolms/feat-span-firstfrom
Conversation
size-limit report 📦
|
8bf8eaf to
c966a4a
Compare
1a5cfb3 to
28c0d45
Compare
Semver Impact of This PR🟡 Minor (new features) 📋 Changelog PreviewThis is how your changes will appear in the changelog. New Features ✨Deps
Other
Bug Fixes 🐛Cloudflare
Core
Deps
Other
Internal Changes 🔧Deps Dev
Other
🤖 This preview updates automatically when you update the PR. |
c966a4a to
5963170
Compare
28c0d45 to
af2969e
Compare
There was a problem hiding this comment.
Pull request overview
Adds support for emitting certain Web Vitals as streamed (v2 pipeline) spans when traceLifecycle: 'stream' / span streaming is enabled, while keeping existing pageload measurements in place.
Changes:
- Gate standalone CLS/LCP spans off when span streaming is enabled, and wire up streamed LCP/CLS/INP emission from the browser tracing integration.
- Introduce
webVitalSpans.tshelpers + unit tests for emitting streamed Web Vital spans. - Add Playwright integration tests for streamed LCP and CLS spans; export
INP_ENTRY_MAP; add (currently-unused) FCP metric instrumentation.
Reviewed changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/browser/src/tracing/browserTracingIntegration.ts | Enables streamed Web Vital span tracking when span streaming is enabled; disables standalone CLS/LCP in that mode |
| packages/browser-utils/src/metrics/webVitalSpans.ts | Implements streamed span emitters for LCP/CLS/INP |
| packages/browser-utils/test/metrics/webVitalSpans.test.ts | Unit tests for streamed web vital span emission helpers |
| packages/browser-utils/src/metrics/instrument.ts | Adds FCP metric instrumentation plumbing (fcp observer + handler) |
| packages/browser-utils/src/metrics/inp.ts | Exports INP_ENTRY_MAP for reuse by streamed INP span logic |
| packages/browser-utils/src/index.ts | Re-exports streamed web vital span trackers from browser-utils |
| dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-lcp-streamed-spans/test.ts | Playwright test validating streamed LCP span + attributes |
| dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-lcp-streamed-spans/template.html | Test page for streamed LCP |
| dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-lcp-streamed-spans/init.js | Initializes SDK with span streaming enabled for LCP test |
| dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-lcp-streamed-spans/assets/sentry-logo-600x179.png | Asset used to trigger LCP |
| dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-cls-streamed-spans/test.ts | Playwright test validating streamed CLS span + attributes |
| dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-cls-streamed-spans/template.html | Test page for streamed CLS |
| dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-cls-streamed-spans/subject.js | Simulates CLS for the CLS streamed span test |
| dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-cls-streamed-spans/init.js | Initializes SDK with span streaming enabled for CLS test |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
d764f2e to
b80ddd4
Compare
a74fec7 to
7206304
Compare
| const clsValue = clsSpan.attributes?.['browser.web_vital.cls.value']?.value as number; | ||
| expect(clsValue).toBeGreaterThan(0.05); | ||
| expect(clsValue).toBeLessThan(0.15); | ||
| }); |
There was a problem hiding this comment.
Missing INP streamed span E2E test coverage
Low Severity
This feat PR adds three new streamed span code paths (LCP, CLS, INP), but only LCP and CLS have E2E/integration tests. The INP streaming path in trackInpAsSpan / _sendInpSpan — which has its own distinct logic (no listenForWebVitalReportEvents, no pageloadSpanId, no registerInpInteractionListener fallback for element names) — only has unit test coverage. Adding an E2E test for the INP streaming path would help catch integration issues between addInpInstrumentationHandler and the _emitWebVitalSpan pipeline.
Additional Locations (1)
Triggered by project rule: PR Review Guidelines for Cursor Bot
879b35b to
86d1929
Compare
…NTRY_MAP Add `addFcpInstrumentationHandler` using the existing `onFCP` web-vitals library integration, following the same pattern as the other metric handlers. Export `INP_ENTRY_MAP` from inp.ts for reuse in the new web vital spans module. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…is enabled Add non-standalone web vital spans that flow through the v2 span streaming pipeline (afterSpanEnd -> captureSpan -> SpanBuffer). Each web vital gets `browser.web_vital.<metric>.value` attributes and span events for measurement extraction. Spans have meaningful durations showing time from navigation start to the web vital event (except CLS which is a score, not a duration). New tracking functions: trackLcpAsSpan, trackClsAsSpan, trackInpAsSpan, trackTtfbAsSpan, trackFcpAsSpan, trackFpAsSpan — wired up in browserTracingIntegration.setup() when hasSpanStreamingEnabled(client). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add Playwright integration tests verifying CLS, LCP, FCP, FP, and TTFB are emitted as streamed spans with correct attributes, value attributes, and meaningful durations when span streaming is enabled. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…dalone spans when streaming TTFB, FCP, and FP should remain as attributes on the pageload span rather than separate streamed spans. Also ensures standalone CLS/LCP spans are disabled when span streaming is enabled to prevent duplicate spans. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…an path The standalone INP handler filters out unrealistically long INP values (>60s) but the streamed span path was missing this sanity check. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Gate standalone INP (`startTrackingINP`) behind `!spanStreamingEnabled` and gate streamed INP (`trackInpAsSpan`) behind `enableInp` so both paths respect the user's preference and don't produce duplicate data. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Remove `addFcpInstrumentationHandler`, `instrumentFcp`, and `_previousFcp` which were added to support FCP streamed spans but are no longer called after FCP spans were removed from the implementation. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…_sendInpSpan Use `|| 0` fallback instead of `as number` cast, consistent with the LCP and CLS span handlers that already guard against undefined. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…cpSpan Avoid calling browserPerformanceTimeOrigin() twice by caching the result in a local variable. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…nabled The streamed INP path does not use INTERACTIONS_SPAN_MAP or ELEMENT_NAME_TIMESTAMP_MAP, so registering the listeners is unnecessary overhead. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
When span streaming is enabled, CLS and LCP are emitted as streamed spans. Previously they were also recorded as measurements on the pageload span because the flags only checked enableStandaloneClsSpans and enableStandaloneLcpSpans, which default to undefined. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… handlers Export the constant from inp.ts and import it in webVitalSpans.ts to avoid the two definitions drifting apart. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…Setup spanStreamingEnabled was declared in setup() but referenced in afterAllSetup(), a separate scope. Replace with inline hasSpanStreamingEnabled(client) call. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
86d1929 to
4bf129b
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Redundant CLS/LCP tracking when streaming is enabled
- Added recordClsOnPageloadSpan and recordLcpOnPageloadSpan parameters to startTrackingWebVitals to prevent redundant handler registration when span streaming is enabled, eliminating the double-handler issue where one handler did throwaway work.
Or push these changes by commenting:
@cursor push d3ccbaa211
Preview (d3ccbaa211)
diff --git a/packages/browser-utils/src/metrics/browserMetrics.ts b/packages/browser-utils/src/metrics/browserMetrics.ts
--- a/packages/browser-utils/src/metrics/browserMetrics.ts
+++ b/packages/browser-utils/src/metrics/browserMetrics.ts
@@ -77,6 +77,8 @@
interface StartTrackingWebVitalsOptions {
recordClsStandaloneSpans: boolean;
recordLcpStandaloneSpans: boolean;
+ recordClsOnPageloadSpan?: boolean;
+ recordLcpOnPageloadSpan?: boolean;
client: Client;
}
@@ -89,6 +91,8 @@
export function startTrackingWebVitals({
recordClsStandaloneSpans,
recordLcpStandaloneSpans,
+ recordClsOnPageloadSpan = true,
+ recordLcpOnPageloadSpan = true,
client,
}: StartTrackingWebVitalsOptions): () => void {
const performance = getBrowserPerformanceAPI();
@@ -97,10 +101,22 @@
if (performance.mark) {
WINDOW.performance.mark('sentry-tracing-init');
}
- const lcpCleanupCallback = recordLcpStandaloneSpans ? trackLcpAsStandaloneSpan(client) : _trackLCP();
+ let lcpCleanupCallback: (() => void) | undefined;
+ if (recordLcpStandaloneSpans) {
+ trackLcpAsStandaloneSpan(client);
+ } else if (recordLcpOnPageloadSpan) {
+ lcpCleanupCallback = _trackLCP();
+ }
+
const ttfbCleanupCallback = _trackTtfb();
- const clsCleanupCallback = recordClsStandaloneSpans ? trackClsAsStandaloneSpan(client) : _trackCLS();
+ let clsCleanupCallback: (() => void) | undefined;
+ if (recordClsStandaloneSpans) {
+ trackClsAsStandaloneSpan(client);
+ } else if (recordClsOnPageloadSpan) {
+ clsCleanupCallback = _trackCLS();
+ }
+
return (): void => {
lcpCleanupCallback?.();
ttfbCleanupCallback();
diff --git a/packages/browser/src/tracing/browserTracingIntegration.ts b/packages/browser/src/tracing/browserTracingIntegration.ts
--- a/packages/browser/src/tracing/browserTracingIntegration.ts
+++ b/packages/browser/src/tracing/browserTracingIntegration.ts
@@ -519,6 +519,8 @@
_collectWebVitals = startTrackingWebVitals({
recordClsStandaloneSpans: !spanStreamingEnabled && (enableStandaloneClsSpans || false),
recordLcpStandaloneSpans: !spanStreamingEnabled && (enableStandaloneLcpSpans || false),
+ recordClsOnPageloadSpan: !spanStreamingEnabled,
+ recordLcpOnPageloadSpan: !spanStreamingEnabled,
client,
});This Bugbot Autofix run was free. To enable autofix for future PRs, go to the Cursor dashboard.
| recordClsStandaloneSpans: enableStandaloneClsSpans || false, | ||
| recordLcpStandaloneSpans: enableStandaloneLcpSpans || false, | ||
| recordClsStandaloneSpans: !spanStreamingEnabled && (enableStandaloneClsSpans || false), | ||
| recordLcpStandaloneSpans: !spanStreamingEnabled && (enableStandaloneLcpSpans || false), |
There was a problem hiding this comment.
Redundant CLS/LCP tracking when streaming is enabled
Low Severity
When span streaming is enabled, startTrackingWebVitals is called with both recordClsStandaloneSpans: false and recordLcpStandaloneSpans: false, causing _trackCLS() and _trackLCP() to register handlers that accumulate values in _measurements. These measurements are then immediately deleted in addPerformanceEntries because recordClsOnPageloadSpan and recordLcpOnPageloadSpan are also false. Meanwhile, trackClsAsSpan() and trackLcpAsSpan() register their own handlers doing the real work. This results in two handlers firing on every CLS/LCP update — one doing entirely throwaway work. Contrast with the standalone path, where trackClsAsStandaloneSpan replaces _trackCLS() entirely. A similar replacement pattern for streaming would avoid this redundancy.



Summary
Closes #17931
When span streaming is enabled (
traceLifecycle: 'stream'), emit web vital values as non-standalone spans that flow through the v2 pipeline (afterSpanEnd→captureSpan()→SpanBuffer).Only LCP, CLS, and INP are emitted as streamed spans — TTFB, FCP, and FP remain as attributes on the pageload span. When span streaming is enabled, standalone v1 CLS/LCP spans are automatically disabled to prevent duplicates.
Each web vital span carries
browser.web_vital.*attributes per sentry-conventions PRs 229, 233-235:browser.web_vital.lcp.{value,element,id,url,size,load_time,render_time}browser.web_vital.cls.{value,source.<N>}browser.web_vital.inp.value(with MAX_PLAUSIBLE_INP_DURATION sanity check)Spans have meaningful durations (navigation start → event time) instead of being point-in-time, except CLS which is a score.
Changes
hasSpanStreamingEnabled(client)is true!spanStreamingEnabled && enableStandaloneClsSpans)MAX_PLAUSIBLE_INP_DURATION(60s) sanity check to streamed INP path, matching the existing standalone handler🤖 Generated with Claude Code