From c34f45fe484ba2dc22c5a1dae647451c106482fe Mon Sep 17 00:00:00 2001 From: Surya Sashank Nistala Date: Tue, 12 Sep 2023 18:40:57 -0700 Subject: [PATCH 1/2] add workflow null or empty check only when empty workflow id passed. change Rest get alert handler to add empty string singleton list for workflow ids when none is passed Signed-off-by: Surya Sashank Nistala --- .../alerting/resthandler/RestGetAlertsAction.kt | 2 ++ .../alerting/transport/TransportGetAlertsAction.kt | 13 ++++++++++--- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/alerting/src/main/kotlin/org/opensearch/alerting/resthandler/RestGetAlertsAction.kt b/alerting/src/main/kotlin/org/opensearch/alerting/resthandler/RestGetAlertsAction.kt index cf83cec8c..aabcf8d6c 100644 --- a/alerting/src/main/kotlin/org/opensearch/alerting/resthandler/RestGetAlertsAction.kt +++ b/alerting/src/main/kotlin/org/opensearch/alerting/resthandler/RestGetAlertsAction.kt @@ -61,6 +61,8 @@ class RestGetAlertsAction : BaseRestHandler() { val workflowIds = mutableListOf() if (workflowId.isNullOrEmpty() == false) { workflowIds.add(workflowId) + } else { + workflowIds.add("") } val table = Table( sortOrder, diff --git a/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportGetAlertsAction.kt b/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportGetAlertsAction.kt index 9308d8fb6..7fd825a45 100644 --- a/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportGetAlertsAction.kt +++ b/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportGetAlertsAction.kt @@ -115,7 +115,9 @@ class TransportGetAlertsAction @Inject constructor( if (getAlertsRequest.monitorId != null) { queryBuilder.filter(QueryBuilders.termQuery("monitor_id", getAlertsRequest.monitorId)) - if (getAlertsRequest.workflowIds.isNullOrEmpty()) { + if ( + getAlertsRequest.workflowIds != null && getAlertsRequest.workflowIds!!.size == 1 && getAlertsRequest.workflowIds!![0] == "" + ) { val noWorkflowIdQuery = QueryBuilders.boolQuery() .should(QueryBuilders.boolQuery().mustNot(QueryBuilders.existsQuery(Alert.WORKFLOW_ID_FIELD))) .should(QueryBuilders.termsQuery(Alert.WORKFLOW_ID_FIELD, "")) @@ -123,14 +125,19 @@ class TransportGetAlertsAction @Inject constructor( } } else if (getAlertsRequest.monitorIds.isNullOrEmpty() == false) { queryBuilder.filter(QueryBuilders.termsQuery("monitor_id", getAlertsRequest.monitorIds)) - if (getAlertsRequest.workflowIds.isNullOrEmpty()) { + if ( + getAlertsRequest.workflowIds != null && getAlertsRequest.workflowIds!!.size == 1 && getAlertsRequest.workflowIds!![0] == "" + ) { val noWorkflowIdQuery = QueryBuilders.boolQuery() .should(QueryBuilders.boolQuery().mustNot(QueryBuilders.existsQuery(Alert.WORKFLOW_ID_FIELD))) .should(QueryBuilders.termsQuery(Alert.WORKFLOW_ID_FIELD, "")) queryBuilder.must(noWorkflowIdQuery) } } - if (getAlertsRequest.workflowIds.isNullOrEmpty() == false) { + if ( + getAlertsRequest.workflowIds.isNullOrEmpty() == false && + !(getAlertsRequest.workflowIds!!.size == 1 && getAlertsRequest.workflowIds!![0] == "") + ) { queryBuilder.must(QueryBuilders.termsQuery("workflow_id", getAlertsRequest.workflowIds)) } if (!tableProp.searchString.isNullOrBlank()) { From 7232df7d869537bfa3301494eae6f59badca14ea Mon Sep 17 00:00:00 2001 From: Surya Sashank Nistala Date: Wed, 13 Sep 2023 12:29:11 -0700 Subject: [PATCH 2/2] extract check into method Signed-off-by: Surya Sashank Nistala --- .../transport/TransportGetAlertsAction.kt | 34 ++++++++++--------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportGetAlertsAction.kt b/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportGetAlertsAction.kt index 7fd825a45..11eda858a 100644 --- a/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportGetAlertsAction.kt +++ b/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportGetAlertsAction.kt @@ -41,6 +41,7 @@ import org.opensearch.core.action.ActionListener import org.opensearch.core.xcontent.NamedXContentRegistry import org.opensearch.core.xcontent.XContentParser import org.opensearch.core.xcontent.XContentParserUtils +import org.opensearch.index.query.BoolQueryBuilder import org.opensearch.index.query.Operator import org.opensearch.index.query.QueryBuilders import org.opensearch.search.builder.SearchSourceBuilder @@ -115,24 +116,10 @@ class TransportGetAlertsAction @Inject constructor( if (getAlertsRequest.monitorId != null) { queryBuilder.filter(QueryBuilders.termQuery("monitor_id", getAlertsRequest.monitorId)) - if ( - getAlertsRequest.workflowIds != null && getAlertsRequest.workflowIds!!.size == 1 && getAlertsRequest.workflowIds!![0] == "" - ) { - val noWorkflowIdQuery = QueryBuilders.boolQuery() - .should(QueryBuilders.boolQuery().mustNot(QueryBuilders.existsQuery(Alert.WORKFLOW_ID_FIELD))) - .should(QueryBuilders.termsQuery(Alert.WORKFLOW_ID_FIELD, "")) - queryBuilder.must(noWorkflowIdQuery) - } + addWorkflowIdNullOrEmptyCheck(getAlertsRequest, queryBuilder) } else if (getAlertsRequest.monitorIds.isNullOrEmpty() == false) { queryBuilder.filter(QueryBuilders.termsQuery("monitor_id", getAlertsRequest.monitorIds)) - if ( - getAlertsRequest.workflowIds != null && getAlertsRequest.workflowIds!!.size == 1 && getAlertsRequest.workflowIds!![0] == "" - ) { - val noWorkflowIdQuery = QueryBuilders.boolQuery() - .should(QueryBuilders.boolQuery().mustNot(QueryBuilders.existsQuery(Alert.WORKFLOW_ID_FIELD))) - .should(QueryBuilders.termsQuery(Alert.WORKFLOW_ID_FIELD, "")) - queryBuilder.must(noWorkflowIdQuery) - } + addWorkflowIdNullOrEmptyCheck(getAlertsRequest, queryBuilder) } if ( getAlertsRequest.workflowIds.isNullOrEmpty() == false && @@ -175,6 +162,21 @@ class TransportGetAlertsAction @Inject constructor( } } + // we add this check when we want to fetch alerts for monitors not generated as part of a workflow i.e. non-delegate monitor alerts + private fun addWorkflowIdNullOrEmptyCheck( + getAlertsRequest: GetAlertsRequest, + queryBuilder: BoolQueryBuilder, + ) { + if ( + getAlertsRequest.workflowIds != null && getAlertsRequest.workflowIds!!.size == 1 && getAlertsRequest.workflowIds!![0] == "" + ) { + val noWorkflowIdQuery = QueryBuilders.boolQuery() + .should(QueryBuilders.boolQuery().mustNot(QueryBuilders.existsQuery(Alert.WORKFLOW_ID_FIELD))) + .should(QueryBuilders.termsQuery(Alert.WORKFLOW_ID_FIELD, "")) + queryBuilder.must(noWorkflowIdQuery) + } + } + /** Precedence order for resolving alert index to be queried: 1. alertIndex param. 2. alert index mentioned in monitor data sources.