From fb59574bd3d1f1f565da2bf6e5ce4e9379803892 Mon Sep 17 00:00:00 2001 From: cauemarcondes Date: Tue, 7 Apr 2020 14:30:23 +0200 Subject: [PATCH 1/2] changing duration min value for span_frames_min_duration --- .../agent_configuration/amount_and_unit.ts | 3 +- .../runtime_types/duration_rt.test.ts | 44 ++++++++++++++++--- .../runtime_types/duration_rt.ts | 44 ++++++++++--------- .../__snapshots__/index.test.ts.snap | 1 + .../setting_definitions/general_settings.ts | 5 ++- .../setting_definitions/types.d.ts | 1 + 6 files changed, 70 insertions(+), 28 deletions(-) diff --git a/x-pack/plugins/apm/common/agent_configuration/amount_and_unit.ts b/x-pack/plugins/apm/common/agent_configuration/amount_and_unit.ts index 447e529c9c199..d6520ae150539 100644 --- a/x-pack/plugins/apm/common/agent_configuration/amount_and_unit.ts +++ b/x-pack/plugins/apm/common/agent_configuration/amount_and_unit.ts @@ -10,7 +10,8 @@ interface AmountAndUnit { } export function amountAndUnitToObject(value: string): AmountAndUnit { - const [, amount = '', unit = ''] = value.match(/(\d+)?(\w+)?/) || []; + // matches any postive and negative number and its unit. + const [, amount = '', unit = ''] = value.match(/(^-?\d+)?(\w+)?/) || []; return { amount, unit }; } diff --git a/x-pack/plugins/apm/common/agent_configuration/runtime_types/duration_rt.test.ts b/x-pack/plugins/apm/common/agent_configuration/runtime_types/duration_rt.test.ts index a83ee9262cad6..98d0cb5f028c3 100644 --- a/x-pack/plugins/apm/common/agent_configuration/runtime_types/duration_rt.test.ts +++ b/x-pack/plugins/apm/common/agent_configuration/runtime_types/duration_rt.test.ts @@ -10,24 +10,56 @@ * you may not use this file except in compliance with the Elastic License. */ -import { durationRt } from './duration_rt'; +import { durationRt, getDurationRt } from './duration_rt'; import { isRight } from 'fp-ts/lib/Either'; describe('durationRt', () => { describe('it should not accept', () => { - [undefined, null, '', 0, 'foo', true, false, '100', 's', 'm', '0h'].map( + [ + undefined, + null, + '', + 0, + 'foo', + true, + false, + '100', + 's', + 'm', + '0ms', + '-1ms' + ].map(input => { + it(`${JSON.stringify(input)}`, () => { + expect(isRight(durationRt.decode(input))).toBe(false); + }); + }); + }); + + describe('it should accept', () => { + ['1000ms', '2s', '3m', '1s'].map(input => { + it(`${JSON.stringify(input)}`, () => { + expect(isRight(durationRt.decode(input))).toBe(true); + }); + }); + }); +}); + +describe('getDurationRt', () => { + const customDurationRt = getDurationRt({ min: -1 }); + describe('it should not accept', () => { + [undefined, null, '', 0, 'foo', true, false, '100', 's', 'm', '-2ms'].map( input => { it(`${JSON.stringify(input)}`, () => { - expect(isRight(durationRt.decode(input))).toBe(false); + expect(isRight(customDurationRt.decode(input))).toBe(false); }); } ); }); - describe('It should accept', () => { - ['1000ms', '2s', '3m'].map(input => { + describe('it should accept', () => { + ['1000ms', '2s', '3m', '1s', '-1s', '0ms'].map(input => { it(`${JSON.stringify(input)}`, () => { - expect(isRight(durationRt.decode(input))).toBe(true); + expect(isRight(customDurationRt.decode(input))).toBe(true); }); }); }); diff --git a/x-pack/plugins/apm/common/agent_configuration/runtime_types/duration_rt.ts b/x-pack/plugins/apm/common/agent_configuration/runtime_types/duration_rt.ts index 383fd69be9a78..b691276854fb0 100644 --- a/x-pack/plugins/apm/common/agent_configuration/runtime_types/duration_rt.ts +++ b/x-pack/plugins/apm/common/agent_configuration/runtime_types/duration_rt.ts @@ -10,24 +10,28 @@ import { amountAndUnitToObject } from '../amount_and_unit'; export const DURATION_UNITS = ['ms', 's', 'm']; -export const durationRt = new t.Type( - 'durationRt', - t.string.is, - (input, context) => { - return either.chain(t.string.validate(input, context), inputAsString => { - const { amount, unit } = amountAndUnitToObject(inputAsString); - const amountAsInt = parseInt(amount, 10); - const isValidUnit = DURATION_UNITS.includes(unit); - const isValid = amountAsInt > 0 && isValidUnit; +export function getDurationRt({ min }: { min: number }) { + return new t.Type( + 'durationRt', + t.string.is, + (input, context) => { + return either.chain(t.string.validate(input, context), inputAsString => { + const { amount, unit } = amountAndUnitToObject(inputAsString); + const amountAsInt = parseInt(amount, 10); + const isValidUnit = DURATION_UNITS.includes(unit); + const isValid = amountAsInt >= min && isValidUnit; - return isValid - ? t.success(inputAsString) - : t.failure( - input, - context, - `Must have numeric amount and a valid unit (${DURATION_UNITS})` - ); - }); - }, - t.identity -); + return isValid + ? t.success(inputAsString) + : t.failure( + input, + context, + `Must have numeric amount and a valid unit (${DURATION_UNITS})` + ); + }); + }, + t.identity + ); +} + +export const durationRt = getDurationRt({ min: 1 }); diff --git a/x-pack/plugins/apm/common/agent_configuration/setting_definitions/__snapshots__/index.test.ts.snap b/x-pack/plugins/apm/common/agent_configuration/setting_definitions/__snapshots__/index.test.ts.snap index 4b74b07fc8e27..bc435179762a2 100644 --- a/x-pack/plugins/apm/common/agent_configuration/setting_definitions/__snapshots__/index.test.ts.snap +++ b/x-pack/plugins/apm/common/agent_configuration/setting_definitions/__snapshots__/index.test.ts.snap @@ -118,6 +118,7 @@ Array [ }, Object { "key": "span_frames_min_duration", + "min": -1, "type": "duration", "units": Array [ "ms", diff --git a/x-pack/plugins/apm/common/agent_configuration/setting_definitions/general_settings.ts b/x-pack/plugins/apm/common/agent_configuration/setting_definitions/general_settings.ts index 152db37a1bff3..f1d11c5c70c2b 100644 --- a/x-pack/plugins/apm/common/agent_configuration/setting_definitions/general_settings.ts +++ b/x-pack/plugins/apm/common/agent_configuration/setting_definitions/general_settings.ts @@ -8,6 +8,7 @@ import { i18n } from '@kbn/i18n'; import { getIntegerRt } from '../runtime_types/integer_rt'; import { captureBodyRt } from '../runtime_types/capture_body_rt'; import { RawSettingDefinition } from './types'; +import { getDurationRt } from '../runtime_types/duration_rt'; /* * Settings added here will show up in the UI and will be validated on the client and server @@ -143,6 +144,7 @@ export const generalSettings: RawSettingDefinition[] = [ { key: 'span_frames_min_duration', type: 'duration', + validation: getDurationRt({ min: -1 }), defaultValue: '5ms', label: i18n.translate('xpack.apm.agentConfig.spanFramesMinDuration.label', { defaultMessage: 'Span frames minimum duration' @@ -154,7 +156,8 @@ export const generalSettings: RawSettingDefinition[] = [ 'In its default settings, the APM agent will collect a stack trace with every recorded span.\nWhile this is very helpful to find the exact place in your code that causes the span, collecting this stack trace does have some overhead. \nWhen setting this option to a negative value, like `-1ms`, stack traces will be collected for all spans. Setting it to a positive value, e.g. `5ms`, will limit stack trace collection to spans with durations equal to or longer than the given value, e.g. 5 milliseconds.\n\nTo disable stack trace collection for spans completely, set the value to `0ms`.' } ), - excludeAgents: ['js-base', 'rum-js', 'nodejs'] + excludeAgents: ['js-base', 'rum-js', 'nodejs'], + min: -1 }, // STACK_TRACE_LIMIT diff --git a/x-pack/plugins/apm/common/agent_configuration/setting_definitions/types.d.ts b/x-pack/plugins/apm/common/agent_configuration/setting_definitions/types.d.ts index 6b584fc7e2048..282ced346dda0 100644 --- a/x-pack/plugins/apm/common/agent_configuration/setting_definitions/types.d.ts +++ b/x-pack/plugins/apm/common/agent_configuration/setting_definitions/types.d.ts @@ -91,6 +91,7 @@ interface BytesSetting extends BaseSetting { interface DurationSetting extends BaseSetting { type: 'duration'; units?: string[]; + min?: number; } export type RawSettingDefinition = From 43219927ae6b25a79c789637a5f747f2753d2864 Mon Sep 17 00:00:00 2001 From: cauemarcondes Date: Tue, 7 Apr 2020 17:53:37 +0200 Subject: [PATCH 2/2] adding min property to number field --- .../AgentConfigurationCreateEdit/SettingsPage/SettingFormRow.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/x-pack/legacy/plugins/apm/public/components/app/Settings/AgentConfigurations/AgentConfigurationCreateEdit/SettingsPage/SettingFormRow.tsx b/x-pack/legacy/plugins/apm/public/components/app/Settings/AgentConfigurations/AgentConfigurationCreateEdit/SettingsPage/SettingFormRow.tsx index b1959e4d68aa4..30c772bf5f634 100644 --- a/x-pack/legacy/plugins/apm/public/components/app/Settings/AgentConfigurations/AgentConfigurationCreateEdit/SettingsPage/SettingFormRow.tsx +++ b/x-pack/legacy/plugins/apm/public/components/app/Settings/AgentConfigurations/AgentConfigurationCreateEdit/SettingsPage/SettingFormRow.tsx @@ -90,6 +90,7 @@ function FormRow({ onChange( setting.key,