Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[AO] Add API integration test scenarios to the Threshold rule #162501

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ export const deleteDataView = async ({
id: string;
}) => {
const { body } = await supertest
.delete(`/api/content_management/rpc/create`)
.post(`/api/content_management/rpc/delete`)
.set('kbn-xsrf', 'foo')
.send({
contentTypeId: 'index-pattern',
Expand Down
10 changes: 7 additions & 3 deletions x-pack/test/alerting_api_integration/observability/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,16 @@
// eslint-disable-next-line import/no-default-export
export default function ({ loadTestFile }: any) {
describe('Observability Rules', () => {
describe('MetricsUI Endpoints', () => {
describe('Rules Endpoints', () => {
loadTestFile(require.resolve('./metric_threshold_rule'));
loadTestFile(require.resolve('./threshold_rule'));
loadTestFile(require.resolve('./threshold_rule/avg_pct_fired'));
loadTestFile(require.resolve('./threshold_rule/avg_pct_no_data'));
loadTestFile(require.resolve('./threshold_rule/avg_us_fired'));
loadTestFile(require.resolve('./threshold_rule/custom_eq_avg_bytes_fired'));
loadTestFile(require.resolve('./threshold_rule/documents_count_fired'));
loadTestFile(require.resolve('./threshold_rule/group_by_fired'));
loadTestFile(require.resolve('./threshold_rule_data_view'));
});

describe('Synthetics', () => {
loadTestFile(require.resolve('./synthetics_rule'));
});
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,182 @@
/*
* 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 { cleanup, generate } from '@kbn/infra-forge';
import { Aggregators, Comparator } from '@kbn/observability-plugin/common/threshold_rule/types';
import { FIRED_ACTIONS_ID } from '@kbn/observability-plugin/server/lib/rules/threshold/threshold_executor';
import expect from '@kbn/expect';
import { OBSERVABILITY_THRESHOLD_RULE_TYPE_ID } from '@kbn/observability-plugin/common/constants';
import { createIndexConnector, createRule } from '../helpers/alerting_api_helper';
import { createDataView, deleteDataView } from '../helpers/data_view';
import { waitForAlertInIndex, waitForRuleStatus } from '../helpers/alerting_wait_for_helpers';
import { FtrProviderContext } from '../../common/ftr_provider_context';

// eslint-disable-next-line import/no-default-export
export default function ({ getService }: FtrProviderContext) {
const esClient = getService('es');
const supertest = getService('supertest');
const esDeleteAllIndices = getService('esDeleteAllIndices');
const logger = getService('log');

describe('Threshold rule - AVG - PCT - FIRED', () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are the differences between Threshold rule - AVG - PCT - FIRED and Threshold rule - AVG - US - FIRED?
Initially, I was thinking about having multiple scenarios to check the reason to make sure they are properly formatted, I am curious to know what other differences we can test.

FYI, I created another ticket for the action variables, then we can also check the reason to ensure the correct formating is used.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are the differences between Threshold rule - AVG - PCT - FIRED and Threshold rule - AVG - US - FIRED?

Field data type % and microseconds

Initially, I was thinking about having multiple scenarios to check the reason to make sure they are properly formatted, I am curious to know what other differences we can test.

I am not sure If understand what you mean, but please create a ticket with the check you want to do with the scenarios you have in mind.

Copy link
Member

@maryam-saeidi maryam-saeidi Aug 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My question is, what do we test for different field data types related to their type? Can you please point me to the code that is different between these two tests related to their field types?

Copy link
Contributor Author

@fkanout fkanout Aug 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@maryam-saeidi, I'm fulfilling the 1st scenario you requested in your issue:

Multiple conditions with different types of fields (such as number, string, percent) without grouping (Can we have one condition for every condition type? 😄)

I thought you knew the difference, so you asked to cover it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changing the field type will make many areas of code to be tested. I will list some files to answer the question
x-pack/plugins/observability/server/lib/rules/threshold/lib/get_data.ts
x-pack/plugins/observability/server/lib/rules/threshold/lib/metric_query.ts
x-pack/plugins/observability/server/lib/rules/threshold/threshold_executor.ts
x-pack/plugins/observability/server/lib/rules/threshold/lib/create_percentile_aggregation.ts
... and maybe others

As this is an integration test (not a unit test), asking about what lines of code are covered is inadequate and hard to answer.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought you knew the difference, so you asked to cover it.

Yes, the difference was the reason message that we are not checking at the moment.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As this is an integration test (not a unit test), asking about what lines of code are covered is inadequate and hard to answer.

By the line of code, I meant related to the new tests that you added, not the code behind the scene, something maybe like: the value that is saved in AAD is formatted differently for different field types.

const THRESHOLD_RULE_ALERT_INDEX = '.alerts-observability.threshold.alerts-default';
const ALERT_ACTION_INDEX = 'alert-action-threshold';
const DATA_VIEW_ID = 'data-view-id';
let infraDataIndex: string;
let actionId: string;
let ruleId: string;

before(async () => {
infraDataIndex = await generate({ esClient, lookback: 'now-15m', logger });
await createDataView({
supertest,
name: 'metrics-fake_hosts',
id: DATA_VIEW_ID,
title: 'metrics-fake_hosts',
});
});

after(async () => {
await supertest.delete(`/api/alerting/rule/${ruleId}`).set('kbn-xsrf', 'foo');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have a utility ObjectRemover which may simplify some of this sort of stuff - or not! Example usage here.

await supertest.delete(`/api/actions/connector/${actionId}`).set('kbn-xsrf', 'foo');
await esClient.deleteByQuery({
index: THRESHOLD_RULE_ALERT_INDEX,
query: { term: { 'kibana.alert.rule.uuid': ruleId } },
});
await esClient.deleteByQuery({
index: '.kibana-event-log-*',
query: { term: { 'kibana.alert.rule.consumer': 'alerts' } },
});
await deleteDataView({
supertest,
id: DATA_VIEW_ID,
});
await esDeleteAllIndices([ALERT_ACTION_INDEX, infraDataIndex]);
await cleanup({ esClient, logger });
});

describe('Rule creation', () => {
it('creates rule successfully', async () => {
actionId = await createIndexConnector({
supertest,
name: 'Index Connector: Threshold API test',
indexName: ALERT_ACTION_INDEX,
});

const createdRule = await createRule({
supertest,
tags: ['observability'],
consumer: 'alerts',
name: 'Threshold rule',
ruleTypeId: OBSERVABILITY_THRESHOLD_RULE_TYPE_ID,
params: {
criteria: [
{
aggType: Aggregators.CUSTOM,
comparator: Comparator.GT,
threshold: [0.5],
timeSize: 5,
timeUnit: 'm',
customMetrics: [
{ name: 'A', field: 'system.cpu.user.pct', aggType: Aggregators.AVERAGE },
],
},
],
alertOnNoData: true,
alertOnGroupDisappear: true,
searchConfiguration: {
query: {
query: '',
language: 'kuery',
},
index: DATA_VIEW_ID,
},
},
actions: [
{
group: FIRED_ACTIONS_ID,
id: actionId,
params: {
documents: [
{
ruleType: '{{rule.type}}',
},
],
},
frequency: {
notify_when: 'onActionGroupChange',
throttle: null,
summary: false,
},
},
],
});
ruleId = createdRule.id;
expect(ruleId).not.to.be(undefined);
});

it('should be active', async () => {
const executionStatus = await waitForRuleStatus({
id: ruleId,
expectedStatus: 'active',
supertest,
});
expect(executionStatus.status).to.be('active');
});

it('should set correct information in the alert document', async () => {
const resp = await waitForAlertInIndex({
esClient,
indexName: THRESHOLD_RULE_ALERT_INDEX,
ruleId,
});

expect(resp.hits.hits[0]._source).property(
'kibana.alert.rule.category',
'Threshold (Technical Preview)'
);
expect(resp.hits.hits[0]._source).property('kibana.alert.rule.consumer', 'alerts');
expect(resp.hits.hits[0]._source).property('kibana.alert.rule.name', 'Threshold rule');
expect(resp.hits.hits[0]._source).property('kibana.alert.rule.producer', 'observability');
expect(resp.hits.hits[0]._source).property('kibana.alert.rule.revision', 0);
expect(resp.hits.hits[0]._source).property(
'kibana.alert.rule.rule_type_id',
'observability.rules.threshold'
);
expect(resp.hits.hits[0]._source).property('kibana.alert.rule.uuid', ruleId);
expect(resp.hits.hits[0]._source).property('kibana.space_ids').contain('default');
expect(resp.hits.hits[0]._source)
.property('kibana.alert.rule.tags')
.contain('observability');
expect(resp.hits.hits[0]._source).property('kibana.alert.action_group', 'threshold.fired');
expect(resp.hits.hits[0]._source).property('tags').contain('observability');
expect(resp.hits.hits[0]._source).property('kibana.alert.instance.id', '*');
expect(resp.hits.hits[0]._source).property('kibana.alert.workflow_status', 'open');
expect(resp.hits.hits[0]._source).property('event.kind', 'signal');
expect(resp.hits.hits[0]._source).property('event.action', 'open');

expect(resp.hits.hits[0]._source)
.property('kibana.alert.rule.parameters')
.eql({
criteria: [
{
aggType: 'custom',
comparator: '>',
threshold: [0.5],
timeSize: 5,
timeUnit: 'm',
customMetrics: [{ name: 'A', field: 'system.cpu.user.pct', aggType: 'avg' }],
},
],
alertOnNoData: true,
alertOnGroupDisappear: true,
searchConfiguration: { index: 'data-view-id', query: { query: '', language: 'kuery' } },
});
});
});
});
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,179 @@
/*
* 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 { Aggregators, Comparator } from '@kbn/observability-plugin/common/threshold_rule/types';
import { FIRED_ACTIONS_ID } from '@kbn/observability-plugin/server/lib/rules/threshold/threshold_executor';
import expect from '@kbn/expect';
import { OBSERVABILITY_THRESHOLD_RULE_TYPE_ID } from '@kbn/observability-plugin/common/constants';

import { createIndexConnector, createRule } from '../helpers/alerting_api_helper';
import { createDataView, deleteDataView } from '../helpers/data_view';
import { waitForAlertInIndex, waitForRuleStatus } from '../helpers/alerting_wait_for_helpers';
import { FtrProviderContext } from '../../common/ftr_provider_context';

// eslint-disable-next-line import/no-default-export
export default function ({ getService }: FtrProviderContext) {
const esClient = getService('es');
const supertest = getService('supertest');

describe('Threshold rule - AVG - PCT - NoData', () => {
const THRESHOLD_RULE_ALERT_INDEX = '.alerts-observability.threshold.alerts-default';
const ALERT_ACTION_INDEX = 'alert-action-threshold';
const DATA_VIEW_ID = 'data-view-id-no-data';
let actionId: string;
let ruleId: string;

before(async () => {
await createDataView({
supertest,
name: 'no-data-pattern',
id: DATA_VIEW_ID,
title: 'no-data-pattern',
});
});

after(async () => {
await supertest.delete(`/api/alerting/rule/${ruleId}`).set('kbn-xsrf', 'foo');
await supertest.delete(`/api/actions/connector/${actionId}`).set('kbn-xsrf', 'foo');
await esClient.deleteByQuery({
index: THRESHOLD_RULE_ALERT_INDEX,
query: { term: { 'kibana.alert.rule.uuid': ruleId } },
});
await esClient.deleteByQuery({
index: '.kibana-event-log-*',
query: { term: { 'kibana.alert.rule.consumer': 'alerts' } },
});
await deleteDataView({
supertest,
id: DATA_VIEW_ID,
});
});

describe('Rule creation', () => {
it('creates rule successfully', async () => {
actionId = await createIndexConnector({
supertest,
name: 'Index Connector: Threshold API test',
indexName: ALERT_ACTION_INDEX,
});

const createdRule = await createRule({
supertest,
tags: ['observability'],
consumer: 'alerts',
name: 'Threshold rule',
ruleTypeId: OBSERVABILITY_THRESHOLD_RULE_TYPE_ID,
params: {
criteria: [
{
aggType: Aggregators.CUSTOM,
comparator: Comparator.GT,
threshold: [0.5],
timeSize: 5,
timeUnit: 'm',
customMetrics: [
{ name: 'A', field: 'system.cpu.user.pct', aggType: Aggregators.AVERAGE },
],
},
],
alertOnNoData: true,
alertOnGroupDisappear: true,
searchConfiguration: {
query: {
query: '',
language: 'kuery',
},
index: DATA_VIEW_ID,
},
},
actions: [
{
group: FIRED_ACTIONS_ID,
id: actionId,
params: {
documents: [
{
ruleType: '{{rule.type}}',
},
],
},
frequency: {
notify_when: 'onActionGroupChange',
throttle: null,
summary: false,
},
},
],
});
ruleId = createdRule.id;
expect(ruleId).not.to.be(undefined);
});

it('should be active', async () => {
const executionStatus = await waitForRuleStatus({
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kinda up to you, since you own these tests, but we generally treat each test as stand-alone, so that you could change it(... to it.only(... when diagnosing tests. I think we'd more generally write a function here to create the rule / connector, and call it from each test, and then delete it (somehow - ObjectRemover or other) in an afterEach().

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pmuellr, thank you for the feedback!
I wrote these tests as a file-per-scenario as we are sharing the same tests in serverless and stateful env to maintain them easily (copy/paste 😄) with minimum dependencies until we can share one test suite across ENVs (agnostic tests)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ya, will be interesting to see how serverless / stateful test sharing will go!

maintain them easily (copy/paste 😄)

Probably more of a risk of these being problematic, in that each test depends on the previous to work. Small number of tests here though, so hard to imagine it going wrong in this file, now. Something to think about if these get more complex.

id: ruleId,
expectedStatus: 'active',
supertest,
});
expect(executionStatus.status).to.be('active');
});

it('should set correct information in the alert document', async () => {
const resp = await waitForAlertInIndex({
esClient,
indexName: THRESHOLD_RULE_ALERT_INDEX,
ruleId,
});

expect(resp.hits.hits[0]._source).property(
'kibana.alert.rule.category',
'Threshold (Technical Preview)'
);
expect(resp.hits.hits[0]._source).property('kibana.alert.rule.consumer', 'alerts');
expect(resp.hits.hits[0]._source).property('kibana.alert.rule.name', 'Threshold rule');
expect(resp.hits.hits[0]._source).property('kibana.alert.rule.producer', 'observability');
expect(resp.hits.hits[0]._source).property('kibana.alert.rule.revision', 0);
expect(resp.hits.hits[0]._source).property(
'kibana.alert.rule.rule_type_id',
'observability.rules.threshold'
);
expect(resp.hits.hits[0]._source).property('kibana.alert.rule.uuid', ruleId);
expect(resp.hits.hits[0]._source).property('kibana.space_ids').contain('default');
expect(resp.hits.hits[0]._source)
.property('kibana.alert.rule.tags')
.contain('observability');
expect(resp.hits.hits[0]._source).property('kibana.alert.action_group', 'threshold.nodata');
expect(resp.hits.hits[0]._source).property('tags').contain('observability');
expect(resp.hits.hits[0]._source).property('kibana.alert.instance.id', '*');
expect(resp.hits.hits[0]._source).property('kibana.alert.workflow_status', 'open');
expect(resp.hits.hits[0]._source).property('event.kind', 'signal');
expect(resp.hits.hits[0]._source).property('event.action', 'open');

expect(resp.hits.hits[0]._source)
.property('kibana.alert.rule.parameters')
.eql({
criteria: [
{
aggType: 'custom',
comparator: '>',
threshold: [0.5],
timeSize: 5,
timeUnit: 'm',
customMetrics: [{ name: 'A', field: 'system.cpu.user.pct', aggType: 'avg' }],
},
],
alertOnNoData: true,
alertOnGroupDisappear: true,
searchConfiguration: {
index: 'data-view-id-no-data',
query: { query: '', language: 'kuery' },
},
});
});
});
});
}
Loading
Loading