From 33fbefced15b1ca7665b88f16a8f9932150d883d Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Tue, 30 Jul 2024 13:44:09 -0400 Subject: [PATCH 1/9] fix(tracing): Fix flakey web vitals LCP test This has been super flakey for me... its possible that we were clicking button before image has loaded so that it thinks the LCP is the button. Closes #13115 --- .../suites/tracing/metrics/web-vitals-lcp/test.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-lcp/test.ts b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-lcp/test.ts index 2cfcbe58806e..9252a639b07a 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-lcp/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-lcp/test.ts @@ -9,7 +9,7 @@ sentryTest('should capture a LCP vital with element details.', async ({ browserN if (shouldSkipTracingTest() || browserName !== 'chromium') { sentryTest.skip(); } - + const imageSrc = 'https://example.com/path/to/image.png'; page.route('**', route => route.continue()); page.route('**/path/to/image.png', async (route: Route) => { return route.fulfill({ path: `${__dirname}/assets/sentry-logo-600x179.png` }); @@ -19,6 +19,7 @@ sentryTest('should capture a LCP vital with element details.', async ({ browserN const [eventData] = await Promise.all([ getFirstSentryEnvelopeRequest(page), page.goto(url), + page.waitForResponse(imageSrc), page.locator('button').click(), ]); @@ -27,5 +28,5 @@ sentryTest('should capture a LCP vital with element details.', async ({ browserN expect(eventData.contexts?.trace?.data?.['lcp.element']).toBe('body > img'); expect(eventData.contexts?.trace?.data?.['lcp.size']).toBe(107400); - expect(eventData.contexts?.trace?.data?.['lcp.url']).toBe('https://example.com/path/to/image.png'); + expect(eventData.contexts?.trace?.data?.['lcp.url']).toBe(imageSrc); }); From 1761b0d6a6cb6b894d2b6a3fe0fba8f214425f15 Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Tue, 30 Jul 2024 14:06:36 -0400 Subject: [PATCH 2/9] oops, move click to after image loads --- .../suites/tracing/metrics/web-vitals-lcp/test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-lcp/test.ts b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-lcp/test.ts index 9252a639b07a..cf089a295941 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-lcp/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-lcp/test.ts @@ -20,9 +20,10 @@ sentryTest('should capture a LCP vital with element details.', async ({ browserN getFirstSentryEnvelopeRequest(page), page.goto(url), page.waitForResponse(imageSrc), - page.locator('button').click(), ]); + await page.locator('button').click(); + expect(eventData.measurements).toBeDefined(); expect(eventData.measurements?.lcp?.value).toBeDefined(); From aa2e45fcd27ed84ac63104b82286ec7f278682d8 Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Tue, 30 Jul 2024 14:09:51 -0400 Subject: [PATCH 3/9] add comment --- .../suites/tracing/metrics/web-vitals-lcp/test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-lcp/test.ts b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-lcp/test.ts index cf089a295941..93a42fd1dfe1 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-lcp/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-lcp/test.ts @@ -22,6 +22,7 @@ sentryTest('should capture a LCP vital with element details.', async ({ browserN page.waitForResponse(imageSrc), ]); + // Clicking the button before image loads will result in the button being the LCP await page.locator('button').click(); expect(eventData.measurements).toBeDefined(); From 8892f89b28f5a9788a501ab832055a5f78dd2e3d Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Tue, 30 Jul 2024 14:52:37 -0400 Subject: [PATCH 4/9] try waiting for image response finish --- .../suites/tracing/metrics/web-vitals-lcp/test.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-lcp/test.ts b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-lcp/test.ts index 93a42fd1dfe1..99403ea7addc 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-lcp/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-lcp/test.ts @@ -10,18 +10,21 @@ sentryTest('should capture a LCP vital with element details.', async ({ browserN sentryTest.skip(); } const imageSrc = 'https://example.com/path/to/image.png'; + const imageResponsePromise = page.waitForResponse(imageSrc); page.route('**', route => route.continue()); page.route('**/path/to/image.png', async (route: Route) => { return route.fulfill({ path: `${__dirname}/assets/sentry-logo-600x179.png` }); }); const url = await getLocalTestPath({ testDir: __dirname }); - const [eventData] = await Promise.all([ + const [eventData, imageResponse] = await Promise.all([ getFirstSentryEnvelopeRequest(page), page.goto(url), - page.waitForResponse(imageSrc), + imageResponsePromise, ]); + await imageResponse?.finished(); + // Clicking the button before image loads will result in the button being the LCP await page.locator('button').click(); From 216437e408c6305d2ed656a92dd735976f5e7274 Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Tue, 30 Jul 2024 15:50:44 -0400 Subject: [PATCH 5/9] try #6 --- .../suites/tracing/metrics/web-vitals-lcp/test.ts | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-lcp/test.ts b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-lcp/test.ts index 99403ea7addc..ef8126d93efa 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-lcp/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-lcp/test.ts @@ -9,23 +9,22 @@ sentryTest('should capture a LCP vital with element details.', async ({ browserN if (shouldSkipTracingTest() || browserName !== 'chromium') { sentryTest.skip(); } - const imageSrc = 'https://example.com/path/to/image.png'; - const imageResponsePromise = page.waitForResponse(imageSrc); page.route('**', route => route.continue()); page.route('**/path/to/image.png', async (route: Route) => { return route.fulfill({ path: `${__dirname}/assets/sentry-logo-600x179.png` }); }); const url = await getLocalTestPath({ testDir: __dirname }); - const [eventData, imageResponse] = await Promise.all([ + const [eventData] = await Promise.all([ getFirstSentryEnvelopeRequest(page), page.goto(url), - imageResponsePromise, + // Clicking the button before image loads will result in the button being the LCP + page.waitForFunction(() => { + const images = Array.from(document.querySelectorAll('img')); + return images.every(img => img.complete); + }), ]); - await imageResponse?.finished(); - - // Clicking the button before image loads will result in the button being the LCP await page.locator('button').click(); expect(eventData.measurements).toBeDefined(); @@ -33,5 +32,5 @@ sentryTest('should capture a LCP vital with element details.', async ({ browserN expect(eventData.contexts?.trace?.data?.['lcp.element']).toBe('body > img'); expect(eventData.contexts?.trace?.data?.['lcp.size']).toBe(107400); - expect(eventData.contexts?.trace?.data?.['lcp.url']).toBe(imageSrc); + expect(eventData.contexts?.trace?.data?.['lcp.url']).toBe('https://example.com/path/to/image.png'); }); From 563a89bc51b85bab838eefc5b34a6669fb85cd58 Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Wed, 31 Jul 2024 11:13:53 -0400 Subject: [PATCH 6/9] debug --- .../suites/tracing/metrics/web-vitals-lcp/test.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-lcp/test.ts b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-lcp/test.ts index ef8126d93efa..1021235a8d45 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-lcp/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-lcp/test.ts @@ -25,6 +25,8 @@ sentryTest('should capture a LCP vital with element details.', async ({ browserN }), ]); + console.log(eventData.contexts?.trace); + expect(eventData.contexts?.trace?.data?.['lcp.element']).toBe('body > img'); await page.locator('button').click(); expect(eventData.measurements).toBeDefined(); From 6406110749d194d47ee6b8a96514ecea758ee630 Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Wed, 31 Jul 2024 11:39:38 -0400 Subject: [PATCH 7/9] debug in package --- .../tracing/metrics/web-vitals-lcp/test.ts | 19 ++++++++----------- .../src/metrics/web-vitals/getLCP.ts | 1 + 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-lcp/test.ts b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-lcp/test.ts index 1021235a8d45..e0a6c2387f74 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-lcp/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-lcp/test.ts @@ -15,18 +15,15 @@ sentryTest('should capture a LCP vital with element details.', async ({ browserN }); const url = await getLocalTestPath({ testDir: __dirname }); - const [eventData] = await Promise.all([ - getFirstSentryEnvelopeRequest(page), - page.goto(url), - // Clicking the button before image loads will result in the button being the LCP - page.waitForFunction(() => { - const images = Array.from(document.querySelectorAll('img')); - return images.every(img => img.complete); - }), - ]); + const [eventData] = await Promise.all([getFirstSentryEnvelopeRequest(page), page.goto(url)]); + + // Clicking the button before image loads will result in the button being the LCP + await page.waitForFunction(() => { + const images = Array.from(document.querySelectorAll('img')); + return images.every(img => img.complete); + }), + expect(eventData.contexts?.trace?.data?.['lcp.element']).toBe('body > img'); - console.log(eventData.contexts?.trace); - expect(eventData.contexts?.trace?.data?.['lcp.element']).toBe('body > img'); await page.locator('button').click(); expect(eventData.measurements).toBeDefined(); diff --git a/packages/browser-utils/src/metrics/web-vitals/getLCP.ts b/packages/browser-utils/src/metrics/web-vitals/getLCP.ts index b50358c98d61..6597c294c3eb 100644 --- a/packages/browser-utils/src/metrics/web-vitals/getLCP.ts +++ b/packages/browser-utils/src/metrics/web-vitals/getLCP.ts @@ -60,6 +60,7 @@ export const onLCP = (onReport: LCPReportCallback, opts: ReportOpts = {}) => { // clamped at 0. metric.value = Math.max(lastEntry.startTime - getActivationStart(), 0); metric.entries = [lastEntry]; + console.log(lastEntry); report(); } } From 4ff87c3d3ba336c1afe71557f554bed900e2c981 Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Wed, 31 Jul 2024 11:56:23 -0400 Subject: [PATCH 8/9] fwd console log --- .../suites/tracing/metrics/web-vitals-lcp/test.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-lcp/test.ts b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-lcp/test.ts index e0a6c2387f74..585dc4e66d6d 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-lcp/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-lcp/test.ts @@ -14,6 +14,8 @@ sentryTest('should capture a LCP vital with element details.', async ({ browserN return route.fulfill({ path: `${__dirname}/assets/sentry-logo-600x179.png` }); }); + page.on('console', msg => console.log(msg.text())); + const url = await getLocalTestPath({ testDir: __dirname }); const [eventData] = await Promise.all([getFirstSentryEnvelopeRequest(page), page.goto(url)]); From 3acfb6dd506281bf0ae1c0808cd93bcf9352f49a Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Wed, 31 Jul 2024 18:18:54 -0400 Subject: [PATCH 9/9] ok one last try --- .../suites/tracing/metrics/web-vitals-lcp/test.ts | 14 +++++++------- .../browser-utils/src/metrics/web-vitals/getLCP.ts | 1 - 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-lcp/test.ts b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-lcp/test.ts index 585dc4e66d6d..5ba55ca224a2 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-lcp/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-lcp/test.ts @@ -14,19 +14,19 @@ sentryTest('should capture a LCP vital with element details.', async ({ browserN return route.fulfill({ path: `${__dirname}/assets/sentry-logo-600x179.png` }); }); - page.on('console', msg => console.log(msg.text())); - const url = await getLocalTestPath({ testDir: __dirname }); - const [eventData] = await Promise.all([getFirstSentryEnvelopeRequest(page), page.goto(url)]); + const [eventData] = await Promise.all([ + getFirstSentryEnvelopeRequest(page), + page.goto(url), + page.waitForLoadState('networkidle'), + ]); // Clicking the button before image loads will result in the button being the LCP await page.waitForFunction(() => { const images = Array.from(document.querySelectorAll('img')); - return images.every(img => img.complete); + return images.every(img => img.naturalWidth > 0); }), - expect(eventData.contexts?.trace?.data?.['lcp.element']).toBe('body > img'); - - await page.locator('button').click(); + await page.locator('button').click(); expect(eventData.measurements).toBeDefined(); expect(eventData.measurements?.lcp?.value).toBeDefined(); diff --git a/packages/browser-utils/src/metrics/web-vitals/getLCP.ts b/packages/browser-utils/src/metrics/web-vitals/getLCP.ts index 6597c294c3eb..b50358c98d61 100644 --- a/packages/browser-utils/src/metrics/web-vitals/getLCP.ts +++ b/packages/browser-utils/src/metrics/web-vitals/getLCP.ts @@ -60,7 +60,6 @@ export const onLCP = (onReport: LCPReportCallback, opts: ReportOpts = {}) => { // clamped at 0. metric.value = Math.max(lastEntry.startTime - getActivationStart(), 0); metric.entries = [lastEntry]; - console.log(lastEntry); report(); } }