Skip to content

Commit

Permalink
[ResponseOps] es query rule params not compatible between 8.6 and 8.7 (
Browse files Browse the repository at this point in the history
…#157710)

Resolves #156856

## Summary

Adds a default value to `aggType` and `groupBy` fields 


### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
  • Loading branch information
doakalexi authored May 15, 2023
1 parent 6bc337c commit 8ecd2d6
Show file tree
Hide file tree
Showing 5 changed files with 78 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,28 @@ describe('ruleType Params validate()', () => {
);
});

it('uses default value "count" if aggType is undefined', async () => {
params.aggType = undefined;
expect(onValidate()).not.toThrow();
});

it('fails for invalid groupBy', async () => {
params.groupBy = 42;
expect(onValidate()).toThrowErrorMatchingInlineSnapshot(
`"[groupBy]: expected value of type [string] but got [number]"`
);

params.groupBy = '-not-a-valid-groupBy-';
expect(onValidate()).toThrowErrorMatchingInlineSnapshot(
`"[groupBy]: invalid groupBy: \\"-not-a-valid-groupBy-\\""`
);
});

it('uses default value "all" if groupBy is undefined', async () => {
params.groupBy = undefined;
expect(onValidate()).not.toThrow();
});

it('fails for invalid aggField', async () => {
params.aggField = 42;
expect(onValidate()).toThrowErrorMatchingInlineSnapshot(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,11 @@ const EsQueryRuleParamsSchemaProperties = {
threshold: schema.arrayOf(schema.number(), { minSize: 1, maxSize: 2 }),
thresholdComparator: getComparatorSchemaType(validateComparator),
// aggregation type
aggType: schema.string({ validate: validateAggType }),
aggType: schema.string({ validate: validateAggType, defaultValue: 'count' }),
// aggregation field
aggField: schema.maybe(schema.string({ minLength: 1 })),
// how to group
groupBy: schema.string({ validate: validateGroupBy }),
groupBy: schema.string({ validate: validateGroupBy, defaultValue: 'all' }),
// field to group on (for groupBy: top)
termField: schema.maybe(schema.string({ minLength: 1 })),
// limit on number of groups returned
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,28 @@ export function runTests(schema: ObjectType, defaultTypeParams: Record<string, u
);
});

it('uses default value "count" if aggType is undefined', async () => {
params.aggType = undefined;
expect(onValidate()).not.toThrow();
});

it('fails for invalid groupBy', async () => {
params.groupBy = 42;
expect(onValidate()).toThrowErrorMatchingInlineSnapshot(
`"[groupBy]: expected value of type [string] but got [number]"`
);

params.groupBy = '-not-a-valid-groupBy-';
expect(onValidate()).toThrowErrorMatchingInlineSnapshot(
`"[groupBy]: invalid groupBy: \\"-not-a-valid-groupBy-\\""`
);
});

it('uses default value "all" if groupBy is undefined', async () => {
params.groupBy = undefined;
expect(onValidate()).not.toThrow();
});

it('fails for invalid aggField', async () => {
params.aggField = 42;
expect(onValidate()).toThrowErrorMatchingInlineSnapshot(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,11 @@ export const CoreQueryParamsSchemaProperties = {
// field in index used for date/time
timeField: schema.string({ minLength: 1 }),
// aggregation type
aggType: schema.string({ validate: validateAggType }),
aggType: schema.string({ validate: validateAggType, defaultValue: 'count' }),
// aggregation field
aggField: schema.maybe(schema.string({ minLength: 1 })),
// how to group
groupBy: schema.string({ validate: validateGroupBy }),
groupBy: schema.string({ validate: validateGroupBy, defaultValue: 'all' }),
// field to group on (for groupBy: top)
termField: schema.maybe(schema.string({ minLength: 1 })),
// filter field
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -979,6 +979,34 @@ export default function ruleTests({ getService }: FtrProviderContext) {
});
});

describe('aggType and groupBy', () => {
it('sets aggType: "count" and groupBy: "all" when they are undefined', async () => {
endDate = new Date().toISOString();

await createEsDocumentsInGroups(ES_GROUPS_TO_WRITE, endDate);

await createRule({
name: 'always fire',
esQuery: `{\n \"query\":{\n \"match_all\" : {}\n }\n}`,
size: 100,
thresholdComparator: '>',
threshold: [0],
aggType: undefined,
groupBy: undefined,
});

const docs = await waitForDocs(2);

expect(docs[0]._source.hits.length).greaterThan(0);
const messagePattern =
/rule 'always fire' is active:\n\n- Value: \d+\n- Conditions Met: Number of matching documents is greater than 0 over 20s\n- Timestamp: \d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}.\d{3}Z/;
expect(docs[0]._source.params.message).to.match(messagePattern);

expect(docs[1]._source.hits.length).to.be(0);
expect(docs[1]._source.params.message).to.match(/rule 'always fire' is recovered/);
});
});

async function waitForDocs(count: number): Promise<any[]> {
return await esTestIndexToolOutput.waitForDocs(
ES_TEST_INDEX_SOURCE,
Expand Down Expand Up @@ -1080,8 +1108,8 @@ export default function ruleTests({ getService }: FtrProviderContext) {
thresholdComparator: params.thresholdComparator,
threshold: params.threshold,
searchType: params.searchType,
aggType: params.aggType ?? 'count',
groupBy: params.groupBy ?? 'all',
aggType: params.aggType,
groupBy: params.groupBy,
aggField: params.aggField,
termField: params.termField,
termSize: params.termSize,
Expand Down

0 comments on commit 8ecd2d6

Please sign in to comment.