Skip to content

Commit 2bb3b87

Browse files
authored
feat(aci): Format percentage based thresholds/data (#104035)
- fixes an issue where release dataset wasn't formatted correctly on aci details - fixes thresholds on % based charts being (10 being rendered as 10k percent instead of 0.1 - removes yAxis min for percentage based charts since its a bit more useful sometimes to see 90 -> 100% <img width="1072" height="201" alt="image" src="https://github.com/user-attachments/assets/7a46a227-e5b9-47b6-961f-3c7baf56c0b8" />
1 parent 8e34b54 commit 2bb3b87

File tree

4 files changed

+123
-71
lines changed

4 files changed

+123
-71
lines changed

static/app/views/detectors/components/details/metric/chart.tsx

Lines changed: 23 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import {
2323
buildDetectorZoomQuery,
2424
computeZoomRangeMs,
2525
} from 'sentry/views/detectors/components/details/common/buildDetectorZoomQuery';
26+
import {useDetectorChartAxisBounds} from 'sentry/views/detectors/components/details/metric/utils/useDetectorChartAxisBounds';
2627
import {getDatasetConfig} from 'sentry/views/detectors/datasetConfig/getDatasetConfig';
2728
import {getDetectorDataset} from 'sentry/views/detectors/datasetConfig/getDetectorDataset';
2829
import {
@@ -161,11 +162,12 @@ export function useMetricDetectorChart({
161162
const snubaQuery = detector.dataSources[0].queryObj.snubaQuery;
162163
const dataset = getDetectorDataset(snubaQuery.dataset, snubaQuery.eventTypes);
163164
const datasetConfig = getDatasetConfig(dataset);
165+
const aggregate = datasetConfig.fromApiAggregate(snubaQuery.aggregate);
164166
const {series, comparisonSeries, isLoading, error} = useMetricDetectorSeries({
165167
detectorDataset: dataset,
166168
dataset: snubaQuery.dataset,
167169
extrapolationMode: snubaQuery.extrapolationMode,
168-
aggregate: datasetConfig.fromApiAggregate(snubaQuery.aggregate),
170+
aggregate,
169171
interval: snubaQuery.timeWindow,
170172
query: snubaQuery.query,
171173
environment: snubaQuery.environment,
@@ -181,6 +183,7 @@ export function useMetricDetectorChart({
181183
useMetricDetectorThresholdSeries({
182184
conditions: detector.conditionGroup?.conditions,
183185
detectionType,
186+
aggregate,
184187
comparisonSeries,
185188
});
186189

@@ -221,30 +224,7 @@ export function useMetricDetectorChart({
221224
usePageDate: true,
222225
});
223226

224-
// Calculate y-axis bounds to ensure all thresholds are visible
225-
const maxValue = useMemo(() => {
226-
// Get max from series data
227-
let seriesMax = 0;
228-
if (series.length > 0) {
229-
const allSeriesValues = series.flatMap(s =>
230-
s.data
231-
.map(point => point.value)
232-
.filter(val => typeof val === 'number' && !isNaN(val))
233-
);
234-
seriesMax = allSeriesValues.length > 0 ? Math.max(...allSeriesValues) : 0;
235-
}
236-
237-
// Combine with threshold max and round to nearest whole number
238-
const combinedMax = thresholdMaxValue
239-
? Math.max(seriesMax, thresholdMaxValue)
240-
: seriesMax;
241-
242-
const roundedMax = Math.round(combinedMax);
243-
244-
// Add padding to the bounds
245-
const padding = roundedMax * 0.1;
246-
return roundedMax + padding;
247-
}, [series, thresholdMaxValue]);
227+
const {maxValue, minValue} = useDetectorChartAxisBounds({series, thresholdMaxValue});
248228

249229
const additionalSeries = useMemo(() => {
250230
const baseSeries = [...thresholdAdditionalSeries];
@@ -256,18 +236,25 @@ export function useMetricDetectorChart({
256236
}, [thresholdAdditionalSeries, openPeriodMarkerResult.incidentMarkerSeries]);
257237

258238
const yAxes = useMemo(() => {
259-
const {formatYAxisLabel} = getDetectorChartFormatters({
239+
const {formatYAxisLabel, outputType} = getDetectorChartFormatters({
260240
detectionType,
261-
aggregate: snubaQuery.aggregate,
241+
aggregate,
262242
});
263243

244+
const isPercentage = outputType === 'percentage';
245+
// For percentage aggregates, use fixed max of 1 (100%) and calculated min
246+
const yAxisMax = isPercentage ? 1 : maxValue > 0 ? maxValue : undefined;
247+
// Start charts at 0 for non-percentage aggregates
248+
const yAxisMin = isPercentage ? minValue : 0;
249+
264250
const mainYAxis: YAXisComponentOption = {
265-
max: maxValue > 0 ? maxValue : undefined,
266-
min: 0,
251+
max: yAxisMax,
252+
min: yAxisMin,
267253
axisLabel: {
268-
// Hide the maximum y-axis label to avoid showing arbitrary threshold values
269-
showMaxLabel: false,
270-
formatter: (value: number) => formatYAxisLabel(value),
254+
// Show max label for percentage (100%) but hide for other types to avoid arbitrary values
255+
showMaxLabel: isPercentage,
256+
// Format the axis labels with units
257+
formatter: formatYAxisLabel,
271258
},
272259
// Disable the y-axis grid lines
273260
splitLine: {show: false},
@@ -282,8 +269,9 @@ export function useMetricDetectorChart({
282269
return axes;
283270
}, [
284271
detectionType,
285-
snubaQuery.aggregate,
272+
aggregate,
286273
maxValue,
274+
minValue,
287275
openPeriodMarkerResult.incidentMarkerYAxis,
288276
]);
289277

@@ -314,7 +302,7 @@ export function useMetricDetectorChart({
314302
tooltip: {
315303
valueFormatter: getDetectorChartFormatters({
316304
detectionType,
317-
aggregate: snubaQuery.aggregate,
305+
aggregate,
318306
}).formatTooltipValue,
319307
},
320308
...chartZoomProps,
@@ -325,6 +313,7 @@ export function useMetricDetectorChart({
325313
};
326314
}, [
327315
additionalSeries,
316+
aggregate,
328317
chartZoomProps,
329318
detectionType,
330319
error,
@@ -333,7 +322,6 @@ export function useMetricDetectorChart({
333322
isLoading,
334323
openPeriodMarkerResult,
335324
series,
336-
snubaQuery.aggregate,
337325
yAxes,
338326
]);
339327

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
import {useMemo} from 'react';
2+
3+
import type {Series} from 'sentry/types/echarts';
4+
5+
interface UseChartAxisBoundsProps {
6+
series: Series[];
7+
thresholdMaxValue: number | undefined;
8+
}
9+
10+
interface ChartAxisBounds {
11+
maxValue: number;
12+
minValue: number;
13+
}
14+
15+
/**
16+
* Calculates y-axis bounds for detector charts based on series data and threshold values.
17+
* Adds padding to ensure all data points and thresholds are visible.
18+
*/
19+
export function useDetectorChartAxisBounds({
20+
series,
21+
thresholdMaxValue,
22+
}: UseChartAxisBoundsProps): ChartAxisBounds {
23+
return useMemo(() => {
24+
if (series.length === 0) {
25+
return {maxValue: 0, minValue: 0};
26+
}
27+
28+
const allSeriesValues = series.flatMap(s =>
29+
s.data
30+
.map(point => point.value)
31+
.filter(val => typeof val === 'number' && !isNaN(val))
32+
);
33+
34+
if (allSeriesValues.length === 0) {
35+
return {maxValue: 0, minValue: 0};
36+
}
37+
38+
const seriesMax = Math.max(...allSeriesValues);
39+
const seriesMin = Math.min(...allSeriesValues);
40+
41+
// Combine with threshold max and round to nearest whole number
42+
const combinedMax = thresholdMaxValue
43+
? Math.max(seriesMax, thresholdMaxValue)
44+
: seriesMax;
45+
46+
const roundedMax = Math.round(combinedMax);
47+
48+
// Add padding to the bounds
49+
const maxPadding = roundedMax * 0.1;
50+
const minPadding = seriesMin * 0.1;
51+
52+
return {
53+
maxValue: roundedMax + maxPadding,
54+
minValue: Math.max(0, seriesMin - minPadding),
55+
};
56+
}, [series, thresholdMaxValue]);
57+
}

static/app/views/detectors/components/forms/metric/metricDetectorChart.tsx

Lines changed: 14 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import {
2525
EventTypes,
2626
ExtrapolationMode,
2727
} from 'sentry/views/alerts/rules/metric/types';
28+
import {useDetectorChartAxisBounds} from 'sentry/views/detectors/components/details/metric/utils/useDetectorChartAxisBounds';
2829
import {getBackendDataset} from 'sentry/views/detectors/components/forms/metric/metricFormData';
2930
import type {DetectorDataset} from 'sentry/views/detectors/datasetConfig/types';
3031
import {useIncidentMarkers} from 'sentry/views/detectors/hooks/useIncidentMarkers';
@@ -177,6 +178,7 @@ export function MetricDetectorChart({
177178
useMetricDetectorThresholdSeries({
178179
conditions,
179180
detectionType,
181+
aggregate,
180182
comparisonSeries,
181183
});
182184

@@ -216,30 +218,7 @@ export function MetricDetectorChart({
216218
markLineTooltip: anomalyMarklineTooltip,
217219
});
218220

219-
// Calculate y-axis bounds to ensure all thresholds are visible
220-
const maxValue = useMemo(() => {
221-
// Get max from series data
222-
let seriesMax = 0;
223-
if (series.length > 0) {
224-
const allSeriesValues = series.flatMap(s =>
225-
s.data
226-
.map(point => point.value)
227-
.filter(val => typeof val === 'number' && !isNaN(val))
228-
);
229-
seriesMax = allSeriesValues.length > 0 ? Math.max(...allSeriesValues) : 0;
230-
}
231-
232-
// Combine with threshold max and round to nearest whole number
233-
const combinedMax = thresholdMaxValue
234-
? Math.max(seriesMax, thresholdMaxValue)
235-
: seriesMax;
236-
237-
const roundedMax = Math.round(combinedMax);
238-
239-
// Add padding to the bounds
240-
const padding = roundedMax * 0.1;
241-
return roundedMax + padding;
242-
}, [series, thresholdMaxValue]);
221+
const {maxValue, minValue} = useDetectorChartAxisBounds({series, thresholdMaxValue});
243222

244223
const additionalSeries = useMemo(() => {
245224
const baseSeries = [...thresholdAdditionalSeries];
@@ -257,17 +236,22 @@ export function MetricDetectorChart({
257236
]);
258237

259238
const yAxes = useMemo(() => {
260-
const {formatYAxisLabel} = getDetectorChartFormatters({
239+
const {formatYAxisLabel, outputType} = getDetectorChartFormatters({
261240
detectionType,
262241
aggregate,
263242
});
264243

244+
const isPercentage = outputType === 'percentage';
245+
// For percentage aggregates, use fixed max of 1 (100%) and calculated min
246+
const yAxisMax = isPercentage ? 1 : maxValue > 0 ? maxValue : undefined;
247+
const yAxisMin = isPercentage ? minValue : 0;
248+
265249
const mainYAxis: YAXisComponentOption = {
266-
max: maxValue > 0 ? maxValue : undefined,
267-
min: 0,
250+
max: yAxisMax,
251+
min: yAxisMin,
268252
axisLabel: {
269-
// Hide the maximum y-axis label to avoid showing arbitrary threshold values
270-
showMaxLabel: false,
253+
// Show max label for percentage (100%) but hide for other types to avoid arbitrary values
254+
showMaxLabel: isPercentage,
271255
// Format the axis labels with units
272256
formatter: formatYAxisLabel,
273257
},
@@ -285,6 +269,7 @@ export function MetricDetectorChart({
285269
return axes;
286270
}, [
287271
maxValue,
272+
minValue,
288273
isAnomalyDetection,
289274
anomalyMarkerResult.incidentMarkerYAxis,
290275
detectionType,

static/app/views/detectors/hooks/useMetricDetectorThresholdSeries.tsx

Lines changed: 29 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import type {
1515
MetricCondition,
1616
MetricDetectorConfig,
1717
} from 'sentry/types/workflowEngine/detectors';
18+
import {aggregateOutputType} from 'sentry/utils/discover/fields';
1819

1920
function createThresholdMarkLine(lineColor: string, threshold: number) {
2021
return MarkLine({
@@ -117,6 +118,10 @@ function extractThresholdsFromConditions(
117118
interface UseMetricDetectorThresholdSeriesProps {
118119
conditions: Array<Omit<MetricCondition, 'id'>> | undefined;
119120
detectionType: MetricDetectorConfig['detectionType'];
121+
/**
122+
* The aggregate function to determine if thresholds should be scaled for percentage display
123+
*/
124+
aggregate?: string;
120125
comparisonSeries?: Series[];
121126
}
122127

@@ -134,11 +139,16 @@ interface UseMetricDetectorThresholdSeriesResult {
134139
export function useMetricDetectorThresholdSeries({
135140
conditions,
136141
detectionType,
142+
aggregate,
137143
comparisonSeries = [],
138144
}: UseMetricDetectorThresholdSeriesProps): UseMetricDetectorThresholdSeriesResult {
139145
const theme = useTheme();
140146

141147
return useMemo((): UseMetricDetectorThresholdSeriesResult => {
148+
// For percentage aggregates (e.g., crash-free rate), thresholds are input as whole numbers
149+
// (e.g., 95 for 95%) but need to be displayed as decimals (0.95) on the chart
150+
const isPercentageAggregate =
151+
aggregate && aggregateOutputType(aggregate) === 'percentage';
142152
if (!conditions) {
143153
return {maxValue: undefined, additionalSeries: []};
144154
}
@@ -216,11 +226,15 @@ export function useMetricDetectorThresholdSeries({
216226
? theme.red300
217227
: theme.yellow300;
218228
const areaColor = lineColor;
229+
// Scale threshold for percentage aggregates (e.g., 95 -> 0.95)
230+
const displayThreshold = isPercentageAggregate
231+
? threshold.value / 100
232+
: threshold.value;
219233

220234
return {
221235
type: 'line',
222-
markLine: createThresholdMarkLine(lineColor, threshold.value),
223-
markArea: createThresholdMarkArea(areaColor, threshold.value, isAbove),
236+
markLine: createThresholdMarkLine(lineColor, displayThreshold),
237+
markArea: createThresholdMarkArea(areaColor, displayThreshold, isAbove),
224238
data: [],
225239
};
226240
});
@@ -231,12 +245,16 @@ export function useMetricDetectorThresholdSeries({
231245
resolution && !thresholds.some(threshold => threshold.value === resolution.value)
232246
);
233247
if (resolution && isResolutionManual) {
248+
// Scale resolution threshold for percentage aggregates
249+
const displayResolution = isPercentageAggregate
250+
? resolution.value / 100
251+
: resolution.value;
234252
const resolutionSeries: LineSeriesOption = {
235253
type: 'line',
236-
markLine: createThresholdMarkLine(theme.green300, resolution.value),
254+
markLine: createThresholdMarkLine(theme.green300, displayResolution),
237255
markArea: createThresholdMarkArea(
238256
theme.green300,
239-
resolution.value,
257+
displayResolution,
240258
resolution.type === DataConditionType.GREATER
241259
),
242260
data: [],
@@ -245,14 +263,18 @@ export function useMetricDetectorThresholdSeries({
245263
}
246264

247265
const valuesForMax = [
248-
...thresholds.map(threshold => threshold.value),
249-
...(resolution && isResolutionManual ? [resolution.value] : []),
266+
...thresholds.map(threshold =>
267+
isPercentageAggregate ? threshold.value / 100 : threshold.value
268+
),
269+
...(resolution && isResolutionManual
270+
? [isPercentageAggregate ? resolution.value / 100 : resolution.value]
271+
: []),
250272
];
251273
const maxValue = valuesForMax.length > 0 ? Math.max(...valuesForMax) : undefined;
252274
return {maxValue, additionalSeries: additional};
253275
}
254276

255277
// Other detection types not supported yet
256278
return {maxValue: undefined, additionalSeries: additional};
257-
}, [conditions, detectionType, comparisonSeries, theme]);
279+
}, [conditions, detectionType, aggregate, comparisonSeries, theme]);
258280
}

0 commit comments

Comments
 (0)