[ML] APM Latency Correlations: Fix formatting of duration values for …

…log x axis and selection badge. (#109214) - Fix, clean up and unit tests for the log log charts x axis duration based ticks. This extends existing duration utilities to support a custom threshold to be able to fine tune the formatting for either single values with more detail or chart axis ticks where less detail is required. This is useful for log axis that cover a wider range of units. As can be seen in the screenshot, axis ticks will be formatted as full seconds from 1s onwards instead of 1,000 ms already up to 10 seconds. Because the threshold parameter is optional and defaults to 10, other uses of getDurationFormatter don't need to be changed. - Fixes the formatting of the selection badge.
elastic · Aug 20, 2021 · ca6bb4b · ca6bb4b
1 parent 9fb152a
commit ca6bb4b
Show file tree

Hide file tree

Showing 7 changed files with 172 additions and 70 deletions.
diff --git a/x-pack/plugins/apm/common/utils/formatters/duration.test.ts b/x-pack/plugins/apm/common/utils/formatters/duration.test.ts
@@ -5,7 +5,12 @@
  * 2.0.
  */
 
-import { asDuration, toMicroseconds, asMillisecondDuration } from './duration';
+import {
+  asDuration,
+  getDurationFormatter,
+  toMicroseconds,
+  asMillisecondDuration,
+} from './duration';
 
 describe('duration formatters', () => {
   describe('asDuration', () => {
@@ -35,6 +40,50 @@ describe('duration formatters', () => {
     });
   });
 
+  describe('getDurationFormatter', () => {
+    // Formatting with a default threshold of 10 for more detail for single values
+    it('formats correctly with defaults', () => {
+      expect(getDurationFormatter(987654)(987654).formatted).toEqual('988 ms');
+      expect(getDurationFormatter(1000000)(1000000).formatted).toEqual(
+        '1,000 ms'
+      );
+      expect(getDurationFormatter(1234567)(1234567).formatted).toEqual(
+        '1,235 ms'
+      );
+      expect(getDurationFormatter(9876543)(9876543).formatted).toEqual(
+        '9,877 ms'
+      );
+      expect(getDurationFormatter(10000000)(10000000).formatted).toEqual(
+        '10,000 ms'
+      );
+      expect(getDurationFormatter(12345678)(12345678).formatted).toEqual(
+        '12 s'
+      );
+    });
+
+    // Formatting useful for axis ticks with a lower threshold where less detail is sufficient
+    it('formats correctly with a threshold of 0.9999', () => {
+      expect(getDurationFormatter(987654, 0.9999)(987654).formatted).toEqual(
+        '988 ms'
+      );
+      expect(getDurationFormatter(1000000, 0.9999)(1000000).formatted).toEqual(
+        '1 s'
+      );
+      expect(getDurationFormatter(1234567, 0.9999)(1234567).formatted).toEqual(
+        '1 s'
+      );
+      expect(getDurationFormatter(9876543, 0.9999)(9876543).formatted).toEqual(
+        '10 s'
+      );
+      expect(
+        getDurationFormatter(10000000, 0.9999)(10000000).formatted
+      ).toEqual('10 s');
+      expect(
+        getDurationFormatter(12345678, 0.9999)(12345678).formatted
+      ).toEqual('12 s');
+    });
+  });
+
   describe('toMicroseconds', () => {
     it('transformes to microseconds', () => {
       expect(toMicroseconds(1, 'hours')).toEqual(3600000000);

diff --git a/x-pack/plugins/apm/common/utils/formatters/duration.ts b/x-pack/plugins/apm/common/utils/formatters/duration.ts
@@ -33,20 +33,25 @@ export type TimeFormatter = (
   options?: FormatterOptions
 ) => ConvertedDuration;
 
-type TimeFormatterBuilder = (max: number) => TimeFormatter;
+type TimeFormatterBuilder = (max: number, threshold?: number) => TimeFormatter;
 
-export function getUnitLabelAndConvertedValue(
+// threshold defines the value from which upwards there should be no decimal places.
+function getUnitLabelAndConvertedValue(
   unitKey: DurationTimeUnit,
-  value: number
+  value: number,
+  threshold: number = 10
 ) {
+  const ms = value / 1000;
+
   switch (unitKey) {
     case 'hours': {
       return {
         unitLabel: i18n.translate('xpack.apm.formatters.hoursTimeUnitLabel', {
           defaultMessage: 'h',
         }),
         convertedValue: asDecimalOrInteger(
-          moment.duration(value / 1000).asHours()
+          moment.duration(ms).asHours(),
+          threshold
         ),
       };
     }
@@ -56,7 +61,8 @@ export function getUnitLabelAndConvertedValue(
           defaultMessage: 'min',
         }),
         convertedValue: asDecimalOrInteger(
-          moment.duration(value / 1000).asMinutes()
+          moment.duration(ms).asMinutes(),
+          threshold
         ),
       };
     }
@@ -66,7 +72,8 @@ export function getUnitLabelAndConvertedValue(
           defaultMessage: 's',
         }),
         convertedValue: asDecimalOrInteger(
-          moment.duration(value / 1000).asSeconds()
+          moment.duration(ms).asSeconds(),
+          threshold
         ),
       };
     }
@@ -76,7 +83,8 @@ export function getUnitLabelAndConvertedValue(
           defaultMessage: 'ms',
         }),
         convertedValue: asDecimalOrInteger(
-          moment.duration(value / 1000).asMilliseconds()
+          moment.duration(ms).asMilliseconds(),
+          threshold
         ),
       };
     }
@@ -98,18 +106,21 @@ function convertTo({
   unit,
   microseconds,
   defaultValue = NOT_AVAILABLE_LABEL,
+  threshold = 10,
 }: {
   unit: DurationTimeUnit;
   microseconds: Maybe<number>;
   defaultValue?: string;
+  threshold?: number;
 }): ConvertedDuration {
   if (!isFiniteNumber(microseconds)) {
     return { value: defaultValue, formatted: defaultValue };
   }
 
   const { convertedValue, unitLabel } = getUnitLabelAndConvertedValue(
     unit,
-    microseconds
+    microseconds,
+    threshold
   );
 
   return {
@@ -122,10 +133,7 @@ function convertTo({
 export const toMicroseconds = (value: number, timeUnit: TimeUnit) =>
   moment.duration(value, timeUnit).asMilliseconds() * 1000;
 
-export function getDurationUnitKey(
-  max: number,
-  threshold = 10
-): DurationTimeUnit {
+function getDurationUnitKey(max: number, threshold = 10): DurationTimeUnit {
   if (max > toMicroseconds(threshold, 'hours')) {
     return 'hours';
   }
@@ -141,13 +149,16 @@ export function getDurationUnitKey(
   return 'microseconds';
 }
 
+// memoizer with a custom resolver to consider both arguments max/threshold.
+// by default lodash's memoize only considers the first argument.
 export const getDurationFormatter: TimeFormatterBuilder = memoize(
-  (max: number) => {
-    const unit = getDurationUnitKey(max);
-    return (value, { defaultValue }: FormatterOptions = {}) => {
-      return convertTo({ unit, microseconds: value, defaultValue });
+  (max: number, threshold: number = 10) => {
+    const unit = getDurationUnitKey(max, threshold);
+    return (value: Maybe<number>, { defaultValue }: FormatterOptions = {}) => {
+      return convertTo({ unit, microseconds: value, defaultValue, threshold });
     };
-  }
+  },
+  (max, threshold) => `${max}_${threshold}`
 );
 
 export function asTransactionRate(value: Maybe<number>) {

diff --git a/x-pack/plugins/apm/common/utils/formatters/formatters.test.ts b/x-pack/plugins/apm/common/utils/formatters/formatters.test.ts
@@ -36,19 +36,40 @@ describe('formatters', () => {
   });
 
   describe('asDecimalOrInteger', () => {
-    it('formats as integer when number equals to 0 ', () => {
-      expect(asDecimalOrInteger(0)).toEqual('0');
-    });
-    it('formats as integer when number is above or equals 10 ', () => {
-      expect(asDecimalOrInteger(10.123)).toEqual('10');
-      expect(asDecimalOrInteger(15.123)).toEqual('15');
-    });
-    it('formats as decimal when number is below 10 ', () => {
-      expect(asDecimalOrInteger(0.25435632645)).toEqual('0.3');
-      expect(asDecimalOrInteger(1)).toEqual('1.0');
-      expect(asDecimalOrInteger(3.374329704990765)).toEqual('3.4');
-      expect(asDecimalOrInteger(5)).toEqual('5.0');
-      expect(asDecimalOrInteger(9)).toEqual('9.0');
+    describe('with default threshold of 10', () => {
+      it('formats as integer when number equals to 0 ', () => {
+        expect(asDecimalOrInteger(0)).toEqual('0');
+      });
+      it('formats as integer when number is above or equals 10 ', () => {
+        expect(asDecimalOrInteger(10.123)).toEqual('10');
+        expect(asDecimalOrInteger(15.123)).toEqual('15');
+      });
+      it('formats as decimal when number is below 10 ', () => {
+        expect(asDecimalOrInteger(0.25435632645)).toEqual('0.3');
+        expect(asDecimalOrInteger(1)).toEqual('1.0');
+        expect(asDecimalOrInteger(3.374329704990765)).toEqual('3.4');
+        expect(asDecimalOrInteger(5)).toEqual('5.0');
+        expect(asDecimalOrInteger(9)).toEqual('9.0');
+      });
+    });
+
+    describe('with custom threshold of 1', () => {
+      it('formats as integer when number equals to 0 ', () => {
+        expect(asDecimalOrInteger(0, 1)).toEqual('0');
+      });
+      it('formats as integer when number is above or equals 1 ', () => {
+        expect(asDecimalOrInteger(1, 1)).toEqual('1');
+        expect(asDecimalOrInteger(1.123, 1)).toEqual('1');
+        expect(asDecimalOrInteger(3.374329704990765, 1)).toEqual('3');
+        expect(asDecimalOrInteger(5, 1)).toEqual('5');
+        expect(asDecimalOrInteger(9, 1)).toEqual('9');
+        expect(asDecimalOrInteger(10, 1)).toEqual('10');
+        expect(asDecimalOrInteger(10.123, 1)).toEqual('10');
+        expect(asDecimalOrInteger(15.123, 1)).toEqual('15');
+      });
+      it('formats as decimal when number is below 1 ', () => {
+        expect(asDecimalOrInteger(0.25435632645, 1)).toEqual('0.3');
+      });
     });
   });
 });
diff --git a/x-pack/plugins/apm/common/utils/formatters/formatters.ts b/x-pack/plugins/apm/common/utils/formatters/formatters.ts
@@ -55,9 +55,9 @@ export function asPercent(
   return numeral(decimal).format('0.0%');
 }
 
-export function asDecimalOrInteger(value: number) {
-  // exact 0 or above 10 should not have decimal
-  if (value === 0 || value >= 10) {
+export function asDecimalOrInteger(value: number, threshold = 10) {
+  // exact 0 or above threshold should not have decimal
+  if (value === 0 || value >= threshold) {
     return asInteger(value);
   }
   return asDecimal(value);

diff --git a/x-pack/plugins/apm/public/components/app/transaction_details/distribution/index.test.ts b/x-pack/plugins/apm/public/components/app/transaction_details/distribution/index.test.ts
@@ -0,0 +1,22 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0; you may not use this file except in compliance with the Elastic License
+ * 2.0.
+ */
+
+import { getFormattedSelection } from './index';
+
+describe('transaction_details/distribution', () => {
+  describe('getFormattedSelection', () => {
+    it('displays only one unit if from and to share the same unit', () => {
+      expect(getFormattedSelection([10000, 100000])).toEqual('10 - 100 ms');
+    });
+
+    it('displays two units when from and to have different units', () => {
+      expect(getFormattedSelection([100000, 1000000000])).toEqual(
+        '100 ms - 17 min'
+      );
+    });
+  });
+});
diff --git a/x-pack/plugins/apm/public/components/app/transaction_details/distribution/index.tsx b/x-pack/plugins/apm/public/components/app/transaction_details/distribution/index.tsx
@@ -17,6 +17,7 @@ import {
   EuiTitle,
 } from '@elastic/eui';
 import { i18n } from '@kbn/i18n';
+import { getDurationFormatter } from '../../../../../common/utils/formatters';
 import { useUrlParams } from '../../../../context/url_params_context/use_url_params';
 import { useApmPluginContext } from '../../../../context/apm_plugin/use_apm_plugin_context';
 import { useTransactionDistributionFetcher } from '../../../../hooks/use_transaction_distribution_fetcher';
@@ -28,11 +29,25 @@ import { isErrorMessage } from '../../correlations/utils/is_error_message';
 
 const DEFAULT_PERCENTILE_THRESHOLD = 95;
 
+type Selection = [number, number];
+
+// Format the selected latency range for the "Clear selection" badge.
+// If the two values share the same unit, it will only displayed once.
+// For example: 12 - 23 ms / 12 ms - 3 s
+export function getFormattedSelection(selection: Selection): string {
+  const from = getDurationFormatter(selection[0])(selection[0]);
+  const to = getDurationFormatter(selection[1])(selection[1]);
+
+  return `${from.unit === to.unit ? from.value : from.formatted} - ${
+    to.formatted
+  }`;
+}
+
 interface Props {
   markerCurrentTransaction?: number;
   onChartSelection: BrushEndListener;
   onClearSelection: () => void;
-  selection?: [number, number];
+  selection?: Selection;
 }
 
 export function TransactionDistribution({
@@ -177,10 +192,9 @@ export function TransactionDistribution({
               {i18n.translate(
                 'xpack.apm.transactionDetails.distribution.selectionText',
                 {
-                  defaultMessage: `Selection: {selectionFrom} - {selectionTo}ms`,
+                  defaultMessage: `Selection: {formattedSelection}`,
                   values: {
-                    selectionFrom: Math.round(selection[0] / 1000),
-                    selectionTo: Math.round(selection[1] / 1000),
+                    formattedSelection: getFormattedSelection(selection),
                   },
                 }
               )}