[Discover] Cancel long running requests in Discover alert #130077

dimaanj · 2022-04-13T06:35:55Z

Summary

This PR adds ability to cancel search source request processing when it takes long within Discover alert rule. SearchSource object instance was wraped to pass abortSignal.

searchSource.fetch$({ ...options, abortSignal: abortController.signal });

To propogate such behaviour per each further created searchSource children, the following methods were also overwritten: searchSource.createChild(...), searchSource.createCopy(...), searchSource.create(...),

Checklist

Delete any items that are not applicable to this PR.

Unit or functional tests were updated or added to match the most common scenarios

…rt rule

x-pack/plugins/alerting/server/lib/wrap_scoped_cluster_client.ts

…-long-running-requests-for-alert-rule # Conflicts: # x-pack/plugins/alerting/server/lib/wrap_scoped_cluster_client.ts # x-pack/plugins/alerting/server/types.ts # x-pack/plugins/stack_alerts/server/alert_types/es_query/lib/fetch_search_source_query.ts

dimaanj · 2022-04-21T13:45:18Z

@elasticmachine merge upstream

elasticmachine · 2022-04-25T07:04:08Z

Pinging @elastic/kibana-data-discovery (Team:DataDiscovery)

…-long-running-requests-for-alert-rule # Conflicts: # x-pack/plugins/alerting/server/types.ts # x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/preview_rules_route.ts

dimaanj · 2022-04-25T10:31:00Z

@elasticmachine merge upstream

dimaanj · 2022-04-25T11:39:02Z

@elasticmachine merge upstream

…-long-running-requests-for-alert-rule # Conflicts: # x-pack/plugins/alerting/server/task_runner/task_runner.ts

mikecote

I did an initial pass, looking good! I had two questions before continuing my review 🙏

x-pack/plugins/alerting/common/rule_task_instance.ts

x-pack/plugins/alerting/server/task_runner/task_runner.ts

…-long-running-requests-for-alert-rule # Conflicts: # x-pack/plugins/alerting/common/rule_task_instance.ts # x-pack/plugins/alerting/server/lib/wrap_scoped_cluster_client.ts # x-pack/plugins/alerting/server/task_runner/task_runner.test.ts # x-pack/plugins/alerting/server/task_runner/task_runner.ts # x-pack/plugins/alerting/server/task_runner/task_runner_cancel.test.ts

dimaanj · 2022-04-30T14:02:16Z

@elasticmachine merge upstream

mikecote

I finished my review, I was able to see it working locally 👌 I think type check will catch this but some tests were still referencing searchSourceUtils.

LGTM but I think we should sum up the metrics from search source and scoped cluster client before reporting them.

x-pack/plugins/alerting/server/task_runner/task_runner.ts

...security_solution/server/lib/detection_engine/routes/rules/utils/wrap_search_source_fetch.ts

…-long-running-requests-for-alert-rule # Conflicts: # x-pack/plugins/alerting/server/task_runner/task_runner.ts

dimaanj · 2022-05-14T18:34:22Z

@elasticmachine merge upstream

dimaanj · 2022-05-15T08:01:31Z

@elasticmachine merge upstream

ymao1

LGTM!

x-pack/plugins/alerting/server/lib/wrap_scoped_cluster_client.ts

dimaanj · 2022-05-17T12:16:04Z

@elasticmachine merge upstream

kibana-ci · 2022-05-17T13:50:00Z

💚 Build Succeeded

Metrics [docs]

Unknown metric groups

References to deprecated APIs

id	before	after	diff
`alerting`	5	21	+16
`securitySolution`	48	58	+10
total			+26

History

💛 Build #44719 was flaky b5e1cc9
💔 Build #44715 failed 63df64c
💔 Build #41631 failed 157362d
💚 Build #40491 succeeded 7d03945
💚 Build #40159 succeeded 7c86fe8
💔 Build #40138 failed 1fc32ca

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @dmitriynj

kertal

Code LGTM 👍 , did not test, just review.
Only ask, there are plenty of changes under the hood that should be mentioned in the PR description ... like wrapping the searchSource client also get metrics

x-pack/plugins/alerting/server/lib/wrap_search_source_client.test.ts

YulNaumenko

LGTM

mgiota

LGTM!

[Discover] improve long running requests for search source within ale…

5a4335f

…rt rule

dimaanj added Feature:Discover Discover Application Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. v8.3.0 labels Apr 13, 2022

dimaanj self-assigned this Apr 13, 2022

kertal reviewed Apr 13, 2022

View reviewed changes

x-pack/plugins/alerting/server/lib/wrap_scoped_cluster_client.ts Outdated Show resolved Hide resolved

dimaanj added 2 commits April 20, 2022 16:57

[Discover] add tests

30492e7

kibanamachine and others added 5 commits April 21, 2022 08:45

Merge branch 'main' into improve-long-running-requests-for-alert-rule

4ff15e2

[Discover] fix linting

9e717c2

[Discover] fix unit test

98f90fd

[Discover] add getMetrics test

239b90e

[Discover] fix unit test

12d6312

dimaanj marked this pull request as ready for review April 25, 2022 07:04

dimaanj requested review from a team as code owners April 25, 2022 07:04

dimaanj requested a review from a team April 25, 2022 07:04

Merge branch 'main' of https://github.com/elastic/kibana into improve…

a3e0ff7

…-long-running-requests-for-alert-rule # Conflicts: # x-pack/plugins/alerting/server/types.ts # x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/preview_rules_route.ts

dimaanj added the release_note:skip Skip the PR/issue when compiling release notes label Apr 25, 2022

Merge branch 'main' into improve-long-running-requests-for-alert-rule

1fc32ca

kibanamachine and others added 3 commits April 25, 2022 06:39

Merge branch 'main' into improve-long-running-requests-for-alert-rule

7c86fe8

Merge branch 'main' of https://github.com/elastic/kibana into improve…

4f1a917

…-long-running-requests-for-alert-rule # Conflicts: # x-pack/plugins/alerting/server/task_runner/task_runner.ts

[Discover] merge search clients metrics

7d03945

dimaanj requested review from ymao1 and kertal April 26, 2022 14:00

mikecote reviewed Apr 27, 2022

View reviewed changes

x-pack/plugins/alerting/common/rule_task_instance.ts Outdated Show resolved Hide resolved

x-pack/plugins/alerting/server/task_runner/task_runner.ts Outdated Show resolved Hide resolved

dimaanj added 2 commits April 28, 2022 10:52

[Discover] wrap searchSourceClient

bcb52ae

Merge branch 'main' into improve-long-running-requests-for-alert-rule

157362d

kertal requested a review from mikecote May 3, 2022 10:16

mikecote approved these changes May 5, 2022

View reviewed changes

x-pack/plugins/alerting/server/task_runner/task_runner.ts Outdated Show resolved Hide resolved

YulNaumenko reviewed May 6, 2022

View reviewed changes

...security_solution/server/lib/detection_engine/routes/rules/utils/wrap_search_source_fetch.ts Outdated Show resolved Hide resolved

dimaanj added 2 commits May 11, 2022 18:33

Merge branch 'main' of https://github.com/elastic/kibana into improve…

5b03b8a

…-long-running-requests-for-alert-rule # Conflicts: # x-pack/plugins/alerting/server/task_runner/task_runner.ts

[Discover] add unit tests

80cf064

kibanamachine and others added 2 commits May 14, 2022 13:34

Merge branch 'main' into improve-long-running-requests-for-alert-rule

63df64c

[Discover] replace searchSourceUtils with searchSourceClient in tests

7ab49d7

dimaanj force-pushed the improve-long-running-requests-for-alert-rule branch from 1ea6a98 to 7ab49d7 Compare May 15, 2022 08:00

Merge branch 'main' into improve-long-running-requests-for-alert-rule

b5e1cc9

dimaanj requested a review from YulNaumenko May 15, 2022 12:38

ymao1 approved these changes May 16, 2022

View reviewed changes

x-pack/plugins/alerting/server/lib/wrap_scoped_cluster_client.ts Outdated Show resolved Hide resolved

[Discover] apply suggestions

efb5d59

Merge branch 'main' into improve-long-running-requests-for-alert-rule

59a5ebf

kertal approved these changes May 18, 2022

View reviewed changes

mgiota reviewed May 18, 2022

View reviewed changes

x-pack/plugins/alerting/server/lib/wrap_search_source_client.test.ts Show resolved Hide resolved

dimaanj requested a review from mgiota May 18, 2022 21:05

YulNaumenko approved these changes May 18, 2022

View reviewed changes

dimaanj merged commit fdf2086 into elastic:main May 19, 2022

kibanamachine added the backport:skip This commit does not require backporting label May 19, 2022

mgiota reviewed May 19, 2022

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Discover] Cancel long running requests in Discover alert #130077

[Discover] Cancel long running requests in Discover alert #130077

dimaanj commented Apr 13, 2022 •

edited

Loading

dimaanj commented Apr 21, 2022

elasticmachine commented Apr 25, 2022

dimaanj commented Apr 25, 2022

dimaanj commented Apr 25, 2022

mikecote left a comment

dimaanj commented Apr 30, 2022

mikecote left a comment

dimaanj commented May 14, 2022

dimaanj commented May 15, 2022

ymao1 left a comment

dimaanj commented May 17, 2022

kibana-ci commented May 17, 2022

References to deprecated APIs

kertal left a comment •

edited

Loading

YulNaumenko left a comment

mgiota left a comment

[Discover] Cancel long running requests in Discover alert #130077

[Discover] Cancel long running requests in Discover alert #130077

Conversation

dimaanj commented Apr 13, 2022 • edited Loading

Summary

Checklist

dimaanj commented Apr 21, 2022

elasticmachine commented Apr 25, 2022

dimaanj commented Apr 25, 2022

dimaanj commented Apr 25, 2022

mikecote left a comment

Choose a reason for hiding this comment

dimaanj commented Apr 30, 2022

mikecote left a comment

Choose a reason for hiding this comment

dimaanj commented May 14, 2022

dimaanj commented May 15, 2022

ymao1 left a comment

Choose a reason for hiding this comment

dimaanj commented May 17, 2022

kibana-ci commented May 17, 2022

💚 Build Succeeded

Metrics [docs]

References to deprecated APIs

History

kertal left a comment • edited Loading

Choose a reason for hiding this comment

YulNaumenko left a comment

Choose a reason for hiding this comment

mgiota left a comment

Choose a reason for hiding this comment

dimaanj commented Apr 13, 2022 •

edited

Loading

kertal left a comment •

edited

Loading