ref(tracing): Remove mark measurements (#5605)

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.
getsentry · Aug 19, 2022 · f563eae · f563eae
1 parent f411999
commit f563eae
Show file tree

Hide file tree

Showing 4 changed files with 8 additions and 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
@@ -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
@@ -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
@@ -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
@@ -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