Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Backport 2.x] Add alerting tools IT; fix missing system index bug of SearchMonitorsTool #141

Merged
merged 1 commit into from
Jan 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
}
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."
}
]
}
Loading