Skip to content

Commit 8cafc05

Browse files
committed
fix(browser): Ensure Standalone CLS span timestamps are correct
1 parent 1285e4b commit 8cafc05

File tree

3 files changed

+46
-4
lines changed
  • dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-cls-standalone-spans
  • packages/browser-utils/src/metrics

3 files changed

+46
-4
lines changed

dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-cls-standalone-spans/test.ts

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -453,3 +453,44 @@ sentryTest("doesn't send further CLS after the first page hide", async ({ getLoc
453453
// a timeout or something similar.
454454
await navigationTxnPromise;
455455
});
456+
457+
sentryTest('CLS span timestamps are set correctly', async ({ getLocalTestPath, page }) => {
458+
const url = await getLocalTestPath({ testDir: __dirname });
459+
460+
const eventData = await getFirstSentryEnvelopeRequest<SentryEvent>(page, url);
461+
462+
expect(eventData.type).toBe('transaction');
463+
expect(eventData.contexts?.trace?.op).toBe('pageload');
464+
expect(eventData.timestamp).toBeDefined();
465+
466+
const pageloadEndTimestamp = eventData.timestamp!;
467+
468+
const spanEnvelopePromise = getMultipleSentryEnvelopeRequests<SpanEnvelope>(
469+
page,
470+
1,
471+
{ envelopeType: 'span' },
472+
properFullEnvelopeRequestParser,
473+
);
474+
475+
await triggerAndWaitForLayoutShift(page);
476+
477+
await hidePage(page);
478+
479+
const spanEnvelope = (await spanEnvelopePromise)[0];
480+
const spanEnvelopeItem = spanEnvelope[1][0][1];
481+
482+
expect(spanEnvelopeItem.start_timestamp).toBeDefined();
483+
expect(spanEnvelopeItem.timestamp).toBeDefined();
484+
485+
const clsSpanStartTimestamp = spanEnvelopeItem.start_timestamp!;
486+
const clsSpanEndTimestamp = spanEnvelopeItem.timestamp!;
487+
488+
// CLS performance entries have no duration ==> start and end timestamp should be the same
489+
expect(clsSpanStartTimestamp).toEqual(clsSpanEndTimestamp);
490+
491+
// We don't really care that they are very close together but rather about the order of magnitude
492+
// Previously, we had a bug where the timestamps would be significantly off (by multiple hours)
493+
// so we only ensure that this bug is fixed. 60 seconds should be more than enough.
494+
expect(clsSpanStartTimestamp - pageloadEndTimestamp).toBeLessThan(60);
495+
expect(clsSpanStartTimestamp).toBeGreaterThan(pageloadEndTimestamp);
496+
});

packages/browser-utils/src/metrics/cls.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -84,8 +84,7 @@ export function trackClsAsStandaloneSpan(): void {
8484
function sendStandaloneClsSpan(clsValue: number, entry: LayoutShift | undefined, pageloadSpanId: string) {
8585
DEBUG_BUILD && logger.log(`Sending CLS span (${clsValue})`);
8686

87-
const startTime = msToSec(browserPerformanceTimeOrigin as number) + (entry?.startTime || 0);
88-
const duration = msToSec(entry?.duration || 0);
87+
const startTime = msToSec((browserPerformanceTimeOrigin || 0) + (entry?.startTime || 0));
8988
const routeName = getCurrentScope().getScopeData().transactionName;
9089

9190
const name = entry ? htmlTreeAsString(entry.sources[0]?.node) : 'Layout shift';
@@ -110,7 +109,9 @@ function sendStandaloneClsSpan(clsValue: number, entry: LayoutShift | undefined,
110109
[SEMANTIC_ATTRIBUTE_SENTRY_MEASUREMENT_VALUE]: clsValue,
111110
});
112111

113-
span?.end(startTime + duration);
112+
// LayoutShift performance entries always have a duration of 0, so we don't need to add `enntry.duration` here
113+
// see: https://developer.mozilla.org/en-US/docs/Web/API/PerformanceEntry/duration
114+
span?.end(startTime);
114115
}
115116

116117
function supportsLayoutShift(): boolean {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ export function startStandaloneWebVitalSpan(options: StandaloneWebVitalSpanOptio
8686
const user = scope.getUser();
8787
const userDisplay = user !== undefined ? user.email || user.id || user.ip_address : undefined;
8888

89-
let profileId: string | undefined = undefined;
89+
let profileId: string | undefined;
9090
try {
9191
// @ts-expect-error skip optional chaining to save bundle size with try catch
9292
profileId = scope.getScopeData().contexts.profile.profile_id;

0 commit comments

Comments
 (0)