From c5fa197737be263d3d12fa822ad5300ae95e4013 Mon Sep 17 00:00:00 2001 From: Siddhant Deshmukh Date: Tue, 3 Sep 2024 15:57:16 -0700 Subject: [PATCH 1/3] Add Query Grouping Integtests Signed-off-by: Siddhant Deshmukh --- .../insights/QueryInsightsRestTestCase.java | 118 ++++++++++++- .../grouper/MinMaxQueryGrouperByNoneIT.java | 75 ++++++++ .../MinMaxQueryGrouperBySimilarityIT.java | 160 ++++++++++++++++++ .../top_queries/TopQueriesRestIT.java | 33 ++-- 4 files changed, 372 insertions(+), 14 deletions(-) create mode 100644 src/test/java/org/opensearch/plugin/insights/core/service/grouper/MinMaxQueryGrouperByNoneIT.java create mode 100644 src/test/java/org/opensearch/plugin/insights/core/service/grouper/MinMaxQueryGrouperBySimilarityIT.java diff --git a/src/test/java/org/opensearch/plugin/insights/QueryInsightsRestTestCase.java b/src/test/java/org/opensearch/plugin/insights/QueryInsightsRestTestCase.java index f9c90fd7..d973b26a 100644 --- a/src/test/java/org/opensearch/plugin/insights/QueryInsightsRestTestCase.java +++ b/src/test/java/org/opensearch/plugin/insights/QueryInsightsRestTestCase.java @@ -9,10 +9,13 @@ package org.opensearch.plugin.insights; import java.io.IOException; +import java.nio.charset.StandardCharsets; import java.util.Collections; import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import java.util.stream.Collectors; import org.apache.hc.client5.http.auth.AuthScope; import org.apache.hc.client5.http.auth.UsernamePasswordCredentials; @@ -184,8 +187,22 @@ protected String defaultTopQueriesSettings() { return "{\n" + " \"persistent\" : {\n" + " \"search.insights.top_queries.latency.enabled\" : \"true\",\n" - + " \"search.insights.top_queries.latency.window_size\" : \"600s\",\n" - + " \"search.insights.top_queries.latency.top_n_size\" : 5\n" + + " \"search.insights.top_queries.latency.window_size\" : \"1m\",\n" + + " \"search.insights.top_queries.latency.top_n_size\" : 5,\n" + + " \"search.insights.top_queries.group_by\" : \"none\"\n" + + " }\n" + + "}"; + } + + + protected String defaultTopQueryGroupingSettings() { + return "{\n" + + " \"persistent\" : {\n" + + " \"search.insights.top_queries.latency.enabled\" : \"true\",\n" + + " \"search.insights.top_queries.latency.window_size\" : \"1m\",\n" + + " \"search.insights.top_queries.latency.top_n_size\" : 5,\n" + + " \"search.insights.top_queries.group_by\" : \"similarity\",\n" + + " \"search.insights.top_queries.max_groups\" : 5\n" + " }\n" + "}"; } @@ -213,4 +230,101 @@ protected void doSearch(int times) throws IOException { Assert.assertEquals(200, response.getStatusLine().getStatusCode()); } } + + protected void doSearch(String queryType, int times) throws IOException { + for (int i = 0; i < times; i++) { + // Do Search + Request request = new Request("GET", "/my-index-0/_search?size=20&pretty"); + + // Set query based on the query type + request.setJsonEntity(searchBody(queryType)); + + Response response = client().performRequest(request); + Assert.assertEquals(200, response.getStatusLine().getStatusCode()); + } + } + + private String searchBody(String queryType) { + switch (queryType) { + case "match": + // Query shape 1: Match query + return "{\n" + + " \"query\": {\n" + + " \"match\": {\n" + + " \"field1\": \"value1\"\n" + + " }\n" + + " }\n" + + "}"; + + case "range": + // Query shape 2: Range query + return "{\n" + + " \"query\": {\n" + + " \"range\": {\n" + + " \"field2\": {\n" + + " \"gte\": 10,\n" + + " \"lte\": 50\n" + + " }\n" + + " }\n" + + " }\n" + + "}"; + + case "term": + // Query shape 3: Term query + return "{\n" + + " \"query\": {\n" + + " \"term\": {\n" + + " \"field3\": {\n" + + " \"value\": \"exact-value\"\n" + + " }\n" + + " }\n" + + " }\n" + + "}"; + + default: + throw new IllegalArgumentException("Unknown query type: " + queryType); + } + } + + protected int countTopQueries(String json) { + // Basic pattern to match JSON array elements in `top_queries` + Pattern pattern = Pattern.compile("\\{\\s*\"timestamp\""); + Matcher matcher = pattern.matcher(json); + + int count = 0; + while (matcher.find()) { + count++; + } + + return count; + } + + protected void waitForEmptyTopQueriesResponse() throws IOException, InterruptedException { + boolean isEmpty = false; + long timeoutMillis = 70000; // 70 seconds timeout + long startTime = System.currentTimeMillis(); + + while (!isEmpty && (System.currentTimeMillis() - startTime) < timeoutMillis) { + Request request = new Request("GET", "/_insights/top_queries?pretty"); + Response response = client().performRequest(request); + + if (response.getStatusLine().getStatusCode() != 200) { + Thread.sleep(1000); // Sleep before retrying + continue; + } + + String responseBody = new String(response.getEntity().getContent().readAllBytes(), StandardCharsets.UTF_8); + + if (countTopQueries(responseBody) == 0) { + isEmpty = true; + } else { + Thread.sleep(1000); // Sleep before retrying + } + } + + if (!isEmpty) { + throw new IllegalStateException("Top queries response did not become empty within the timeout period"); + } + } + } diff --git a/src/test/java/org/opensearch/plugin/insights/core/service/grouper/MinMaxQueryGrouperByNoneIT.java b/src/test/java/org/opensearch/plugin/insights/core/service/grouper/MinMaxQueryGrouperByNoneIT.java new file mode 100644 index 00000000..6d9ef132 --- /dev/null +++ b/src/test/java/org/opensearch/plugin/insights/core/service/grouper/MinMaxQueryGrouperByNoneIT.java @@ -0,0 +1,75 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ +package org.opensearch.plugin.insights.core.service.grouper; + +import org.junit.Assert; +import org.opensearch.client.Request; +import org.opensearch.client.Response; +import org.opensearch.plugin.insights.QueryInsightsRestTestCase; +import org.opensearch.plugin.insights.settings.QueryInsightsSettings; + +import java.io.IOException; +import java.nio.charset.StandardCharsets; + +/** + * ITs for Grouping Top Queries by none + */ +public class MinMaxQueryGrouperByNoneIT extends QueryInsightsRestTestCase { + + /** + * Grouping by none should not group queries + * @throws IOException + * @throws InterruptedException + */ + public void testGroupingByNone() throws IOException, InterruptedException { + + waitForEmptyTopQueriesResponse(); + + // Enable top N feature and grouping by none + Request request = new Request("PUT", "/_cluster/settings"); + request.setJsonEntity(groupByNoneSettings()); + Response response = client().performRequest(request); + Assert.assertEquals(200, response.getStatusLine().getStatusCode()); + + // Search + doSearch("range", 2); + doSearch("match", 6); + doSearch("term", 4); + + // Ensure records are drained to the top queries service + Thread.sleep(QueryInsightsSettings.QUERY_RECORD_QUEUE_DRAIN_INTERVAL.millis()); + + // run five times to make sure the records are drained to the top queries services + for (int i = 0; i < 5; i++) { + // Get Top Queries and validate + request = new Request("GET", "/_insights/top_queries?pretty"); + response = client().performRequest(request); + Assert.assertEquals(200, response.getStatusLine().getStatusCode()); + String top_requests = new String(response.getEntity().getContent().readAllBytes(), StandardCharsets.UTF_8); + + int top_n_array_size = countTopQueries(top_requests); + + // Validate that all queries are listed separately (no grouping) + Assert.assertEquals(12, top_n_array_size); + } + } + + private String groupByNoneSettings() { + return "{\n" + + " \"persistent\" : {\n" + + " \"search.insights.top_queries.latency.enabled\" : \"true\",\n" + + " \"search.insights.top_queries.latency.window_size\" : \"1m\",\n" + + " \"search.insights.top_queries.latency.top_n_size\" : 100,\n" + + " \"search.insights.top_queries.group_by\" : \"none\",\n" + + " \"search.insights.top_queries.max_groups\" : 5\n" + + " }\n" + + "}"; + } +} + + diff --git a/src/test/java/org/opensearch/plugin/insights/core/service/grouper/MinMaxQueryGrouperBySimilarityIT.java b/src/test/java/org/opensearch/plugin/insights/core/service/grouper/MinMaxQueryGrouperBySimilarityIT.java new file mode 100644 index 00000000..2af9ec6f --- /dev/null +++ b/src/test/java/org/opensearch/plugin/insights/core/service/grouper/MinMaxQueryGrouperBySimilarityIT.java @@ -0,0 +1,160 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ +package org.opensearch.plugin.insights.core.service.grouper; + +import org.junit.Assert; +import org.opensearch.client.Request; +import org.opensearch.client.Response; +import org.opensearch.client.ResponseException; +import org.opensearch.plugin.insights.QueryInsightsRestTestCase; +import org.opensearch.plugin.insights.settings.QueryInsightsSettings; + +import java.io.IOException; +import java.nio.charset.StandardCharsets; + +/** + * ITs for Grouping Top Queries by similarity + */ +public class MinMaxQueryGrouperBySimilarityIT extends QueryInsightsRestTestCase { + + /** + * test grouping top queries + * + * @throws IOException IOException + */ + public void testGroupingBySimilarity() throws IOException, InterruptedException { + + waitForEmptyTopQueriesResponse(); + + // Enable top N feature and grouping feature + Request request = new Request("PUT", "/_cluster/settings"); + request.setJsonEntity(defaultTopQueryGroupingSettings()); + Response response = client().performRequest(request); + Assert.assertEquals(200, response.getStatusLine().getStatusCode()); + + // Search + doSearch("range", 2); + doSearch("match", 6); + doSearch("term", 4); + + // run five times to make sure the records are drained to the top queries services + for (int i = 0; i < 5; i++) { + // Get Top Queries + request = new Request("GET", "/_insights/top_queries?pretty"); + response = client().performRequest(request); + Assert.assertEquals(200, response.getStatusLine().getStatusCode()); + + String responseBody = new String(response.getEntity().getContent().readAllBytes(), StandardCharsets.UTF_8); + + // Extract and count top_queries + int topNArraySize = countTopQueries(responseBody); + + if (topNArraySize == 0) { + Thread.sleep(QueryInsightsSettings.QUERY_RECORD_QUEUE_DRAIN_INTERVAL.millis()); + continue; + } + + Assert.assertEquals(3, topNArraySize); + } + } + + /** + * Test invalid query grouping settings + * + * @throws IOException IOException + */ + public void testInvalidQueryGroupingSettings() throws IOException { + for (String setting : invalidQueryGroupingSettings()) { + Request request = new Request("PUT", "/_cluster/settings"); + request.setJsonEntity(setting); + try { + client().performRequest(request); + fail("Should not succeed with invalid query grouping settings"); + } catch (ResponseException e) { + assertEquals(400, e.getResponse().getStatusLine().getStatusCode()); + } + } + } + + /** + * Test valid query grouping settings + * + * @throws IOException IOException + */ + public void testValidQueryGroupingSettings() throws IOException { + for (String setting : validQueryGroupingSettings()) { + Request request = new Request("PUT", "/_cluster/settings"); + request.setJsonEntity(setting); + Response response = client().performRequest(request); + assertEquals(200, response.getStatusLine().getStatusCode()); + } + } + + private String[] invalidQueryGroupingSettings() { + return new String[] { + // Invalid max_groups: below minimum (0) + "{\n" + + " \"persistent\" : {\n" + + " \"search.insights.top_queries.max_groups\" : 0\n" + + " }\n" + + "}", + + // Invalid max_groups: above maximum (10001) + "{\n" + + " \"persistent\" : {\n" + + " \"search.insights.top_queries.max_groups\" : 10001\n" + + " }\n" + + "}", + + // Invalid group_by: unsupported value + "{\n" + + " \"persistent\" : {\n" + + " \"search.insights.top_queries.group_by\" : \"unsupported_value\"\n" + + " }\n" + + "}" + }; + } + + private String[] validQueryGroupingSettings() { + return new String[]{ + // Valid max_groups: minimum value (1) + "{\n" + + " \"persistent\" : {\n" + + " \"search.insights.top_queries.max_groups\" : 1\n" + + " }\n" + + "}", + + // Valid max_groups: maximum value (10000) + "{\n" + + " \"persistent\" : {\n" + + " \"search.insights.top_queries.max_groups\" : 10000\n" + + " }\n" + + "}", + + // Valid group_by: supported value (SIMILARITY) + "{\n" + + " \"persistent\" : {\n" + + " \"search.insights.top_queries.group_by\" : \"SIMILARITY\"\n" + + " }\n" + + "}" + }; + } + + private String groupByNoneSettings() { + return "{\n" + + " \"persistent\" : {\n" + + " \"search.insights.top_queries.latency.enabled\" : \"true\",\n" + + " \"search.insights.top_queries.latency.window_size\" : \"1m\",\n" + + " \"search.insights.top_queries.latency.top_n_size\" : 100,\n" + + " \"search.insights.top_queries.group_by\" : \"none\",\n" + + " \"search.insights.top_queries.max_groups\" : 5\n" + + " }\n" + + "}"; + } +} + diff --git a/src/test/java/org/opensearch/plugin/insights/rules/resthandler/top_queries/TopQueriesRestIT.java b/src/test/java/org/opensearch/plugin/insights/rules/resthandler/top_queries/TopQueriesRestIT.java index 50f49df8..0a1c6eb0 100644 --- a/src/test/java/org/opensearch/plugin/insights/rules/resthandler/top_queries/TopQueriesRestIT.java +++ b/src/test/java/org/opensearch/plugin/insights/rules/resthandler/top_queries/TopQueriesRestIT.java @@ -49,6 +49,8 @@ public void testQueryInsightsPluginInstalled() throws IOException { * @throws IOException IOException */ public void testTopQueriesResponses() throws IOException, InterruptedException { + waitForEmptyTopQueriesResponse(); + // Enable Top N Queries feature Request request = new Request("PUT", "/_cluster/settings"); request.setJsonEntity(defaultTopQueriesSettings()); @@ -61,14 +63,16 @@ public void testTopQueriesResponses() throws IOException, InterruptedException { request = new Request("GET", "/_insights/top_queries?pretty"); response = client().performRequest(request); Assert.assertEquals(200, response.getStatusLine().getStatusCode()); - String top_requests = new String(response.getEntity().getContent().readAllBytes(), StandardCharsets.UTF_8); - Assert.assertTrue(top_requests.contains("top_queries")); - int top_n_array_size = top_requests.split("timestamp", -1).length - 1; - if (top_n_array_size == 0) { + String topRequests = new String(response.getEntity().getContent().readAllBytes(), StandardCharsets.UTF_8); + Assert.assertTrue(topRequests.contains("top_queries")); + + int topNArraySize = countTopQueries(topRequests); + + if (topNArraySize == 0) { Thread.sleep(QueryInsightsSettings.QUERY_RECORD_QUEUE_DRAIN_INTERVAL.millis()); continue; } - Assert.assertEquals(2, top_n_array_size); + Assert.assertEquals(2, topNArraySize); } // Enable Top N Queries by resource usage @@ -78,20 +82,24 @@ public void testTopQueriesResponses() throws IOException, InterruptedException { Assert.assertEquals(200, response.getStatusLine().getStatusCode()); // Do Search doSearch(2); - // run five times to make sure the records are drained to the top queries services + + // Run five times to make sure the records are drained to the top queries services for (int i = 0; i < 5; i++) { // Get Top Queries request = new Request("GET", "/_insights/top_queries?type=cpu&pretty"); response = client().performRequest(request); Assert.assertEquals(200, response.getStatusLine().getStatusCode()); - String top_requests = new String(response.getEntity().getContent().readAllBytes(), StandardCharsets.UTF_8); - Assert.assertTrue(top_requests.contains("top_queries")); - int top_n_array_size = top_requests.split("timestamp", -1).length - 1; - if (top_n_array_size == 0) { + String topRequests = new String(response.getEntity().getContent().readAllBytes(), StandardCharsets.UTF_8); + Assert.assertTrue(topRequests.contains("top_queries")); + + // Use the countTopQueries method to determine the number of top queries + int topNArraySize = countTopQueries(topRequests); + + if (topNArraySize == 0) { Thread.sleep(QueryInsightsSettings.QUERY_RECORD_QUEUE_DRAIN_INTERVAL.millis()); continue; } - Assert.assertEquals(2, top_n_array_size); + Assert.assertEquals(2, topNArraySize); } } @@ -121,7 +129,8 @@ private String topQueriesByResourceUsagesSettings() { + " \"search.insights.top_queries.memory.top_n_size\" : \"5\",\n" + " \"search.insights.top_queries.cpu.enabled\" : \"true\",\n" + " \"search.insights.top_queries.cpu.window_size\" : \"600s\",\n" - + " \"search.insights.top_queries.cpu.top_n_size\" : 5\n" + + " \"search.insights.top_queries.cpu.top_n_size\" : 5,\n" + + " \"search.insights.top_queries.group_by\" : \"none\"\n" + " }\n" + "}"; } From 33726b6e406275bcf241aba7cccdc2f149fff18f Mon Sep 17 00:00:00 2001 From: Siddhant Deshmukh Date: Tue, 3 Sep 2024 15:59:52 -0700 Subject: [PATCH 2/3] Spotless apply Signed-off-by: Siddhant Deshmukh --- .../insights/QueryInsightsRestTestCase.java | 47 ++++++------- .../grouper/MinMaxQueryGrouperByNoneIT.java | 25 +++---- .../MinMaxQueryGrouperBySimilarityIT.java | 68 ++++++------------- 3 files changed, 53 insertions(+), 87 deletions(-) diff --git a/src/test/java/org/opensearch/plugin/insights/QueryInsightsRestTestCase.java b/src/test/java/org/opensearch/plugin/insights/QueryInsightsRestTestCase.java index d973b26a..2852dec0 100644 --- a/src/test/java/org/opensearch/plugin/insights/QueryInsightsRestTestCase.java +++ b/src/test/java/org/opensearch/plugin/insights/QueryInsightsRestTestCase.java @@ -194,7 +194,6 @@ protected String defaultTopQueriesSettings() { + "}"; } - protected String defaultTopQueryGroupingSettings() { return "{\n" + " \"persistent\" : {\n" @@ -248,38 +247,32 @@ private String searchBody(String queryType) { switch (queryType) { case "match": // Query shape 1: Match query - return "{\n" + - " \"query\": {\n" + - " \"match\": {\n" + - " \"field1\": \"value1\"\n" + - " }\n" + - " }\n" + - "}"; + return "{\n" + " \"query\": {\n" + " \"match\": {\n" + " \"field1\": \"value1\"\n" + " }\n" + " }\n" + "}"; case "range": // Query shape 2: Range query - return "{\n" + - " \"query\": {\n" + - " \"range\": {\n" + - " \"field2\": {\n" + - " \"gte\": 10,\n" + - " \"lte\": 50\n" + - " }\n" + - " }\n" + - " }\n" + - "}"; + return "{\n" + + " \"query\": {\n" + + " \"range\": {\n" + + " \"field2\": {\n" + + " \"gte\": 10,\n" + + " \"lte\": 50\n" + + " }\n" + + " }\n" + + " }\n" + + "}"; case "term": // Query shape 3: Term query - return "{\n" + - " \"query\": {\n" + - " \"term\": {\n" + - " \"field3\": {\n" + - " \"value\": \"exact-value\"\n" + - " }\n" + - " }\n" + - " }\n" + - "}"; + return "{\n" + + " \"query\": {\n" + + " \"term\": {\n" + + " \"field3\": {\n" + + " \"value\": \"exact-value\"\n" + + " }\n" + + " }\n" + + " }\n" + + "}"; default: throw new IllegalArgumentException("Unknown query type: " + queryType); diff --git a/src/test/java/org/opensearch/plugin/insights/core/service/grouper/MinMaxQueryGrouperByNoneIT.java b/src/test/java/org/opensearch/plugin/insights/core/service/grouper/MinMaxQueryGrouperByNoneIT.java index 6d9ef132..86ae2440 100644 --- a/src/test/java/org/opensearch/plugin/insights/core/service/grouper/MinMaxQueryGrouperByNoneIT.java +++ b/src/test/java/org/opensearch/plugin/insights/core/service/grouper/MinMaxQueryGrouperByNoneIT.java @@ -7,15 +7,14 @@ */ package org.opensearch.plugin.insights.core.service.grouper; +import java.io.IOException; +import java.nio.charset.StandardCharsets; import org.junit.Assert; import org.opensearch.client.Request; import org.opensearch.client.Response; import org.opensearch.plugin.insights.QueryInsightsRestTestCase; import org.opensearch.plugin.insights.settings.QueryInsightsSettings; -import java.io.IOException; -import java.nio.charset.StandardCharsets; - /** * ITs for Grouping Top Queries by none */ @@ -60,16 +59,14 @@ public void testGroupingByNone() throws IOException, InterruptedException { } private String groupByNoneSettings() { - return "{\n" + - " \"persistent\" : {\n" + - " \"search.insights.top_queries.latency.enabled\" : \"true\",\n" + - " \"search.insights.top_queries.latency.window_size\" : \"1m\",\n" + - " \"search.insights.top_queries.latency.top_n_size\" : 100,\n" + - " \"search.insights.top_queries.group_by\" : \"none\",\n" + - " \"search.insights.top_queries.max_groups\" : 5\n" + - " }\n" + - "}"; + return "{\n" + + " \"persistent\" : {\n" + + " \"search.insights.top_queries.latency.enabled\" : \"true\",\n" + + " \"search.insights.top_queries.latency.window_size\" : \"1m\",\n" + + " \"search.insights.top_queries.latency.top_n_size\" : 100,\n" + + " \"search.insights.top_queries.group_by\" : \"none\",\n" + + " \"search.insights.top_queries.max_groups\" : 5\n" + + " }\n" + + "}"; } } - - diff --git a/src/test/java/org/opensearch/plugin/insights/core/service/grouper/MinMaxQueryGrouperBySimilarityIT.java b/src/test/java/org/opensearch/plugin/insights/core/service/grouper/MinMaxQueryGrouperBySimilarityIT.java index 2af9ec6f..66567b18 100644 --- a/src/test/java/org/opensearch/plugin/insights/core/service/grouper/MinMaxQueryGrouperBySimilarityIT.java +++ b/src/test/java/org/opensearch/plugin/insights/core/service/grouper/MinMaxQueryGrouperBySimilarityIT.java @@ -7,6 +7,8 @@ */ package org.opensearch.plugin.insights.core.service.grouper; +import java.io.IOException; +import java.nio.charset.StandardCharsets; import org.junit.Assert; import org.opensearch.client.Request; import org.opensearch.client.Response; @@ -14,9 +16,6 @@ import org.opensearch.plugin.insights.QueryInsightsRestTestCase; import org.opensearch.plugin.insights.settings.QueryInsightsSettings; -import java.io.IOException; -import java.nio.charset.StandardCharsets; - /** * ITs for Grouping Top Queries by similarity */ @@ -98,63 +97,40 @@ public void testValidQueryGroupingSettings() throws IOException { private String[] invalidQueryGroupingSettings() { return new String[] { // Invalid max_groups: below minimum (0) - "{\n" + - " \"persistent\" : {\n" + - " \"search.insights.top_queries.max_groups\" : 0\n" + - " }\n" + - "}", + "{\n" + " \"persistent\" : {\n" + " \"search.insights.top_queries.max_groups\" : 0\n" + " }\n" + "}", // Invalid max_groups: above maximum (10001) - "{\n" + - " \"persistent\" : {\n" + - " \"search.insights.top_queries.max_groups\" : 10001\n" + - " }\n" + - "}", + "{\n" + " \"persistent\" : {\n" + " \"search.insights.top_queries.max_groups\" : 10001\n" + " }\n" + "}", // Invalid group_by: unsupported value - "{\n" + - " \"persistent\" : {\n" + - " \"search.insights.top_queries.group_by\" : \"unsupported_value\"\n" + - " }\n" + - "}" - }; + "{\n" + + " \"persistent\" : {\n" + + " \"search.insights.top_queries.group_by\" : \"unsupported_value\"\n" + + " }\n" + + "}" }; } private String[] validQueryGroupingSettings() { - return new String[]{ + return new String[] { // Valid max_groups: minimum value (1) - "{\n" + - " \"persistent\" : {\n" + - " \"search.insights.top_queries.max_groups\" : 1\n" + - " }\n" + - "}", + "{\n" + " \"persistent\" : {\n" + " \"search.insights.top_queries.max_groups\" : 1\n" + " }\n" + "}", // Valid max_groups: maximum value (10000) - "{\n" + - " \"persistent\" : {\n" + - " \"search.insights.top_queries.max_groups\" : 10000\n" + - " }\n" + - "}", + "{\n" + " \"persistent\" : {\n" + " \"search.insights.top_queries.max_groups\" : 10000\n" + " }\n" + "}", // Valid group_by: supported value (SIMILARITY) - "{\n" + - " \"persistent\" : {\n" + - " \"search.insights.top_queries.group_by\" : \"SIMILARITY\"\n" + - " }\n" + - "}" - }; + "{\n" + " \"persistent\" : {\n" + " \"search.insights.top_queries.group_by\" : \"SIMILARITY\"\n" + " }\n" + "}" }; } private String groupByNoneSettings() { - return "{\n" + - " \"persistent\" : {\n" + - " \"search.insights.top_queries.latency.enabled\" : \"true\",\n" + - " \"search.insights.top_queries.latency.window_size\" : \"1m\",\n" + - " \"search.insights.top_queries.latency.top_n_size\" : 100,\n" + - " \"search.insights.top_queries.group_by\" : \"none\",\n" + - " \"search.insights.top_queries.max_groups\" : 5\n" + - " }\n" + - "}"; + return "{\n" + + " \"persistent\" : {\n" + + " \"search.insights.top_queries.latency.enabled\" : \"true\",\n" + + " \"search.insights.top_queries.latency.window_size\" : \"1m\",\n" + + " \"search.insights.top_queries.latency.top_n_size\" : 100,\n" + + " \"search.insights.top_queries.group_by\" : \"none\",\n" + + " \"search.insights.top_queries.max_groups\" : 5\n" + + " }\n" + + "}"; } } - From 8c23d8e467b4170908b9311401cc982c5890feff Mon Sep 17 00:00:00 2001 From: Siddhant Deshmukh Date: Wed, 4 Sep 2024 11:30:21 -0700 Subject: [PATCH 3/3] Fix settings name in tests Signed-off-by: Siddhant Deshmukh --- .../insights/QueryInsightsRestTestCase.java | 2 +- .../grouper/MinMaxQueryGrouperByNoneIT.java | 2 +- .../MinMaxQueryGrouperBySimilarityIT.java | 30 ++++++++++++++----- 3 files changed, 25 insertions(+), 9 deletions(-) diff --git a/src/test/java/org/opensearch/plugin/insights/QueryInsightsRestTestCase.java b/src/test/java/org/opensearch/plugin/insights/QueryInsightsRestTestCase.java index 2852dec0..15f52399 100644 --- a/src/test/java/org/opensearch/plugin/insights/QueryInsightsRestTestCase.java +++ b/src/test/java/org/opensearch/plugin/insights/QueryInsightsRestTestCase.java @@ -201,7 +201,7 @@ protected String defaultTopQueryGroupingSettings() { + " \"search.insights.top_queries.latency.window_size\" : \"1m\",\n" + " \"search.insights.top_queries.latency.top_n_size\" : 5,\n" + " \"search.insights.top_queries.group_by\" : \"similarity\",\n" - + " \"search.insights.top_queries.max_groups\" : 5\n" + + " \"search.insights.top_queries.max_groups_excluding_topn\" : 5\n" + " }\n" + "}"; } diff --git a/src/test/java/org/opensearch/plugin/insights/core/service/grouper/MinMaxQueryGrouperByNoneIT.java b/src/test/java/org/opensearch/plugin/insights/core/service/grouper/MinMaxQueryGrouperByNoneIT.java index 86ae2440..c7779cba 100644 --- a/src/test/java/org/opensearch/plugin/insights/core/service/grouper/MinMaxQueryGrouperByNoneIT.java +++ b/src/test/java/org/opensearch/plugin/insights/core/service/grouper/MinMaxQueryGrouperByNoneIT.java @@ -65,7 +65,7 @@ private String groupByNoneSettings() { + " \"search.insights.top_queries.latency.window_size\" : \"1m\",\n" + " \"search.insights.top_queries.latency.top_n_size\" : 100,\n" + " \"search.insights.top_queries.group_by\" : \"none\",\n" - + " \"search.insights.top_queries.max_groups\" : 5\n" + + " \"search.insights.top_queries.max_groups_excluding_topn\" : 5\n" + " }\n" + "}"; } diff --git a/src/test/java/org/opensearch/plugin/insights/core/service/grouper/MinMaxQueryGrouperBySimilarityIT.java b/src/test/java/org/opensearch/plugin/insights/core/service/grouper/MinMaxQueryGrouperBySimilarityIT.java index 66567b18..e32b6092 100644 --- a/src/test/java/org/opensearch/plugin/insights/core/service/grouper/MinMaxQueryGrouperBySimilarityIT.java +++ b/src/test/java/org/opensearch/plugin/insights/core/service/grouper/MinMaxQueryGrouperBySimilarityIT.java @@ -96,11 +96,19 @@ public void testValidQueryGroupingSettings() throws IOException { private String[] invalidQueryGroupingSettings() { return new String[] { - // Invalid max_groups: below minimum (0) - "{\n" + " \"persistent\" : {\n" + " \"search.insights.top_queries.max_groups\" : 0\n" + " }\n" + "}", + // Invalid max_groups: below minimum (-1) + "{\n" + + " \"persistent\" : {\n" + + " \"search.insights.top_queries.max_groups_excluding_topn\" : -1\n" + + " }\n" + + "}", // Invalid max_groups: above maximum (10001) - "{\n" + " \"persistent\" : {\n" + " \"search.insights.top_queries.max_groups\" : 10001\n" + " }\n" + "}", + "{\n" + + " \"persistent\" : {\n" + + " \"search.insights.top_queries.max_groups_excluding_topn\" : 10001\n" + + " }\n" + + "}", // Invalid group_by: unsupported value "{\n" @@ -112,11 +120,19 @@ private String[] invalidQueryGroupingSettings() { private String[] validQueryGroupingSettings() { return new String[] { - // Valid max_groups: minimum value (1) - "{\n" + " \"persistent\" : {\n" + " \"search.insights.top_queries.max_groups\" : 1\n" + " }\n" + "}", + // Valid max_groups: minimum value (0) + "{\n" + + " \"persistent\" : {\n" + + " \"search.insights.top_queries.max_groups_excluding_topn\" : 0\n" + + " }\n" + + "}", // Valid max_groups: maximum value (10000) - "{\n" + " \"persistent\" : {\n" + " \"search.insights.top_queries.max_groups\" : 10000\n" + " }\n" + "}", + "{\n" + + " \"persistent\" : {\n" + + " \"search.insights.top_queries.max_groups_excluding_topn\" : 10000\n" + + " }\n" + + "}", // Valid group_by: supported value (SIMILARITY) "{\n" + " \"persistent\" : {\n" + " \"search.insights.top_queries.group_by\" : \"SIMILARITY\"\n" + " }\n" + "}" }; @@ -129,7 +145,7 @@ private String groupByNoneSettings() { + " \"search.insights.top_queries.latency.window_size\" : \"1m\",\n" + " \"search.insights.top_queries.latency.top_n_size\" : 100,\n" + " \"search.insights.top_queries.group_by\" : \"none\",\n" - + " \"search.insights.top_queries.max_groups\" : 5\n" + + " \"search.insights.top_queries.max_groups_excluding_topn\" : 5\n" + " }\n" + "}"; }