From 2db2c43cb22777764338c248102e1b25a9022398 Mon Sep 17 00:00:00 2001 From: Ash Anand Date: Thu, 18 Aug 2022 14:06:34 -0400 Subject: [PATCH 1/3] Calculate timestamp values of measurements without using mark --- .../events/interfaces/spans/utils.tsx | 37 +++++++++++++++---- 1 file changed, 30 insertions(+), 7 deletions(-) diff --git a/static/app/components/events/interfaces/spans/utils.tsx b/static/app/components/events/interfaces/spans/utils.tsx index b5f1f096acbd1a..b060559ba50042 100644 --- a/static/app/components/events/interfaces/spans/utils.tsx +++ b/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,18 +543,29 @@ export function getMeasurements( event: EventTransaction, generateBounds: (bounds: SpanBoundsType) => SpanGeneratedBoundsType ): Map { - 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([ + 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, }; }); @@ -560,7 +573,7 @@ export function getMeasurements( const mergedMeasurements = new Map(); 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,8 +617,14 @@ 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, { @@ -610,6 +632,7 @@ export function getMeasurements( failedThreshold: hasFailedThreshold(marks), }); }); + return mergedMeasurements; } From c455ba51e35c117dc846eedda834cf0eccf4db78 Mon Sep 17 00:00:00 2001 From: Ash Anand Date: Thu, 18 Aug 2022 14:39:32 -0400 Subject: [PATCH 2/3] Refactor to not require failedThreshold for label containers --- .../interfaces/spans/measurementsPanel.tsx | 53 ++++++++++--------- 1 file changed, 29 insertions(+), 24 deletions(-) diff --git a/static/app/components/events/interfaces/spans/measurementsPanel.tsx b/static/app/components/events/interfaces/spans/measurementsPanel.tsx index 885e64c2f637e7..9b9e2fcb383d76 100644 --- a/static/app/components/events/interfaces/spans/measurementsPanel.tsx +++ b/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,20 +49,17 @@ function MeasurementsPanel(props: Props) { return null; } - // Measurements are referred to by their full name `measurements.` - // here but are stored using their abbreviated 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 ( ); } @@ -65,9 +67,8 @@ function MeasurementsPanel(props: Props) { return ( ); })} @@ -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 { elementDOMRef = createRef(); render() { - const {left, failedThreshold, vital} = this.props; + const {left, vitalLabel} = this.props; return ( { left: `clamp(calc(0.5 * ${this.state.width}px), ${left}, calc(100% - 0.5 * ${this.state.width}px))`, }} > - @@ -168,8 +172,9 @@ class LabelContainer extends Component { } } -type MultiLabelContainerProps = Omit & { - vitals: Vital[]; +type MultiLabelContainerProps = { + left: string; + vitalLabels: VitalLabel[]; }; class MultiLabelContainer extends Component { @@ -190,7 +195,7 @@ class MultiLabelContainer extends Component { elementDOMRef = createRef(); render() { - const {left, failedThreshold, vitals} = this.props; + const {left, vitalLabels} = this.props; return ( { left: `clamp(calc(0.5 * ${this.state.width}px), ${left}, calc(100% - 0.5 * ${this.state.width}px))`, }} > - {vitals.map(vital => ( -