From f563eae7e032f2b3d71147be922f95d98babdcb1 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Fri, 19 Aug 2022 16:01:41 -0400 Subject: [PATCH] ref(tracing): Remove mark measurements (#5605) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently the JS SDK sends extra measurements though, specifically `mark.lcp`, `mark.fid`, `mark.fp`, `mark.fcp`. This is done because these measurements are being used in the Sentry front-end UI to draw lines on the span waterfall component on the transaction details page. These measurements should be removed, as they count toward a user’s custom measurement quota - hence we would be reducing the amount of custom measurements they can send and use because the SDK is setting them automatically. In addition, there is no way these measurements are ever going to be indexed, so we can’t move them to be built-in. --- .../tracing/metrics/web-vitals-fid/test.ts | 1 - .../tracing/metrics/web-vitals-fp-fcp/test.ts | 4 ---- .../tracing/metrics/web-vitals-lcp/test.ts | 1 - packages/tracing/src/browser/metrics/index.ts | 17 ++++++++--------- 4 files changed, 8 insertions(+), 15 deletions(-) diff --git a/packages/integration-tests/suites/tracing/metrics/web-vitals-fid/test.ts b/packages/integration-tests/suites/tracing/metrics/web-vitals-fid/test.ts index 0eaa60a75031..de2d3d8395f9 100644 --- a/packages/integration-tests/suites/tracing/metrics/web-vitals-fid/test.ts +++ b/packages/integration-tests/suites/tracing/metrics/web-vitals-fid/test.ts @@ -20,7 +20,6 @@ sentryTest('should capture a FID vital.', async ({ browserName, getLocalTestPath expect(eventData.measurements).toBeDefined(); expect(eventData.measurements?.fid?.value).toBeDefined(); - expect(eventData.measurements?.['mark.fid']?.value).toBeDefined(); const fidSpan = eventData.spans?.filter(({ description }) => description === 'first input delay')[0]; diff --git a/packages/integration-tests/suites/tracing/metrics/web-vitals-fp-fcp/test.ts b/packages/integration-tests/suites/tracing/metrics/web-vitals-fp-fcp/test.ts index a9858c84eec8..8162523542ba 100644 --- a/packages/integration-tests/suites/tracing/metrics/web-vitals-fp-fcp/test.ts +++ b/packages/integration-tests/suites/tracing/metrics/web-vitals-fp-fcp/test.ts @@ -16,8 +16,6 @@ sentryTest('should capture FP vital.', async ({ browserName, getLocalTestPath, p expect(eventData.measurements).toBeDefined(); expect(eventData.measurements?.fp?.value).toBeDefined(); - expect(eventData.measurements?.['mark.fp']?.value).toBeDefined(); - const fpSpan = eventData.spans?.filter(({ description }) => description === 'first-paint')[0]; expect(fpSpan).toBeDefined(); @@ -32,8 +30,6 @@ sentryTest('should capture FCP vital.', async ({ getLocalTestPath, page }) => { expect(eventData.measurements).toBeDefined(); expect(eventData.measurements?.fcp?.value).toBeDefined(); - expect(eventData.measurements?.['mark.fcp']?.value).toBeDefined(); - const fcpSpan = eventData.spans?.filter(({ description }) => description === 'first-contentful-paint')[0]; expect(fcpSpan).toBeDefined(); diff --git a/packages/integration-tests/suites/tracing/metrics/web-vitals-lcp/test.ts b/packages/integration-tests/suites/tracing/metrics/web-vitals-lcp/test.ts index 32f86d3c54d1..01a8b5c2e268 100644 --- a/packages/integration-tests/suites/tracing/metrics/web-vitals-lcp/test.ts +++ b/packages/integration-tests/suites/tracing/metrics/web-vitals-lcp/test.ts @@ -22,7 +22,6 @@ sentryTest('should capture a LCP vital with element details.', async ({ browserN expect(eventData.measurements).toBeDefined(); expect(eventData.measurements?.lcp?.value).toBeDefined(); - expect(eventData.measurements?.['mark.lcp']?.value).toBeDefined(); expect(eventData.tags?.['lcp.element']).toBe('body > img'); expect(eventData.tags?.['lcp.size']).toBe(107400); diff --git a/packages/tracing/src/browser/metrics/index.ts b/packages/tracing/src/browser/metrics/index.ts index 3bc3c9282f32..4e09e4a72bd1 100644 --- a/packages/tracing/src/browser/metrics/index.ts +++ b/packages/tracing/src/browser/metrics/index.ts @@ -87,11 +87,8 @@ function _trackLCP(reportAllChanges: boolean): void { return; } - const timeOrigin = msToSec(browserPerformanceTimeOrigin as number); - const startTime = msToSec(entry.startTime); __DEBUG_BUILD__ && logger.log('[Measurements] Adding LCP'); _measurements['lcp'] = { value: metric.value, unit: 'millisecond' }; - _measurements['mark.lcp'] = { value: timeOrigin + startTime, unit: 'second' }; _lcpEntry = entry as LargestContentfulPaint; }, reportAllChanges); } @@ -147,7 +144,7 @@ export function addPerformanceEntries(transaction: Transaction): void { case 'mark': case 'paint': case 'measure': { - const startTimestamp = _addMeasureSpans(transaction, entry, startTime, duration, timeOrigin); + _addMeasureSpans(transaction, entry, startTime, duration, timeOrigin); // capture web vitals const firstHidden = getVisibilityWatcher(); @@ -157,12 +154,10 @@ export function addPerformanceEntries(transaction: Transaction): void { if (entry.name === 'first-paint' && shouldRecord) { __DEBUG_BUILD__ && logger.log('[Measurements] Adding FP'); _measurements['fp'] = { value: entry.startTime, unit: 'millisecond' }; - _measurements['mark.fp'] = { value: startTimestamp, unit: 'second' }; } if (entry.name === 'first-contentful-paint' && shouldRecord) { __DEBUG_BUILD__ && logger.log('[Measurements] Adding FCP'); _measurements['fcp'] = { value: entry.startTime, unit: 'millisecond' }; - _measurements['mark.fcp'] = { value: startTimestamp, unit: 'second' }; } break; } @@ -220,14 +215,18 @@ export function addPerformanceEntries(transaction: Transaction): void { _measurements[name].value = normalizedValue; }); - if (_measurements['mark.fid'] && _measurements['fid']) { + const fidMark = _measurements['mark.fid']; + if (fidMark && _measurements['fid']) { // create span for FID _startChild(transaction, { description: 'first input delay', - endTimestamp: _measurements['mark.fid'].value + msToSec(_measurements['fid'].value), + endTimestamp: fidMark.value + msToSec(_measurements['fid'].value), op: 'web.vitals', - startTimestamp: _measurements['mark.fid'].value, + startTimestamp: fidMark.value, }); + + // Delete mark.fid as we don't want it to be part of final payload + delete _measurements['mark.fid']; } // If FCP is not recorded we should not record the cls value