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.

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

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

Merged
merged 12 commits into from
Oct 15, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
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))