Skip to content

Commit 3edae9c

Browse files
priscilawebdevpriscilawebdev
authored
fix(performance-metrics): Handle null serie values (#30594)
1 parent d41082e commit 3edae9c

File tree

6 files changed

+174
-66
lines changed

6 files changed

+174
-66
lines changed

‎static/app/types/metrics.tsx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@ export type MetricsApiResponse = {
44
intervals: string[];
55
groups: {
66
by: Record<string, string>;
7-
totals: Record<string, number>;
8-
series: Record<string, number[]>;
7+
totals: Record<string, number | null>;
8+
series: Record<string, Array<number | null>>;
99
}[];
1010
start: DateString;
1111
end: DateString;

‎static/app/utils/discover/charts.tsx‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import {t} from 'sentry/locale';
2+
import {defined} from 'sentry/utils';
23
import {aggregateOutputType} from 'sentry/utils/discover/fields';
34
import {
45
DAY,
@@ -12,9 +13,13 @@ import {
1213
} from 'sentry/utils/formatters';
1314

1415
/**
15-
* Formatter for chart tooltips that handle a variety of discover result values
16+
* Formatter for chart tooltips that handle a variety of discover and metrics result values.
17+
* If the result is metric values, the value can be of type number or null
1618
*/
17-
export function tooltipFormatter(value: number, seriesName: string = ''): string {
19+
export function tooltipFormatter(value: number | null, seriesName: string = ''): string {
20+
if (!defined(value)) {
21+
return '\u2014';
22+
}
1823
switch (aggregateOutputType(seriesName)) {
1924
case 'integer':
2025
case 'number':

‎static/app/views/performance/landing/widgets/transforms/transformMetricsToArea.tsx‎

Lines changed: 69 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import mean from 'lodash/mean';
22
import moment from 'moment';
33

44
import {getParams} from 'sentry/components/organizations/globalSelectionHeader/getParams';
5+
import {Series} from 'sentry/types/echarts';
56
import {defined} from 'sentry/utils';
67
import {axisLabelFormatter} from 'sentry/utils/discover/charts';
78
import {aggregateOutputType} from 'sentry/utils/discover/fields';
@@ -60,38 +61,65 @@ export function transformMetricsToArea<T extends WidgetDataConstraint>(
6061
const totalPerBucket = isFailureRateWidget
6162
? response.intervals.map((_intervalValue, intervalIndex) =>
6263
response.groups.reduce(
63-
(acc, group) => acc + group.series[metricsField][intervalIndex],
64+
(acc, group) => acc + (group.series[metricsField][intervalIndex] ?? 0),
6465
0
6566
)
6667
)
6768
: undefined;
6869

69-
const data = groups.map(group => ({
70-
seriesName: metricsField,
71-
totals: group.totals[metricsField],
72-
data: response.intervals.map((intervalValue, intervalIndex) => ({
73-
name: moment(intervalValue).valueOf(),
74-
value: defined(totalPerBucket)
75-
? group.series[metricsField][intervalIndex] / totalPerBucket[intervalIndex]
76-
: group.series[metricsField][intervalIndex],
77-
})),
78-
}));
70+
const data = groups.map(group => {
71+
const series = response.intervals.map((intervalValue, intervalIndex) => {
72+
const serieBucket = group.series[metricsField][intervalIndex];
73+
const totalSerieBucket = totalPerBucket?.[intervalIndex];
74+
return {
75+
name: moment(intervalValue).valueOf(),
76+
value:
77+
defined(totalSerieBucket) &&
78+
defined(serieBucket) &&
79+
serieBucket > 0 &&
80+
totalSerieBucket > 0
81+
? serieBucket / totalSerieBucket
82+
: serieBucket,
83+
};
84+
});
85+
86+
return {
87+
seriesName: metricsField,
88+
totals: group.totals[metricsField],
89+
data: series.some(serie => defined(serie.value)) ? series : [],
90+
};
91+
});
7992

8093
const seriesTotal = isFailureRateWidget
81-
? response.groups.reduce((acc, group) => acc + group.totals[metricsField], 0)
94+
? response.groups.reduce((acc, group) => acc + (group.totals[metricsField] ?? 0), 0)
8295
: undefined;
8396

8497
const dataMean = data.map(serie => {
85-
const meanData = defined(seriesTotal)
86-
? serie.totals / seriesTotal
87-
: mean(serie.data.map(({value}) => value));
98+
let meanData: undefined | number = undefined;
99+
100+
if (
101+
defined(seriesTotal) &&
102+
defined(serie.totals) &&
103+
serie.totals > 0 &&
104+
seriesTotal > 0
105+
) {
106+
meanData = serie.totals / seriesTotal;
107+
} else {
108+
const serieData = serie.data
109+
.filter(({value}) => defined(value))
110+
.map(({value}) => value);
111+
112+
if (serieData.length > 0) {
113+
meanData = mean(serieData);
114+
}
115+
}
88116

89117
const seriesName = defined(seriesTotal) ? 'failure_rate()' : serie.seriesName;
90118

91119
return {
92120
mean: meanData,
93121
outputType: aggregateOutputType(seriesName),
94-
label: axisLabelFormatter(meanData, seriesName),
122+
label: defined(meanData) ? axisLabelFormatter(meanData, seriesName) : undefined,
95123
};
96124
});
97125

@@ -104,28 +132,40 @@ export function transformMetricsToArea<T extends WidgetDataConstraint>(
104132
const previousTotalPerBucket = isFailureRateWidget
105133
? responsePrevious?.intervals.map((_intervalValue, intervalIndex) =>
106134
responsePrevious?.groups.reduce(
107-
(acc, group) => acc + group.series[metricsField][intervalIndex],
135+
(acc, group) => acc + (group.series[metricsField][intervalIndex] ?? 0),
108136
0
109137
)
110138
)
111139
: undefined;
112140

113-
const previousData = previousGroups?.map(group => ({
114-
seriesName: `previous ${metricsField}`,
115-
data: response?.intervals.map((intervalValue, intervalIndex) => ({
116-
name: moment(intervalValue).valueOf(),
117-
value: defined(previousTotalPerBucket)
118-
? group.series[metricsField][intervalIndex] /
119-
previousTotalPerBucket[intervalIndex]
120-
: group.series[metricsField][intervalIndex],
121-
})),
122-
stack: 'previous',
123-
}));
141+
const previousData = previousGroups?.map(group => {
142+
const series = response?.intervals.map((intervalValue, intervalIndex) => {
143+
const serieBucket = group.series[metricsField][intervalIndex];
144+
const totalSerieBucket = previousTotalPerBucket?.[intervalIndex];
145+
146+
return {
147+
name: moment(intervalValue).valueOf(),
148+
value:
149+
defined(totalSerieBucket) &&
150+
defined(serieBucket) &&
151+
serieBucket > 0 &&
152+
totalSerieBucket > 0
153+
? serieBucket / totalSerieBucket
154+
: serieBucket,
155+
};
156+
});
157+
158+
return {
159+
seriesName: `previous ${metricsField}`,
160+
stack: 'previous',
161+
data: series.some(serie => defined(serie.value)) ? series : [],
162+
};
163+
}) as null | Series[];
124164

125165
return {
126166
...commonChildData,
127167
hasData: defined(data) && !!data.length && !!data[0].data.length,
128-
data,
168+
data: data as Series[],
129169
dataMean,
130170
previousData: previousData ?? undefined,
131171
};

‎static/app/views/performance/landing/widgets/widgets/lineChartListWidgetMetrics.tsx‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import _EventsRequest from 'sentry/components/charts/eventsRequest';
55
import Count from 'sentry/components/count';
66
import Truncate from 'sentry/components/truncate';
77
import {t} from 'sentry/locale';
8+
import {defined} from 'sentry/utils';
89
import MetricsRequest from 'sentry/utils/metrics/metricsRequest';
910
import {decodeList} from 'sentry/utils/queryString';
1011
import {MutableSearch} from 'sentry/utils/tokenizeSearch';
@@ -165,7 +166,12 @@ export function LineChartListWidgetMetrics(props: PerformanceWidgetProps) {
165166
selectedIndex={selectedListIndex}
166167
setSelectedIndex={setSelectListIndex}
167168
items={provided.widgetData.list.data.map(listItem => () => {
168-
const transaction = listItem.transaction as string;
169+
const transaction = listItem.transaction as string | null;
170+
const countValue = listItem[field];
171+
172+
if (!transaction) {
173+
return null;
174+
}
169175

170176
const transactionTarget = transactionSummaryRouteWithQuery({
171177
orgSlug: organization.slug,
@@ -180,11 +186,11 @@ export function LineChartListWidgetMetrics(props: PerformanceWidgetProps) {
180186
<Truncate value={transaction} maxLength={40} />
181187
</GrowLink>
182188
<RightAlignedCell>
183-
<Count value={listItem[field]} />
189+
{defined(countValue) ? <Count value={countValue} /> : '\u2014'}
184190
</RightAlignedCell>
185191
<ListClose
186192
setSelectListIndex={setSelectListIndex}
187-
onClick={() => excludeTransaction(listItem.transaction, props)}
193+
onClick={() => excludeTransaction(transaction, props)}
188194
/>
189195
</Fragment>
190196
);

‎static/app/views/performance/landing/widgets/widgets/vitalWidgetMetrics.tsx‎

Lines changed: 40 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import Truncate from 'sentry/components/truncate';
66
import {t} from 'sentry/locale';
77
import space from 'sentry/styles/space';
88
import {MetricsApiResponse} from 'sentry/types';
9+
import {defined} from 'sentry/utils';
910
import {WebVital} from 'sentry/utils/discover/fields';
1011
import MetricsRequest from 'sentry/utils/metrics/metricsRequest';
1112
import {VitalData} from 'sentry/utils/performance/vitals/vitalsCardsDiscoverQuery';
@@ -183,33 +184,37 @@ export function VitalWidgetMetrics(props: PerformanceWidgetProps) {
183184
Queries={Queries}
184185
Visualizations={[
185186
{
186-
component: provided => (
187-
<_VitalChart
188-
{...provided.widgetData.chart}
189-
data={
190-
provided.widgetData.chart.data[
191-
provided.widgetData.list.data[selectedListIndex].transaction
192-
]
193-
}
194-
{...provided}
195-
field={field}
196-
vitalFields={{
197-
poorCountField: 'poor',
198-
mehCountField: 'meh',
199-
goodCountField: 'good',
200-
}}
201-
organization={organization}
202-
query={eventView.query}
203-
project={eventView.project}
204-
environment={eventView.environment}
205-
grid={{
206-
left: space(0),
207-
right: space(0),
208-
top: space(2),
209-
bottom: space(2),
210-
}}
211-
/>
212-
),
187+
component: provided => {
188+
const transaction =
189+
provided.widgetData.list.data[selectedListIndex].transaction;
190+
return (
191+
<_VitalChart
192+
{...provided.widgetData.chart}
193+
data={
194+
defined(transaction)
195+
? provided.widgetData.chart.data[transaction]
196+
: undefined
197+
}
198+
{...provided}
199+
field={field}
200+
vitalFields={{
201+
poorCountField: 'poor',
202+
mehCountField: 'meh',
203+
goodCountField: 'good',
204+
}}
205+
organization={organization}
206+
query={eventView.query}
207+
project={eventView.project}
208+
environment={eventView.environment}
209+
grid={{
210+
left: space(0),
211+
right: space(0),
212+
top: space(2),
213+
bottom: space(2),
214+
}}
215+
/>
216+
);
217+
},
213218
height: 160,
214219
},
215220
{
@@ -220,7 +225,12 @@ export function VitalWidgetMetrics(props: PerformanceWidgetProps) {
220225
selectedIndex={selectedListIndex}
221226
setSelectedIndex={setSelectListIndex}
222227
items={widgetData.list.data.map(listItem => () => {
223-
const transaction = listItem.transaction as string;
228+
const transaction = listItem.transaction as string | null;
229+
230+
if (!transaction) {
231+
return null;
232+
}
233+
224234
const _eventView = eventView.clone();
225235

226236
const initialConditions = new MutableSearch(_eventView.query);
@@ -259,7 +269,7 @@ export function VitalWidgetMetrics(props: PerformanceWidgetProps) {
259269
</VitalBarCell>
260270
<ListClose
261271
setSelectListIndex={setSelectListIndex}
262-
onClick={() => excludeTransaction(listItem.transaction, props)}
272+
onClick={() => excludeTransaction(transaction, props)}
263273
/>
264274
</Fragment>
265275
);
@@ -292,7 +302,7 @@ function getVitalData(
292302
acc[group.by.measurement_rating] = group.totals[field];
293303
return acc;
294304
}, {}),
295-
total: groups.reduce((acc, group) => acc + group.totals[field], 0),
305+
total: groups.reduce((acc, group) => acc + (group.totals[field] ?? 0), 0),
296306
};
297307

298308
return vitalData;

‎tests/js/spec/views/performance/landing/widgets/widgetContainer.spec.jsx‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -924,6 +924,53 @@ describe('Performance > Widgets > WidgetContainer', function () {
924924
);
925925
});
926926

927+
it('P95 Duration - with null values', async function () {
928+
const field = `p95(${TransactionMetric.SENTRY_TRANSACTIONS_TRANSACTION_DURATION})`;
929+
930+
const metricsData = TestStubs.MetricsField({field});
931+
metricsData.groups[0].series[field][0] = null;
932+
metricsData.groups[0].series[field][1] = null;
933+
934+
MockApiClient.addMockResponse({
935+
method: 'GET',
936+
url: `/organizations/org-slug/metrics/data/`,
937+
body: metricsData,
938+
match: [(...args) => !issuesPredicate(...args)],
939+
});
940+
941+
const previousData = TestStubs.MetricsField({field});
942+
previousData.groups[0].series[field][0] = null;
943+
previousData.groups[0].series[field][1] = null;
944+
945+
MockApiClient.addMockResponse({
946+
method: 'GET',
947+
url: `/organizations/org-slug/metrics/data/`,
948+
body: previousData,
949+
match: [
950+
(...args) => {
951+
return (
952+
!issuesPredicate(...args) &&
953+
args[1].query.statsPeriodStart &&
954+
args[1].query.statsPeriodEnd
955+
);
956+
},
957+
],
958+
});
959+
960+
wrapper = mountWithTheme(
961+
<WrappedComponent
962+
data={data}
963+
defaultChartSetting={PerformanceWidgetSetting.P95_DURATION_AREA}
964+
isMetricsData
965+
/>,
966+
data.routerContext
967+
);
968+
await tick();
969+
wrapper.update();
970+
971+
expect(wrapper.find('HighlightNumber').text()).toEqual('536ms');
972+
});
973+
927974
it('P99 Duration', async function () {
928975
const field = `p99(${TransactionMetric.SENTRY_TRANSACTIONS_TRANSACTION_DURATION})`;
929976

0 commit comments

Comments
 (0)