Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[ResponseOps] Lazy load dataViews and wrappedSearchSourceClient services when running alerting rules #189929

Merged
merged 13 commits into from
Aug 19, 2024
Merged

[ResponseOps] Lazy load dataViews and wrappedSearchSourceClient services when running alerting rules #189929

merged 13 commits into from
Aug 19, 2024

Conversation

doakalexi
Copy link
Contributor

@doakalexi doakalexi commented Aug 5, 2024 •
edited
Loading

Resolves #184322

Summary

This PR updates getExecutorServices to allow alerting rules to only load the dataViews and wrappedSearchSourceClient services when needed. I updated the rule types dependent on dataViews and/or wrappedSearchSourceClient.

Checklist

To verify

  • Verify that the dataviews and searchsource are only loaded when needed. I think the best way to verify this is create an Index threshold rule and make sure that the dataViews and searchSourceClient services are not created.
  • Verify that the updated rules work correctly. I updated the following rule types:
    Custom threshold
    SLO burn rate
    ES query
    Indicator match

@doakalexi
Copy link
Contributor Author

/ci

@doakalexi
Copy link
Contributor Author

/ci

@doakalexi
Copy link
Contributor Author

/ci

@doakalexi
Fixing tests
69b0b6f
@doakalexi
Merge branch 'alerting/lazy-load-dataviews-searchsource' of github.co…
0895fb1 
…m:doakalexi/kibana into alerting/lazy-load-dataviews-searchsource
@doakalexi
Copy link
Contributor Author

/ci

@doakalexi
Copy link
Contributor Author

/ci

@doakalexi
Fixing test
ebbacb9
@doakalexi
Merge branch 'alerting/lazy-load-dataviews-searchsource' of github.co…
e620f7e 
…m:doakalexi/kibana into alerting/lazy-load-dataviews-searchsource
@doakalexi
Copy link
Contributor Author

/ci

@doakalexi doakalexi changed the title Lazy load the dataviews and search source [ResponseOps] Lazy load dataViews and wrappedSearchSourceClient services when running alerting rules Aug 7, 2024
@doakalexi doakalexi added release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v8.16.0 labels Aug 8, 2024
@doakalexi doakalexi marked this pull request as ready for review August 8, 2024 17:52
@doakalexi doakalexi requested review from a team as code owners August 8, 2024 17:52
@elasticmachine
Copy link
Contributor

Pinging @elastic/response-ops (Team:ResponseOps)

dominiqueclarke
dominiqueclarke reviewed Aug 9, 2024
View reviewed changes
Copy link
Contributor

@dominiqueclarke dominiqueclarke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Obs ux management changes LGTM.

@botelastic botelastic bot added the ci:project-deploy-observability Create an Observability project label Aug 9, 2024
@botelastic botelastic bot added the Team:obs-ux-management Observability Management User Experience Team label Aug 9, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/obs-ux-management-team (Team:obs-ux-management)

nkhristinin
nkhristinin approved these changes Aug 19, 2024
View reviewed changes
Copy link
Contributor

@nkhristinin nkhristinin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DE changes LGTM

ymao1
ymao1 approved these changes Aug 19, 2024
View reviewed changes
Copy link
Contributor

@ymao1 ymao1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Verified that all the impacted rule types ran and generated alerts. Looked at the APM transactions and saw the decrease in transaction latency! 🎉

Before:
Screenshot 2024-08-19 at 1 29 21 PM

After:
Screenshot 2024-08-19 at 1 29 42 PM

Interesting that the ES query latency improved as well even though it is still calling both functions.

@doakalexi doakalexi enabled auto-merge (squash) August 19, 2024 17:39
@kibana-ci
Copy link
Collaborator

kibana-ci commented Aug 19, 2024 •
edited
Loading

💚 Build Succeeded

  • Buildkite Build
  • Commit: dc60120
  • Kibana Serverless Image: docker.elastic.co/kibana-ci/kibana-serverless:pr-189929-dc60120f69dc

Metrics [docs]

✅ unchanged

History

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

@doakalexi doakalexi merged commit cbaa3a8 into elastic:main Aug 19, 2024
44 checks passed
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Aug 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nkhristinin nkhristinin nkhristinin approved these changes

@dominiqueclarke dominiqueclarke dominiqueclarke approved these changes

@ymao1 ymao1 ymao1 approved these changes

@adcoelho adcoelho Awaiting requested review from adcoelho

Assignees
No one assigned
Labels
backport:skip This commit does not require backporting ci:project-deploy-observability Create an Observability project release_note:skip Skip the PR/issue when compiling release notes Team:obs-ux-management Observability Management User Experience Team Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v8.16.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Lazy load dataViews and wrappedSearchSourceClient services when running alerting rules
7 participants
@doakalexi @elasticmachine @kibana-ci @nkhristinin @dominiqueclarke @ymao1 @kibanamachine