[APM] Only add decimals for numbers below 10 (#69334)

elastic · Jun 17, 2020 · 8a6aab3 · 8a6aab3
1 parent 3ee0bf2
commit 8a6aab3
Show file tree

Hide file tree

Showing 8 changed files with 102 additions and 79 deletions.
diff --git a/x-pack/plugins/apm/e2e/cypress/integration/snapshots.js b/x-pack/plugins/apm/e2e/cypress/integration/snapshots.js
@@ -1,9 +1,9 @@
 module.exports = {
   APM: {
     'Transaction duration charts': {
-      '1': '350.0 ms',
-      '2': '175.0 ms',
-      '3': '0.0 ms',
+      '1': '350 ms',
+      '2': '175 ms',
+      '3': '0 ms',
     },
   },
   __version: '4.5.0',

diff --git a/x-pack/plugins/apm/public/components/app/ServiceOverview/ServiceList/__test__/List.test.js b/x-pack/plugins/apm/public/components/app/ServiceOverview/ServiceList/__test__/List.test.js
@@ -40,7 +40,7 @@ describe('ServiceOverview -> List', () => {
     expect(renderedColumns[0]).toMatchSnapshot();
     expect(renderedColumns.slice(2)).toEqual([
       'python',
-      '91.5 ms',
+      '92 ms',
       '86.9 tpm',
       '12.6 err.',
     ]);

diff --git a/...apm/public/components/shared/charts/CustomPlot/test/__snapshots__/CustomPlot.test.js.snap b/...apm/public/components/shared/charts/CustomPlot/test/__snapshots__/CustomPlot.test.js.snap
diff --git a/x-pack/plugins/apm/public/components/shared/charts/Histogram/__test__/Histogram.test.js b/x-pack/plugins/apm/public/components/shared/charts/Histogram/__test__/Histogram.test.js
@@ -11,8 +11,8 @@ import d3 from 'd3';
 import { HistogramInner } from '../index';
 import response from './response.json';
 import {
-  asDecimal,
   getDurationFormatter,
+  asInteger,
 } from '../../../../../utils/formatters';
 import { toJson } from '../../../../../utils/testHelpers';
 import { getFormattedBuckets } from '../../../../app/TransactionDetails/Distribution/index';
@@ -33,8 +33,8 @@ describe('Histogram', () => {
         transactionId="myTransactionId"
         onClick={onClick}
         formatX={(time) => timeFormatter(time).formatted}
-        formatYShort={(t) => `${asDecimal(t)} occ.`}
-        formatYLong={(t) => `${asDecimal(t)} occurrences`}
+        formatYShort={(t) => `${asInteger(t)} occ.`}
+        formatYLong={(t) => `${asInteger(t)} occurrences`}
         tooltipHeader={(bucket) => {
           const xFormatted = timeFormatter(bucket.x);
           const x0Formatted = timeFormatter(bucket.x0);
@@ -78,9 +78,9 @@ describe('Histogram', () => {
       const tooltips = wrapper.find('Tooltip');
 
       expect(tooltips.length).toBe(1);
-      expect(tooltips.prop('header')).toBe('811.1 - 926.9 ms');
+      expect(tooltips.prop('header')).toBe('811 - 927 ms');
       expect(tooltips.prop('tooltipPoints')).toEqual([
-        { value: '49.0 occurrences' },
+        { value: '49 occurrences' },
       ]);
       expect(tooltips.prop('x')).toEqual(869010);
       expect(tooltips.prop('y')).toEqual(27.5);

diff --git a/...m/public/components/shared/charts/Histogram/__test__/__snapshots__/Histogram.test.js.snap b/...m/public/components/shared/charts/Histogram/__test__/__snapshots__/Histogram.test.js.snap
diff --git a/x-pack/plugins/apm/public/components/shared/charts/Timeline/Marker/ErrorMarker.test.tsx b/x-pack/plugins/apm/public/components/shared/charts/Timeline/Marker/ErrorMarker.test.tsx
@@ -37,7 +37,7 @@ describe('ErrorMarker', () => {
     act(() => {
       fireEvent.click(component.getByTestId('popover'));
     });
-    expectTextsInDocument(component, ['10.0 ms']);
+    expectTextsInDocument(component, ['10 ms']);
     return component;
   }
   function getKueryDecoded(url: string) {

diff --git a/x-pack/plugins/apm/public/utils/formatters/__test__/duration.test.ts b/x-pack/plugins/apm/public/utils/formatters/__test__/duration.test.ts
@@ -14,14 +14,14 @@ describe('duration formatters', () => {
       expect(asDuration(1)).toEqual('1 μs');
       expect(asDuration(toMicroseconds(1, 'milliseconds'))).toEqual('1,000 μs');
       expect(asDuration(toMicroseconds(1000, 'milliseconds'))).toEqual(
-        '1,000.0 ms'
+        '1,000 ms'
       );
       expect(asDuration(toMicroseconds(10000, 'milliseconds'))).toEqual(
-        '10,000.0 ms'
+        '10,000 ms'
       );
-      expect(asDuration(toMicroseconds(20, 'seconds'))).toEqual('20.0 s');
-      expect(asDuration(toMicroseconds(10, 'minutes'))).toEqual('10.0 min');
-      expect(asDuration(toMicroseconds(1, 'hours'))).toEqual('60.0 min');
+      expect(asDuration(toMicroseconds(20, 'seconds'))).toEqual('20 s');
+      expect(asDuration(toMicroseconds(10, 'minutes'))).toEqual('10 min');
+      expect(asDuration(toMicroseconds(1, 'hours'))).toEqual('60 min');
       expect(asDuration(toMicroseconds(1.5, 'hours'))).toEqual('1.5 h');
     });
 
@@ -41,7 +41,7 @@ describe('duration formatters', () => {
 
   describe('asMilliseconds', () => {
     it('converts to formatted decimal milliseconds', () => {
-      expect(asMillisecondDuration(0)).toEqual('0.0 ms');
+      expect(asMillisecondDuration(0)).toEqual('0 ms');
     });
   });
 });
diff --git a/x-pack/plugins/apm/public/utils/formatters/duration.ts b/x-pack/plugins/apm/public/utils/formatters/duration.ts
@@ -18,13 +18,6 @@ interface FormatterOptions {
 
 type DurationTimeUnit = TimeUnit | 'microseconds';
 
-interface DurationUnit {
-  [unit: string]: {
-    label: string;
-    convert: (value: number) => string;
-  };
-}
-
 interface ConvertedDuration {
   value: string;
   unit?: string;
@@ -38,42 +31,69 @@ export type TimeFormatter = (
 
 type TimeFormatterBuilder = (max: number) => TimeFormatter;
 
-const durationUnit: DurationUnit = {
-  hours: {
-    label: i18n.translate('xpack.apm.formatters.hoursTimeUnitLabel', {
-      defaultMessage: 'h',
-    }),
-    convert: (value: number) =>
-      asDecimal(moment.duration(value / 1000).asHours()),
-  },
-  minutes: {
-    label: i18n.translate('xpack.apm.formatters.minutesTimeUnitLabel', {
-      defaultMessage: 'min',
-    }),
-    convert: (value: number) =>
-      asDecimal(moment.duration(value / 1000).asMinutes()),
-  },
-  seconds: {
-    label: i18n.translate('xpack.apm.formatters.secondsTimeUnitLabel', {
-      defaultMessage: 's',
-    }),
-    convert: (value: number) =>
-      asDecimal(moment.duration(value / 1000).asSeconds()),
-  },
-  milliseconds: {
-    label: i18n.translate('xpack.apm.formatters.millisTimeUnitLabel', {
-      defaultMessage: 'ms',
-    }),
-    convert: (value: number) =>
-      asDecimal(moment.duration(value / 1000).asMilliseconds()),
-  },
-  microseconds: {
-    label: i18n.translate('xpack.apm.formatters.microsTimeUnitLabel', {
-      defaultMessage: 'μs',
-    }),
-    convert: (value: number) => asInteger(value),
-  },
-};
+function asDecimalOrInteger(value: number) {
+  // exact 0 or above 10 should not have decimal
+  if (value === 0 || value >= 10) {
+    return asInteger(value);
+  }
+  return asDecimal(value);
+}
+
+function getUnitLabelAndConvertedValue(
+  unitKey: DurationTimeUnit,
+  value: number
+) {
+  switch (unitKey) {
+    case 'hours': {
+      return {
+        unitLabel: i18n.translate('xpack.apm.formatters.hoursTimeUnitLabel', {
+          defaultMessage: 'h',
+        }),
+        convertedValue: asDecimalOrInteger(
+          moment.duration(value / 1000).asHours()
+        ),
+      };
+    }
+    case 'minutes': {
+      return {
+        unitLabel: i18n.translate('xpack.apm.formatters.minutesTimeUnitLabel', {
+          defaultMessage: 'min',
+        }),
+        convertedValue: asDecimalOrInteger(
+          moment.duration(value / 1000).asMinutes()
+        ),
+      };
+    }
+    case 'seconds': {
+      return {
+        unitLabel: i18n.translate('xpack.apm.formatters.secondsTimeUnitLabel', {
+          defaultMessage: 's',
+        }),
+        convertedValue: asDecimalOrInteger(
+          moment.duration(value / 1000).asSeconds()
+        ),
+      };
+    }
+    case 'milliseconds': {
+      return {
+        unitLabel: i18n.translate('xpack.apm.formatters.millisTimeUnitLabel', {
+          defaultMessage: 'ms',
+        }),
+        convertedValue: asDecimalOrInteger(
+          moment.duration(value / 1000).asMilliseconds()
+        ),
+      };
+    }
+    case 'microseconds': {
+      return {
+        unitLabel: i18n.translate('xpack.apm.formatters.microsTimeUnitLabel', {
+          defaultMessage: 'μs',
+        }),
+        convertedValue: asInteger(value),
+      };
+    }
+  }
+}
 
 /**
  * Converts a microseconds value into the unit defined.
@@ -87,16 +107,19 @@ function convertTo({
   microseconds: Maybe<number>;
   defaultValue?: string;
 }): ConvertedDuration {
-  const duration = durationUnit[unit];
-  if (!duration || microseconds == null) {
+  if (microseconds == null) {
     return { value: defaultValue, formatted: defaultValue };
   }
 
-  const convertedValue = duration.convert(microseconds);
+  const { convertedValue, unitLabel } = getUnitLabelAndConvertedValue(
+    unit,
+    microseconds
+  );
+
   return {
     value: convertedValue,
-    unit: duration.label,
-    formatted: `${convertedValue} ${duration.label}`,
+    unit: unitLabel,
+    formatted: `${convertedValue} ${unitLabel}`,
   };
 }