Browse Source

feat(perf): Improve `MetricReadout` behaviour with small numbers (#69307)

Percentages and rates are susceptible to bad rounding errors. e.g., if a
percentage is very small like `0.0000123`, the rendered percentage will
be `"0%"` but that's wrong! It's not _really_ 0, is it?

A `minimumValue` option makes it so that if the number is small, it
shows up as `<0.01%` which is closer to the truth.
George Gritsouk 10 months ago
parent
commit
3eb3934089

+ 8 - 0
static/app/utils/formatters.spec.tsx

@@ -340,6 +340,14 @@ describe('formatPercentage()', function () {
     expect(formatPercentage(0.10513494, 3)).toBe('10.513%');
     expect(formatPercentage(0.10513494, 4)).toBe('10.5135%');
   });
+
+  it('obeys a minimum value option', () => {
+    expect(formatPercentage(0.0101, 0, {minimumValue: 0.01})).toBe('1%');
+    expect(formatPercentage(0.01, 0, {minimumValue: 0.001})).toBe('1%');
+    expect(formatPercentage(0.0001, 0, {minimumValue: 0.001})).toBe('<0.1%');
+    expect(formatPercentage(-0.0001, 0, {minimumValue: 0.001})).toBe('<0.1%');
+    expect(formatPercentage(0.00000234, 0, {minimumValue: 0.0001})).toBe('<0.01%');
+  });
 });
 
 describe('userDisplayName', function () {

+ 14 - 1
static/app/utils/formatters.tsx

@@ -373,10 +373,23 @@ export function formatFloat(number: number, places: number) {
 /**
  * Format a value between 0 and 1 as a percentage
  */
-export function formatPercentage(value: number, places: number = 2) {
+export function formatPercentage(
+  value: number,
+  places: number = 2,
+  options: {
+    minimumValue?: number;
+  } = {}
+) {
   if (value === 0) {
     return '0%';
   }
+
+  const minimumValue = options.minimumValue ?? 0;
+
+  if (Math.abs(value) <= minimumValue) {
+    return `<${minimumValue * 100}%`;
+  }
+
   return (
     round(value * 100, places).toLocaleString(undefined, {
       maximumFractionDigits: places,

+ 14 - 0
static/app/views/performance/metricReadout.spec.tsx

@@ -41,6 +41,13 @@ describe('MetricReadout', function () {
     expect(screen.getByText('17.8/min')).toBeInTheDocument();
   });
 
+  it('limits smallest rate', () => {
+    render(<MetricReadout title="Rate" unit={RateUnit.PER_MINUTE} value={0.0002441} />);
+
+    expect(screen.getByRole('heading', {name: 'Rate'})).toBeInTheDocument();
+    expect(screen.getByText('<0.01/min')).toBeInTheDocument();
+  });
+
   it('renders milliseconds', () => {
     render(
       <MetricReadout title="Duration" unit={DurationUnit.MILLISECOND} value={223142123} />
@@ -64,6 +71,13 @@ describe('MetricReadout', function () {
     expect(screen.getByText('23.52%')).toBeInTheDocument();
   });
 
+  it('limits smallest percentage', () => {
+    render(<MetricReadout title="Percentage" unit="percentage" value={0.000022317} />);
+
+    expect(screen.getByRole('heading', {name: 'Percentage'})).toBeInTheDocument();
+    expect(screen.getByText('<0.01%')).toBeInTheDocument();
+  });
+
   it('renders counts', () => {
     render(<MetricReadout title="Count" unit="count" value={7800123} />);
 

+ 11 - 2
static/app/views/performance/metricReadout.tsx

@@ -58,7 +58,9 @@ function ReadoutContent({unit, value, tooltip, align = 'right', isLoading}: Prop
   if (isARateUnit(unit)) {
     renderedValue = (
       <NumberContainer align={align}>
-        {formatRate(typeof value === 'string' ? parseFloat(value) : value, unit)}
+        {formatRate(typeof value === 'string' ? parseFloat(value) : value, unit, {
+          minimumValue: MINIMUM_RATE_VALUE,
+        })}
       </NumberContainer>
     );
   }
@@ -96,7 +98,11 @@ function ReadoutContent({unit, value, tooltip, align = 'right', isLoading}: Prop
   if (unit === 'percentage') {
     renderedValue = (
       <NumberContainer align={align}>
-        {formatPercentage(typeof value === 'string' ? parseFloat(value) : value)}
+        {formatPercentage(
+          typeof value === 'string' ? parseFloat(value) : value,
+          undefined,
+          {minimumValue: MINIMUM_PERCENTAGE_VALUE}
+        )}
       </NumberContainer>
     );
   }
@@ -114,6 +120,9 @@ function ReadoutContent({unit, value, tooltip, align = 'right', isLoading}: Prop
   return <NumberContainer align={align}>{renderedValue}</NumberContainer>;
 }
 
+const MINIMUM_RATE_VALUE = 0.01;
+const MINIMUM_PERCENTAGE_VALUE = 0.0001; // 0.01%
+
 const NumberContainer = styled('div')<{align: 'left' | 'right'}>`
   text-align: ${p => p.align};
   font-variant-numeric: tabular-nums;