From 3e133c5534f634582103dd3a57ef3739a7654d3d Mon Sep 17 00:00:00 2001 From: AWSHurneyt Date: Thu, 29 Jun 2023 11:09:14 -0700 Subject: [PATCH 1/8] Fixed a bug in the search monitor API when which prevent alert counts from being returned. Signed-off-by: AWSHurneyt --- .../alerting/transport/TransportSearchMonitorAction.kt | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportSearchMonitorAction.kt b/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportSearchMonitorAction.kt index 61713b128..fb51a7220 100644 --- a/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportSearchMonitorAction.kt +++ b/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportSearchMonitorAction.kt @@ -13,6 +13,7 @@ import org.opensearch.action.support.ActionFilters import org.opensearch.action.support.HandledTransportAction import org.opensearch.alerting.action.SearchMonitorAction import org.opensearch.alerting.action.SearchMonitorRequest +import org.opensearch.alerting.alerts.AlertIndices.Companion.ALL_ALERT_INDEX_PATTERN import org.opensearch.alerting.opensearchapi.addFilter import org.opensearch.alerting.settings.AlertingSettings import org.opensearch.alerting.util.AlertingException @@ -54,7 +55,13 @@ class TransportSearchMonitorAction @Inject constructor( val searchSourceBuilder = searchMonitorRequest.searchRequest.source() val queryBuilder = if (searchSourceBuilder.query() == null) BoolQueryBuilder() else QueryBuilders.boolQuery().must(searchSourceBuilder.query()) - queryBuilder.filter(QueryBuilders.existsQuery(Monitor.MONITOR_TYPE)) + + if (searchMonitorRequest.searchRequest.indices().size == 1 && + !searchMonitorRequest.searchRequest.indices().contains(ALL_ALERT_INDEX_PATTERN) + ) { + queryBuilder.filter(QueryBuilders.existsQuery(Monitor.MONITOR_TYPE)) + } + searchSourceBuilder.query(queryBuilder) .seqNoAndPrimaryTerm(true) .version(true) From f6798b1c4a8ab311e611834a8bbecf4ceb1e073a Mon Sep 17 00:00:00 2001 From: AWSHurneyt Date: Wed, 5 Jul 2023 17:25:54 -0700 Subject: [PATCH 2/8] Implemented integration test for frontend use case. Signed-off-by: AWSHurneyt --- .../alerting/resthandler/MonitorRestApiIT.kt | 106 ++++++++++++++++++ 1 file changed, 106 insertions(+) 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 0359f8e03..c4b9d82d6 100644 --- a/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/MonitorRestApiIT.kt +++ b/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/MonitorRestApiIT.kt @@ -54,7 +54,9 @@ import org.opensearch.core.xcontent.XContentBuilder import org.opensearch.index.query.QueryBuilders import org.opensearch.rest.RestStatus import org.opensearch.script.Script +import org.opensearch.search.aggregations.AggregationBuilders import org.opensearch.search.builder.SearchSourceBuilder +import org.opensearch.search.sort.SortOrder import org.opensearch.test.OpenSearchTestCase import org.opensearch.test.junit.annotations.TestLogging import org.opensearch.test.rest.OpenSearchRestTestCase @@ -1264,6 +1266,110 @@ class MonitorRestApiIT : AlertingRestTestCase() { } } + /** + * This use case is needed by the frontend plugin for displaying alert counts on the Monitors list page. + * https://github.com/opensearch-project/alerting-dashboards-plugin/blob/main/server/services/MonitorService.js#L235 + */ + fun `test get acknowledged, active, error, and ignored alerts counts`() { + putAlertMappings() + val monitorAlertCounts = hashMapOf>() + val numMonitors = randomIntBetween(1, 10) + repeat(numMonitors) { + val monitor = createRandomMonitor(refresh = true) + + val numAcknowledgedAlerts = randomIntBetween(1, 10) + val numActiveAlerts = randomIntBetween(1, 10) + var numCompletedAlerts = randomIntBetween(1, 10) + val numErrorAlerts = randomIntBetween(1, 10) + val numIgnoredAlerts = randomIntBetween(1, numCompletedAlerts) + numCompletedAlerts -= numIgnoredAlerts + + val alertCounts = hashMapOf( + Alert.State.ACKNOWLEDGED.name to numAcknowledgedAlerts, + Alert.State.ACTIVE.name to numActiveAlerts, + Alert.State.COMPLETED.name to numCompletedAlerts, + Alert.State.ERROR.name to numErrorAlerts, + "IGNORED" to numIgnoredAlerts + ) + monitorAlertCounts[monitor.id] = alertCounts + + repeat(numAcknowledgedAlerts) { + createAlert(randomAlert(monitor).copy(acknowledgedTime = Instant.now(),state = Alert.State.ACKNOWLEDGED)) + } + repeat(numActiveAlerts) { + createAlert(randomAlert(monitor).copy(state = Alert.State.ACTIVE)) + } + repeat(numCompletedAlerts) { + createAlert(randomAlert(monitor).copy(acknowledgedTime = Instant.now(), state = Alert.State.COMPLETED)) + } + repeat(numErrorAlerts) { + createAlert(randomAlert(monitor).copy(state = Alert.State.ERROR)) + } + repeat(numIgnoredAlerts) { + createAlert(randomAlert(monitor).copy(acknowledgedTime = null, state = Alert.State.COMPLETED)) + } + } + + val sourceBuilder = SearchSourceBuilder() + .size(0) + .query(QueryBuilders.termsQuery("monitor_id", monitorAlertCounts.keys)) + .aggregation( + AggregationBuilders + .terms("uniq_monitor_ids").field("monitor_id") + .subAggregation(AggregationBuilders.filter("active", QueryBuilders.termQuery("state", "ACTIVE"))) + .subAggregation(AggregationBuilders.filter("acknowledged", QueryBuilders.termQuery("state", "ACKNOWLEDGED"))) + .subAggregation(AggregationBuilders.filter("errors", QueryBuilders.termQuery("state", "ERROR"))) + .subAggregation( + AggregationBuilders.filter( + "ignored", + QueryBuilders.boolQuery() + .filter(QueryBuilders.termQuery("state", "COMPLETED")) + .mustNot(QueryBuilders.existsQuery("acknowledged_time")) + ) + ) + .subAggregation(AggregationBuilders.max("last_notification_time").field("last_notification_time")) + .subAggregation( + AggregationBuilders.topHits("latest_alert") + .size(1) + .sort("start_time", SortOrder.DESC) + .fetchSource(arrayOf("last_notification_time", "trigger_name"), null) + ) + ) + + val searchResponse = client().makeRequest( + "GET", + "$ALERTING_BASE_URI/_search", + hashMapOf("index" to AlertIndices.ALL_ALERT_INDEX_PATTERN), + NStringEntity(sourceBuilder.toString(), ContentType.APPLICATION_JSON) + ) + val xcp = createParser(XContentType.JSON.xContent(), searchResponse.entity.content).map() + val aggregations = (xcp["aggregations"]!! as Map>) + val uniqMonitorIds = aggregations["uniq_monitor_ids"]!! + val buckets = uniqMonitorIds["buckets"]!! as ArrayList> + + assertEquals("Incorrect number of monitors returned", monitorAlertCounts.keys.size, buckets.size) + buckets.forEach { bucket -> + val id = bucket["key"]!! + val monitorCounts = monitorAlertCounts[id]!! + + val acknowledged = (bucket["acknowledged"]!! as Map)["doc_count"]!! + assertEquals("Incorrect ${Alert.State.ACKNOWLEDGED} count returned for monitor $id", + monitorCounts[Alert.State.ACKNOWLEDGED.name], acknowledged) + + val active = (bucket["active"]!! as Map)["doc_count"]!! + assertEquals("Incorrect ${Alert.State.ACTIVE} count returned for monitor $id", + monitorCounts[Alert.State.ACTIVE.name], active) + + val errors = (bucket["errors"]!! as Map)["doc_count"]!! + assertEquals("Incorrect ${Alert.State.ERROR} count returned for monitor $id", + monitorCounts[Alert.State.ERROR.name], errors) + + val ignored = (bucket["ignored"]!! as Map)["doc_count"]!! + assertEquals("Incorrect IGNORED count returned for monitor $id", + monitorCounts["IGNORED"], ignored) + } + } + private fun validateAlertingStatsNodeResponse(nodesResponse: Map) { assertEquals("Incorrect number of nodes", numberOfNodes, nodesResponse["total"]) assertEquals("Failed nodes found during monitor stats call", 0, nodesResponse["failed"]) From 59003cd26f8ca484934bd4f571cc0bf642797a22 Mon Sep 17 00:00:00 2001 From: AWSHurneyt Date: Wed, 5 Jul 2023 17:45:57 -0700 Subject: [PATCH 3/8] Fixed ktlint errors. Signed-off-by: AWSHurneyt --- .../alerting/resthandler/MonitorRestApiIT.kt | 26 ++++++++++++------- 1 file changed, 17 insertions(+), 9 deletions(-) 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 c4b9d82d6..398d563b8 100644 --- a/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/MonitorRestApiIT.kt +++ b/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/MonitorRestApiIT.kt @@ -1294,7 +1294,7 @@ class MonitorRestApiIT : AlertingRestTestCase() { monitorAlertCounts[monitor.id] = alertCounts repeat(numAcknowledgedAlerts) { - createAlert(randomAlert(monitor).copy(acknowledgedTime = Instant.now(),state = Alert.State.ACKNOWLEDGED)) + createAlert(randomAlert(monitor).copy(acknowledgedTime = Instant.now(), state = Alert.State.ACKNOWLEDGED)) } repeat(numActiveAlerts) { createAlert(randomAlert(monitor).copy(state = Alert.State.ACTIVE)) @@ -1353,20 +1353,28 @@ class MonitorRestApiIT : AlertingRestTestCase() { val monitorCounts = monitorAlertCounts[id]!! val acknowledged = (bucket["acknowledged"]!! as Map)["doc_count"]!! - assertEquals("Incorrect ${Alert.State.ACKNOWLEDGED} count returned for monitor $id", - monitorCounts[Alert.State.ACKNOWLEDGED.name], acknowledged) + assertEquals( + "Incorrect ${Alert.State.ACKNOWLEDGED} count returned for monitor $id", + monitorCounts[Alert.State.ACKNOWLEDGED.name], acknowledged + ) val active = (bucket["active"]!! as Map)["doc_count"]!! - assertEquals("Incorrect ${Alert.State.ACTIVE} count returned for monitor $id", - monitorCounts[Alert.State.ACTIVE.name], active) + assertEquals( + "Incorrect ${Alert.State.ACTIVE} count returned for monitor $id", + monitorCounts[Alert.State.ACTIVE.name], active + ) val errors = (bucket["errors"]!! as Map)["doc_count"]!! - assertEquals("Incorrect ${Alert.State.ERROR} count returned for monitor $id", - monitorCounts[Alert.State.ERROR.name], errors) + assertEquals( + "Incorrect ${Alert.State.ERROR} count returned for monitor $id", + monitorCounts[Alert.State.ERROR.name], errors + ) val ignored = (bucket["ignored"]!! as Map)["doc_count"]!! - assertEquals("Incorrect IGNORED count returned for monitor $id", - monitorCounts["IGNORED"], ignored) + assertEquals( + "Incorrect IGNORED count returned for monitor $id", + monitorCounts["IGNORED"], ignored + ) } } From 207f13201406193c6ab133d84cad291e93e6905f Mon Sep 17 00:00:00 2001 From: AWSHurneyt Date: Wed, 5 Jul 2023 17:54:01 -0700 Subject: [PATCH 4/8] Fixed ktlint errors. Signed-off-by: AWSHurneyt --- .../org/opensearch/alerting/resthandler/MonitorRestApiIT.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 398d563b8..4b0c74928 100644 --- a/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/MonitorRestApiIT.kt +++ b/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/MonitorRestApiIT.kt @@ -1340,7 +1340,7 @@ class MonitorRestApiIT : AlertingRestTestCase() { "GET", "$ALERTING_BASE_URI/_search", hashMapOf("index" to AlertIndices.ALL_ALERT_INDEX_PATTERN), - NStringEntity(sourceBuilder.toString(), ContentType.APPLICATION_JSON) + StringEntity(sourceBuilder.toString(), ContentType.APPLICATION_JSON) ) val xcp = createParser(XContentType.JSON.xContent(), searchResponse.entity.content).map() val aggregations = (xcp["aggregations"]!! as Map>) From f4211ac119d2fd54a0bc15afaa9e9004b9d8f38a Mon Sep 17 00:00:00 2001 From: AWSHurneyt Date: Thu, 6 Jul 2023 17:50:53 -0700 Subject: [PATCH 5/8] Removed redundant code. Signed-off-by: AWSHurneyt --- .../transport/TransportSearchMonitorAction.kt | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportSearchMonitorAction.kt b/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportSearchMonitorAction.kt index fb51a7220..d5c2b5687 100644 --- a/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportSearchMonitorAction.kt +++ b/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportSearchMonitorAction.kt @@ -22,6 +22,7 @@ import org.opensearch.cluster.service.ClusterService import org.opensearch.common.inject.Inject import org.opensearch.common.settings.Settings import org.opensearch.commons.alerting.model.Monitor +import org.opensearch.commons.alerting.model.ScheduledJob import org.opensearch.commons.authuser.User import org.opensearch.index.query.BoolQueryBuilder import org.opensearch.index.query.ExistsQueryBuilder @@ -53,12 +54,14 @@ class TransportSearchMonitorAction @Inject constructor( override fun doExecute(task: Task, searchMonitorRequest: SearchMonitorRequest, actionListener: ActionListener) { val searchSourceBuilder = searchMonitorRequest.searchRequest.source() - val queryBuilder = if (searchSourceBuilder.query() == null) BoolQueryBuilder() - else QueryBuilders.boolQuery().must(searchSourceBuilder.query()) + .seqNoAndPrimaryTerm(true) + .version(true) + val queryBuilder = QueryBuilders.boolQuery().must(searchSourceBuilder.query()) - if (searchMonitorRequest.searchRequest.indices().size == 1 && - !searchMonitorRequest.searchRequest.indices().contains(ALL_ALERT_INDEX_PATTERN) - ) { + // The SearchMonitor API supports one 'index' parameter of either the SCHEDULED_JOBS_INDEX or ALL_ALERT_INDEX_PATTERN. + // When querying the ALL_ALERT_INDEX_PATTERN, we don't want to check whether the MONITOR_TYPE field exists + // because we're querying alert indexes. + if (searchMonitorRequest.searchRequest.indices().contains(ScheduledJob.SCHEDULED_JOBS_INDEX)) { queryBuilder.filter(QueryBuilders.existsQuery(Monitor.MONITOR_TYPE)) } From 2e0f1279b31e272fd64456f085538bf43828b34b Mon Sep 17 00:00:00 2001 From: AWSHurneyt Date: Fri, 7 Jul 2023 12:07:15 -0700 Subject: [PATCH 6/8] Fixed ktlint errors. Signed-off-by: AWSHurneyt --- .../alerting/transport/TransportSearchMonitorAction.kt | 1 - 1 file changed, 1 deletion(-) diff --git a/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportSearchMonitorAction.kt b/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportSearchMonitorAction.kt index d5c2b5687..f7cc9d1b1 100644 --- a/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportSearchMonitorAction.kt +++ b/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportSearchMonitorAction.kt @@ -13,7 +13,6 @@ import org.opensearch.action.support.ActionFilters import org.opensearch.action.support.HandledTransportAction import org.opensearch.alerting.action.SearchMonitorAction import org.opensearch.alerting.action.SearchMonitorRequest -import org.opensearch.alerting.alerts.AlertIndices.Companion.ALL_ALERT_INDEX_PATTERN import org.opensearch.alerting.opensearchapi.addFilter import org.opensearch.alerting.settings.AlertingSettings import org.opensearch.alerting.util.AlertingException From be041d9c14c6f1b0ae069be173ffeb9709cde020 Mon Sep 17 00:00:00 2001 From: AWSHurneyt Date: Fri, 7 Jul 2023 14:34:42 -0700 Subject: [PATCH 7/8] Added missing import. Signed-off-by: AWSHurneyt --- .../org/opensearch/alerting/resthandler/MonitorRestApiIT.kt | 1 + 1 file changed, 1 insertion(+) 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 4b0c74928..0a44f7c9b 100644 --- a/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/MonitorRestApiIT.kt +++ b/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/MonitorRestApiIT.kt @@ -6,6 +6,7 @@ package org.opensearch.alerting.resthandler import org.apache.http.HttpHeaders import org.apache.http.entity.ContentType +import org.apache.http.entity.StringEntity import org.apache.http.message.BasicHeader import org.apache.http.nio.entity.NStringEntity import org.opensearch.alerting.ALERTING_BASE_URI From 969ddfcd8fa86e12cbf29059d2f01ac981beac1a Mon Sep 17 00:00:00 2001 From: AWSHurneyt Date: Fri, 7 Jul 2023 14:35:02 -0700 Subject: [PATCH 8/8] Added back check for null query. Signed-off-by: AWSHurneyt --- .../alerting/transport/TransportSearchMonitorAction.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportSearchMonitorAction.kt b/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportSearchMonitorAction.kt index f7cc9d1b1..ac8a81a30 100644 --- a/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportSearchMonitorAction.kt +++ b/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportSearchMonitorAction.kt @@ -55,7 +55,8 @@ class TransportSearchMonitorAction @Inject constructor( val searchSourceBuilder = searchMonitorRequest.searchRequest.source() .seqNoAndPrimaryTerm(true) .version(true) - val queryBuilder = QueryBuilders.boolQuery().must(searchSourceBuilder.query()) + val queryBuilder = if (searchSourceBuilder.query() == null) BoolQueryBuilder() + else QueryBuilders.boolQuery().must(searchSourceBuilder.query()) // The SearchMonitor API supports one 'index' parameter of either the SCHEDULED_JOBS_INDEX or ALL_ALERT_INDEX_PATTERN. // When querying the ALL_ALERT_INDEX_PATTERN, we don't want to check whether the MONITOR_TYPE field exists