Skip to content

Commit 370834f

Browse files
authored
fix(aci): use aggregate output type for % change detection charts (#104061)
fixes an issue where percent change metrics were displaying the yAxis as a percentage when we want the actual count/seconds etc <img width="576" height="215" alt="image" src="https://github.com/user-attachments/assets/8238ba08-f572-428a-8917-8beab169ba8f" />
1 parent e5271c5 commit 370834f

File tree

2 files changed

+24
-11
lines changed

2 files changed

+24
-11
lines changed

static/app/views/detectors/utils/detectorChartFormatting.spec.tsx

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,14 +23,27 @@ describe('getDetectorChartFormatters', () => {
2323
expect(result.formatTooltipValue(2000)).toBe('2.00s');
2424
});
2525

26-
it('percent detection: returns percentage type and % suffix', () => {
26+
it('percent detection: uses aggregate output type and does not append % to formatters', () => {
2727
const result = getDetectorChartFormatters({
2828
detectionType: 'percent',
2929
aggregate: 'count()',
3030
});
3131

32-
expect(result.outputType).toBe('percentage');
32+
// % change detection should use the aggregate's actual type (number), not percentage
33+
expect(result.outputType).toBe('number');
34+
// unitSuffix is still % for threshold display purposes
3335
expect(result.unitSuffix).toBe('%');
34-
expect(result.formatTooltipValue(0.25)).toBe('25%');
36+
// But formatters should NOT append % since primary series shows actual metric values
37+
expect(result.formatTooltipValue(1000)).toBe('1,000');
38+
expect(result.formatYAxisLabel(1000)).toBe('1k');
39+
});
40+
41+
it('percentage aggregate (crash_free_rate): returns percentage type', () => {
42+
const result = getDetectorChartFormatters({
43+
detectionType: 'static',
44+
aggregate: 'crash_free_rate(session)',
45+
});
46+
47+
expect(result.outputType).toBe('percentage');
3548
});
3649
});

static/app/views/detectors/utils/detectorChartFormatting.tsx

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,22 +17,22 @@ export function getDetectorChartFormatters({
1717
detectionType,
1818
aggregate,
1919
}: DetectorChartFormatterOptions) {
20-
const outputType =
21-
detectionType === 'percent' ? 'percentage' : aggregateOutputType(aggregate);
20+
const outputType = aggregateOutputType(aggregate);
2221
const unitSuffix = getMetricDetectorSuffix(detectionType, aggregate);
2322

23+
// For % change detection, the primary series shows actual metric values (counts, durations, etc.)
24+
// The % suffix is only for threshold display, not for y-axis/tooltip formatting
25+
const shouldAppendSuffix =
26+
detectionType !== 'percent' && (outputType === 'number' || outputType === 'integer');
27+
2428
const formatYAxisLabel = (value: number): string => {
2529
const base = axisLabelFormatterUsingAggregateOutputType(value, outputType, true);
26-
return outputType === 'number' || outputType === 'integer'
27-
? `${base}${unitSuffix}`
28-
: base;
30+
return shouldAppendSuffix ? `${base}${unitSuffix}` : base;
2931
};
3032

3133
const formatTooltipValue = (value: number): string => {
3234
const base = tooltipFormatterUsingAggregateOutputType(value, outputType);
33-
return outputType === 'number' || outputType === 'integer'
34-
? `${base}${unitSuffix}`
35-
: base;
35+
return shouldAppendSuffix ? `${base}${unitSuffix}` : base;
3636
};
3737

3838
return {

0 commit comments

Comments
 (0)