Skip to content

fix(trace): Prefer standalone span indicators over pageload span indicators#110297

Open
nsdeschenes wants to merge 3 commits intomasterfrom
nd/EXP-824/fix-standalone-lcp-trend-line
Open

fix(trace): Prefer standalone span indicators over pageload span indicators#110297
nsdeschenes wants to merge 3 commits intomasterfrom
nd/EXP-824/fix-standalone-lcp-trend-line

Conversation

@nsdeschenes
Copy link
Contributor

@nsdeschenes nsdeschenes commented Mar 10, 2026

Fix trend line placement for standalone LCP spans in the trace waterfall.

When both a pageload span and a standalone LCP span are present, the trace
waterfall should prefer the standalone span's indicator instead of leaving the
pageload-derived indicator in place.

This branch now does two things. First, standalone span measurements replace an
existing indicator of the same type so the more specific LCP span wins.
Second, the indicator timestamp still applies the measurement offset from the
provided origin rather than assuming standalone spans always occur exactly at
span start. That matters for both the node-start path and the
performance.timeOrigin path used by EAP trace data.

The updated tests cover standalone indicator priority and the trace-origin case
so we don't regress the offset calculation again.

Refs EXP-824

…cators

When both a pageload span and a standalone LCP span exist in a trace,
the indicator was incorrectly placed using the pageload span's timing.
Standalone spans (ui.webvital.*) have precise timing at their
start_timestamp, so their indicators should take priority over
indicators from pageload spans that use start_timestamp + measurement
offset.

Refs EXP-824
Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>

Made-with: Cursor
Verify that when a pageload span and a standalone ui.webvital.lcp span
both have LCP measurements, only the standalone span's indicator is
used with its precise start_timestamp.

Refs EXP-824
Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>

Made-with: Cursor
@linear-code
Copy link

linear-code bot commented Mar 10, 2026

@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Mar 10, 2026
@nsdeschenes
Copy link
Contributor Author

@sentry review

@nsdeschenes
Copy link
Contributor Author

@cursor review

Copy link
Contributor

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Standalone LCP spans should still apply their measurement offset even when
the trace waterfall uses the trace time origin as the timestamp base. This
updates the indicator calculation and adds coverage for both span-start and
trace-origin paths.

Refs EXP-824
Co-Authored-By: GPT-5.4 <noreply@openai.com>

Made-with: Cursor
@nsdeschenes nsdeschenes marked this pull request as ready for review March 10, 2026 13:54
@nsdeschenes nsdeschenes requested a review from a team as a code owner March 10, 2026 13:54
Copy link
Member

@Lms24 Lms24 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing! I have to admit I'm a bit surprised that this happens because the SDK should only send either an LCP measurement on the pageload span, or a standalone LCP span. Never both. Same for CLS. Do you have a link to a trace where both are sent? This sounds like an SDK bug we should fix.

Comment on lines +836 to +837
children: [
makeEAPSpan({
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

standalone LCP spans aren't children of the pageload span. They're siblings. Not sure if we can model this in the test. Just wanted to call it out

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants