From 5d0e9027a3753825b01e2035fe3680774557ec41 Mon Sep 17 00:00:00 2001 From: cauemarcondes Date: Mon, 5 Oct 2020 13:14:53 +0200 Subject: [PATCH] addressing pr comments --- .../alerting/register_apm_alerts.ts | 6 ++--- .../apm/server/lib/alerts/action_variables.ts | 16 +++---------- .../register_error_count_alert_type.test.ts | 18 +++++--------- .../alerts/register_error_count_alert_type.ts | 6 ++--- ...egister_transaction_duration_alert_type.ts | 6 ++--- ..._transaction_error_rate_alert_type.test.ts | 24 +++++++------------ ...ister_transaction_error_rate_alert_type.ts | 6 ++--- 7 files changed, 26 insertions(+), 56 deletions(-) diff --git a/x-pack/plugins/apm/public/components/alerting/register_apm_alerts.ts b/x-pack/plugins/apm/public/components/alerting/register_apm_alerts.ts index 306c8e32ef6be..5407c32c882ae 100644 --- a/x-pack/plugins/apm/public/components/alerting/register_apm_alerts.ts +++ b/x-pack/plugins/apm/public/components/alerting/register_apm_alerts.ts @@ -31,7 +31,7 @@ export function registerApmAlerts( - Service name: \\{\\{context.serviceName\\}\\} - Environment: \\{\\{context.environment\\}\\} - Threshold: \\{\\{context.threshold\\}\\} errors -- Triggered value: \\{\\{context.triggerValue\\}\\} errors over the last \\{\\{context.intervalSize\\}\\}\\{\\{context.intervalUnit\\}\\}`, +- Triggered value: \\{\\{context.triggerValue\\}\\} errors over the last \\{\\{context.interval\\}\\}`, } ), }); @@ -58,7 +58,7 @@ export function registerApmAlerts( - Type: \\{\\{context.transactionType\\}\\} - Environment: \\{\\{context.environment\\}\\} - Threshold: \\{\\{context.threshold\\}\\}ms -- Triggered value: \\{\\{context.triggerValue\\}\\} over the last \\{\\{context.intervalSize\\}\\}\\{\\{context.intervalUnit\\}\\}`, +- Triggered value: \\{\\{context.triggerValue\\}\\} over the last \\{\\{context.interval\\}\\}`, } ), }); @@ -85,7 +85,7 @@ export function registerApmAlerts( - Type: \\{\\{context.transactionType\\}\\} - Environment: \\{\\{context.environment\\}\\} - Threshold: \\{\\{context.threshold\\}\\}% -- Triggered value: \\{\\{context.triggerValue\\}\\}% of errors over the last \\{\\{context.intervalSize\\}\\}\\{\\{context.intervalUnit\\}\\}`, +- Triggered value: \\{\\{context.triggerValue\\}\\}% of errors over the last \\{\\{context.interval\\}\\}`, } ), }); diff --git a/x-pack/plugins/apm/server/lib/alerts/action_variables.ts b/x-pack/plugins/apm/server/lib/alerts/action_variables.ts index 6361279b3437c..19af337e18811 100644 --- a/x-pack/plugins/apm/server/lib/alerts/action_variables.ts +++ b/x-pack/plugins/apm/server/lib/alerts/action_variables.ts @@ -45,24 +45,14 @@ export const apmActionVariables = { ), name: 'triggerValue', }, - windowSize: { + interval: { description: i18n.translate( 'xpack.apm.alerts.action_variables.intervalSize', { defaultMessage: - 'The interval size the alert will use to check for any incident', + 'The length and unit of the time period where the alert conditions were met', } ), - name: 'intervalSize', - }, - windowUnit: { - description: i18n.translate( - 'xpack.apm.alerts.action_variables.intervalUnit', - { - defaultMessage: - 'The interval unit the alert will use to check for any incident', - } - ), - name: 'intervalUnit', + name: 'interval', }, }; diff --git a/x-pack/plugins/apm/server/lib/alerts/register_error_count_alert_type.test.ts b/x-pack/plugins/apm/server/lib/alerts/register_error_count_alert_type.test.ts index ca121a40d4d32..8dbc8054d4371 100644 --- a/x-pack/plugins/apm/server/lib/alerts/register_error_count_alert_type.test.ts +++ b/x-pack/plugins/apm/server/lib/alerts/register_error_count_alert_type.test.ts @@ -117,32 +117,28 @@ describe('Error count alert', () => { environment: 'env-foo', threshold: 1, triggerValue: 2, - intervalSize: 5, - intervalUnit: 'm', + interval: '5m', }); expect(scheduleActions).toHaveBeenCalledWith('threshold_met', { serviceName: 'foo', environment: 'env-foo-2', threshold: 1, triggerValue: 2, - intervalSize: 5, - intervalUnit: 'm', + interval: '5m', }); expect(scheduleActions).toHaveBeenCalledWith('threshold_met', { serviceName: 'bar', environment: 'env-bar', threshold: 1, triggerValue: 2, - intervalSize: 5, - intervalUnit: 'm', + interval: '5m', }); expect(scheduleActions).toHaveBeenCalledWith('threshold_met', { serviceName: 'bar', environment: 'env-bar-2', threshold: 1, triggerValue: 2, - intervalSize: 5, - intervalUnit: 'm', + interval: '5m', }); }); it('sends alerts with service name', async () => { @@ -194,16 +190,14 @@ describe('Error count alert', () => { environment: undefined, threshold: 1, triggerValue: 2, - intervalSize: 5, - intervalUnit: 'm', + interval: '5m', }); expect(scheduleActions).toHaveBeenCalledWith('threshold_met', { serviceName: 'bar', environment: undefined, threshold: 1, triggerValue: 2, - intervalSize: 5, - intervalUnit: 'm', + interval: '5m', }); }); }); diff --git a/x-pack/plugins/apm/server/lib/alerts/register_error_count_alert_type.ts b/x-pack/plugins/apm/server/lib/alerts/register_error_count_alert_type.ts index 20fbd3db25f4e..7b63f2c354916 100644 --- a/x-pack/plugins/apm/server/lib/alerts/register_error_count_alert_type.ts +++ b/x-pack/plugins/apm/server/lib/alerts/register_error_count_alert_type.ts @@ -55,8 +55,7 @@ export function registerErrorCountAlertType({ apmActionVariables.environment, apmActionVariables.threshold, apmActionVariables.triggerValue, - apmActionVariables.windowSize, - apmActionVariables.windowUnit, + apmActionVariables.interval, ], }, producer: 'apm', @@ -140,8 +139,7 @@ export function registerErrorCountAlertType({ environment, threshold: alertParams.threshold, triggerValue: errorCount, - intervalSize: alertParams.windowSize, - intervalUnit: alertParams.windowUnit, + interval: `${alertParams.windowSize}${alertParams.windowUnit}`, }); } response.aggregations?.services.buckets.forEach((serviceBucket) => { diff --git a/x-pack/plugins/apm/server/lib/alerts/register_transaction_duration_alert_type.ts b/x-pack/plugins/apm/server/lib/alerts/register_transaction_duration_alert_type.ts index 57201b3f267f1..1d8b664751ba2 100644 --- a/x-pack/plugins/apm/server/lib/alerts/register_transaction_duration_alert_type.ts +++ b/x-pack/plugins/apm/server/lib/alerts/register_transaction_duration_alert_type.ts @@ -64,8 +64,7 @@ export function registerTransactionDurationAlertType({ apmActionVariables.environment, apmActionVariables.threshold, apmActionVariables.triggerValue, - apmActionVariables.windowSize, - apmActionVariables.windowUnit, + apmActionVariables.interval, ], }, producer: 'apm', @@ -153,8 +152,7 @@ export function registerTransactionDurationAlertType({ environment, threshold, triggerValue: transactionDurationFormatted, - intervalSize: alertParams.windowSize, - intervalUnit: alertParams.windowUnit, + interval: `${alertParams.windowSize}${alertParams.windowUnit}`, }); }); } diff --git a/x-pack/plugins/apm/server/lib/alerts/register_transaction_error_rate_alert_type.test.ts b/x-pack/plugins/apm/server/lib/alerts/register_transaction_error_rate_alert_type.test.ts index 207e6108cd83c..7c13f2a17b255 100644 --- a/x-pack/plugins/apm/server/lib/alerts/register_transaction_error_rate_alert_type.test.ts +++ b/x-pack/plugins/apm/server/lib/alerts/register_transaction_error_rate_alert_type.test.ts @@ -133,8 +133,7 @@ describe('Transaction error rate alert', () => { environment: 'env-foo', threshold: 10, triggerValue: '50', - intervalSize: 5, - intervalUnit: 'm', + interval: '5m', }); expect(scheduleActions).toHaveBeenCalledWith('threshold_met', { serviceName: 'foo', @@ -142,8 +141,7 @@ describe('Transaction error rate alert', () => { environment: 'env-foo-2', threshold: 10, triggerValue: '50', - intervalSize: 5, - intervalUnit: 'm', + interval: '5m', }); expect(scheduleActions).toHaveBeenCalledWith('threshold_met', { serviceName: 'bar', @@ -151,8 +149,7 @@ describe('Transaction error rate alert', () => { environment: 'env-bar', threshold: 10, triggerValue: '50', - intervalSize: 5, - intervalUnit: 'm', + interval: '5m', }); expect(scheduleActions).toHaveBeenCalledWith('threshold_met', { serviceName: 'bar', @@ -160,8 +157,7 @@ describe('Transaction error rate alert', () => { environment: 'env-bar-2', threshold: 10, triggerValue: '50', - intervalSize: 5, - intervalUnit: 'm', + interval: '5m', }); }); it('sends alerts with service name and transaction type', async () => { @@ -226,8 +222,7 @@ describe('Transaction error rate alert', () => { environment: undefined, threshold: 10, triggerValue: '50', - intervalSize: 5, - intervalUnit: 'm', + interval: '5m', }); expect(scheduleActions).toHaveBeenCalledWith('threshold_met', { serviceName: 'bar', @@ -235,8 +230,7 @@ describe('Transaction error rate alert', () => { environment: undefined, threshold: 10, triggerValue: '50', - intervalSize: 5, - intervalUnit: 'm', + interval: '5m', }); }); @@ -289,8 +283,7 @@ describe('Transaction error rate alert', () => { environment: undefined, threshold: 10, triggerValue: '50', - intervalSize: 5, - intervalUnit: 'm', + interval: '5m', }); expect(scheduleActions).toHaveBeenCalledWith('threshold_met', { serviceName: 'bar', @@ -298,8 +291,7 @@ describe('Transaction error rate alert', () => { environment: undefined, threshold: 10, triggerValue: '50', - intervalSize: 5, - intervalUnit: 'm', + interval: '5m', }); }); }); 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 26a23e33f8730..7bb9894a837f5 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 @@ -60,8 +60,7 @@ export function registerTransactionErrorRateAlertType({ apmActionVariables.environment, apmActionVariables.threshold, apmActionVariables.triggerValue, - apmActionVariables.windowSize, - apmActionVariables.windowUnit, + apmActionVariables.interval, ], }, producer: 'apm', @@ -175,8 +174,7 @@ export function registerTransactionErrorRateAlertType({ triggerValue: String( parseFloat(transactionErrorRate.toPrecision(3)) ), - intervalSize: alertParams.windowSize, - intervalUnit: alertParams.windowUnit, + interval: `${alertParams.windowSize}${alertParams.windowUnit}`, }); }