Update parameter handling of tools

release-notes/opensearch-skills.release-notes-3.2.0.0.md

@@ -1,20 +1,18 @@
-## Version 3.2.0 Release Notes
+## Version 3.2.0.0 Release Notes
 
-Compatible with OpenSearch and OpenSearch Dashboards version 3.2.0
+Compatible with OpenSearch and OpenSearch Dashboards version 3.2.0.0
 
 ### Features
 * Support dynamic tool in agent framework ([#606](https://github.com/opensearch-project/skills/pull/606))
 
 ### Enhancements
 * Merge index schema meta ([#596](https://github.com/opensearch-project/skills/pull/596))
+* Mask error message in PPLTool ([#609](https://github.com/opensearch-project/skills/pull/609))
 
 ### Bug Fixes
 * Fix attributes handling in dynamic tool ([#607](https://github.com/opensearch-project/skills/pull/607))
-* Mask error message in PPLTool ([#609](https://github.com/opensearch-project/skills/pull/609))
-
-### Infrastructure
-* Update the maven snapshot publish endpoint and credential ([#601](https://github.com/opensearch-project/skills/pull/601))
-* Gradle and Lombok bump, changing CI java to 24 and adjusting AD getConfigRequest ([#615](https://github.com/opensearch-project/skills/pull/615))
 
 ### Maintenance
-* [AUTO] Increment version to 3.2.0-SNAPSHOT ([#605](https://github.com/opensearch-project/skills/pull/605))
+* Update the maven snapshot publish endpoint and credential ([#601](https://github.com/opensearch-project/skills/pull/601))
+* Bump gradle, java, lombok and fix ad configrequest change ([#615](https://github.com/opensearch-project/skills/pull/615))
+* Bump version to 3.2.0.0 ([#605](https://github.com/opensearch-project/skills/pull/605))

src/main/java/org/opensearch/agent/tools/AbstractRetrieverTool.java

@@ -22,6 +22,7 @@
 import org.opensearch.core.xcontent.NamedXContentRegistry;
 import org.opensearch.core.xcontent.XContentParser;
 import org.opensearch.ml.common.spi.tools.Tool;
+import org.opensearch.ml.common.utils.ToolUtils;
 import org.opensearch.search.SearchHit;
 import org.opensearch.search.builder.SearchSourceBuilder;
 import org.opensearch.transport.client.Client;
@@ -94,7 +95,8 @@ protected <T> SearchRequest buildSearchRequest(Map<String, String> parameters) t
     }
 
     @Override
-    public <T> void run(Map<String, String> parameters, ActionListener<T> listener) {
+    public <T> void run(Map<String, String> originalParameters, ActionListener<T> listener) {
+        Map<String, String> parameters = ToolUtils.extractInputParameters(originalParameters, attributes);
         SearchRequest searchRequest;
         try {
             searchRequest = buildSearchRequest(parameters);

src/main/java/org/opensearch/agent/tools/CreateAlertTool.java

@@ -39,6 +39,7 @@
 import org.opensearch.ml.common.transport.prediction.MLPredictionTaskAction;
 import org.opensearch.ml.common.transport.prediction.MLPredictionTaskRequest;
 import org.opensearch.ml.common.utils.StringUtils;
+import org.opensearch.ml.common.utils.ToolUtils;
 import org.opensearch.transport.client.Client;
 
 import com.google.gson.reflect.TypeToken;
@@ -133,7 +134,8 @@ public boolean validate(Map<String, String> parameters) {
     }
 
     @Override
-    public <T> void run(Map<String, String> parameters, ActionListener<T> listener) {
+    public <T> void run(Map<String, String> originalParameters, ActionListener<T> listener) {
+        Map<String, String> parameters = ToolUtils.extractInputParameters(originalParameters, attributes);
         Map<String, String> tmpParams = new HashMap<>(parameters);
         if (!tmpParams.containsKey("indices") || Strings.isEmpty(tmpParams.get("indices"))) {
             throw new IllegalArgumentException(

src/main/java/org/opensearch/agent/tools/CreateAnomalyDetectorTool.java

@@ -44,6 +44,7 @@
 import org.opensearch.ml.common.spi.tools.WithModelTool;
 import org.opensearch.ml.common.transport.prediction.MLPredictionTaskAction;
 import org.opensearch.ml.common.transport.prediction.MLPredictionTaskRequest;
+import org.opensearch.ml.common.utils.ToolUtils;
 import org.opensearch.transport.client.Client;
 
 import com.google.common.collect.ImmutableMap;
@@ -169,6 +170,7 @@ public CreateAnomalyDetectorTool(Client client, String modelId, String modelType
      */
     @Override
     public <T> void run(Map<String, String> parameters, ActionListener<T> listener) {
+        parameters = ToolUtils.extractInputParameters(parameters, attributes);
         final String tenantId = parameters.get(TENANT_ID_FIELD);
         Map<String, String> enrichedParameters = enrichParameters(parameters);
         String indexName = enrichedParameters.get("index");

src/main/java/org/opensearch/agent/tools/DynamicTool.java

@@ -25,6 +25,7 @@
 import org.opensearch.core.xcontent.XContentParser;
 import org.opensearch.ml.common.spi.tools.Tool;
 import org.opensearch.ml.common.spi.tools.ToolAnnotation;
+import org.opensearch.ml.common.utils.ToolUtils;
 import org.opensearch.rest.DynamicRestRequestCreator;
 import org.opensearch.rest.DynamicToolExecutor;
 import org.opensearch.rest.RestRequest;
@@ -114,7 +115,8 @@ public boolean validate(Map<String, String> map) {
     }
 
     @Override
-    public <T> void run(Map<String, String> parameters, ActionListener<T> listener) {
+    public <T> void run(Map<String, String> originalParameters, ActionListener<T> listener) {
+        Map<String, String> parameters = ToolUtils.extractInputParameters(originalParameters, attributes);
         RestRequest.Method method = RestRequest.Method.valueOf(parameters.get(METHOD_KEY));
         String uri = parameters.get(URI_KEY);
         String requestBody = parameters.get(REQUEST_BODY_KEY);

src/main/java/org/opensearch/agent/tools/LogPatternTool.java

@@ -31,6 +31,7 @@
 import org.opensearch.core.common.util.CollectionUtils;
 import org.opensearch.core.xcontent.NamedXContentRegistry;
 import org.opensearch.ml.common.spi.tools.ToolAnnotation;
+import org.opensearch.ml.common.utils.ToolUtils;
 import org.opensearch.search.SearchHit;
 import org.opensearch.sql.plugin.transport.PPLQueryAction;
 import org.opensearch.sql.plugin.transport.TransportPPLQueryRequest;
@@ -107,7 +108,8 @@ protected String getQueryBody(String queryText) {
     }
 
     @Override
-    public <T> void run(Map<String, String> parameters, ActionListener<T> listener) {
+    public <T> void run(Map<String, String> originalParameters, ActionListener<T> listener) {
+        Map<String, String> parameters = ToolUtils.extractInputParameters(originalParameters, attributes);
         String dsl = parameters.get(INPUT_FIELD);
         String ppl = parameters.get(PPL_FIELD);
         if (!StringUtils.isBlank(dsl)) {

src/main/java/org/opensearch/agent/tools/PPLTool.java

@@ -52,6 +52,7 @@
 import org.opensearch.ml.common.spi.tools.WithModelTool;
 import org.opensearch.ml.common.transport.prediction.MLPredictionTaskAction;
 import org.opensearch.ml.common.transport.prediction.MLPredictionTaskRequest;
+import org.opensearch.ml.common.utils.ToolUtils;
 import org.opensearch.search.SearchHit;
 import org.opensearch.search.builder.SearchSourceBuilder;
 import org.opensearch.sql.plugin.transport.PPLQueryAction;
@@ -196,7 +197,8 @@ public PPLTool(
 
     @SuppressWarnings("unchecked")
     @Override
-    public <T> void run(Map<String, String> parameters, ActionListener<T> listener) {
+    public <T> void run(Map<String, String> originalParameters, ActionListener<T> listener) {
+        Map<String, String> parameters = ToolUtils.extractInputParameters(originalParameters, attributes);
         final String tenantId = parameters.get(TENANT_ID_FIELD);
         extractFromChatParameters(parameters);
         String indexName = getIndexNameFromParameters(parameters);

src/main/java/org/opensearch/agent/tools/RAGTool.java

@@ -28,6 +28,7 @@
 import org.opensearch.ml.common.spi.tools.WithModelTool;
 import org.opensearch.ml.common.transport.prediction.MLPredictionTaskAction;
 import org.opensearch.ml.common.transport.prediction.MLPredictionTaskRequest;
+import org.opensearch.ml.common.utils.ToolUtils;
 import org.opensearch.transport.client.Client;
 
 import com.google.gson.Gson;
@@ -95,7 +96,9 @@ public Object parse(Object o) {
         };
     }
 
-    public <T> void run(Map<String, String> parameters, ActionListener<T> listener) {
+    public <T> void run(Map<String, String> originalParameters, ActionListener<T> listener) {
+        Map<String, String> parameters = ToolUtils.extractInputParameters(originalParameters, attributes);
+
         final String tenantId = parameters.get(TENANT_ID_FIELD);
 
         String input = null;

src/main/java/org/opensearch/agent/tools/SearchAlertsTool.java

@@ -21,6 +21,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.ml.common.utils.ToolUtils;
 import org.opensearch.transport.client.Client;
 import org.opensearch.transport.client.node.NodeClient;
 
@@ -70,7 +71,8 @@ public Object parse(Object o) {
     }
 
     @Override
-    public <T> void run(Map<String, String> parameters, ActionListener<T> listener) {
+    public <T> void run(Map<String, String> originalParameters, ActionListener<T> listener) {
+        Map<String, String> parameters = ToolUtils.extractInputParameters(originalParameters, attributes);
         final String tableSortOrder = parameters.getOrDefault("sortOrder", "asc");
         final String tableSortString = parameters.getOrDefault("sortString", "monitor_name.keyword");
         final int tableSize = parameters.containsKey("size") && StringUtils.isNumeric(parameters.get("size"))

src/main/java/org/opensearch/agent/tools/SearchAnomalyDetectorsTool.java

@@ -36,6 +36,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.ml.common.utils.ToolUtils;
 import org.opensearch.search.SearchHit;
 import org.opensearch.search.builder.SearchSourceBuilder;
 import org.opensearch.search.sort.SortOrder;
@@ -94,7 +95,8 @@ public Object parse(Object o) {
     // number of total detectors. 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 <T> void run(Map<String, String> parameters, ActionListener<T> listener) {
+    public <T> void run(Map<String, String> originalParameters, ActionListener<T> listener) {
+        Map<String, String> parameters = ToolUtils.extractInputParameters(originalParameters, attributes);
         final String detectorName = parameters.getOrDefault("detectorName", null);
         final String detectorNamePattern = parameters.getOrDefault("detectorNamePattern", null);
         final String indices = parameters.getOrDefault("indices", null);

src/main/java/org/opensearch/agent/tools/SearchAnomalyResultsTool.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.ml.common.utils.ToolUtils;
 import org.opensearch.search.SearchHit;
 import org.opensearch.search.SearchHits;
 import org.opensearch.search.builder.SearchSourceBuilder;
@@ -84,7 +85,8 @@ public Object parse(Object o) {
     // and total # of results. 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 <T> void run(Map<String, String> parameters, ActionListener<T> listener) {
+    public <T> void run(Map<String, String> originalParameters, ActionListener<T> listener) {
+        Map<String, String> parameters = ToolUtils.extractInputParameters(originalParameters, attributes);
         final String detectorId = parameters.getOrDefault("detectorId", null);
         final Boolean realTime = parameters.containsKey("realTime") ? Boolean.parseBoolean(parameters.get("realTime")) : null;
         final Double anomalyGradeThreshold = parameters.containsKey("anomalyGradeThreshold")

src/main/java/org/opensearch/agent/tools/SearchMonitorsTool.java

@@ -30,6 +30,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.ml.common.utils.ToolUtils;
 import org.opensearch.search.SearchHit;
 import org.opensearch.search.builder.SearchSourceBuilder;
 import org.opensearch.search.sort.SortOrder;
@@ -84,7 +85,8 @@ public Object parse(Object o) {
     // 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 <T> void run(Map<String, String> parameters, ActionListener<T> listener) {
+    public <T> void run(Map<String, String> originalParameters, ActionListener<T> listener) {
+        Map<String, String> parameters = ToolUtils.extractInputParameters(originalParameters, attributes);
         final String monitorId = parameters.getOrDefault("monitorId", null);
         final String monitorName = parameters.getOrDefault("monitorName", null);
         final String monitorNamePattern = parameters.getOrDefault("monitorNamePattern", null);

src/main/java/org/opensearch/agent/tools/WebSearchTool.java

@@ -33,6 +33,7 @@
 import org.opensearch.ml.common.spi.tools.Tool;
 import org.opensearch.ml.common.spi.tools.ToolAnnotation;
 import org.opensearch.ml.common.utils.StringUtils;
+import org.opensearch.ml.common.utils.ToolUtils;
 import org.opensearch.threadpool.ThreadPool;
 
 import com.google.common.collect.ImmutableMap;
@@ -99,7 +100,8 @@ public WebSearchTool(ThreadPool threadPool) {
     }
 
     @Override
-    public <T> void run(Map<String, String> parameters, ActionListener<T> listener) {
+    public <T> void run(Map<String, String> originalParameters, ActionListener<T> listener) {
+        Map<String, String> parameters = ToolUtils.extractInputParameters(originalParameters, attributes);
         try {
             // common search parameters
             String query = parameters.getOrDefault("query", parameters.get("question")).replaceAll(" ", "+");

src/main/java/org/opensearch/agent/tools/utils/ToolConstants.java

@@ -36,5 +36,4 @@ public static ModelType from(String value) {
 
     public static final String ALERTING_CONFIG_INDEX = ".opendistro-alerting-config";
     public static final String ALERTING_ALERTS_INDEX = ".opendistro-alerting-alerts";
-
 }

src/test/java/org/opensearch/agent/tools/AbstractRetrieverToolTests.java

@@ -165,7 +165,9 @@ public void testRunAsyncWithIllegalQueryThenListenerOnFailure() {
         mockedImpl.run(null, listener4);
 
         Exception exception4 = assertThrows(Exception.class, future4::join);
-        assertTrue(exception4.getCause() instanceof NullPointerException);
+        // parameter is re-created with extractInputParameters, thus will not be null
+        assertTrue(exception4.getCause() instanceof IllegalArgumentException);
+        assertEquals(exception4.getCause().getMessage(), "[input] is null or empty, can not process it.");
     }
 
     @Test