From c93715f1163eef16cc119704aa99540e9023db73 Mon Sep 17 00:00:00 2001 From: AWSHurneyt <79280347+AWSHurneyt@users.noreply.github.com> Date: Fri, 15 Oct 2021 10:11:34 -0700 Subject: [PATCH] Fixed a bug that was preventing the AcknowledgeAlerts API from acknowledging more than 10 alerts at once. (#205) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 <79280347+AWSHurneyt@users.noreply.github.com> * Add Integtest.sh for OpenSearch integtest setups (#121) * Add integtest script to the repo Signed-off-by: Peter Zhu * Add Alerting specific security param for integTest Signed-off-by: Peter Zhu * Remove default assignee (#127) Signed-off-by: Ashish Agrawal * Removing All Usages of Action Get Method Calls and adding the listeners (#130) Signed-off-by: Aditya Jindal * Fix snapshot build and increment to 1.1.0. (#142) Signed-off-by: dblock * Refactor MonitorRunner (#143) Signed-off-by: Mohammad Qureshi * Update Bucket-Level Alerting RFC (#145) Signed-off-by: Mohammad Qureshi * Add BucketSelector pipeline aggregation extension (#144) Signed-off-by: Mohammad Qureshi Co-authored-by: Rishabh Maurya * Add AggregationResultBucket (#148) Signed-off-by: Mohammad Qureshi Co-authored-by: Rishabh Maurya * Add ActionExecutionPolicy (#149) * Add ActionExecutionPolicy Signed-off-by: Mohammad Qureshi * Throw exception if there is an invalid field in PER_ALERT config when parsing Signed-off-by: Mohammad Qureshi * 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 * 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 * Require condition to not be null when parsing Bucket-Level Trigger Signed-off-by: Mohammad Qureshi * Update InputService for Bucket-Level Alerting (#152) Signed-off-by: Mohammad Qureshi Co-authored-by: Rishabh Maurya * Update TriggerService for Bucket-Level Alerting (#153) * Update TriggerService for Bucket-Level Alerting Signed-off-by: Mohammad Qureshi * Remove client from TriggerService Signed-off-by: Mohammad Qureshi * Update AlertService for Bucket-Level Alerting (#154) * Update AlertService for Bucket-Level Alerting Signed-off-by: Mohammad Qureshi * Move Alert search size for Bucket-Level Monitors to a const Signed-off-by: Mohammad Qureshi * Add worksheets to help with testing (#151) Signed-off-by: Mohammad Qureshi * Update MonitorRunner for Bucket-Level Alerting (#155) * Update MonitorRunner for Bucket-Level Alerting Signed-off-by: Mohammad Qureshi * Update regressed comment in MonitorRunnerIT Signed-off-by: Mohammad Qureshi * Add TODO to break down runBucketLevelMonitor method in MonitorRunner Signed-off-by: Mohammad Qureshi * Fix ktlint formatting issues (#156) Signed-off-by: Mohammad Qureshi * Execute Actions on runTrigger exceptions for Bucket-Level Monitor (#157) Signed-off-by: Mohammad Qureshi * Skip execution of Actions on ACKNOWLEDGED Alerts for Bucket-Level Monitors (#158) Signed-off-by: Mohammad Qureshi * Return first page of input results in MonitorRunResult for Bucket-Level Monitor (#159) Signed-off-by: Mohammad Qureshi * Add setting to limit per alert action executions and don't save Alerts for test Bucket-Level Monitors (#161) Signed-off-by: Mohammad Qureshi * 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 * Change trigger after key conditionals to when statement Signed-off-by: Mohammad Qureshi * 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 * 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 Co-authored-by: AWSHurneyt <79280347+AWSHurneyt@users.noreply.github.com> Co-authored-by: Peter Zhu Co-authored-by: Ashish Agrawal Co-authored-by: Daniel Doubrovkine (dB.) Co-authored-by: Mohammad Qureshi <47198598+qreshi@users.noreply.github.com> Co-authored-by: Rishabh Maurya Co-authored-by: Sriram <59816283+skkosuri-amzn@users.noreply.github.com> * Add release notes for 1.1.0.0 release (#166) (#167) Signed-off-by: Mohammad Qureshi * Remove default integtest.sh. (#181) Signed-off-by: dblock * Add valid search filters. (#191) * Add valid search filters. * Added this fix to release notes * Publish notification JARs checksums. (#197) Signed-off-by: dblock * Also publish SHA 256 and 512 checksums. (#198) * Also publish SHA 256 and 512 checksums. Signed-off-by: dblock * Remove sonatype staging. Signed-off-by: dblock * Fixed a bug that was preventing the AcknowledgeAlerts API from acknowledging more than 10 alerts at once. Signed-off-by: Thomas Hurney * Implemented integration tests to ensure fix for issue 203 is working as expected. Signed-off-by: Thomas Hurney * Refactored integ tests based on PR feedback, and listed the bug fix in the release notes. Signed-off-by: Thomas Hurney * Removing bug fixes from release notes. Currently discussing adding separate notes for this patch. Signed-off-by: Thomas Hurney Co-authored-by: Aditya Jindal <13850971+aditjind@users.noreply.github.com> Co-authored-by: Peter Zhu Co-authored-by: Ashish Agrawal Co-authored-by: Daniel Doubrovkine (dB.) Co-authored-by: Mohammad Qureshi <47198598+qreshi@users.noreply.github.com> Co-authored-by: Rishabh Maurya Co-authored-by: Sriram <59816283+skkosuri-amzn@users.noreply.github.com> --- .github/workflows/test-workflow.yml | 1 + .../TransportAcknowledgeAlertAction.kt | 8 ++- .../alerting/resthandler/MonitorRestApiIT.kt | 72 +++++++++++++++++++ ...ensearch-alerting.release-notes-1.1.0.0.md | 2 +- 4 files changed, 81 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test-workflow.yml b/.github/workflows/test-workflow.yml index 30de39ef5..5dba154c3 100644 --- a/.github/workflows/test-workflow.yml +++ b/.github/workflows/test-workflow.yml @@ -29,6 +29,7 @@ jobs: with: java-version: ${{ matrix.java }} + - name: Build and run with Gradle run: ./gradlew build -Dopensearch.version=1.2.0-SNAPSHOT diff --git a/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportAcknowledgeAlertAction.kt b/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportAcknowledgeAlertAction.kt index ae8653dbd..2780277a0 100644 --- a/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportAcknowledgeAlertAction.kt +++ b/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportAcknowledgeAlertAction.kt @@ -75,7 +75,13 @@ class TransportAcknowledgeAlertAction @Inject constructor( val searchRequest = SearchRequest() .indices(AlertIndices.ALERT_INDEX) .routing(request.monitorId) - .source(SearchSourceBuilder().query(queryBuilder).version(true).seqNoAndPrimaryTerm(true)) + .source( + SearchSourceBuilder() + .query(queryBuilder) + .version(true) + .seqNoAndPrimaryTerm(true) + .size(request.alertIds.size) + ) client.search( searchRequest, diff --git a/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/MonitorRestApiIT.kt b/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/MonitorRestApiIT.kt index 730b3b241..ccc23f894 100644 --- a/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/MonitorRestApiIT.kt +++ b/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/MonitorRestApiIT.kt @@ -569,6 +569,78 @@ class MonitorRestApiIT : AlertingRestTestCase() { assertFalse("Alert in state ${activeAlert.state} found in failed list", failedResponseList.contains(activeAlert.id)) } + fun `test acknowledging more than 10 alerts at once`() { + // GIVEN + putAlertMappings() // Required as we do not have a create alert API. + val monitor = createRandomMonitor(refresh = true) + val alertsToAcknowledge = (1..15).map { createAlert(randomAlert(monitor).copy(state = Alert.State.ACTIVE)) }.toTypedArray() + + // WHEN + val response = acknowledgeAlerts(monitor, *alertsToAcknowledge) + + // THEN + val responseMap = response.asMap() + val expectedAcknowledgedCount = alertsToAcknowledge.size + + val acknowledgedAlerts = responseMap["success"] as List + assertTrue("Expected $expectedAcknowledgedCount alerts to be acknowledged successfully.", acknowledgedAlerts.size == expectedAcknowledgedCount) + + val acknowledgedAlertsList = acknowledgedAlerts.toString() + alertsToAcknowledge.forEach { alert -> assertTrue("Alert with ID ${alert.id} not found in failed list.", acknowledgedAlertsList.contains(alert.id)) } + + val failedResponse = responseMap["failed"] as List + assertTrue("Expected 0 alerts to fail acknowledgment.", failedResponse.isEmpty()) + } + + fun `test acknowledging more than 10 alerts at once, including acknowledged alerts`() { + // GIVEN + putAlertMappings() // Required as we do not have a create alert API. + val monitor = createRandomMonitor(refresh = true) + val alertsGroup1 = (1..15).map { createAlert(randomAlert(monitor).copy(state = Alert.State.ACTIVE)) }.toTypedArray() + acknowledgeAlerts(monitor, *alertsGroup1) // Acknowledging the first array of alerts. + + val alertsGroup2 = (1..15).map { createAlert(randomAlert(monitor).copy(state = Alert.State.ACTIVE)) }.toTypedArray() + + val alertsToAcknowledge = arrayOf(*alertsGroup1, *alertsGroup2) // Creating an array of alerts that includes alerts that have been already acknowledged, and new alerts. + + // WHEN + val response = acknowledgeAlerts(monitor, *alertsToAcknowledge) + + // THEN + val responseMap = response.asMap() + val expectedAcknowledgedCount = alertsToAcknowledge.size - alertsGroup1.size + + val acknowledgedAlerts = responseMap["success"] as List + assertTrue("Expected $expectedAcknowledgedCount alerts to be acknowledged successfully.", acknowledgedAlerts.size == expectedAcknowledgedCount) + + val acknowledgedAlertsList = acknowledgedAlerts.toString() + alertsGroup2.forEach { alert -> assertTrue("Alert with ID ${alert.id} not found in failed list.", acknowledgedAlertsList.contains(alert.id)) } + alertsGroup1.forEach { alert -> assertFalse("Alert with ID ${alert.id} found in failed list.", acknowledgedAlertsList.contains(alert.id)) } + + val failedResponse = responseMap["failed"] as List + assertTrue("Expected ${alertsGroup1.size} alerts to fail acknowledgment.", failedResponse.size == alertsGroup1.size) + + val failedResponseList = failedResponse.toString() + alertsGroup1.forEach { alert -> assertTrue("Alert with ID ${alert.id} not found in failed list.", failedResponseList.contains(alert.id)) } + alertsGroup2.forEach { alert -> assertFalse("Alert with ID ${alert.id} found in failed list.", failedResponseList.contains(alert.id)) } + } + + @Throws(Exception::class) + fun `test acknowledging 0 alerts`() { + // GIVEN + putAlertMappings() // Required as we do not have a create alert API. + val monitor = createRandomMonitor(refresh = true) + val alertsToAcknowledge = arrayOf() + + // WHEN & THEN + try { + acknowledgeAlerts(monitor, *alertsToAcknowledge) + fail("Expected acknowledgeAlerts to throw an exception.") + } catch (e: ResponseException) { + assertEquals("Unexpected status", RestStatus.BAD_REQUEST, e.response.restStatus()) + } + } + fun `test get all alerts in all states`() { putAlertMappings() // Required as we do not have a create alert API. val monitor = createRandomMonitor(refresh = true) diff --git a/release-notes/opensearch-alerting.release-notes-1.1.0.0.md b/release-notes/opensearch-alerting.release-notes-1.1.0.0.md index 2ec826afd..59d83ea09 100644 --- a/release-notes/opensearch-alerting.release-notes-1.1.0.0.md +++ b/release-notes/opensearch-alerting.release-notes-1.1.0.0.md @@ -42,4 +42,4 @@ Compatible with OpenSearch 1.1.0 ### Refactoring -* Refactor MonitorRunner ([#143](https://github.com/opensearch-project/alerting/pull/143)) \ No newline at end of file +* Refactor MonitorRunner ([#143](https://github.com/opensearch-project/alerting/pull/143))