From de32d9314efa99b6adfdd00d0e1e8dfb106b8e38 Mon Sep 17 00:00:00 2001 From: "opensearch-trigger-bot[bot]" <98922864+opensearch-trigger-bot[bot]@users.noreply.github.com> Date: Fri, 14 Oct 2022 13:58:58 -0700 Subject: [PATCH] Enhance Get Alerts and Get Findings for list of monitors in bulk (#590) (#605) Signed-off-by: Surya Sashank Nistala (cherry picked from commit f4a3509453de2bce1aaa45f5a47cd26b4136624d) Co-authored-by: Surya Sashank Nistala --- .../transport/TransportGetAlertsAction.kt | 2 + .../transport/TransportGetFindingsAction.kt | 6 ++ .../alerting/MonitorDataSourcesIT.kt | 86 +++++++++++-------- 3 files changed, 60 insertions(+), 34 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 aca172a46..756caeeab 100644 --- a/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportGetAlertsAction.kt +++ b/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportGetAlertsAction.kt @@ -101,6 +101,8 @@ class TransportGetAlertsAction @Inject constructor( if (getAlertsRequest.monitorId != null) { queryBuilder.filter(QueryBuilders.termQuery("monitor_id", getAlertsRequest.monitorId)) + } else if (getAlertsRequest.monitorIds.isNullOrEmpty() == false) { + queryBuilder.filter(QueryBuilders.termsQuery("monitor_id", getAlertsRequest.monitorIds)) } if (!tableProp.searchString.isNullOrBlank()) { queryBuilder diff --git a/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportGetFindingsAction.kt b/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportGetFindingsAction.kt index 9b49968bc..b99c5c381 100644 --- a/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportGetFindingsAction.kt +++ b/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportGetFindingsAction.kt @@ -104,6 +104,12 @@ class TransportGetFindingsSearchAction @Inject constructor( if (!getFindingsRequest.findingId.isNullOrBlank()) queryBuilder.filter(QueryBuilders.termQuery("_id", getFindingsRequest.findingId)) + if (getFindingsRequest.monitorId != null) { + queryBuilder.filter(QueryBuilders.termQuery("monitor_id", getFindingsRequest.monitorId)) + } else if (getFindingsRequest.monitorIds.isNullOrEmpty() == false) { + queryBuilder.filter(QueryBuilders.termsQuery("monitor_id", getFindingsRequest.monitorIds)) + } + if (!tableProp.searchString.isNullOrBlank()) { queryBuilder .should( diff --git a/alerting/src/test/kotlin/org/opensearch/alerting/MonitorDataSourcesIT.kt b/alerting/src/test/kotlin/org/opensearch/alerting/MonitorDataSourcesIT.kt index 955b18092..839c07f29 100644 --- a/alerting/src/test/kotlin/org/opensearch/alerting/MonitorDataSourcesIT.kt +++ b/alerting/src/test/kotlin/org/opensearch/alerting/MonitorDataSourcesIT.kt @@ -374,7 +374,7 @@ class MonitorDataSourcesIT : AlertingSingleNodeTestCase() { ) } - fun `test execute GetFindingsAction with params monitorId and findingIndex`() { + fun `test execute GetFindingsAction with unknown monitorId`() { val docQuery = DocLevelQuery(query = "test_field:\"us-west-2\"", name = "3") val docLevelInput = DocLevelMonitorInput("description", listOf(index), listOf(docQuery)) val trigger = randomDocumentLevelTrigger(condition = ALWAYS_RUN) @@ -402,16 +402,20 @@ class MonitorDataSourcesIT : AlertingSingleNodeTestCase() { val findings = searchFindings(monitorId, customFindingsIndex) assertEquals("Findings saved for test monitor", 1, findings.size) assertTrue("Findings saved for test monitor", findings[0].relatedDocIds.contains("1")) - // fetch findings - pass both monitorId and findingIndexName name. Monitor id should be ignored - val findingsFromAPI = getFindings(findings.get(0).id, "incorrect_monitor_id", customFindingsIndex) - assertEquals( - "Findings mismatch between manually searched and fetched via GetFindingsAction", - findings.get(0).id, - findingsFromAPI.get(0).id - ) + // fetch findings - don't send monitorId or findingIndexName. It should fall back to hardcoded finding index name + try { + getFindings(findings.get(0).id, "unknown_monitor_id_123456789", null) + } catch (e: Exception) { + e.message?.let { + assertTrue( + "Exception not returning GetMonitor Action error ", + it.contains("Monitor not found") + ) + } + } } - fun `test execute GetFindingsAction with unknown monitorId`() { + fun `test execute GetFindingsAction with unknown findingIndex param`() { val docQuery = DocLevelQuery(query = "test_field:\"us-west-2\"", name = "3") val docLevelInput = DocLevelMonitorInput("description", listOf(index), listOf(docQuery)) val trigger = randomDocumentLevelTrigger(condition = ALWAYS_RUN) @@ -441,26 +445,24 @@ class MonitorDataSourcesIT : AlertingSingleNodeTestCase() { assertTrue("Findings saved for test monitor", findings[0].relatedDocIds.contains("1")) // fetch findings - don't send monitorId or findingIndexName. It should fall back to hardcoded finding index name try { - getFindings(findings.get(0).id, "unknown_monitor_id_123456789", null) + getFindings(findings.get(0).id, null, "unknown_finding_index_123456789") } catch (e: Exception) { e.message?.let { assertTrue( "Exception not returning GetMonitor Action error ", - it.contains("Monitor not found") + it.contains("no such index") ) } } } - fun `test execute GetFindingsAction with unknown findingIndex param`() { + fun `test get alerts by list of monitors containing both existent and non-existent ids`() { val docQuery = DocLevelQuery(query = "test_field:\"us-west-2\"", name = "3") val docLevelInput = DocLevelMonitorInput("description", listOf(index), listOf(docQuery)) val trigger = randomDocumentLevelTrigger(condition = ALWAYS_RUN) - val customFindingsIndex = "custom_findings_index" var monitor = randomDocumentLevelMonitor( inputs = listOf(docLevelInput), triggers = listOf(trigger), - dataSources = DataSources(findingsIndex = customFindingsIndex) ) val monitorResponse = createMonitor(monitor) val testTime = DateTimeFormatter.ISO_OFFSET_DATE_TIME.format(ZonedDateTime.now().truncatedTo(MILLIS)) @@ -469,27 +471,43 @@ class MonitorDataSourcesIT : AlertingSingleNodeTestCase() { "test_strict_date_time" : "$testTime", "test_field" : "us-west-2" }""" - assertFalse(monitorResponse?.id.isNullOrEmpty()) + monitor = monitorResponse!!.monitor + + val id = monitorResponse.id + + var monitor1 = randomDocumentLevelMonitor( + inputs = listOf(docLevelInput), + triggers = listOf(trigger), + ) + val monitorResponse1 = createMonitor(monitor1) + monitor1 = monitorResponse1!!.monitor + val id1 = monitorResponse1.id indexDoc(index, "1", testDoc) - val monitorId = monitorResponse.id - val executeMonitorResponse = executeMonitor(monitor, monitorId, false) - Assert.assertEquals(executeMonitorResponse!!.monitorRunResult.monitorName, monitor.name) - Assert.assertEquals(executeMonitorResponse.monitorRunResult.triggerResults.size, 1) - searchAlerts(monitorId) - val findings = searchFindings(monitorId, customFindingsIndex) - assertEquals("Findings saved for test monitor", 1, findings.size) - assertTrue("Findings saved for test monitor", findings[0].relatedDocIds.contains("1")) - // fetch findings - don't send monitorId or findingIndexName. It should fall back to hardcoded finding index name - try { - getFindings(findings.get(0).id, null, "unknown_finding_index_123456789") - } catch (e: Exception) { - e.message?.let { - assertTrue( - "Exception not returning GetMonitor Action error ", - it.contains("no such index") - ) - } - } + executeMonitor(monitor1, id1, false) + executeMonitor(monitor, id, false) + val alerts = searchAlerts(id) + assertEquals("Alert saved for test monitor", 1, alerts.size) + val alerts1 = searchAlerts(id) + assertEquals("Alert saved for test monitor", 1, alerts1.size) + val table = Table("asc", "id", null, 1000, 0, "") + var getAlertsResponse = client() + .execute( + AlertingActions.GET_ALERTS_ACTION_TYPE, + GetAlertsRequest(table, "ALL", "ALL", null, null) + ) + .get() + + Assert.assertTrue(getAlertsResponse != null) + Assert.assertTrue(getAlertsResponse.alerts.size == 2) + + var alertsResponseForRequestWithoutCustomIndex = client() + .execute( + AlertingActions.GET_ALERTS_ACTION_TYPE, + GetAlertsRequest(table, "ALL", "ALL", null, null, listOf(id, id1, "1", "2")) + ) + .get() + Assert.assertTrue(alertsResponseForRequestWithoutCustomIndex != null) + Assert.assertTrue(alertsResponseForRequestWithoutCustomIndex.alerts.size == 2) } }