[Search]Add EQL search strategy#78645
Conversation
Since EQL is an x-pack feature, this strategy will live in the x-pack plugin data_enhanced.
|
Pinging @elastic/kibana-app-arch (Team:AppArch) |
* Ensures that the same variable is not used for both test setup and test assertions * Ensures that mocks are reinstantiated on every test
| const eqlClient = context.core.elasticsearch.client.asCurrentUser.eql; | ||
| const uiSettingsClient = await context.core.uiSettings.client; | ||
| const asyncOptions = { | ||
| waitForCompletionTimeout: '100ms', // Wait up to 100ms for the response to return |
There was a problem hiding this comment.
These are needed only if the EQL endpint supports async search
There was a problem hiding this comment.
@rylnd is this strategy a copy of the DSL search strategy, just with a different endpoint?
If so, lets think how we can reuse the code. I wouldn't want to maintain this logic twice.
There was a problem hiding this comment.
@lizozom I've tried to keep it as similar as possible but there's plenty of deviation. I'll see if I can't extract some of the shared logic to some helpers.
This was repeated in five places, time to consolidate.
|
@lizozom I believe this is ready for another 👀 ! I do have a few outstanding questions in the PR description, as well, if you could take a look. |
We export a few new helper functions.
|
|
||
| export const eqlSearchStrategyProvider = ( | ||
| logger: Logger | ||
| ): ISearchStrategy<EqlSearchStrategyRequest, EqlSearchStrategyResponse> => { |
There was a problem hiding this comment.
@rylnd I still suspect the two strategies are identical, except for the endpoint being used.
Don't you think we could do with adding a parameter to the normal strategy and using that one?
Lets talk about this.
Updates documentation accordingly.
|
@elasticmachine merge upstream |
lizozom
left a comment
There was a problem hiding this comment.
Looks good.
We'll extract common code as we go 👍
💚 Build SucceededMetrics [docs]distributable file count
page load bundle size
History
To update your PR or re-run it, just comment with: |
* Add EQL search strategy Since EQL is an x-pack feature, this strategy will live in the x-pack plugin data_enhanced. * Refactor our test setup to minimize shared state * Ensures that the same variable is not used for both test setup and test assertions * Ensures that mocks are reinstantiated on every test * Use explicit top-level exports * Move async search options to a helper function * Move our workaround to a helper function This was repeated in five places, time to consolidate. * Commit documentation changes We export a few new helper functions. * Mark our internal methods as such Updates documentation accordingly. Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
* Add EQL search strategy Since EQL is an x-pack feature, this strategy will live in the x-pack plugin data_enhanced. * Refactor our test setup to minimize shared state * Ensures that the same variable is not used for both test setup and test assertions * Ensures that mocks are reinstantiated on every test * Use explicit top-level exports * Move async search options to a helper function * Move our workaround to a helper function This was repeated in five places, time to consolidate. * Commit documentation changes We export a few new helper functions. * Mark our internal methods as such Updates documentation accordingly. Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Summary
The immediate need for this strategy for 7.10 is security_solution and its EQL detection rules. Since EQL is an x-pack feature, this strategy lives in the data_enhanced plugin.
While EQL does not return partial results, I plumbed through as much async functionality as possible (get, search, delete) using the other search strategies as a guide.
Outstanding questions:
timeoutfrom config do not appear to be supportedmax_concurrent_shard_requestsandtrack_total_hitsdo not appear to be supportedquerystringon options but it's unclear whether I need to do thatChecklist
Delete any items that are not applicable to this PR.
For maintainers