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

Add filter field to index threshold rule type #142255

Merged
merged 24 commits into from
Oct 13, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
33820f6
Add filter field to index threshold rule type
ersin-erdal Sep 29, 2022
ce0aa9e
remove test skip
ersin-erdal Sep 29, 2022
c7785d2
fix tests
ersin-erdal Sep 29, 2022
6473f80
remove FE validation
ersin-erdal Oct 3, 2022
ca45ea4
fix tests
ersin-erdal Oct 3, 2022
1ecf740
fix tests
ersin-erdal Oct 4, 2022
eaffaa8
fix integration test
ersin-erdal Oct 5, 2022
9b50171
add FE validation back
ersin-erdal Oct 5, 2022
95ad5bc
Merge branch 'main' into 66046-index-threshold-filter
ersin-erdal Oct 5, 2022
d250284
Merge branch 'main' into 66046-index-threshold-filter
ersin-erdal Oct 5, 2022
bd9935e
Merge branch 'main' into 66046-index-threshold-filter
ersin-erdal Oct 5, 2022
5a18351
Merge remote-tracking branch 'origin/66046-index-threshold-filter' in…
ersin-erdal Oct 5, 2022
1170fc4
debounce validation
ersin-erdal Oct 6, 2022
2c36ae8
Merge branch 'main' into 66046-index-threshold-filter
ersin-erdal Oct 6, 2022
3704feb
fix jest test
ersin-erdal Oct 6, 2022
619b657
fix debounce bug in functional test
ersin-erdal Oct 6, 2022
d669f27
remove filterKueryError and increase bundle size
ersin-erdal Oct 6, 2022
52a9dad
Merge branch 'main' into 66046-index-threshold-filter
ersin-erdal Oct 10, 2022
2439bed
Merge branch 'main' into 66046-index-threshold-filter
ersin-erdal Oct 10, 2022
8783f3f
merge
ersin-erdal Oct 12, 2022
1da2bb5
Merge remote-tracking branch 'origin/66046-index-threshold-filter' in…
ersin-erdal Oct 12, 2022
c03041f
fix the flaky test
ersin-erdal Oct 12, 2022
1e836d2
Merge branch 'main' into 66046-index-threshold-filter
ersin-erdal Oct 13, 2022
2ea592c
Merge branch 'main' into 66046-index-threshold-filter
ersin-erdal Oct 13, 2022
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
2 changes: 1 addition & 1 deletion packages/kbn-optimizer/limits.yml
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ pageLoadAssetSize:
share: 71239
snapshotRestore: 79032
spaces: 57868
stackAlerts: 29684
stackAlerts: 58316
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Created a follow-up issue to reduce bundle size: #142902

stackConnectors: 36314
synthetics: 40958
telemetry: 51957
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ describe('IndexThresholdAlertTypeExpression', () => {
expect(wrapper.find('[data-test-subj="forLastExpression"]').exists()).toBeTruthy();
expect(wrapper.find('[data-test-subj="visualizationPlaceholder"]').exists()).toBeTruthy();
expect(wrapper.find('[data-test-subj="thresholdVisualization"]').exists()).toBeFalsy();
expect(wrapper.find('[data-test-subj="filterKuery"]').exists()).toBeTruthy();
});

test(`should render IndexThresholdAlertTypeExpression with expected components when aggType does require field`, async () => {
Expand All @@ -133,6 +134,7 @@ describe('IndexThresholdAlertTypeExpression', () => {
expect(wrapper.find('[data-test-subj="forLastExpression"]').exists()).toBeTruthy();
expect(wrapper.find('[data-test-subj="visualizationPlaceholder"]').exists()).toBeTruthy();
expect(wrapper.find('[data-test-subj="thresholdVisualization"]').exists()).toBeFalsy();
expect(wrapper.find('[data-test-subj="filterKuery"]').exists()).toBeTruthy();
});

test(`should render IndexThresholdAlertTypeExpression with visualization when there are no expression errors`, async () => {
Expand Down Expand Up @@ -175,6 +177,7 @@ describe('IndexThresholdAlertTypeExpression', () => {
expect(
wrapper.find('EuiEmptyPrompt[data-test-subj="visualizationPlaceholder"]').text()
).toEqual(`Complete the expression to generate a preview.`);
expect(wrapper.find('input[data-test-subj="filterKuery"]').text()).toEqual('');
});

test(`should use alert params when params are defined`, async () => {
Expand All @@ -186,6 +189,7 @@ describe('IndexThresholdAlertTypeExpression', () => {
const groupBy = 'top';
const termSize = '27';
const termField = 'host.name';

const wrapper = await setup(
getAlertParams({
aggType,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,18 @@
* 2.0.
*/

import React, { useState, Fragment, useEffect } from 'react';
import React, { useState, Fragment, useEffect, useCallback, ChangeEvent } from 'react';
import { i18n } from '@kbn/i18n';
import { FormattedMessage } from '@kbn/i18n-react';
import { EuiSpacer, EuiCallOut, EuiEmptyPrompt, EuiText, EuiTitle } from '@elastic/eui';
import {
EuiSpacer,
EuiCallOut,
EuiEmptyPrompt,
EuiText,
EuiTitle,
EuiFieldSearch,
EuiFormRow,
} from '@elastic/eui';
import { HttpSetup } from '@kbn/core/public';
import { useKibana } from '@kbn/kibana-react-plugin/public';
import {
Expand Down Expand Up @@ -78,6 +86,7 @@ export const IndexThresholdAlertTypeExpression: React.FunctionComponent<
threshold,
timeWindowSize,
timeWindowUnit,
filterKuery,
} = ruleParams;

const indexArray = indexParamToArray(index);
Expand Down Expand Up @@ -133,6 +142,13 @@ export const IndexThresholdAlertTypeExpression: React.FunctionComponent<
setEsFields(currentEsFields);
};

const handleFilterChange = useCallback(
Copy link
Contributor

Choose a reason for hiding this comment

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

is there any way to add a debounce here so not every keystroke is validated?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Debouncing here just prevents the value to be set.
Current validation is triggered on every field change of a rule. We need to change the whole validation implementation on framework level in order to tackle validations for each field separately.

(e: ChangeEvent<HTMLInputElement>) => {
setRuleParams('filterKuery', e.target.value || undefined);
},
[setRuleParams]
);

useEffect(() => {
setDefaultExpressionValues();
// eslint-disable-next-line react-hooks/exhaustive-deps
Expand Down Expand Up @@ -261,6 +277,33 @@ export const IndexThresholdAlertTypeExpression: React.FunctionComponent<
}
/>
<EuiSpacer />
<EuiTitle size="xs">
<h5>
<FormattedMessage
id="xpack.stackAlerts.threshold.ui.filterTitle"
defaultMessage="Filter (Optional)"
/>
</h5>
</EuiTitle>
<EuiSpacer size="s" />
<EuiFormRow
helpText={i18n.translate('xpack.stackAlerts.threshold.ui.filterKQLHelpText', {
defaultMessage: 'Use a KQL expression to limit the scope of your alert trigger.',
})}
fullWidth
display="rowCompressed"
isInvalid={errors.filterKuery.length > 0}
error={errors.filterKuery}
>
<EuiFieldSearch
data-test-subj="filterKuery"
onChange={handleFilterChange}
ersin-erdal marked this conversation as resolved.
Show resolved Hide resolved
value={filterKuery}
fullWidth
isInvalid={errors.filterKuery.length > 0}
/>
</EuiFormRow>
<EuiSpacer />
<div className="actAlertVisualization__chart">
{cannotShowVisualization ? (
<Fragment>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ export async function getThresholdAlertVisualizationData({
termSize: model.termSize,
timeWindowSize: model.timeWindowSize,
timeWindowUnit: model.timeWindowUnit,
filterKuery: model.filterKuery,
dateStart: new Date(visualizeOptions.rangeFrom).toISOString(),
dateEnd: new Date(visualizeOptions.rangeTo).toISOString(),
interval: visualizeOptions.interval,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,5 @@ export interface IndexThresholdAlertParams extends RuleTypeParams {
threshold: number[];
timeWindowSize: number;
timeWindowUnit: string;
filterKuery?: string;
}
Original file line number Diff line number Diff line change
Expand Up @@ -94,4 +94,21 @@ describe('expression params validation', () => {
expect(validateExpression(initialParams).errors.threshold1.length).toBeGreaterThan(0);
expect(validateExpression(initialParams).errors.threshold1[0]).toBe('Threshold1 is required.');
});

test('if filterKuery is invalid should return proper error message', () => {
const initialParams: IndexThresholdAlertParams = {
index: ['test'],
aggType: 'count',
groupBy: 'top',
threshold: [1],
timeWindowSize: 1,
timeWindowUnit: 's',
thresholdComparator: 'between',
filterKuery: 'group:',
};
expect(validateExpression(initialParams).errors.filterKuery.length).toBeGreaterThan(0);
expect(validateExpression(initialParams).errors.filterKuery[0]).toBe(
'Filter query is invalid.'
);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* 2.0.
*/

import { toElasticsearchQuery, fromKueryExpression } from '@kbn/es-query';
import { i18n } from '@kbn/i18n';
import {
ValidationResult,
Expand All @@ -26,6 +27,7 @@ export const validateExpression = (alertParams: IndexThresholdAlertParams): Vali
threshold,
timeWindowSize,
thresholdComparator,
filterKuery,
} = alertParams;
const validationResult = { errors: {} };
const errors = {
Expand All @@ -37,8 +39,22 @@ export const validateExpression = (alertParams: IndexThresholdAlertParams): Vali
threshold1: new Array<string>(),
index: new Array<string>(),
timeField: new Array<string>(),
filterKuery: new Array<string>(),
};
validationResult.errors = errors;

ersin-erdal marked this conversation as resolved.
Show resolved Hide resolved
if (!!filterKuery) {
try {
toElasticsearchQuery(fromKueryExpression(filterKuery as string));
} catch (e) {
errors.filterKuery.push(
i18n.translate('xpack.stackAlerts.threshold.ui.validation.error.invalidKql', {
defaultMessage: 'Filter query is invalid.',
})
);
}
}

if (!index || index.length === 0) {
errors.index.push(
i18n.translate('xpack.stackAlerts.threshold.ui.validation.error.requiredIndexText', {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,10 @@ describe('ruleType', () => {
"description": "termField",
"name": "termField",
},
Object {
"description": "filterKuery",
"name": "filterKuery",
},
Object {
"description": "termSize",
"name": "termSize",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,7 @@ export function getRuleType(
timeWindowSize: params.timeWindowSize,
timeWindowUnit: params.timeWindowUnit,
interval: undefined,
filterKuery: params.filterKuery,
};
// console.log(`index_threshold: query: ${JSON.stringify(queryParams, null, 4)}`);
const result = await (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ const DefaultParams: Writable<Partial<CoreQueryParams>> = {
groupBy: 'all',
timeWindowSize: 5,
timeWindowUnit: 'm',
filterKuery: 'event.provider: alerting',
};

export function runTests(schema: ObjectType, defaultTypeParams: Record<string, unknown>): void {
Expand Down Expand Up @@ -183,6 +184,13 @@ export function runTests(schema: ObjectType, defaultTypeParams: Record<string, u
`"[aggField]: must have a value when [aggType] is \\"avg\\""`
);
});

it('fails for invalid filterKuery', async () => {
params.filterKuery = 'event:';
expect(onValidate()).toThrowErrorMatchingInlineSnapshot(
'"[filterKuery]: Filter query is invalid."'
);
});
});

function onValidate(): () => void {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import { i18n } from '@kbn/i18n';
import { schema, TypeOf } from '@kbn/config-schema';

import { toElasticsearchQuery, fromKueryExpression } from '@kbn/es-query';
import { MAX_GROUPS } from '..';

export const CoreQueryParamsSchemaProperties = {
Expand All @@ -28,6 +29,8 @@ export const CoreQueryParamsSchemaProperties = {
groupBy: schema.string({ validate: validateGroupBy }),
// field to group on (for groupBy: top)
termField: schema.maybe(schema.string({ minLength: 1 })),
// filter field
filterKuery: schema.maybe(schema.string({ validate: validateKuery })),
// limit on number of groups returned
termSize: schema.maybe(schema.number({ min: 1 })),
// size of time window for date range aggregations
Expand Down Expand Up @@ -135,3 +138,16 @@ export function validateTimeWindowUnits(timeWindowUnit: string): string | undefi
}
);
}

export function validateKuery(query: string): string | undefined {
try {
toElasticsearchQuery(fromKueryExpression(query));
} catch (e) {
return i18n.translate(
'xpack.triggersActionsUI.data.coreQueryParams.invalidKQLQueryErrorMessage',
{
defaultMessage: 'Filter query is invalid.',
}
);
}
}
Loading