From 7c01d772233007e346e81a90e4945267ecc74095 Mon Sep 17 00:00:00 2001 From: Tyler Ohlsen Date: Mon, 27 Nov 2023 20:41:50 +0000 Subject: [PATCH 1/7] Add search monitors tool; Signed-off-by: Tyler Ohlsen --- .../ml/engine/tools/SearchMonitorsTool.java | 137 ++++++++++++++++++ 1 file changed, 137 insertions(+) create mode 100644 ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/SearchMonitorsTool.java diff --git a/ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/SearchMonitorsTool.java b/ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/SearchMonitorsTool.java new file mode 100644 index 0000000000..0058ab48eb --- /dev/null +++ b/ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/SearchMonitorsTool.java @@ -0,0 +1,137 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.ml.engine.tools; + +import java.util.List; +import java.util.Map; + +import org.opensearch.action.search.SearchRequest; +import org.opensearch.action.search.SearchResponse; +import org.opensearch.client.Client; +import org.opensearch.core.action.ActionListener; +import org.opensearch.index.query.MatchQueryBuilder; +import org.opensearch.ml.common.output.model.ModelTensors; +import org.opensearch.ml.common.spi.tools.Parser; +import org.opensearch.ml.common.spi.tools.Tool; +import org.opensearch.ml.common.spi.tools.ToolAnnotation; +import org.opensearch.search.builder.SearchSourceBuilder; + +import lombok.Getter; +import lombok.Setter; + +@ToolAnnotation(SearchMonitorsTool.TYPE) +public class SearchMonitorsTool implements Tool { + public static final String TYPE = "SearchMonitorsTool"; + private static final String DEFAULT_DESCRIPTION = "Use this tool to search alerting monitors."; + + @Setter + @Getter + private String name = TYPE; + @Getter + @Setter + private String description = DEFAULT_DESCRIPTION; + @Getter + private String type; + @Getter + private String version; + + private Client client; + @Setter + private Parser inputParser; + @Setter + private Parser outputParser; + + public SearchMonitorsTool(Client client) { + this.client = client; + + // probably keep this overridden output parser. need to ensure the output matches what's expected + outputParser = new Parser<>() { + @Override + public Object parse(Object o) { + @SuppressWarnings("unchecked") + List mlModelOutputs = (List) o; + return mlModelOutputs.get(0).getMlModelTensors().get(0).getDataAsMap().get("response"); + } + }; + } + + @Override + public void run(Map parameters, ActionListener listener) { + final String monitorName = parameters.getOrDefault("monitorName", null); + + // generate the search request based on parameters + SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder().query(new MatchQueryBuilder("monitor.name", monitorName)); + + SearchRequest searchRequest = new SearchRequest().source(searchSourceBuilder); + + /// SearchMonitorRequest searchMonitorRequest = new SearchMonitorRequest(searchRequest); + + // create response listener + // stringify the response, may change to a standard format in the future + ActionListener searchMonitorsListener = ActionListener.wrap(response -> { + StringBuilder sb = new StringBuilder(); + sb.append("Response placeholder"); + listener.onResponse((T) sb.toString()); + }, e -> { listener.onFailure(e); }); + + // execute the search + // AlertingPluginInterface.INSTANCE.searchMonitors((NodeClient) client, searchMonitorsRequest, searchMonitorsListener); + } + + @Override + public boolean validate(Map parameters) { + return true; + } + + @Override + public String getType() { + return TYPE; + } + + /** + * Factory for the {@link SearchAlertsTool} + */ + public static class Factory implements Tool.Factory { + private Client client; + + private static Factory INSTANCE; + + /** + * Create or return the singleton factory instance + */ + public static Factory getInstance() { + if (INSTANCE != null) { + return INSTANCE; + } + synchronized (SearchAlertsTool.class) { + if (INSTANCE != null) { + return INSTANCE; + } + INSTANCE = new Factory(); + return INSTANCE; + } + } + + /** + * Initialize this factory + * @param client The OpenSearch client + */ + public void init(Client client) { + this.client = client; + } + + @Override + public SearchAlertsTool create(Map map) { + return new SearchAlertsTool(client); + } + + @Override + public String getDefaultDescription() { + return DEFAULT_DESCRIPTION; + } + } + +} From 220cde164e4a68b463601602b4e16303a0339b19 Mon Sep 17 00:00:00 2001 From: Tyler Ohlsen Date: Mon, 27 Nov 2023 20:55:12 +0000 Subject: [PATCH 2/7] Add final set of optional params Signed-off-by: Tyler Ohlsen --- .../ml/engine/tools/CatIndexTool.java | 2 +- .../ml/engine/tools/SearchMonitorsTool.java | 20 ++++++++++++++++++- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/CatIndexTool.java b/ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/CatIndexTool.java index c74f53ba17..51fdc3fba5 100644 --- a/ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/CatIndexTool.java +++ b/ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/CatIndexTool.java @@ -98,7 +98,7 @@ public void run(Map parameters, ActionListener listener) final String[] indices = indexList.toArray(Strings.EMPTY_ARRAY); final IndicesOptions indicesOptions = IndicesOptions.strictExpand(); - final boolean local = parameters.containsKey("local") ? Boolean.parseBoolean("local") : false; + final boolean local = parameters.containsKey("local") ? Boolean.parseBoolean(parameters.get("local")) : false; final TimeValue clusterManagerNodeTimeout = DEFAULT_CLUSTER_MANAGER_NODE_TIMEOUT; final boolean includeUnloadedSegments = parameters.containsKey("include_unloaded_segments") ? Boolean.parseBoolean(parameters.get("include_unloaded_segments")) diff --git a/ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/SearchMonitorsTool.java b/ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/SearchMonitorsTool.java index 0058ab48eb..81a1ac0ce1 100644 --- a/ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/SearchMonitorsTool.java +++ b/ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/SearchMonitorsTool.java @@ -12,7 +12,9 @@ import org.opensearch.action.search.SearchResponse; import org.opensearch.client.Client; import org.opensearch.core.action.ActionListener; +import org.opensearch.index.query.MatchAllQueryBuilder; import org.opensearch.index.query.MatchQueryBuilder; +import org.opensearch.index.query.QueryBuilder; import org.opensearch.ml.common.output.model.ModelTensors; import org.opensearch.ml.common.spi.tools.Parser; import org.opensearch.ml.common.spi.tools.Tool; @@ -60,10 +62,26 @@ public Object parse(Object o) { @Override public void run(Map parameters, ActionListener listener) { + final String monitorId = parameters.getOrDefault("monitorId", null); final String monitorName = parameters.getOrDefault("monitorName", null); + final String monitorNamePattern = parameters.getOrDefault("monitorNamePattern", null); + final boolean enabled = parameters.containsKey("enabled") ? Boolean.parseBoolean(parameters.get("enabled")) : null; + final boolean hasTriggers = parameters.containsKey("hasTriggers") ? Boolean.parseBoolean(parameters.get("hasTriggers")) : null; + final String index = parameters.getOrDefault("index", null); + final String sortOrder = parameters.getOrDefault("sortOrder", "asc"); + final String sortString = parameters.getOrDefault("sortString", "monitor.name.keyword"); + final int size = parameters.containsKey("size") ? Integer.parseInt(parameters.get("size")) : 20; + final int startIndex = parameters.containsKey("startIndex") ? Integer.parseInt(parameters.get("startIndex")) : 0; + + QueryBuilder queryBuilder; + if (monitorName == null) { + queryBuilder = new MatchAllQueryBuilder(); + } else { + queryBuilder = new MatchQueryBuilder("monitor.name", monitorName); + } // generate the search request based on parameters - SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder().query(new MatchQueryBuilder("monitor.name", monitorName)); + SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder().query(queryBuilder); SearchRequest searchRequest = new SearchRequest().source(searchSourceBuilder); From 4505ac221abc58900399b62a99721f51cf50d443 Mon Sep 17 00:00:00 2001 From: Tyler Ohlsen Date: Mon, 27 Nov 2023 23:26:50 +0000 Subject: [PATCH 3/7] Add query generation logic; add test stubs Signed-off-by: Tyler Ohlsen --- .../ml/engine/tools/SearchMonitorsTool.java | 98 +++++++++++++------ .../engine/tools/SearchMonitorsToolTests.java | 70 +++++++++++++ 2 files changed, 137 insertions(+), 31 deletions(-) create mode 100644 ml-algorithms/src/test/java/org/opensearch/ml/engine/tools/SearchMonitorsToolTests.java diff --git a/ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/SearchMonitorsTool.java b/ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/SearchMonitorsTool.java index 81a1ac0ce1..c6fbc17fd0 100644 --- a/ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/SearchMonitorsTool.java +++ b/ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/SearchMonitorsTool.java @@ -11,15 +11,20 @@ import org.opensearch.action.search.SearchRequest; import org.opensearch.action.search.SearchResponse; import org.opensearch.client.Client; +import org.opensearch.commons.alerting.action.SearchMonitorRequest; import org.opensearch.core.action.ActionListener; -import org.opensearch.index.query.MatchAllQueryBuilder; -import org.opensearch.index.query.MatchQueryBuilder; +import org.opensearch.index.query.BoolQueryBuilder; +import org.opensearch.index.query.ExistsQueryBuilder; +import org.opensearch.index.query.NestedQueryBuilder; import org.opensearch.index.query.QueryBuilder; +import org.opensearch.index.query.TermQueryBuilder; +import org.opensearch.index.query.WildcardQueryBuilder; import org.opensearch.ml.common.output.model.ModelTensors; import org.opensearch.ml.common.spi.tools.Parser; import org.opensearch.ml.common.spi.tools.Tool; import org.opensearch.ml.common.spi.tools.ToolAnnotation; import org.opensearch.search.builder.SearchSourceBuilder; +import org.opensearch.search.sort.SortOrder; import lombok.Getter; import lombok.Setter; @@ -67,36 +72,67 @@ public void run(Map parameters, ActionListener listener) final String monitorNamePattern = parameters.getOrDefault("monitorNamePattern", null); final boolean enabled = parameters.containsKey("enabled") ? Boolean.parseBoolean(parameters.get("enabled")) : null; final boolean hasTriggers = parameters.containsKey("hasTriggers") ? Boolean.parseBoolean(parameters.get("hasTriggers")) : null; - final String index = parameters.getOrDefault("index", null); - final String sortOrder = parameters.getOrDefault("sortOrder", "asc"); + final String indices = parameters.getOrDefault("indices", null); + final String sortOrderStr = parameters.getOrDefault("sortOrder", "asc"); + final SortOrder sortOrder = sortOrderStr == "asc" ? SortOrder.ASC : SortOrder.DESC; final String sortString = parameters.getOrDefault("sortString", "monitor.name.keyword"); final int size = parameters.containsKey("size") ? Integer.parseInt(parameters.get("size")) : 20; final int startIndex = parameters.containsKey("startIndex") ? Integer.parseInt(parameters.get("startIndex")) : 0; - QueryBuilder queryBuilder; - if (monitorName == null) { - queryBuilder = new MatchAllQueryBuilder(); + // If a monitor ID is specified, all other params will be ignored. Simply return the monitor details based on that ID + // via the get monitor transport action + if (monitorId != null) { + // TODO } else { - queryBuilder = new MatchQueryBuilder("monitor.name", monitorName); + List mustList = new ArrayList(); + if (monitorName != null) { + mustList.add(new TermQueryBuilder("monitor.name.keyword", monitorName)); + } + if (monitorNamePattern != null) { + mustList.add(new WildcardQueryBuilder("monitor.name.keyword", monitorNamePattern)); + } + if (enabled != null) { + mustList.add(new TermQueryBuilder("monitor.enabled", enabled)); + } + if (hasTriggers != null) { + NestedQueryBuilder nestedTriggerQuery = new NestedQueryBuilder( + "monitor.triggers", + new ExistsQueryBuilder("monitor.triggers"), + null + ); + BoolQueryBuilder triggerQuery = new BoolQueryBuilder(); + if (hasTriggers) { + triggerQuery.must(nestedTriggerQuery); + } else { + triggerQuery.mustNot(nestedTriggerQuery); + } + mustList.add(triggerQuery); + } + if (indices != null) { + mustList + .add( + new NestedQueryBuilder("monitor.inputs", new WildcardQueryBuilder("monitor.inputs.search.indices", indices, null)) + ); + } + SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder() + .query(new BoolQueryBuilder().must(mustList)) + .size(size) + .from(startIndex) + .sort(sortString, sortOrder); + + SearchMonitorRequest searchMonitorRequest = new SearchMonitorRequest(new SearchRequest().source(searchSourceBuilder)); + + // create response listener + // stringify the aresponse, may change to a standard format in the future + ActionListener searchMonitorListener = ActionListener.wrap(response -> { + StringBuilder sb = new StringBuilder(); + sb.append("Response placeholder"); + listener.onResponse((T) sb.toString()); + }, e -> { listener.onFailure(e); }); + + // execute the search + AlertingPluginInterface.INSTANCE.searchMonitors((NodeClient) client, searchMonitorRequest, searchMonitorListener); } - - // generate the search request based on parameters - SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder().query(queryBuilder); - - SearchRequest searchRequest = new SearchRequest().source(searchSourceBuilder); - - /// SearchMonitorRequest searchMonitorRequest = new SearchMonitorRequest(searchRequest); - - // create response listener - // stringify the response, may change to a standard format in the future - ActionListener searchMonitorsListener = ActionListener.wrap(response -> { - StringBuilder sb = new StringBuilder(); - sb.append("Response placeholder"); - listener.onResponse((T) sb.toString()); - }, e -> { listener.onFailure(e); }); - - // execute the search - // AlertingPluginInterface.INSTANCE.searchMonitors((NodeClient) client, searchMonitorsRequest, searchMonitorsListener); } @Override @@ -110,9 +146,9 @@ public String getType() { } /** - * Factory for the {@link SearchAlertsTool} + * Factory for the {@link SearchMonitorsTool} */ - public static class Factory implements Tool.Factory { + public static class Factory implements Tool.Factory { private Client client; private static Factory INSTANCE; @@ -124,7 +160,7 @@ public static Factory getInstance() { if (INSTANCE != null) { return INSTANCE; } - synchronized (SearchAlertsTool.class) { + synchronized (SearchMonitorsTool.class) { if (INSTANCE != null) { return INSTANCE; } @@ -142,8 +178,8 @@ public void init(Client client) { } @Override - public SearchAlertsTool create(Map map) { - return new SearchAlertsTool(client); + public SearchMonitorsTool create(Map map) { + return new SearchMonitorsTool(client); } @Override diff --git a/ml-algorithms/src/test/java/org/opensearch/ml/engine/tools/SearchMonitorsToolTests.java b/ml-algorithms/src/test/java/org/opensearch/ml/engine/tools/SearchMonitorsToolTests.java new file mode 100644 index 0000000000..31ef73b2fa --- /dev/null +++ b/ml-algorithms/src/test/java/org/opensearch/ml/engine/tools/SearchMonitorsToolTests.java @@ -0,0 +1,70 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.ml.engine.tools; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; + +import java.time.Instant; +import java.util.Collections; +import java.util.List; +import java.util.Map; + +import org.junit.Before; +import org.junit.Test; +import org.mockito.ArgumentCaptor; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.MockitoAnnotations; +import org.opensearch.action.ActionType; +import org.opensearch.client.AdminClient; +import org.opensearch.client.ClusterAdminClient; +import org.opensearch.client.IndicesAdminClient; +import org.opensearch.client.node.NodeClient; +import org.opensearch.commons.alerting.action.GetAlertsResponse; +import org.opensearch.commons.alerting.model.Alert; +import org.opensearch.core.action.ActionListener; +import org.opensearch.ml.common.spi.tools.Tool; + +public class SearchMonitorsToolTests { + @Mock + private NodeClient nodeClient; + @Mock + private AdminClient adminClient; + @Mock + private IndicesAdminClient indicesAdminClient; + @Mock + private ClusterAdminClient clusterAdminClient; + + private Map nullParams; + private Map emptyParams; + private Map nonEmptyParams; + + @Before + public void setup() { + MockitoAnnotations.openMocks(this); + SearchMonitorsTool.Factory.getInstance().init(nodeClient); + + nullParams = null; + emptyParams = Collections.emptyMap(); + nonEmptyParams = Map.of("monitorName", "foo"); + } + + // TODO: add tests to trigger run() with different param values + + @Test + public void testValidate() { + Tool tool = SearchMonitorsTool.Factory.getInstance().create(Collections.emptyMap()); + assertEquals(SearchMonitorsTool.TYPE, tool.getType()); + assertTrue(tool.validate(emptyParams)); + assertTrue(tool.validate(nonEmptyParams)); + assertTrue(tool.validate(nullParams)); + } +} From 78d641fd472dbdb8da1509f62f33a4fd76f3b278 Mon Sep 17 00:00:00 2001 From: Tyler Ohlsen Date: Fri, 1 Dec 2023 21:43:22 +0000 Subject: [PATCH 4/7] Get compile and all tests passing with local custom maven deps Signed-off-by: Tyler Ohlsen --- .../ml/engine/tools/SearchMonitorsTool.java | 16 +++++-- .../engine/tools/SearchMonitorsToolTests.java | 48 +++++++++++++++++-- 2 files changed, 54 insertions(+), 10 deletions(-) diff --git a/ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/SearchMonitorsTool.java b/ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/SearchMonitorsTool.java index c6fbc17fd0..6feb22a6ff 100644 --- a/ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/SearchMonitorsTool.java +++ b/ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/SearchMonitorsTool.java @@ -5,12 +5,15 @@ package org.opensearch.ml.engine.tools; +import java.util.ArrayList; import java.util.List; import java.util.Map; import org.opensearch.action.search.SearchRequest; import org.opensearch.action.search.SearchResponse; import org.opensearch.client.Client; +import org.opensearch.client.node.NodeClient; +import org.opensearch.commons.alerting.AlertingPluginInterface; import org.opensearch.commons.alerting.action.SearchMonitorRequest; import org.opensearch.core.action.ActionListener; import org.opensearch.index.query.BoolQueryBuilder; @@ -70,8 +73,8 @@ public void run(Map parameters, ActionListener listener) final String monitorId = parameters.getOrDefault("monitorId", null); final String monitorName = parameters.getOrDefault("monitorName", null); final String monitorNamePattern = parameters.getOrDefault("monitorNamePattern", null); - final boolean enabled = parameters.containsKey("enabled") ? Boolean.parseBoolean(parameters.get("enabled")) : null; - final boolean hasTriggers = parameters.containsKey("hasTriggers") ? Boolean.parseBoolean(parameters.get("hasTriggers")) : null; + final Boolean enabled = parameters.containsKey("enabled") ? Boolean.parseBoolean(parameters.get("enabled")) : null; + final Boolean hasTriggers = parameters.containsKey("hasTriggers") ? Boolean.parseBoolean(parameters.get("hasTriggers")) : null; final String indices = parameters.getOrDefault("indices", null); final String sortOrderStr = parameters.getOrDefault("sortOrder", "asc"); final SortOrder sortOrder = sortOrderStr == "asc" ? SortOrder.ASC : SortOrder.DESC; @@ -111,11 +114,14 @@ public void run(Map parameters, ActionListener listener) if (indices != null) { mustList .add( - new NestedQueryBuilder("monitor.inputs", new WildcardQueryBuilder("monitor.inputs.search.indices", indices, null)) + new NestedQueryBuilder("monitor.inputs", new WildcardQueryBuilder("monitor.inputs.search.indices", indices), null) ); } + + BoolQueryBuilder boolQueryBuilder = new BoolQueryBuilder(); + boolQueryBuilder.must().addAll(mustList); SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder() - .query(new BoolQueryBuilder().must(mustList)) + .query(boolQueryBuilder) .size(size) .from(startIndex) .sort(sortString, sortOrder); @@ -123,7 +129,7 @@ public void run(Map parameters, ActionListener listener) SearchMonitorRequest searchMonitorRequest = new SearchMonitorRequest(new SearchRequest().source(searchSourceBuilder)); // create response listener - // stringify the aresponse, may change to a standard format in the future + // stringify the response, may change to a standard format in the future ActionListener searchMonitorListener = ActionListener.wrap(response -> { StringBuilder sb = new StringBuilder(); sb.append("Response placeholder"); diff --git a/ml-algorithms/src/test/java/org/opensearch/ml/engine/tools/SearchMonitorsToolTests.java b/ml-algorithms/src/test/java/org/opensearch/ml/engine/tools/SearchMonitorsToolTests.java index 31ef73b2fa..4ff78e46f5 100644 --- a/ml-algorithms/src/test/java/org/opensearch/ml/engine/tools/SearchMonitorsToolTests.java +++ b/ml-algorithms/src/test/java/org/opensearch/ml/engine/tools/SearchMonitorsToolTests.java @@ -12,9 +12,8 @@ import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; -import java.time.Instant; +import java.util.ArrayList; import java.util.Collections; -import java.util.List; import java.util.Map; import org.junit.Before; @@ -24,14 +23,17 @@ import org.mockito.Mockito; import org.mockito.MockitoAnnotations; import org.opensearch.action.ActionType; +import org.opensearch.action.search.SearchResponse; +import org.opensearch.action.search.SearchResponseSections; import org.opensearch.client.AdminClient; import org.opensearch.client.ClusterAdminClient; import org.opensearch.client.IndicesAdminClient; import org.opensearch.client.node.NodeClient; -import org.opensearch.commons.alerting.action.GetAlertsResponse; -import org.opensearch.commons.alerting.model.Alert; import org.opensearch.core.action.ActionListener; import org.opensearch.ml.common.spi.tools.Tool; +import org.opensearch.search.SearchHit; +import org.opensearch.search.SearchHits; +import org.opensearch.search.aggregations.Aggregations; public class SearchMonitorsToolTests { @Mock @@ -57,7 +59,43 @@ public void setup() { nonEmptyParams = Map.of("monitorName", "foo"); } - // TODO: add tests to trigger run() with different param values + @Test + public void testRunWithNoMonitors() throws Exception { + Tool tool = SearchMonitorsTool.Factory.getInstance().create(Collections.emptyMap()); + SearchResponse getMonitorsResponse = new SearchResponse( + new SearchResponseSections( + new SearchHits(new SearchHit[] {}, null, 0), + new Aggregations(new ArrayList<>()), + null, + false, + null, + null, + 0 + ), + null, + 0, + 0, + 0, + 0, + null, + null + ); + String expectedResponseStr = "Response placeholder"; + + @SuppressWarnings("unchecked") + ActionListener listener = Mockito.mock(ActionListener.class); + + doAnswer((invocation) -> { + ActionListener responseListener = invocation.getArgument(2); + responseListener.onResponse(getMonitorsResponse); + return null; + }).when(nodeClient).execute(any(ActionType.class), any(), any()); + + tool.run(emptyParams, listener); + ArgumentCaptor responseCaptor = ArgumentCaptor.forClass(String.class); + verify(listener, times(1)).onResponse(responseCaptor.capture()); + assertEquals(expectedResponseStr, responseCaptor.getValue()); + } @Test public void testValidate() { From 1dfe0a204318a877e14cfe7dcd38730c5dd14682 Mon Sep 17 00:00:00 2001 From: Tyler Ohlsen Date: Fri, 1 Dec 2023 23:42:53 +0000 Subject: [PATCH 5/7] Improve stringified response; add basic tests Signed-off-by: Tyler Ohlsen --- .../ml/engine/tools/SearchMonitorsTool.java | 12 +++- .../engine/tools/SearchMonitorsToolTests.java | 66 ++++++++++++++++--- 2 files changed, 67 insertions(+), 11 deletions(-) diff --git a/ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/SearchMonitorsTool.java b/ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/SearchMonitorsTool.java index 6feb22a6ff..e24221ae9d 100644 --- a/ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/SearchMonitorsTool.java +++ b/ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/SearchMonitorsTool.java @@ -26,6 +26,7 @@ import org.opensearch.ml.common.spi.tools.Parser; import org.opensearch.ml.common.spi.tools.Tool; import org.opensearch.ml.common.spi.tools.ToolAnnotation; +import org.opensearch.search.SearchHit; import org.opensearch.search.builder.SearchSourceBuilder; import org.opensearch.search.sort.SortOrder; @@ -132,7 +133,16 @@ public void run(Map parameters, ActionListener listener) // stringify the response, may change to a standard format in the future ActionListener searchMonitorListener = ActionListener.wrap(response -> { StringBuilder sb = new StringBuilder(); - sb.append("Response placeholder"); + SearchHit[] hits = response.getHits().getHits(); + sb.append("Monitors=["); + for (SearchHit hit : hits) { + sb.append("{"); + sb.append("id=").append(hit.getId()).append(","); + sb.append("name=").append(hit.getSourceAsMap().get("name")); + sb.append("}"); + } + sb.append("]"); + sb.append("TotalMonitors=").append(response.getHits().getTotalHits().value); listener.onResponse((T) sb.toString()); }, e -> { listener.onFailure(e); }); diff --git a/ml-algorithms/src/test/java/org/opensearch/ml/engine/tools/SearchMonitorsToolTests.java b/ml-algorithms/src/test/java/org/opensearch/ml/engine/tools/SearchMonitorsToolTests.java index 4ff78e46f5..a310d80fbb 100644 --- a/ml-algorithms/src/test/java/org/opensearch/ml/engine/tools/SearchMonitorsToolTests.java +++ b/ml-algorithms/src/test/java/org/opensearch/ml/engine/tools/SearchMonitorsToolTests.java @@ -16,6 +16,7 @@ import java.util.Collections; import java.util.Map; +import org.apache.lucene.search.TotalHits; import org.junit.Before; import org.junit.Test; import org.mockito.ArgumentCaptor; @@ -29,7 +30,10 @@ import org.opensearch.client.ClusterAdminClient; import org.opensearch.client.IndicesAdminClient; import org.opensearch.client.node.NodeClient; +import org.opensearch.common.xcontent.XContentType; import org.opensearch.core.action.ActionListener; +import org.opensearch.core.common.bytes.BytesReference; +import org.opensearch.core.xcontent.XContentBuilder; import org.opensearch.ml.common.spi.tools.Tool; import org.opensearch.search.SearchHit; import org.opensearch.search.SearchHits; @@ -61,17 +65,59 @@ public void setup() { @Test public void testRunWithNoMonitors() throws Exception { + final String monitorName = "monitor-1"; + final String monitorId = "monitor-1-id"; Tool tool = SearchMonitorsTool.Factory.getInstance().create(Collections.emptyMap()); + + SearchHit[] hits = new SearchHit[0]; + + TotalHits totalHits = new TotalHits(hits.length, TotalHits.Relation.EQUAL_TO); + + SearchResponse getMonitorsResponse = new SearchResponse( + new SearchResponseSections(new SearchHits(hits, totalHits, 0), new Aggregations(new ArrayList<>()), null, false, null, null, 0), + null, + 0, + 0, + 0, + 0, + null, + null + ); + String expectedResponseStr = String.format("Monitors=[]TotalMonitors=%d", hits.length); + + @SuppressWarnings("unchecked") + ActionListener listener = Mockito.mock(ActionListener.class); + + doAnswer((invocation) -> { + ActionListener responseListener = invocation.getArgument(2); + responseListener.onResponse(getMonitorsResponse); + return null; + }).when(nodeClient).execute(any(ActionType.class), any(), any()); + + tool.run(emptyParams, listener); + ArgumentCaptor responseCaptor = ArgumentCaptor.forClass(String.class); + verify(listener, times(1)).onResponse(responseCaptor.capture()); + assertEquals(expectedResponseStr, responseCaptor.getValue()); + } + + @Test + public void testRunWitSingleMonitor() throws Exception { + final String monitorName = "monitor-1"; + final String monitorId = "monitor-1-id"; + Tool tool = SearchMonitorsTool.Factory.getInstance().create(Collections.emptyMap()); + + XContentBuilder content = XContentBuilder.builder(XContentType.JSON.xContent()); + content.startObject(); + content.field("type", "monitor"); + content.field("name", monitorName); + content.endObject(); + SearchHit[] hits = new SearchHit[1]; + hits[0] = new SearchHit(0, monitorId, null, null).sourceRef(BytesReference.bytes(content)); + + TotalHits totalHits = new TotalHits(hits.length, TotalHits.Relation.EQUAL_TO); + SearchResponse getMonitorsResponse = new SearchResponse( - new SearchResponseSections( - new SearchHits(new SearchHit[] {}, null, 0), - new Aggregations(new ArrayList<>()), - null, - false, - null, - null, - 0 - ), + new SearchResponseSections(new SearchHits(hits, totalHits, 0), new Aggregations(new ArrayList<>()), null, false, null, null, 0), null, 0, 0, @@ -80,7 +126,7 @@ public void testRunWithNoMonitors() throws Exception { null, null ); - String expectedResponseStr = "Response placeholder"; + String expectedResponseStr = String.format("Monitors=[{id=%s,name=%s}]TotalMonitors=%d", monitorId, monitorName, hits.length); @SuppressWarnings("unchecked") ActionListener listener = Mockito.mock(ActionListener.class); From 2fd930f0a6e552ee14482228a09a3ce2422c2ac2 Mon Sep 17 00:00:00 2001 From: Tyler Ohlsen Date: Fri, 1 Dec 2023 23:58:43 +0000 Subject: [PATCH 6/7] Add TODOs Signed-off-by: Tyler Ohlsen --- .../engine/tools/SearchMonitorsToolTests.java | 20 ++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/ml-algorithms/src/test/java/org/opensearch/ml/engine/tools/SearchMonitorsToolTests.java b/ml-algorithms/src/test/java/org/opensearch/ml/engine/tools/SearchMonitorsToolTests.java index a310d80fbb..6890c3f423 100644 --- a/ml-algorithms/src/test/java/org/opensearch/ml/engine/tools/SearchMonitorsToolTests.java +++ b/ml-algorithms/src/test/java/org/opensearch/ml/engine/tools/SearchMonitorsToolTests.java @@ -65,8 +65,6 @@ public void setup() { @Test public void testRunWithNoMonitors() throws Exception { - final String monitorName = "monitor-1"; - final String monitorId = "monitor-1-id"; Tool tool = SearchMonitorsTool.Factory.getInstance().create(Collections.emptyMap()); SearchHit[] hits = new SearchHit[0]; @@ -100,12 +98,28 @@ public void testRunWithNoMonitors() throws Exception { assertEquals(expectedResponseStr, responseCaptor.getValue()); } + // TODO: add tests to exercise get monitor action vs. search monitor action + @Test - public void testRunWitSingleMonitor() throws Exception { + public void testRunWithSingleMonitor() throws Exception { final String monitorName = "monitor-1"; final String monitorId = "monitor-1-id"; Tool tool = SearchMonitorsTool.Factory.getInstance().create(Collections.emptyMap()); + // TODO: explore more detailed monitor response. IndexMonitorRequest translates to a PUT request + // to the system index, and is the same result as what's returned by the search response in search monitors API. + // explore converting the response to this to test it + + // IndexMonitorRequest indexedMonitor = new IndexMonitorRequest( + // "1234", + // 1L, + // 2L, + // WriteRequest.RefreshPolicy.IMMEDIATE, + // RestRequest.Method.POST, + // // randomQueryLevelMonitor().copy(inputs = listOf(SearchInput(emptyList(), SearchSourceBuilder()))); + // randomQueryLevelMonitor() + // ); + XContentBuilder content = XContentBuilder.builder(XContentType.JSON.xContent()); content.startObject(); content.field("type", "monitor"); From d49e55afa9ce9838f460668c8f20523cc3a6bbb3 Mon Sep 17 00:00:00 2001 From: Tyler Ohlsen Date: Mon, 4 Dec 2023 23:09:56 +0000 Subject: [PATCH 7/7] Add get monitor ID option + test Signed-off-by: Tyler Ohlsen --- .../ml/engine/tools/SearchMonitorsTool.java | 29 +++++++-- .../engine/tools/SearchMonitorsToolTests.java | 63 ++++++++++++++----- 2 files changed, 72 insertions(+), 20 deletions(-) diff --git a/ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/SearchMonitorsTool.java b/ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/SearchMonitorsTool.java index e24221ae9d..7b12f1dfc0 100644 --- a/ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/SearchMonitorsTool.java +++ b/ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/SearchMonitorsTool.java @@ -14,7 +14,10 @@ import org.opensearch.client.Client; import org.opensearch.client.node.NodeClient; import org.opensearch.commons.alerting.AlertingPluginInterface; +import org.opensearch.commons.alerting.action.GetMonitorRequest; +import org.opensearch.commons.alerting.action.GetMonitorResponse; import org.opensearch.commons.alerting.action.SearchMonitorRequest; +import org.opensearch.commons.alerting.model.Monitor; import org.opensearch.core.action.ActionListener; import org.opensearch.index.query.BoolQueryBuilder; import org.opensearch.index.query.ExistsQueryBuilder; @@ -26,6 +29,7 @@ import org.opensearch.ml.common.spi.tools.Parser; import org.opensearch.ml.common.spi.tools.Tool; import org.opensearch.ml.common.spi.tools.ToolAnnotation; +import org.opensearch.rest.RestRequest; import org.opensearch.search.SearchHit; import org.opensearch.search.builder.SearchSourceBuilder; import org.opensearch.search.sort.SortOrder; @@ -69,6 +73,9 @@ public Object parse(Object o) { }; } + // Response is currently in a simple string format including the list of monitors (only name and ID attached), and + // number of total monitors. The output will likely need to be updated, standardized, and include more fields in the + // future to cover a sufficient amount of potential questions the agent will need to handle. @Override public void run(Map parameters, ActionListener listener) { final String monitorId = parameters.getOrDefault("monitorId", null); @@ -86,7 +93,23 @@ public void run(Map parameters, ActionListener listener) // If a monitor ID is specified, all other params will be ignored. Simply return the monitor details based on that ID // via the get monitor transport action if (monitorId != null) { - // TODO + GetMonitorRequest getMonitorRequest = new GetMonitorRequest(monitorId, 1L, RestRequest.Method.GET, null); + ActionListener getMonitorListener = ActionListener.wrap(response -> { + StringBuilder sb = new StringBuilder(); + Monitor monitor = response.getMonitor(); + if (monitor != null) { + sb.append("Monitors=["); + sb.append("{"); + sb.append("id=").append(monitor.getId()).append(","); + sb.append("name=").append(monitor.getName()); + sb.append("}]"); + sb.append("TotalMonitors=1"); + } else { + sb.append("Monitors=[]TotalMonitors=0"); + } + listener.onResponse((T) sb.toString()); + }, e -> { listener.onFailure(e); }); + AlertingPluginInterface.INSTANCE.getMonitor((NodeClient) client, getMonitorRequest, getMonitorListener); } else { List mustList = new ArrayList(); if (monitorName != null) { @@ -129,8 +152,6 @@ public void run(Map parameters, ActionListener listener) SearchMonitorRequest searchMonitorRequest = new SearchMonitorRequest(new SearchRequest().source(searchSourceBuilder)); - // create response listener - // stringify the response, may change to a standard format in the future ActionListener searchMonitorListener = ActionListener.wrap(response -> { StringBuilder sb = new StringBuilder(); SearchHit[] hits = response.getHits().getHits(); @@ -145,8 +166,6 @@ public void run(Map parameters, ActionListener listener) sb.append("TotalMonitors=").append(response.getHits().getTotalHits().value); listener.onResponse((T) sb.toString()); }, e -> { listener.onFailure(e); }); - - // execute the search AlertingPluginInterface.INSTANCE.searchMonitors((NodeClient) client, searchMonitorRequest, searchMonitorListener); } } diff --git a/ml-algorithms/src/test/java/org/opensearch/ml/engine/tools/SearchMonitorsToolTests.java b/ml-algorithms/src/test/java/org/opensearch/ml/engine/tools/SearchMonitorsToolTests.java index 6890c3f423..265e9f637a 100644 --- a/ml-algorithms/src/test/java/org/opensearch/ml/engine/tools/SearchMonitorsToolTests.java +++ b/ml-algorithms/src/test/java/org/opensearch/ml/engine/tools/SearchMonitorsToolTests.java @@ -12,6 +12,8 @@ import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import java.time.Instant; +import java.time.ZoneId; import java.util.ArrayList; import java.util.Collections; import java.util.Map; @@ -31,6 +33,11 @@ import org.opensearch.client.IndicesAdminClient; import org.opensearch.client.node.NodeClient; import org.opensearch.common.xcontent.XContentType; +import org.opensearch.commons.alerting.action.GetMonitorResponse; +import org.opensearch.commons.alerting.model.CronSchedule; +import org.opensearch.commons.alerting.model.DataSources; +import org.opensearch.commons.alerting.model.Monitor; +import org.opensearch.commons.authuser.User; import org.opensearch.core.action.ActionListener; import org.opensearch.core.common.bytes.BytesReference; import org.opensearch.core.xcontent.XContentBuilder; @@ -98,7 +105,47 @@ public void testRunWithNoMonitors() throws Exception { assertEquals(expectedResponseStr, responseCaptor.getValue()); } - // TODO: add tests to exercise get monitor action vs. search monitor action + @Test + public void testRunWithMonitorId() throws Exception { + final String monitorId = "monitor-1-id"; + final String monitorName = "monitor-1"; + Tool tool = SearchMonitorsTool.Factory.getInstance().create(Collections.emptyMap()); + + Monitor monitor = new Monitor( + monitorId, + 0L, + monitorName, + true, + new CronSchedule("31 * * * *", ZoneId.of("Asia/Kolkata"), null), + Instant.now(), + Instant.now(), + Monitor.MonitorType.QUERY_LEVEL_MONITOR, + new User("test-user", Collections.emptyList(), Collections.emptyList(), Collections.emptyList()), + 0, + Collections.emptyList(), + Collections.emptyList(), + Collections.emptyMap(), + new DataSources(), + "" + ); + + GetMonitorResponse getMonitorResponse = new GetMonitorResponse(monitorId, 1L, 2L, 0L, monitor, Collections.emptyList()); + String expectedResponseStr = String.format("Monitors=[{id=%s,name=%s}]TotalMonitors=%d", monitorId, monitorName, 1); + + @SuppressWarnings("unchecked") + ActionListener listener = Mockito.mock(ActionListener.class); + + doAnswer((invocation) -> { + ActionListener responseListener = invocation.getArgument(2); + responseListener.onResponse(getMonitorResponse); + return null; + }).when(nodeClient).execute(any(ActionType.class), any(), any()); + + tool.run(emptyParams, listener); + ArgumentCaptor responseCaptor = ArgumentCaptor.forClass(String.class); + verify(listener, times(1)).onResponse(responseCaptor.capture()); + assertEquals(expectedResponseStr, responseCaptor.getValue()); + } @Test public void testRunWithSingleMonitor() throws Exception { @@ -106,20 +153,6 @@ public void testRunWithSingleMonitor() throws Exception { final String monitorId = "monitor-1-id"; Tool tool = SearchMonitorsTool.Factory.getInstance().create(Collections.emptyMap()); - // TODO: explore more detailed monitor response. IndexMonitorRequest translates to a PUT request - // to the system index, and is the same result as what's returned by the search response in search monitors API. - // explore converting the response to this to test it - - // IndexMonitorRequest indexedMonitor = new IndexMonitorRequest( - // "1234", - // 1L, - // 2L, - // WriteRequest.RefreshPolicy.IMMEDIATE, - // RestRequest.Method.POST, - // // randomQueryLevelMonitor().copy(inputs = listOf(SearchInput(emptyList(), SearchSourceBuilder()))); - // randomQueryLevelMonitor() - // ); - XContentBuilder content = XContentBuilder.builder(XContentType.JSON.xContent()); content.startObject(); content.field("type", "monitor");