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

Elasticsearch Query rule can group by multi-terms #166146

Merged
merged 20 commits into from Sep 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions x-pack/plugins/stack_alerts/common/constants.ts
Expand Up @@ -6,3 +6,5 @@
*/

export const STACK_ALERTS_FEATURE_ID = 'stackAlerts';

export const MAX_SELECTABLE_GROUP_BY_TERMS = 4;
Expand Up @@ -22,6 +22,7 @@ export const DEFAULT_VALUES = {
TERM_SIZE: 5,
GROUP_BY: 'all',
EXCLUDE_PREVIOUS_HITS: true,
CAN_SELECT_MULTI_TERMS: true,
};

export const COMMON_EXPRESSION_ERRORS = {
Expand Down
Expand Up @@ -351,6 +351,7 @@ export const EsQueryExpression: React.FC<
(exclude) => setParam('excludeHitsFromPreviousRun', exclude),
[setParam]
)}
canSelectMultiTerms={DEFAULT_VALUES.CAN_SELECT_MULTI_TERMS}
/>

<EuiSpacer />
Expand Down
Expand Up @@ -14,12 +14,8 @@ import { EuiSpacer, EuiTitle } from '@elastic/eui';
import { IErrorObject } from '@kbn/triggers-actions-ui-plugin/public';
import type { SearchBarProps } from '@kbn/unified-search-plugin/public';
import type { DataView } from '@kbn/data-views-plugin/public';
import {
mapAndFlattenFilters,
getTime,
type SavedQuery,
type ISearchSource,
} from '@kbn/data-plugin/public';
import { mapAndFlattenFilters, getTime } from '@kbn/data-plugin/public';
import type { SavedQuery, ISearchSource } from '@kbn/data-plugin/public';
import {
BUCKET_SELECTOR_FIELD,
buildAggregation,
Expand Down Expand Up @@ -51,7 +47,9 @@ interface LocalState extends CommonRuleParams {

interface LocalStateAction {
type: SearchSourceParamsAction['type'] | keyof CommonRuleParams;
payload: SearchSourceParamsAction['payload'] | (number[] | number | string | boolean | undefined);
payload:
| SearchSourceParamsAction['payload']
| (number[] | number | string | string[] | boolean | undefined);
}

type LocalStateReducer = (prevState: LocalState, action: LocalStateAction) => LocalState;
Expand Down Expand Up @@ -201,7 +199,8 @@ export const SearchSourceExpressionForm = (props: SearchSourceExpressionFormProp
);

const onChangeSelectedTermField = useCallback(
(selectedTermField?: string) => dispatch({ type: 'termField', payload: selectedTermField }),
(selectedTermField?: string | string[]) =>
dispatch({ type: 'termField', payload: selectedTermField }),
[]
);

Expand Down Expand Up @@ -372,6 +371,7 @@ export const SearchSourceExpressionForm = (props: SearchSourceExpressionFormProp
onCopyQuery={onCopyQuery}
excludeHitsFromPreviousRun={ruleConfiguration.excludeHitsFromPreviousRun}
onChangeExcludeHitsFromPreviousRun={onChangeExcludeHitsFromPreviousRun}
canSelectMultiTerms={DEFAULT_VALUES.CAN_SELECT_MULTI_TERMS}
/>
<EuiSpacer />
</Fragment>
Expand Down
Expand Up @@ -217,6 +217,33 @@ describe('RuleCommonExpressions', () => {
);
});

test(`should use multiple group by terms`, async () => {
const aggType = 'avg';
const thresholdComparator = 'between';
const timeWindowSize = 987;
const timeWindowUnit = 's';
const threshold = [3, 1003];
const groupBy = 'top';
const termSize = '27';
const termField = ['term', 'term2'];

const wrapper = await setup({
ruleParams: getCommonParams({
aggType,
thresholdComparator,
timeWindowSize,
timeWindowUnit,
termSize,
termField,
groupBy,
threshold,
}),
});
expect(wrapper.find('button[data-test-subj="groupByExpression"]').text()).toEqual(
`grouped over ${groupBy} ${termSize} 'term,term2'`
);
});

test(`should disable excludeHitsFromPreviousRuns when groupBy is not all`, async () => {
const aggType = 'avg';
const thresholdComparator = 'between';
Expand Down
Expand Up @@ -52,6 +52,7 @@ export interface RuleCommonExpressionsProps extends CommonRuleParams {
onTestFetch: TestQueryRowProps['fetch'];
onCopyQuery?: TestQueryRowProps['copyQuery'];
onChangeExcludeHitsFromPreviousRun: (exclude: boolean) => void;
canSelectMultiTerms?: boolean;
}

export const RuleCommonExpressions: React.FC<RuleCommonExpressionsProps> = ({
Expand Down Expand Up @@ -82,6 +83,7 @@ export const RuleCommonExpressions: React.FC<RuleCommonExpressionsProps> = ({
onCopyQuery,
excludeHitsFromPreviousRun,
onChangeExcludeHitsFromPreviousRun,
canSelectMultiTerms,
}) => {
const [isExcludeHitsDisabled, setIsExcludeHitsDisabled] = useState<boolean>(false);

Expand Down Expand Up @@ -127,6 +129,7 @@ export const RuleCommonExpressions: React.FC<RuleCommonExpressionsProps> = ({
errors={errors}
fields={esFields}
display="fullWidth"
canSelectMultiTerms={canSelectMultiTerms}
onChangeSelectedGroupBy={onChangeSelectedGroupBy}
onChangeSelectedTermField={onChangeSelectedTermField}
onChangeSelectedTermSize={onChangeSelectedTermSize}
Expand Down
Expand Up @@ -27,7 +27,7 @@ export interface CommonRuleParams {
aggField?: string;
groupBy?: string;
termSize?: number;
termField?: string;
termField?: string | string[];
excludeHitsFromPreviousRun: boolean;
}

Expand Down
Expand Up @@ -103,6 +103,46 @@ describe('expression params validation', () => {
expect(validateExpression(initialParams).errors.termField[0]).toBe('Term field is required.');
});

test('if termField property is an array but has no items should return proper error message', () => {
const initialParams: EsQueryRuleParams<SearchType.esQuery> = {
index: ['test'],
esQuery: `{\n \"query\":{\n \"match_all\" : {}\n }\n}`,
size: 100,
timeWindowSize: 1,
timeWindowUnit: 's',
threshold: [0],
timeField: '',
excludeHitsFromPreviousRun: true,
aggType: 'count',
groupBy: 'top',
termSize: 10,
termField: [],
};
expect(validateExpression(initialParams).errors.termField.length).toBeGreaterThan(0);
expect(validateExpression(initialParams).errors.termField[0]).toBe('Term field is required.');
});

test('if termField property is an array but has more than 4 items, should return proper error message', () => {
const initialParams: EsQueryRuleParams<SearchType.esQuery> = {
index: ['test'],
esQuery: `{\n \"query\":{\n \"match_all\" : {}\n }\n}`,
size: 100,
timeWindowSize: 1,
timeWindowUnit: 's',
threshold: [0],
timeField: '',
excludeHitsFromPreviousRun: true,
aggType: 'count',
groupBy: 'top',
termSize: 10,
termField: ['term', 'term2', 'term3', 'term4', 'term5'],
};
expect(validateExpression(initialParams).errors.termField.length).toBeGreaterThan(0);
expect(validateExpression(initialParams).errors.termField[0]).toBe(
'Cannot select more than 4 terms'
);
});

test('if esQuery property is invalid JSON should return proper error message', () => {
const initialParams: EsQueryRuleParams<SearchType.esQuery> = {
index: ['test'],
Expand Down
Expand Up @@ -14,6 +14,7 @@ import {
builtInGroupByTypes,
COMPARATORS,
} from '@kbn/triggers-actions-ui-plugin/public';
import { MAX_SELECTABLE_GROUP_BY_TERMS } from '../../../common/constants';
import { EsQueryRuleParams, SearchType } from './types';
import { isEsqlQueryRule, isSearchSourceRule } from './util';
import {
Expand Down Expand Up @@ -72,7 +73,7 @@ const validateCommonParams = (ruleParams: EsQueryRuleParams) => {
groupBy &&
builtInGroupByTypes[groupBy].validNormalizedTypes &&
builtInGroupByTypes[groupBy].validNormalizedTypes.length > 0 &&
!termField
(!termField || termField.length <= 0)
) {
errors.termField.push(
i18n.translate('xpack.stackAlerts.esQuery.ui.validation.error.requiredTermFieldText', {
Expand All @@ -81,6 +82,22 @@ const validateCommonParams = (ruleParams: EsQueryRuleParams) => {
);
}

if (
groupBy &&
builtInGroupByTypes[groupBy].validNormalizedTypes &&
builtInGroupByTypes[groupBy].validNormalizedTypes.length > 0 &&
termField &&
Array.isArray(termField) &&
termField.length > MAX_SELECTABLE_GROUP_BY_TERMS
) {
errors.termField.push(
i18n.translate('xpack.stackAlerts.esQuery.ui.validation.error.overNumberedTermFieldText', {
defaultMessage: `Cannot select more than {max} terms`,
values: { max: MAX_SELECTABLE_GROUP_BY_TERMS },
})
);
}

if (!threshold || threshold.length === 0 || threshold[0] === undefined) {
errors.threshold0.push(
i18n.translate('xpack.stackAlerts.esQuery.ui.validation.error.requiredThreshold0Text', {
Expand Down
Expand Up @@ -34,7 +34,7 @@ export interface IndexThresholdRuleParams extends RuleTypeParams {
aggField?: string;
groupBy?: string;
termSize?: number;
termField?: string;
termField?: string | string[];
thresholdComparator?: string;
threshold: number[];
timeWindowSize: number;
Expand Down
Expand Up @@ -179,15 +179,51 @@ describe('ruleType Params validate()', () => {
});

it('fails for invalid termField', async () => {
params.termField = ['term', 'term 2'];
params.termSize = 1;
expect(onValidate()).not.toThrow();

params.termField = 'term';
params.termSize = 1;
expect(onValidate()).not.toThrow();

// string or array of string
params.groupBy = 'top';
params.termField = 42;
expect(onValidate()).toThrowErrorMatchingInlineSnapshot(
`"[termField]: expected value of type [string] but got [number]"`
expect(onValidate()).toThrow(`[termField]: types that failed validation:
- [termField.0]: expected value of type [string] but got [number]
- [termField.1]: expected value of type [array] but got [number]`);

// no array other than array of stings
params.termField = [1, 2, 3];
expect(onValidate()).toThrow(
`[termField]: types that failed validation:
- [termField.0]: expected value of type [string] but got [Array]
- [termField.1.0]: expected value of type [string] but got [number]`
);

// no empty string
params.termField = '';
expect(onValidate()).toThrowErrorMatchingInlineSnapshot(
`"[termField]: value has length [0] but it must have a minimum length of [1]."`
expect(onValidate()).toThrow(
`[termField]: types that failed validation:
- [termField.0]: value has length [0] but it must have a minimum length of [1].
- [termField.1]: could not parse array value from json input`
);

// no array with one element -> has to be a string
params.termField = ['term'];
expect(onValidate()).toThrow(
`[termField]: types that failed validation:
- [termField.0]: expected value of type [string] but got [Array]
- [termField.1]: array size is [1], but cannot be smaller than [2]`
);

// no array that has more than 4 elements
params.termField = ['term', 'term2', 'term3', 'term4', 'term4'];
expect(onValidate()).toThrow(
`[termField]: types that failed validation:
- [termField.0]: expected value of type [string] but got [Array]
- [termField.1]: array size is [5], but cannot be greater than [4]`
);
});

Expand Down
Expand Up @@ -15,6 +15,7 @@ import {
} from '@kbn/triggers-actions-ui-plugin/server';
import { RuleTypeState } from '@kbn/alerting-plugin/server';
import { SerializedSearchSourceFields } from '@kbn/data-plugin/common';
import { MAX_SELECTABLE_GROUP_BY_TERMS } from '../../../common/constants';
import { ComparatorFnNames } from '../../../common';
import { Comparator } from '../../../common/comparator_types';
import { getComparatorSchemaType } from '../lib/comparator';
Expand Down Expand Up @@ -48,7 +49,12 @@ const EsQueryRuleParamsSchemaProperties = {
// how to group
groupBy: schema.string({ validate: validateGroupBy, defaultValue: 'all' }),
// field to group on (for groupBy: top)
termField: schema.maybe(schema.string({ minLength: 1 })),
termField: schema.maybe(
schema.oneOf([
schema.string({ minLength: 1 }),
schema.arrayOf(schema.string(), { minSize: 2, maxSize: MAX_SELECTABLE_GROUP_BY_TERMS }),
ymao1 marked this conversation as resolved.
Show resolved Hide resolved
])
),
// limit on number of groups returned
termSize: schema.maybe(schema.number({ min: 1 })),
searchType: schema.oneOf(
Expand Down
10 changes: 5 additions & 5 deletions x-pack/plugins/triggers_actions_ui/README.md
Expand Up @@ -535,10 +535,10 @@ Props definition:
interface GroupByExpressionProps {
groupBy: string;
termSize?: number;
termField?: string;
termField?: string | string[];
errors: { [key: string]: string[] };
onChangeSelectedTermSize: (selectedTermSize?: number) => void;
onChangeSelectedTermField: (selectedTermField?: string) => void;
onChangeSelectedTermField: (selectedTermField?: string | string[]) => void;
onChangeSelectedGroupBy: (selectedGroupBy?: string) => void;
fields: Record<string, any>;
customGroupByTypes?: {
Expand All @@ -555,9 +555,9 @@ interface GroupByExpressionProps {
| termSize | Selected term size that will be set as the alert type property. |
| termField | Selected term field that will be set as the alert type property. |
| errors | List of errors with proper messages for the alert params that should be validated. In current component is validated `termSize` and `termField`. |
| onChangeSelectedTermSize | Event handler that will be excuted if selected term size is changed. |
| onChangeSelectedTermField | Event handler that will be excuted if selected term field is changed. |
| onChangeSelectedGroupBy | Event handler that will be excuted if selected group by is changed. |
| onChangeSelectedTermSize | Event handler that will be executed if selected term size is changed. |
| onChangeSelectedTermField | Event handler that will be executed if selected term field is changed. |
| onChangeSelectedGroupBy | Event handler that will be executed if selected group by is changed. |
| fields | Fields list with options for the `termField` dropdown. |
| customGroupByTypes | (Optional) List of group by types that replaces the default options defined in constants `x-pack/plugins/triggers_actions_ui/public/common/constants/group_by_types.ts`. |
| popupPosition | (Optional) expression popup position. Default is `downLeft`. Recommend changing it for a small parent window space. |
Expand Down