From 1db4ab5a3a73cb27e7f856ab9ede6d32d900c102 Mon Sep 17 00:00:00 2001 From: Chenyang Ji Date: Tue, 16 Jan 2024 22:19:08 -0800 Subject: [PATCH 1/5] Top N Queries by latency implementation Signed-off-by: Chenyang Ji --- CHANGELOG.md | 1 + .../QueryInsightsPluginTransportIT.java | 218 +++++++++++++++ .../plugin/insights/TopQueriesRestIT.java | 107 ++++++++ .../listener/SearchQueryLatencyListener.java | 121 ++++++++ .../insights/core/listener/package-info.java | 12 + .../service/TopQueriesByLatencyService.java | 240 ++++++++++++++++ .../insights/rules/action/package-info.java | 12 + .../rules/action/top_queries/TopQueries.java | 71 +++++ .../action/top_queries/TopQueriesAction.java | 26 ++ .../action/top_queries/TopQueriesRequest.java | 96 +++++++ .../top_queries/TopQueriesResponse.java | 120 ++++++++ .../action/top_queries/package-info.java | 12 + .../rules/resthandler/package-info.java | 12 + .../top_queries/RestTopQueriesAction.java | 102 +++++++ .../resthandler/top_queries/package-info.java | 12 + .../rules/transport/package-info.java | 12 + .../TransportTopQueriesAction.java | 131 +++++++++ .../transport/top_queries/package-info.java | 12 + .../SearchQueryLatencyListenerTests.java | 182 ++++++++++++ .../TopQueriesByLatencyServiceTests.java | 259 ++++++++++++++++++ .../top_queries/TopQueriesRequestTests.java | 43 +++ .../top_queries/TopQueriesResponseTests.java | 49 ++++ .../action/top_queries/TopQueriesTests.java | 42 +++ .../model/SearchQueryLatencyRecordTests.java | 50 ++++ .../RestTopQueriesActionTests.java | 62 +++++ .../TransportTopQueriesActionTests.java | 88 ++++++ 26 files changed, 2092 insertions(+) create mode 100644 plugins/query-insights/src/internalClusterTest/java/org/opensearch/plugin/insights/QueryInsightsPluginTransportIT.java create mode 100644 plugins/query-insights/src/javaRestTest/java/org/opensearch/plugin/insights/TopQueriesRestIT.java create mode 100644 plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/listener/SearchQueryLatencyListener.java create mode 100644 plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/listener/package-info.java create mode 100644 plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/TopQueriesByLatencyService.java create mode 100644 plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/package-info.java create mode 100644 plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueries.java create mode 100644 plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesAction.java create mode 100644 plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesRequest.java create mode 100644 plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesResponse.java create mode 100644 plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/package-info.java create mode 100644 plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/resthandler/package-info.java create mode 100644 plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/resthandler/top_queries/RestTopQueriesAction.java create mode 100644 plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/resthandler/top_queries/package-info.java create mode 100644 plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/transport/package-info.java create mode 100644 plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/transport/top_queries/TransportTopQueriesAction.java create mode 100644 plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/transport/top_queries/package-info.java create mode 100644 plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/listener/SearchQueryLatencyListenerTests.java create mode 100644 plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/service/TopQueriesByLatencyServiceTests.java create mode 100644 plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesRequestTests.java create mode 100644 plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesResponseTests.java create mode 100644 plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesTests.java create mode 100644 plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/model/SearchQueryLatencyRecordTests.java create mode 100644 plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/resthandler/top_queries/RestTopQueriesActionTests.java create mode 100644 plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/transport/top_queries/TransportTopQueriesActionTests.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 5afdf6b707d1e..da341454fcb40 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -224,6 +224,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Workaround for https://bugs.openjdk.org/browse/JDK-8323659 regression, introduced in JDK-21.0.2 ([#11968](https://github.com/opensearch-project/OpenSearch/pull/11968)) - Updates IpField to be searchable when only `doc_values` are enabled ([#11508](https://github.com/opensearch-project/OpenSearch/pull/11508)) - [Query Insights] Query Insights Framework which currently supports retrieving the most time-consuming queries within the last configured time window ([#11903](https://github.com/opensearch-project/OpenSearch/pull/11903)) +- [Query Insights] Implement Top N Queries Feature in Query Insights Framework([#11904](https://github.com/opensearch-project/OpenSearch/pull/11904)) ### Deprecated diff --git a/plugins/query-insights/src/internalClusterTest/java/org/opensearch/plugin/insights/QueryInsightsPluginTransportIT.java b/plugins/query-insights/src/internalClusterTest/java/org/opensearch/plugin/insights/QueryInsightsPluginTransportIT.java new file mode 100644 index 0000000000000..f1d8594affca7 --- /dev/null +++ b/plugins/query-insights/src/internalClusterTest/java/org/opensearch/plugin/insights/QueryInsightsPluginTransportIT.java @@ -0,0 +1,218 @@ +/* + * 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; + +import org.opensearch.action.admin.cluster.health.ClusterHealthResponse; +import org.opensearch.action.admin.cluster.node.info.NodeInfo; +import org.opensearch.action.admin.cluster.node.info.NodesInfoRequest; +import org.opensearch.action.admin.cluster.node.info.NodesInfoResponse; +import org.opensearch.action.admin.cluster.node.info.PluginsAndModules; +import org.opensearch.action.index.IndexResponse; +import org.opensearch.action.search.SearchResponse; +import org.opensearch.common.settings.Settings; +import org.opensearch.index.query.QueryBuilders; +import org.opensearch.plugin.insights.rules.action.top_queries.TopQueriesAction; +import org.opensearch.plugin.insights.rules.action.top_queries.TopQueriesRequest; +import org.opensearch.plugin.insights.rules.action.top_queries.TopQueriesResponse; +import org.opensearch.plugins.Plugin; +import org.opensearch.plugins.PluginInfo; +import org.opensearch.test.OpenSearchIntegTestCase; +import org.junit.Assert; + +import java.util.Arrays; +import java.util.Collection; +import java.util.List; +import java.util.function.Function; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.TOP_N_LATENCY_QUERIES_ENABLED; +import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.TOP_N_LATENCY_QUERIES_SIZE; +import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.TOP_N_LATENCY_QUERIES_WINDOW_SIZE; +import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked; + +/** + * Transport Action tests for Query Insights Plugin + */ + +@OpenSearchIntegTestCase.ClusterScope(numDataNodes = 0, scope = OpenSearchIntegTestCase.Scope.TEST) +public class QueryInsightsPluginTransportIT extends OpenSearchIntegTestCase { + + private final int TOTAL_NUMBER_OF_NODES = 2; + private final int TOTAL_SEARCH_REQUESTS = 5; + + @Override + protected Collection> nodePlugins() { + return Arrays.asList(QueryInsightsPlugin.class); + } + + /** + * Test Query Insights Plugin is installed + */ + public void testQueryInsightPluginInstalled() { + NodesInfoRequest nodesInfoRequest = new NodesInfoRequest(); + nodesInfoRequest.addMetric(NodesInfoRequest.Metric.PLUGINS.metricName()); + NodesInfoResponse nodesInfoResponse = OpenSearchIntegTestCase.client().admin().cluster().nodesInfo(nodesInfoRequest).actionGet(); + List pluginInfos = nodesInfoResponse.getNodes() + .stream() + .flatMap( + (Function>) nodeInfo -> nodeInfo.getInfo(PluginsAndModules.class).getPluginInfos().stream() + ) + .collect(Collectors.toList()); + Assert.assertTrue( + pluginInfos.stream().anyMatch(pluginInfo -> pluginInfo.getName().equals("org.opensearch.plugin.insights.QueryInsightsPlugin")) + ); + } + + /** + * Test get top queries when feature disabled + */ + public void testGetTopQueriesWhenFeatureDisabled() { + TopQueriesRequest request = new TopQueriesRequest(); + TopQueriesResponse response = OpenSearchIntegTestCase.client().execute(TopQueriesAction.INSTANCE, request).actionGet(); + Assert.assertNotEquals(0, response.failures().size()); + Assert.assertEquals( + "Cannot get query data when query insight feature is not enabled.", + response.failures().get(0).getCause().getCause().getMessage() + ); + } + + /** + * Test get top queries when feature enabled + */ + public void testGetTopQueriesWhenFeatureEnabled() { + Settings commonSettings = Settings.builder() + .put(TOP_N_LATENCY_QUERIES_ENABLED.getKey(), "true") + .put(TOP_N_LATENCY_QUERIES_SIZE.getKey(), "100") + .put(TOP_N_LATENCY_QUERIES_WINDOW_SIZE.getKey(), "600s") + .build(); + + logger.info("--> starting 2 nodes for query insight testing"); + List nodes = internalCluster().startNodes(TOTAL_NUMBER_OF_NODES, Settings.builder().put(commonSettings).build()); + + logger.info("--> waiting for nodes to form a cluster"); + ClusterHealthResponse health = client().admin().cluster().prepareHealth().setWaitForNodes("2").execute().actionGet(); + assertFalse(health.isTimedOut()); + + assertAcked( + prepareCreate("test").setSettings(Settings.builder().put("index.number_of_shards", 2).put("index.number_of_replicas", 2)) + ); + ensureStableCluster(2); + logger.info("--> creating indices for query insight testing"); + for (int i = 0; i < 5; i++) { + IndexResponse response = client().prepareIndex("test_" + i).setId("" + i).setSource("field_" + i, "value_" + i).get(); + assertEquals("CREATED", response.status().toString()); + } + // making search requests to get top queries + for (int i = 0; i < TOTAL_SEARCH_REQUESTS; i++) { + SearchResponse searchResponse = internalCluster().client(randomFrom(nodes)) + .prepareSearch() + .setQuery(QueryBuilders.matchAllQuery()) + .get(); + assertEquals(searchResponse.getFailedShards(), 0); + } + + TopQueriesRequest request = new TopQueriesRequest(); + TopQueriesResponse response = OpenSearchIntegTestCase.client().execute(TopQueriesAction.INSTANCE, request).actionGet(); + Assert.assertEquals(0, response.failures().size()); + Assert.assertEquals(TOTAL_NUMBER_OF_NODES, response.getNodes().size()); + Assert.assertEquals(TOTAL_SEARCH_REQUESTS, response.getNodes().stream().mapToInt(o -> o.getLatencyRecords().size()).sum()); + + internalCluster().stopAllNodes(); + } + + /** + * Test get top queries with small top n size + */ + public void testGetTopQueriesWithSmallTopN() { + Settings commonSettings = Settings.builder() + .put(TOP_N_LATENCY_QUERIES_ENABLED.getKey(), "true") + .put(TOP_N_LATENCY_QUERIES_SIZE.getKey(), "1") + .put(TOP_N_LATENCY_QUERIES_WINDOW_SIZE.getKey(), "600s") + .build(); + + logger.info("--> starting 2 nodes for query insight testing"); + List nodes = internalCluster().startNodes(TOTAL_NUMBER_OF_NODES, Settings.builder().put(commonSettings).build()); + + logger.info("--> waiting for nodes to form a cluster"); + ClusterHealthResponse health = client().admin().cluster().prepareHealth().setWaitForNodes("2").execute().actionGet(); + assertFalse(health.isTimedOut()); + + assertAcked( + prepareCreate("test").setSettings(Settings.builder().put("index.number_of_shards", 2).put("index.number_of_replicas", 2)) + ); + ensureStableCluster(2); + logger.info("--> creating indices for query insight testing"); + for (int i = 0; i < 5; i++) { + IndexResponse response = client().prepareIndex("test_" + i).setId("" + i).setSource("field_" + i, "value_" + i).get(); + assertEquals("CREATED", response.status().toString()); + } + // making search requests to get top queries + for (int i = 0; i < TOTAL_SEARCH_REQUESTS; i++) { + SearchResponse searchResponse = internalCluster().client(randomFrom(nodes)) + .prepareSearch() + .setQuery(QueryBuilders.matchAllQuery()) + .get(); + assertEquals(searchResponse.getFailedShards(), 0); + } + + TopQueriesRequest request = new TopQueriesRequest(); + TopQueriesResponse response = OpenSearchIntegTestCase.client().execute(TopQueriesAction.INSTANCE, request).actionGet(); + Assert.assertEquals(0, response.failures().size()); + Assert.assertEquals(TOTAL_NUMBER_OF_NODES, response.getNodes().size()); + // TODO: this should be 1 after changing to cluster level top N. + Assert.assertEquals(2, response.getNodes().stream().mapToInt(o -> o.getLatencyRecords().size()).sum()); + + internalCluster().stopAllNodes(); + } + + /** + * Test get top queries with small window size + */ + public void testGetTopQueriesWithSmallWindowSize() { + Settings commonSettings = Settings.builder() + .put(TOP_N_LATENCY_QUERIES_ENABLED.getKey(), "true") + .put(TOP_N_LATENCY_QUERIES_SIZE.getKey(), "100") + .put(TOP_N_LATENCY_QUERIES_WINDOW_SIZE.getKey(), "0ms") + .build(); + + logger.info("--> starting 2 nodes for query insight testing"); + List nodes = internalCluster().startNodes(TOTAL_NUMBER_OF_NODES, Settings.builder().put(commonSettings).build()); + + logger.info("--> waiting for nodes to form a cluster"); + ClusterHealthResponse health = client().admin().cluster().prepareHealth().setWaitForNodes("2").execute().actionGet(); + assertFalse(health.isTimedOut()); + + assertAcked( + prepareCreate("test").setSettings(Settings.builder().put("index.number_of_shards", 2).put("index.number_of_replicas", 2)) + ); + ensureStableCluster(2); + logger.info("--> creating indices for query insight testing"); + for (int i = 0; i < 5; i++) { + IndexResponse response = client().prepareIndex("test_" + i).setId("" + i).setSource("field_" + i, "value_" + i).get(); + assertEquals("CREATED", response.status().toString()); + } + // making search requests to get top queries + for (int i = 0; i < TOTAL_SEARCH_REQUESTS; i++) { + SearchResponse searchResponse = internalCluster().client(randomFrom(nodes)) + .prepareSearch() + .setQuery(QueryBuilders.matchAllQuery()) + .get(); + assertEquals(searchResponse.getFailedShards(), 0); + } + + TopQueriesRequest request = new TopQueriesRequest(); + TopQueriesResponse response = OpenSearchIntegTestCase.client().execute(TopQueriesAction.INSTANCE, request).actionGet(); + Assert.assertEquals(0, response.failures().size()); + Assert.assertEquals(TOTAL_NUMBER_OF_NODES, response.getNodes().size()); + Assert.assertEquals(0, response.getNodes().stream().mapToInt(o -> o.getLatencyRecords().size()).sum()); + + internalCluster().stopAllNodes(); + } +} diff --git a/plugins/query-insights/src/javaRestTest/java/org/opensearch/plugin/insights/TopQueriesRestIT.java b/plugins/query-insights/src/javaRestTest/java/org/opensearch/plugin/insights/TopQueriesRestIT.java new file mode 100644 index 0000000000000..57dea6ad8d5ff --- /dev/null +++ b/plugins/query-insights/src/javaRestTest/java/org/opensearch/plugin/insights/TopQueriesRestIT.java @@ -0,0 +1,107 @@ +/* + * 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; + +import org.opensearch.client.Request; +import org.opensearch.client.Response; +import org.opensearch.common.xcontent.LoggingDeprecationHandler; +import org.opensearch.common.xcontent.json.JsonXContent; +import org.opensearch.core.xcontent.NamedXContentRegistry; +import org.opensearch.test.rest.OpenSearchRestTestCase; +import org.junit.Assert; + +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.util.List; +import java.util.Map; + +/** + * Rest Action tests for Query Insights + */ +public class TopQueriesRestIT extends OpenSearchRestTestCase { + + /** + * test Query Insights is installed + * @throws IOException IOException + */ + @SuppressWarnings("unchecked") + public void testQueryInsightsPluginInstalled() throws IOException { + Request request = new Request("GET", "/_cat/plugins?s=component&h=name,component,version,description&format=json"); + Response response = client().performRequest(request); + List pluginsList = JsonXContent.jsonXContent.createParser( + NamedXContentRegistry.EMPTY, + LoggingDeprecationHandler.INSTANCE, + response.getEntity().getContent() + ).list(); + Assert.assertTrue( + pluginsList.stream().map(o -> (Map) o).anyMatch(plugin -> plugin.get("component").equals("query-insights")) + ); + } + + /** + * test enabling top queries + * @throws IOException IOException + */ + public void testTopQueriesResponses() throws IOException { + // Enable Top N Queries feature + Request request = new Request("PUT", "/_cluster/settings"); + request.setJsonEntity(defaultTopQueriesSettings()); + Response response = client().performRequest(request); + + Assert.assertEquals(200, response.getStatusLine().getStatusCode()); + + // Create documents for search + request = new Request("POST", "/my-index-0/_doc"); + request.setJsonEntity(createDocumentsBody()); + response = client().performRequest(request); + + Assert.assertEquals(201, response.getStatusLine().getStatusCode()); + + // Do Search + request = new Request("GET", "/my-index-0/_search?size=20&pretty"); + request.setJsonEntity(searchBody()); + response = client().performRequest(request); + Assert.assertEquals(200, response.getStatusLine().getStatusCode()); + response = client().performRequest(request); + Assert.assertEquals(200, response.getStatusLine().getStatusCode()); + + // Get Top Queries + 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")); + Assert.assertEquals(2, top_requests.split("searchType", -1).length - 1); + } + + private String defaultTopQueriesSettings() { + return "{\n" + + " \"persistent\" : {\n" + + " \"search.top_n_queries.latency.enabled\" : \"true\",\n" + + " \"search.top_n_queries.latency.window_size\" : \"600s\",\n" + + " \"search.top_n_queries.latency.top_n_size\" : 5\n" + + " }\n" + + "}"; + } + + private String createDocumentsBody() { + return "{\n" + + " \"@timestamp\": \"2099-11-15T13:12:00\",\n" + + " \"message\": \"this is document 1\",\n" + + " \"user\": {\n" + + " \"id\": \"cyji\"\n" + + " }\n" + + "}"; + } + + private String searchBody() { + return "{}"; + } +} diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/listener/SearchQueryLatencyListener.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/listener/SearchQueryLatencyListener.java new file mode 100644 index 0000000000000..4a7a3ff9dc547 --- /dev/null +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/listener/SearchQueryLatencyListener.java @@ -0,0 +1,121 @@ +/* + * 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.listener; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.opensearch.action.search.SearchPhaseContext; +import org.opensearch.action.search.SearchRequest; +import org.opensearch.action.search.SearchRequestContext; +import org.opensearch.action.search.SearchRequestOperationsListener; +import org.opensearch.cluster.service.ClusterService; +import org.opensearch.common.inject.Inject; +import org.opensearch.core.xcontent.ToXContent; +import org.opensearch.plugin.insights.core.service.TopQueriesByLatencyService; + +import java.util.Collections; +import java.util.HashMap; +import java.util.Locale; + +import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.TOP_N_LATENCY_QUERIES_ENABLED; +import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.TOP_N_LATENCY_QUERIES_EXPORTER_ENABLED; +import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.TOP_N_LATENCY_QUERIES_EXPORTER_IDENTIFIER; +import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.TOP_N_LATENCY_QUERIES_EXPORTER_INTERVAL; +import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.TOP_N_LATENCY_QUERIES_EXPORTER_TYPE; +import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.TOP_N_LATENCY_QUERIES_SIZE; +import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.TOP_N_LATENCY_QUERIES_WINDOW_SIZE; + +/** + * The listener for top N queries by latency + * + * @opensearch.internal + */ +public final class SearchQueryLatencyListener extends SearchRequestOperationsListener { + private static final ToXContent.Params FORMAT_PARAMS = new ToXContent.MapParams(Collections.singletonMap("pretty", "false")); + + private static final Logger log = LogManager.getLogger(SearchQueryLatencyListener.class); + + private final TopQueriesByLatencyService topQueriesByLatencyService; + + @Inject + public SearchQueryLatencyListener(ClusterService clusterService, TopQueriesByLatencyService topQueriesByLatencyService) { + this.topQueriesByLatencyService = topQueriesByLatencyService; + clusterService.getClusterSettings().addSettingsUpdateConsumer(TOP_N_LATENCY_QUERIES_ENABLED, this::setEnabled); + clusterService.getClusterSettings() + .addSettingsUpdateConsumer( + TOP_N_LATENCY_QUERIES_SIZE, + this.topQueriesByLatencyService::setTopNSize, + this.topQueriesByLatencyService::validateTopNSize + ); + clusterService.getClusterSettings() + .addSettingsUpdateConsumer( + TOP_N_LATENCY_QUERIES_WINDOW_SIZE, + this.topQueriesByLatencyService::setWindowSize, + this.topQueriesByLatencyService::validateWindowSize + ); + clusterService.getClusterSettings() + .addSettingsUpdateConsumer(TOP_N_LATENCY_QUERIES_EXPORTER_TYPE, this.topQueriesByLatencyService::setExporterType); + clusterService.getClusterSettings() + .addSettingsUpdateConsumer( + TOP_N_LATENCY_QUERIES_EXPORTER_INTERVAL, + this.topQueriesByLatencyService::setExportInterval, + this.topQueriesByLatencyService::validateExportInterval + ); + clusterService.getClusterSettings() + .addSettingsUpdateConsumer(TOP_N_LATENCY_QUERIES_EXPORTER_IDENTIFIER, this.topQueriesByLatencyService::setExporterIdentifier); + clusterService.getClusterSettings() + .addSettingsUpdateConsumer(TOP_N_LATENCY_QUERIES_EXPORTER_ENABLED, this.topQueriesByLatencyService::setExporterEnabled); + + this.setEnabled(clusterService.getClusterSettings().get(TOP_N_LATENCY_QUERIES_ENABLED)); + this.topQueriesByLatencyService.setTopNSize(clusterService.getClusterSettings().get(TOP_N_LATENCY_QUERIES_SIZE)); + this.topQueriesByLatencyService.setWindowSize(clusterService.getClusterSettings().get(TOP_N_LATENCY_QUERIES_WINDOW_SIZE)); + } + + @Override + public void setEnabled(boolean enabled) { + super.setEnabled(enabled); + this.topQueriesByLatencyService.setEnableCollect(enabled); + } + + @Override + public boolean isEnabled() { + return super.isEnabled(); + } + + @Override + public void onPhaseStart(SearchPhaseContext context) {} + + @Override + public void onPhaseEnd(SearchPhaseContext context, SearchRequestContext searchRequestContext) {} + + @Override + public void onPhaseFailure(SearchPhaseContext context) {} + + @Override + public void onRequestStart(SearchRequestContext searchRequestContext) {} + + @Override + public void onRequestEnd(SearchPhaseContext context, SearchRequestContext searchRequestContext) { + SearchRequest request = context.getRequest(); + try { + topQueriesByLatencyService.ingestQueryData( + request.getOrCreateAbsoluteStartMillis(), + request.searchType(), + request.source().toString(FORMAT_PARAMS), + context.getNumShards(), + request.indices(), + new HashMap<>(), + searchRequestContext.phaseTookMap(), + System.nanoTime() - searchRequestContext.getAbsoluteStartNanos() + ); + } catch (Exception e) { + log.error(String.format(Locale.ROOT, "fail to ingest query insight data, error: %s", e)); + } + } +} diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/listener/package-info.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/listener/package-info.java new file mode 100644 index 0000000000000..3cb9cacf7fd1c --- /dev/null +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/listener/package-info.java @@ -0,0 +1,12 @@ +/* + * 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. + */ + +/** + * Listeners for Query Insights + */ +package org.opensearch.plugin.insights.core.listener; diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/TopQueriesByLatencyService.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/TopQueriesByLatencyService.java new file mode 100644 index 0000000000000..5914239b4054e --- /dev/null +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/TopQueriesByLatencyService.java @@ -0,0 +1,240 @@ +/* + * 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; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.opensearch.action.search.SearchType; +import org.opensearch.client.Client; +import org.opensearch.cluster.service.ClusterService; +import org.opensearch.common.inject.Inject; +import org.opensearch.common.unit.TimeValue; +import org.opensearch.plugin.insights.core.exporter.QueryInsightsExporter; +import org.opensearch.plugin.insights.core.exporter.QueryInsightsExporterType; +import org.opensearch.plugin.insights.core.exporter.QueryInsightsLocalIndexExporter; +import org.opensearch.plugin.insights.rules.model.SearchQueryLatencyRecord; +import org.opensearch.plugin.insights.settings.QueryInsightsSettings; +import org.opensearch.threadpool.ThreadPool; + +import java.util.Locale; +import java.util.Map; +import java.util.concurrent.PriorityBlockingQueue; + +import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.DEFAULT_LOCAL_INDEX_MAPPING; +import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.MIN_EXPORT_INTERVAL; + +/** + * Service responsible for gathering, analyzing, storing and exporting + * top N queries with high latency data for search queries + * + * @opensearch.internal + */ +public class TopQueriesByLatencyService extends QueryInsightsService< + SearchQueryLatencyRecord, + PriorityBlockingQueue, + QueryInsightsExporter> { + private static final Logger log = LogManager.getLogger(TopQueriesByLatencyService.class); + + private static final TimeValue delay = TimeValue.ZERO; + + private int topNSize = QueryInsightsSettings.DEFAULT_TOP_N_SIZE; + + private TimeValue windowSize = TimeValue.timeValueSeconds(QueryInsightsSettings.DEFAULT_WINDOW_SIZE); + + private final ClusterService clusterService; + private final Client client; + + @Inject + public TopQueriesByLatencyService(ThreadPool threadPool, ClusterService clusterService, Client client) { + super(threadPool, new PriorityBlockingQueue<>(), null); + this.clusterService = clusterService; + this.client = client; + } + + /** + * Ingest the query data into to the top N queries with latency store + * + * @param timestamp The timestamp of the query. + * @param searchType The manner at which the search operation is executed. see {@link SearchType} + * @param source The search source that was executed by the query. + * @param totalShards Total number of shards as part of the search query across all indices + * @param indices The indices involved in the search query + * @param propertyMap Extra attributes and information about a search query + * @param phaseLatencyMap Contains phase level latency information in a search query + * @param tookInNanos Total search request took time in nanoseconds + */ + public void ingestQueryData( + final Long timestamp, + final SearchType searchType, + final String source, + final int totalShards, + final String[] indices, + final Map propertyMap, + final Map phaseLatencyMap, + final Long tookInNanos + ) { + if (timestamp <= 0) { + log.error( + String.format( + Locale.ROOT, + "Invalid timestamp %s when ingesting query data to compute top n queries with latency", + timestamp + ) + ); + return; + } + if (totalShards <= 0) { + log.error( + String.format( + Locale.ROOT, + "Invalid totalShards %s when ingesting query data to compute top n queries with latency", + totalShards + ) + ); + return; + } + this.threadPool.schedule(() -> { + super.ingestQueryData( + new SearchQueryLatencyRecord(timestamp, searchType, source, totalShards, indices, propertyMap, phaseLatencyMap, tookInNanos) + ); + // remove top elements for fix sizing priority queue + if (this.store.size() > this.getTopNSize()) { + this.store.poll(); + } + }, delay, ThreadPool.Names.GENERIC); + + log.debug(String.format(Locale.ROOT, "successfully ingested: %s", this.store)); + } + + @Override + public void clearOutdatedData() { + store.removeIf(record -> record.getTimestamp() < System.currentTimeMillis() - windowSize.getMillis()); + } + + public void setTopNSize(int size) { + this.topNSize = size; + } + + public void validateTopNSize(int size) { + if (size > QueryInsightsSettings.MAX_N_SIZE) { + throw new IllegalArgumentException( + "Top N size setting [" + + QueryInsightsSettings.TOP_N_LATENCY_QUERIES_SIZE.getKey() + + "]" + + " should be smaller than max top N size [" + + QueryInsightsSettings.MAX_N_SIZE + + "was (" + + size + + " > " + + QueryInsightsSettings.MAX_N_SIZE + + ")" + ); + } + } + + public int getTopNSize() { + return this.topNSize; + } + + public void setWindowSize(TimeValue windowSize) { + this.windowSize = windowSize; + } + + public void validateWindowSize(TimeValue windowSize) { + if (windowSize.compareTo(QueryInsightsSettings.MAX_WINDOW_SIZE) > 0) { + throw new IllegalArgumentException( + "Window size setting [" + + QueryInsightsSettings.TOP_N_LATENCY_QUERIES_WINDOW_SIZE.getKey() + + "]" + + " should be smaller than max window size [" + + QueryInsightsSettings.MAX_WINDOW_SIZE + + "was (" + + windowSize + + " > " + + QueryInsightsSettings.MAX_WINDOW_SIZE + + ")" + ); + } + } + + public TimeValue getWindowSize() { + return this.windowSize; + } + + public void setExporterType(QueryInsightsExporterType type) { + resetExporter( + getEnableExport(), + type, + clusterService.getClusterSettings().get(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_EXPORTER_IDENTIFIER) + ); + } + + public void setExporterEnabled(boolean enabled) { + super.setEnableExport(enabled); + resetExporter( + enabled, + clusterService.getClusterSettings().get(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_EXPORTER_TYPE), + clusterService.getClusterSettings().get(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_EXPORTER_IDENTIFIER) + ); + } + + public void setExporterIdentifier(String identifier) { + resetExporter( + getEnableExport(), + clusterService.getClusterSettings().get(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_EXPORTER_TYPE), + identifier + ); + } + + public void setExportInterval(TimeValue interval) { + super.setExportInterval(interval); + resetExporter( + getEnableExport(), + clusterService.getClusterSettings().get(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_EXPORTER_TYPE), + clusterService.getClusterSettings().get(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_EXPORTER_IDENTIFIER) + ); + } + + public void validateExportInterval(TimeValue exportInterval) { + if (exportInterval.getSeconds() < MIN_EXPORT_INTERVAL.getSeconds()) { + throw new IllegalArgumentException( + "Export Interval setting [" + + QueryInsightsSettings.TOP_N_LATENCY_QUERIES_EXPORTER_INTERVAL.getKey() + + "]" + + " should not be smaller than minimal export interval size [" + + MIN_EXPORT_INTERVAL + + "]" + + "was (" + + exportInterval + + " < " + + MIN_EXPORT_INTERVAL + + ")" + ); + } + } + + public void resetExporter(boolean enabled, QueryInsightsExporterType type, String identifier) { + this.stop(); + this.exporter = null; + + if (!enabled) { + return; + } + if (type.equals(QueryInsightsExporterType.LOCAL_INDEX)) { + this.exporter = new QueryInsightsLocalIndexExporter<>( + clusterService, + client, + identifier, + TopQueriesByLatencyService.class.getClassLoader().getResourceAsStream(DEFAULT_LOCAL_INDEX_MAPPING) + ); + } + this.start(); + } + +} diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/package-info.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/package-info.java new file mode 100644 index 0000000000000..9b6b5856f7d27 --- /dev/null +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/package-info.java @@ -0,0 +1,12 @@ +/* + * 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. + */ + +/** + * Transport Actions, Requests and Responses for Query Insights + */ +package org.opensearch.plugin.insights.rules.action; diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueries.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueries.java new file mode 100644 index 0000000000000..1bb250e5d57b7 --- /dev/null +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueries.java @@ -0,0 +1,71 @@ +/* + * 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.rules.action.top_queries; + +import org.opensearch.action.support.nodes.BaseNodeResponse; +import org.opensearch.cluster.node.DiscoveryNode; +import org.opensearch.common.Nullable; +import org.opensearch.core.common.io.stream.StreamInput; +import org.opensearch.core.common.io.stream.StreamOutput; +import org.opensearch.core.xcontent.ToXContentObject; +import org.opensearch.core.xcontent.XContentBuilder; +import org.opensearch.plugin.insights.rules.model.SearchQueryLatencyRecord; + +import java.io.IOException; +import java.util.List; + +/** + * Top Queries by resource usage / latency on a node + *

+ * Mainly used in the top N queries node response workflow. + * + * @opensearch.internal + */ +public class TopQueries extends BaseNodeResponse implements ToXContentObject { + /** The store to keep the top N queries with latency records */ + @Nullable + private final List latencyRecords; + + public TopQueries(StreamInput in) throws IOException { + super(in); + latencyRecords = in.readList(SearchQueryLatencyRecord::new); + } + + public TopQueries(DiscoveryNode node, @Nullable List latencyRecords) { + super(node); + this.latencyRecords = latencyRecords; + } + + @Override + public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { + if (latencyRecords != null) { + for (SearchQueryLatencyRecord record : latencyRecords) { + record.toXContent(builder, params); + } + } + return builder; + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + super.writeTo(out); + if (latencyRecords != null) { + out.writeList(latencyRecords); + } + } + + /** + * Get all latency records + * + * @return the latency records in this node response + */ + public List getLatencyRecords() { + return latencyRecords; + } +} diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesAction.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesAction.java new file mode 100644 index 0000000000000..69abf001d18d9 --- /dev/null +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesAction.java @@ -0,0 +1,26 @@ +/* + * 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.rules.action.top_queries; + +import org.opensearch.action.ActionType; + +/** + * Transport action for cluster/node level top queries information. + * + * @opensearch.internal + */ +public class TopQueriesAction extends ActionType { + + public static final TopQueriesAction INSTANCE = new TopQueriesAction(); + public static final String NAME = "cluster:monitor/insights/top_queries"; + + private TopQueriesAction() { + super(NAME, TopQueriesResponse::new); + } +} diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesRequest.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesRequest.java new file mode 100644 index 0000000000000..435f5188a4e0b --- /dev/null +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesRequest.java @@ -0,0 +1,96 @@ +/* + * 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.rules.action.top_queries; + +import org.opensearch.action.support.nodes.BaseNodesRequest; +import org.opensearch.common.annotation.PublicApi; +import org.opensearch.core.common.io.stream.StreamInput; +import org.opensearch.core.common.io.stream.StreamOutput; + +import java.io.IOException; +import java.util.Arrays; +import java.util.Locale; +import java.util.Set; +import java.util.stream.Collectors; + +/** + * A request to get cluster/node level top queries information. + * + * @opensearch.internal + */ +@PublicApi(since = "1.0.0") +public class TopQueriesRequest extends BaseNodesRequest { + + Metric metricType = Metric.LATENCY; + + /** + * Create a new TopQueriesRequest from a {@link StreamInput} object. + * + * @param in A stream input object. + * @throws IOException if the stream cannot be deserialized. + */ + public TopQueriesRequest(StreamInput in) throws IOException { + super(in); + setMetricType(in.readString()); + } + + /** + * Get top queries from nodes based on the nodes ids specified. + * If none are passed, cluster level top queries will be returned. + */ + public TopQueriesRequest(String... nodesIds) { + super(nodesIds); + } + + /** + * Get the type of requested metrics + */ + public Metric getMetricType() { + return metricType; + } + + /** + * Set the type of requested metrics + */ + public TopQueriesRequest setMetricType(String metricType) { + metricType = metricType.toUpperCase(Locale.ROOT); + if (false == Metric.allMetrics().contains(metricType)) { + throw new IllegalStateException("Invalid metric used in top queries request: " + metricType); + } + this.metricType = Metric.valueOf(metricType); + return this; + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + super.writeTo(out); + out.writeString(metricType.metricName()); + } + + /** + * ALl supported metrics for top queries + */ + public enum Metric { + LATENCY("LATENCY"); + + private final String metricName; + + Metric(String name) { + this.metricName = name; + } + + public String metricName() { + return this.metricName; + } + + public static Set allMetrics() { + return Arrays.stream(values()).map(Metric::metricName).collect(Collectors.toSet()); + } + } +} diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesResponse.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesResponse.java new file mode 100644 index 0000000000000..37767dcee8fe5 --- /dev/null +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesResponse.java @@ -0,0 +1,120 @@ +/* + * 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.rules.action.top_queries; + +import org.opensearch.action.FailedNodeException; +import org.opensearch.action.support.nodes.BaseNodesResponse; +import org.opensearch.cluster.ClusterName; +import org.opensearch.common.annotation.PublicApi; +import org.opensearch.common.xcontent.XContentFactory; +import org.opensearch.core.common.io.stream.StreamInput; +import org.opensearch.core.common.io.stream.StreamOutput; +import org.opensearch.core.xcontent.ToXContentFragment; +import org.opensearch.core.xcontent.XContentBuilder; +import org.opensearch.plugin.insights.rules.model.SearchQueryLatencyRecord; + +import java.io.IOException; +import java.util.Collection; +import java.util.Collections; +import java.util.List; +import java.util.stream.Collectors; + +/** + * Transport response for cluster/node level top queries information. + * + * @opensearch.internal + */ +@PublicApi(since = "1.0.0") +public class TopQueriesResponse extends BaseNodesResponse implements ToXContentFragment { + + private static final String CLUSTER_LEVEL_RESULTS_KEY = "top_queries"; + private final int top_n_size; + + public TopQueriesResponse(StreamInput in) throws IOException { + super(in); + top_n_size = in.readInt(); + } + + public TopQueriesResponse(ClusterName clusterName, List nodes, List failures, int top_n_size) { + super(clusterName, nodes, failures); + this.top_n_size = top_n_size; + } + + @Override + protected List readNodesFrom(StreamInput in) throws IOException { + return in.readList(TopQueries::new); + } + + @Override + protected void writeNodesTo(StreamOutput out, List nodes) throws IOException { + out.writeList(nodes); + out.writeLong(top_n_size); + } + + @Override + public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { + List results = getNodes(); + builder.startObject(); + toClusterLevelResult(builder, params, results); + return builder.endObject(); + } + + @Override + public String toString() { + try { + XContentBuilder builder = XContentFactory.jsonBuilder().prettyPrint(); + builder.startObject(); + this.toXContent(builder, EMPTY_PARAMS); + builder.endObject(); + return builder.toString(); + } catch (IOException e) { + return "{ \"error\" : \"" + e.getMessage() + "\"}"; + } + } + + /** + * Merge top n queries results from nodes into cluster level results in XContent format. + * + * @param builder XContent builder + * @param params serialization parameters + * @param results top queries results from all nodes + * @throws IOException if an error occurs + */ + private void toClusterLevelResult(XContentBuilder builder, Params params, List results) throws IOException { + List all_records = results.stream() + .map(TopQueries::getLatencyRecords) + .flatMap(Collection::stream) + .sorted(Collections.reverseOrder()) + .limit(top_n_size) + .collect(Collectors.toList()); + builder.startArray(CLUSTER_LEVEL_RESULTS_KEY); + for (SearchQueryLatencyRecord record : all_records) { + record.toXContent(builder, params); + } + builder.endArray(); + } + + /** + * build node level top n queries results in XContent format. + * + * @param builder XContent builder + * @param params serialization parameters + * @param results top queries results from all nodes + * @throws IOException if an error occurs + */ + private void toNodeLevelResult(XContentBuilder builder, Params params, List results) throws IOException { + builder.startObject(CLUSTER_LEVEL_RESULTS_KEY); + for (TopQueries topQueries : results) { + builder.startArray(topQueries.getNode().getId()); + topQueries.toXContent(builder, params); + builder.endArray(); + } + builder.endObject(); + } +} diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/package-info.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/package-info.java new file mode 100644 index 0000000000000..3cc7900e5ce7d --- /dev/null +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/package-info.java @@ -0,0 +1,12 @@ +/* + * 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. + */ + +/** + * Transport Actions, Requests and Responses for Top N Queries + */ +package org.opensearch.plugin.insights.rules.action.top_queries; diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/resthandler/package-info.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/resthandler/package-info.java new file mode 100644 index 0000000000000..3787f05f65552 --- /dev/null +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/resthandler/package-info.java @@ -0,0 +1,12 @@ +/* + * 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. + */ + +/** + * Rest Handlers for Query Insights + */ +package org.opensearch.plugin.insights.rules.resthandler; diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/resthandler/top_queries/RestTopQueriesAction.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/resthandler/top_queries/RestTopQueriesAction.java new file mode 100644 index 0000000000000..874df86ebb4fc --- /dev/null +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/resthandler/top_queries/RestTopQueriesAction.java @@ -0,0 +1,102 @@ +/* + * 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.rules.resthandler.top_queries; + +import org.opensearch.client.node.NodeClient; +import org.opensearch.common.settings.Settings; +import org.opensearch.core.common.Strings; +import org.opensearch.core.rest.RestStatus; +import org.opensearch.core.xcontent.ToXContent; +import org.opensearch.plugin.insights.rules.action.top_queries.TopQueriesAction; +import org.opensearch.plugin.insights.rules.action.top_queries.TopQueriesRequest; +import org.opensearch.plugin.insights.rules.action.top_queries.TopQueriesResponse; +import org.opensearch.rest.BaseRestHandler; +import org.opensearch.rest.BytesRestResponse; +import org.opensearch.rest.RestChannel; +import org.opensearch.rest.RestRequest; +import org.opensearch.rest.RestResponse; +import org.opensearch.rest.action.RestResponseListener; + +import java.io.IOException; +import java.util.List; +import java.util.Locale; +import java.util.Set; + +import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.TOP_QUERIES_BASE_URI; +import static org.opensearch.rest.RestRequest.Method.GET; + +/** + * Transport action to get Top N queries by certain metric type + * + * @opensearch.api + */ +public class RestTopQueriesAction extends BaseRestHandler { + /** The metric types that are allowed in top N queries */ + static final Set ALLOWED_METRICS = TopQueriesRequest.Metric.allMetrics(); + + public RestTopQueriesAction() {} + + @Override + public List routes() { + return List.of( + new Route(GET, TOP_QUERIES_BASE_URI), + new Route(GET, String.format(Locale.ROOT, "%s/{nodeId}", TOP_QUERIES_BASE_URI)) + ); + } + + @Override + public String getName() { + return "top_queries_action"; + } + + @Override + public RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) throws IOException { + final TopQueriesRequest topQueriesRequest = prepareRequest(request); + topQueriesRequest.timeout(request.param("timeout")); + + return channel -> client.execute( + TopQueriesAction.INSTANCE, + topQueriesRequest, + topQueriesResponse(channel, request.method()) + + ); + } + + static TopQueriesRequest prepareRequest(final RestRequest request) { + String[] nodesIds = Strings.splitStringByCommaToArray(request.param("nodeId")); + String metricType = request.param("type", TopQueriesRequest.Metric.LATENCY.metricName()).toUpperCase(Locale.ROOT); + if (!ALLOWED_METRICS.contains(metricType)) { + throw new IllegalArgumentException( + String.format(Locale.ROOT, "request [%s] contains invalid metric type [%s]", request.path(), metricType) + ); + } + TopQueriesRequest topQueriesRequest = new TopQueriesRequest(nodesIds); + topQueriesRequest.setMetricType(metricType); + return topQueriesRequest; + } + + @Override + protected Set responseParams() { + return Settings.FORMAT_PARAMS; + } + + @Override + public boolean canTripCircuitBreaker() { + return false; + } + + private RestResponseListener topQueriesResponse(RestChannel channel, RestRequest.Method restMethod) { + return new RestResponseListener<>(channel) { + @Override + public RestResponse buildResponse(TopQueriesResponse response) throws Exception { + return new BytesRestResponse(RestStatus.OK, response.toXContent(channel.newBuilder(), ToXContent.EMPTY_PARAMS)); + } + }; + } +} diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/resthandler/top_queries/package-info.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/resthandler/top_queries/package-info.java new file mode 100644 index 0000000000000..087cf7d765f8c --- /dev/null +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/resthandler/top_queries/package-info.java @@ -0,0 +1,12 @@ +/* + * 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. + */ + +/** + * Rest Handlers for Top N Queries + */ +package org.opensearch.plugin.insights.rules.resthandler.top_queries; diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/transport/package-info.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/transport/package-info.java new file mode 100644 index 0000000000000..f3a1c70b9af57 --- /dev/null +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/transport/package-info.java @@ -0,0 +1,12 @@ +/* + * 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. + */ + +/** + * Transport Actions for Query Insights. + */ +package org.opensearch.plugin.insights.rules.transport; diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/transport/top_queries/TransportTopQueriesAction.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/transport/top_queries/TransportTopQueriesAction.java new file mode 100644 index 0000000000000..fa7996cc6f3f7 --- /dev/null +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/transport/top_queries/TransportTopQueriesAction.java @@ -0,0 +1,131 @@ +/* + * 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.rules.transport.top_queries; + +import org.opensearch.OpenSearchException; +import org.opensearch.action.FailedNodeException; +import org.opensearch.action.support.ActionFilters; +import org.opensearch.action.support.nodes.TransportNodesAction; +import org.opensearch.cluster.service.ClusterService; +import org.opensearch.common.inject.Inject; +import org.opensearch.core.common.io.stream.StreamInput; +import org.opensearch.core.common.io.stream.StreamOutput; +import org.opensearch.plugin.insights.core.service.TopQueriesByLatencyService; +import org.opensearch.plugin.insights.rules.action.top_queries.TopQueries; +import org.opensearch.plugin.insights.rules.action.top_queries.TopQueriesAction; +import org.opensearch.plugin.insights.rules.action.top_queries.TopQueriesRequest; +import org.opensearch.plugin.insights.rules.action.top_queries.TopQueriesResponse; +import org.opensearch.plugin.insights.settings.QueryInsightsSettings; +import org.opensearch.threadpool.ThreadPool; +import org.opensearch.transport.TransportRequest; +import org.opensearch.transport.TransportService; + +import java.io.IOException; +import java.util.List; +import java.util.Locale; + +/** + * Transport action for cluster/node level top queries information. + * + * @opensearch.internal + */ +public class TransportTopQueriesAction extends TransportNodesAction< + TopQueriesRequest, + TopQueriesResponse, + TransportTopQueriesAction.NodeRequest, + TopQueries> { + + private final TopQueriesByLatencyService topQueriesByLatencyService; + + @Inject + public TransportTopQueriesAction( + ThreadPool threadPool, + ClusterService clusterService, + TransportService transportService, + TopQueriesByLatencyService topQueriesByLatencyService, + ActionFilters actionFilters + ) { + super( + TopQueriesAction.NAME, + threadPool, + clusterService, + transportService, + actionFilters, + TopQueriesRequest::new, + NodeRequest::new, + ThreadPool.Names.GENERIC, + TopQueries.class + ); + this.topQueriesByLatencyService = topQueriesByLatencyService; + } + + @Override + protected TopQueriesResponse newResponse( + TopQueriesRequest topQueriesRequest, + List responses, + List failures + ) { + if (topQueriesRequest.getMetricType() == TopQueriesRequest.Metric.LATENCY) { + return new TopQueriesResponse( + clusterService.getClusterName(), + responses, + failures, + clusterService.getClusterSettings().get(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_SIZE) + ); + } else { + throw new OpenSearchException(String.format(Locale.ROOT, "invalid metric type %s", topQueriesRequest.getMetricType())); + } + } + + @Override + protected NodeRequest newNodeRequest(TopQueriesRequest request) { + return new NodeRequest(request); + } + + @Override + protected TopQueries newNodeResponse(StreamInput in) throws IOException { + return new TopQueries(in); + } + + @Override + protected TopQueries nodeOperation(NodeRequest nodeRequest) { + TopQueriesRequest request = nodeRequest.request; + if (request.getMetricType() == TopQueriesRequest.Metric.LATENCY) { + return new TopQueries(clusterService.localNode(), topQueriesByLatencyService.getQueryData()); + } else { + throw new OpenSearchException(String.format(Locale.ROOT, "invalid metric type %s", request.getMetricType())); + } + + } + + /** + * Inner Node Top Queries Request + * + * @opensearch.internal + */ + public static class NodeRequest extends TransportRequest { + + TopQueriesRequest request; + + public NodeRequest(StreamInput in) throws IOException { + super(in); + request = new TopQueriesRequest(in); + } + + public NodeRequest(TopQueriesRequest request) { + this.request = request; + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + super.writeTo(out); + request.writeTo(out); + } + } +} diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/transport/top_queries/package-info.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/transport/top_queries/package-info.java new file mode 100644 index 0000000000000..54da0980deff8 --- /dev/null +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/transport/top_queries/package-info.java @@ -0,0 +1,12 @@ +/* + * 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. + */ + +/** + * Transport Actions for Top N Queries. + */ +package org.opensearch.plugin.insights.rules.transport.top_queries; diff --git a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/listener/SearchQueryLatencyListenerTests.java b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/listener/SearchQueryLatencyListenerTests.java new file mode 100644 index 0000000000000..5228e0054c440 --- /dev/null +++ b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/listener/SearchQueryLatencyListenerTests.java @@ -0,0 +1,182 @@ +/* + * 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.listener; + +import org.opensearch.action.search.SearchPhaseContext; +import org.opensearch.action.search.SearchRequest; +import org.opensearch.action.search.SearchRequestContext; +import org.opensearch.action.search.SearchType; +import org.opensearch.cluster.service.ClusterService; +import org.opensearch.common.settings.ClusterSettings; +import org.opensearch.common.settings.Settings; +import org.opensearch.plugin.insights.core.service.TopQueriesByLatencyService; +import org.opensearch.plugin.insights.settings.QueryInsightsSettings; +import org.opensearch.search.aggregations.bucket.terms.TermsAggregationBuilder; +import org.opensearch.search.aggregations.support.ValueType; +import org.opensearch.search.builder.SearchSourceBuilder; +import org.opensearch.test.OpenSearchTestCase; + +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.Phaser; + +import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.Mockito.anyMap; +import static org.mockito.Mockito.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +/** + * Unit Tests for {@link SearchQueryLatencyListener}. + */ +public class SearchQueryLatencyListenerTests extends OpenSearchTestCase { + + public void testOnRequestEnd() { + final SearchRequestContext searchRequestContext = mock(SearchRequestContext.class); + final SearchPhaseContext searchPhaseContext = mock(SearchPhaseContext.class); + final SearchRequest searchRequest = mock(SearchRequest.class); + final TopQueriesByLatencyService topQueriesByLatencyService = mock(TopQueriesByLatencyService.class); + + Settings.Builder settingsBuilder = Settings.builder(); + Settings settings = settingsBuilder.build(); + ClusterSettings clusterSettings = new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); + clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_ENABLED); + clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_SIZE); + clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_WINDOW_SIZE); + clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_EXPORTER_ENABLED); + clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_EXPORTER_TYPE); + clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_EXPORTER_INTERVAL); + clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_EXPORTER_IDENTIFIER); + + ClusterService clusterService = new ClusterService(settings, clusterSettings, null); + + Long timestamp = System.currentTimeMillis() - 100L; + SearchType searchType = SearchType.QUERY_THEN_FETCH; + + SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder(); + searchSourceBuilder.aggregation(new TermsAggregationBuilder("agg1").userValueTypeHint(ValueType.STRING).field("type.keyword")); + searchSourceBuilder.size(0); + + String[] indices = new String[] { "index-1", "index-2" }; + + Map phaseLatencyMap = new HashMap<>(); + phaseLatencyMap.put("expand", 0L); + phaseLatencyMap.put("query", 20L); + phaseLatencyMap.put("fetch", 1L); + + int numberOfShards = 10; + + SearchQueryLatencyListener searchQueryLatencyListener = new SearchQueryLatencyListener(clusterService, topQueriesByLatencyService); + + when(searchRequest.getOrCreateAbsoluteStartMillis()).thenReturn(timestamp); + when(searchRequest.searchType()).thenReturn(searchType); + when(searchRequest.source()).thenReturn(searchSourceBuilder); + when(searchRequest.indices()).thenReturn(indices); + when(searchRequestContext.phaseTookMap()).thenReturn(phaseLatencyMap); + when(searchPhaseContext.getRequest()).thenReturn(searchRequest); + when(searchPhaseContext.getNumShards()).thenReturn(numberOfShards); + + searchQueryLatencyListener.onRequestEnd(searchPhaseContext, searchRequestContext); + + verify(topQueriesByLatencyService, times(1)).ingestQueryData( + eq(timestamp), + eq(searchType), + eq(searchSourceBuilder.toString()), + eq(numberOfShards), + eq(indices), + anyMap(), + eq(phaseLatencyMap), + anyLong() + ); + } + + public void testConcurrentOnRequestEnd() throws InterruptedException { + final SearchRequestContext searchRequestContext = mock(SearchRequestContext.class); + final SearchPhaseContext searchPhaseContext = mock(SearchPhaseContext.class); + final SearchRequest searchRequest = mock(SearchRequest.class); + final TopQueriesByLatencyService topQueriesByLatencyService = mock(TopQueriesByLatencyService.class); + + Settings.Builder settingsBuilder = Settings.builder(); + Settings settings = settingsBuilder.build(); + ClusterSettings clusterSettings = new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); + clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_ENABLED); + clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_SIZE); + clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_WINDOW_SIZE); + clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_EXPORTER_ENABLED); + clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_EXPORTER_TYPE); + clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_EXPORTER_INTERVAL); + clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_EXPORTER_IDENTIFIER); + + ClusterService clusterService = new ClusterService(settings, clusterSettings, null); + + Long timestamp = System.currentTimeMillis() - 100L; + SearchType searchType = SearchType.QUERY_THEN_FETCH; + + SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder(); + searchSourceBuilder.aggregation(new TermsAggregationBuilder("agg1").userValueTypeHint(ValueType.STRING).field("type.keyword")); + searchSourceBuilder.size(0); + + String[] indices = new String[] { "index-1", "index-2" }; + + Map phaseLatencyMap = new HashMap<>(); + phaseLatencyMap.put("expand", 0L); + phaseLatencyMap.put("query", 20L); + phaseLatencyMap.put("fetch", 1L); + + int numberOfShards = 10; + + final List searchListenersList = new ArrayList<>(); + + when(searchRequest.getOrCreateAbsoluteStartMillis()).thenReturn(timestamp); + when(searchRequest.searchType()).thenReturn(searchType); + when(searchRequest.source()).thenReturn(searchSourceBuilder); + when(searchRequest.indices()).thenReturn(indices); + when(searchRequestContext.phaseTookMap()).thenReturn(phaseLatencyMap); + when(searchPhaseContext.getRequest()).thenReturn(searchRequest); + when(searchPhaseContext.getNumShards()).thenReturn(numberOfShards); + + int numRequests = 50; + Thread[] threads = new Thread[numRequests]; + Phaser phaser = new Phaser(numRequests + 1); + CountDownLatch countDownLatch = new CountDownLatch(numRequests); + + for (int i = 0; i < numRequests; i++) { + searchListenersList.add(new SearchQueryLatencyListener(clusterService, topQueriesByLatencyService)); + } + + for (int i = 0; i < numRequests; i++) { + int finalI = i; + threads[i] = new Thread(() -> { + phaser.arriveAndAwaitAdvance(); + SearchQueryLatencyListener thisListener = searchListenersList.get(finalI); + thisListener.onRequestEnd(searchPhaseContext, searchRequestContext); + countDownLatch.countDown(); + }); + threads[i].start(); + } + phaser.arriveAndAwaitAdvance(); + countDownLatch.await(); + + verify(topQueriesByLatencyService, times(numRequests)).ingestQueryData( + eq(timestamp), + eq(searchType), + eq(searchSourceBuilder.toString()), + eq(numberOfShards), + eq(indices), + anyMap(), + eq(phaseLatencyMap), + anyLong() + ); + } +} diff --git a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/service/TopQueriesByLatencyServiceTests.java b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/service/TopQueriesByLatencyServiceTests.java new file mode 100644 index 0000000000000..4ccb04e618f1a --- /dev/null +++ b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/service/TopQueriesByLatencyServiceTests.java @@ -0,0 +1,259 @@ +/* + * 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; + +import org.opensearch.client.Client; +import org.opensearch.cluster.coordination.DeterministicTaskQueue; +import org.opensearch.cluster.service.ClusterService; +import org.opensearch.common.settings.ClusterSettings; +import org.opensearch.common.settings.Settings; +import org.opensearch.common.unit.TimeValue; +import org.opensearch.node.Node; +import org.opensearch.plugin.insights.QueryInsightsTestUtils; +import org.opensearch.plugin.insights.core.exporter.QueryInsightsExporterType; +import org.opensearch.plugin.insights.core.exporter.QueryInsightsLocalIndexExporter; +import org.opensearch.plugin.insights.rules.model.SearchQueryLatencyRecord; +import org.opensearch.plugin.insights.settings.QueryInsightsSettings; +import org.opensearch.test.OpenSearchTestCase; +import org.opensearch.threadpool.ThreadPool; +import org.junit.After; +import org.junit.Before; + +import java.util.List; +import java.util.concurrent.TimeUnit; + +import static org.mockito.Mockito.mock; + +/** + * Unit Tests for {@link TopQueriesByLatencyService}. + */ +public class TopQueriesByLatencyServiceTests extends OpenSearchTestCase { + + private DeterministicTaskQueue deterministicTaskQueue; + private ThreadPool threadPool; + private TopQueriesByLatencyService topQueriesByLatencyService; + + @Before + public void setup() { + final Client client = mock(Client.class); + final Settings settings = Settings.builder().put(Node.NODE_NAME_SETTING.getKey(), "top n queries tests").build(); + deterministicTaskQueue = new DeterministicTaskQueue(settings, random()); + threadPool = deterministicTaskQueue.getThreadPool(); + ClusterSettings clusterSettings = new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); + clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_ENABLED); + clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_SIZE); + clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_WINDOW_SIZE); + clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_EXPORTER_ENABLED); + clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_EXPORTER_TYPE); + clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_EXPORTER_INTERVAL); + clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_EXPORTER_IDENTIFIER); + ClusterService clusterService = new ClusterService(settings, clusterSettings, threadPool); + topQueriesByLatencyService = new TopQueriesByLatencyService(threadPool, clusterService, client); + topQueriesByLatencyService.setEnableCollect(true); + topQueriesByLatencyService.setTopNSize(Integer.MAX_VALUE); + topQueriesByLatencyService.setWindowSize(new TimeValue(Long.MAX_VALUE)); + } + + @After + public void shutdown() { + topQueriesByLatencyService.stop(); + } + + public void testIngestQueryDataWithLargeWindow() { + final List records = QueryInsightsTestUtils.generateQueryInsightRecords(10); + for (SearchQueryLatencyRecord record : records) { + topQueriesByLatencyService.ingestQueryData( + record.getTimestamp(), + record.getSearchType(), + record.getSource(), + record.getTotalShards(), + record.getIndices(), + record.getPropertyMap(), + record.getPhaseLatencyMap(), + record.getValue() + ); + } + runUntilTimeoutOrFinish(deterministicTaskQueue, 5000); + assertTrue(QueryInsightsTestUtils.checkRecordsEqualsWithoutOrder(topQueriesByLatencyService.getQueryData(), records)); + } + + public void testConcurrentIngestQueryDataWithLargeWindow() { + final List records = QueryInsightsTestUtils.generateQueryInsightRecords(10); + + int numRequests = records.size(); + for (int i = 0; i < numRequests; i++) { + int finalI = i; + threadPool.schedule(() -> { + topQueriesByLatencyService.ingestQueryData( + records.get(finalI).getTimestamp(), + records.get(finalI).getSearchType(), + records.get(finalI).getSource(), + records.get(finalI).getTotalShards(), + records.get(finalI).getIndices(), + records.get(finalI).getPropertyMap(), + records.get(finalI).getPhaseLatencyMap(), + records.get(finalI).getValue() + ); + }, TimeValue.ZERO, ThreadPool.Names.GENERIC); + } + runUntilTimeoutOrFinish(deterministicTaskQueue, 5000); + assertTrue(QueryInsightsTestUtils.checkRecordsEqualsWithoutOrder(topQueriesByLatencyService.getQueryData(), records)); + } + + public void testSmallWindowClearOutdatedData() { + final List records = QueryInsightsTestUtils.generateQueryInsightRecords(10); + topQueriesByLatencyService.setWindowSize(new TimeValue(-1)); + + for (SearchQueryLatencyRecord record : records) { + topQueriesByLatencyService.ingestQueryData( + record.getTimestamp(), + record.getSearchType(), + record.getSource(), + record.getTotalShards(), + record.getIndices(), + record.getPropertyMap(), + record.getPhaseLatencyMap(), + record.getValue() + ); + } + runUntilTimeoutOrFinish(deterministicTaskQueue, 5000); + assertEquals(0, topQueriesByLatencyService.getQueryData().size()); + } + + public void testSmallNSize() { + final List records = QueryInsightsTestUtils.generateQueryInsightRecords(10); + topQueriesByLatencyService.setTopNSize(1); + + for (SearchQueryLatencyRecord record : records) { + topQueriesByLatencyService.ingestQueryData( + record.getTimestamp(), + record.getSearchType(), + record.getSource(), + record.getTotalShards(), + record.getIndices(), + record.getPropertyMap(), + record.getPhaseLatencyMap(), + record.getValue() + ); + } + runUntilTimeoutOrFinish(deterministicTaskQueue, 5000); + assertEquals(1, topQueriesByLatencyService.getQueryData().size()); + } + + public void testIngestQueryDataWithInvalidData() { + final SearchQueryLatencyRecord record = QueryInsightsTestUtils.generateQueryInsightRecords(1).get(0); + topQueriesByLatencyService.ingestQueryData( + -1L, + record.getSearchType(), + record.getSource(), + record.getTotalShards(), + record.getIndices(), + record.getPropertyMap(), + record.getPhaseLatencyMap(), + record.getValue() + ); + assertEquals(0, topQueriesByLatencyService.getQueryData().size()); + + topQueriesByLatencyService.ingestQueryData( + record.getTimestamp(), + record.getSearchType(), + record.getSource(), + -1, + record.getIndices(), + record.getPropertyMap(), + record.getPhaseLatencyMap(), + record.getValue() + ); + assertEquals(0, topQueriesByLatencyService.getQueryData().size()); + + } + + public void testValidateTopNSize() { + assertThrows( + IllegalArgumentException.class, + () -> { topQueriesByLatencyService.validateTopNSize(QueryInsightsSettings.MAX_N_SIZE + 1); } + ); + } + + public void testValidateWindowSize() { + assertThrows(IllegalArgumentException.class, () -> { + topQueriesByLatencyService.validateWindowSize( + new TimeValue(QueryInsightsSettings.MAX_WINDOW_SIZE.getSeconds() + 1, TimeUnit.SECONDS) + ); + }); + } + + public void testValidateInterval() { + assertThrows(IllegalArgumentException.class, () -> { + topQueriesByLatencyService.validateExportInterval( + new TimeValue(QueryInsightsSettings.MIN_EXPORT_INTERVAL.getSeconds() - 1, TimeUnit.SECONDS) + ); + }); + } + + public void testSetExporterTypeWhenDisabled() { + topQueriesByLatencyService.setExporterEnabled(false); + topQueriesByLatencyService.setExporterType(QueryInsightsExporterType.LOCAL_INDEX); + assertNull(topQueriesByLatencyService.exporter); + } + + public void testSetExporterTypeWhenEnabled() { + topQueriesByLatencyService.setExporterEnabled(true); + topQueriesByLatencyService.setExporterType(QueryInsightsExporterType.LOCAL_INDEX); + assertEquals(QueryInsightsLocalIndexExporter.class, topQueriesByLatencyService.exporter.getClass()); + } + + public void testSetExporterEnabled() { + topQueriesByLatencyService.setExporterEnabled(true); + assertEquals(QueryInsightsLocalIndexExporter.class, topQueriesByLatencyService.exporter.getClass()); + } + + public void testSetExporterDisabled() { + topQueriesByLatencyService.setExporterEnabled(false); + assertNull(topQueriesByLatencyService.exporter); + } + + public void testChangeIdentifierWhenEnabled() { + topQueriesByLatencyService.setExporterEnabled(true); + topQueriesByLatencyService.setExporterIdentifier("changed"); + assertEquals("changed", topQueriesByLatencyService.exporter.getIdentifier()); + } + + public void testChangeIdentifierWhenDisabled() { + topQueriesByLatencyService.setExporterEnabled(false); + topQueriesByLatencyService.setExporterIdentifier("changed"); + assertNull(topQueriesByLatencyService.exporter); + } + + public void testChangeIntervalWhenEnabled() { + topQueriesByLatencyService.setExporterEnabled(true); + TimeValue newInterval = TimeValue.timeValueMillis(randomLongBetween(1, 9999)); + topQueriesByLatencyService.setExportInterval(newInterval); + assertEquals(newInterval, topQueriesByLatencyService.getExportInterval()); + } + + public void testChangeIntervalWhenDisabled() { + topQueriesByLatencyService.setExporterEnabled(false); + TimeValue newInterval = TimeValue.timeValueMillis(randomLongBetween(1, 9999)); + topQueriesByLatencyService.setExportInterval(newInterval); + assertNull(topQueriesByLatencyService.exporter); + } + + private static void runUntilTimeoutOrFinish(DeterministicTaskQueue deterministicTaskQueue, long duration) { + final long endTime = deterministicTaskQueue.getCurrentTimeMillis() + duration; + while (deterministicTaskQueue.getCurrentTimeMillis() < endTime + && (deterministicTaskQueue.hasRunnableTasks() || deterministicTaskQueue.hasDeferredTasks())) { + if (deterministicTaskQueue.hasDeferredTasks() && randomBoolean()) { + deterministicTaskQueue.advanceTime(); + } else if (deterministicTaskQueue.hasRunnableTasks()) { + deterministicTaskQueue.runRandomTask(); + } + } + } +} diff --git a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesRequestTests.java b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesRequestTests.java new file mode 100644 index 0000000000000..07e98970fb628 --- /dev/null +++ b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesRequestTests.java @@ -0,0 +1,43 @@ +/* + * 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.rules.action.top_queries; + +import org.opensearch.common.io.stream.BytesStreamOutput; +import org.opensearch.core.common.io.stream.StreamInput; +import org.opensearch.test.OpenSearchTestCase; + +/** + * Granular tests for the {@link TopQueriesRequest} class. + */ +public class TopQueriesRequestTests extends OpenSearchTestCase { + + /** + * Check that we can set the metric type + */ + public void testSetMetricType() throws Exception { + TopQueriesRequest request = new TopQueriesRequest(randomAlphaOfLength(5)); + request.setMetricType(randomFrom(TopQueriesRequest.Metric.allMetrics())); + TopQueriesRequest deserializedRequest = roundTripRequest(request); + assertEquals(request.getMetricType(), deserializedRequest.getMetricType()); + } + + /** + * Serialize and deserialize a request. + * @param request A request to serialize. + * @return The deserialized, "round-tripped" request. + */ + private static TopQueriesRequest roundTripRequest(TopQueriesRequest request) throws Exception { + try (BytesStreamOutput out = new BytesStreamOutput()) { + request.writeTo(out); + try (StreamInput in = out.bytes().streamInput()) { + return new TopQueriesRequest(in); + } + } + } +} diff --git a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesResponseTests.java b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesResponseTests.java new file mode 100644 index 0000000000000..11063cbedd248 --- /dev/null +++ b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesResponseTests.java @@ -0,0 +1,49 @@ +/* + * 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.rules.action.top_queries; + +import org.opensearch.cluster.ClusterName; +import org.opensearch.common.io.stream.BytesStreamOutput; +import org.opensearch.core.common.io.stream.StreamInput; +import org.opensearch.plugin.insights.QueryInsightsTestUtils; +import org.opensearch.test.OpenSearchTestCase; + +import java.util.ArrayList; +import java.util.List; + +/** + * Granular tests for the {@link TopQueriesResponse} class. + */ +public class TopQueriesResponseTests extends OpenSearchTestCase { + + /** + * Check serialization and deserialization + */ + public void testToXContent() throws Exception { + TopQueries topQueries = QueryInsightsTestUtils.createTopQueries(); + ClusterName clusterName = new ClusterName("test-cluster"); + TopQueriesResponse response = new TopQueriesResponse(clusterName, List.of(topQueries), new ArrayList<>(), 10); + TopQueriesResponse deserializedResponse = roundTripResponse(response); + assertEquals(response.toString(), deserializedResponse.toString()); + } + + /** + * Serialize and deserialize a TopQueriesResponse. + * @param response A response to serialize. + * @return The deserialized, "round-tripped" response. + */ + private static TopQueriesResponse roundTripResponse(TopQueriesResponse response) throws Exception { + try (BytesStreamOutput out = new BytesStreamOutput()) { + response.writeTo(out); + try (StreamInput in = out.bytes().streamInput()) { + return new TopQueriesResponse(in); + } + } + } +} diff --git a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesTests.java b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesTests.java new file mode 100644 index 0000000000000..4d0d641cf1a8d --- /dev/null +++ b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesTests.java @@ -0,0 +1,42 @@ +/* + * 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.rules.action.top_queries; + +import org.opensearch.common.io.stream.BytesStreamOutput; +import org.opensearch.core.common.io.stream.StreamInput; +import org.opensearch.plugin.insights.QueryInsightsTestUtils; +import org.opensearch.test.OpenSearchTestCase; + +import java.io.IOException; + +/** + * Tests for {@link TopQueries}. + */ +public class TopQueriesTests extends OpenSearchTestCase { + + public void testTopQueries() throws IOException { + TopQueries topQueries = QueryInsightsTestUtils.createTopQueries(); + try (BytesStreamOutput out = new BytesStreamOutput()) { + topQueries.writeTo(out); + try (StreamInput in = out.bytes().streamInput()) { + TopQueries readTopQueries = new TopQueries(in); + assertExpected(topQueries, readTopQueries); + } + } + } + + /** + * checks all properties that are expected to be unchanged. + */ + private void assertExpected(TopQueries topQueries, TopQueries readTopQueries) throws IOException { + for (int i = 0; i < topQueries.getLatencyRecords().size(); i++) { + QueryInsightsTestUtils.compareJson(topQueries.getLatencyRecords().get(i), readTopQueries.getLatencyRecords().get(i)); + } + } +} diff --git a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/model/SearchQueryLatencyRecordTests.java b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/model/SearchQueryLatencyRecordTests.java new file mode 100644 index 0000000000000..e704e768a43e6 --- /dev/null +++ b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/model/SearchQueryLatencyRecordTests.java @@ -0,0 +1,50 @@ +/* + * 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.rules.model; + +import org.opensearch.common.io.stream.BytesStreamOutput; +import org.opensearch.core.common.io.stream.StreamInput; +import org.opensearch.plugin.insights.QueryInsightsTestUtils; +import org.opensearch.test.OpenSearchTestCase; + +import java.util.ArrayList; +import java.util.List; + +/** + * Granular tests for the {@link SearchQueryLatencyRecord} class. + */ +public class SearchQueryLatencyRecordTests extends OpenSearchTestCase { + + /** + * Check that if the serialization, deserialization and equals functions are working as expected + */ + public void testSerializationAndEquals() throws Exception { + List records = QueryInsightsTestUtils.generateQueryInsightRecords(10); + List copiedRecords = new ArrayList<>(); + for (SearchQueryLatencyRecord record : records) { + copiedRecords.add(roundTripRecord(record)); + } + assertTrue(QueryInsightsTestUtils.checkRecordsEquals(records, copiedRecords)); + + } + + /** + * Serialize and deserialize a SearchQueryLatencyRecord. + * @param record A SearchQueryLatencyRecord to serialize. + * @return The deserialized, "round-tripped" record. + */ + private static SearchQueryLatencyRecord roundTripRecord(SearchQueryLatencyRecord record) throws Exception { + try (BytesStreamOutput out = new BytesStreamOutput()) { + record.writeTo(out); + try (StreamInput in = out.bytes().streamInput()) { + return new SearchQueryLatencyRecord(in); + } + } + } +} diff --git a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/resthandler/top_queries/RestTopQueriesActionTests.java b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/resthandler/top_queries/RestTopQueriesActionTests.java new file mode 100644 index 0000000000000..627d794731eed --- /dev/null +++ b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/resthandler/top_queries/RestTopQueriesActionTests.java @@ -0,0 +1,62 @@ +/* + * 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.rules.resthandler.top_queries; + +import org.opensearch.plugin.insights.rules.action.top_queries.TopQueriesRequest; +import org.opensearch.rest.RestRequest; +import org.opensearch.test.OpenSearchTestCase; +import org.opensearch.test.rest.FakeRestRequest; + +import java.util.HashMap; +import java.util.Locale; +import java.util.Map; + +import static org.opensearch.plugin.insights.rules.resthandler.top_queries.RestTopQueriesAction.ALLOWED_METRICS; + +public class RestTopQueriesActionTests extends OpenSearchTestCase { + + public void testEmptyNodeIdsValidType() { + Map params = new HashMap<>(); + params.put("type", randomFrom(ALLOWED_METRICS)); + + RestRequest restRequest = buildRestRequest(params); + TopQueriesRequest actual = RestTopQueriesAction.prepareRequest(restRequest); + assertEquals(0, actual.nodesIds().length); + } + + public void testNodeIdsValid() { + Map params = new HashMap<>(); + params.put("type", randomFrom(ALLOWED_METRICS)); + String[] nodes = randomArray(1, 10, String[]::new, () -> randomAlphaOfLengthBetween(5, 10)); + params.put("nodeId", String.join(",", nodes)); + + RestRequest restRequest = buildRestRequest(params); + TopQueriesRequest actual = RestTopQueriesAction.prepareRequest(restRequest); + assertArrayEquals(nodes, actual.nodesIds()); + } + + public void testInValidType() { + Map params = new HashMap<>(); + params.put("type", randomAlphaOfLengthBetween(5, 10).toUpperCase(Locale.ROOT)); + + RestRequest restRequest = buildRestRequest(params); + Exception exception = assertThrows(IllegalArgumentException.class, () -> { RestTopQueriesAction.prepareRequest(restRequest); }); + assertEquals( + String.format(Locale.ROOT, "request [/_insights/top_queries] contains invalid metric type [%s]", params.get("type")), + exception.getMessage() + ); + } + + private FakeRestRequest buildRestRequest(Map params) { + return new FakeRestRequest.Builder(xContentRegistry()).withMethod(RestRequest.Method.GET) + .withPath("/_insights/top_queries") + .withParams(params) + .build(); + } +} diff --git a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/transport/top_queries/TransportTopQueriesActionTests.java b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/transport/top_queries/TransportTopQueriesActionTests.java new file mode 100644 index 0000000000000..304a4e3c8a096 --- /dev/null +++ b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/transport/top_queries/TransportTopQueriesActionTests.java @@ -0,0 +1,88 @@ +/* + * 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.rules.transport.top_queries; + +import org.opensearch.action.support.ActionFilters; +import org.opensearch.cluster.service.ClusterService; +import org.opensearch.common.settings.ClusterSettings; +import org.opensearch.common.settings.Settings; +import org.opensearch.plugin.insights.core.service.TopQueriesByLatencyService; +import org.opensearch.plugin.insights.rules.action.top_queries.TopQueriesRequest; +import org.opensearch.plugin.insights.rules.action.top_queries.TopQueriesResponse; +import org.opensearch.plugin.insights.rules.transport.top_queries.TransportTopQueriesAction; +import org.opensearch.plugin.insights.settings.QueryInsightsSettings; +import org.opensearch.test.OpenSearchTestCase; +import org.opensearch.threadpool.ThreadPool; +import org.opensearch.transport.TransportService; +import org.junit.Before; + +import java.util.List; + +import static org.mockito.Mockito.mock; + +public class TransportTopQueriesActionTests extends OpenSearchTestCase { + + private final ThreadPool threadPool = mock(ThreadPool.class); + + private final Settings.Builder settingsBuilder = Settings.builder(); + private final Settings settings = settingsBuilder.build(); + private final ClusterSettings clusterSettings = new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); + private final ClusterService clusterService = new ClusterService(settings, clusterSettings, threadPool); + private final TransportService transportService = mock(TransportService.class); + private final TopQueriesByLatencyService topQueriesByLatencyService = mock(TopQueriesByLatencyService.class); + private final ActionFilters actionFilters = mock(ActionFilters.class); + private final TransportTopQueriesAction transportTopQueriesAction = new TransportTopQueriesAction( + threadPool, + clusterService, + transportService, + topQueriesByLatencyService, + actionFilters + ); + private final DummyParentAction dummyParentAction = new DummyParentAction( + threadPool, + clusterService, + transportService, + topQueriesByLatencyService, + actionFilters + ); + + class DummyParentAction extends TransportTopQueriesAction { + public DummyParentAction( + ThreadPool threadPool, + ClusterService clusterService, + TransportService transportService, + TopQueriesByLatencyService topQueriesByLatencyService, + ActionFilters actionFilters + ) { + super(threadPool, clusterService, transportService, topQueriesByLatencyService, actionFilters); + } + + public TopQueriesResponse createNewResponse() { + TopQueriesRequest request = new TopQueriesRequest(); + return newResponse(request, List.of(), List.of()); + } + } + + @Before + public void setup() { + clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_ENABLED); + clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_SIZE); + clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_WINDOW_SIZE); + clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_EXPORTER_ENABLED); + clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_EXPORTER_TYPE); + clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_EXPORTER_INTERVAL); + clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_EXPORTER_IDENTIFIER); + } + + public void testNewResponse() { + TopQueriesResponse response = dummyParentAction.createNewResponse(); + assertNotNull(response); + } + +} From ae9e33617737569eb1401c2ff3a384b2b7afebe9 Mon Sep 17 00:00:00 2001 From: Chenyang Ji Date: Mon, 22 Jan 2024 18:46:58 -0800 Subject: [PATCH 2/5] Increase JavaDoc coverage and update PR based comments Signed-off-by: Chenyang Ji --- .../listener/SearchQueryLatencyListener.java | 6 ++ .../service/TopQueriesByLatencyService.java | 62 +++++++++++++++++++ .../rules/action/top_queries/TopQueries.java | 10 +++ .../action/top_queries/TopQueriesAction.java | 6 ++ .../action/top_queries/TopQueriesRequest.java | 22 ++++++- .../top_queries/TopQueriesResponse.java | 32 +++++----- .../top_queries/RestTopQueriesAction.java | 12 ++-- .../TransportTopQueriesAction.java | 18 ++++++ .../TransportTopQueriesActionTests.java | 1 - 9 files changed, 142 insertions(+), 27 deletions(-) diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/listener/SearchQueryLatencyListener.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/listener/SearchQueryLatencyListener.java index 4a7a3ff9dc547..c97391c74bf1c 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/listener/SearchQueryLatencyListener.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/listener/SearchQueryLatencyListener.java @@ -43,6 +43,12 @@ public final class SearchQueryLatencyListener extends SearchRequestOperationsLis private final TopQueriesByLatencyService topQueriesByLatencyService; + /** + * Constructor for SearchQueryLatencyListener + * + * @param clusterService The Node's cluster service. + * @param topQueriesByLatencyService The topQueriesByLatencyService associated with this listener + */ @Inject public SearchQueryLatencyListener(ClusterService clusterService, TopQueriesByLatencyService topQueriesByLatencyService) { this.topQueriesByLatencyService = topQueriesByLatencyService; diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/TopQueriesByLatencyService.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/TopQueriesByLatencyService.java index 5914239b4054e..087600db2e1d9 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/TopQueriesByLatencyService.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/TopQueriesByLatencyService.java @@ -50,6 +50,12 @@ public class TopQueriesByLatencyService extends QueryInsightsService< private final ClusterService clusterService; private final Client client; + /** + * Create the TopQueriesByLatencyService Object + * @param threadPool The OpenSearch thread pool to run async tasks + * @param clusterService The clusterService of this node + * @param client The OpenSearch Client + */ @Inject public TopQueriesByLatencyService(ThreadPool threadPool, ClusterService clusterService, Client client) { super(threadPool, new PriorityBlockingQueue<>(), null); @@ -100,6 +106,7 @@ public void ingestQueryData( return; } this.threadPool.schedule(() -> { + clearOutdatedData(); super.ingestQueryData( new SearchQueryLatencyRecord(timestamp, searchType, source, totalShards, indices, propertyMap, phaseLatencyMap, tookInNanos) ); @@ -117,10 +124,18 @@ public void clearOutdatedData() { store.removeIf(record -> record.getTimestamp() < System.currentTimeMillis() - windowSize.getMillis()); } + /** + * Set the top N size for TopQueriesByLatencyService service. + * @param size the top N size to set + */ public void setTopNSize(int size) { this.topNSize = size; } + /** + * Validate the top N size based on the internal constrains + * @param size the wanted top N size + */ public void validateTopNSize(int size) { if (size > QueryInsightsSettings.MAX_N_SIZE) { throw new IllegalArgumentException( @@ -138,14 +153,26 @@ public void validateTopNSize(int size) { } } + /** + * Get the top N size set for TopQueriesByLatencyService + * @return the top N size + */ public int getTopNSize() { return this.topNSize; } + /** + * Set the window size for TopQueriesByLatencyService + * @param windowSize window size to set + */ public void setWindowSize(TimeValue windowSize) { this.windowSize = windowSize; } + /** + * Validate if the window size is valid, based on internal constrains. + * @param windowSize the window size to validate + */ public void validateWindowSize(TimeValue windowSize) { if (windowSize.compareTo(QueryInsightsSettings.MAX_WINDOW_SIZE) > 0) { throw new IllegalArgumentException( @@ -163,10 +190,18 @@ public void validateWindowSize(TimeValue windowSize) { } } + /** + * Get the window size set for TopQueriesByLatencyService + * @return the window size + */ public TimeValue getWindowSize() { return this.windowSize; } + /** + * Set the exporter type to export data generated in TopQueriesByLatencyService + * @param type The type of exporter, defined in {@link QueryInsightsExporterType} + */ public void setExporterType(QueryInsightsExporterType type) { resetExporter( getEnableExport(), @@ -175,6 +210,10 @@ public void setExporterType(QueryInsightsExporterType type) { ); } + /** + * Set if the exporter is enabled + * @param enabled if the exporter is enabled + */ public void setExporterEnabled(boolean enabled) { super.setEnableExport(enabled); resetExporter( @@ -184,6 +223,12 @@ public void setExporterEnabled(boolean enabled) { ); } + /** + * Set the identifier of this exporter, which will be used when exporting the data + * + * For example, for local index exporter, this identifier would be used to define the index name + * @param identifier the identifier for the exporter + */ public void setExporterIdentifier(String identifier) { resetExporter( getEnableExport(), @@ -192,6 +237,10 @@ public void setExporterIdentifier(String identifier) { ); } + /** + * Set the export interval for the exporter + * @param interval export interval + */ public void setExportInterval(TimeValue interval) { super.setExportInterval(interval); resetExporter( @@ -201,6 +250,10 @@ public void setExportInterval(TimeValue interval) { ); } + /** + * Validate if the export interval is valid, based on internal constrains. + * @param exportInterval the export interval to validate + */ public void validateExportInterval(TimeValue exportInterval) { if (exportInterval.getSeconds() < MIN_EXPORT_INTERVAL.getSeconds()) { throw new IllegalArgumentException( @@ -219,6 +272,15 @@ public void validateExportInterval(TimeValue exportInterval) { } } + /** + * Reset the exporter with new config + * + * This function can be used to enable/disable an exporter, change the type of the exporter, + * or change the identifier of the exporter. + * @param enabled the enable flag to set on the exporter + * @param type The QueryInsightsExporterType to set on the exporter + * @param identifier the Identifier to set on the exporter + */ public void resetExporter(boolean enabled, QueryInsightsExporterType type, String identifier) { this.stop(); this.exporter = null; diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueries.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueries.java index 1bb250e5d57b7..138d23a365de3 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueries.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueries.java @@ -32,11 +32,21 @@ public class TopQueries extends BaseNodeResponse implements ToXContentObject { @Nullable private final List latencyRecords; + /** + * Create the TopQueries Object from StreamInput + * @param in A {@link StreamInput} object. + * @throws IOException IOException + */ public TopQueries(StreamInput in) throws IOException { super(in); latencyRecords = in.readList(SearchQueryLatencyRecord::new); } + /** + * Create the TopQueries Object + * @param node A node that is part of the cluster. + * @param latencyRecords The top queries by latency records stored on this node + */ public TopQueries(DiscoveryNode node, @Nullable List latencyRecords) { super(node); this.latencyRecords = latencyRecords; diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesAction.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesAction.java index 69abf001d18d9..13dd198681ca8 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesAction.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesAction.java @@ -17,7 +17,13 @@ */ public class TopQueriesAction extends ActionType { + /** + * The TopQueriesAction Instance. + */ public static final TopQueriesAction INSTANCE = new TopQueriesAction(); + /** + * The name of this Action + */ public static final String NAME = "cluster:monitor/insights/top_queries"; private TopQueriesAction() { diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesRequest.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesRequest.java index 435f5188a4e0b..a671e3a7fe176 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesRequest.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesRequest.java @@ -30,9 +30,9 @@ public class TopQueriesRequest extends BaseNodesRequest { Metric metricType = Metric.LATENCY; /** - * Create a new TopQueriesRequest from a {@link StreamInput} object. + * Constructor for TopQueriesRequest * - * @param in A stream input object. + * @param in A {@link StreamInput} object. * @throws IOException if the stream cannot be deserialized. */ public TopQueriesRequest(StreamInput in) throws IOException { @@ -43,6 +43,8 @@ public TopQueriesRequest(StreamInput in) throws IOException { /** * Get top queries from nodes based on the nodes ids specified. * If none are passed, cluster level top queries will be returned. + * + * @param nodesIds the nodeIds specified in the request */ public TopQueriesRequest(String... nodesIds) { super(nodesIds); @@ -56,7 +58,10 @@ public Metric getMetricType() { } /** - * Set the type of requested metrics + * Validate and set the metric type of requested metrics + * + * @param metricType the metric type to set to + * @return The current TopQueriesRequest */ public TopQueriesRequest setMetricType(String metricType) { metricType = metricType.toUpperCase(Locale.ROOT); @@ -77,6 +82,9 @@ public void writeTo(StreamOutput out) throws IOException { * ALl supported metrics for top queries */ public enum Metric { + /** + * Latency metric type, used by top queries by latency + */ LATENCY("LATENCY"); private final String metricName; @@ -85,10 +93,18 @@ public enum Metric { this.metricName = name; } + /** + * Get the metric name of the Metric + * @return the metric name + */ public String metricName() { return this.metricName; } + /** + * Get all valid metrics + * @return A set of String that contains all valid metrics + */ public static Set allMetrics() { return Arrays.stream(values()).map(Metric::metricName).collect(Collectors.toSet()); } diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesResponse.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesResponse.java index 37767dcee8fe5..b9afd2898ab07 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesResponse.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesResponse.java @@ -36,11 +36,25 @@ public class TopQueriesResponse extends BaseNodesResponse implements private static final String CLUSTER_LEVEL_RESULTS_KEY = "top_queries"; private final int top_n_size; + /** + * Constructor for TopQueriesResponse. + * + * @param in A {@link StreamInput} object. + * @throws IOException if the stream cannot be deserialized. + */ public TopQueriesResponse(StreamInput in) throws IOException { super(in); top_n_size = in.readInt(); } + /** + * Constructor for TopQueriesResponse + * + * @param clusterName The current cluster name + * @param nodes A list that contains top queries results from all nodes + * @param failures A list that contains FailedNodeException + * @param top_n_size The top N size to return to the user + */ public TopQueriesResponse(ClusterName clusterName, List nodes, List failures, int top_n_size) { super(clusterName, nodes, failures); this.top_n_size = top_n_size; @@ -99,22 +113,4 @@ private void toClusterLevelResult(XContentBuilder builder, Params params, List results) throws IOException { - builder.startObject(CLUSTER_LEVEL_RESULTS_KEY); - for (TopQueries topQueries : results) { - builder.startArray(topQueries.getNode().getId()); - topQueries.toXContent(builder, params); - builder.endArray(); - } - builder.endObject(); - } } diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/resthandler/top_queries/RestTopQueriesAction.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/resthandler/top_queries/RestTopQueriesAction.java index 874df86ebb4fc..9c0271842372d 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/resthandler/top_queries/RestTopQueriesAction.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/resthandler/top_queries/RestTopQueriesAction.java @@ -23,7 +23,6 @@ import org.opensearch.rest.RestResponse; import org.opensearch.rest.action.RestResponseListener; -import java.io.IOException; import java.util.List; import java.util.Locale; import java.util.Set; @@ -40,6 +39,9 @@ public class RestTopQueriesAction extends BaseRestHandler { /** The metric types that are allowed in top N queries */ static final Set ALLOWED_METRICS = TopQueriesRequest.Metric.allMetrics(); + /** + * Constructor for RestTopQueriesAction + */ public RestTopQueriesAction() {} @Override @@ -52,18 +54,18 @@ public List routes() { @Override public String getName() { - return "top_queries_action"; + return "query_insights_top_queries_action"; } @Override - public RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) throws IOException { + public RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) { final TopQueriesRequest topQueriesRequest = prepareRequest(request); topQueriesRequest.timeout(request.param("timeout")); return channel -> client.execute( TopQueriesAction.INSTANCE, topQueriesRequest, - topQueriesResponse(channel, request.method()) + topQueriesResponse(channel) ); } @@ -91,7 +93,7 @@ public boolean canTripCircuitBreaker() { return false; } - private RestResponseListener topQueriesResponse(RestChannel channel, RestRequest.Method restMethod) { + private RestResponseListener topQueriesResponse(RestChannel channel) { return new RestResponseListener<>(channel) { @Override public RestResponse buildResponse(TopQueriesResponse response) throws Exception { diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/transport/top_queries/TransportTopQueriesAction.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/transport/top_queries/TransportTopQueriesAction.java index fa7996cc6f3f7..66ba350f281b2 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/transport/top_queries/TransportTopQueriesAction.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/transport/top_queries/TransportTopQueriesAction.java @@ -43,6 +43,15 @@ public class TransportTopQueriesAction extends TransportNodesAction< private final TopQueriesByLatencyService topQueriesByLatencyService; + /** + * Create the TransportTopQueriesAction Object + + * @param threadPool The OpenSearch thread pool to run async tasks + * @param clusterService The clusterService of this node + * @param transportService The TransportService of this node + * @param topQueriesByLatencyService The topQueriesByLatencyService associated with this Transport Action + * @param actionFilters the action filters + */ @Inject public TransportTopQueriesAction( ThreadPool threadPool, @@ -113,11 +122,20 @@ public static class NodeRequest extends TransportRequest { TopQueriesRequest request; + /** + * Create the NodeResponse object from StreamInput + * @param in the StreamInput to read the object + * @throws IOException IOException + */ public NodeRequest(StreamInput in) throws IOException { super(in); request = new TopQueriesRequest(in); } + /** + * Create the NodeResponse object from a TopQueriesRequest + * @param request the TopQueriesRequest object + */ public NodeRequest(TopQueriesRequest request) { this.request = request; } diff --git a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/transport/top_queries/TransportTopQueriesActionTests.java b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/transport/top_queries/TransportTopQueriesActionTests.java index 304a4e3c8a096..96b8ec63b5a47 100644 --- a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/transport/top_queries/TransportTopQueriesActionTests.java +++ b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/transport/top_queries/TransportTopQueriesActionTests.java @@ -15,7 +15,6 @@ import org.opensearch.plugin.insights.core.service.TopQueriesByLatencyService; import org.opensearch.plugin.insights.rules.action.top_queries.TopQueriesRequest; import org.opensearch.plugin.insights.rules.action.top_queries.TopQueriesResponse; -import org.opensearch.plugin.insights.rules.transport.top_queries.TransportTopQueriesAction; import org.opensearch.plugin.insights.settings.QueryInsightsSettings; import org.opensearch.test.OpenSearchTestCase; import org.opensearch.threadpool.ThreadPool; From c4fb7ae6ed6f6d972f12e9e22dc86a1712a3bf8c Mon Sep 17 00:00:00 2001 From: Chenyang Ji Date: Mon, 29 Jan 2024 19:12:20 -0800 Subject: [PATCH 3/5] Refactor record and service to make them generic Signed-off-by: Chenyang Ji --- CHANGELOG.md | 2 +- .../QueryInsightsPluginTransportIT.java | 81 ++++- .../core/listener/QueryInsightsListener.java | 141 ++++++++ .../listener/SearchQueryLatencyListener.java | 127 -------- .../service/TopQueriesByLatencyService.java | 302 ------------------ .../rules/action/top_queries/TopQueries.java | 33 +- .../action/top_queries/TopQueriesAction.java | 2 +- .../action/top_queries/TopQueriesRequest.java | 68 +--- .../top_queries/TopQueriesResponse.java | 42 ++- .../top_queries/RestTopQueriesAction.java | 10 +- .../TransportTopQueriesAction.java | 20 +- ...s.java => QueryInsightsListenerTests.java} | 102 +++--- .../TopQueriesByLatencyServiceTests.java | 259 --------------- .../top_queries/TopQueriesRequestTests.java | 4 +- .../top_queries/TopQueriesResponseTests.java | 28 +- .../action/top_queries/TopQueriesTests.java | 15 +- .../model/SearchQueryLatencyRecordTests.java | 50 --- .../TransportTopQueriesActionTests.java | 13 +- 18 files changed, 364 insertions(+), 935 deletions(-) create mode 100644 plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/listener/QueryInsightsListener.java delete mode 100644 plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/listener/SearchQueryLatencyListener.java delete mode 100644 plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/TopQueriesByLatencyService.java rename plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/listener/{SearchQueryLatencyListenerTests.java => QueryInsightsListenerTests.java} (57%) delete mode 100644 plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/service/TopQueriesByLatencyServiceTests.java delete mode 100644 plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/model/SearchQueryLatencyRecordTests.java diff --git a/CHANGELOG.md b/CHANGELOG.md index da341454fcb40..6b78222aa742f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -224,7 +224,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Workaround for https://bugs.openjdk.org/browse/JDK-8323659 regression, introduced in JDK-21.0.2 ([#11968](https://github.com/opensearch-project/OpenSearch/pull/11968)) - Updates IpField to be searchable when only `doc_values` are enabled ([#11508](https://github.com/opensearch-project/OpenSearch/pull/11508)) - [Query Insights] Query Insights Framework which currently supports retrieving the most time-consuming queries within the last configured time window ([#11903](https://github.com/opensearch-project/OpenSearch/pull/11903)) -- [Query Insights] Implement Top N Queries Feature in Query Insights Framework([#11904](https://github.com/opensearch-project/OpenSearch/pull/11904)) +- [Query Insights] Implement Top N Queries feature to collect and gather information about high latency queries in a window ([#11904](https://github.com/opensearch-project/OpenSearch/pull/11904)) ### Deprecated diff --git a/plugins/query-insights/src/internalClusterTest/java/org/opensearch/plugin/insights/QueryInsightsPluginTransportIT.java b/plugins/query-insights/src/internalClusterTest/java/org/opensearch/plugin/insights/QueryInsightsPluginTransportIT.java index f1d8594affca7..c16a2edbdf3de 100644 --- a/plugins/query-insights/src/internalClusterTest/java/org/opensearch/plugin/insights/QueryInsightsPluginTransportIT.java +++ b/plugins/query-insights/src/internalClusterTest/java/org/opensearch/plugin/insights/QueryInsightsPluginTransportIT.java @@ -13,6 +13,7 @@ import org.opensearch.action.admin.cluster.node.info.NodesInfoRequest; import org.opensearch.action.admin.cluster.node.info.NodesInfoResponse; import org.opensearch.action.admin.cluster.node.info.PluginsAndModules; +import org.opensearch.action.admin.cluster.settings.ClusterUpdateSettingsRequest; import org.opensearch.action.index.IndexResponse; import org.opensearch.action.search.SearchResponse; import org.opensearch.common.settings.Settings; @@ -20,6 +21,7 @@ import org.opensearch.plugin.insights.rules.action.top_queries.TopQueriesAction; import org.opensearch.plugin.insights.rules.action.top_queries.TopQueriesRequest; import org.opensearch.plugin.insights.rules.action.top_queries.TopQueriesResponse; +import org.opensearch.plugin.insights.rules.model.MetricType; import org.opensearch.plugins.Plugin; import org.opensearch.plugins.PluginInfo; import org.opensearch.test.OpenSearchIntegTestCase; @@ -28,6 +30,7 @@ import java.util.Arrays; import java.util.Collection; import java.util.List; +import java.util.concurrent.ExecutionException; import java.util.function.Function; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -74,15 +77,69 @@ public void testQueryInsightPluginInstalled() { * Test get top queries when feature disabled */ public void testGetTopQueriesWhenFeatureDisabled() { - TopQueriesRequest request = new TopQueriesRequest(); + TopQueriesRequest request = new TopQueriesRequest(MetricType.LATENCY); TopQueriesResponse response = OpenSearchIntegTestCase.client().execute(TopQueriesAction.INSTANCE, request).actionGet(); Assert.assertNotEquals(0, response.failures().size()); Assert.assertEquals( - "Cannot get query data when query insight feature is not enabled.", + "Cannot get query data when query insight feature is not enabled for MetricType [latency].", response.failures().get(0).getCause().getCause().getMessage() ); } + /** + * Test update top query record when feature enabled + */ + public void testUpdateRecordWhenFeatureEnabled() throws ExecutionException, InterruptedException { + Settings commonSettings = Settings.builder().put(TOP_N_LATENCY_QUERIES_ENABLED.getKey(), "false").build(); + + logger.info("--> starting nodes for query insight testing"); + List nodes = internalCluster().startNodes(TOTAL_NUMBER_OF_NODES, Settings.builder().put(commonSettings).build()); + + logger.info("--> waiting for nodes to form a cluster"); + ClusterHealthResponse health = client().admin().cluster().prepareHealth().setWaitForNodes("2").execute().actionGet(); + assertFalse(health.isTimedOut()); + + assertAcked( + prepareCreate("test").setSettings(Settings.builder().put("index.number_of_shards", 2).put("index.number_of_replicas", 2)) + ); + ensureStableCluster(2); + logger.info("--> creating indices for query insight testing"); + for (int i = 0; i < 5; i++) { + IndexResponse response = client().prepareIndex("test_" + i).setId("" + i).setSource("field_" + i, "value_" + i).get(); + assertEquals("CREATED", response.status().toString()); + } + // making search requests to get top queries + for (int i = 0; i < TOTAL_SEARCH_REQUESTS; i++) { + SearchResponse searchResponse = internalCluster().client(randomFrom(nodes)) + .prepareSearch() + .setQuery(QueryBuilders.matchAllQuery()) + .get(); + assertEquals(searchResponse.getFailedShards(), 0); + } + + TopQueriesRequest request = new TopQueriesRequest(MetricType.LATENCY); + TopQueriesResponse response = OpenSearchIntegTestCase.client().execute(TopQueriesAction.INSTANCE, request).actionGet(); + Assert.assertNotEquals(0, response.failures().size()); + Assert.assertEquals( + "Cannot get query data when query insight feature is not enabled for MetricType [latency].", + response.failures().get(0).getCause().getCause().getMessage() + ); + + ClusterUpdateSettingsRequest updateSettingsRequest = new ClusterUpdateSettingsRequest().persistentSettings( + Settings.builder().put(TOP_N_LATENCY_QUERIES_ENABLED.getKey(), "true").build() + ); + assertAcked(internalCluster().client().admin().cluster().updateSettings(updateSettingsRequest).get()); + TopQueriesRequest request2 = new TopQueriesRequest(MetricType.LATENCY); + TopQueriesResponse response2 = OpenSearchIntegTestCase.client().execute(TopQueriesAction.INSTANCE, request2).actionGet(); + Assert.assertEquals(0, response2.failures().size()); + Assert.assertEquals(TOTAL_NUMBER_OF_NODES, response2.getNodes().size()); + for (int i = 0; i < TOTAL_NUMBER_OF_NODES; i++) { + Assert.assertEquals(0, response2.getNodes().get(i).getTopQueriesRecord().size()); + } + + internalCluster().stopAllNodes(); + } + /** * Test get top queries when feature enabled */ @@ -93,7 +150,7 @@ public void testGetTopQueriesWhenFeatureEnabled() { .put(TOP_N_LATENCY_QUERIES_WINDOW_SIZE.getKey(), "600s") .build(); - logger.info("--> starting 2 nodes for query insight testing"); + logger.info("--> starting nodes for query insight testing"); List nodes = internalCluster().startNodes(TOTAL_NUMBER_OF_NODES, Settings.builder().put(commonSettings).build()); logger.info("--> waiting for nodes to form a cluster"); @@ -118,11 +175,11 @@ public void testGetTopQueriesWhenFeatureEnabled() { assertEquals(searchResponse.getFailedShards(), 0); } - TopQueriesRequest request = new TopQueriesRequest(); + TopQueriesRequest request = new TopQueriesRequest(MetricType.LATENCY); TopQueriesResponse response = OpenSearchIntegTestCase.client().execute(TopQueriesAction.INSTANCE, request).actionGet(); Assert.assertEquals(0, response.failures().size()); Assert.assertEquals(TOTAL_NUMBER_OF_NODES, response.getNodes().size()); - Assert.assertEquals(TOTAL_SEARCH_REQUESTS, response.getNodes().stream().mapToInt(o -> o.getLatencyRecords().size()).sum()); + Assert.assertEquals(TOTAL_SEARCH_REQUESTS, response.getNodes().stream().mapToInt(o -> o.getTopQueriesRecord().size()).sum()); internalCluster().stopAllNodes(); } @@ -137,7 +194,7 @@ public void testGetTopQueriesWithSmallTopN() { .put(TOP_N_LATENCY_QUERIES_WINDOW_SIZE.getKey(), "600s") .build(); - logger.info("--> starting 2 nodes for query insight testing"); + logger.info("--> starting nodes for query insight testing"); List nodes = internalCluster().startNodes(TOTAL_NUMBER_OF_NODES, Settings.builder().put(commonSettings).build()); logger.info("--> waiting for nodes to form a cluster"); @@ -162,12 +219,11 @@ public void testGetTopQueriesWithSmallTopN() { assertEquals(searchResponse.getFailedShards(), 0); } - TopQueriesRequest request = new TopQueriesRequest(); + TopQueriesRequest request = new TopQueriesRequest(MetricType.LATENCY); TopQueriesResponse response = OpenSearchIntegTestCase.client().execute(TopQueriesAction.INSTANCE, request).actionGet(); Assert.assertEquals(0, response.failures().size()); Assert.assertEquals(TOTAL_NUMBER_OF_NODES, response.getNodes().size()); - // TODO: this should be 1 after changing to cluster level top N. - Assert.assertEquals(2, response.getNodes().stream().mapToInt(o -> o.getLatencyRecords().size()).sum()); + Assert.assertEquals(2, response.getNodes().stream().mapToInt(o -> o.getTopQueriesRecord().size()).sum()); internalCluster().stopAllNodes(); } @@ -179,10 +235,10 @@ public void testGetTopQueriesWithSmallWindowSize() { Settings commonSettings = Settings.builder() .put(TOP_N_LATENCY_QUERIES_ENABLED.getKey(), "true") .put(TOP_N_LATENCY_QUERIES_SIZE.getKey(), "100") - .put(TOP_N_LATENCY_QUERIES_WINDOW_SIZE.getKey(), "0ms") + .put(TOP_N_LATENCY_QUERIES_WINDOW_SIZE.getKey(), "1m") .build(); - logger.info("--> starting 2 nodes for query insight testing"); + logger.info("--> starting nodes for query insight testing"); List nodes = internalCluster().startNodes(TOTAL_NUMBER_OF_NODES, Settings.builder().put(commonSettings).build()); logger.info("--> waiting for nodes to form a cluster"); @@ -207,11 +263,10 @@ public void testGetTopQueriesWithSmallWindowSize() { assertEquals(searchResponse.getFailedShards(), 0); } - TopQueriesRequest request = new TopQueriesRequest(); + TopQueriesRequest request = new TopQueriesRequest(MetricType.LATENCY); TopQueriesResponse response = OpenSearchIntegTestCase.client().execute(TopQueriesAction.INSTANCE, request).actionGet(); Assert.assertEquals(0, response.failures().size()); Assert.assertEquals(TOTAL_NUMBER_OF_NODES, response.getNodes().size()); - Assert.assertEquals(0, response.getNodes().stream().mapToInt(o -> o.getLatencyRecords().size()).sum()); internalCluster().stopAllNodes(); } diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/listener/QueryInsightsListener.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/listener/QueryInsightsListener.java new file mode 100644 index 0000000000000..3bc8215ec19e5 --- /dev/null +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/listener/QueryInsightsListener.java @@ -0,0 +1,141 @@ +/* + * 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.listener; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.opensearch.action.search.SearchPhaseContext; +import org.opensearch.action.search.SearchRequest; +import org.opensearch.action.search.SearchRequestContext; +import org.opensearch.action.search.SearchRequestOperationsListener; +import org.opensearch.cluster.service.ClusterService; +import org.opensearch.common.inject.Inject; +import org.opensearch.core.xcontent.ToXContent; +import org.opensearch.plugin.insights.core.service.QueryInsightsService; +import org.opensearch.plugin.insights.rules.model.Attribute; +import org.opensearch.plugin.insights.rules.model.Measurement; +import org.opensearch.plugin.insights.rules.model.MetricType; +import org.opensearch.plugin.insights.rules.model.SearchQueryRecord; + +import java.util.Collections; +import java.util.HashMap; +import java.util.Locale; +import java.util.Map; +import java.util.concurrent.TimeUnit; + +import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.TOP_N_LATENCY_QUERIES_ENABLED; +import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.TOP_N_LATENCY_QUERIES_SIZE; +import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.TOP_N_LATENCY_QUERIES_WINDOW_SIZE; + +/** + * The listener for top N queries by latency + * + * @opensearch.internal + */ +public final class QueryInsightsListener extends SearchRequestOperationsListener { + private static final ToXContent.Params FORMAT_PARAMS = new ToXContent.MapParams(Collections.singletonMap("pretty", "false")); + + private static final Logger log = LogManager.getLogger(QueryInsightsListener.class); + + private final QueryInsightsService queryInsightsService; + + /** + * Constructor for QueryInsightsListener + * + * @param clusterService The Node's cluster service. + * @param queryInsightsService The topQueriesByLatencyService associated with this listener + */ + @Inject + public QueryInsightsListener(ClusterService clusterService, QueryInsightsService queryInsightsService) { + this.queryInsightsService = queryInsightsService; + clusterService.getClusterSettings() + .addSettingsUpdateConsumer(TOP_N_LATENCY_QUERIES_ENABLED, v -> this.setEnabled(MetricType.LATENCY, v)); + clusterService.getClusterSettings() + .addSettingsUpdateConsumer( + TOP_N_LATENCY_QUERIES_SIZE, + this.queryInsightsService::setTopNSize, + this.queryInsightsService::validateTopNSize + ); + clusterService.getClusterSettings() + .addSettingsUpdateConsumer( + TOP_N_LATENCY_QUERIES_WINDOW_SIZE, + this.queryInsightsService::setWindowSize, + this.queryInsightsService::validateWindowSize + ); + this.setEnabled(MetricType.LATENCY, clusterService.getClusterSettings().get(TOP_N_LATENCY_QUERIES_ENABLED)); + this.queryInsightsService.setTopNSize(clusterService.getClusterSettings().get(TOP_N_LATENCY_QUERIES_SIZE)); + this.queryInsightsService.setWindowSize(clusterService.getClusterSettings().get(TOP_N_LATENCY_QUERIES_WINDOW_SIZE)); + } + + /** + * Enable or disable metric collection for {@link MetricType} + * + * @param metricType {@link MetricType} + * @param enabled boolean + */ + public void setEnabled(MetricType metricType, boolean enabled) { + this.queryInsightsService.enableCollection(metricType, enabled); + + // disable QueryInsightsListener only if collection for all metrics are disabled. + if (!enabled) { + for (MetricType t : MetricType.allMetricTypes()) { + if (this.queryInsightsService.isCollectionEnabled(t)) { + return; + } + } + super.setEnabled(false); + } else { + super.setEnabled(true); + } + } + + @Override + public boolean isEnabled() { + return super.isEnabled(); + } + + @Override + public void onPhaseStart(SearchPhaseContext context) {} + + @Override + public void onPhaseEnd(SearchPhaseContext context, SearchRequestContext searchRequestContext) {} + + @Override + public void onPhaseFailure(SearchPhaseContext context) {} + + @Override + public void onRequestStart(SearchRequestContext searchRequestContext) {} + + @Override + public void onRequestEnd(SearchPhaseContext context, SearchRequestContext searchRequestContext) { + SearchRequest request = context.getRequest(); + try { + Map> measurements = new HashMap<>(); + if (queryInsightsService.isCollectionEnabled(MetricType.LATENCY)) { + measurements.put( + MetricType.LATENCY, + new Measurement<>( + MetricType.LATENCY.name(), + TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - searchRequestContext.getAbsoluteStartNanos()) + ) + ); + } + Map attributes = new HashMap<>(); + attributes.put(Attribute.SEARCH_TYPE, request.searchType().toString().toLowerCase(Locale.ROOT)); + attributes.put(Attribute.SOURCE, request.source().toString(FORMAT_PARAMS)); + attributes.put(Attribute.TOTAL_SHARDS, context.getNumShards()); + attributes.put(Attribute.INDICES, request.indices()); + attributes.put(Attribute.PHASE_LATENCY_MAP, searchRequestContext.phaseTookMap()); + SearchQueryRecord record = new SearchQueryRecord(request.getOrCreateAbsoluteStartMillis(), measurements, attributes); + queryInsightsService.addRecord(record); + } catch (Exception e) { + log.error(String.format(Locale.ROOT, "fail to ingest query insight data, error: %s", e)); + } + } +} diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/listener/SearchQueryLatencyListener.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/listener/SearchQueryLatencyListener.java deleted file mode 100644 index c97391c74bf1c..0000000000000 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/listener/SearchQueryLatencyListener.java +++ /dev/null @@ -1,127 +0,0 @@ -/* - * 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.listener; - -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; -import org.opensearch.action.search.SearchPhaseContext; -import org.opensearch.action.search.SearchRequest; -import org.opensearch.action.search.SearchRequestContext; -import org.opensearch.action.search.SearchRequestOperationsListener; -import org.opensearch.cluster.service.ClusterService; -import org.opensearch.common.inject.Inject; -import org.opensearch.core.xcontent.ToXContent; -import org.opensearch.plugin.insights.core.service.TopQueriesByLatencyService; - -import java.util.Collections; -import java.util.HashMap; -import java.util.Locale; - -import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.TOP_N_LATENCY_QUERIES_ENABLED; -import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.TOP_N_LATENCY_QUERIES_EXPORTER_ENABLED; -import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.TOP_N_LATENCY_QUERIES_EXPORTER_IDENTIFIER; -import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.TOP_N_LATENCY_QUERIES_EXPORTER_INTERVAL; -import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.TOP_N_LATENCY_QUERIES_EXPORTER_TYPE; -import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.TOP_N_LATENCY_QUERIES_SIZE; -import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.TOP_N_LATENCY_QUERIES_WINDOW_SIZE; - -/** - * The listener for top N queries by latency - * - * @opensearch.internal - */ -public final class SearchQueryLatencyListener extends SearchRequestOperationsListener { - private static final ToXContent.Params FORMAT_PARAMS = new ToXContent.MapParams(Collections.singletonMap("pretty", "false")); - - private static final Logger log = LogManager.getLogger(SearchQueryLatencyListener.class); - - private final TopQueriesByLatencyService topQueriesByLatencyService; - - /** - * Constructor for SearchQueryLatencyListener - * - * @param clusterService The Node's cluster service. - * @param topQueriesByLatencyService The topQueriesByLatencyService associated with this listener - */ - @Inject - public SearchQueryLatencyListener(ClusterService clusterService, TopQueriesByLatencyService topQueriesByLatencyService) { - this.topQueriesByLatencyService = topQueriesByLatencyService; - clusterService.getClusterSettings().addSettingsUpdateConsumer(TOP_N_LATENCY_QUERIES_ENABLED, this::setEnabled); - clusterService.getClusterSettings() - .addSettingsUpdateConsumer( - TOP_N_LATENCY_QUERIES_SIZE, - this.topQueriesByLatencyService::setTopNSize, - this.topQueriesByLatencyService::validateTopNSize - ); - clusterService.getClusterSettings() - .addSettingsUpdateConsumer( - TOP_N_LATENCY_QUERIES_WINDOW_SIZE, - this.topQueriesByLatencyService::setWindowSize, - this.topQueriesByLatencyService::validateWindowSize - ); - clusterService.getClusterSettings() - .addSettingsUpdateConsumer(TOP_N_LATENCY_QUERIES_EXPORTER_TYPE, this.topQueriesByLatencyService::setExporterType); - clusterService.getClusterSettings() - .addSettingsUpdateConsumer( - TOP_N_LATENCY_QUERIES_EXPORTER_INTERVAL, - this.topQueriesByLatencyService::setExportInterval, - this.topQueriesByLatencyService::validateExportInterval - ); - clusterService.getClusterSettings() - .addSettingsUpdateConsumer(TOP_N_LATENCY_QUERIES_EXPORTER_IDENTIFIER, this.topQueriesByLatencyService::setExporterIdentifier); - clusterService.getClusterSettings() - .addSettingsUpdateConsumer(TOP_N_LATENCY_QUERIES_EXPORTER_ENABLED, this.topQueriesByLatencyService::setExporterEnabled); - - this.setEnabled(clusterService.getClusterSettings().get(TOP_N_LATENCY_QUERIES_ENABLED)); - this.topQueriesByLatencyService.setTopNSize(clusterService.getClusterSettings().get(TOP_N_LATENCY_QUERIES_SIZE)); - this.topQueriesByLatencyService.setWindowSize(clusterService.getClusterSettings().get(TOP_N_LATENCY_QUERIES_WINDOW_SIZE)); - } - - @Override - public void setEnabled(boolean enabled) { - super.setEnabled(enabled); - this.topQueriesByLatencyService.setEnableCollect(enabled); - } - - @Override - public boolean isEnabled() { - return super.isEnabled(); - } - - @Override - public void onPhaseStart(SearchPhaseContext context) {} - - @Override - public void onPhaseEnd(SearchPhaseContext context, SearchRequestContext searchRequestContext) {} - - @Override - public void onPhaseFailure(SearchPhaseContext context) {} - - @Override - public void onRequestStart(SearchRequestContext searchRequestContext) {} - - @Override - public void onRequestEnd(SearchPhaseContext context, SearchRequestContext searchRequestContext) { - SearchRequest request = context.getRequest(); - try { - topQueriesByLatencyService.ingestQueryData( - request.getOrCreateAbsoluteStartMillis(), - request.searchType(), - request.source().toString(FORMAT_PARAMS), - context.getNumShards(), - request.indices(), - new HashMap<>(), - searchRequestContext.phaseTookMap(), - System.nanoTime() - searchRequestContext.getAbsoluteStartNanos() - ); - } catch (Exception e) { - log.error(String.format(Locale.ROOT, "fail to ingest query insight data, error: %s", e)); - } - } -} diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/TopQueriesByLatencyService.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/TopQueriesByLatencyService.java deleted file mode 100644 index 087600db2e1d9..0000000000000 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/TopQueriesByLatencyService.java +++ /dev/null @@ -1,302 +0,0 @@ -/* - * 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; - -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; -import org.opensearch.action.search.SearchType; -import org.opensearch.client.Client; -import org.opensearch.cluster.service.ClusterService; -import org.opensearch.common.inject.Inject; -import org.opensearch.common.unit.TimeValue; -import org.opensearch.plugin.insights.core.exporter.QueryInsightsExporter; -import org.opensearch.plugin.insights.core.exporter.QueryInsightsExporterType; -import org.opensearch.plugin.insights.core.exporter.QueryInsightsLocalIndexExporter; -import org.opensearch.plugin.insights.rules.model.SearchQueryLatencyRecord; -import org.opensearch.plugin.insights.settings.QueryInsightsSettings; -import org.opensearch.threadpool.ThreadPool; - -import java.util.Locale; -import java.util.Map; -import java.util.concurrent.PriorityBlockingQueue; - -import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.DEFAULT_LOCAL_INDEX_MAPPING; -import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.MIN_EXPORT_INTERVAL; - -/** - * Service responsible for gathering, analyzing, storing and exporting - * top N queries with high latency data for search queries - * - * @opensearch.internal - */ -public class TopQueriesByLatencyService extends QueryInsightsService< - SearchQueryLatencyRecord, - PriorityBlockingQueue, - QueryInsightsExporter> { - private static final Logger log = LogManager.getLogger(TopQueriesByLatencyService.class); - - private static final TimeValue delay = TimeValue.ZERO; - - private int topNSize = QueryInsightsSettings.DEFAULT_TOP_N_SIZE; - - private TimeValue windowSize = TimeValue.timeValueSeconds(QueryInsightsSettings.DEFAULT_WINDOW_SIZE); - - private final ClusterService clusterService; - private final Client client; - - /** - * Create the TopQueriesByLatencyService Object - * @param threadPool The OpenSearch thread pool to run async tasks - * @param clusterService The clusterService of this node - * @param client The OpenSearch Client - */ - @Inject - public TopQueriesByLatencyService(ThreadPool threadPool, ClusterService clusterService, Client client) { - super(threadPool, new PriorityBlockingQueue<>(), null); - this.clusterService = clusterService; - this.client = client; - } - - /** - * Ingest the query data into to the top N queries with latency store - * - * @param timestamp The timestamp of the query. - * @param searchType The manner at which the search operation is executed. see {@link SearchType} - * @param source The search source that was executed by the query. - * @param totalShards Total number of shards as part of the search query across all indices - * @param indices The indices involved in the search query - * @param propertyMap Extra attributes and information about a search query - * @param phaseLatencyMap Contains phase level latency information in a search query - * @param tookInNanos Total search request took time in nanoseconds - */ - public void ingestQueryData( - final Long timestamp, - final SearchType searchType, - final String source, - final int totalShards, - final String[] indices, - final Map propertyMap, - final Map phaseLatencyMap, - final Long tookInNanos - ) { - if (timestamp <= 0) { - log.error( - String.format( - Locale.ROOT, - "Invalid timestamp %s when ingesting query data to compute top n queries with latency", - timestamp - ) - ); - return; - } - if (totalShards <= 0) { - log.error( - String.format( - Locale.ROOT, - "Invalid totalShards %s when ingesting query data to compute top n queries with latency", - totalShards - ) - ); - return; - } - this.threadPool.schedule(() -> { - clearOutdatedData(); - super.ingestQueryData( - new SearchQueryLatencyRecord(timestamp, searchType, source, totalShards, indices, propertyMap, phaseLatencyMap, tookInNanos) - ); - // remove top elements for fix sizing priority queue - if (this.store.size() > this.getTopNSize()) { - this.store.poll(); - } - }, delay, ThreadPool.Names.GENERIC); - - log.debug(String.format(Locale.ROOT, "successfully ingested: %s", this.store)); - } - - @Override - public void clearOutdatedData() { - store.removeIf(record -> record.getTimestamp() < System.currentTimeMillis() - windowSize.getMillis()); - } - - /** - * Set the top N size for TopQueriesByLatencyService service. - * @param size the top N size to set - */ - public void setTopNSize(int size) { - this.topNSize = size; - } - - /** - * Validate the top N size based on the internal constrains - * @param size the wanted top N size - */ - public void validateTopNSize(int size) { - if (size > QueryInsightsSettings.MAX_N_SIZE) { - throw new IllegalArgumentException( - "Top N size setting [" - + QueryInsightsSettings.TOP_N_LATENCY_QUERIES_SIZE.getKey() - + "]" - + " should be smaller than max top N size [" - + QueryInsightsSettings.MAX_N_SIZE - + "was (" - + size - + " > " - + QueryInsightsSettings.MAX_N_SIZE - + ")" - ); - } - } - - /** - * Get the top N size set for TopQueriesByLatencyService - * @return the top N size - */ - public int getTopNSize() { - return this.topNSize; - } - - /** - * Set the window size for TopQueriesByLatencyService - * @param windowSize window size to set - */ - public void setWindowSize(TimeValue windowSize) { - this.windowSize = windowSize; - } - - /** - * Validate if the window size is valid, based on internal constrains. - * @param windowSize the window size to validate - */ - public void validateWindowSize(TimeValue windowSize) { - if (windowSize.compareTo(QueryInsightsSettings.MAX_WINDOW_SIZE) > 0) { - throw new IllegalArgumentException( - "Window size setting [" - + QueryInsightsSettings.TOP_N_LATENCY_QUERIES_WINDOW_SIZE.getKey() - + "]" - + " should be smaller than max window size [" - + QueryInsightsSettings.MAX_WINDOW_SIZE - + "was (" - + windowSize - + " > " - + QueryInsightsSettings.MAX_WINDOW_SIZE - + ")" - ); - } - } - - /** - * Get the window size set for TopQueriesByLatencyService - * @return the window size - */ - public TimeValue getWindowSize() { - return this.windowSize; - } - - /** - * Set the exporter type to export data generated in TopQueriesByLatencyService - * @param type The type of exporter, defined in {@link QueryInsightsExporterType} - */ - public void setExporterType(QueryInsightsExporterType type) { - resetExporter( - getEnableExport(), - type, - clusterService.getClusterSettings().get(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_EXPORTER_IDENTIFIER) - ); - } - - /** - * Set if the exporter is enabled - * @param enabled if the exporter is enabled - */ - public void setExporterEnabled(boolean enabled) { - super.setEnableExport(enabled); - resetExporter( - enabled, - clusterService.getClusterSettings().get(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_EXPORTER_TYPE), - clusterService.getClusterSettings().get(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_EXPORTER_IDENTIFIER) - ); - } - - /** - * Set the identifier of this exporter, which will be used when exporting the data - * - * For example, for local index exporter, this identifier would be used to define the index name - * @param identifier the identifier for the exporter - */ - public void setExporterIdentifier(String identifier) { - resetExporter( - getEnableExport(), - clusterService.getClusterSettings().get(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_EXPORTER_TYPE), - identifier - ); - } - - /** - * Set the export interval for the exporter - * @param interval export interval - */ - public void setExportInterval(TimeValue interval) { - super.setExportInterval(interval); - resetExporter( - getEnableExport(), - clusterService.getClusterSettings().get(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_EXPORTER_TYPE), - clusterService.getClusterSettings().get(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_EXPORTER_IDENTIFIER) - ); - } - - /** - * Validate if the export interval is valid, based on internal constrains. - * @param exportInterval the export interval to validate - */ - public void validateExportInterval(TimeValue exportInterval) { - if (exportInterval.getSeconds() < MIN_EXPORT_INTERVAL.getSeconds()) { - throw new IllegalArgumentException( - "Export Interval setting [" - + QueryInsightsSettings.TOP_N_LATENCY_QUERIES_EXPORTER_INTERVAL.getKey() - + "]" - + " should not be smaller than minimal export interval size [" - + MIN_EXPORT_INTERVAL - + "]" - + "was (" - + exportInterval - + " < " - + MIN_EXPORT_INTERVAL - + ")" - ); - } - } - - /** - * Reset the exporter with new config - * - * This function can be used to enable/disable an exporter, change the type of the exporter, - * or change the identifier of the exporter. - * @param enabled the enable flag to set on the exporter - * @param type The QueryInsightsExporterType to set on the exporter - * @param identifier the Identifier to set on the exporter - */ - public void resetExporter(boolean enabled, QueryInsightsExporterType type, String identifier) { - this.stop(); - this.exporter = null; - - if (!enabled) { - return; - } - if (type.equals(QueryInsightsExporterType.LOCAL_INDEX)) { - this.exporter = new QueryInsightsLocalIndexExporter<>( - clusterService, - client, - identifier, - TopQueriesByLatencyService.class.getClassLoader().getResourceAsStream(DEFAULT_LOCAL_INDEX_MAPPING) - ); - } - this.start(); - } - -} diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueries.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueries.java index 138d23a365de3..640a0b82260b5 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueries.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueries.java @@ -10,12 +10,11 @@ import org.opensearch.action.support.nodes.BaseNodeResponse; import org.opensearch.cluster.node.DiscoveryNode; -import org.opensearch.common.Nullable; import org.opensearch.core.common.io.stream.StreamInput; import org.opensearch.core.common.io.stream.StreamOutput; import org.opensearch.core.xcontent.ToXContentObject; import org.opensearch.core.xcontent.XContentBuilder; -import org.opensearch.plugin.insights.rules.model.SearchQueryLatencyRecord; +import org.opensearch.plugin.insights.rules.model.SearchQueryRecord; import java.io.IOException; import java.util.List; @@ -28,9 +27,8 @@ * @opensearch.internal */ public class TopQueries extends BaseNodeResponse implements ToXContentObject { - /** The store to keep the top N queries with latency records */ - @Nullable - private final List latencyRecords; + /** The store to keep the top N queries records */ + private final List topQueriesRecords; /** * Create the TopQueries Object from StreamInput @@ -39,23 +37,23 @@ public class TopQueries extends BaseNodeResponse implements ToXContentObject { */ public TopQueries(StreamInput in) throws IOException { super(in); - latencyRecords = in.readList(SearchQueryLatencyRecord::new); + topQueriesRecords = in.readList(SearchQueryRecord::new); } /** * Create the TopQueries Object * @param node A node that is part of the cluster. - * @param latencyRecords The top queries by latency records stored on this node + * @param searchQueryRecords A list of SearchQueryRecord associated in this TopQueries. */ - public TopQueries(DiscoveryNode node, @Nullable List latencyRecords) { + public TopQueries(DiscoveryNode node, List searchQueryRecords) { super(node); - this.latencyRecords = latencyRecords; + topQueriesRecords = searchQueryRecords; } @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { - if (latencyRecords != null) { - for (SearchQueryLatencyRecord record : latencyRecords) { + if (topQueriesRecords != null) { + for (SearchQueryRecord record : topQueriesRecords) { record.toXContent(builder, params); } } @@ -65,17 +63,16 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws @Override public void writeTo(StreamOutput out) throws IOException { super.writeTo(out); - if (latencyRecords != null) { - out.writeList(latencyRecords); - } + out.writeList(topQueriesRecords); + } /** - * Get all latency records + * Get all top queries records * - * @return the latency records in this node response + * @return the top queries records in this node response */ - public List getLatencyRecords() { - return latencyRecords; + public List getTopQueriesRecord() { + return topQueriesRecords; } } diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesAction.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesAction.java index 13dd198681ca8..b8ed69fa5692b 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesAction.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesAction.java @@ -24,7 +24,7 @@ public class TopQueriesAction extends ActionType { /** * The name of this Action */ - public static final String NAME = "cluster:monitor/insights/top_queries"; + public static final String NAME = "cluster:admin/opensearch/insights/top_queries"; private TopQueriesAction() { super(NAME, TopQueriesResponse::new); diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesRequest.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesRequest.java index a671e3a7fe176..27177fef25bea 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesRequest.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesRequest.java @@ -12,12 +12,9 @@ import org.opensearch.common.annotation.PublicApi; import org.opensearch.core.common.io.stream.StreamInput; import org.opensearch.core.common.io.stream.StreamOutput; +import org.opensearch.plugin.insights.rules.model.MetricType; import java.io.IOException; -import java.util.Arrays; -import java.util.Locale; -import java.util.Set; -import java.util.stream.Collectors; /** * A request to get cluster/node level top queries information. @@ -27,7 +24,7 @@ @PublicApi(since = "1.0.0") public class TopQueriesRequest extends BaseNodesRequest { - Metric metricType = Metric.LATENCY; + MetricType metricType; /** * Constructor for TopQueriesRequest @@ -37,76 +34,35 @@ public class TopQueriesRequest extends BaseNodesRequest { */ public TopQueriesRequest(StreamInput in) throws IOException { super(in); - setMetricType(in.readString()); + MetricType metricType = MetricType.readFromStream(in); + if (false == MetricType.allMetricTypes().contains(metricType)) { + throw new IllegalStateException("Invalid metric used in top queries request: " + metricType); + } + this.metricType = metricType; } /** * Get top queries from nodes based on the nodes ids specified. * If none are passed, cluster level top queries will be returned. * + * @param metricType {@link MetricType} * @param nodesIds the nodeIds specified in the request */ - public TopQueriesRequest(String... nodesIds) { + public TopQueriesRequest(MetricType metricType, String... nodesIds) { super(nodesIds); + this.metricType = metricType; } /** * Get the type of requested metrics */ - public Metric getMetricType() { + public MetricType getMetricType() { return metricType; } - /** - * Validate and set the metric type of requested metrics - * - * @param metricType the metric type to set to - * @return The current TopQueriesRequest - */ - public TopQueriesRequest setMetricType(String metricType) { - metricType = metricType.toUpperCase(Locale.ROOT); - if (false == Metric.allMetrics().contains(metricType)) { - throw new IllegalStateException("Invalid metric used in top queries request: " + metricType); - } - this.metricType = Metric.valueOf(metricType); - return this; - } - @Override public void writeTo(StreamOutput out) throws IOException { super.writeTo(out); - out.writeString(metricType.metricName()); - } - - /** - * ALl supported metrics for top queries - */ - public enum Metric { - /** - * Latency metric type, used by top queries by latency - */ - LATENCY("LATENCY"); - - private final String metricName; - - Metric(String name) { - this.metricName = name; - } - - /** - * Get the metric name of the Metric - * @return the metric name - */ - public String metricName() { - return this.metricName; - } - - /** - * Get all valid metrics - * @return A set of String that contains all valid metrics - */ - public static Set allMetrics() { - return Arrays.stream(values()).map(Metric::metricName).collect(Collectors.toSet()); - } + out.writeString(metricType.toString()); } } diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesResponse.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesResponse.java index b9afd2898ab07..fe7644de5629f 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesResponse.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesResponse.java @@ -17,11 +17,12 @@ import org.opensearch.core.common.io.stream.StreamOutput; import org.opensearch.core.xcontent.ToXContentFragment; import org.opensearch.core.xcontent.XContentBuilder; -import org.opensearch.plugin.insights.rules.model.SearchQueryLatencyRecord; +import org.opensearch.plugin.insights.rules.model.Attribute; +import org.opensearch.plugin.insights.rules.model.MetricType; +import org.opensearch.plugin.insights.rules.model.SearchQueryRecord; import java.io.IOException; import java.util.Collection; -import java.util.Collections; import java.util.List; import java.util.stream.Collectors; @@ -34,6 +35,7 @@ public class TopQueriesResponse extends BaseNodesResponse implements ToXContentFragment { private static final String CLUSTER_LEVEL_RESULTS_KEY = "top_queries"; + private final MetricType metricType; private final int top_n_size; /** @@ -45,6 +47,7 @@ public class TopQueriesResponse extends BaseNodesResponse implements public TopQueriesResponse(StreamInput in) throws IOException { super(in); top_n_size = in.readInt(); + metricType = in.readEnum(MetricType.class); } /** @@ -54,10 +57,18 @@ public TopQueriesResponse(StreamInput in) throws IOException { * @param nodes A list that contains top queries results from all nodes * @param failures A list that contains FailedNodeException * @param top_n_size The top N size to return to the user + * @param metricType the {@link MetricType} to be returned in this response */ - public TopQueriesResponse(ClusterName clusterName, List nodes, List failures, int top_n_size) { + public TopQueriesResponse( + ClusterName clusterName, + List nodes, + List failures, + int top_n_size, + MetricType metricType + ) { super(clusterName, nodes, failures); this.top_n_size = top_n_size; + this.metricType = metricType; } @Override @@ -69,11 +80,13 @@ protected List readNodesFrom(StreamInput in) throws IOException { protected void writeNodesTo(StreamOutput out, List nodes) throws IOException { out.writeList(nodes); out.writeLong(top_n_size); + out.writeEnum(metricType); } @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { List results = getNodes(); + postProcess(results); builder.startObject(); toClusterLevelResult(builder, params, results); return builder.endObject(); @@ -92,6 +105,20 @@ public String toString() { } } + /** + * Post process the top queries results to add customized attributes + * + * @param results the top queries results + */ + private void postProcess(List results) { + for (TopQueries topQueries : results) { + String nodeId = topQueries.getNode().getId(); + for (SearchQueryRecord record : topQueries.getTopQueriesRecord()) { + record.addAttribute(Attribute.NODE_ID, nodeId); + } + } + } + /** * Merge top n queries results from nodes into cluster level results in XContent format. * @@ -101,16 +128,17 @@ public String toString() { * @throws IOException if an error occurs */ private void toClusterLevelResult(XContentBuilder builder, Params params, List results) throws IOException { - List all_records = results.stream() - .map(TopQueries::getLatencyRecords) + List all_records = results.stream() + .map(TopQueries::getTopQueriesRecord) .flatMap(Collection::stream) - .sorted(Collections.reverseOrder()) + .sorted((a, b) -> SearchQueryRecord.compare(a, b, metricType) * -1) .limit(top_n_size) .collect(Collectors.toList()); builder.startArray(CLUSTER_LEVEL_RESULTS_KEY); - for (SearchQueryLatencyRecord record : all_records) { + for (SearchQueryRecord record : all_records) { record.toXContent(builder, params); } builder.endArray(); } + } diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/resthandler/top_queries/RestTopQueriesAction.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/resthandler/top_queries/RestTopQueriesAction.java index 9c0271842372d..654aab6eb1563 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/resthandler/top_queries/RestTopQueriesAction.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/resthandler/top_queries/RestTopQueriesAction.java @@ -16,6 +16,7 @@ import org.opensearch.plugin.insights.rules.action.top_queries.TopQueriesAction; import org.opensearch.plugin.insights.rules.action.top_queries.TopQueriesRequest; import org.opensearch.plugin.insights.rules.action.top_queries.TopQueriesResponse; +import org.opensearch.plugin.insights.rules.model.MetricType; import org.opensearch.rest.BaseRestHandler; import org.opensearch.rest.BytesRestResponse; import org.opensearch.rest.RestChannel; @@ -26,6 +27,7 @@ import java.util.List; import java.util.Locale; import java.util.Set; +import java.util.stream.Collectors; import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.TOP_QUERIES_BASE_URI; import static org.opensearch.rest.RestRequest.Method.GET; @@ -37,7 +39,7 @@ */ public class RestTopQueriesAction extends BaseRestHandler { /** The metric types that are allowed in top N queries */ - static final Set ALLOWED_METRICS = TopQueriesRequest.Metric.allMetrics(); + static final Set ALLOWED_METRICS = MetricType.allMetricTypes().stream().map(MetricType::toString).collect(Collectors.toSet()); /** * Constructor for RestTopQueriesAction @@ -72,15 +74,13 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC static TopQueriesRequest prepareRequest(final RestRequest request) { String[] nodesIds = Strings.splitStringByCommaToArray(request.param("nodeId")); - String metricType = request.param("type", TopQueriesRequest.Metric.LATENCY.metricName()).toUpperCase(Locale.ROOT); + String metricType = request.param("type", MetricType.LATENCY.toString()); if (!ALLOWED_METRICS.contains(metricType)) { throw new IllegalArgumentException( String.format(Locale.ROOT, "request [%s] contains invalid metric type [%s]", request.path(), metricType) ); } - TopQueriesRequest topQueriesRequest = new TopQueriesRequest(nodesIds); - topQueriesRequest.setMetricType(metricType); - return topQueriesRequest; + return new TopQueriesRequest(MetricType.fromString(metricType), nodesIds); } @Override diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/transport/top_queries/TransportTopQueriesAction.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/transport/top_queries/TransportTopQueriesAction.java index 66ba350f281b2..a4ddaca0a6cdc 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/transport/top_queries/TransportTopQueriesAction.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/transport/top_queries/TransportTopQueriesAction.java @@ -16,11 +16,12 @@ import org.opensearch.common.inject.Inject; import org.opensearch.core.common.io.stream.StreamInput; import org.opensearch.core.common.io.stream.StreamOutput; -import org.opensearch.plugin.insights.core.service.TopQueriesByLatencyService; +import org.opensearch.plugin.insights.core.service.QueryInsightsService; import org.opensearch.plugin.insights.rules.action.top_queries.TopQueries; import org.opensearch.plugin.insights.rules.action.top_queries.TopQueriesAction; import org.opensearch.plugin.insights.rules.action.top_queries.TopQueriesRequest; import org.opensearch.plugin.insights.rules.action.top_queries.TopQueriesResponse; +import org.opensearch.plugin.insights.rules.model.MetricType; import org.opensearch.plugin.insights.settings.QueryInsightsSettings; import org.opensearch.threadpool.ThreadPool; import org.opensearch.transport.TransportRequest; @@ -41,7 +42,7 @@ public class TransportTopQueriesAction extends TransportNodesAction< TransportTopQueriesAction.NodeRequest, TopQueries> { - private final TopQueriesByLatencyService topQueriesByLatencyService; + private final QueryInsightsService queryInsightsService; /** * Create the TransportTopQueriesAction Object @@ -49,7 +50,7 @@ public class TransportTopQueriesAction extends TransportNodesAction< * @param threadPool The OpenSearch thread pool to run async tasks * @param clusterService The clusterService of this node * @param transportService The TransportService of this node - * @param topQueriesByLatencyService The topQueriesByLatencyService associated with this Transport Action + * @param queryInsightsService The topQueriesByLatencyService associated with this Transport Action * @param actionFilters the action filters */ @Inject @@ -57,7 +58,7 @@ public TransportTopQueriesAction( ThreadPool threadPool, ClusterService clusterService, TransportService transportService, - TopQueriesByLatencyService topQueriesByLatencyService, + QueryInsightsService queryInsightsService, ActionFilters actionFilters ) { super( @@ -71,7 +72,7 @@ public TransportTopQueriesAction( ThreadPool.Names.GENERIC, TopQueries.class ); - this.topQueriesByLatencyService = topQueriesByLatencyService; + this.queryInsightsService = queryInsightsService; } @Override @@ -80,12 +81,13 @@ protected TopQueriesResponse newResponse( List responses, List failures ) { - if (topQueriesRequest.getMetricType() == TopQueriesRequest.Metric.LATENCY) { + if (topQueriesRequest.getMetricType() == MetricType.LATENCY) { return new TopQueriesResponse( clusterService.getClusterName(), responses, failures, - clusterService.getClusterSettings().get(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_SIZE) + clusterService.getClusterSettings().get(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_SIZE), + MetricType.LATENCY ); } else { throw new OpenSearchException(String.format(Locale.ROOT, "invalid metric type %s", topQueriesRequest.getMetricType())); @@ -105,8 +107,8 @@ protected TopQueries newNodeResponse(StreamInput in) throws IOException { @Override protected TopQueries nodeOperation(NodeRequest nodeRequest) { TopQueriesRequest request = nodeRequest.request; - if (request.getMetricType() == TopQueriesRequest.Metric.LATENCY) { - return new TopQueries(clusterService.localNode(), topQueriesByLatencyService.getQueryData()); + if (request.getMetricType() == MetricType.LATENCY) { + return new TopQueries(clusterService.localNode(), queryInsightsService.getTopNRecords(MetricType.LATENCY, true)); } else { throw new OpenSearchException(String.format(Locale.ROOT, "invalid metric type %s", request.getMetricType())); } diff --git a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/listener/SearchQueryLatencyListenerTests.java b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/listener/QueryInsightsListenerTests.java similarity index 57% rename from plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/listener/SearchQueryLatencyListenerTests.java rename to plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/listener/QueryInsightsListenerTests.java index 5228e0054c440..a6368bb0e180f 100644 --- a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/listener/SearchQueryLatencyListenerTests.java +++ b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/listener/QueryInsightsListenerTests.java @@ -15,12 +15,14 @@ import org.opensearch.cluster.service.ClusterService; import org.opensearch.common.settings.ClusterSettings; import org.opensearch.common.settings.Settings; -import org.opensearch.plugin.insights.core.service.TopQueriesByLatencyService; +import org.opensearch.plugin.insights.core.service.QueryInsightsService; +import org.opensearch.plugin.insights.rules.model.MetricType; import org.opensearch.plugin.insights.settings.QueryInsightsSettings; import org.opensearch.search.aggregations.bucket.terms.TermsAggregationBuilder; import org.opensearch.search.aggregations.support.ValueType; import org.opensearch.search.builder.SearchSourceBuilder; import org.opensearch.test.OpenSearchTestCase; +import org.junit.Before; import java.util.ArrayList; import java.util.HashMap; @@ -29,38 +31,35 @@ import java.util.concurrent.CountDownLatch; import java.util.concurrent.Phaser; -import static org.mockito.ArgumentMatchers.anyLong; -import static org.mockito.Mockito.anyMap; -import static org.mockito.Mockito.eq; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; /** - * Unit Tests for {@link SearchQueryLatencyListener}. + * Unit Tests for {@link QueryInsightsListener}. */ -public class SearchQueryLatencyListenerTests extends OpenSearchTestCase { - - public void testOnRequestEnd() { - final SearchRequestContext searchRequestContext = mock(SearchRequestContext.class); - final SearchPhaseContext searchPhaseContext = mock(SearchPhaseContext.class); - final SearchRequest searchRequest = mock(SearchRequest.class); - final TopQueriesByLatencyService topQueriesByLatencyService = mock(TopQueriesByLatencyService.class); - +public class QueryInsightsListenerTests extends OpenSearchTestCase { + private final SearchRequestContext searchRequestContext = mock(SearchRequestContext.class); + private final SearchPhaseContext searchPhaseContext = mock(SearchPhaseContext.class); + private final SearchRequest searchRequest = mock(SearchRequest.class); + private final QueryInsightsService queryInsightsService = mock(QueryInsightsService.class); + private ClusterService clusterService; + + @Before + public void setup() { Settings.Builder settingsBuilder = Settings.builder(); Settings settings = settingsBuilder.build(); ClusterSettings clusterSettings = new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_ENABLED); clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_SIZE); clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_WINDOW_SIZE); - clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_EXPORTER_ENABLED); - clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_EXPORTER_TYPE); - clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_EXPORTER_INTERVAL); - clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_EXPORTER_IDENTIFIER); - - ClusterService clusterService = new ClusterService(settings, clusterSettings, null); + clusterService = new ClusterService(settings, clusterSettings, null); + when(queryInsightsService.isCollectionEnabled(MetricType.LATENCY)).thenReturn(true); + } + public void testOnRequestEnd() { Long timestamp = System.currentTimeMillis() - 100L; SearchType searchType = SearchType.QUERY_THEN_FETCH; @@ -77,7 +76,7 @@ public void testOnRequestEnd() { int numberOfShards = 10; - SearchQueryLatencyListener searchQueryLatencyListener = new SearchQueryLatencyListener(clusterService, topQueriesByLatencyService); + QueryInsightsListener queryInsightsListener = new QueryInsightsListener(clusterService, queryInsightsService); when(searchRequest.getOrCreateAbsoluteStartMillis()).thenReturn(timestamp); when(searchRequest.searchType()).thenReturn(searchType); @@ -87,39 +86,12 @@ public void testOnRequestEnd() { when(searchPhaseContext.getRequest()).thenReturn(searchRequest); when(searchPhaseContext.getNumShards()).thenReturn(numberOfShards); - searchQueryLatencyListener.onRequestEnd(searchPhaseContext, searchRequestContext); - - verify(topQueriesByLatencyService, times(1)).ingestQueryData( - eq(timestamp), - eq(searchType), - eq(searchSourceBuilder.toString()), - eq(numberOfShards), - eq(indices), - anyMap(), - eq(phaseLatencyMap), - anyLong() - ); + queryInsightsListener.onRequestEnd(searchPhaseContext, searchRequestContext); + + verify(queryInsightsService, times(1)).addRecord(any()); } public void testConcurrentOnRequestEnd() throws InterruptedException { - final SearchRequestContext searchRequestContext = mock(SearchRequestContext.class); - final SearchPhaseContext searchPhaseContext = mock(SearchPhaseContext.class); - final SearchRequest searchRequest = mock(SearchRequest.class); - final TopQueriesByLatencyService topQueriesByLatencyService = mock(TopQueriesByLatencyService.class); - - Settings.Builder settingsBuilder = Settings.builder(); - Settings settings = settingsBuilder.build(); - ClusterSettings clusterSettings = new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); - clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_ENABLED); - clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_SIZE); - clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_WINDOW_SIZE); - clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_EXPORTER_ENABLED); - clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_EXPORTER_TYPE); - clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_EXPORTER_INTERVAL); - clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_EXPORTER_IDENTIFIER); - - ClusterService clusterService = new ClusterService(settings, clusterSettings, null); - Long timestamp = System.currentTimeMillis() - 100L; SearchType searchType = SearchType.QUERY_THEN_FETCH; @@ -136,7 +108,7 @@ public void testConcurrentOnRequestEnd() throws InterruptedException { int numberOfShards = 10; - final List searchListenersList = new ArrayList<>(); + final List searchListenersList = new ArrayList<>(); when(searchRequest.getOrCreateAbsoluteStartMillis()).thenReturn(timestamp); when(searchRequest.searchType()).thenReturn(searchType); @@ -152,14 +124,14 @@ public void testConcurrentOnRequestEnd() throws InterruptedException { CountDownLatch countDownLatch = new CountDownLatch(numRequests); for (int i = 0; i < numRequests; i++) { - searchListenersList.add(new SearchQueryLatencyListener(clusterService, topQueriesByLatencyService)); + searchListenersList.add(new QueryInsightsListener(clusterService, queryInsightsService)); } for (int i = 0; i < numRequests; i++) { int finalI = i; threads[i] = new Thread(() -> { phaser.arriveAndAwaitAdvance(); - SearchQueryLatencyListener thisListener = searchListenersList.get(finalI); + QueryInsightsListener thisListener = searchListenersList.get(finalI); thisListener.onRequestEnd(searchPhaseContext, searchRequestContext); countDownLatch.countDown(); }); @@ -168,15 +140,19 @@ public void testConcurrentOnRequestEnd() throws InterruptedException { phaser.arriveAndAwaitAdvance(); countDownLatch.await(); - verify(topQueriesByLatencyService, times(numRequests)).ingestQueryData( - eq(timestamp), - eq(searchType), - eq(searchSourceBuilder.toString()), - eq(numberOfShards), - eq(indices), - anyMap(), - eq(phaseLatencyMap), - anyLong() - ); + verify(queryInsightsService, times(numRequests)).addRecord(any()); + } + + public void testSetEnabled() { + when(queryInsightsService.isCollectionEnabled(MetricType.LATENCY)).thenReturn(true); + QueryInsightsListener queryInsightsListener = new QueryInsightsListener(clusterService, queryInsightsService); + queryInsightsListener.setEnabled(MetricType.LATENCY, true); + assertTrue(queryInsightsListener.isEnabled()); + + when(queryInsightsService.isCollectionEnabled(MetricType.LATENCY)).thenReturn(false); + when(queryInsightsService.isCollectionEnabled(MetricType.CPU)).thenReturn(false); + when(queryInsightsService.isCollectionEnabled(MetricType.JVM)).thenReturn(false); + queryInsightsListener.setEnabled(MetricType.LATENCY, false); + assertFalse(queryInsightsListener.isEnabled()); } } diff --git a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/service/TopQueriesByLatencyServiceTests.java b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/service/TopQueriesByLatencyServiceTests.java deleted file mode 100644 index 4ccb04e618f1a..0000000000000 --- a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/service/TopQueriesByLatencyServiceTests.java +++ /dev/null @@ -1,259 +0,0 @@ -/* - * 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; - -import org.opensearch.client.Client; -import org.opensearch.cluster.coordination.DeterministicTaskQueue; -import org.opensearch.cluster.service.ClusterService; -import org.opensearch.common.settings.ClusterSettings; -import org.opensearch.common.settings.Settings; -import org.opensearch.common.unit.TimeValue; -import org.opensearch.node.Node; -import org.opensearch.plugin.insights.QueryInsightsTestUtils; -import org.opensearch.plugin.insights.core.exporter.QueryInsightsExporterType; -import org.opensearch.plugin.insights.core.exporter.QueryInsightsLocalIndexExporter; -import org.opensearch.plugin.insights.rules.model.SearchQueryLatencyRecord; -import org.opensearch.plugin.insights.settings.QueryInsightsSettings; -import org.opensearch.test.OpenSearchTestCase; -import org.opensearch.threadpool.ThreadPool; -import org.junit.After; -import org.junit.Before; - -import java.util.List; -import java.util.concurrent.TimeUnit; - -import static org.mockito.Mockito.mock; - -/** - * Unit Tests for {@link TopQueriesByLatencyService}. - */ -public class TopQueriesByLatencyServiceTests extends OpenSearchTestCase { - - private DeterministicTaskQueue deterministicTaskQueue; - private ThreadPool threadPool; - private TopQueriesByLatencyService topQueriesByLatencyService; - - @Before - public void setup() { - final Client client = mock(Client.class); - final Settings settings = Settings.builder().put(Node.NODE_NAME_SETTING.getKey(), "top n queries tests").build(); - deterministicTaskQueue = new DeterministicTaskQueue(settings, random()); - threadPool = deterministicTaskQueue.getThreadPool(); - ClusterSettings clusterSettings = new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); - clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_ENABLED); - clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_SIZE); - clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_WINDOW_SIZE); - clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_EXPORTER_ENABLED); - clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_EXPORTER_TYPE); - clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_EXPORTER_INTERVAL); - clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_EXPORTER_IDENTIFIER); - ClusterService clusterService = new ClusterService(settings, clusterSettings, threadPool); - topQueriesByLatencyService = new TopQueriesByLatencyService(threadPool, clusterService, client); - topQueriesByLatencyService.setEnableCollect(true); - topQueriesByLatencyService.setTopNSize(Integer.MAX_VALUE); - topQueriesByLatencyService.setWindowSize(new TimeValue(Long.MAX_VALUE)); - } - - @After - public void shutdown() { - topQueriesByLatencyService.stop(); - } - - public void testIngestQueryDataWithLargeWindow() { - final List records = QueryInsightsTestUtils.generateQueryInsightRecords(10); - for (SearchQueryLatencyRecord record : records) { - topQueriesByLatencyService.ingestQueryData( - record.getTimestamp(), - record.getSearchType(), - record.getSource(), - record.getTotalShards(), - record.getIndices(), - record.getPropertyMap(), - record.getPhaseLatencyMap(), - record.getValue() - ); - } - runUntilTimeoutOrFinish(deterministicTaskQueue, 5000); - assertTrue(QueryInsightsTestUtils.checkRecordsEqualsWithoutOrder(topQueriesByLatencyService.getQueryData(), records)); - } - - public void testConcurrentIngestQueryDataWithLargeWindow() { - final List records = QueryInsightsTestUtils.generateQueryInsightRecords(10); - - int numRequests = records.size(); - for (int i = 0; i < numRequests; i++) { - int finalI = i; - threadPool.schedule(() -> { - topQueriesByLatencyService.ingestQueryData( - records.get(finalI).getTimestamp(), - records.get(finalI).getSearchType(), - records.get(finalI).getSource(), - records.get(finalI).getTotalShards(), - records.get(finalI).getIndices(), - records.get(finalI).getPropertyMap(), - records.get(finalI).getPhaseLatencyMap(), - records.get(finalI).getValue() - ); - }, TimeValue.ZERO, ThreadPool.Names.GENERIC); - } - runUntilTimeoutOrFinish(deterministicTaskQueue, 5000); - assertTrue(QueryInsightsTestUtils.checkRecordsEqualsWithoutOrder(topQueriesByLatencyService.getQueryData(), records)); - } - - public void testSmallWindowClearOutdatedData() { - final List records = QueryInsightsTestUtils.generateQueryInsightRecords(10); - topQueriesByLatencyService.setWindowSize(new TimeValue(-1)); - - for (SearchQueryLatencyRecord record : records) { - topQueriesByLatencyService.ingestQueryData( - record.getTimestamp(), - record.getSearchType(), - record.getSource(), - record.getTotalShards(), - record.getIndices(), - record.getPropertyMap(), - record.getPhaseLatencyMap(), - record.getValue() - ); - } - runUntilTimeoutOrFinish(deterministicTaskQueue, 5000); - assertEquals(0, topQueriesByLatencyService.getQueryData().size()); - } - - public void testSmallNSize() { - final List records = QueryInsightsTestUtils.generateQueryInsightRecords(10); - topQueriesByLatencyService.setTopNSize(1); - - for (SearchQueryLatencyRecord record : records) { - topQueriesByLatencyService.ingestQueryData( - record.getTimestamp(), - record.getSearchType(), - record.getSource(), - record.getTotalShards(), - record.getIndices(), - record.getPropertyMap(), - record.getPhaseLatencyMap(), - record.getValue() - ); - } - runUntilTimeoutOrFinish(deterministicTaskQueue, 5000); - assertEquals(1, topQueriesByLatencyService.getQueryData().size()); - } - - public void testIngestQueryDataWithInvalidData() { - final SearchQueryLatencyRecord record = QueryInsightsTestUtils.generateQueryInsightRecords(1).get(0); - topQueriesByLatencyService.ingestQueryData( - -1L, - record.getSearchType(), - record.getSource(), - record.getTotalShards(), - record.getIndices(), - record.getPropertyMap(), - record.getPhaseLatencyMap(), - record.getValue() - ); - assertEquals(0, topQueriesByLatencyService.getQueryData().size()); - - topQueriesByLatencyService.ingestQueryData( - record.getTimestamp(), - record.getSearchType(), - record.getSource(), - -1, - record.getIndices(), - record.getPropertyMap(), - record.getPhaseLatencyMap(), - record.getValue() - ); - assertEquals(0, topQueriesByLatencyService.getQueryData().size()); - - } - - public void testValidateTopNSize() { - assertThrows( - IllegalArgumentException.class, - () -> { topQueriesByLatencyService.validateTopNSize(QueryInsightsSettings.MAX_N_SIZE + 1); } - ); - } - - public void testValidateWindowSize() { - assertThrows(IllegalArgumentException.class, () -> { - topQueriesByLatencyService.validateWindowSize( - new TimeValue(QueryInsightsSettings.MAX_WINDOW_SIZE.getSeconds() + 1, TimeUnit.SECONDS) - ); - }); - } - - public void testValidateInterval() { - assertThrows(IllegalArgumentException.class, () -> { - topQueriesByLatencyService.validateExportInterval( - new TimeValue(QueryInsightsSettings.MIN_EXPORT_INTERVAL.getSeconds() - 1, TimeUnit.SECONDS) - ); - }); - } - - public void testSetExporterTypeWhenDisabled() { - topQueriesByLatencyService.setExporterEnabled(false); - topQueriesByLatencyService.setExporterType(QueryInsightsExporterType.LOCAL_INDEX); - assertNull(topQueriesByLatencyService.exporter); - } - - public void testSetExporterTypeWhenEnabled() { - topQueriesByLatencyService.setExporterEnabled(true); - topQueriesByLatencyService.setExporterType(QueryInsightsExporterType.LOCAL_INDEX); - assertEquals(QueryInsightsLocalIndexExporter.class, topQueriesByLatencyService.exporter.getClass()); - } - - public void testSetExporterEnabled() { - topQueriesByLatencyService.setExporterEnabled(true); - assertEquals(QueryInsightsLocalIndexExporter.class, topQueriesByLatencyService.exporter.getClass()); - } - - public void testSetExporterDisabled() { - topQueriesByLatencyService.setExporterEnabled(false); - assertNull(topQueriesByLatencyService.exporter); - } - - public void testChangeIdentifierWhenEnabled() { - topQueriesByLatencyService.setExporterEnabled(true); - topQueriesByLatencyService.setExporterIdentifier("changed"); - assertEquals("changed", topQueriesByLatencyService.exporter.getIdentifier()); - } - - public void testChangeIdentifierWhenDisabled() { - topQueriesByLatencyService.setExporterEnabled(false); - topQueriesByLatencyService.setExporterIdentifier("changed"); - assertNull(topQueriesByLatencyService.exporter); - } - - public void testChangeIntervalWhenEnabled() { - topQueriesByLatencyService.setExporterEnabled(true); - TimeValue newInterval = TimeValue.timeValueMillis(randomLongBetween(1, 9999)); - topQueriesByLatencyService.setExportInterval(newInterval); - assertEquals(newInterval, topQueriesByLatencyService.getExportInterval()); - } - - public void testChangeIntervalWhenDisabled() { - topQueriesByLatencyService.setExporterEnabled(false); - TimeValue newInterval = TimeValue.timeValueMillis(randomLongBetween(1, 9999)); - topQueriesByLatencyService.setExportInterval(newInterval); - assertNull(topQueriesByLatencyService.exporter); - } - - private static void runUntilTimeoutOrFinish(DeterministicTaskQueue deterministicTaskQueue, long duration) { - final long endTime = deterministicTaskQueue.getCurrentTimeMillis() + duration; - while (deterministicTaskQueue.getCurrentTimeMillis() < endTime - && (deterministicTaskQueue.hasRunnableTasks() || deterministicTaskQueue.hasDeferredTasks())) { - if (deterministicTaskQueue.hasDeferredTasks() && randomBoolean()) { - deterministicTaskQueue.advanceTime(); - } else if (deterministicTaskQueue.hasRunnableTasks()) { - deterministicTaskQueue.runRandomTask(); - } - } - } -} diff --git a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesRequestTests.java b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesRequestTests.java index 07e98970fb628..619fd4b33a3dc 100644 --- a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesRequestTests.java +++ b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesRequestTests.java @@ -10,6 +10,7 @@ import org.opensearch.common.io.stream.BytesStreamOutput; import org.opensearch.core.common.io.stream.StreamInput; +import org.opensearch.plugin.insights.rules.model.MetricType; import org.opensearch.test.OpenSearchTestCase; /** @@ -21,8 +22,7 @@ public class TopQueriesRequestTests extends OpenSearchTestCase { * Check that we can set the metric type */ public void testSetMetricType() throws Exception { - TopQueriesRequest request = new TopQueriesRequest(randomAlphaOfLength(5)); - request.setMetricType(randomFrom(TopQueriesRequest.Metric.allMetrics())); + TopQueriesRequest request = new TopQueriesRequest(MetricType.LATENCY, randomAlphaOfLength(5)); TopQueriesRequest deserializedRequest = roundTripRequest(request); assertEquals(request.getMetricType(), deserializedRequest.getMetricType()); } diff --git a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesResponseTests.java b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesResponseTests.java index 11063cbedd248..eeee50d3da703 100644 --- a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesResponseTests.java +++ b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesResponseTests.java @@ -10,11 +10,18 @@ import org.opensearch.cluster.ClusterName; import org.opensearch.common.io.stream.BytesStreamOutput; +import org.opensearch.core.common.bytes.BytesReference; import org.opensearch.core.common.io.stream.StreamInput; +import org.opensearch.core.xcontent.MediaTypeRegistry; +import org.opensearch.core.xcontent.ToXContent; +import org.opensearch.core.xcontent.XContentBuilder; import org.opensearch.plugin.insights.QueryInsightsTestUtils; +import org.opensearch.plugin.insights.rules.model.MetricType; import org.opensearch.test.OpenSearchTestCase; +import java.io.IOException; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; /** @@ -25,14 +32,29 @@ public class TopQueriesResponseTests extends OpenSearchTestCase { /** * Check serialization and deserialization */ - public void testToXContent() throws Exception { - TopQueries topQueries = QueryInsightsTestUtils.createTopQueries(); + public void testSerialize() throws Exception { + TopQueries topQueries = QueryInsightsTestUtils.createRandomTopQueries(); ClusterName clusterName = new ClusterName("test-cluster"); - TopQueriesResponse response = new TopQueriesResponse(clusterName, List.of(topQueries), new ArrayList<>(), 10); + TopQueriesResponse response = new TopQueriesResponse(clusterName, List.of(topQueries), new ArrayList<>(), 10, MetricType.LATENCY); TopQueriesResponse deserializedResponse = roundTripResponse(response); assertEquals(response.toString(), deserializedResponse.toString()); } + public void testToXContent() throws IOException { + char[] expectedXcontent = + "{\"top_queries\":[{\"timestamp\":1706574180000,\"node_id\":\"node_for_top_queries_test\",\"search_type\":\"query_then_fetch\",\"latency\":1}]}" + .toCharArray(); + TopQueries topQueries = QueryInsightsTestUtils.createFixedTopQueries(); + ClusterName clusterName = new ClusterName("test-cluster"); + TopQueriesResponse response = new TopQueriesResponse(clusterName, List.of(topQueries), new ArrayList<>(), 10, MetricType.LATENCY); + XContentBuilder builder = MediaTypeRegistry.contentBuilder(MediaTypeRegistry.JSON); + char[] xContent = BytesReference.bytes(response.toXContent(builder, ToXContent.EMPTY_PARAMS)).utf8ToString().toCharArray(); + Arrays.sort(expectedXcontent); + Arrays.sort(xContent); + + assertEquals(Arrays.hashCode(expectedXcontent), Arrays.hashCode(xContent)); + } + /** * Serialize and deserialize a TopQueriesResponse. * @param response A response to serialize. diff --git a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesTests.java b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesTests.java index 4d0d641cf1a8d..7db08b53ad1df 100644 --- a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesTests.java +++ b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesTests.java @@ -21,22 +21,15 @@ public class TopQueriesTests extends OpenSearchTestCase { public void testTopQueries() throws IOException { - TopQueries topQueries = QueryInsightsTestUtils.createTopQueries(); + TopQueries topQueries = QueryInsightsTestUtils.createRandomTopQueries(); try (BytesStreamOutput out = new BytesStreamOutput()) { topQueries.writeTo(out); try (StreamInput in = out.bytes().streamInput()) { TopQueries readTopQueries = new TopQueries(in); - assertExpected(topQueries, readTopQueries); + assertTrue( + QueryInsightsTestUtils.checkRecordsEquals(topQueries.getTopQueriesRecord(), readTopQueries.getTopQueriesRecord()) + ); } } } - - /** - * checks all properties that are expected to be unchanged. - */ - private void assertExpected(TopQueries topQueries, TopQueries readTopQueries) throws IOException { - for (int i = 0; i < topQueries.getLatencyRecords().size(); i++) { - QueryInsightsTestUtils.compareJson(topQueries.getLatencyRecords().get(i), readTopQueries.getLatencyRecords().get(i)); - } - } } diff --git a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/model/SearchQueryLatencyRecordTests.java b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/model/SearchQueryLatencyRecordTests.java deleted file mode 100644 index e704e768a43e6..0000000000000 --- a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/model/SearchQueryLatencyRecordTests.java +++ /dev/null @@ -1,50 +0,0 @@ -/* - * 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.rules.model; - -import org.opensearch.common.io.stream.BytesStreamOutput; -import org.opensearch.core.common.io.stream.StreamInput; -import org.opensearch.plugin.insights.QueryInsightsTestUtils; -import org.opensearch.test.OpenSearchTestCase; - -import java.util.ArrayList; -import java.util.List; - -/** - * Granular tests for the {@link SearchQueryLatencyRecord} class. - */ -public class SearchQueryLatencyRecordTests extends OpenSearchTestCase { - - /** - * Check that if the serialization, deserialization and equals functions are working as expected - */ - public void testSerializationAndEquals() throws Exception { - List records = QueryInsightsTestUtils.generateQueryInsightRecords(10); - List copiedRecords = new ArrayList<>(); - for (SearchQueryLatencyRecord record : records) { - copiedRecords.add(roundTripRecord(record)); - } - assertTrue(QueryInsightsTestUtils.checkRecordsEquals(records, copiedRecords)); - - } - - /** - * Serialize and deserialize a SearchQueryLatencyRecord. - * @param record A SearchQueryLatencyRecord to serialize. - * @return The deserialized, "round-tripped" record. - */ - private static SearchQueryLatencyRecord roundTripRecord(SearchQueryLatencyRecord record) throws Exception { - try (BytesStreamOutput out = new BytesStreamOutput()) { - record.writeTo(out); - try (StreamInput in = out.bytes().streamInput()) { - return new SearchQueryLatencyRecord(in); - } - } - } -} diff --git a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/transport/top_queries/TransportTopQueriesActionTests.java b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/transport/top_queries/TransportTopQueriesActionTests.java index 96b8ec63b5a47..a5f36b6e8cce0 100644 --- a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/transport/top_queries/TransportTopQueriesActionTests.java +++ b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/transport/top_queries/TransportTopQueriesActionTests.java @@ -12,9 +12,10 @@ import org.opensearch.cluster.service.ClusterService; import org.opensearch.common.settings.ClusterSettings; import org.opensearch.common.settings.Settings; -import org.opensearch.plugin.insights.core.service.TopQueriesByLatencyService; +import org.opensearch.plugin.insights.core.service.QueryInsightsService; import org.opensearch.plugin.insights.rules.action.top_queries.TopQueriesRequest; import org.opensearch.plugin.insights.rules.action.top_queries.TopQueriesResponse; +import org.opensearch.plugin.insights.rules.model.MetricType; import org.opensearch.plugin.insights.settings.QueryInsightsSettings; import org.opensearch.test.OpenSearchTestCase; import org.opensearch.threadpool.ThreadPool; @@ -34,7 +35,7 @@ public class TransportTopQueriesActionTests extends OpenSearchTestCase { private final ClusterSettings clusterSettings = new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); private final ClusterService clusterService = new ClusterService(settings, clusterSettings, threadPool); private final TransportService transportService = mock(TransportService.class); - private final TopQueriesByLatencyService topQueriesByLatencyService = mock(TopQueriesByLatencyService.class); + private final QueryInsightsService topQueriesByLatencyService = mock(QueryInsightsService.class); private final ActionFilters actionFilters = mock(ActionFilters.class); private final TransportTopQueriesAction transportTopQueriesAction = new TransportTopQueriesAction( threadPool, @@ -56,14 +57,14 @@ public DummyParentAction( ThreadPool threadPool, ClusterService clusterService, TransportService transportService, - TopQueriesByLatencyService topQueriesByLatencyService, + QueryInsightsService topQueriesByLatencyService, ActionFilters actionFilters ) { super(threadPool, clusterService, transportService, topQueriesByLatencyService, actionFilters); } public TopQueriesResponse createNewResponse() { - TopQueriesRequest request = new TopQueriesRequest(); + TopQueriesRequest request = new TopQueriesRequest(MetricType.LATENCY); return newResponse(request, List.of(), List.of()); } } @@ -73,10 +74,6 @@ public void setup() { clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_ENABLED); clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_SIZE); clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_WINDOW_SIZE); - clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_EXPORTER_ENABLED); - clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_EXPORTER_TYPE); - clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_EXPORTER_INTERVAL); - clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_EXPORTER_IDENTIFIER); } public void testNewResponse() { From 409fc92901b9af3f908136275e6b66c8e98b105a Mon Sep 17 00:00:00 2001 From: Chenyang Ji Date: Sat, 3 Feb 2024 00:15:40 -0800 Subject: [PATCH 4/5] refactor service for improving multithreading efficiency Signed-off-by: Chenyang Ji --- .../QueryInsightsPluginTransportIT.java | 19 +++--- .../core/listener/QueryInsightsListener.java | 60 ++++++++++--------- .../rules/action/top_queries/TopQueries.java | 13 ++-- .../action/top_queries/TopQueriesRequest.java | 16 ++--- .../top_queries/TopQueriesResponse.java | 33 +++++----- .../top_queries/RestTopQueriesAction.java | 17 ++---- .../TransportTopQueriesAction.java | 36 ++++++----- .../listener/QueryInsightsListenerTests.java | 9 ++- 8 files changed, 102 insertions(+), 101 deletions(-) diff --git a/plugins/query-insights/src/internalClusterTest/java/org/opensearch/plugin/insights/QueryInsightsPluginTransportIT.java b/plugins/query-insights/src/internalClusterTest/java/org/opensearch/plugin/insights/QueryInsightsPluginTransportIT.java index c16a2edbdf3de..711faa9323520 100644 --- a/plugins/query-insights/src/internalClusterTest/java/org/opensearch/plugin/insights/QueryInsightsPluginTransportIT.java +++ b/plugins/query-insights/src/internalClusterTest/java/org/opensearch/plugin/insights/QueryInsightsPluginTransportIT.java @@ -81,7 +81,7 @@ public void testGetTopQueriesWhenFeatureDisabled() { TopQueriesResponse response = OpenSearchIntegTestCase.client().execute(TopQueriesAction.INSTANCE, request).actionGet(); Assert.assertNotEquals(0, response.failures().size()); Assert.assertEquals( - "Cannot get query data when query insight feature is not enabled for MetricType [latency].", + "Cannot get top n queries when [search.top_n_queries.latency.enabled] is not enabled.", response.failures().get(0).getCause().getCause().getMessage() ); } @@ -89,7 +89,7 @@ public void testGetTopQueriesWhenFeatureDisabled() { /** * Test update top query record when feature enabled */ - public void testUpdateRecordWhenFeatureEnabled() throws ExecutionException, InterruptedException { + public void testUpdateRecordWhenFeatureDisabledThenEnabled() throws ExecutionException, InterruptedException { Settings commonSettings = Settings.builder().put(TOP_N_LATENCY_QUERIES_ENABLED.getKey(), "false").build(); logger.info("--> starting nodes for query insight testing"); @@ -121,7 +121,7 @@ public void testUpdateRecordWhenFeatureEnabled() throws ExecutionException, Inte TopQueriesResponse response = OpenSearchIntegTestCase.client().execute(TopQueriesAction.INSTANCE, request).actionGet(); Assert.assertNotEquals(0, response.failures().size()); Assert.assertEquals( - "Cannot get query data when query insight feature is not enabled for MetricType [latency].", + "Cannot get top n queries when [search.top_n_queries.latency.enabled] is not enabled.", response.failures().get(0).getCause().getCause().getMessage() ); @@ -143,7 +143,7 @@ public void testUpdateRecordWhenFeatureEnabled() throws ExecutionException, Inte /** * Test get top queries when feature enabled */ - public void testGetTopQueriesWhenFeatureEnabled() { + public void testGetTopQueriesWhenFeatureEnabled() throws InterruptedException { Settings commonSettings = Settings.builder() .put(TOP_N_LATENCY_QUERIES_ENABLED.getKey(), "true") .put(TOP_N_LATENCY_QUERIES_SIZE.getKey(), "100") @@ -174,7 +174,8 @@ public void testGetTopQueriesWhenFeatureEnabled() { .get(); assertEquals(searchResponse.getFailedShards(), 0); } - + // Sleep to wait for queue drained to top queries store + Thread.sleep(6000); TopQueriesRequest request = new TopQueriesRequest(MetricType.LATENCY); TopQueriesResponse response = OpenSearchIntegTestCase.client().execute(TopQueriesAction.INSTANCE, request).actionGet(); Assert.assertEquals(0, response.failures().size()); @@ -187,7 +188,7 @@ public void testGetTopQueriesWhenFeatureEnabled() { /** * Test get top queries with small top n size */ - public void testGetTopQueriesWithSmallTopN() { + public void testGetTopQueriesWithSmallTopN() throws InterruptedException { Settings commonSettings = Settings.builder() .put(TOP_N_LATENCY_QUERIES_ENABLED.getKey(), "true") .put(TOP_N_LATENCY_QUERIES_SIZE.getKey(), "1") @@ -218,7 +219,7 @@ public void testGetTopQueriesWithSmallTopN() { .get(); assertEquals(searchResponse.getFailedShards(), 0); } - + Thread.sleep(6000); TopQueriesRequest request = new TopQueriesRequest(MetricType.LATENCY); TopQueriesResponse response = OpenSearchIntegTestCase.client().execute(TopQueriesAction.INSTANCE, request).actionGet(); Assert.assertEquals(0, response.failures().size()); @@ -231,7 +232,7 @@ public void testGetTopQueriesWithSmallTopN() { /** * Test get top queries with small window size */ - public void testGetTopQueriesWithSmallWindowSize() { + public void testGetTopQueriesWithSmallWindowSize() throws InterruptedException { Settings commonSettings = Settings.builder() .put(TOP_N_LATENCY_QUERIES_ENABLED.getKey(), "true") .put(TOP_N_LATENCY_QUERIES_SIZE.getKey(), "100") @@ -267,7 +268,7 @@ public void testGetTopQueriesWithSmallWindowSize() { TopQueriesResponse response = OpenSearchIntegTestCase.client().execute(TopQueriesAction.INSTANCE, request).actionGet(); Assert.assertEquals(0, response.failures().size()); Assert.assertEquals(TOTAL_NUMBER_OF_NODES, response.getNodes().size()); - + Thread.sleep(6000); internalCluster().stopAllNodes(); } } diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/listener/QueryInsightsListener.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/listener/QueryInsightsListener.java index 3bc8215ec19e5..705273f52a567 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/listener/QueryInsightsListener.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/listener/QueryInsightsListener.java @@ -19,7 +19,6 @@ import org.opensearch.core.xcontent.ToXContent; import org.opensearch.plugin.insights.core.service.QueryInsightsService; import org.opensearch.plugin.insights.rules.model.Attribute; -import org.opensearch.plugin.insights.rules.model.Measurement; import org.opensearch.plugin.insights.rules.model.MetricType; import org.opensearch.plugin.insights.rules.model.SearchQueryRecord; @@ -34,7 +33,9 @@ import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.TOP_N_LATENCY_QUERIES_WINDOW_SIZE; /** - * The listener for top N queries by latency + * The listener for query insights services. + * It forwards query-related data to the appropriate query insights stores, + * either for each request or for each phase. * * @opensearch.internal */ @@ -52,47 +53,55 @@ public final class QueryInsightsListener extends SearchRequestOperationsListener * @param queryInsightsService The topQueriesByLatencyService associated with this listener */ @Inject - public QueryInsightsListener(ClusterService clusterService, QueryInsightsService queryInsightsService) { + public QueryInsightsListener(final ClusterService clusterService, final QueryInsightsService queryInsightsService) { this.queryInsightsService = queryInsightsService; clusterService.getClusterSettings() - .addSettingsUpdateConsumer(TOP_N_LATENCY_QUERIES_ENABLED, v -> this.setEnabled(MetricType.LATENCY, v)); + .addSettingsUpdateConsumer(TOP_N_LATENCY_QUERIES_ENABLED, v -> this.setEnableTopQueries(MetricType.LATENCY, v)); clusterService.getClusterSettings() .addSettingsUpdateConsumer( TOP_N_LATENCY_QUERIES_SIZE, - this.queryInsightsService::setTopNSize, - this.queryInsightsService::validateTopNSize + v -> this.queryInsightsService.getTopQueriesService(MetricType.LATENCY).setTopNSize(v), + v -> this.queryInsightsService.getTopQueriesService(MetricType.LATENCY).validateTopNSize(v) ); clusterService.getClusterSettings() .addSettingsUpdateConsumer( TOP_N_LATENCY_QUERIES_WINDOW_SIZE, - this.queryInsightsService::setWindowSize, - this.queryInsightsService::validateWindowSize + v -> this.queryInsightsService.getTopQueriesService(MetricType.LATENCY).setWindowSize(v), + v -> this.queryInsightsService.getTopQueriesService(MetricType.LATENCY).validateWindowSize(v) ); - this.setEnabled(MetricType.LATENCY, clusterService.getClusterSettings().get(TOP_N_LATENCY_QUERIES_ENABLED)); - this.queryInsightsService.setTopNSize(clusterService.getClusterSettings().get(TOP_N_LATENCY_QUERIES_SIZE)); - this.queryInsightsService.setWindowSize(clusterService.getClusterSettings().get(TOP_N_LATENCY_QUERIES_WINDOW_SIZE)); + this.setEnableTopQueries(MetricType.LATENCY, clusterService.getClusterSettings().get(TOP_N_LATENCY_QUERIES_ENABLED)); + this.queryInsightsService.getTopQueriesService(MetricType.LATENCY) + .setTopNSize(clusterService.getClusterSettings().get(TOP_N_LATENCY_QUERIES_SIZE)); + this.queryInsightsService.getTopQueriesService(MetricType.LATENCY) + .setWindowSize(clusterService.getClusterSettings().get(TOP_N_LATENCY_QUERIES_WINDOW_SIZE)); } /** - * Enable or disable metric collection for {@link MetricType} + * Enable or disable top queries insights collection for {@link MetricType} + * This function will enable or disable the corresponding listeners + * and query insights services. * * @param metricType {@link MetricType} * @param enabled boolean */ - public void setEnabled(MetricType metricType, boolean enabled) { + public void setEnableTopQueries(final MetricType metricType, final boolean enabled) { + boolean isAllMetricsDisabled = !queryInsightsService.isEnabled(); this.queryInsightsService.enableCollection(metricType, enabled); - - // disable QueryInsightsListener only if collection for all metrics are disabled. if (!enabled) { - for (MetricType t : MetricType.allMetricTypes()) { - if (this.queryInsightsService.isCollectionEnabled(t)) { - return; - } + // disable QueryInsightsListener only if all metrics collections are disabled now. + if (!queryInsightsService.isEnabled()) { + super.setEnabled(false); + this.queryInsightsService.stop(); } - super.setEnabled(false); } else { super.setEnabled(true); + // restart QueryInsightsListener only if none of metrics collections is enabled before. + if (isAllMetricsDisabled) { + this.queryInsightsService.stop(); + this.queryInsightsService.start(); + } } + } @Override @@ -113,17 +122,14 @@ public void onPhaseFailure(SearchPhaseContext context) {} public void onRequestStart(SearchRequestContext searchRequestContext) {} @Override - public void onRequestEnd(SearchPhaseContext context, SearchRequestContext searchRequestContext) { - SearchRequest request = context.getRequest(); + public void onRequestEnd(final SearchPhaseContext context, final SearchRequestContext searchRequestContext) { + final SearchRequest request = context.getRequest(); try { - Map> measurements = new HashMap<>(); + Map measurements = new HashMap<>(); if (queryInsightsService.isCollectionEnabled(MetricType.LATENCY)) { measurements.put( MetricType.LATENCY, - new Measurement<>( - MetricType.LATENCY.name(), - TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - searchRequestContext.getAbsoluteStartNanos()) - ) + TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - searchRequestContext.getAbsoluteStartNanos()) ); } Map attributes = new HashMap<>(); diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueries.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueries.java index 640a0b82260b5..26cff82aae52e 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueries.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueries.java @@ -20,14 +20,13 @@ import java.util.List; /** - * Top Queries by resource usage / latency on a node - *

+ * Holds all top queries records by resource usage or latency on a node * Mainly used in the top N queries node response workflow. * * @opensearch.internal */ public class TopQueries extends BaseNodeResponse implements ToXContentObject { - /** The store to keep the top N queries records */ + /** The store to keep the top queries records */ private final List topQueriesRecords; /** @@ -35,7 +34,7 @@ public class TopQueries extends BaseNodeResponse implements ToXContentObject { * @param in A {@link StreamInput} object. * @throws IOException IOException */ - public TopQueries(StreamInput in) throws IOException { + public TopQueries(final StreamInput in) throws IOException { super(in); topQueriesRecords = in.readList(SearchQueryRecord::new); } @@ -45,13 +44,13 @@ public TopQueries(StreamInput in) throws IOException { * @param node A node that is part of the cluster. * @param searchQueryRecords A list of SearchQueryRecord associated in this TopQueries. */ - public TopQueries(DiscoveryNode node, List searchQueryRecords) { + public TopQueries(final DiscoveryNode node, final List searchQueryRecords) { super(node); topQueriesRecords = searchQueryRecords; } @Override - public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { + public XContentBuilder toXContent(final XContentBuilder builder, final Params params) throws IOException { if (topQueriesRecords != null) { for (SearchQueryRecord record : topQueriesRecords) { record.toXContent(builder, params); @@ -61,7 +60,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws } @Override - public void writeTo(StreamOutput out) throws IOException { + public void writeTo(final StreamOutput out) throws IOException { super.writeTo(out); out.writeList(topQueriesRecords); diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesRequest.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesRequest.java index 27177fef25bea..3bdff2c403161 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesRequest.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesRequest.java @@ -9,7 +9,6 @@ package org.opensearch.plugin.insights.rules.action.top_queries; import org.opensearch.action.support.nodes.BaseNodesRequest; -import org.opensearch.common.annotation.PublicApi; import org.opensearch.core.common.io.stream.StreamInput; import org.opensearch.core.common.io.stream.StreamOutput; import org.opensearch.plugin.insights.rules.model.MetricType; @@ -21,10 +20,9 @@ * * @opensearch.internal */ -@PublicApi(since = "1.0.0") public class TopQueriesRequest extends BaseNodesRequest { - MetricType metricType; + final MetricType metricType; /** * Constructor for TopQueriesRequest @@ -32,13 +30,9 @@ public class TopQueriesRequest extends BaseNodesRequest { * @param in A {@link StreamInput} object. * @throws IOException if the stream cannot be deserialized. */ - public TopQueriesRequest(StreamInput in) throws IOException { + public TopQueriesRequest(final StreamInput in) throws IOException { super(in); - MetricType metricType = MetricType.readFromStream(in); - if (false == MetricType.allMetricTypes().contains(metricType)) { - throw new IllegalStateException("Invalid metric used in top queries request: " + metricType); - } - this.metricType = metricType; + this.metricType = MetricType.readFromStream(in); } /** @@ -48,7 +42,7 @@ public TopQueriesRequest(StreamInput in) throws IOException { * @param metricType {@link MetricType} * @param nodesIds the nodeIds specified in the request */ - public TopQueriesRequest(MetricType metricType, String... nodesIds) { + public TopQueriesRequest(final MetricType metricType, final String... nodesIds) { super(nodesIds); this.metricType = metricType; } @@ -61,7 +55,7 @@ public MetricType getMetricType() { } @Override - public void writeTo(StreamOutput out) throws IOException { + public void writeTo(final StreamOutput out) throws IOException { super.writeTo(out); out.writeString(metricType.toString()); } diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesResponse.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesResponse.java index fe7644de5629f..2e66bb7f77baf 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesResponse.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesResponse.java @@ -11,7 +11,6 @@ import org.opensearch.action.FailedNodeException; import org.opensearch.action.support.nodes.BaseNodesResponse; import org.opensearch.cluster.ClusterName; -import org.opensearch.common.annotation.PublicApi; import org.opensearch.common.xcontent.XContentFactory; import org.opensearch.core.common.io.stream.StreamInput; import org.opensearch.core.common.io.stream.StreamOutput; @@ -31,7 +30,6 @@ * * @opensearch.internal */ -@PublicApi(since = "1.0.0") public class TopQueriesResponse extends BaseNodesResponse implements ToXContentFragment { private static final String CLUSTER_LEVEL_RESULTS_KEY = "top_queries"; @@ -44,7 +42,7 @@ public class TopQueriesResponse extends BaseNodesResponse implements * @param in A {@link StreamInput} object. * @throws IOException if the stream cannot be deserialized. */ - public TopQueriesResponse(StreamInput in) throws IOException { + public TopQueriesResponse(final StreamInput in) throws IOException { super(in); top_n_size = in.readInt(); metricType = in.readEnum(MetricType.class); @@ -60,11 +58,11 @@ public TopQueriesResponse(StreamInput in) throws IOException { * @param metricType the {@link MetricType} to be returned in this response */ public TopQueriesResponse( - ClusterName clusterName, - List nodes, - List failures, - int top_n_size, - MetricType metricType + final ClusterName clusterName, + final List nodes, + final List failures, + final int top_n_size, + final MetricType metricType ) { super(clusterName, nodes, failures); this.top_n_size = top_n_size; @@ -72,20 +70,20 @@ public TopQueriesResponse( } @Override - protected List readNodesFrom(StreamInput in) throws IOException { + protected List readNodesFrom(final StreamInput in) throws IOException { return in.readList(TopQueries::new); } @Override - protected void writeNodesTo(StreamOutput out, List nodes) throws IOException { + protected void writeNodesTo(final StreamOutput out, final List nodes) throws IOException { out.writeList(nodes); out.writeLong(top_n_size); out.writeEnum(metricType); } @Override - public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { - List results = getNodes(); + public XContentBuilder toXContent(final XContentBuilder builder, final Params params) throws IOException { + final List results = getNodes(); postProcess(results); builder.startObject(); toClusterLevelResult(builder, params, results); @@ -95,7 +93,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws @Override public String toString() { try { - XContentBuilder builder = XContentFactory.jsonBuilder().prettyPrint(); + final XContentBuilder builder = XContentFactory.jsonBuilder().prettyPrint(); builder.startObject(); this.toXContent(builder, EMPTY_PARAMS); builder.endObject(); @@ -110,9 +108,9 @@ public String toString() { * * @param results the top queries results */ - private void postProcess(List results) { + private void postProcess(final List results) { for (TopQueries topQueries : results) { - String nodeId = topQueries.getNode().getId(); + final String nodeId = topQueries.getNode().getId(); for (SearchQueryRecord record : topQueries.getTopQueriesRecord()) { record.addAttribute(Attribute.NODE_ID, nodeId); } @@ -127,8 +125,9 @@ private void postProcess(List results) { * @param results top queries results from all nodes * @throws IOException if an error occurs */ - private void toClusterLevelResult(XContentBuilder builder, Params params, List results) throws IOException { - List all_records = results.stream() + private void toClusterLevelResult(final XContentBuilder builder, final Params params, final List results) + throws IOException { + final List all_records = results.stream() .map(TopQueries::getTopQueriesRecord) .flatMap(Collection::stream) .sorted((a, b) -> SearchQueryRecord.compare(a, b, metricType) * -1) diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/resthandler/top_queries/RestTopQueriesAction.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/resthandler/top_queries/RestTopQueriesAction.java index 654aab6eb1563..6aa511c626ab1 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/resthandler/top_queries/RestTopQueriesAction.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/resthandler/top_queries/RestTopQueriesAction.java @@ -33,7 +33,7 @@ import static org.opensearch.rest.RestRequest.Method.GET; /** - * Transport action to get Top N queries by certain metric type + * Rest action to get Top N queries by certain metric type * * @opensearch.api */ @@ -64,17 +64,12 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC final TopQueriesRequest topQueriesRequest = prepareRequest(request); topQueriesRequest.timeout(request.param("timeout")); - return channel -> client.execute( - TopQueriesAction.INSTANCE, - topQueriesRequest, - topQueriesResponse(channel) - - ); + return channel -> client.execute(TopQueriesAction.INSTANCE, topQueriesRequest, topQueriesResponse(channel)); } static TopQueriesRequest prepareRequest(final RestRequest request) { - String[] nodesIds = Strings.splitStringByCommaToArray(request.param("nodeId")); - String metricType = request.param("type", MetricType.LATENCY.toString()); + final String[] nodesIds = Strings.splitStringByCommaToArray(request.param("nodeId")); + final String metricType = request.param("type", MetricType.LATENCY.toString()); if (!ALLOWED_METRICS.contains(metricType)) { throw new IllegalArgumentException( String.format(Locale.ROOT, "request [%s] contains invalid metric type [%s]", request.path(), metricType) @@ -93,10 +88,10 @@ public boolean canTripCircuitBreaker() { return false; } - private RestResponseListener topQueriesResponse(RestChannel channel) { + private RestResponseListener topQueriesResponse(final RestChannel channel) { return new RestResponseListener<>(channel) { @Override - public RestResponse buildResponse(TopQueriesResponse response) throws Exception { + public RestResponse buildResponse(final TopQueriesResponse response) throws Exception { return new BytesRestResponse(RestStatus.OK, response.toXContent(channel.newBuilder(), ToXContent.EMPTY_PARAMS)); } }; diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/transport/top_queries/TransportTopQueriesAction.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/transport/top_queries/TransportTopQueriesAction.java index a4ddaca0a6cdc..ddf614211bc41 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/transport/top_queries/TransportTopQueriesAction.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/transport/top_queries/TransportTopQueriesAction.java @@ -55,11 +55,11 @@ public class TransportTopQueriesAction extends TransportNodesAction< */ @Inject public TransportTopQueriesAction( - ThreadPool threadPool, - ClusterService clusterService, - TransportService transportService, - QueryInsightsService queryInsightsService, - ActionFilters actionFilters + final ThreadPool threadPool, + final ClusterService clusterService, + final TransportService transportService, + final QueryInsightsService queryInsightsService, + final ActionFilters actionFilters ) { super( TopQueriesAction.NAME, @@ -77,9 +77,9 @@ public TransportTopQueriesAction( @Override protected TopQueriesResponse newResponse( - TopQueriesRequest topQueriesRequest, - List responses, - List failures + final TopQueriesRequest topQueriesRequest, + final List responses, + final List failures ) { if (topQueriesRequest.getMetricType() == MetricType.LATENCY) { return new TopQueriesResponse( @@ -95,20 +95,23 @@ protected TopQueriesResponse newResponse( } @Override - protected NodeRequest newNodeRequest(TopQueriesRequest request) { + protected NodeRequest newNodeRequest(final TopQueriesRequest request) { return new NodeRequest(request); } @Override - protected TopQueries newNodeResponse(StreamInput in) throws IOException { + protected TopQueries newNodeResponse(final StreamInput in) throws IOException { return new TopQueries(in); } @Override - protected TopQueries nodeOperation(NodeRequest nodeRequest) { - TopQueriesRequest request = nodeRequest.request; + protected TopQueries nodeOperation(final NodeRequest nodeRequest) { + final TopQueriesRequest request = nodeRequest.request; if (request.getMetricType() == MetricType.LATENCY) { - return new TopQueries(clusterService.localNode(), queryInsightsService.getTopNRecords(MetricType.LATENCY, true)); + return new TopQueries( + clusterService.localNode(), + queryInsightsService.getTopQueriesService(MetricType.LATENCY).getTopQueriesRecords(true) + ); } else { throw new OpenSearchException(String.format(Locale.ROOT, "invalid metric type %s", request.getMetricType())); } @@ -122,10 +125,11 @@ protected TopQueries nodeOperation(NodeRequest nodeRequest) { */ public static class NodeRequest extends TransportRequest { - TopQueriesRequest request; + final TopQueriesRequest request; /** * Create the NodeResponse object from StreamInput + * * @param in the StreamInput to read the object * @throws IOException IOException */ @@ -138,12 +142,12 @@ public NodeRequest(StreamInput in) throws IOException { * Create the NodeResponse object from a TopQueriesRequest * @param request the TopQueriesRequest object */ - public NodeRequest(TopQueriesRequest request) { + public NodeRequest(final TopQueriesRequest request) { this.request = request; } @Override - public void writeTo(StreamOutput out) throws IOException { + public void writeTo(final StreamOutput out) throws IOException { super.writeTo(out); request.writeTo(out); } diff --git a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/listener/QueryInsightsListenerTests.java b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/listener/QueryInsightsListenerTests.java index a6368bb0e180f..f340950017a5c 100644 --- a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/listener/QueryInsightsListenerTests.java +++ b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/listener/QueryInsightsListenerTests.java @@ -16,6 +16,7 @@ import org.opensearch.common.settings.ClusterSettings; import org.opensearch.common.settings.Settings; import org.opensearch.plugin.insights.core.service.QueryInsightsService; +import org.opensearch.plugin.insights.core.service.TopQueriesService; import org.opensearch.plugin.insights.rules.model.MetricType; import org.opensearch.plugin.insights.settings.QueryInsightsSettings; import org.opensearch.search.aggregations.bucket.terms.TermsAggregationBuilder; @@ -45,6 +46,7 @@ public class QueryInsightsListenerTests extends OpenSearchTestCase { private final SearchPhaseContext searchPhaseContext = mock(SearchPhaseContext.class); private final SearchRequest searchRequest = mock(SearchRequest.class); private final QueryInsightsService queryInsightsService = mock(QueryInsightsService.class); + private final TopQueriesService topQueriesService = mock(TopQueriesService.class); private ClusterService clusterService; @Before @@ -57,9 +59,10 @@ public void setup() { clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_WINDOW_SIZE); clusterService = new ClusterService(settings, clusterSettings, null); when(queryInsightsService.isCollectionEnabled(MetricType.LATENCY)).thenReturn(true); + when(queryInsightsService.getTopQueriesService(MetricType.LATENCY)).thenReturn(topQueriesService); } - public void testOnRequestEnd() { + public void testOnRequestEnd() throws InterruptedException { Long timestamp = System.currentTimeMillis() - 100L; SearchType searchType = SearchType.QUERY_THEN_FETCH; @@ -146,13 +149,13 @@ public void testConcurrentOnRequestEnd() throws InterruptedException { public void testSetEnabled() { when(queryInsightsService.isCollectionEnabled(MetricType.LATENCY)).thenReturn(true); QueryInsightsListener queryInsightsListener = new QueryInsightsListener(clusterService, queryInsightsService); - queryInsightsListener.setEnabled(MetricType.LATENCY, true); + queryInsightsListener.setEnableTopQueries(MetricType.LATENCY, true); assertTrue(queryInsightsListener.isEnabled()); when(queryInsightsService.isCollectionEnabled(MetricType.LATENCY)).thenReturn(false); when(queryInsightsService.isCollectionEnabled(MetricType.CPU)).thenReturn(false); when(queryInsightsService.isCollectionEnabled(MetricType.JVM)).thenReturn(false); - queryInsightsListener.setEnabled(MetricType.LATENCY, false); + queryInsightsListener.setEnableTopQueries(MetricType.LATENCY, false); assertFalse(queryInsightsListener.isEnabled()); } } From c7c7ef637944b0e3e40338961510e6e192e1cd39 Mon Sep 17 00:00:00 2001 From: Chenyang Ji Date: Mon, 5 Feb 2024 18:23:31 -0800 Subject: [PATCH 5/5] rebase from master to pick up query insights plugin changes Signed-off-by: Chenyang Ji --- .../QueryInsightsPluginTransportIT.java | 4 +-- .../plugin/insights/QueryInsightsPlugin.java | 10 ++++-- .../insights/QueryInsightsPluginTests.java | 22 ++++++++++-- .../insights/QueryInsightsTestUtils.java | 34 +++++++++++++++++++ .../RestTopQueriesActionTests.java | 10 +++++- 5 files changed, 71 insertions(+), 9 deletions(-) diff --git a/plugins/query-insights/src/internalClusterTest/java/org/opensearch/plugin/insights/QueryInsightsPluginTransportIT.java b/plugins/query-insights/src/internalClusterTest/java/org/opensearch/plugin/insights/QueryInsightsPluginTransportIT.java index 711faa9323520..04e715444f50a 100644 --- a/plugins/query-insights/src/internalClusterTest/java/org/opensearch/plugin/insights/QueryInsightsPluginTransportIT.java +++ b/plugins/query-insights/src/internalClusterTest/java/org/opensearch/plugin/insights/QueryInsightsPluginTransportIT.java @@ -81,7 +81,7 @@ public void testGetTopQueriesWhenFeatureDisabled() { TopQueriesResponse response = OpenSearchIntegTestCase.client().execute(TopQueriesAction.INSTANCE, request).actionGet(); Assert.assertNotEquals(0, response.failures().size()); Assert.assertEquals( - "Cannot get top n queries when [search.top_n_queries.latency.enabled] is not enabled.", + "Cannot get top n queries for [latency] when it is not enabled.", response.failures().get(0).getCause().getCause().getMessage() ); } @@ -121,7 +121,7 @@ public void testUpdateRecordWhenFeatureDisabledThenEnabled() throws ExecutionExc TopQueriesResponse response = OpenSearchIntegTestCase.client().execute(TopQueriesAction.INSTANCE, request).actionGet(); Assert.assertNotEquals(0, response.failures().size()); Assert.assertEquals( - "Cannot get top n queries when [search.top_n_queries.latency.enabled] is not enabled.", + "Cannot get top n queries for [latency] when it is not enabled.", response.failures().get(0).getCause().getCause().getMessage() ); diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/QueryInsightsPlugin.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/QueryInsightsPlugin.java index 0acd7e2dcac0d..4d7e0d486068a 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/QueryInsightsPlugin.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/QueryInsightsPlugin.java @@ -25,7 +25,11 @@ import org.opensearch.core.xcontent.NamedXContentRegistry; import org.opensearch.env.Environment; import org.opensearch.env.NodeEnvironment; +import org.opensearch.plugin.insights.core.listener.QueryInsightsListener; import org.opensearch.plugin.insights.core.service.QueryInsightsService; +import org.opensearch.plugin.insights.rules.action.top_queries.TopQueriesAction; +import org.opensearch.plugin.insights.rules.resthandler.top_queries.RestTopQueriesAction; +import org.opensearch.plugin.insights.rules.transport.top_queries.TransportTopQueriesAction; import org.opensearch.plugin.insights.settings.QueryInsightsSettings; import org.opensearch.plugins.ActionPlugin; import org.opensearch.plugins.Plugin; @@ -67,7 +71,7 @@ public Collection createComponents( ) { // create top n queries service final QueryInsightsService queryInsightsService = new QueryInsightsService(threadPool); - return List.of(queryInsightsService); + return List.of(queryInsightsService, new QueryInsightsListener(clusterService, queryInsightsService)); } @Override @@ -92,12 +96,12 @@ public List getRestHandlers( final IndexNameExpressionResolver indexNameExpressionResolver, final Supplier nodesInCluster ) { - return List.of(); + return List.of(new RestTopQueriesAction()); } @Override public List> getActions() { - return List.of(); + return List.of(new ActionPlugin.ActionHandler<>(TopQueriesAction.INSTANCE, TransportTopQueriesAction.class)); } @Override diff --git a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/QueryInsightsPluginTests.java b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/QueryInsightsPluginTests.java index f6d95450855de..273b69e483e8c 100644 --- a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/QueryInsightsPluginTests.java +++ b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/QueryInsightsPluginTests.java @@ -14,11 +14,16 @@ import org.opensearch.common.settings.ClusterSettings; import org.opensearch.common.settings.Settings; import org.opensearch.core.action.ActionResponse; +import org.opensearch.plugin.insights.core.listener.QueryInsightsListener; import org.opensearch.plugin.insights.core.service.QueryInsightsService; +import org.opensearch.plugin.insights.rules.action.top_queries.TopQueriesAction; +import org.opensearch.plugin.insights.rules.resthandler.top_queries.RestTopQueriesAction; import org.opensearch.plugin.insights.settings.QueryInsightsSettings; import org.opensearch.plugins.ActionPlugin; import org.opensearch.rest.RestHandler; import org.opensearch.test.OpenSearchTestCase; +import org.opensearch.threadpool.ExecutorBuilder; +import org.opensearch.threadpool.ScalingExecutorBuilder; import org.opensearch.threadpool.ThreadPool; import org.junit.Before; @@ -74,18 +79,29 @@ public void testCreateComponent() { null, null ); - assertEquals(1, components.size()); + assertEquals(2, components.size()); assertTrue(components.get(0) instanceof QueryInsightsService); + assertTrue(components.get(1) instanceof QueryInsightsListener); + } + + public void testGetExecutorBuilders() { + Settings.Builder settingsBuilder = Settings.builder(); + Settings settings = settingsBuilder.build(); + List> executorBuilders = queryInsightsPlugin.getExecutorBuilders(settings); + assertEquals(1, executorBuilders.size()); + assertTrue(executorBuilders.get(0) instanceof ScalingExecutorBuilder); } public void testGetRestHandlers() { List components = queryInsightsPlugin.getRestHandlers(Settings.EMPTY, null, null, null, null, null, null); - assertEquals(0, components.size()); + assertEquals(1, components.size()); + assertTrue(components.get(0) instanceof RestTopQueriesAction); } public void testGetActions() { List> components = queryInsightsPlugin.getActions(); - assertEquals(0, components.size()); + assertEquals(1, components.size()); + assertTrue(components.get(0).getAction() instanceof TopQueriesAction); } } diff --git a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/QueryInsightsTestUtils.java b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/QueryInsightsTestUtils.java index 04f49d5fbc7dd..870ef5b9c8be9 100644 --- a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/QueryInsightsTestUtils.java +++ b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/QueryInsightsTestUtils.java @@ -9,12 +9,15 @@ package org.opensearch.plugin.insights; import org.opensearch.action.search.SearchType; +import org.opensearch.cluster.node.DiscoveryNode; import org.opensearch.common.util.Maps; import org.opensearch.core.xcontent.ToXContent; import org.opensearch.core.xcontent.XContentBuilder; +import org.opensearch.plugin.insights.rules.action.top_queries.TopQueries; import org.opensearch.plugin.insights.rules.model.Attribute; import org.opensearch.plugin.insights.rules.model.MetricType; import org.opensearch.plugin.insights.rules.model.SearchQueryRecord; +import org.opensearch.test.VersionUtils; import java.io.IOException; import java.util.ArrayList; @@ -26,7 +29,11 @@ import java.util.Set; import java.util.TreeSet; +import static java.util.Collections.emptyMap; +import static java.util.Collections.emptySet; import static org.opensearch.common.xcontent.XContentFactory.jsonBuilder; +import static org.opensearch.test.OpenSearchTestCase.buildNewFakeTransportAddress; +import static org.opensearch.test.OpenSearchTestCase.random; import static org.opensearch.test.OpenSearchTestCase.randomAlphaOfLengthBetween; import static org.opensearch.test.OpenSearchTestCase.randomArray; import static org.opensearch.test.OpenSearchTestCase.randomDouble; @@ -79,6 +86,33 @@ public static List generateQueryInsightRecords(int lower, int return records; } + public static TopQueries createRandomTopQueries() { + DiscoveryNode node = new DiscoveryNode( + "node_for_top_queries_test", + buildNewFakeTransportAddress(), + emptyMap(), + emptySet(), + VersionUtils.randomVersion(random()) + ); + List records = generateQueryInsightRecords(10); + + return new TopQueries(node, records); + } + + public static TopQueries createFixedTopQueries() { + DiscoveryNode node = new DiscoveryNode( + "node_for_top_queries_test", + buildNewFakeTransportAddress(), + emptyMap(), + emptySet(), + VersionUtils.randomVersion(random()) + ); + List records = new ArrayList<>(); + records.add(createFixedSearchQueryRecord()); + + return new TopQueries(node, records); + } + public static SearchQueryRecord createFixedSearchQueryRecord() { long timestamp = 1706574180000L; Map measurements = Map.of(MetricType.LATENCY, 1L); diff --git a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/resthandler/top_queries/RestTopQueriesActionTests.java b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/resthandler/top_queries/RestTopQueriesActionTests.java index 627d794731eed..ac19fa2a7348f 100644 --- a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/resthandler/top_queries/RestTopQueriesActionTests.java +++ b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/resthandler/top_queries/RestTopQueriesActionTests.java @@ -9,11 +9,13 @@ package org.opensearch.plugin.insights.rules.resthandler.top_queries; import org.opensearch.plugin.insights.rules.action.top_queries.TopQueriesRequest; +import org.opensearch.rest.RestHandler; import org.opensearch.rest.RestRequest; import org.opensearch.test.OpenSearchTestCase; import org.opensearch.test.rest.FakeRestRequest; import java.util.HashMap; +import java.util.List; import java.util.Locale; import java.util.Map; @@ -24,7 +26,6 @@ public class RestTopQueriesActionTests extends OpenSearchTestCase { public void testEmptyNodeIdsValidType() { Map params = new HashMap<>(); params.put("type", randomFrom(ALLOWED_METRICS)); - RestRequest restRequest = buildRestRequest(params); TopQueriesRequest actual = RestTopQueriesAction.prepareRequest(restRequest); assertEquals(0, actual.nodesIds().length); @@ -53,6 +54,13 @@ public void testInValidType() { ); } + public void testGetRoutes() { + RestTopQueriesAction action = new RestTopQueriesAction(); + List routes = action.routes(); + assertEquals(2, routes.size()); + assertEquals("query_insights_top_queries_action", action.getName()); + } + private FakeRestRequest buildRestRequest(Map params) { return new FakeRestRequest.Builder(xContentRegistry()).withMethod(RestRequest.Method.GET) .withPath("/_insights/top_queries")