-
Notifications
You must be signed in to change notification settings - Fork 105
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.
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
Update MonitorRunner for Bucket-Level Alerting #155
Conversation
Signed-off-by: Mohammad Qureshi <[email protected]>
alerting/src/test/kotlin/org/opensearch/alerting/MonitorRunnerIT.kt
Outdated
Show resolved
Hide resolved
// TODO: This is largely a duplicate of runAction above for BucketLevelTriggerExecutionContext for now. | ||
// After suppression logic implementation, if this remains mostly the same, it can be refactored. | ||
private suspend fun runAction(action: Action, ctx: BucketLevelTriggerExecutionContext, dryrun: Boolean): ActionRunResult { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do you want to revisit it now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's get main caught up first. Once it's stable and I make some of the functional changes that we discussed in the previous PRs, I'll try to do some TODO code clean-up. Just to make sure we prioritize the bigger things for release kick-off.
@@ -348,6 +320,199 @@ object MonitorRunner : JobRunner, CoroutineScope, AbstractLifecycleComponent() { | |||
return monitorResult.copy(triggerResults = triggerResults) | |||
} | |||
|
|||
suspend fun runBucketLevelMonitor( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function is giant, let's break it down.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. Although, there are some follow-up changes that we're targeting for 1.2's release that will require some refactoring here. If you don't find it to be a blocker, could we revisit this breakdown during that refactor? If so, I'll add a TODO
comment here and revisit this then.
Signed-off-by: Mohammad Qureshi <[email protected]>
Signed-off-by: Mohammad Qureshi <[email protected]>
* Added release notes for OpenSearch 1.0.0.0. (#123) (#124) Co-authored-by: AWSHurneyt <[email protected]> * Add Integtest.sh for OpenSearch integtest setups (#121) * Add integtest script to the repo Signed-off-by: Peter Zhu <[email protected]> * Add Alerting specific security param for integTest Signed-off-by: Peter Zhu <[email protected]> * Remove default assignee (#127) Signed-off-by: Ashish Agrawal <[email protected]> * Removing All Usages of Action Get Method Calls and adding the listeners (#130) Signed-off-by: Aditya Jindal <[email protected]> * Fix snapshot build and increment to 1.1.0. (#142) Signed-off-by: dblock <[email protected]> * Refactor MonitorRunner (#143) Signed-off-by: Mohammad Qureshi <[email protected]> * Update Bucket-Level Alerting RFC (#145) Signed-off-by: Mohammad Qureshi <[email protected]> * Add BucketSelector pipeline aggregation extension (#144) Signed-off-by: Mohammad Qureshi <[email protected]> Co-authored-by: Rishabh Maurya <[email protected]> * Add AggregationResultBucket (#148) Signed-off-by: Mohammad Qureshi <[email protected]> Co-authored-by: Rishabh Maurya <[email protected]> * Add ActionExecutionPolicy (#149) * Add ActionExecutionPolicy Signed-off-by: Mohammad Qureshi <[email protected]> * Throw exception if there is an invalid field in PER_ALERT config when parsing Signed-off-by: Mohammad Qureshi <[email protected]> * Don't allow throttle to be configured for PerExecutionActionScope at the data class level since it is not supported yet Signed-off-by: Mohammad Qureshi <[email protected]> * Refactor Monitor and Trigger to split into Query-Level and Bucket-Lev… (#150) * Refactor Monitor and Trigger to split into Query-Level and Bucket-Level Monitors Signed-off-by: Mohammad Qureshi <[email protected]> * Require condition to not be null when parsing Bucket-Level Trigger Signed-off-by: Mohammad Qureshi <[email protected]> * Update InputService for Bucket-Level Alerting (#152) Signed-off-by: Mohammad Qureshi <[email protected]> Co-authored-by: Rishabh Maurya <[email protected]> * Update TriggerService for Bucket-Level Alerting (#153) * Update TriggerService for Bucket-Level Alerting Signed-off-by: Mohammad Qureshi <[email protected]> * Remove client from TriggerService Signed-off-by: Mohammad Qureshi <[email protected]> * Update AlertService for Bucket-Level Alerting (#154) * Update AlertService for Bucket-Level Alerting Signed-off-by: Mohammad Qureshi <[email protected]> * Move Alert search size for Bucket-Level Monitors to a const Signed-off-by: Mohammad Qureshi <[email protected]> * Add worksheets to help with testing (#151) Signed-off-by: Mohammad Qureshi <[email protected]> * Update MonitorRunner for Bucket-Level Alerting (#155) * Update MonitorRunner for Bucket-Level Alerting Signed-off-by: Mohammad Qureshi <[email protected]> * Update regressed comment in MonitorRunnerIT Signed-off-by: Mohammad Qureshi <[email protected]> * Add TODO to break down runBucketLevelMonitor method in MonitorRunner Signed-off-by: Mohammad Qureshi <[email protected]> * Fix ktlint formatting issues (#156) Signed-off-by: Mohammad Qureshi <[email protected]> * Execute Actions on runTrigger exceptions for Bucket-Level Monitor (#157) Signed-off-by: Mohammad Qureshi <[email protected]> * Skip execution of Actions on ACKNOWLEDGED Alerts for Bucket-Level Monitors (#158) Signed-off-by: Mohammad Qureshi <[email protected]> * Return first page of input results in MonitorRunResult for Bucket-Level Monitor (#159) Signed-off-by: Mohammad Qureshi <[email protected]> * Add setting to limit per alert action executions and don't save Alerts for test Bucket-Level Monitors (#161) Signed-off-by: Mohammad Qureshi <[email protected]> * Fix bug in paginating multiple bucket paths for Bucket-Level Monitor (#163) * Fix bug in paginating multiple bucket paths for Bucket-Level Monitor Signed-off-by: Mohammad Qureshi <[email protected]> * Change trigger after key conditionals to when statement Signed-off-by: Mohammad Qureshi <[email protected]> * Various bug fixes pertaining to throttling on PER_ALERT, saving COMPLETED Alerts and rewriting input query for Bucket-Level Monitors (#164) Signed-off-by: Mohammad Qureshi <[email protected]> * Return only monitors for /monitors/_search. (#162) * Return only monitors for /monitors/_search. * Added missing imports * Added additional check to the unit test * Resolve default for ActionExecutionPolicy at runtime (#165) Signed-off-by: Mohammad Qureshi <[email protected]> Co-authored-by: AWSHurneyt <[email protected]> Co-authored-by: Peter Zhu <[email protected]> Co-authored-by: Ashish Agrawal <[email protected]> Co-authored-by: Daniel Doubrovkine (dB.) <[email protected]> Co-authored-by: Mohammad Qureshi <[email protected]> Co-authored-by: Rishabh Maurya <[email protected]> Co-authored-by: Sriram <[email protected]>
…ledging more than 10 alerts at once. (#205) * Added release notes for OpenSearch 1.0.0.0. (#123) * Merge commits from the main branch to the 1.x branch. (#133) * Added release notes for OpenSearch 1.0.0.0. (#123) (#124) Co-authored-by: AWSHurneyt <[email protected]> * Add Integtest.sh for OpenSearch integtest setups (#121) * Add integtest script to the repo Signed-off-by: Peter Zhu <[email protected]> * Add Alerting specific security param for integTest Signed-off-by: Peter Zhu <[email protected]> * Remove default assignee (#127) Signed-off-by: Ashish Agrawal <[email protected]> * Removing All Usages of Action Get Method Calls and adding the listeners (#130) Signed-off-by: Aditya Jindal <[email protected]> * Fix snapshot build and increment to 1.1.0. (#142) Signed-off-by: dblock <[email protected]> * Refactor MonitorRunner (#143) Signed-off-by: Mohammad Qureshi <[email protected]> * Update Bucket-Level Alerting RFC (#145) Signed-off-by: Mohammad Qureshi <[email protected]> * Add BucketSelector pipeline aggregation extension (#144) Signed-off-by: Mohammad Qureshi <[email protected]> Co-authored-by: Rishabh Maurya <[email protected]> * Add AggregationResultBucket (#148) Signed-off-by: Mohammad Qureshi <[email protected]> Co-authored-by: Rishabh Maurya <[email protected]> * Add ActionExecutionPolicy (#149) * Add ActionExecutionPolicy Signed-off-by: Mohammad Qureshi <[email protected]> * Throw exception if there is an invalid field in PER_ALERT config when parsing Signed-off-by: Mohammad Qureshi <[email protected]> * Don't allow throttle to be configured for PerExecutionActionScope at the data class level since it is not supported yet Signed-off-by: Mohammad Qureshi <[email protected]> * Refactor Monitor and Trigger to split into Query-Level and Bucket-Lev… (#150) * Refactor Monitor and Trigger to split into Query-Level and Bucket-Level Monitors Signed-off-by: Mohammad Qureshi <[email protected]> * Require condition to not be null when parsing Bucket-Level Trigger Signed-off-by: Mohammad Qureshi <[email protected]> * Update InputService for Bucket-Level Alerting (#152) Signed-off-by: Mohammad Qureshi <[email protected]> Co-authored-by: Rishabh Maurya <[email protected]> * Update TriggerService for Bucket-Level Alerting (#153) * Update TriggerService for Bucket-Level Alerting Signed-off-by: Mohammad Qureshi <[email protected]> * Remove client from TriggerService Signed-off-by: Mohammad Qureshi <[email protected]> * Update AlertService for Bucket-Level Alerting (#154) * Update AlertService for Bucket-Level Alerting Signed-off-by: Mohammad Qureshi <[email protected]> * Move Alert search size for Bucket-Level Monitors to a const Signed-off-by: Mohammad Qureshi <[email protected]> * Add worksheets to help with testing (#151) Signed-off-by: Mohammad Qureshi <[email protected]> * Update MonitorRunner for Bucket-Level Alerting (#155) * Update MonitorRunner for Bucket-Level Alerting Signed-off-by: Mohammad Qureshi <[email protected]> * Update regressed comment in MonitorRunnerIT Signed-off-by: Mohammad Qureshi <[email protected]> * Add TODO to break down runBucketLevelMonitor method in MonitorRunner Signed-off-by: Mohammad Qureshi <[email protected]> * Fix ktlint formatting issues (#156) Signed-off-by: Mohammad Qureshi <[email protected]> * Execute Actions on runTrigger exceptions for Bucket-Level Monitor (#157) Signed-off-by: Mohammad Qureshi <[email protected]> * Skip execution of Actions on ACKNOWLEDGED Alerts for Bucket-Level Monitors (#158) Signed-off-by: Mohammad Qureshi <[email protected]> * Return first page of input results in MonitorRunResult for Bucket-Level Monitor (#159) Signed-off-by: Mohammad Qureshi <[email protected]> * Add setting to limit per alert action executions and don't save Alerts for test Bucket-Level Monitors (#161) Signed-off-by: Mohammad Qureshi <[email protected]> * Fix bug in paginating multiple bucket paths for Bucket-Level Monitor (#163) * Fix bug in paginating multiple bucket paths for Bucket-Level Monitor Signed-off-by: Mohammad Qureshi <[email protected]> * Change trigger after key conditionals to when statement Signed-off-by: Mohammad Qureshi <[email protected]> * Various bug fixes pertaining to throttling on PER_ALERT, saving COMPLETED Alerts and rewriting input query for Bucket-Level Monitors (#164) Signed-off-by: Mohammad Qureshi <[email protected]> * Return only monitors for /monitors/_search. (#162) * Return only monitors for /monitors/_search. * Added missing imports * Added additional check to the unit test * Resolve default for ActionExecutionPolicy at runtime (#165) Signed-off-by: Mohammad Qureshi <[email protected]> Co-authored-by: AWSHurneyt <[email protected]> Co-authored-by: Peter Zhu <[email protected]> Co-authored-by: Ashish Agrawal <[email protected]> Co-authored-by: Daniel Doubrovkine (dB.) <[email protected]> Co-authored-by: Mohammad Qureshi <[email protected]> Co-authored-by: Rishabh Maurya <[email protected]> Co-authored-by: Sriram <[email protected]> * Add release notes for 1.1.0.0 release (#166) (#167) Signed-off-by: Mohammad Qureshi <[email protected]> * Remove default integtest.sh. (#181) Signed-off-by: dblock <[email protected]> * Add valid search filters. (#191) * Add valid search filters. * Added this fix to release notes * Publish notification JARs checksums. (#197) Signed-off-by: dblock <[email protected]> * Also publish SHA 256 and 512 checksums. (#198) * Also publish SHA 256 and 512 checksums. Signed-off-by: dblock <[email protected]> * Remove sonatype staging. Signed-off-by: dblock <[email protected]> * Fixed a bug that was preventing the AcknowledgeAlerts API from acknowledging more than 10 alerts at once. Signed-off-by: Thomas Hurney <[email protected]> * Implemented integration tests to ensure fix for issue 203 is working as expected. Signed-off-by: Thomas Hurney <[email protected]> * Refactored integ tests based on PR feedback, and listed the bug fix in the release notes. Signed-off-by: Thomas Hurney <[email protected]> * Removing bug fixes from release notes. Currently discussing adding separate notes for this patch. Signed-off-by: Thomas Hurney <[email protected]> Co-authored-by: Aditya Jindal <[email protected]> Co-authored-by: Peter Zhu <[email protected]> Co-authored-by: Ashish Agrawal <[email protected]> Co-authored-by: Daniel Doubrovkine (dB.) <[email protected]> Co-authored-by: Mohammad Qureshi <[email protected]> Co-authored-by: Rishabh Maurya <[email protected]> Co-authored-by: Sriram <[email protected]>
…ject#133) * Added release notes for OpenSearch 1.0.0.0. (opensearch-project#123) (opensearch-project#124) Co-authored-by: AWSHurneyt <[email protected]> * Add Integtest.sh for OpenSearch integtest setups (opensearch-project#121) * Add integtest script to the repo Signed-off-by: Peter Zhu <[email protected]> * Add Alerting specific security param for integTest Signed-off-by: Peter Zhu <[email protected]> * Remove default assignee (opensearch-project#127) Signed-off-by: Ashish Agrawal <[email protected]> * Removing All Usages of Action Get Method Calls and adding the listeners (opensearch-project#130) Signed-off-by: Aditya Jindal <[email protected]> * Fix snapshot build and increment to 1.1.0. (opensearch-project#142) Signed-off-by: dblock <[email protected]> * Refactor MonitorRunner (opensearch-project#143) Signed-off-by: Mohammad Qureshi <[email protected]> * Update Bucket-Level Alerting RFC (opensearch-project#145) Signed-off-by: Mohammad Qureshi <[email protected]> * Add BucketSelector pipeline aggregation extension (opensearch-project#144) Signed-off-by: Mohammad Qureshi <[email protected]> Co-authored-by: Rishabh Maurya <[email protected]> * Add AggregationResultBucket (opensearch-project#148) Signed-off-by: Mohammad Qureshi <[email protected]> Co-authored-by: Rishabh Maurya <[email protected]> * Add ActionExecutionPolicy (opensearch-project#149) * Add ActionExecutionPolicy Signed-off-by: Mohammad Qureshi <[email protected]> * Throw exception if there is an invalid field in PER_ALERT config when parsing Signed-off-by: Mohammad Qureshi <[email protected]> * Don't allow throttle to be configured for PerExecutionActionScope at the data class level since it is not supported yet Signed-off-by: Mohammad Qureshi <[email protected]> * Refactor Monitor and Trigger to split into Query-Level and Bucket-Lev… (opensearch-project#150) * Refactor Monitor and Trigger to split into Query-Level and Bucket-Level Monitors Signed-off-by: Mohammad Qureshi <[email protected]> * Require condition to not be null when parsing Bucket-Level Trigger Signed-off-by: Mohammad Qureshi <[email protected]> * Update InputService for Bucket-Level Alerting (opensearch-project#152) Signed-off-by: Mohammad Qureshi <[email protected]> Co-authored-by: Rishabh Maurya <[email protected]> * Update TriggerService for Bucket-Level Alerting (opensearch-project#153) * Update TriggerService for Bucket-Level Alerting Signed-off-by: Mohammad Qureshi <[email protected]> * Remove client from TriggerService Signed-off-by: Mohammad Qureshi <[email protected]> * Update AlertService for Bucket-Level Alerting (opensearch-project#154) * Update AlertService for Bucket-Level Alerting Signed-off-by: Mohammad Qureshi <[email protected]> * Move Alert search size for Bucket-Level Monitors to a const Signed-off-by: Mohammad Qureshi <[email protected]> * Add worksheets to help with testing (opensearch-project#151) Signed-off-by: Mohammad Qureshi <[email protected]> * Update MonitorRunner for Bucket-Level Alerting (opensearch-project#155) * Update MonitorRunner for Bucket-Level Alerting Signed-off-by: Mohammad Qureshi <[email protected]> * Update regressed comment in MonitorRunnerIT Signed-off-by: Mohammad Qureshi <[email protected]> * Add TODO to break down runBucketLevelMonitor method in MonitorRunner Signed-off-by: Mohammad Qureshi <[email protected]> * Fix ktlint formatting issues (opensearch-project#156) Signed-off-by: Mohammad Qureshi <[email protected]> * Execute Actions on runTrigger exceptions for Bucket-Level Monitor (opensearch-project#157) Signed-off-by: Mohammad Qureshi <[email protected]> * Skip execution of Actions on ACKNOWLEDGED Alerts for Bucket-Level Monitors (opensearch-project#158) Signed-off-by: Mohammad Qureshi <[email protected]> * Return first page of input results in MonitorRunResult for Bucket-Level Monitor (opensearch-project#159) Signed-off-by: Mohammad Qureshi <[email protected]> * Add setting to limit per alert action executions and don't save Alerts for test Bucket-Level Monitors (opensearch-project#161) Signed-off-by: Mohammad Qureshi <[email protected]> * Fix bug in paginating multiple bucket paths for Bucket-Level Monitor (opensearch-project#163) * Fix bug in paginating multiple bucket paths for Bucket-Level Monitor Signed-off-by: Mohammad Qureshi <[email protected]> * Change trigger after key conditionals to when statement Signed-off-by: Mohammad Qureshi <[email protected]> * Various bug fixes pertaining to throttling on PER_ALERT, saving COMPLETED Alerts and rewriting input query for Bucket-Level Monitors (opensearch-project#164) Signed-off-by: Mohammad Qureshi <[email protected]> * Return only monitors for /monitors/_search. (opensearch-project#162) * Return only monitors for /monitors/_search. * Added missing imports * Added additional check to the unit test * Resolve default for ActionExecutionPolicy at runtime (opensearch-project#165) Signed-off-by: Mohammad Qureshi <[email protected]> Co-authored-by: AWSHurneyt <[email protected]> Co-authored-by: Peter Zhu <[email protected]> Co-authored-by: Ashish Agrawal <[email protected]> Co-authored-by: Daniel Doubrovkine (dB.) <[email protected]> Co-authored-by: Mohammad Qureshi <[email protected]> Co-authored-by: Rishabh Maurya <[email protected]> Co-authored-by: Sriram <[email protected]> Signed-off-by: AWSHurneyt <[email protected]>
Signed-off-by: Mohammad Qureshi [email protected]
Issue #, if available: #86
Description of changes:
CheckList:
[x] Commits are signed per the DCO using --signoff
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.