Skip to content

Commit

Permalink
[ML] AIOps: Improve cleanup of default queries. (elastic#176534)
Browse files Browse the repository at this point in the history
## Summary

- Moves some query utils from the `transform` plugin to
`@kbn/ml-query-utils` to make them available for the `aiops` plugin.
This allows us to better clean up default queries before sending them
off to API endpoints. Some more unit tests have been added as well as
query utils to clean up the default queries we get from EUI search query
bars.
- Adds assertions for url state to `aiops` functional tests. These
ensure that the overall time frame and window parameters for analysis
get correctly set.

### 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
- [x] [Flaky Test
Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was
used on any tests changed
- [x] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
  • Loading branch information
walterra authored Mar 8, 2024
1 parent f9a9ef0 commit 7edaa68
Show file tree
Hide file tree
Showing 34 changed files with 692 additions and 111 deletions.
11 changes: 10 additions & 1 deletion x-pack/packages/ml/query_utils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,17 @@

export { addExcludeFrozenToQuery } from './src/add_exclude_frozen_to_query';
export { buildBaseFilterCriteria } from './src/build_base_filter_criteria';
export { isDefaultQuery } from './src/default_query';
export { ES_CLIENT_TOTAL_HITS_RELATION } from './src/es_client_total_hits_relation';
export { getSafeAggregationName } from './src/get_safe_aggregation_name';
export { matchAllQuery, isMatchAllQuery } from './src/match_all_query';
export { isSimpleQuery } from './src/simple_query';
export { SEARCH_QUERY_LANGUAGE } from './src/types';
export type { SearchQueryLanguage } from './src/types';
export type {
BoolFilterBasedSimpleQuery,
SavedSearchQuery,
SearchQueryLanguage,
SearchQueryVariant,
SimpleQuery,
} from './src/types';
export { getDefaultDSLQuery } from './src/get_default_query';
10 changes: 10 additions & 0 deletions x-pack/packages/ml/query_utils/src/__mocks__/simple_query.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
/*
* 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 type { SearchQueryVariant } from '../types';

export const simpleQueryMock: SearchQueryVariant = { query_string: { query: 'airline:AAL' } };
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
/*
* 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 { isBoolFilterBasedSimpleQuery } from './bool_filter_based_simple_query';
import { simpleQueryMock } from './__mocks__/simple_query';
import { matchAllQuery } from './match_all_query';
import { defaultSimpleQuery } from './simple_query';

describe('isBoolFilterBasedSimpleQuery', () => {
it('should identify bool filter based simple queries', () => {
expect(
isBoolFilterBasedSimpleQuery({
bool: { filter: [simpleQueryMock] },
})
).toBe(true);

expect(
isBoolFilterBasedSimpleQuery({
bool: { filter: [simpleQueryMock], must: [], must_not: [], should: [] },
})
).toBe(true);
});

it('should identify non-simple queries or differently structured simple queries', () => {
expect(isBoolFilterBasedSimpleQuery(defaultSimpleQuery)).toBe(false);
expect(isBoolFilterBasedSimpleQuery(matchAllQuery)).toBe(false);
expect(isBoolFilterBasedSimpleQuery(simpleQueryMock)).toBe(false);

expect(
isBoolFilterBasedSimpleQuery({
bool: { filter: [], must: [], must_not: [], should: [] },
})
).toBe(false);

expect(
isBoolFilterBasedSimpleQuery({
bool: { filter: [] },
})
).toBe(false);

expect(
isBoolFilterBasedSimpleQuery({
bool: { filter: [matchAllQuery], must: [], must_not: [], should: [] },
})
).toBe(false);

expect(
isBoolFilterBasedSimpleQuery({
bool: { filter: [matchAllQuery] },
})
).toBe(false);

expect(
isBoolFilterBasedSimpleQuery({
bool: { filter: [], must: [matchAllQuery], must_not: [] },
})
).toBe(false);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/*
* 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 { isPopulatedObject } from '@kbn/ml-is-populated-object';

import { isSimpleQuery } from './simple_query';
import type { BoolFilterBasedSimpleQuery } from './types';

/**
* Type guard to check if the provided argument is a boolean filter-based simple query.
*
* A valid `BoolFilterBasedSimpleQuery` must have a `bool` property, which itself
* must have a `filter` property. This `filter` must be an array with exactly
* one element, and that element must satisfy the conditions of a simple query
* as defined by `isSimpleQuery`.
*
* The type guard is useful to identify simple queries within bool filter
* queries exposed from Kibana/EUI search bars.
*
* @param arg - The argument to be checked.
* @returns `true` if `arg` meets the criteria of a `BoolFilterBasedSimpleQuery`, otherwise `false`.
*/
export function isBoolFilterBasedSimpleQuery(arg: unknown): arg is BoolFilterBasedSimpleQuery {
return (
isPopulatedObject(arg, ['bool']) &&
isPopulatedObject(arg.bool, ['filter']) &&
Array.isArray(arg.bool.filter) &&
arg.bool.filter.length === 1 &&
isSimpleQuery(arg.bool.filter[0])
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@
import type * as estypes from '@elastic/elasticsearch/lib/api/typesWithBodyKey';
import type { Query } from '@kbn/es-query';

import { isDefaultQuery } from './default_query';
import { isFilterBasedDefaultQuery } from './filter_based_default_query';

/**
* Builds the base filter criteria used in queries,
* adding criteria for the time range and an optional query.
Expand Down Expand Up @@ -38,7 +41,12 @@ export function buildBaseFilterCriteria(
});
}

if (query && typeof query === 'object') {
if (
query &&
typeof query === 'object' &&
!isDefaultQuery(query) &&
!isFilterBasedDefaultQuery(query)
) {
filterCriteria.push(query);
}

Expand Down
19 changes: 19 additions & 0 deletions x-pack/packages/ml/query_utils/src/default_query.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
/*
* 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 { simpleQueryMock } from './__mocks__/simple_query';
import { isDefaultQuery } from './default_query';
import { matchAllQuery } from './match_all_query';
import { defaultSimpleQuery } from './simple_query';

describe('isDefaultQuery', () => {
it("should return if it's a default query", () => {
expect(isDefaultQuery(defaultSimpleQuery)).toBe(true);
expect(isDefaultQuery(matchAllQuery)).toBe(true);
expect(isDefaultQuery(simpleQueryMock)).toBe(false);
});
});
28 changes: 28 additions & 0 deletions x-pack/packages/ml/query_utils/src/default_query.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/*
* 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 { isBoolFilterBasedSimpleQuery } from './bool_filter_based_simple_query';
import { isMatchAllQuery } from './match_all_query';
import { isSimpleDefaultQuery } from './simple_query';
import type { SearchQueryVariant } from './types';

/**
* Checks if the provided query is a default query. A default query is considered as one that matches all documents,
* either directly through a `match_all` query, a `SimpleQuery` with a wildcard query string, or a `BoolFilterBasedSimpleQuery`
* that contains a filter with a wildcard query or a `match_all` condition.
*
* @param query - The query to check.
* @returns True if the query is a default query, false otherwise.
*/
export function isDefaultQuery(query: SearchQueryVariant): boolean {
return (
isMatchAllQuery(query) ||
isSimpleDefaultQuery(query) ||
(isBoolFilterBasedSimpleQuery(query) &&
(query.bool.filter[0].query_string.query === '*' || isMatchAllQuery(query.bool.filter[0])))
);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
/*
* 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 { isFilterBasedDefaultQuery } from './filter_based_default_query';
import { simpleQueryMock } from './__mocks__/simple_query';
import { matchAllQuery } from './match_all_query';
import { defaultSimpleQuery } from './simple_query';

describe('isFilterBasedDefaultQuery', () => {
it('should identify filter based default queries', () => {
expect(
isFilterBasedDefaultQuery({
bool: { filter: [], must: [], must_not: [], should: [] },
})
).toBe(true);
expect(
isFilterBasedDefaultQuery({
bool: { filter: [matchAllQuery], must: [], must_not: [], should: [] },
})
).toBe(true);
expect(
isFilterBasedDefaultQuery({
bool: { filter: [], must: [matchAllQuery], must_not: [] },
})
).toBe(true);
});

it('should identify non-default queries', () => {
expect(isFilterBasedDefaultQuery(defaultSimpleQuery)).toBe(false);
expect(isFilterBasedDefaultQuery(matchAllQuery)).toBe(false);
expect(isFilterBasedDefaultQuery(simpleQueryMock)).toBe(false);
expect(
isFilterBasedDefaultQuery({
bool: { filter: [simpleQueryMock], must: [], must_not: [], should: [] },
})
).toBe(false);
expect(
isFilterBasedDefaultQuery({
bool: { filter: [], must: [matchAllQuery], must_not: [], should: [simpleQueryMock] },
})
).toBe(false);
expect(
isFilterBasedDefaultQuery({
bool: { filter: [], must: [matchAllQuery], must_not: [simpleQueryMock] },
})
).toBe(false);
});
});
44 changes: 44 additions & 0 deletions x-pack/packages/ml/query_utils/src/filter_based_default_query.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/*
* 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 { isPopulatedObject } from '@kbn/ml-is-populated-object';

import { isDefaultQuery } from './default_query';

const boolRequiredAttributes = ['filter', 'must', 'must_not'];

/**
* Determines if the provided argument is a filter-based default query within a boolean filter context.
*
* A valid filter-based default query must include a `bool` property that contains
* `filter`, `must`, and `must_not` properties. These properties should either be empty arrays
* or arrays containing exactly one default query. The function checks for these conditions
* to identify variants of default queries structured within a boolean filter.
*
* Examples of valid structures include:
* - `{ bool: { filter: [{ match_all: {} }], must: [], must_not: [], should: [] } }`
* - `{ bool: { filter: [], must: [{ match_all: {} }], must_not: [] } }`
*
* Useful to identify simple queries within bool queries
* exposed from Kibana/EUI search bars.
*
* @param arg - The argument to be checked, its structure is unknown upfront.
* @returns Returns `true` if `arg` matches the expected structure of a
* filter-based default query, otherwise `false`.
*/
export function isFilterBasedDefaultQuery(arg: unknown): boolean {
return (
isPopulatedObject(arg, ['bool']) &&
isPopulatedObject(arg.bool, boolRequiredAttributes) &&
Object.values(arg.bool).every(
// should be either an empty array or an array with just 1 default query
(d) => {
return Array.isArray(d) && (d.length === 0 || (d.length === 1 && isDefaultQuery(d[0])));
}
)
);
}
16 changes: 16 additions & 0 deletions x-pack/packages/ml/query_utils/src/match_all_query.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
/*
* 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 { isMatchAllQuery, matchAllQuery } from './match_all_query';

describe('isMatchAllQuery', () => {
it("should return if it's a match_all query", () => {
expect(isMatchAllQuery(matchAllQuery)).toBe(true);
expect(isMatchAllQuery({ query_string: { query: '*' } })).toBe(false);
expect(isMatchAllQuery({ query_string: { query: 'airline:AAL' } })).toBe(false);
});
});
32 changes: 32 additions & 0 deletions x-pack/packages/ml/query_utils/src/match_all_query.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/*
* 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 { isPopulatedObject } from '@kbn/ml-is-populated-object';

/**
* Represents a query that matches all documents.
*/
export const matchAllQuery = {
/**
* 'match_all' property specifies a query that matches all documents.
*/
match_all: {},
};

/**
* Checks if an argument is a `match_all` query.
* @param {unknown} query - Argument to check.
* @returns {boolean} True if `query` is a `match_all` query, false otherwise.
*/
export function isMatchAllQuery(query: unknown): boolean {
return (
isPopulatedObject(query, ['match_all']) &&
typeof query.match_all === 'object' &&
query.match_all !== null &&
Object.keys(query.match_all).length === 0
);
}
26 changes: 26 additions & 0 deletions x-pack/packages/ml/query_utils/src/simple_query.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/*
* 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 { simpleQueryMock } from './__mocks__/simple_query';
import { defaultSimpleQuery, isSimpleQuery, isSimpleDefaultQuery } from './simple_query';
import { matchAllQuery } from './match_all_query';

describe('isSimpleQuery', () => {
it("should return if it's a simple query", () => {
expect(isSimpleQuery(defaultSimpleQuery)).toBe(true);
expect(isSimpleQuery(matchAllQuery)).toBe(false);
expect(isSimpleQuery(simpleQueryMock)).toBe(true);
});
});

describe('isSimpleDefaultQuery', () => {
it("should return if it's a simple default query", () => {
expect(isSimpleDefaultQuery(defaultSimpleQuery)).toBe(true);
expect(isSimpleDefaultQuery(matchAllQuery)).toBe(false);
expect(isSimpleDefaultQuery(simpleQueryMock)).toBe(false);
});
});
Loading

0 comments on commit 7edaa68

Please sign in to comment.