Browse Source

ref(starfish): Show 3 significant digits for queries per minute try 2 (#54100)

This was previously done and merged in with
https://github.com/getsentry/sentry/pull/54033, but had to be reverted
with
https://github.com/getsentry/sentry/commit/dd75a2cb157fa61c247ccff97ba91f596968d2dd
because of
[JAVASCRIPT-2NGX](https://sentry.sentry.io/issues/4367379265/).

The fix for the Sentry issue was related to
`static/app/views/performance/transactionSummary/transactionVitals/vitalCard.tsx`,
which was blindly forwarding the echarts formatter args to
`formatAbbreviatedNumber`. When we introduced a new precision arg to
`formatAbbreviatedNumber`, the echarts arg was being used, which would
lead to unexpected values being provided to the function.

fixes JAVASCRIPT-2NGX
fixes JAVASCRIPT-2NGW
Abhijeet Prasad 1 year ago
parent
commit
9696656da3

+ 8 - 6
static/app/utils/discover/fieldRenderers.tsx

@@ -119,6 +119,8 @@ type FieldFormatters = {
 
 export type FieldTypes = keyof FieldFormatters;
 
+const DEFAULT_RATE_SIG_DIGITS = 3;
+
 const EmptyValueContainer = styled('span')`
   color: ${p => p.theme.gray300};
 `;
@@ -234,12 +236,12 @@ export const FIELD_FORMATTERS: FieldFormatters = {
     isSortable: true,
     renderFunc: (field, data, baggage) => {
       const {unit} = baggage ?? {};
-
-      return (
-        <NumberContainer>
-          {`${formatAbbreviatedNumber(data[field])}${unit ? RATE_UNIT_LABELS[unit] : ''}`}
-        </NumberContainer>
-      );
+      const renderedUnit = unit ? RATE_UNIT_LABELS[unit] : '';
+      const formattedNumber = `${formatAbbreviatedNumber(
+        data[field],
+        DEFAULT_RATE_SIG_DIGITS
+      )}${renderedUnit}`;
+      return <NumberContainer>{formattedNumber}</NumberContainer>;
     },
   },
   integer: {

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

@@ -202,6 +202,18 @@ describe('formatAbbreviatedNumber()', function () {
     expect(formatAbbreviatedNumber(1500)).toBe('1.5k');
     expect(formatAbbreviatedNumber(1213122)).toBe('1.2m');
   });
+
+  it('should round to set amount of significant digits', () => {
+    expect(formatAbbreviatedNumber(100.12, 3)).toBe('100');
+    expect(formatAbbreviatedNumber(199.99, 3)).toBe('200');
+    expect(formatAbbreviatedNumber(1500, 3)).toBe('1.5k');
+    expect(formatAbbreviatedNumber(1213122, 3)).toBe('1.21m');
+    expect(formatAbbreviatedNumber(1500000000000, 3)).toBe('1500b');
+
+    expect(formatAbbreviatedNumber('1249.23421', 3)).toBe('1.25k');
+    expect(formatAbbreviatedNumber('1239567891299', 3)).toBe('1240b');
+    expect(formatAbbreviatedNumber('158.80421626984128', 3)).toBe('159');
+  });
 });
 
 describe('formatFloat()', function () {

+ 21 - 5
static/app/utils/formatters.tsx

@@ -286,7 +286,16 @@ const numberFormats = [
   [1000, 'k'],
 ] as const;
 
-export function formatAbbreviatedNumber(number: number | string) {
+/**
+ * Formats a number to a string with a suffix
+ *
+ * @param number the number to format
+ * @param precision the number of significant digits to include
+ */
+export function formatAbbreviatedNumber(
+  number: number | string,
+  precision?: number
+): string {
   number = Number(number);
 
   let lookup: (typeof numberFormats)[number];
@@ -301,12 +310,19 @@ export function formatAbbreviatedNumber(number: number | string) {
       continue;
     }
 
-    return shortValue / 10 > 1 || !fitsBound
-      ? `${shortValue}${suffix}`
-      : `${formatFloat(number / suffixNum, 1)}${suffix}`;
+    const formattedNumber =
+      shortValue / 10 > 1 || !fitsBound
+        ? precision === undefined
+          ? shortValue
+          : parseFloat(shortValue.toPrecision(precision)).toString()
+        : formatFloat(number / suffixNum, precision || 1).toLocaleString(undefined, {
+            maximumSignificantDigits: precision,
+          });
+
+    return `${formattedNumber}${suffix}`;
   }
 
-  return number.toLocaleString();
+  return number.toLocaleString(undefined, {maximumSignificantDigits: precision});
 }
 
 export function formatRate(value: number, unit: RateUnits = RateUnits.PER_SECOND) {

+ 1 - 1
static/app/views/performance/landing/chart/histogramChart.tsx

@@ -171,7 +171,7 @@ export function Chart(props: ChartProps) {
     type: 'value' as const,
     axisLabel: {
       color: theme.chartLabel,
-      formatter: formatAbbreviatedNumber,
+      formatter: (value: number | string) => formatAbbreviatedNumber(value),
     },
   };
 

+ 1 - 1
static/app/views/performance/landing/utils.tsx

@@ -197,7 +197,7 @@ export const vitalCardDetails = (
     'tpm()': {
       title: t('Throughput'),
       tooltip: getTermHelp(organization, PerformanceTerm.THROUGHPUT),
-      formatter: formatAbbreviatedNumber,
+      formatter: (value: number | string) => formatAbbreviatedNumber(value),
     },
     'failure_rate()': {
       title: t('Failure Rate'),

+ 1 - 1
static/app/views/performance/transactionSummary/transactionVitals/vitalCard.tsx

@@ -294,7 +294,7 @@ class VitalCard extends Component<Props, State> {
       max,
       axisLabel: {
         color: theme.chartLabel,
-        formatter: formatAbbreviatedNumber,
+        formatter: (value: string | number) => formatAbbreviatedNumber(value),
       },
     };