Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions static/app/utils/number/NUMBER_FORMATTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,10 @@ Each entry shows the function signature, the rounding/precision logic, and concr

[formatYAxisValue.tsx](static/app/views/dashboards/widgets/timeSeriesWidget/formatters/formatYAxisValue.tsx) · Integers → `formatAbbreviatedNumber`. Non-integers → `toLocaleString({maximumFractionDigits: 20})` (full precision, trusts ECharts to provide round values).

### `formatYAxisValue(value, 'number'/'integer', ...)`

[formatYAxisValue.tsx](static/app/views/dashboards/widgets/heatMapWidget/formatters/formatYAxisValue.tsx) · NOTE: This function is ONLY for HEAT MAPS! Integers → `formatAbbreviatedNumber`. Non-integers → `formatNumberWithDynamicDecimalPoints(value)` (ECharts treats heat map y-axis as categories so it will not do a great job at formatting and providing round values. Hence we are rounding them off ourselves).

### `formatTooltipValue(value, 'number'/'integer', ...)`

[formatTooltipValue.tsx](static/app/views/dashboards/widgets/timeSeriesWidget/formatters/formatTooltipValue.tsx) · `toLocaleString({maximumFractionDigits: 4})`. If `0 < value < 0.0001`: switches to `{maximumSignificantDigits: 4}` to avoid `"0.0000"`.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
import {formatYAxisValue} from './formatYAxisValue';

describe('formatYAxisValue', () => {
describe('integer', () => {
it.each([
[0, '0'],
[17, '17'],
[171, '171'],
[17111, '17K'],
[17_000_110, '17M'],
[1_000_110_000, '1B'],
])('Formats %s as %s', (value, formattedValue) => {
expect(formatYAxisValue(value, 'integer')).toEqual(formattedValue);
});
});

describe('number', () => {
it.each([
[0.000033452, '0.00003345'],
[0.00003, '0.00003'],
[17.1238, '17.12'],
[170, '170'],
[17111, '17K'],
[17_000_110, '17M'],
[1772313.1, '1,772,313.1'],
[1772313.11123, '1,772,313.11'],
])('Formats %s as %s', (value, formattedValue) => {
expect(formatYAxisValue(value, 'number')).toEqual(formattedValue);
});
});

describe('percentage', () => {
it.each([
[0, '0'],
[0.00005, '0.005%'],
[0.712, '71.2%'],
[17.123, '1,712.3%'],
[1, '100%'],
])('Formats %s as %s', (value, formattedValue) => {
expect(formatYAxisValue(value, 'percentage')).toEqual(formattedValue);
});
});

describe('duration', () => {
it.each([
[0, 'millisecond', '0'],
[0.712, 'second', '712ms'],
[1230, 'second', '20.5min'],
])('Formats %s as %s', (value, unit, formattedValue) => {
expect(formatYAxisValue(value, 'duration', unit)).toEqual(formattedValue);
});
});

describe('size', () => {
it.each([
[0, 'byte', '0'],
[0.712, 'megabyte', '712 KB'],
[1231, 'kibibyte', '1.2 MiB'],
])('Formats %s as %s', (value, unit, formattedValue) => {
expect(formatYAxisValue(value, 'size', unit)).toEqual(formattedValue);
});
});

describe('rate', () => {
it.each([
[0, '1/second', '0'],
[-3, '1/second', '-3/s'],
[0.712, '1/second', '0.712/s'],
[12700, '1/second', '12.7K/s'],
[0.0003, '1/second', '0.0003/s'],
[0.00000153, '1/second', '0.00000153/s'],
[0.35, '1/second', '0.35/s'],
[10, '1/second', '10/s'],
// eslint-disable-next-line unicorn/no-zero-fractions
[10.0, '1/second', '10/s'],
[1231, '1/minute', '1.231K/min'],
[110000, '1/second', '110K/s'],
[110001, '1/second', '110.001K/s'],
[123456789, '1/second', '123.457M/s'],
])('Formats %s as %s', (value, unit, formattedValue) => {
expect(formatYAxisValue(value, 'rate', unit)).toEqual(formattedValue);
});
});

describe('currency', () => {
it.each([
[0, '0'],
[17, '$17'],
[171, '$171'],
[17111, '$17.11K'],
[17_000_110, '$17M'],
[1_000_110_000, '$1B'],
])('Formats %s as %s', (value, formattedValue) => {
expect(formatYAxisValue(value, 'currency')).toEqual(formattedValue);
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
import {formatBytesBase2} from 'sentry/utils/bytes/formatBytesBase2';
import {formatBytesBase10} from 'sentry/utils/bytes/formatBytesBase10';
import {
ABYTE_UNITS,
DurationUnit,
RATE_UNIT_LABELS,
RateUnit,
SizeUnit,
} from 'sentry/utils/discover/fields';
import {formatAbbreviatedNumber, formatDollars} from 'sentry/utils/formatters';
import {formatNumberWithDynamicDecimalPoints} from 'sentry/utils/number/formatNumberWithDynamicDecimalPoints';
import {formatPercentage} from 'sentry/utils/number/formatPercentage';
import {convertDuration} from 'sentry/utils/unitConversion/convertDuration';
import {convertSize} from 'sentry/utils/unitConversion/convertSize';
import {
NUMBER_MIN_VALUE,
NUMBER_MAX_FRACTION_DIGITS,
} from 'sentry/views/dashboards/widgets/common/settings';
import {
isADurationUnit,
isARateUnit,
isASizeUnit,
} from 'sentry/views/dashboards/widgets/common/typePredicates';
import {formatYAxisDuration} from 'sentry/views/dashboards/widgets/timeSeriesWidget/formatters/formatYAxisDuration';

/**
* Format a value for the Y axis on an ECharts heat map graph.
*
* The values on the Y axis are chosen by ECharts. ECharts will automatically
* select, when possible, nice round values. With heat maps this is not the case.
* Since the Y axis in heat maps are considered categories to ECharts,
* We need to format the values ourselves to the precision we'd like to see,
* especially with floating point numbers.
*
* The rest of the logic is the same as the time series widget Y axis formatter
* (static/app/views/dashboards/widgets/timeSeriesWidget/formatters/formatYAxisValue.tsx).
*/
export function formatYAxisValue(value: number, type: string, unit?: string): string {
if (value === 0) {
return '0';
}

switch (type) {
case 'integer':
return formatAbbreviatedNumber(value);
case 'number':
if (Number.isInteger(value)) {
return formatAbbreviatedNumber(value);
}
if (value > 0 && value < NUMBER_MIN_VALUE) {
return value.toLocaleString(undefined, {
maximumSignificantDigits: NUMBER_MAX_FRACTION_DIGITS,
});
Comment thread
cursor[bot] marked this conversation as resolved.
}
return formatNumberWithDynamicDecimalPoints(value);
case 'percentage':
return formatPercentage(value, 3);
case 'duration': {
const durationUnit = isADurationUnit(unit) ? unit : DurationUnit.MILLISECOND;
const durationInMilliseconds = convertDuration(
value,
durationUnit,
DurationUnit.MILLISECOND
);
return formatYAxisDuration(durationInMilliseconds);
}
case 'size': {
const sizeUnit = isASizeUnit(unit) ? unit : SizeUnit.BYTE;
const sizeInBytes = convertSize(value, sizeUnit, SizeUnit.BYTE);

const formatter = ABYTE_UNITS.includes(unit ?? 'byte')
? formatBytesBase10
: formatBytesBase2;

return formatter(sizeInBytes);
}
case 'rate': {
// Always show rate in the original dataset's unit. If the unit is not
// appropriate, always convert the unit in the original dataset first.
// This way, named rate functions like `epm()` will be shows in per minute
// units
const rateUnit = isARateUnit(unit) ? unit : RateUnit.PER_SECOND;
return `${value.toLocaleString(undefined, {
notation: 'compact',
maximumSignificantDigits: 6,
})}${RATE_UNIT_LABELS[rateUnit]}`;
}
case 'currency': {
return formatDollars(value);
}
default:
return value.toString();
}
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Near-complete duplication of time series Y-axis formatter

Medium Severity

The new heat map formatYAxisValue is a near-verbatim copy of the time series formatYAxisValue — the 'integer', 'percentage', 'duration', 'size', 'rate', 'currency', and default cases are all identical line-by-line. Only the 'number' case differs. Any future bug fix or behavior change to these shared cases (e.g., duration or size formatting) would need to be applied to both files independently, risking silent divergence. The common logic could be extracted into a shared helper, with only the 'number' formatting strategy parameterized or overridden per widget type.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit b30133b. Configure here.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes but this is NEEDED

Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,10 @@ import {formatAbbreviatedNumber} from 'sentry/utils/formatters';
import {ECHARTS_MISSING_DATA_VALUE} from 'sentry/utils/timeSeries/timeSeriesItemToEChartsDataPoint';
import {useOrganization} from 'sentry/utils/useOrganization';
import {NO_PLOTTABLE_VALUES} from 'sentry/views/dashboards/widgets/common/settings';
import {formatYAxisValue} from 'sentry/views/dashboards/widgets/heatMapWidget/formatters/formatYAxisValue';
import {plottablesCanBeVisualized} from 'sentry/views/dashboards/widgets/plottablesCanBeVisualized';
import {formatTooltipValue} from 'sentry/views/dashboards/widgets/timeSeriesWidget/formatters/formatTooltipValue';
import {formatXAxisTimestamp} from 'sentry/views/dashboards/widgets/timeSeriesWidget/formatters/formatXAxisTimestamp';
import {formatYAxisValue} from 'sentry/views/dashboards/widgets/timeSeriesWidget/formatters/formatYAxisValue';
import {FALLBACK_TYPE} from 'sentry/views/dashboards/widgets/timeSeriesWidget/settings';
import {getExploreUrl, type GetExploreUrlArgs} from 'sentry/views/explore/utils';

Expand Down
Loading