[ML] AIOps: Improve cleanup of default queries. #176534
Conversation
eafbf00 to
ba76eb8
Compare
|
Pinging @elastic/ml-ui (:ml) |
|
✅ 25x Flaky Test Runner: https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/5236 |
| export function isDefaultQuery(query: SearchQueryVariant): boolean { | ||
| return ( | ||
| isMatchAllQuery(query) || | ||
| (isSimpleQuery(query) && query.query_string.query === '*') || |
There was a problem hiding this comment.
we could add a isDefaultSimplyQuery function to check this * value
There was a problem hiding this comment.
Good idea, updated in 9a6c5c7.
| Object.keys(arg.bool).every( | ||
| // should be either an empty array or an array with just 1 default query | ||
| (d) => { | ||
| if (!isPopulatedObject(arg.bool, [d])) return false; |
There was a problem hiding this comment.
Unless I'm misunderstanding this, I don't think this check is needed.
We know d must exist in arg.bool because we've used Object.keys?
If this is the case and this check isn't needed, then this loop could use Object.entries.
There was a problem hiding this comment.
You're right, this is not necessary and more complex than needed; I changed it to just use Object.values because we don't need the key at all. Updated in aa908b2.
| import { defaultSimpleQuery } from './simple_query'; | ||
|
|
||
| describe('isFilterBasedDefaultQuery', () => { | ||
| it('should identify filter based default queries', () => { |
There was a problem hiding this comment.
I would separate should identify filter-based default queries and should identify non-default queries
There was a problem hiding this comment.
👍 updated in 6e6d393.
| export function isFilterBasedSimpleQuery(arg: unknown): arg is FilterBasedSimpleQuery { | ||
| return ( | ||
| isPopulatedObject(arg, ['bool']) && | ||
| isPopulatedObject(arg.bool, ['filter']) && |
There was a problem hiding this comment.
just to clarify, is it intentional that this function does not check for other keys in the bool query?
There was a problem hiding this comment.
In this case yes, we're just interested of a simple query within the filter, not the others options like must and others. I tried to clarify this a bit more in the code, did a bit of renaming and added more comments. The comments include that the purpose of these helpers is to identify stuff in the bool queries the Kibana/EUI search bars expose.
|
@jgowdyelastic @darnautov addressed your comments, ready for another look. |
…kibana into ml-176387-fix-functional-tests
💚 Build Succeeded
Metrics [docs]Module Count
Async chunks
Page load bundle
History
To update your PR or re-run it, just comment with: cc @walterra |
Summary
transformplugin to@kbn/ml-query-utilsto make them available for theaiopsplugin. 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.aiopsfunctional tests. These ensure that the overall time frame and window parameters for analysis get correctly set.Checklist