From bf64c22717525fafb5946bd400cb3b70434e3f6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B8ren=20Louv-Jansen?= Date: Tue, 31 Oct 2023 14:23:34 +0100 Subject: [PATCH] [APM] Simplify cleanup of alerts in API tests (#170111) --- .../test/apm_api_integration/configs/index.ts | 23 +++---- .../tests/alerts/anomaly_alert.spec.ts | 14 ++--- .../alerts/error_count_threshold.spec.ts | 22 +------ ...ate.ts => cleanup_rule_and_alert_state.ts} | 8 ++- .../tests/alerts/transaction_duration.spec.ts | 30 ++------- .../alerts/transaction_error_rate.spec.ts | 30 ++------- .../tests/anomalies/anomaly_charts.spec.ts | 63 ++++++++----------- 7 files changed, 56 insertions(+), 134 deletions(-) rename x-pack/test/apm_api_integration/tests/alerts/helpers/{cleanup_state.ts => cleanup_rule_and_alert_state.ts} (79%) diff --git a/x-pack/test/apm_api_integration/configs/index.ts b/x-pack/test/apm_api_integration/configs/index.ts index 2e45cca1c50ae..cdefe3011e49f 100644 --- a/x-pack/test/apm_api_integration/configs/index.ts +++ b/x-pack/test/apm_api_integration/configs/index.ts @@ -14,36 +14,33 @@ const apmDebugLogger = { appenders: ['console'], }; +const kibanaConfig = { + 'xpack.apm.forceSyntheticSource': 'true', + 'logging.loggers': [apmDebugLogger], + 'server.publicBaseUrl': 'http://mockedPublicBaseUrl', +}; + const apmFtrConfigs = { basic: { license: 'basic' as const, - kibanaConfig: { - 'xpack.apm.forceSyntheticSource': 'true', - 'logging.loggers': [apmDebugLogger], - 'server.publicBaseUrl': 'http://mockedPublicBaseUrl', - }, + kibanaConfig, }, trial: { license: 'trial' as const, - kibanaConfig: { - 'xpack.apm.forceSyntheticSource': 'true', - 'logging.loggers': [apmDebugLogger], - }, + kibanaConfig, }, rules: { license: 'trial' as const, kibanaConfig: { + ...kibanaConfig, 'xpack.ruleRegistry.write.enabled': 'true', - 'xpack.apm.forceSyntheticSource': 'true', - 'logging.loggers': [apmDebugLogger], }, }, cloud: { license: 'basic' as const, kibanaConfig: { + ...kibanaConfig, 'xpack.apm.agent.migrations.enabled': 'true', - 'xpack.apm.forceSyntheticSource': 'true', - 'logging.loggers': [apmDebugLogger], }, }, }; diff --git a/x-pack/test/apm_api_integration/tests/alerts/anomaly_alert.spec.ts b/x-pack/test/apm_api_integration/tests/alerts/anomaly_alert.spec.ts index 430cebb4d0c02..cae769750b4cc 100644 --- a/x-pack/test/apm_api_integration/tests/alerts/anomaly_alert.spec.ts +++ b/x-pack/test/apm_api_integration/tests/alerts/anomaly_alert.spec.ts @@ -13,8 +13,9 @@ import { range } from 'lodash'; import { ML_ANOMALY_SEVERITY } from '@kbn/ml-anomaly-utils/anomaly_severity'; import { FtrProviderContext } from '../../common/ftr_provider_context'; import { createAndRunApmMlJobs } from '../../common/utils/create_and_run_apm_ml_jobs'; -import { createApmRule, deleteApmRules } from './helpers/alerting_api_helper'; +import { createApmRule } from './helpers/alerting_api_helper'; import { waitForActiveRule } from './helpers/wait_for_active_rule'; +import { cleanupRuleAndAlertState } from './helpers/cleanup_rule_and_alert_state'; export default function ApiTest({ getService }: FtrProviderContext) { const registry = getService('registry'); @@ -69,14 +70,9 @@ export default function ApiTest({ getService }: FtrProviderContext) { }); async function cleanup() { - try { - await synthtraceEsClient.clean(); - await deleteApmRules(supertest); - await ml.cleanMlIndices(); - logger.info('Completed cleaned up'); - } catch (e) { - logger.info('Could not cleanup', e); - } + await synthtraceEsClient.clean(); + await cleanupRuleAndAlertState({ es, supertest, logger }); + await ml.cleanMlIndices(); } describe('with ml jobs', () => { diff --git a/x-pack/test/apm_api_integration/tests/alerts/error_count_threshold.spec.ts b/x-pack/test/apm_api_integration/tests/alerts/error_count_threshold.spec.ts index 73a6386ea1ed3..1cfcbc165b17f 100644 --- a/x-pack/test/apm_api_integration/tests/alerts/error_count_threshold.spec.ts +++ b/x-pack/test/apm_api_integration/tests/alerts/error_count_threshold.spec.ts @@ -14,16 +14,13 @@ import { omit } from 'lodash'; import { FtrProviderContext } from '../../common/ftr_provider_context'; import { createApmRule, - deleteRuleById, - deleteAlertsByRuleId, fetchServiceInventoryAlertCounts, fetchServiceTabAlertCount, ApmAlertFields, createIndexConnector, - deleteActionConnector, getIndexAction, } from './helpers/alerting_api_helper'; -import { cleanupAllState } from './helpers/cleanup_state'; +import { cleanupRuleAndAlertState } from './helpers/cleanup_rule_and_alert_state'; import { waitForAlertsForRule } from './helpers/wait_for_alerts_for_rule'; import { waitForIndexConnectorResults } from './helpers/wait_for_index_connector_results'; import { waitForActiveRule } from './helpers/wait_for_active_rule'; @@ -55,8 +52,6 @@ export default function ApiTest({ getService }: FtrProviderContext) { }; before(async () => { - cleanupAllState({ es, supertest }); - const opbeansJava = apm .service({ name: 'opbeans-java', environment: 'production', agentName: 'java' }) .instance('instance'); @@ -134,13 +129,7 @@ export default function ApiTest({ getService }: FtrProviderContext) { }); after(async () => { - try { - await deleteActionConnector({ supertest, es, actionId }); - await deleteRuleById({ supertest, ruleId }); - await deleteAlertsByRuleId({ es, ruleId }); - } catch (e) { - logger.info('Could not delete rule or action connector', e); - } + await cleanupRuleAndAlertState({ es, supertest, logger }); }); it('checks if rule is active', async () => { @@ -285,12 +274,7 @@ export default function ApiTest({ getService }: FtrProviderContext) { }); after(async () => { - try { - await deleteRuleById({ supertest, ruleId }); - await deleteAlertsByRuleId({ es, ruleId }); - } catch (e) { - logger.info('Could not delete rule', e); - } + await cleanupRuleAndAlertState({ es, supertest, logger }); }); it('produces one alert for the opbeans-php service', async () => { diff --git a/x-pack/test/apm_api_integration/tests/alerts/helpers/cleanup_state.ts b/x-pack/test/apm_api_integration/tests/alerts/helpers/cleanup_rule_and_alert_state.ts similarity index 79% rename from x-pack/test/apm_api_integration/tests/alerts/helpers/cleanup_state.ts rename to x-pack/test/apm_api_integration/tests/alerts/helpers/cleanup_rule_and_alert_state.ts index 243ca1f71a36e..144446a28b3d7 100644 --- a/x-pack/test/apm_api_integration/tests/alerts/helpers/cleanup_state.ts +++ b/x-pack/test/apm_api_integration/tests/alerts/helpers/cleanup_rule_and_alert_state.ts @@ -6,6 +6,7 @@ */ import { Client } from '@elastic/elasticsearch'; +import { ToolingLog } from '@kbn/tooling-log'; import type { SuperTest, Test } from 'supertest'; import { clearKibanaApmEventLog, @@ -14,12 +15,14 @@ import { deleteActionConnectorIndex, } from './alerting_api_helper'; -export async function cleanupAllState({ +export async function cleanupRuleAndAlertState({ es, supertest, + logger, }: { es: Client; supertest: SuperTest; + logger: ToolingLog; }) { try { await Promise.all([ @@ -29,7 +32,6 @@ export async function cleanupAllState({ await clearKibanaApmEventLog(es), ]); } catch (e) { - // eslint-disable-next-line no-console - console.error(`An error occured while cleaning up the state: ${e}`); + logger.error(`An error occured while cleaning up the state: ${e}`); } } diff --git a/x-pack/test/apm_api_integration/tests/alerts/transaction_duration.spec.ts b/x-pack/test/apm_api_integration/tests/alerts/transaction_duration.spec.ts index 84d34f29b7c86..bb8c9833bbc6d 100644 --- a/x-pack/test/apm_api_integration/tests/alerts/transaction_duration.spec.ts +++ b/x-pack/test/apm_api_integration/tests/alerts/transaction_duration.spec.ts @@ -15,15 +15,11 @@ import { createApmRule, fetchServiceInventoryAlertCounts, fetchServiceTabAlertCount, - deleteAlertsByRuleId, - deleteRuleById, - clearKibanaApmEventLog, ApmAlertFields, createIndexConnector, getIndexAction, - deleteActionConnector, } from './helpers/alerting_api_helper'; -import { cleanupAllState } from './helpers/cleanup_state'; +import { cleanupRuleAndAlertState } from './helpers/cleanup_rule_and_alert_state'; import { waitForAlertsForRule } from './helpers/wait_for_alerts_for_rule'; import { waitForActiveRule } from './helpers/wait_for_active_rule'; import { waitForIndexConnectorResults } from './helpers/wait_for_index_connector_results'; @@ -49,8 +45,6 @@ export default function ApiTest({ getService }: FtrProviderContext) { registry.when('transaction duration alert', { config: 'basic', archives: [] }, () => { before(async () => { - cleanupAllState({ es, supertest }); - const opbeansJava = apm .service({ name: 'opbeans-java', environment: 'production', agentName: 'java' }) .instance('instance'); @@ -77,12 +71,7 @@ export default function ApiTest({ getService }: FtrProviderContext) { }); after(async () => { - try { - await synthtraceEsClient.clean(); - await clearKibanaApmEventLog(es); - } catch (e) { - logger.info('Could not clear apm event log', e); - } + await synthtraceEsClient.clean(); }); describe('create rule for opbeans-java without kql filter', () => { @@ -111,13 +100,7 @@ export default function ApiTest({ getService }: FtrProviderContext) { }); after(async () => { - try { - await deleteActionConnector({ supertest, es, actionId }); - await deleteRuleById({ supertest, ruleId }); - await deleteAlertsByRuleId({ es, ruleId }); - } catch (e) { - logger.info('Could not delete rule or action connector', e); - } + await cleanupRuleAndAlertState({ es, supertest, logger }); }); it('checks if rule is active', async () => { @@ -229,12 +212,7 @@ export default function ApiTest({ getService }: FtrProviderContext) { }); after(async () => { - try { - await deleteAlertsByRuleId({ es, ruleId }); - await deleteRuleById({ supertest, ruleId }); - } catch (e) { - logger.info('Could not delete rule or action connector', e); - } + await cleanupRuleAndAlertState({ es, supertest, logger }); }); it('checks if rule is active', async () => { diff --git a/x-pack/test/apm_api_integration/tests/alerts/transaction_error_rate.spec.ts b/x-pack/test/apm_api_integration/tests/alerts/transaction_error_rate.spec.ts index cdbe034b4c6ae..056ee4e272d61 100644 --- a/x-pack/test/apm_api_integration/tests/alerts/transaction_error_rate.spec.ts +++ b/x-pack/test/apm_api_integration/tests/alerts/transaction_error_rate.spec.ts @@ -15,15 +15,11 @@ import { createApmRule, fetchServiceInventoryAlertCounts, fetchServiceTabAlertCount, - deleteAlertsByRuleId, - clearKibanaApmEventLog, - deleteRuleById, ApmAlertFields, getIndexAction, createIndexConnector, - deleteActionConnector, } from './helpers/alerting_api_helper'; -import { cleanupAllState } from './helpers/cleanup_state'; +import { cleanupRuleAndAlertState } from './helpers/cleanup_rule_and_alert_state'; import { waitForAlertsForRule } from './helpers/wait_for_alerts_for_rule'; import { waitForActiveRule } from './helpers/wait_for_active_rule'; import { waitForIndexConnectorResults } from './helpers/wait_for_index_connector_results'; @@ -38,8 +34,6 @@ export default function ApiTest({ getService }: FtrProviderContext) { registry.when('transaction error rate alert', { config: 'basic', archives: [] }, () => { before(async () => { - cleanupAllState({ es, supertest }); - const opbeansJava = apm .service({ name: 'opbeans-java', environment: 'production', agentName: 'java' }) .instance('instance'); @@ -76,12 +70,7 @@ export default function ApiTest({ getService }: FtrProviderContext) { }); after(async () => { - try { - await synthtraceEsClient.clean(); - await clearKibanaApmEventLog(es); - } catch (e) { - logger.info('Could not clean up apm event log', e); - } + await synthtraceEsClient.clean(); }); describe('create rule without kql query', () => { @@ -121,13 +110,7 @@ export default function ApiTest({ getService }: FtrProviderContext) { }); after(async () => { - try { - await deleteActionConnector({ supertest, es, actionId }); - await deleteRuleById({ supertest, ruleId }); - await deleteAlertsByRuleId({ es, ruleId }); - } catch (e) { - logger.info('Could not delete rule or action connector', e); - } + await cleanupRuleAndAlertState({ es, supertest, logger }); }); it('checks if rule is active', async () => { @@ -250,12 +233,7 @@ export default function ApiTest({ getService }: FtrProviderContext) { }); after(async () => { - try { - await deleteRuleById({ supertest, ruleId }); - await deleteAlertsByRuleId({ es, ruleId }); - } catch (e) { - logger.info('Could not delete rule', e); - } + await cleanupRuleAndAlertState({ es, supertest, logger }); }); it('indexes alert document with all group-by fields', async () => { diff --git a/x-pack/test/apm_api_integration/tests/anomalies/anomaly_charts.spec.ts b/x-pack/test/apm_api_integration/tests/anomalies/anomaly_charts.spec.ts index 681443b69dff7..05d7da0e2ca30 100644 --- a/x-pack/test/apm_api_integration/tests/anomalies/anomaly_charts.spec.ts +++ b/x-pack/test/apm_api_integration/tests/anomalies/anomaly_charts.spec.ts @@ -11,6 +11,7 @@ import { Environment } from '@kbn/apm-plugin/common/environment_rt'; import { apm, timerange } from '@kbn/apm-synthtrace-client'; import expect from '@kbn/expect'; import { last, omit, range } from 'lodash'; +import moment from 'moment'; import { ApmApiError } from '../../common/apm_api_supertest'; import { FtrProviderContext } from '../../common/ftr_provider_context'; import { createAndRunApmMlJobs } from '../../common/utils/create_and_run_apm_ml_jobs'; @@ -21,9 +22,13 @@ export default function ApiTest({ getService }: FtrProviderContext) { const ml = getService('ml'); const es = getService('es'); const logger = getService('log'); - const synthtraceEsClient = getService('synthtraceEsClient'); + const start = moment().subtract(2, 'days'); + const end = moment(); + const spikeStart = moment().subtract(8, 'hours'); + const spikeEnd = moment().subtract(6, 'hours'); + async function statusOf(p: Promise<{ status: number }>) { try { const { status } = await p; @@ -38,14 +43,10 @@ export default function ApiTest({ getService }: FtrProviderContext) { function getAnomalyCharts( { - start, - end, transactionType, serviceName, environment, }: { - start: string; - end: string; transactionType: string; serviceName: string; environment: Environment; @@ -59,8 +60,8 @@ export default function ApiTest({ getService }: FtrProviderContext) { serviceName, }, query: { - start, - end, + start: start.toISOString(), + end: end.toISOString(), transactionType, environment, }, @@ -77,8 +78,6 @@ export default function ApiTest({ getService }: FtrProviderContext) { getAnomalyCharts({ serviceName: 'a', transactionType: 'request', - start: '2021-01-01T00:00:00.000Z', - end: '2021-01-01T00:15:00.000Z', environment: 'ENVIRONMENT_ALL', }) ); @@ -92,12 +91,6 @@ export default function ApiTest({ getService }: FtrProviderContext) { 'fetching service anomalies with a trial license', { config: 'trial', archives: [] }, () => { - const start = '2021-01-01T00:00:00.000Z'; - const end = '2021-01-08T00:15:00.000Z'; - - const spikeStart = new Date('2021-01-03T00:00:00.000Z').getTime(); - const spikeEnd = new Date('2021-01-03T02:00:00.000Z').getTime(); - const NORMAL_DURATION = 100; const NORMAL_RATE = 1; @@ -110,13 +103,13 @@ export default function ApiTest({ getService }: FtrProviderContext) { .service({ name: 'b', environment: 'development', agentName: 'go' }) .instance('b'); - const events = timerange(new Date(start).getTime(), new Date(end).getTime()) + const events = timerange(start.valueOf(), end.valueOf()) .interval('1m') .rate(1) .generator((timestamp) => { - const isInSpike = timestamp >= spikeStart && timestamp < spikeEnd; + const isInSpike = timestamp >= spikeStart.valueOf() && timestamp < spikeEnd.valueOf(); const count = isInSpike ? 4 : NORMAL_RATE; - const duration = isInSpike ? 1000 : NORMAL_DURATION; + const duration = isInSpike ? 10000 : NORMAL_DURATION; const outcome = isInSpike ? 'failure' : 'success'; return [ @@ -139,9 +132,14 @@ export default function ApiTest({ getService }: FtrProviderContext) { }); after(async () => { - await synthtraceEsClient.clean(); + await cleanup(); }); + async function cleanup() { + await synthtraceEsClient.clean(); + await ml.cleanMlIndices(); + } + it('returns a 403 for a user without access to ML', async () => { expect( await statusOf( @@ -149,8 +147,6 @@ export default function ApiTest({ getService }: FtrProviderContext) { { serviceName: 'a', transactionType: 'request', - start, - end, environment: 'ENVIRONMENT_ALL', }, apmApiClient.noMlAccessUser @@ -165,8 +161,6 @@ export default function ApiTest({ getService }: FtrProviderContext) { getAnomalyCharts({ serviceName: 'a', transactionType: 'request', - start, - end, environment: 'ENVIRONMENT_ALL', }) ); @@ -185,17 +179,11 @@ export default function ApiTest({ getService }: FtrProviderContext) { }); }); - after(async () => { - await ml.cleanMlIndices(); - }); - it('returns a 200 for a user _with_ access to ML', async () => { const status = await statusOf( getAnomalyCharts({ serviceName: 'a', transactionType: 'request', - start, - end, environment: 'ENVIRONMENT_ALL', }) ); @@ -209,15 +197,13 @@ export default function ApiTest({ getService }: FtrProviderContext) { let latencySeries: ServiceAnomalyTimeseries | undefined; let throughputSeries: ServiceAnomalyTimeseries | undefined; let failureRateSeries: ServiceAnomalyTimeseries | undefined; - const endTimeMs = new Date(end).getTime(); + const endTimeMs = end.valueOf(); before(async () => { allAnomalyTimeseries = ( await getAnomalyCharts({ serviceName: 'a', transactionType: 'request', - start, - end, environment: 'ENVIRONMENT_ALL', }) ).body.allAnomalyTimeseries; @@ -246,12 +232,11 @@ export default function ApiTest({ getService }: FtrProviderContext) { expect( allAnomalyTimeseries.every((spec) => - spec.bounds.every( - (bound) => bound.x >= new Date(start).getTime() && bound.x <= endTimeMs - ) + spec.bounds.every((bound) => bound.x >= start.valueOf() && bound.x <= endTimeMs) ) ); }); + it('returns model plots with latest bucket matching the end time', () => { expect(allAnomalyTimeseries.every((spec) => last(spec.bounds)?.x === endTimeMs)); }); @@ -310,19 +295,21 @@ export default function ApiTest({ getService }: FtrProviderContext) { expect( latencyAnomalies?.every( - (anomaly) => anomaly.x >= spikeStart && (anomaly.actual ?? 0) > NORMAL_DURATION + (anomaly) => + anomaly.x >= spikeStart.valueOf() && (anomaly.actual ?? 0) > NORMAL_DURATION ) ); expect( throughputAnomalies?.every( - (anomaly) => anomaly.x >= spikeStart && (anomaly.actual ?? 0) > NORMAL_RATE + (anomaly) => + anomaly.x >= spikeStart.valueOf() && (anomaly.actual ?? 0) > NORMAL_RATE ) ); expect( failureRateAnomalies?.every( - (anomaly) => anomaly.x >= spikeStart && (anomaly.actual ?? 0) > 0 + (anomaly) => anomaly.x >= spikeStart.valueOf() && (anomaly.actual ?? 0) > 0 ) ); });