Fixed a bug that was preventing the AcknowledgeAlerts API from acknowledging more than 10 alerts at once. #205

AWSHurneyt · 2021-10-13T23:24:31Z

Issue #, if available:

[BUG] Cannot acknowledge more than 10 alerts at once. #203
Alerting-Dashboard-Plugin PR #126

Description of changes:

Fixed a bug that was preventing the AcknowledgeAlerts API from acknowledging more than 10 alerts at once. Because TransportAcknowledgeAlertAction::findActiveAlerts wasn't using the number of provided alerts for its size, the search API call was using the default size of 10.

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.

…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]>

…earch-project#167) Signed-off-by: Mohammad Qureshi <[email protected]>

Signed-off-by: dblock <[email protected]>

* Add valid search filters. * Added this fix to release notes

Signed-off-by: dblock <[email protected]>

* Also publish SHA 256 and 512 checksums. Signed-off-by: dblock <[email protected]> * Remove sonatype staging. Signed-off-by: dblock <[email protected]>

…ledging more than 10 alerts at once. Signed-off-by: Thomas Hurney <[email protected]>

skkosuri-amzn · 2021-10-14T03:04:02Z

Could you pls add test case for this change,

…as expected. Signed-off-by: Thomas Hurney <[email protected]>

AWSHurneyt · 2021-10-14T21:03:12Z

Could you pls add test case for this change,

Added a few integration tests to MonitorRestApiIT to test the functionality.

qreshi · 2021-10-14T21:06:25Z

Let's change the target branch to main to merge it in there first. This will also run the GitHub CI since we have it to only run against main (which we should also change later).

AWSHurneyt · 2021-10-14T21:08:03Z

Let's change the target branch to main to merge it in there first. This will also run the GitHub CI since we have it to only run against main (which we should also change later).

Will do.

alerting/src/test/kotlin/org/opensearch/alerting/resthandler/MonitorRestApiIT.kt

codecov-commenter · 2021-10-14T21:42:16Z

Codecov Report

Merging #205 (fd3605e) into main (dc6ecdc) will increase coverage by 0.07%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##               main     #205      +/-   ##
============================================
+ Coverage     78.79%   78.86%   +0.07%     
- Complexity      214      215       +1     
============================================
  Files           172      172              
  Lines          6951     6956       +5     
  Branches        905      905              
============================================
+ Hits           5477     5486       +9     
+ Misses          989      986       -3     
+ Partials        485      484       -1

Impacted Files	Coverage Δ
...rting/transport/TransportAcknowledgeAlertAction.kt	`91.04% <100.00%> (+0.72%)`	⬆️
...in/kotlin/org/opensearch/alerting/MonitorRunner.kt	`71.38% <0.00%> (-0.68%)`	⬇️
...ing/model/destination/DestinationContextFactory.kt	`66.66% <0.00%> (ø)`
...ain/kotlin/org/opensearch/alerting/AlertService.kt	`79.02% <0.00%> (+0.48%)`	⬆️
...lin/org/opensearch/alerting/alerts/AlertIndices.kt	`67.11% <0.00%> (+1.34%)`	⬆️
...alerting/resthandler/RestAcknowledgeAlertAction.kt	`90.00% <0.00%> (+3.33%)`	⬆️
...ing/destination/client/DestinationEmailClient.java	`77.50% <0.00%> (+5.00%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dc6ecdc...fd3605e. Read the comment docs.

…n the release notes. Signed-off-by: Thomas Hurney <[email protected]>

release-notes/opensearch-alerting.release-notes-1.1.0.0.md

…parate notes for this patch. Signed-off-by: Thomas Hurney <[email protected]>

AWSHurneyt and others added 8 commits June 30, 2021 23:06

Added release notes for OpenSearch 1.0.0.0. (opensearch-project#123)

a14ccd4

Add release notes for 1.1.0.0 release (opensearch-project#166) (opens…

e6b5d34

…earch-project#167) Signed-off-by: Mohammad Qureshi <[email protected]>

Remove default integtest.sh. (opensearch-project#181)

05eb22e

Signed-off-by: dblock <[email protected]>

Add valid search filters. (opensearch-project#191)

3a8b874

* Add valid search filters. * Added this fix to release notes

Publish notification JARs checksums. (opensearch-project#197)

0e9ae1f

Signed-off-by: dblock <[email protected]>

Also publish SHA 256 and 512 checksums. (opensearch-project#198)

ff7c1c9

* 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 acknow…

07073ad

…ledging more than 10 alerts at once. Signed-off-by: Thomas Hurney <[email protected]>

AWSHurneyt requested review from annie3431, lezzago, qreshi and skkosuri-amzn October 13, 2021 23:24

Implemented integration tests to ensure fix for issue 203 is working …

9235edd

…as expected. Signed-off-by: Thomas Hurney <[email protected]>

AWSHurneyt changed the base branch from 1.1 to main October 14, 2021 21:08

qreshi reviewed Oct 14, 2021

View reviewed changes

alerting/src/test/kotlin/org/opensearch/alerting/resthandler/MonitorRestApiIT.kt Outdated Show resolved Hide resolved

Merge branch 'main' into issue203fix

fd3605e

qreshi reviewed Oct 14, 2021

View reviewed changes

alerting/src/test/kotlin/org/opensearch/alerting/resthandler/MonitorRestApiIT.kt Outdated Show resolved Hide resolved

Refactored integ tests based on PR feedback, and listed the bug fix i…

e1250d7

…n the release notes. Signed-off-by: Thomas Hurney <[email protected]>

qreshi reviewed Oct 15, 2021

View reviewed changes

release-notes/opensearch-alerting.release-notes-1.1.0.0.md Outdated Show resolved Hide resolved

Removing bug fixes from release notes. Currently discussing adding se…

01ce30a

…parate notes for this patch. Signed-off-by: Thomas Hurney <[email protected]>

qreshi approved these changes Oct 15, 2021

View reviewed changes

skkosuri-amzn approved these changes Oct 15, 2021

View reviewed changes

AWSHurneyt merged commit c93715f into opensearch-project:main Oct 15, 2021

This was referenced Oct 15, 2021

Fixed a bug that was preventing the AcknowledgeAlerts API from acknowledging more than 10 alerts at once. #208

Merged

Fixed a bug that was preventing the AcknowledgeAlerts API from acknowledging more than 10 alerts at once. #209

Closed

AWSHurneyt mentioned this pull request Oct 15, 2021

Fixed a bug that was preventing the AcknowledgeAlerts API from acknowledging more than 10 alerts at once #210

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixed a bug that was preventing the AcknowledgeAlerts API from acknowledging more than 10 alerts at once. #205

Fixed a bug that was preventing the AcknowledgeAlerts API from acknowledging more than 10 alerts at once. #205

AWSHurneyt commented Oct 13, 2021

skkosuri-amzn commented Oct 14, 2021

AWSHurneyt commented Oct 14, 2021

qreshi commented Oct 14, 2021

AWSHurneyt commented Oct 14, 2021

codecov-commenter commented Oct 14, 2021

Fixed a bug that was preventing the AcknowledgeAlerts API from acknowledging more than 10 alerts at once. #205

Fixed a bug that was preventing the AcknowledgeAlerts API from acknowledging more than 10 alerts at once. #205

Conversation

AWSHurneyt commented Oct 13, 2021

skkosuri-amzn commented Oct 14, 2021

AWSHurneyt commented Oct 14, 2021

qreshi commented Oct 14, 2021

AWSHurneyt commented Oct 14, 2021

codecov-commenter commented Oct 14, 2021

Codecov Report