ref(perf): Don't use 'mark.<vital>' for drawing web vitals in event details (#38023)

Author: 0Calories

static/app/components/events/interfaces/spans/measurementsPanel.tsx

@@ -22,6 +22,11 @@ type Props = {
   generateBounds: (bounds: SpanBoundsType) => SpanGeneratedBoundsType;
 };
 
+type VitalLabel = {
+  isPoorValue: boolean;
+  vital: Vital;
+};
+
 function MeasurementsPanel(props: Props) {
   const {event, generateBounds, dividerPosition} = props;
   const measurements = getMeasurements(event, generateBounds);
@@ -44,30 +49,26 @@ function MeasurementsPanel(props: Props) {
           return null;
         }
 
-        // Measurements are referred to by their full name `measurements.<name>`
-        // here but are stored using their abbreviated name `<name>`. Make sure
-        // to convert it appropriately.
-        const vitals: Vital[] = Object.keys(verticalMark.marks).map(
-          name => WEB_VITAL_DETAILS[`measurements.${name}`]
-        );
+        const vitalLabels: VitalLabel[] = Object.keys(verticalMark.marks).map(name => ({
+          vital: WEB_VITAL_DETAILS[`measurements.${name}`],
+          isPoorValue: verticalMark.marks[name].failedThreshold,
+        }));
 
-        if (vitals.length > 1) {
+        if (vitalLabels.length > 1) {
           return (
             <MultiLabelContainer
               key={String(timestamp)}
-              failedThreshold={verticalMark.failedThreshold}
               left={toPercent(bounds.left || 0)}
-              vitals={vitals}
+              vitalLabels={vitalLabels}
             />
           );
         }
 
         return (
           <LabelContainer
             key={String(timestamp)}
-            failedThreshold={verticalMark.failedThreshold}
             left={toPercent(bounds.left || 0)}
-            vital={vitals[0]}
+            vitalLabel={vitalLabels[0]}
           />
         );
       })}
@@ -123,9 +124,8 @@ const Label = styled('div')<{
 export default MeasurementsPanel;
 
 type LabelContainerProps = {
-  failedThreshold: boolean;
   left: string;
-  vital: Vital;
+  vitalLabel: VitalLabel;
 };
 
 type LabelContainerState = {
@@ -149,7 +149,7 @@ class LabelContainer extends Component<LabelContainerProps> {
   elementDOMRef = createRef<HTMLDivElement>();
 
   render() {
-    const {left, failedThreshold, vital} = this.props;
+    const {left, vitalLabel} = this.props;
 
     return (
       <StyledLabelContainer
@@ -158,18 +158,23 @@ class LabelContainer extends Component<LabelContainerProps> {
           left: `clamp(calc(0.5 * ${this.state.width}px), ${left}, calc(100% - 0.5 * ${this.state.width}px))`,
         }}
       >
-        <Label failedThreshold={failedThreshold} isSingleLabel>
-          <Tooltip title={vital.name} position="top" containerDisplayMode="inline-block">
-            {vital.acronym}
+        <Label failedThreshold={vitalLabel.isPoorValue} isSingleLabel>
+          <Tooltip
+            title={vitalLabel.vital.name}
+            position="top"
+            containerDisplayMode="inline-block"
+          >
+            {vitalLabel.vital.acronym}
           </Tooltip>
         </Label>
       </StyledLabelContainer>
     );
   }
 }
 
-type MultiLabelContainerProps = Omit<LabelContainerProps, 'vital'> & {
-  vitals: Vital[];
+type MultiLabelContainerProps = {
+  left: string;
+  vitalLabels: VitalLabel[];
 };
 
 class MultiLabelContainer extends Component<MultiLabelContainerProps> {
@@ -190,7 +195,7 @@ class MultiLabelContainer extends Component<MultiLabelContainerProps> {
   elementDOMRef = createRef<HTMLDivElement>();
 
   render() {
-    const {left, failedThreshold, vitals} = this.props;
+    const {left, vitalLabels} = this.props;
 
     return (
       <StyledMultiLabelContainer
@@ -199,14 +204,14 @@ class MultiLabelContainer extends Component<MultiLabelContainerProps> {
           left: `clamp(calc(0.5 * ${this.state.width}px), ${left}, calc(100% - 0.5 * ${this.state.width}px))`,
         }}
       >
-        {vitals.map(vital => (
-          <Label failedThreshold={failedThreshold} key={`${vital.name}-label`}>
+        {vitalLabels.map(label => (
+          <Label failedThreshold={label.isPoorValue} key={`${label.vital.name}-label`}>
             <Tooltip
-              title={vital.name}
+              title={label.vital.name}
               position="top"
               containerDisplayMode="inline-block"
             >
-              {vital.acronym}
+              {label.vital.acronym}
             </Tooltip>
           </Label>
         ))}

static/app/components/events/interfaces/spans/utils.tsx

@@ -14,6 +14,7 @@ import {Organization} from 'sentry/types';
 import {EntryType, EventTransaction} from 'sentry/types/event';
 import {assert} from 'sentry/types/utils';
 import trackAdvancedAnalyticsEvent from 'sentry/utils/analytics/trackAdvancedAnalyticsEvent';
+import {WebVital} from 'sentry/utils/fields';
 import {TraceError} from 'sentry/utils/performance/quickTrace/types';
 import {WEB_VITAL_DETAILS} from 'sentry/utils/performance/vitals/constants';
 import {getPerformanceTransaction} from 'sentry/utils/performanceForSentry';
@@ -512,6 +513,7 @@ export const durationlessBrowserOps = ['mark', 'paint'];
 
 type Measurements = {
   [name: string]: {
+    failedThreshold: boolean;
     timestamp: number;
     value: number | undefined;
   };
@@ -541,26 +543,37 @@ export function getMeasurements(
   event: EventTransaction,
   generateBounds: (bounds: SpanBoundsType) => SpanGeneratedBoundsType
 ): Map<number, VerticalMark> {
-  if (!event.measurements) {
+  if (!event.measurements || !event.startTimestamp) {
     return new Map();
   }
 
+  const {startTimestamp} = event;
+
+  // Note: CLS and INP should not be included here, since they are not timeline-based measurements.
+  const allowedVitals = new Set<string>([
+    WebVital.FCP,
+    WebVital.FP,
+    WebVital.FID,
+    WebVital.LCP,
+    WebVital.TTFB,
+  ]);
+
   const measurements = Object.keys(event.measurements)
-    .filter(name => name.startsWith('mark.'))
+    .filter(name => allowedVitals.has(`measurements.${name}`))
     .map(name => {
-      const slug = name.slice('mark.'.length);
-      const associatedMeasurement = event.measurements![slug];
+      const associatedMeasurement = event.measurements![name];
       return {
         name,
-        timestamp: event.measurements![name].value,
+        // Time timestamp is in seconds, but the measurement value is given in ms so convert it here
+        timestamp: startTimestamp + associatedMeasurement.value / 1000,
         value: associatedMeasurement ? associatedMeasurement.value : undefined,
       };
     });
 
   const mergedMeasurements = new Map<number, VerticalMark>();
 
   measurements.forEach(measurement => {
-    const name = measurement.name.slice('mark.'.length);
+    const name = measurement.name;
     const value = measurement.value;
 
     const bounds = generateBounds({
@@ -584,11 +597,14 @@ export function getMeasurements(
       if (positionDelta <= MERGE_LABELS_THRESHOLD_PERCENT) {
         const verticalMark = mergedMeasurements.get(otherPos)!;
 
+        const {poorThreshold} = WEB_VITAL_DETAILS[`measurements.${name}`];
+
         verticalMark.marks = {
           ...verticalMark.marks,
           [name]: {
             value,
             timestamp: measurement.timestamp,
+            failedThreshold: value ? value >= poorThreshold : false,
           },
         };
 
@@ -601,15 +617,22 @@ export function getMeasurements(
       }
     }
 
+    const {poorThreshold} = WEB_VITAL_DETAILS[`measurements.${name}`];
+
     const marks = {
-      [name]: {value, timestamp: measurement.timestamp},
+      [name]: {
+        value,
+        timestamp: measurement.timestamp,
+        failedThreshold: value ? value >= poorThreshold : false,
+      },
     };
 
     mergedMeasurements.set(roundedPos, {
       marks,
       failedThreshold: hasFailedThreshold(marks),
     });
   });
+
   return mergedMeasurements;
 }
 

tests/js/spec/components/events/interfaces/spans/traceView.spec.tsx

@@ -734,9 +734,9 @@ describe('TraceView', () => {
     generateSampleSpan('browser', 'test5', 'f000000000000000', 'a000000000000000', event);
 
     event.measurements = {
-      'mark.fcp': {value: 1000},
-      'mark.fp': {value: 1050},
-      'mark.lcp': {value: 1100},
+      fcp: {value: 1000},
+      fp: {value: 1050},
+      lcp: {value: 1100},
     };
 
     const waterfallModel = new WaterfallModel(event);
@@ -758,9 +758,12 @@ describe('TraceView', () => {
     const event = generateSampleEvent();
     generateSampleSpan('browser', 'test1', 'b000000000000000', 'a000000000000000', event);
 
+    event.startTimestamp = 1;
+    event.endTimestamp = 100;
+
     event.measurements = {
-      'mark.fcp': {value: 1000},
-      'mark.lcp': {value: 200000000},
+      fcp: {value: 858.3002090454102, unit: 'millisecond'},
+      lcp: {value: 1000363.800048828125, unit: 'millisecond'},
     };
 
     const waterfallModel = new WaterfallModel(event);