Skip to content

Commit

Permalink
Add alerting tools IT; fix missing system index bug of SearchMonitors…
Browse files Browse the repository at this point in the history
…Tool (opensearch-project#135) (opensearch-project#141)

(cherry picked from commit 3e4d451)

Signed-off-by: Tyler Ohlsen <[email protected]>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Signed-off-by: yuye-aws <[email protected]>
  • Loading branch information
2 people authored and yuye-aws committed Apr 26, 2024
1 parent 026e907 commit caf1aa9
Show file tree
Hide file tree
Showing 7 changed files with 196 additions and 31 deletions.
3 changes: 2 additions & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ dependencies {
zipArchive group: 'org.opensearch.plugin', name:'opensearch-sql-plugin', version: "${version}"
zipArchive group: 'org.opensearch.plugin', name:'opensearch-knn', version: "${version}"
zipArchive group: 'org.opensearch.plugin', name:'neural-search', version: "${version}"
zipArchive group: 'org.opensearch.plugin', name:'alerting', version: "${version}"

// Test dependencies
testImplementation "org.opensearch.test:framework:${opensearch_version}"
Expand Down Expand Up @@ -388,4 +389,4 @@ task updateVersion {
// Include the required files that needs to be updated with new Version
ant.replaceregexp(file:'build.gradle', match: '"opensearch.version", "\\d.*"', replace: '"opensearch.version", "' + newVersion.tokenize('-')[0] + '-SNAPSHOT"', flags:'g', byline:true)
}
}
}
83 changes: 55 additions & 28 deletions src/main/java/org/opensearch/agent/tools/SearchMonitorsTool.java
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,11 @@
package org.opensearch.agent.tools;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;

import org.apache.commons.lang3.StringUtils;
import org.apache.lucene.search.join.ScoreMode;
Expand All @@ -20,6 +23,7 @@
import org.opensearch.commons.alerting.action.GetMonitorResponse;
import org.opensearch.commons.alerting.action.SearchMonitorRequest;
import org.opensearch.commons.alerting.model.Monitor;
import org.opensearch.commons.alerting.model.ScheduledJob;
import org.opensearch.core.action.ActionListener;
import org.opensearch.index.query.BoolQueryBuilder;
import org.opensearch.index.query.ExistsQueryBuilder;
Expand Down Expand Up @@ -104,22 +108,17 @@ public <T> void run(Map<String, String> parameters, ActionListener<T> listener)
if (monitorId != null) {
GetMonitorRequest getMonitorRequest = new GetMonitorRequest(monitorId, 1L, RestRequest.Method.GET, null);
ActionListener<GetMonitorResponse> getMonitorListener = ActionListener.<GetMonitorResponse>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");
processGetMonitorHit(monitor, listener);
}, e -> {
// System index isn't initialized by default, so ignore such errors. Alerting plugin does not return the
// standard IndexNotFoundException so we parse the message instead
if (e.getMessage().contains("Configured indices are not found")) {
processGetMonitorHit(null, listener);
} else {
sb.append("Monitors=[]TotalMonitors=0");
log.error("Failed to get monitor.", e);
listener.onFailure(e);
}
listener.onResponse((T) sb.toString());
}, e -> {
log.error("Failed to search monitors.", e);
listener.onFailure(e);
});
AlertingPluginInterface.INSTANCE.getMonitor((NodeClient) client, getMonitorRequest, getMonitorListener);
} else {
Expand Down Expand Up @@ -167,24 +166,23 @@ public <T> void run(Map<String, String> parameters, ActionListener<T> listener)
.from(startIndex)
.sort(sortString, sortOrder);

SearchMonitorRequest searchMonitorRequest = new SearchMonitorRequest(new SearchRequest().source(searchSourceBuilder));
SearchRequest searchRequest = new SearchRequest().source(searchSourceBuilder).indices(ScheduledJob.SCHEDULED_JOBS_INDEX);
SearchMonitorRequest searchMonitorRequest = new SearchMonitorRequest(searchRequest);

ActionListener<SearchResponse> searchMonitorListener = ActionListener.<SearchResponse>wrap(response -> {
StringBuilder sb = new StringBuilder();
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());
List<SearchHit> hits = Arrays.asList(response.getHits().getHits());
Map<String, SearchHit> hitsAsMap = hits.stream().collect(Collectors.toMap(SearchHit::getId, hit -> hit));
processHits(hitsAsMap, listener);

}, e -> {
log.error("Failed to search monitors.", e);
listener.onFailure(e);
// System index isn't initialized by default, so ignore such errors. Alerting plugin does not return the
// standard IndexNotFoundException so we parse the message instead
if (e.getMessage().contains("Configured indices are not found")) {
processHits(Collections.emptyMap(), listener);
} else {
log.error("Failed to search monitors.", e);
listener.onFailure(e);
}
});
AlertingPluginInterface.INSTANCE.searchMonitors((NodeClient) client, searchMonitorRequest, searchMonitorListener);
}
Expand All @@ -200,6 +198,35 @@ public String getType() {
return TYPE;
}

private <T> void processHits(Map<String, SearchHit> hitsAsMap, ActionListener<T> listener) {
StringBuilder sb = new StringBuilder();
sb.append("Monitors=[");
for (SearchHit hit : hitsAsMap.values()) {
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(hitsAsMap.size());
listener.onResponse((T) sb.toString());
}

private <T> void processGetMonitorHit(Monitor monitor, ActionListener<T> listener) {
StringBuilder sb = new StringBuilder();
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());
}

/**
* Factory for the {@link SearchMonitorsTool}
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,10 @@ public static enum DetectorStateString {
Initializing
}

// System indices constants are not cleanly exposed from the AD plugin, so we persist our
// own constant here.
// System indices constants are not cleanly exposed from the AD & Alerting plugins, so we persist our
// own constants here.
public static final String AD_RESULTS_INDEX_PATTERN = ".opendistro-anomaly-results*";
public static final String AD_DETECTORS_INDEX = ".opendistro-anomaly-detectors";

public static final String ALERTING_CONFIG_INDEX = ".opendistro-alerting-config";
}
57 changes: 57 additions & 0 deletions src/test/java/org/opensearch/integTest/SearchAlertsToolIT.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

package org.opensearch.integTest;

import java.nio.file.Files;
import java.nio.file.Path;

import org.junit.After;
import org.junit.Before;

import lombok.SneakyThrows;

public class SearchAlertsToolIT extends BaseAgentToolsIT {
private String registerAgentRequestBody;
private static final String monitorId = "foo-id";
private static final String monitorName = "foo-name";

@Before
@SneakyThrows
public void setUp() {
super.setUp();
registerAgentRequestBody = Files
.readString(
Path
.of(
this
.getClass()
.getClassLoader()
.getResource("org/opensearch/agent/tools/register_flow_agent_of_search_alerts_tool_request_body.json")
.toURI()
)
);
}

@After
@SneakyThrows
public void tearDown() {
super.tearDown();
deleteExternalIndices();
deleteSystemIndices();
}

@SneakyThrows
public void testSearchAlertsToolInFlowAgent_withNoSystemIndex() {
deleteSystemIndices();
String agentId = createAgent(registerAgentRequestBody);
String agentInput = "{\"parameters\":{\"monitorId\": \"" + monitorId + "\"}}";
String result = executeAgent(agentId, agentInput);
assertEquals("Alerts=[]TotalAlerts=0", result);
}

// TODO: Add IT to test against sample alerts data
// https://github.com/opensearch-project/skills/issues/136
}
58 changes: 58 additions & 0 deletions src/test/java/org/opensearch/integTest/SearchMonitorsToolIT.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

package org.opensearch.integTest;

import static org.junit.jupiter.api.Assertions.assertEquals;

import java.nio.file.Files;
import java.nio.file.Path;

import org.junit.After;
import org.junit.Before;

import lombok.SneakyThrows;

public class SearchMonitorsToolIT extends BaseAgentToolsIT {
private String registerAgentRequestBody;
private static final String monitorId = "foo-id";
private static final String monitorName = "foo-name";

@Before
@SneakyThrows
public void setUp() {
super.setUp();
registerAgentRequestBody = Files
.readString(
Path
.of(
this
.getClass()
.getClassLoader()
.getResource("org/opensearch/agent/tools/register_flow_agent_of_search_monitors_tool_request_body.json")
.toURI()
)
);
}

@After
@SneakyThrows
public void tearDown() {
super.tearDown();
deleteExternalIndices();
deleteSystemIndices();
}

@SneakyThrows
public void testSearchMonitorsToolInFlowAgent_withNoSystemIndex() {
deleteSystemIndices();
String agentId = createAgent(registerAgentRequestBody);
String agentInput = "{\"parameters\":{\"monitorName\": \"" + monitorName + "\"}}";
String result = executeAgent(agentId, agentInput);
assertEquals("Monitors=[]TotalMonitors=0", result);
}

// TODO: Add IT to test against sample monitor data
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"name": "Test_Search_Alerts_Agent",
"type": "flow",
"tools": [
{
"type": "SearchAlertsTool",
"description": "Use this tool to search alerts."
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"name": "Test_Search_Monitors_Agent",
"type": "flow",
"tools": [
{
"type": "SearchMonitorsTool",
"description": "Use this tool to search alerting monitors."
}
]
}

0 comments on commit caf1aa9

Please sign in to comment.