From eb516679cf4e2b27828684dbf38b196f9eefb7dd Mon Sep 17 00:00:00 2001 From: AWSHurneyt Date: Thu, 29 Jun 2023 11:09:14 -0700 Subject: [PATCH 1/7] 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 db55c7bf1..610fdaf99 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 @@ -52,7 +53,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 eff66182b8bdb4121cfdd226509e66f63f95a252 Mon Sep 17 00:00:00 2001 From: AWSHurneyt Date: Wed, 5 Jul 2023 17:25:54 -0700 Subject: [PATCH 2/7] 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 63a4aa015..9f7864a5a 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 @@ -1263,6 +1265,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 0cab4284de2793cb8400d014964486fb1ba5f59f Mon Sep 17 00:00:00 2001 From: AWSHurneyt Date: Wed, 5 Jul 2023 17:45:57 -0700 Subject: [PATCH 3/7] 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 9f7864a5a..db6977617 100644 --- a/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/MonitorRestApiIT.kt +++ b/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/MonitorRestApiIT.kt @@ -1293,7 +1293,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)) @@ -1352,20 +1352,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 ec5b02544775b44447605f868738646de6d08011 Mon Sep 17 00:00:00 2001 From: AWSHurneyt Date: Wed, 5 Jul 2023 17:54:01 -0700 Subject: [PATCH 4/7] 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 db6977617..5ab73786f 100644 --- a/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/MonitorRestApiIT.kt +++ b/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/MonitorRestApiIT.kt @@ -1339,7 +1339,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 04025bfbc6c44ea4623589d5192653db37a561a8 Mon Sep 17 00:00:00 2001 From: AWSHurneyt Date: Thu, 6 Jul 2023 17:50:53 -0700 Subject: [PATCH 5/7] Removed redundant code. Signed-off-by: AWSHurneyt --- .../alerting/resthandler/RestSearchMonitorAction.kt | 8 -------- .../transport/TransportSearchMonitorAction.kt | 13 ++++++++----- 2 files changed, 8 insertions(+), 13 deletions(-) diff --git a/alerting/src/main/kotlin/org/opensearch/alerting/resthandler/RestSearchMonitorAction.kt b/alerting/src/main/kotlin/org/opensearch/alerting/resthandler/RestSearchMonitorAction.kt index 895e3c565..b831ee4c5 100644 --- a/alerting/src/main/kotlin/org/opensearch/alerting/resthandler/RestSearchMonitorAction.kt +++ b/alerting/src/main/kotlin/org/opensearch/alerting/resthandler/RestSearchMonitorAction.kt @@ -97,14 +97,6 @@ class RestSearchMonitorAction( searchSourceBuilder.parseXContent(request.contentOrSourceParamParser()) searchSourceBuilder.fetchSource(context(request)) - val queryBuilder = QueryBuilders.boolQuery().must(searchSourceBuilder.query()) - if (index == SCHEDULED_JOBS_INDEX) { - queryBuilder.filter(QueryBuilders.existsQuery(Monitor.MONITOR_TYPE)) - } - - searchSourceBuilder.query(queryBuilder) - .seqNoAndPrimaryTerm(true) - .version(true) val searchRequest = SearchRequest() .source(searchSourceBuilder) .indices(index) 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 610fdaf99..82a830fe7 100644 --- a/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportSearchMonitorAction.kt +++ b/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportSearchMonitorAction.kt @@ -23,6 +23,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 @@ -51,12 +52,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 d51ee9ee514b7bae8bd1f31e78ec2786b1d1c9b6 Mon Sep 17 00:00:00 2001 From: AWSHurneyt Date: Fri, 7 Jul 2023 12:07:15 -0700 Subject: [PATCH 6/7] Fixed ktlint errors. Signed-off-by: AWSHurneyt --- .../opensearch/alerting/resthandler/RestSearchMonitorAction.kt | 2 -- .../alerting/transport/TransportSearchMonitorAction.kt | 1 - 2 files changed, 3 deletions(-) diff --git a/alerting/src/main/kotlin/org/opensearch/alerting/resthandler/RestSearchMonitorAction.kt b/alerting/src/main/kotlin/org/opensearch/alerting/resthandler/RestSearchMonitorAction.kt index b831ee4c5..5731502d5 100644 --- a/alerting/src/main/kotlin/org/opensearch/alerting/resthandler/RestSearchMonitorAction.kt +++ b/alerting/src/main/kotlin/org/opensearch/alerting/resthandler/RestSearchMonitorAction.kt @@ -21,11 +21,9 @@ import org.opensearch.common.settings.Settings import org.opensearch.common.xcontent.LoggingDeprecationHandler import org.opensearch.common.xcontent.XContentFactory.jsonBuilder import org.opensearch.common.xcontent.XContentType -import org.opensearch.commons.alerting.model.Monitor import org.opensearch.commons.alerting.model.ScheduledJob import org.opensearch.commons.alerting.model.ScheduledJob.Companion.SCHEDULED_JOBS_INDEX import org.opensearch.core.xcontent.ToXContent.EMPTY_PARAMS -import org.opensearch.index.query.QueryBuilders import org.opensearch.rest.BaseRestHandler import org.opensearch.rest.BaseRestHandler.RestChannelConsumer import org.opensearch.rest.BytesRestResponse 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 82a830fe7..ba8aabae7 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 70754e5b6620d77b00251ee297b4addadc85dbf4 Mon Sep 17 00:00:00 2001 From: AWSHurneyt Date: Fri, 7 Jul 2023 14:35:02 -0700 Subject: [PATCH 7/7] 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 ba8aabae7..7f2a5c88f 100644 --- a/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportSearchMonitorAction.kt +++ b/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportSearchMonitorAction.kt @@ -53,7 +53,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