Skip to content

Commit

Permalink
Fixed a bug that was preventing the AcknowledgeAlerts API from acknow…
Browse files Browse the repository at this point in the history
…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]>
  • Loading branch information
8 people authored Oct 15, 2021
1 parent dc6ecdc commit c93715f
Show file tree
Hide file tree
Showing 4 changed files with 81 additions and 2 deletions.
1 change: 1 addition & 0 deletions .github/workflows/test-workflow.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<String>
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<String>
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<String>
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<String>
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<Alert>()

// 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)
Expand Down
2 changes: 1 addition & 1 deletion release-notes/opensearch-alerting.release-notes-1.1.0.0.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,4 +42,4 @@ Compatible with OpenSearch 1.1.0

### Refactoring

* Refactor MonitorRunner ([#143](https://github.com/opensearch-project/alerting/pull/143))
* Refactor MonitorRunner ([#143](https://github.com/opensearch-project/alerting/pull/143))

0 comments on commit c93715f

Please sign in to comment.