From e62cae24dfeec8e36e3e2d46b2c50e728dfac3b0 Mon Sep 17 00:00:00 2001 From: cauemarcondes Date: Mon, 5 Oct 2020 13:51:55 +0200 Subject: [PATCH] addressing pr comments --- .../apm/common/utils/formatters/duration.ts | 10 +----- .../utils/formatters/formatters.test.ts | 31 ++++++++++++++----- .../apm/common/utils/formatters/formatters.ts | 8 +++++ ...ister_transaction_error_rate_alert_type.ts | 5 ++- 4 files changed, 35 insertions(+), 19 deletions(-) diff --git a/x-pack/plugins/apm/common/utils/formatters/duration.ts b/x-pack/plugins/apm/common/utils/formatters/duration.ts index 8381b0afb5f07..c0a99e0152fa7 100644 --- a/x-pack/plugins/apm/common/utils/formatters/duration.ts +++ b/x-pack/plugins/apm/common/utils/formatters/duration.ts @@ -8,7 +8,7 @@ import { i18n } from '@kbn/i18n'; import moment from 'moment'; import { memoize } from 'lodash'; import { NOT_AVAILABLE_LABEL } from '../../../common/i18n'; -import { asDecimal, asInteger } from './formatters'; +import { asDecimalOrInteger, asInteger } from './formatters'; import { TimeUnit } from './datetime'; import { Maybe } from '../../../typings/common'; @@ -31,14 +31,6 @@ export type TimeFormatter = ( type TimeFormatterBuilder = (max: number) => TimeFormatter; -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 diff --git a/x-pack/plugins/apm/common/utils/formatters/formatters.test.ts b/x-pack/plugins/apm/common/utils/formatters/formatters.test.ts index ce317c5a6a827..5583cf9703bc4 100644 --- a/x-pack/plugins/apm/common/utils/formatters/formatters.test.ts +++ b/x-pack/plugins/apm/common/utils/formatters/formatters.test.ts @@ -3,33 +3,50 @@ * or more contributor license agreements. Licensed under the Elastic License; * you may not use this file except in compliance with the Elastic License. */ -import { asPercent } from './formatters'; +import { asPercent, asDecimalOrInteger } from './formatters'; describe('formatters', () => { describe('asPercent', () => { - it('should format as integer when number is above 10', () => { + it('formats as integer when number is above 10', () => { expect(asPercent(3725, 10000, 'n/a')).toEqual('37%'); }); - it('should add a decimal when value is below 10', () => { + it('adds a decimal when value is below 10', () => { expect(asPercent(0.092, 1)).toEqual('9.2%'); }); - it('should format when numerator is 0', () => { + it('formats when numerator is 0', () => { expect(asPercent(0, 1, 'n/a')).toEqual('0%'); }); - it('should return fallback when denominator is undefined', () => { + it('returns fallback when denominator is undefined', () => { expect(asPercent(3725, undefined, 'n/a')).toEqual('n/a'); }); - it('should return fallback when denominator is 0 ', () => { + it('returns fallback when denominator is 0 ', () => { expect(asPercent(3725, 0, 'n/a')).toEqual('n/a'); }); - it('should return fallback when numerator or denominator is NaN', () => { + it('returns fallback when numerator or denominator is NaN', () => { expect(asPercent(3725, NaN, 'n/a')).toEqual('n/a'); expect(asPercent(NaN, 10000, 'n/a')).toEqual('n/a'); }); }); + + 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)).toEqual('10'); + expect(asDecimalOrInteger(15)).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'); + }); + }); }); diff --git a/x-pack/plugins/apm/common/utils/formatters/formatters.ts b/x-pack/plugins/apm/common/utils/formatters/formatters.ts index 649f11063b149..d84bf86d0de2f 100644 --- a/x-pack/plugins/apm/common/utils/formatters/formatters.ts +++ b/x-pack/plugins/apm/common/utils/formatters/formatters.ts @@ -44,3 +44,11 @@ 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) { + return asInteger(value); + } + return asDecimal(value); +} diff --git a/x-pack/plugins/apm/server/lib/alerts/register_transaction_error_rate_alert_type.ts b/x-pack/plugins/apm/server/lib/alerts/register_transaction_error_rate_alert_type.ts index 7bb9894a837f5..969f4ceaca93a 100644 --- a/x-pack/plugins/apm/server/lib/alerts/register_transaction_error_rate_alert_type.ts +++ b/x-pack/plugins/apm/server/lib/alerts/register_transaction_error_rate_alert_type.ts @@ -8,6 +8,7 @@ import { schema } from '@kbn/config-schema'; import { Observable } from 'rxjs'; import { take } from 'rxjs/operators'; import { isEmpty } from 'lodash'; +import { asDecimalOrInteger } from '../../../common/utils/formatters'; import { ProcessorEvent } from '../../../common/processor_event'; import { EventOutcome } from '../../../common/event_outcome'; import { AlertType, ALERT_TYPES_CONFIG } from '../../../common/alert_types'; @@ -171,9 +172,7 @@ export function registerTransactionErrorRateAlertType({ transactionType, environment, threshold: alertParams.threshold, - triggerValue: String( - parseFloat(transactionErrorRate.toPrecision(3)) - ), + triggerValue: asDecimalOrInteger(transactionErrorRate), interval: `${alertParams.windowSize}${alertParams.windowUnit}`, }); }