From cd805a2ecf3eca6ba5ce407c2e18946bce5d772b Mon Sep 17 00:00:00 2001 From: Chenyang Ji Date: Mon, 29 Jan 2024 13:27:29 -0800 Subject: [PATCH] Refactor record and service to make them generic Signed-off-by: Chenyang Ji --- plugins/query-insights/build.gradle | 2 - .../QueryInsightsPluginTransportIT.java | 81 ++++- .../plugin/insights/QueryInsightsPlugin.java | 32 +- .../core/exporter/QueryInsightsExporter.java | 43 --- .../exporter/QueryInsightsExporterType.java | 35 -- .../QueryInsightsLocalIndexExporter.java | 187 ---------- .../insights/core/exporter/package-info.java | 12 - .../core/listener/QueryInsightsListener.java | 141 ++++++++ .../listener/SearchQueryLatencyListener.java | 127 ------- .../core/service/QueryInsightsService.java | 320 +++++++++++------- .../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 ++- .../insights/rules/model/Attribute.java | 70 ++++ .../insights/rules/model/Measurement.java | 114 +++++++ .../insights/rules/model/MetricType.java | 78 +++++ .../rules/model/SearchQueryLatencyRecord.java | 111 ------ .../rules/model/SearchQueryRecord.java | 268 +++++---------- .../top_queries/RestTopQueriesAction.java | 10 +- .../TransportTopQueriesAction.java | 20 +- .../settings/QueryInsightsSettings.java | 89 ++--- .../insights/QueryInsightsPluginTests.java | 18 +- .../insights/QueryInsightsTestUtils.java | 139 ++++---- .../exporter/QueryInsightsExporterTests.java | 35 -- .../QueryInsightsLocalIndexExporterTests.java | 256 -------------- ...s.java => QueryInsightsListenerTests.java} | 102 +++--- .../service/QueryInsightsServiceTests.java | 247 +++++++++----- .../TopQueriesByLatencyServiceTests.java | 259 -------------- .../top_queries/TopQueriesRequestTests.java | 4 +- .../top_queries/TopQueriesResponseTests.java | 28 +- .../action/top_queries/TopQueriesTests.java | 15 +- .../model/SearchQueryLatencyRecordTests.java | 50 --- .../rules/model/SearchQueryRecordTests.java | 71 ++++ .../TransportTopQueriesActionTests.java | 13 +- 36 files changed, 1287 insertions(+), 2137 deletions(-) delete mode 100644 plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/exporter/QueryInsightsExporter.java delete mode 100644 plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/exporter/QueryInsightsExporterType.java delete mode 100644 plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/exporter/QueryInsightsLocalIndexExporter.java delete mode 100644 plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/exporter/package-info.java 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 create mode 100644 plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model/Attribute.java create mode 100644 plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model/Measurement.java create mode 100644 plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model/MetricType.java delete mode 100644 plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model/SearchQueryLatencyRecord.java delete mode 100644 plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/exporter/QueryInsightsExporterTests.java delete mode 100644 plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/exporter/QueryInsightsLocalIndexExporterTests.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 create mode 100644 plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/model/SearchQueryRecordTests.java diff --git a/plugins/query-insights/build.gradle b/plugins/query-insights/build.gradle index f457502794c6c..eabbd395bd3bd 100644 --- a/plugins/query-insights/build.gradle +++ b/plugins/query-insights/build.gradle @@ -8,8 +8,6 @@ * Modifications Copyright OpenSearch Contributors. See * GitHub history for details. */ -apply plugin: 'opensearch.java-rest-test' -apply plugin: 'opensearch.internal-cluster-test' opensearchplugin { description 'OpenSearch Query Insights Plugin.' 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/QueryInsightsPlugin.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/QueryInsightsPlugin.java index 5a765b5e3c628..e35c90e86f41b 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 @@ -18,13 +18,15 @@ import org.opensearch.common.settings.Setting; import org.opensearch.common.settings.Settings; import org.opensearch.common.settings.SettingsFilter; +import org.opensearch.common.unit.TimeValue; +import org.opensearch.common.util.concurrent.OpenSearchExecutors; import org.opensearch.core.action.ActionResponse; import org.opensearch.core.common.io.stream.NamedWriteableRegistry; import org.opensearch.core.xcontent.NamedXContentRegistry; import org.opensearch.env.Environment; import org.opensearch.env.NodeEnvironment; -import org.opensearch.plugin.insights.core.listener.SearchQueryLatencyListener; -import org.opensearch.plugin.insights.core.service.TopQueriesByLatencyService; +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; @@ -35,6 +37,8 @@ import org.opensearch.rest.RestController; import org.opensearch.rest.RestHandler; import org.opensearch.script.ScriptService; +import org.opensearch.threadpool.ExecutorBuilder; +import org.opensearch.threadpool.ScalingExecutorBuilder; import org.opensearch.threadpool.ThreadPool; import org.opensearch.watcher.ResourceWatcherService; @@ -66,10 +70,20 @@ public Collection createComponents( Supplier repositoriesServiceSupplier ) { // create top n queries service - TopQueriesByLatencyService topQueriesByLatencyService = new TopQueriesByLatencyService(threadPool, clusterService, client); - // top n queries listener - SearchQueryLatencyListener searchQueryLatencyListener = new SearchQueryLatencyListener(clusterService, topQueriesByLatencyService); - return List.of(topQueriesByLatencyService, searchQueryLatencyListener); + QueryInsightsService queryInsightsService = new QueryInsightsService(threadPool); + return List.of(queryInsightsService, new QueryInsightsListener(clusterService, queryInsightsService)); + } + + @Override + public List> getExecutorBuilders(Settings settings) { + return List.of( + new ScalingExecutorBuilder( + QueryInsightsSettings.QUERY_INSIGHTS_EXECUTOR, + 1, + Math.min((OpenSearchExecutors.allocatedProcessors(settings) + 1) / 2, QueryInsightsSettings.MAX_THREAD_COUNT), + TimeValue.timeValueMinutes(5) + ) + ); } @Override @@ -96,11 +110,7 @@ public List> getSettings() { // Settings for top N queries QueryInsightsSettings.TOP_N_LATENCY_QUERIES_ENABLED, QueryInsightsSettings.TOP_N_LATENCY_QUERIES_SIZE, - QueryInsightsSettings.TOP_N_LATENCY_QUERIES_WINDOW_SIZE, - QueryInsightsSettings.TOP_N_LATENCY_QUERIES_EXPORTER_ENABLED, - QueryInsightsSettings.TOP_N_LATENCY_QUERIES_EXPORTER_TYPE, - QueryInsightsSettings.TOP_N_LATENCY_QUERIES_EXPORTER_INTERVAL, - QueryInsightsSettings.TOP_N_LATENCY_QUERIES_EXPORTER_IDENTIFIER + QueryInsightsSettings.TOP_N_LATENCY_QUERIES_WINDOW_SIZE ); } } diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/exporter/QueryInsightsExporter.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/exporter/QueryInsightsExporter.java deleted file mode 100644 index 5f53dbb6e39db..0000000000000 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/exporter/QueryInsightsExporter.java +++ /dev/null @@ -1,43 +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.exporter; - -import org.opensearch.plugin.insights.rules.model.SearchQueryRecord; - -import java.util.List; - -/** - * Simple abstract class to export data collected by search query analyzers - *

- * Mainly for use within the Query Insight framework - * - * @opensearch.internal - */ -public abstract class QueryInsightsExporter> { - private final String identifier; - - QueryInsightsExporter(String identifier) { - this.identifier = identifier; - } - - /** - * Export the data with the exporter. - * - * @param records the data to export - */ - public abstract void export(List records) throws Exception; - - /** - * Get the identifier of this exporter - * @return identifier of this exporter - */ - public String getIdentifier() { - return identifier; - } -} diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/exporter/QueryInsightsExporterType.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/exporter/QueryInsightsExporterType.java deleted file mode 100644 index 7c2a9b86fb642..0000000000000 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/exporter/QueryInsightsExporterType.java +++ /dev/null @@ -1,35 +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.exporter; - -import java.util.Locale; - -/** - * Types for the Query Insights Exporters - * - * @opensearch.internal - */ -public enum QueryInsightsExporterType { - /** local index exporter */ - LOCAL_INDEX; - - @Override - public String toString() { - return super.toString().toLowerCase(Locale.ROOT); - } - - /** - * Parse QueryInsightsExporterType from String - * @param type the String representation of the QueryInsightsExporterType - * @return QueryInsightsExporterType - */ - public static QueryInsightsExporterType parse(String type) { - return valueOf(type.toUpperCase(Locale.ROOT)); - } -} diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/exporter/QueryInsightsLocalIndexExporter.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/exporter/QueryInsightsLocalIndexExporter.java deleted file mode 100644 index d6317971f1d52..0000000000000 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/exporter/QueryInsightsLocalIndexExporter.java +++ /dev/null @@ -1,187 +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.exporter; - -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; -import org.opensearch.action.admin.indices.create.CreateIndexRequest; -import org.opensearch.action.admin.indices.create.CreateIndexResponse; -import org.opensearch.action.bulk.BulkRequest; -import org.opensearch.action.bulk.BulkResponse; -import org.opensearch.action.index.IndexRequest; -import org.opensearch.action.support.WriteRequest; -import org.opensearch.client.Client; -import org.opensearch.cluster.ClusterState; -import org.opensearch.cluster.service.ClusterService; -import org.opensearch.common.settings.Settings; -import org.opensearch.common.unit.TimeValue; -import org.opensearch.common.xcontent.XContentFactory; -import org.opensearch.core.action.ActionListener; -import org.opensearch.core.rest.RestStatus; -import org.opensearch.core.xcontent.ToXContent; -import org.opensearch.plugin.insights.rules.model.SearchQueryRecord; - -import java.io.IOException; -import java.io.InputStream; -import java.nio.charset.Charset; -import java.util.List; -import java.util.Locale; -import java.util.Objects; - -/** - * Class to export data collected by search query analyzers to a local OpenSearch index - *

- * Internal used within the Query Insight framework - * - * @opensearch.internal - */ -public class QueryInsightsLocalIndexExporter> extends QueryInsightsExporter { - private static final Logger log = LogManager.getLogger(QueryInsightsLocalIndexExporter.class); - private static final int INDEX_TIMEOUT = 60; - - private final ClusterService clusterService; - private final Client client; - - /** The mapping for the local index that holds the data */ - private final InputStream localIndexMapping; - - /** - * Create a QueryInsightsLocalIndexExporter Object - * @param clusterService The clusterService of the node - * @param client The OpenSearch Client to support index operations - * @param localIndexName The local index name to export the data to - * @param localIndexMapping The mapping for the local index - */ - public QueryInsightsLocalIndexExporter( - ClusterService clusterService, - Client client, - String localIndexName, - InputStream localIndexMapping - ) { - super(localIndexName); - this.clusterService = clusterService; - this.client = client; - this.localIndexMapping = localIndexMapping; - } - - /** - * Export the data to the predefined local OpenSearch Index - * - * @param records the data to export - * @throws IOException if an error occurs - */ - @Override - public void export(List records) throws IOException { - if (records.size() == 0) { - return; - } - boolean indexExists = checkAndInitLocalIndex(new ActionListener<>() { - @Override - public void onResponse(CreateIndexResponse response) { - if (response.isAcknowledged()) { - log.debug(String.format(Locale.ROOT, "successfully initialized local index %s for query insight.", getIdentifier())); - try { - bulkRecord(records); - } catch (IOException e) { - log.error(String.format(Locale.ROOT, "fail to ingest query insight data to local index, error: %s", e)); - } - } else { - log.error( - String.format(Locale.ROOT, "request to created local index %s for query insight not acknowledged.", getIdentifier()) - ); - } - } - - @Override - public void onFailure(Exception e) { - log.error(String.format(Locale.ROOT, "error creating local index for query insight: %s", e)); - } - }); - - if (indexExists) { - bulkRecord(records); - } - } - - /** - * Util function to check if a local OpenSearch Index exists - * - * @return boolean - */ - private boolean checkIfIndexExists() { - ClusterState clusterState = clusterService.state(); - return clusterState.getRoutingTable().hasIndex(this.getIdentifier()); - } - - /** - * Check and initialize the local OpenSearch Index for the exporter - * - * @param listener the listener to be notified upon completion - * @return boolean to represent if the index has already been created before calling this function - * @throws IOException if an error occurs - */ - private synchronized boolean checkAndInitLocalIndex(ActionListener listener) throws IOException { - if (!checkIfIndexExists()) { - CreateIndexRequest createIndexRequest = new CreateIndexRequest(this.getIdentifier()).mapping(getIndexMappings()) - .settings(Settings.builder().put("index.hidden", false).build()); - client.admin().indices().create(createIndexRequest, listener); - return false; - } else { - return true; - } - } - - /** - * Get the index mapping of the local index - * - * @return String to represent the index mapping - * @throws IOException if an error occurs - */ - private String getIndexMappings() throws IOException { - return new String(Objects.requireNonNull(this.localIndexMapping).readAllBytes(), Charset.defaultCharset()); - } - - /** - * Bulk ingest the data into to the predefined local OpenSearch Index - * - * @param records the data to export - * @throws IOException if an error occurs - */ - private void bulkRecord(List records) throws IOException { - BulkRequest bulkRequest = new BulkRequest().setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE) - .timeout(TimeValue.timeValueSeconds(INDEX_TIMEOUT)); - for (T record : records) { - bulkRequest.add( - new IndexRequest(getIdentifier()).source(record.toXContent(XContentFactory.jsonBuilder(), ToXContent.EMPTY_PARAMS)) - ); - } - client.bulk(bulkRequest, new ActionListener<>() { - @Override - public void onResponse(BulkResponse response) { - if (response.status().equals(RestStatus.CREATED) || response.status().equals(RestStatus.OK)) { - log.debug(String.format(Locale.ROOT, "successfully ingest data for %s! ", getIdentifier())); - } else { - log.error( - String.format( - Locale.ROOT, - "error when ingesting data for %s, error: %s", - getIdentifier(), - response.buildFailureMessage() - ) - ); - } - } - - @Override - public void onFailure(Exception e) { - log.error(String.format(Locale.ROOT, "failed to ingest data for %s, %s", getIdentifier(), e)); - } - }); - } -} diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/exporter/package-info.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/exporter/package-info.java deleted file mode 100644 index 3ccbfc5c4cb24..0000000000000 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/exporter/package-info.java +++ /dev/null @@ -1,12 +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. - */ - -/** - * Exporters for Query Insights - */ -package org.opensearch.plugin.insights.core.exporter; 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/QueryInsightsService.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/QueryInsightsService.java index bf97707c7f3bd..579130d2cfefc 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/QueryInsightsService.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/QueryInsightsService.java @@ -10,207 +10,291 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -import org.opensearch.common.Nullable; import org.opensearch.common.inject.Inject; -import org.opensearch.common.lifecycle.AbstractLifecycleComponent; 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.rules.model.MetricType; import org.opensearch.plugin.insights.rules.model.SearchQueryRecord; import org.opensearch.plugin.insights.settings.QueryInsightsSettings; -import org.opensearch.threadpool.Scheduler; import org.opensearch.threadpool.ThreadPool; +import java.time.Instant; +import java.time.LocalDateTime; +import java.time.ZoneId; +import java.time.ZoneOffset; +import java.time.temporal.ChronoUnit; import java.util.ArrayList; import java.util.Collection; -import java.util.Collections; +import java.util.HashMap; import java.util.List; import java.util.Locale; +import java.util.Map; +import java.util.concurrent.PriorityBlockingQueue; +import java.util.stream.Collectors; +import java.util.stream.Stream; /** - * Service responsible for gathering, analyzing, storing and exporting data related to - * search queries, based on certain dimensions. - * - * @param The type of record that stores in the service - * @param The type of Collection that holds the aggregated data - * @param The type of exporter that exports the aggregated and processed data + * Service responsible for gathering, analyzing, storing and exporting + * top N queries with high latency data for search queries * * @opensearch.internal */ -public abstract class QueryInsightsService, S extends Collection, E extends QueryInsightsExporter> - extends AbstractLifecycleComponent { +public class QueryInsightsService { private static final Logger log = LogManager.getLogger(QueryInsightsService.class); - /** enable insight data collection */ - private boolean enableCollect; - - /** enable insight data export */ - private boolean enableExport; - /** The internal thread-safe store that holds the query insight data */ - @Nullable - protected S store; + private static final TimeValue delay = TimeValue.ZERO; + /** + * The internal OpenSearch thread pool that execute async processing and exporting tasks + */ + private final ThreadPool threadPool; - /** The exporter that exports the query insight data to certain sink */ - @Nullable - protected E exporter; + /** + * enable insight data collection + */ + private final Map enableCollect = new HashMap<>(); - /** The export interval of this exporter, default to 1 day */ - protected TimeValue exportInterval = QueryInsightsSettings.MIN_EXPORT_INTERVAL; + private int topNSize = QueryInsightsSettings.DEFAULT_TOP_N_SIZE; - /** The internal OpenSearch thread pool that execute async processing and exporting tasks*/ - protected final ThreadPool threadPool; + private TimeValue windowSize = TimeValue.timeValueSeconds(QueryInsightsSettings.DEFAULT_WINDOW_SIZE); + /** + * The internal thread-safe store that holds the top n queries insight data, by different MetricType + */ + private final Map> topQueriesStores; /** - * Holds a reference to delayed operation {@link Scheduler.Cancellable} so it can be cancelled when - * the service closed concurrently. + * The internal store that holds historical top n queries insight data by different MetricType in the last window */ - protected volatile Scheduler.Cancellable scheduledFuture; + private final Map> topQueriesHistoryStores; + /** + * window start timestamp for each top queries collector + */ + private final Map topQueriesWindowStart; /** - * Create the Query Insights Service object - * @param threadPool The OpenSearch thread pool to run async tasks - * @param store The in memory store to keep the Query Insights data - * @param exporter The optional {@link QueryInsightsExporter} to export the Query Insights data + * Create the TopQueriesByLatencyService Object + * + * @param threadPool The OpenSearch thread pool to run async tasks */ @Inject - public QueryInsightsService(ThreadPool threadPool, @Nullable S store, @Nullable E exporter) { + public QueryInsightsService(ThreadPool threadPool) { + topQueriesStores = new HashMap<>(); + topQueriesHistoryStores = new HashMap<>(); + topQueriesWindowStart = new HashMap<>(); + for (MetricType metricType : MetricType.allMetricTypes()) { + topQueriesStores.put(metricType, new PriorityBlockingQueue<>(topNSize, (a, b) -> SearchQueryRecord.compare(a, b, metricType))); + topQueriesHistoryStores.put(metricType, new ArrayList<>()); + topQueriesWindowStart.put(metricType, -1L); + } this.threadPool = threadPool; - this.store = store; - this.exporter = exporter; } /** - * Ingest one record to the query insight store + * Ingest the query data into in-memory stores * * @param record the record to ingest */ - protected void ingestQueryData(R record) { - if (enableCollect && this.store != null) { - this.store.add(record); + public void addRecord(SearchQueryRecord record) { + for (MetricType metricType : record.getMeasurements().keySet()) { + this.threadPool.schedule(() -> { + if (!topQueriesStores.containsKey(metricType)) { + return; + } + // add the record to corresponding priority queues to calculate top n queries insights + PriorityBlockingQueue store = topQueriesStores.get(metricType); + checkAndResetWindow(metricType, record.getTimestamp()); + if (record.getTimestamp() > topQueriesWindowStart.get(metricType)) { + store.add(record); + // remove top elements for fix sizing priority queue + if (store.size() > this.getTopNSize()) { + store.poll(); + } + } + }, delay, QueryInsightsSettings.QUERY_INSIGHTS_EXECUTOR); + } + } + + private synchronized void checkAndResetWindow(MetricType metricType, Long timestamp) { + Long windowStart = calculateWindowStart(timestamp); + // reset window if the current window is outdated + if (topQueriesWindowStart.get(metricType) < windowStart) { + // rotate the current window to history store only if the data belongs to the last window + if (topQueriesWindowStart.get(metricType) == windowStart - windowSize.getMillis()) { + topQueriesHistoryStores.put(metricType, new ArrayList<>(topQueriesStores.get(metricType))); + } else { + topQueriesHistoryStores.get(metricType).clear(); + } + topQueriesStores.get(metricType).clear(); + topQueriesWindowStart.put(metricType, windowStart); } } /** - * Get all records that are in the query insight store, + * Get all top queries records that are in the current query insight store, based on the input MetricType + * Optionally include top N records from the last window. + * * By default, return the records in sorted order. * + * @param metricType {@link MetricType} + * @param includeLastWindow if the top N queries from the last window should be included * @return List of the records that are in the query insight store * @throws IllegalArgumentException if query insight is disabled in the cluster */ - public List getQueryData() throws IllegalArgumentException { - if (!enableCollect) { - throw new IllegalArgumentException("Cannot get query data when query insight feature is not enabled."); + public List getTopNRecords(MetricType metricType, boolean includeLastWindow) throws IllegalArgumentException { + if (!enableCollect.containsKey(metricType) || !enableCollect.get(metricType)) { + throw new IllegalArgumentException( + String.format( + Locale.ROOT, + "Cannot get query data when query insight feature is not enabled for MetricType [%s].", + metricType + ) + ); } - clearOutdatedData(); - List queries = new ArrayList<>(store); - queries.sort(Collections.reverseOrder()); - return queries; + checkAndResetWindow(metricType, System.currentTimeMillis()); + List queries = new ArrayList<>(topQueriesStores.get(metricType)); + if (includeLastWindow) { + queries.addAll(topQueriesHistoryStores.get(metricType)); + } + return Stream.of(queries) + .flatMap(Collection::stream) + .sorted((a, b) -> SearchQueryRecord.compare(a, b, metricType) * -1) + .collect(Collectors.toList()); } /** - * Clear all outdated data in the store - */ - public abstract void clearOutdatedData(); - - /** - * Reset the exporter with new config + * Set flag to enable or disable Query Insights data collection * - * 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 + * @param metricType {@link MetricType} + * @param enable Flag to enable or disable Query Insights data collection */ - public abstract void resetExporter(boolean enabled, QueryInsightsExporterType type, String identifier); - - /** - * Clear all data in the store - */ - public void clearAllData() { - store.clear(); + public void enableCollection(MetricType metricType, boolean enable) { + this.enableCollect.put(metricType, enable); + // set topQueriesWindowStart to enable top n queries collection + if (enable) { + topQueriesWindowStart.put(metricType, calculateWindowStart(System.currentTimeMillis())); + } } /** - * Set flag to enable or disable Query Insights data collection - * @param enableCollect Flag to enable or disable Query Insights data collection + * Get if the Query Insights data collection is enabled for a MetricType + * + * @param metricType {@link MetricType} + * @return if the Query Insights data collection is enabled */ - public void setEnableCollect(boolean enableCollect) { - this.enableCollect = enableCollect; + public boolean isCollectionEnabled(MetricType metricType) { + return this.enableCollect.containsKey(metricType) && this.enableCollect.get(metricType); } /** - * Get if the Query Insights data collection is enabled - * @return if the Query Insights data collection is enabled + * Set the top N size for TopQueriesByLatencyService service. + * + * @param size the top N size to set */ - public boolean getEnableCollect() { - return this.enableCollect; + public void setTopNSize(int size) { + this.topNSize = size; } /** - * Set flag to enable or disable Query Insights data export - * @param enableExport + * Validate the top N size based on the internal constrains + * + * @param size the wanted top N size */ - public void setEnableExport(boolean enableExport) { - this.enableExport = enableExport; + 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 if the Query Insights data export is enabled - * @return if the Query Insights data export is enabled + * Get the top N size set for TopQueriesByLatencyService + * + * @return the top N size */ - public boolean getEnableExport() { - return this.enableExport; + public int getTopNSize() { + return this.topNSize; } /** - * Start the Query Insight Service. + * Set the window size for TopQueriesByLatencyService + * + * @param windowSize window size to set */ - @Override - protected void doStart() { - if (exporter != null && getEnableExport()) { - scheduledFuture = threadPool.scheduleWithFixedDelay(this::doExport, exportInterval, ThreadPool.Names.GENERIC); + public void setWindowSize(TimeValue windowSize) { + this.windowSize = windowSize; + for (MetricType metricType : MetricType.allMetricTypes()) { + topQueriesWindowStart.put(metricType, -1L); } } /** - * Stop the Query Insight Service + * Validate if the window size is valid, based on internal constrains. + * + * @param windowSize the window size to validate */ - @Override - protected void doStop() { - if (scheduledFuture != null) { - scheduledFuture.cancel(); - if (exporter != null && getEnableExport()) { - doExport(); - } + public void validateWindowSize(TimeValue windowSize) { + if (windowSize.compareTo(QueryInsightsSettings.MAX_WINDOW_SIZE) > 0 + || windowSize.compareTo(QueryInsightsSettings.MIN_WINDOW_SIZE) < 0) { + throw new IllegalArgumentException( + "Window size setting [" + + QueryInsightsSettings.TOP_N_LATENCY_QUERIES_WINDOW_SIZE.getKey() + + "]" + + " should be between [" + + QueryInsightsSettings.MAX_WINDOW_SIZE + + "," + + QueryInsightsSettings.MAX_WINDOW_SIZE + + "]" + + "was (" + + windowSize + + ")" + ); } - } - - private void doExport() { - List storedData = getQueryData(); - try { - exporter.export(storedData); - } catch (Exception e) { - throw new RuntimeException(String.format(Locale.ROOT, "failed to export query insight data to sink, error: %s", e)); + if (!(QueryInsightsSettings.VALID_WINDOW_SIZES_IN_MINUTES.contains(windowSize) || windowSize.getMinutes() % 60 == 0)) { + throw new IllegalArgumentException( + "Window size setting [" + + QueryInsightsSettings.TOP_N_LATENCY_QUERIES_WINDOW_SIZE.getKey() + + "]" + + " should be multiple of 1 hour, or one of " + + QueryInsightsSettings.VALID_WINDOW_SIZES_IN_MINUTES + + ", was (" + + windowSize + + ")" + ); } } - @Override - protected void doClose() {} - /** - * Get the export interval set for the {@link QueryInsightsExporter} - * @return export interval + * Get the size of top N queries store + * @param metricType {@link MetricType} + * @return top N queries store size */ - public TimeValue getExportInterval() { - return exportInterval; + public int getTopNStoreSize(MetricType metricType) { + return topQueriesStores.get(metricType).size(); } /** - * Set the export interval for the exporter. - * - * @param interval export interval + * Get the size of top N queries history store + * @param metricType {@link MetricType} + * @return top N queries history store size */ - public void setExportInterval(TimeValue interval) { - this.exportInterval = interval; + public int getTopNHistoryStoreSize(MetricType metricType) { + return topQueriesHistoryStores.get(metricType).size(); + } + + private Long calculateWindowStart(Long timestamp) { + LocalDateTime currentTime = LocalDateTime.ofInstant(Instant.ofEpochMilli(timestamp), ZoneId.of("UTC")); + LocalDateTime windowStartTime = currentTime.truncatedTo(ChronoUnit.HOURS); + while (!windowStartTime.plusMinutes(windowSize.getMinutes()).isAfter(currentTime)) { + windowStartTime = windowStartTime.plusMinutes(windowSize.getMinutes()); + } + return windowStartTime.toInstant(ZoneOffset.UTC).getEpochSecond() * 1000; } } 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/model/Attribute.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model/Attribute.java new file mode 100644 index 0000000000000..cb798dd6ed1f4 --- /dev/null +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model/Attribute.java @@ -0,0 +1,70 @@ +/* + * 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.core.common.io.stream.StreamInput; +import org.opensearch.core.common.io.stream.StreamOutput; + +import java.io.IOException; +import java.util.Locale; + +/** + * Valid attributes for a search query record + */ +public enum Attribute { + /** + * The search query type + */ + SEARCH_TYPE, + /** + * The search query source + */ + SOURCE, + /** + * Total shards queried + */ + TOTAL_SHARDS, + /** + * The indices involved + */ + INDICES, + /** + * The per phase level latency map for a search query + */ + PHASE_LATENCY_MAP, + /** + * The node id for this request + */ + NODE_ID; + + /** + * Read an Attribute from a StreamInput + * @param in the StreamInput to read from + * @return Attribute + * @throws IOException IOException + */ + public static Attribute readFromStream(StreamInput in) throws IOException { + return Attribute.valueOf(in.readString().toUpperCase(Locale.ROOT)); + } + + /** + * Write Attribute to a StreamOutput + * @param out the StreamOutput to write + * @param attribute the Attribute to write + * @throws IOException IOException + */ + public static void writeTo(StreamOutput out, Attribute attribute) throws IOException { + out.writeString(attribute.toString()); + } + + @Override + public String toString() { + return this.name().toLowerCase(Locale.ROOT); + } +} diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model/Measurement.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model/Measurement.java new file mode 100644 index 0000000000000..5fc27ac745146 --- /dev/null +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model/Measurement.java @@ -0,0 +1,114 @@ +/* + * 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.core.common.io.stream.StreamInput; +import org.opensearch.core.common.io.stream.StreamOutput; + +import java.io.IOException; +import java.util.Locale; +import java.util.Objects; + +/** + * Represents a measurement with a name and a corresponding value. + * + * @param the type of the value + */ +public class Measurement> { + String name; + T value; + + /** + * Create a Measurement from a StreamInput + * @param in the StreamInput to read from + * @throws IOException IOException + */ + @SuppressWarnings("unchecked") + public Measurement(StreamInput in) throws IOException { + this.name = in.readString(); + this.value = (T) in.readGenericValue(); + } + + /** + * Write Measurement to a StreamOutput + * @param out the StreamOutput to write + * @param measurement the Measurement to write + * @throws IOException IOException + */ + public static void writeTo(StreamOutput out, Measurement measurement) throws IOException { + out.writeString(measurement.getName()); + out.writeGenericValue(measurement.getValue()); + } + + /** + * Constructs a new Measurement with input name and value. + * + * @param name the name of the measurement + * @param value the value of the measurement + */ + public Measurement(String name, T value) { + this.name = name; + this.value = value; + } + + /** + * Returns the name of the measurement. + * + * @return the name of the measurement + */ + public String getName() { + return name; + } + + /** + * Returns the value of the measurement. + * + * @return the value of the measurement + */ + public T getValue() { + return value; + } + + /** + * Compare two measurements on the value + * @param other the other measure to compare + * @return 0 if the value that the two measurements holds are the same + * -1 if the value of the measurement is smaller than the other one + * 1 if the value of the measurement is greater than the other one + */ + @SuppressWarnings("unchecked") + public int compareTo(Measurement other) { + Number otherValue = other.getValue(); + if (value.getClass().equals(otherValue.getClass())) { + return value.compareTo((T) otherValue); + } else { + throw new UnsupportedOperationException( + String.format( + Locale.ROOT, + "comparison between different types are not supported : %s, %s", + value.getClass(), + otherValue.getClass() + ) + ); + } + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + Measurement other = (Measurement) o; + return Objects.equals(name, other.name) && Objects.equals(value, other.value); + } + + @Override + public int hashCode() { + return Objects.hash(name, value); + } +} diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model/MetricType.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model/MetricType.java new file mode 100644 index 0000000000000..2b03bec7a722f --- /dev/null +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model/MetricType.java @@ -0,0 +1,78 @@ +/* + * 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.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; + +/** + * Valid metric types for a search query record + */ +public enum MetricType { + /** + * Latency metric type + */ + LATENCY, + /** + * CPU usage metric type + */ + CPU, + /** + * JVM heap usage metric type + */ + JVM; + + /** + * Read a MetricType from a StreamInput + * @param in the StreamInput to read from + * @return MetricType + * @throws IOException IOException + */ + public static MetricType readFromStream(StreamInput in) throws IOException { + return fromString(in.readString()); + } + + /** + * Create MetricType from String + * @param metricType the String representation of MetricType + * @return MetricType + */ + public static MetricType fromString(String metricType) { + return MetricType.valueOf(metricType.toUpperCase(Locale.ROOT)); + } + + /** + * Write MetricType to a StreamOutput + * @param out the StreamOutput to write + * @param metricType the MetricType to write + * @throws IOException IOException + */ + public static void writeTo(StreamOutput out, MetricType metricType) throws IOException { + out.writeString(metricType.toString()); + } + + @Override + public String toString() { + return this.name().toLowerCase(Locale.ROOT); + } + + /** + * Get all valid metrics + * @return A set of String that contains all valid metrics + */ + public static Set allMetricTypes() { + return Arrays.stream(values()).collect(Collectors.toSet()); + } +} diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model/SearchQueryLatencyRecord.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model/SearchQueryLatencyRecord.java deleted file mode 100644 index 97b423e092673..0000000000000 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model/SearchQueryLatencyRecord.java +++ /dev/null @@ -1,111 +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.action.search.SearchType; -import org.opensearch.core.common.io.stream.StreamInput; -import org.opensearch.core.common.io.stream.StreamOutput; -import org.opensearch.core.xcontent.XContentBuilder; - -import java.io.IOException; -import java.util.Map; - -/** - * The Latency record stored in the Query Insight Framework - * - * @opensearch.internal - */ -public final class SearchQueryLatencyRecord extends SearchQueryRecord { - - private static final String PHASE_LATENCY_MAP = "phaseLatencyMap"; - private static final String TOOK = "tookInNs"; - - // latency info for each search phase - private final Map phaseLatencyMap; - - /** - * Constructor for SearchQueryLatencyRecord - * - * @param in A {@link StreamInput} object. - * @throws IOException if the stream cannot be deserialized. - */ - public SearchQueryLatencyRecord(final StreamInput in) throws IOException { - super(in); - this.phaseLatencyMap = in.readMap(StreamInput::readString, StreamInput::readLong); - this.setValue(in.readLong()); - } - - @Override - protected void addCustomXContent(XContentBuilder builder, Params params) throws IOException { - builder.field(PHASE_LATENCY_MAP, this.getPhaseLatencyMap()); - builder.field(TOOK, this.getValue()); - } - - /** - * Constructor of the SearchQueryLatencyRecord - * - * @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 A map contains per-phase latency data - * @param tookInNanos Total time took to finish this request - */ - public SearchQueryLatencyRecord( - final Long timestamp, - final SearchType searchType, - final String source, - final int totalShards, - final String[] indices, - final Map propertyMap, - final Map phaseLatencyMap, - final Long tookInNanos - ) { - super(timestamp, searchType, source, totalShards, indices, propertyMap, tookInNanos); - this.phaseLatencyMap = phaseLatencyMap; - } - - /** - * Get the phase level latency map of this request record - * - * @return Map contains per-phase latency of this request record - */ - public Map getPhaseLatencyMap() { - return phaseLatencyMap; - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - super.writeTo(out); - out.writeMap(phaseLatencyMap, StreamOutput::writeString, StreamOutput::writeLong); - out.writeLong(getValue()); - } - - /** - * Compare if two SearchQueryLatencyRecord are equal - * @param other The Other SearchQueryLatencyRecord to compare to - * @return boolean - */ - public boolean equals(SearchQueryLatencyRecord other) { - if (!super.equals(other)) { - return false; - } - for (String key : phaseLatencyMap.keySet()) { - if (!other.getPhaseLatencyMap().containsKey(key)) { - return false; - } - if (!phaseLatencyMap.get(key).equals(other.getPhaseLatencyMap().get(key))) { - return false; - } - } - return true; - } -} diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model/SearchQueryRecord.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model/SearchQueryRecord.java index f086a9e38c4cd..04392d54af42f 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model/SearchQueryRecord.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model/SearchQueryRecord.java @@ -8,252 +8,170 @@ package org.opensearch.plugin.insights.rules.model; -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; -import org.opensearch.action.search.SearchType; import org.opensearch.core.common.io.stream.StreamInput; import org.opensearch.core.common.io.stream.StreamOutput; import org.opensearch.core.common.io.stream.Writeable; +import org.opensearch.core.xcontent.ToXContent; import org.opensearch.core.xcontent.ToXContentObject; import org.opensearch.core.xcontent.XContentBuilder; import java.io.IOException; -import java.util.Locale; import java.util.Map; +import java.util.Objects; /** * Simple abstract class that represent record stored in the Query Insight Framework * - * @param The value type associated with the record * @opensearch.internal */ -public abstract class SearchQueryRecord> - implements - Comparable>, - Writeable, - ToXContentObject { - - private static final Logger log = LogManager.getLogger(SearchQueryRecord.class); - private static final String TIMESTAMP = "timestamp"; - private static final String SEARCH_TYPE = "searchType"; - private static final String SOURCE = "source"; - private static final String TOTAL_SHARDS = "totalShards"; - private static final String INDICES = "indices"; - private static final String PROPERTY_MAP = "propertyMap"; - private static final String VALUE = "value"; - +public class SearchQueryRecord implements ToXContentObject, Writeable { private final Long timestamp; - - private final SearchType searchType; - - private final String source; - - private final int totalShards; - - private final String[] indices; - - // TODO: add user-account which initialized the request in the future - private final Map propertyMap; - - private T value; + private final Map> measurements; + private final Map attributes; /** - * Constructor of the SearchQueryRecord - * - * @param in A {@link StreamInput} object. - * @throws IOException if the stream cannot be deserialized. + * Constructor of SearchQueryRecord + * @param in the StreamInput to read the SearchQueryRecord from + * @throws IOException IOException * @throws ClassCastException ClassCastException */ public SearchQueryRecord(final StreamInput in) throws IOException, ClassCastException { this.timestamp = in.readLong(); - this.searchType = SearchType.fromString(in.readString().toLowerCase(Locale.ROOT)); - this.source = in.readString(); - this.totalShards = in.readInt(); - this.indices = in.readStringArray(); - this.propertyMap = in.readMap(); + this.measurements = in.readMap(MetricType::readFromStream, Measurement::new); + this.attributes = in.readMap(Attribute::readFromStream, StreamInput::readGenericValue); } /** - * Constructor of the SearchQueryRecord + * Constructor of SearchQueryRecord * * @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 value The value on this SearchQueryRecord + * @param measurements A list of Measurement associated with this query + * @param attributes A list of Attributes associated with this query */ public SearchQueryRecord( final Long timestamp, - final SearchType searchType, - final String source, - final int totalShards, - final String[] indices, - final Map propertyMap, - final T value + Map> measurements, + Map attributes ) { - this(timestamp, searchType, source, totalShards, indices, propertyMap); - this.value = value; - } + if (measurements == null) { + throw new IllegalArgumentException("Measurements cannot be null"); + } - /** - * Constructor of the SearchQueryRecord - * - * @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 - */ - public SearchQueryRecord( - final Long timestamp, - final SearchType searchType, - final String source, - final int totalShards, - final String[] indices, - final Map propertyMap - ) { + this.measurements = measurements; + this.attributes = attributes; this.timestamp = timestamp; - this.searchType = searchType; - this.source = source; - this.totalShards = totalShards; - this.indices = indices; - this.propertyMap = propertyMap; } /** - * The timestamp of the top query. + * Returns the observation time of the metric. + * + * @return the observation time in milliseconds */ - public Long getTimestamp() { + public long getTimestamp() { return timestamp; } /** - * The manner at which the search operation is executed. - */ - public SearchType getSearchType() { - return searchType; - } - - /** - * The search source that was executed by the query. + * Returns the measurement associated with the specified name. + * + * @param name the name of the measurement + * @return the measurement object, or null if not found */ - public String getSource() { - return source; + public Measurement getMeasurement(MetricType name) { + return measurements.get(name); } /** - * Total number of shards as part of the search query across all indices + * Returns an unmodifiable map of all the measurements associated with the metric. + * + * @return an unmodifiable map of measurement names to measurement objects */ - public int getTotalShards() { - return totalShards; + public Map> getMeasurements() { + return measurements; } /** - * The indices involved in the search query + * Returns an unmodifiable map of the attributes associated with the metric. + * + * @return an unmodifiable map of attribute keys to attribute values */ - public String[] getIndices() { - return indices; + public Map getAttributes() { + return attributes; } /** - * Get the value of the query metric record + * Add an attribute to this record + * @param attribute attribute to add + * @param value the value associated with the attribute */ - public T getValue() { - return value; + public void addAttribute(Attribute attribute, Object value) { + attributes.put(attribute, value); } - /** - * Set the value of the query metric record - * @param value The value to set on the record - */ - public void setValue(T value) { - this.value = value; + @Override + public XContentBuilder toXContent(XContentBuilder builder, ToXContent.Params params) throws IOException { + builder.startObject(); + builder.field("timestamp", timestamp); + for (Map.Entry entry : attributes.entrySet()) { + builder.field(entry.getKey().toString(), entry.getValue()); + } + for (Map.Entry> entry : measurements.entrySet()) { + builder.field(entry.getKey().toString(), entry.getValue().getValue()); + } + return builder.endObject(); } /** - * Extra attributes and information about a search query + * Write a SearchQueryRecord to a StreamOutput + * @param out the StreamOutput to write + * @throws IOException IOException */ - public Map getPropertyMap() { - return propertyMap; - } - @Override - public int compareTo(SearchQueryRecord otherRecord) { - return value.compareTo(otherRecord.getValue()); + public void writeTo(StreamOutput out) throws IOException { + out.writeLong(timestamp); + out.writeMap( + measurements, + (stream, metricType) -> MetricType.writeTo(out, metricType), + (stream, measurement) -> Measurement.writeTo(out, measurement) + ); + out.writeMap(attributes, (stream, attribute) -> Attribute.writeTo(out, attribute), StreamOutput::writeGenericValue); } /** - * Compare if two SearchQueryRecord are equal - * @param other The Other SearchQueryRecord to compare to - * @return boolean + * Compare two SearchQueryRecord, based on the given MetricType + * + * @param a the first SearchQueryRecord to compare + * @param b the second SearchQueryRecord to compare + * @param metricType the MetricType to compare on + * @return 0 if the first SearchQueryRecord is numerically equal to the second SearchQueryRecord; + * -1 if the first SearchQueryRecord is numerically less than the second SearchQueryRecord; + * 1 if the first SearchQueryRecord is numerically greater than the second SearchQueryRecord. */ - public boolean equals(SearchQueryRecord other) { - if (!this.timestamp.equals(other.getTimestamp()) - || !this.searchType.equals(other.getSearchType()) - || !this.source.equals(other.getSource()) - || this.totalShards != other.getTotalShards() - || this.indices.length != other.getIndices().length - || this.propertyMap.size() != other.getPropertyMap().size() - || !this.value.equals(other.getValue())) { - return false; - } - for (int i = 0; i < indices.length; i++) { - if (!indices[i].equals(other.getIndices()[i])) { - return false; - } - } - for (String key : propertyMap.keySet()) { - if (!other.getPropertyMap().containsKey(key)) { - return false; - } - if (!propertyMap.get(key).equals(other.getPropertyMap().get(key))) { - return false; - } - } - return true; - } - - @SuppressWarnings("unchecked") - private T castToValue(Object obj) throws ClassCastException { - try { - return (T) obj; - } catch (Exception e) { - log.error(String.format(Locale.ROOT, "error casting query insight record value, error: %s", e)); - throw e; - } + public static int compare(SearchQueryRecord a, SearchQueryRecord b, MetricType metricType) { + return a.getMeasurement(metricType).compareTo(b.getMeasurement(metricType)); } /** - * Add custom XContent fields to the record - * @param builder XContent builder - * @param params XContent parameters - * @throws IOException IOException + * Check if a SearchQueryRecord is deep equal to another record + * @param o the other SearchQueryRecord record + * @return true if two records are deep equal, false otherwise. */ - protected abstract void addCustomXContent(XContentBuilder builder, Params params) throws IOException; - @Override - public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { - builder.startObject(); - builder.field(TIMESTAMP, timestamp); - builder.field(SEARCH_TYPE, searchType); - builder.field(SOURCE, source); - builder.field(TOTAL_SHARDS, totalShards); - builder.field(INDICES, indices); - builder.field(PROPERTY_MAP, propertyMap); - addCustomXContent(builder, params); - return builder.endObject(); + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (!(o instanceof SearchQueryRecord)) { + return false; + } + SearchQueryRecord other = (SearchQueryRecord) o; + return timestamp == other.getTimestamp() + && measurements.equals(other.getMeasurements()) + && attributes.size() == other.getAttributes().size(); } @Override - public void writeTo(StreamOutput out) throws IOException { - out.writeLong(timestamp); - out.writeString(searchType.toString()); - out.writeString(source); - out.writeInt(totalShards); - out.writeStringArray(indices); - out.writeMap(propertyMap); + public int hashCode() { + return Objects.hash(timestamp, measurements, attributes); } } 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/main/java/org/opensearch/plugin/insights/settings/QueryInsightsSettings.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/settings/QueryInsightsSettings.java index 818959bf7f12a..978185cda1268 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/settings/QueryInsightsSettings.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/settings/QueryInsightsSettings.java @@ -10,9 +10,10 @@ import org.opensearch.common.settings.Setting; import org.opensearch.common.unit.TimeValue; -import org.opensearch.plugin.insights.core.exporter.QueryInsightsExporterType; -import java.util.Locale; +import java.util.Arrays; +import java.util.HashSet; +import java.util.Set; import java.util.concurrent.TimeUnit; /** @@ -22,16 +23,36 @@ * @opensearch.experimental */ public class QueryInsightsSettings { + /** + * Executors settings + */ + public static final String QUERY_INSIGHTS_EXECUTOR = "query_insights_executor"; + /** + * Max number of thread + */ + public static final int MAX_THREAD_COUNT = 5; /** * Default Values and Settings */ - public static final TimeValue MAX_WINDOW_SIZE = new TimeValue(21600, TimeUnit.SECONDS); + public static final TimeValue MAX_WINDOW_SIZE = new TimeValue(1, TimeUnit.DAYS); + /** + * Minimal window size + */ + public static final TimeValue MIN_WINDOW_SIZE = new TimeValue(1, TimeUnit.MINUTES); + /** + * Valid window sizes + */ + public static final Set VALID_WINDOW_SIZES_IN_MINUTES = new HashSet<>( + Arrays.asList( + new TimeValue(1, TimeUnit.MINUTES), + new TimeValue(5, TimeUnit.MINUTES), + new TimeValue(10, TimeUnit.MINUTES), + new TimeValue(30, TimeUnit.MINUTES) + ) + ); + /** Default N size for top N queries */ public static final int MAX_N_SIZE = 100; - /** Default min export interval for Query Insights exporters */ - public static final TimeValue MIN_EXPORT_INTERVAL = new TimeValue(1, TimeUnit.SECONDS); - /** Default local index mapping for top n queries records */ - public static final String DEFAULT_LOCAL_INDEX_MAPPING = "mappings/top_n_queries_record.json"; /** Default window size in seconds to keep the top N queries with latency data in query insight store */ public static final int DEFAULT_WINDOW_SIZE = 60; /** Default top N size to keep the data in query insight store */ @@ -80,60 +101,6 @@ public class QueryInsightsSettings { Setting.Property.Dynamic ); - /** - * Settings for exporters - */ - public static final String TOP_N_LATENCY_QUERIES_EXPORTER_PREFIX = TOP_N_LATENCY_QUERIES_PREFIX + ".exporter"; - - /** - * Boolean setting for enabling top queries by latency exporter - */ - public static final Setting TOP_N_LATENCY_QUERIES_EXPORTER_ENABLED = Setting.boolSetting( - TOP_N_LATENCY_QUERIES_EXPORTER_PREFIX + ".enabled", - false, - Setting.Property.Dynamic, - Setting.Property.NodeScope - ); - - /** - * Setting for top queries by latency exporter type - */ - public static final Setting TOP_N_LATENCY_QUERIES_EXPORTER_TYPE = new Setting<>( - TOP_N_LATENCY_QUERIES_EXPORTER_PREFIX + ".type", - QueryInsightsExporterType.LOCAL_INDEX.name(), - QueryInsightsExporterType::parse, - Setting.Property.Dynamic, - Setting.Property.NodeScope - ); - - /** - * Setting for top queries by latency exporter interval - */ - public static final Setting TOP_N_LATENCY_QUERIES_EXPORTER_INTERVAL = Setting.positiveTimeSetting( - TOP_N_LATENCY_QUERIES_EXPORTER_PREFIX + ".interval", - MIN_EXPORT_INTERVAL, - Setting.Property.NodeScope, - Setting.Property.Dynamic - ); - - /** - * Setting for identifier (e.g. index name for index exporter) top queries by latency exporter - * Default value is "top_queries_since_{current_timestamp}" - */ - public static final Setting TOP_N_LATENCY_QUERIES_EXPORTER_IDENTIFIER = Setting.simpleString( - TOP_N_LATENCY_QUERIES_EXPORTER_PREFIX + ".identifier", - "top_queries_since_" + System.currentTimeMillis(), - value -> { - if (value == null || value.length() == 0) { - throw new IllegalArgumentException( - String.format(Locale.ROOT, "Invalid index name for [%s]", TOP_N_LATENCY_QUERIES_EXPORTER_PREFIX + ".identifier") - ); - } - }, - Setting.Property.NodeScope, - Setting.Property.Dynamic - ); - /** * Default constructor */ 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 aa8a3b8e7269b..681979bb5eb0b 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,8 +14,8 @@ 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.SearchQueryLatencyListener; -import org.opensearch.plugin.insights.core.service.TopQueriesByLatencyService; +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; @@ -47,10 +47,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); clusterService = new ClusterService(settings, clusterSettings, threadPool); @@ -61,11 +57,7 @@ public void testGetSettings() { Arrays.asList( QueryInsightsSettings.TOP_N_LATENCY_QUERIES_ENABLED, QueryInsightsSettings.TOP_N_LATENCY_QUERIES_SIZE, - QueryInsightsSettings.TOP_N_LATENCY_QUERIES_WINDOW_SIZE, - QueryInsightsSettings.TOP_N_LATENCY_QUERIES_EXPORTER_ENABLED, - QueryInsightsSettings.TOP_N_LATENCY_QUERIES_EXPORTER_TYPE, - QueryInsightsSettings.TOP_N_LATENCY_QUERIES_EXPORTER_INTERVAL, - QueryInsightsSettings.TOP_N_LATENCY_QUERIES_EXPORTER_IDENTIFIER + QueryInsightsSettings.TOP_N_LATENCY_QUERIES_WINDOW_SIZE ), queryInsightsPlugin.getSettings() ); @@ -86,8 +78,8 @@ public void testCreateComponent() { null ); assertEquals(2, components.size()); - assertTrue(components.get(0) instanceof TopQueriesByLatencyService); - assertTrue(components.get(1) instanceof SearchQueryLatencyListener); + assertTrue(components.get(0) instanceof QueryInsightsService); + assertTrue(components.get(1) instanceof QueryInsightsListener); } public void testGetRestHandlers() { 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 a0de894ace6a0..84c4331e7ec1a 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 @@ -10,17 +10,22 @@ 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.SearchQueryLatencyRecord; +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 org.opensearch.test.VersionUtils; import java.io.IOException; import java.util.ArrayList; -import java.util.Comparator; +import java.util.Arrays; import java.util.HashMap; import java.util.List; +import java.util.Locale; import java.util.Map; import java.util.Set; import java.util.TreeSet; @@ -32,9 +37,10 @@ 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.randomInt; +import static org.opensearch.test.OpenSearchTestCase.randomDouble; import static org.opensearch.test.OpenSearchTestCase.randomIntBetween; import static org.opensearch.test.OpenSearchTestCase.randomLong; +import static org.opensearch.test.OpenSearchTestCase.randomLongBetween; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; @@ -42,44 +48,46 @@ final public class QueryInsightsTestUtils { public QueryInsightsTestUtils() {} - public static List generateQueryInsightRecords(int count) { - return generateQueryInsightRecords(count, count); + public static List generateQueryInsightRecords(int count) { + return generateQueryInsightRecords(count, count, System.currentTimeMillis(), 0); } /** * Creates a List of random Query Insight Records for testing purpose */ - public static List generateQueryInsightRecords(int lower, int upper) { - List records = new ArrayList<>(); + public static List generateQueryInsightRecords(int lower, int upper, long startTimeStamp, long interval) { + List records = new ArrayList<>(); int countOfRecords = randomIntBetween(lower, upper); + long timestamp = startTimeStamp; for (int i = 0; i < countOfRecords; ++i) { - Map propertyMap = new HashMap<>(); - int countOfProperties = randomIntBetween(2, 5); - for (int j = 0; j < countOfProperties; ++j) { - propertyMap.put(randomAlphaOfLengthBetween(5, 10), randomAlphaOfLengthBetween(5, 10)); - } + Map> measurements = Map.of( + MetricType.LATENCY, + new Measurement<>(MetricType.LATENCY.name(), randomLongBetween(1000, 10000)), + MetricType.CPU, + new Measurement<>(MetricType.CPU.name(), randomDouble()), + MetricType.JVM, + new Measurement<>(MetricType.JVM.name(), randomDouble()) + ); + Map phaseLatencyMap = new HashMap<>(); int countOfPhases = randomIntBetween(2, 5); for (int j = 0; j < countOfPhases; ++j) { phaseLatencyMap.put(randomAlphaOfLengthBetween(5, 10), randomLong()); } - records.add( - new SearchQueryLatencyRecord( - System.currentTimeMillis(), - SearchType.QUERY_THEN_FETCH, - "{\"size\":20}", - randomIntBetween(1, 100), - randomArray(1, 3, String[]::new, () -> randomAlphaOfLengthBetween(5, 10)), - propertyMap, - phaseLatencyMap, - phaseLatencyMap.values().stream().mapToLong(x -> x).sum() - ) - ); + Map attributes = new HashMap<>(); + attributes.put(Attribute.SEARCH_TYPE, SearchType.QUERY_THEN_FETCH.toString().toLowerCase(Locale.ROOT)); + attributes.put(Attribute.SOURCE, "{\"size\":20}"); + attributes.put(Attribute.TOTAL_SHARDS, randomIntBetween(1, 100)); + attributes.put(Attribute.INDICES, randomArray(1, 3, Object[]::new, () -> randomAlphaOfLengthBetween(5, 10))); + attributes.put(Attribute.PHASE_LATENCY_MAP, phaseLatencyMap); + + records.add(new SearchQueryRecord(timestamp, measurements, attributes)); + timestamp += interval; } return records; } - public static TopQueries createTopQueries() { + public static TopQueries createRandomTopQueries() { DiscoveryNode node = new DiscoveryNode( "node_for_top_queries_test", buildNewFakeTransportAddress(), @@ -87,32 +95,39 @@ public static TopQueries createTopQueries() { emptySet(), VersionUtils.randomVersion(random()) ); + List records = generateQueryInsightRecords(10); - Map propertyMap = new HashMap<>(); - propertyMap.put("userId", "user1"); + return new TopQueries(node, records); + } - Map phaseLatencyMap = new HashMap<>(); - phaseLatencyMap.put("expand", 0L); - phaseLatencyMap.put("query", 20L); - phaseLatencyMap.put("fetch", 1L); - - List records = new ArrayList<>(); - records.add( - new SearchQueryLatencyRecord( - randomLong(), - SearchType.QUERY_THEN_FETCH, - "{\"size\":20}", - randomInt(), - randomArray(1, 3, String[]::new, () -> randomAlphaOfLengthBetween(5, 10)), - propertyMap, - phaseLatencyMap, - phaseLatencyMap.values().stream().mapToLong(x -> x).sum() - ) + 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, + new Measurement<>(MetricType.LATENCY.name(), 1L) + ); + + Map phaseLatencyMap = new HashMap<>(); + Map attributes = new HashMap<>(); + attributes.put(Attribute.SEARCH_TYPE, SearchType.QUERY_THEN_FETCH.toString().toLowerCase(Locale.ROOT)); + + return new SearchQueryRecord(timestamp, measurements, attributes); + } + public static void compareJson(ToXContent param1, ToXContent param2) throws IOException { if (param1 == null || param2 == null) { assertNull(param1); @@ -130,7 +145,8 @@ public static void compareJson(ToXContent param1, ToXContent param2) throws IOEx assertEquals(param1Builder.toString(), param2Builder.toString()); } - public static boolean checkRecordsEquals(List records1, List records2) { + @SuppressWarnings("unchecked") + public static boolean checkRecordsEquals(List records1, List records2) { if (records1.size() != records2.size()) { return false; } @@ -138,12 +154,31 @@ public static boolean checkRecordsEquals(List records1 if (!records1.get(i).equals(records2.get(i))) { return false; } + Map attributes1 = records1.get(i).getAttributes(); + Map attributes2 = records2.get(i).getAttributes(); + for (Map.Entry entry : attributes1.entrySet()) { + Attribute attribute = entry.getKey(); + Object value = entry.getValue(); + if (!attributes2.containsKey(attribute)) { + return false; + } + if (value instanceof Object[] && !Arrays.deepEquals((Object[]) value, (Object[]) attributes2.get(attribute))) { + return false; + } else if (value instanceof Map + && !Maps.deepEquals((Map) value, (Map) attributes2.get(attribute))) { + return false; + } + } } return true; } - public static boolean checkRecordsEqualsWithoutOrder(List records1, List records2) { - Set set2 = new TreeSet<>(new LatencyRecordComparator()); + public static boolean checkRecordsEqualsWithoutOrder( + List records1, + List records2, + MetricType metricType + ) { + Set set2 = new TreeSet<>((a, b) -> SearchQueryRecord.compare(a, b, metricType)); set2.addAll(records2); if (records1.size() != records2.size()) { return false; @@ -155,14 +190,4 @@ public static boolean checkRecordsEqualsWithoutOrder(List { - @Override - public int compare(SearchQueryLatencyRecord record1, SearchQueryLatencyRecord record2) { - if (record1.equals(record2)) { - return 0; - } - return record1.compareTo(record2); - } - } } diff --git a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/exporter/QueryInsightsExporterTests.java b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/exporter/QueryInsightsExporterTests.java deleted file mode 100644 index 64d296e501519..0000000000000 --- a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/exporter/QueryInsightsExporterTests.java +++ /dev/null @@ -1,35 +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.exporter; - -import org.opensearch.plugin.insights.rules.model.SearchQueryLatencyRecord; -import org.opensearch.plugin.insights.rules.model.SearchQueryRecord; -import org.opensearch.test.OpenSearchTestCase; - -import java.util.List; - -/** - * Unit Tests for {@link QueryInsightsExporter}. - */ -public class QueryInsightsExporterTests extends OpenSearchTestCase { - public class DummyExporter> extends QueryInsightsExporter { - DummyExporter(String identifier) { - super(identifier); - } - - @Override - public void export(List records) {} - } - - public void testGetType() { - DummyExporter exporter = new DummyExporter<>("test-index"); - String identifier = exporter.getIdentifier(); - assertEquals("test-index", identifier); - } -} diff --git a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/exporter/QueryInsightsLocalIndexExporterTests.java b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/exporter/QueryInsightsLocalIndexExporterTests.java deleted file mode 100644 index f1cac56447cfb..0000000000000 --- a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/exporter/QueryInsightsLocalIndexExporterTests.java +++ /dev/null @@ -1,256 +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.exporter; - -import org.opensearch.action.admin.indices.create.CreateIndexRequest; -import org.opensearch.action.admin.indices.create.CreateIndexResponse; -import org.opensearch.action.bulk.BulkItemResponse; -import org.opensearch.action.bulk.BulkRequest; -import org.opensearch.action.bulk.BulkResponse; -import org.opensearch.action.index.IndexRequest; -import org.opensearch.action.support.WriteRequest; -import org.opensearch.client.AdminClient; -import org.opensearch.client.Client; -import org.opensearch.client.IndicesAdminClient; -import org.opensearch.cluster.ClusterState; -import org.opensearch.cluster.routing.RoutingTable; -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.common.xcontent.XContentFactory; -import org.opensearch.core.action.ActionListener; -import org.opensearch.core.common.io.stream.InputStreamStreamInput; -import org.opensearch.core.xcontent.ToXContent; -import org.opensearch.plugin.insights.QueryInsightsTestUtils; -import org.opensearch.plugin.insights.rules.model.SearchQueryLatencyRecord; -import org.opensearch.plugin.insights.settings.QueryInsightsSettings; -import org.opensearch.test.OpenSearchTestCase; -import org.junit.Before; - -import java.io.ByteArrayInputStream; -import java.io.IOException; -import java.util.ArrayList; -import java.util.List; -import java.util.concurrent.CountDownLatch; -import java.util.concurrent.Phaser; - -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.anyString; -import static org.mockito.ArgumentMatchers.argThat; -import static org.mockito.Mockito.doAnswer; -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 QueryInsightsLocalIndexExporter}. - */ -public class QueryInsightsLocalIndexExporterTests extends OpenSearchTestCase { - private final String LOCAL_INDEX_NAME = "top-queries"; - private final IndicesAdminClient indicesAdminClient = mock(IndicesAdminClient.class); - private final AdminClient adminClient = mock(AdminClient.class); - private final Client client = mock(Client.class); - - private final ClusterState clusterState = mock(ClusterState.class); - private final RoutingTable mockRoutingTable = mock(RoutingTable.class); - - private final List records = QueryInsightsTestUtils.generateQueryInsightRecords(5, 10); - - private final Settings.Builder settingsBuilder = Settings.builder(); - private final Settings settings = settingsBuilder.build(); - - private final ClusterService clusterService = mock(ClusterService.class); - private final BulkRequest bulkRequest = new BulkRequest().setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE) - .timeout(TimeValue.timeValueSeconds(60)); - - private QueryInsightsLocalIndexExporter queryInsightLocalIndexExporter; - - @SuppressWarnings("unchecked") - @Before - public void setup() throws IOException { - final CreateIndexResponse createIndexResponse = new CreateIndexResponse(true, false, "test-index"); - doAnswer(invocation -> { - Object[] args = invocation.getArguments(); - assert args.length == 2; - ActionListener listener = (ActionListener) args[1]; - listener.onResponse(createIndexResponse); - return null; - }).when(indicesAdminClient).create(any(CreateIndexRequest.class), any(ActionListener.class)); - - final BulkResponse bulkResponse = new BulkResponse(new BulkItemResponse[] {}, randomLong()); - doAnswer(invocation -> { - Object[] args = invocation.getArguments(); - assert args.length == 2; - ActionListener listener = (ActionListener) args[1]; - listener.onResponse(bulkResponse); - return null; - }).when(client).bulk(any(BulkRequest.class), any(ActionListener.class)); - - when(adminClient.indices()).thenReturn(indicesAdminClient); - when(client.admin()).thenReturn(adminClient); - for (SearchQueryLatencyRecord record : records) { - bulkRequest.add( - new IndexRequest(LOCAL_INDEX_NAME).source(record.toXContent(XContentFactory.jsonBuilder(), ToXContent.EMPTY_PARAMS)) - ); - } - 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); - - when(clusterService.getClusterSettings()).thenReturn(clusterSettings); - when(clusterService.state()).thenReturn(clusterState); - - final int length = randomIntBetween(1, 1024); - ByteArrayInputStream is = new ByteArrayInputStream(new byte[length]); - InputStreamStreamInput streamInput = new InputStreamStreamInput(is); - - queryInsightLocalIndexExporter = new QueryInsightsLocalIndexExporter<>(clusterService, client, LOCAL_INDEX_NAME, streamInput); - } - - public void testExportWhenIndexExists() throws IOException { - when(mockRoutingTable.hasIndex(anyString())).thenReturn(true); - when(clusterState.getRoutingTable()).thenReturn(mockRoutingTable); - queryInsightLocalIndexExporter.export(records); - verify(client, times(1)).bulk( - argThat((BulkRequest request) -> request.requests().toString().equals(bulkRequest.requests().toString())), - any() - ); - } - - public void testConcurrentExportWhenIndexExists() throws InterruptedException { - when(mockRoutingTable.hasIndex(anyString())).thenReturn(true); - when(clusterState.getRoutingTable()).thenReturn(mockRoutingTable); - int numBulk = 50; - Thread[] threads = new Thread[numBulk]; - Phaser phaser = new Phaser(numBulk + 1); - CountDownLatch countDownLatch = new CountDownLatch(numBulk); - - final List> queryInsightLocalIndexExporters = new ArrayList<>(); - for (int i = 0; i < numBulk; i++) { - queryInsightLocalIndexExporters.add(new QueryInsightsLocalIndexExporter<>(clusterService, client, LOCAL_INDEX_NAME, null)); - } - - for (int i = 0; i < numBulk; i++) { - int finalI = i; - threads[i] = new Thread(() -> { - phaser.arriveAndAwaitAdvance(); - QueryInsightsLocalIndexExporter thisExporter = queryInsightLocalIndexExporters.get(finalI); - try { - thisExporter.export(records); - } catch (IOException e) { - throw new RuntimeException(e); - } - countDownLatch.countDown(); - }); - threads[i].start(); - } - phaser.arriveAndAwaitAdvance(); - countDownLatch.await(); - - verify(client, times(numBulk)).bulk( - argThat((BulkRequest request) -> request.requests().toString().equals(bulkRequest.requests().toString())), - any() - ); - } - - public void testExportWhenIndexNotExists() throws IOException { - when(mockRoutingTable.hasIndex(anyString())).thenReturn(false); - when(clusterState.getRoutingTable()).thenReturn(mockRoutingTable); - queryInsightLocalIndexExporter.export(records); - verify(indicesAdminClient, times(1)).create( - argThat((CreateIndexRequest request) -> request.index().equals(LOCAL_INDEX_NAME)), - any() - ); - } - - public void testConcurrentExportWhenIndexNotExists() throws InterruptedException { - when(mockRoutingTable.hasIndex(anyString())).thenReturn(false).thenReturn(true); - when(clusterState.getRoutingTable()).thenReturn(mockRoutingTable); - - int numBulk = 50; - Thread[] threads = new Thread[numBulk]; - Phaser phaser = new Phaser(numBulk + 1); - CountDownLatch countDownLatch = new CountDownLatch(numBulk); - - final int length = randomIntBetween(1, 1024); - ByteArrayInputStream is = new ByteArrayInputStream(new byte[length]); - InputStreamStreamInput streamInput = new InputStreamStreamInput(is); - - final List> queryInsightLocalIndexExporters = new ArrayList<>(); - for (int i = 0; i < numBulk; i++) { - queryInsightLocalIndexExporters.add( - new QueryInsightsLocalIndexExporter<>(clusterService, client, LOCAL_INDEX_NAME, streamInput) - ); - } - - for (int i = 0; i < numBulk; i++) { - int finalI = i; - threads[i] = new Thread(() -> { - phaser.arriveAndAwaitAdvance(); - QueryInsightsLocalIndexExporter thisExporter = queryInsightLocalIndexExporters.get(finalI); - try { - thisExporter.export(records); - } catch (IOException e) { - throw new RuntimeException(e); - } - countDownLatch.countDown(); - }); - threads[i].start(); - } - phaser.arriveAndAwaitAdvance(); - countDownLatch.await(); - - verify(indicesAdminClient, times(1)).create( - argThat((CreateIndexRequest request) -> request.index().equals(LOCAL_INDEX_NAME)), - any() - ); - } - - public void testInitIndexSucceed() throws IOException { - when(mockRoutingTable.hasIndex(anyString())).thenReturn(false); - when(clusterState.getRoutingTable()).thenReturn(mockRoutingTable); - queryInsightLocalIndexExporter.export(records); - verify(client, times(1)).bulk( - argThat((BulkRequest request) -> request.requests().toString().equals(bulkRequest.requests().toString())), - any() - ); - } - - @SuppressWarnings("unchecked") - public void testInitIndexFailed() throws IOException { - when(mockRoutingTable.hasIndex(anyString())).thenReturn(false); - when(clusterState.getRoutingTable()).thenReturn(mockRoutingTable); - final CreateIndexResponse response = new CreateIndexResponse(false, false, "test-index"); - doAnswer(invocation -> { - Object[] args = invocation.getArguments(); - assert args.length == 2; - ActionListener listener = (ActionListener) args[1]; - listener.onResponse(response); - return null; - }).when(indicesAdminClient).create(any(CreateIndexRequest.class), any(ActionListener.class)); - queryInsightLocalIndexExporter.export(records); - verify(client, times(0)).bulk( - argThat((BulkRequest request) -> request.requests().toString().equals(bulkRequest.requests().toString())), - any() - ); - } - - public void testBulkSucceed() throws IOException { - when(mockRoutingTable.hasIndex(anyString())).thenReturn(true); - when(clusterState.getRoutingTable()).thenReturn(mockRoutingTable); - queryInsightLocalIndexExporter.export(records); - verify(client, times(1)).bulk( - argThat((BulkRequest request) -> request.requests().toString().equals(bulkRequest.requests().toString())), - any() - ); - } -} 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/QueryInsightsServiceTests.java b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/service/QueryInsightsServiceTests.java index b3eaf61dbb9a7..2d875375eddc1 100644 --- a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/service/QueryInsightsServiceTests.java +++ b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/service/QueryInsightsServiceTests.java @@ -8,124 +8,197 @@ package org.opensearch.plugin.insights.core.service; -import org.opensearch.client.Client; -import org.opensearch.cluster.service.ClusterService; -import org.opensearch.common.settings.ClusterSettings; +import org.opensearch.cluster.coordination.DeterministicTaskQueue; 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.rules.model.MetricType; +import org.opensearch.plugin.insights.rules.model.SearchQueryRecord; import org.opensearch.plugin.insights.settings.QueryInsightsSettings; import org.opensearch.test.OpenSearchTestCase; -import org.opensearch.threadpool.Scheduler; import org.opensearch.threadpool.ThreadPool; import org.junit.Before; -import java.io.IOException; -import java.util.ArrayList; import java.util.List; - -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; +import java.util.concurrent.TimeUnit; /** * Unit Tests for {@link QueryInsightsService}. */ public class QueryInsightsServiceTests extends OpenSearchTestCase { - private ThreadPool threadPool = mock(ThreadPool.class); - private DummyQueryInsightsService dummyQueryInsightsService; - @SuppressWarnings("unchecked") - private QueryInsightsLocalIndexExporter exporter = mock(QueryInsightsLocalIndexExporter.class); - - static class DummyQueryInsightsService extends QueryInsightsService< - SearchQueryLatencyRecord, - ArrayList, - QueryInsightsLocalIndexExporter> { - public DummyQueryInsightsService( - ThreadPool threadPool, - ClusterService clusterService, - Client client, - QueryInsightsLocalIndexExporter exporter - ) { - super(threadPool, new ArrayList<>(), exporter); + + private DeterministicTaskQueue deterministicTaskQueue; + private ThreadPool threadPool; + private QueryInsightsService queryInsightsService; + + @Before + public void setup() { + final Settings settings = Settings.builder().put(Node.NODE_NAME_SETTING.getKey(), "top n queries tests").build(); + deterministicTaskQueue = new DeterministicTaskQueue(settings, random()); + threadPool = deterministicTaskQueue.getThreadPool(); + queryInsightsService = new QueryInsightsService(threadPool); + queryInsightsService.enableCollection(MetricType.LATENCY, true); + queryInsightsService.enableCollection(MetricType.CPU, true); + queryInsightsService.enableCollection(MetricType.JVM, true); + queryInsightsService.setTopNSize(Integer.MAX_VALUE); + queryInsightsService.setWindowSize(new TimeValue(Long.MAX_VALUE)); + } + + public void testIngestQueryDataWithLargeWindow() { + final List records = QueryInsightsTestUtils.generateQueryInsightRecords(10); + for (SearchQueryRecord record : records) { + queryInsightsService.addRecord(record); } + runUntilTimeoutOrFinish(deterministicTaskQueue, 5000); + assertTrue( + QueryInsightsTestUtils.checkRecordsEqualsWithoutOrder( + queryInsightsService.getTopNRecords(MetricType.LATENCY, false), + records, + MetricType.LATENCY + ) + ); + } - @Override - public void clearOutdatedData() {} + public void testConcurrentIngestQueryDataWithLargeWindow() { + final List records = QueryInsightsTestUtils.generateQueryInsightRecords(10); - @Override - public void resetExporter(boolean enabled, QueryInsightsExporterType type, String identifier) {} + int numRequests = records.size(); + for (int i = 0; i < numRequests; i++) { + int finalI = i; + threadPool.schedule(() -> { queryInsightsService.addRecord(records.get(finalI)); }, TimeValue.ZERO, ThreadPool.Names.GENERIC); + } + runUntilTimeoutOrFinish(deterministicTaskQueue, 5000); + assertTrue( + QueryInsightsTestUtils.checkRecordsEqualsWithoutOrder( + queryInsightsService.getTopNRecords(MetricType.LATENCY, false), + records, + MetricType.LATENCY + ) + ); + } - public void doStart() { - super.doStart(); + public void testRollingWindow() { + // Create records with starting timestamp Monday, January 1, 2024 8:13:23 PM, with interval 10 minutes + final List records = QueryInsightsTestUtils.generateQueryInsightRecords(10, 10, 1704140003000L, 1000 * 60 * 10); + queryInsightsService.setWindowSize(TimeValue.timeValueMinutes(10)); + queryInsightsService.addRecord(records.get(0)); + runUntilTimeoutOrFinish(deterministicTaskQueue, 5000); + assertEquals(1, queryInsightsService.getTopNStoreSize(MetricType.LATENCY)); + assertEquals(0, queryInsightsService.getTopNHistoryStoreSize(MetricType.LATENCY)); + for (SearchQueryRecord record : records.subList(1, records.size())) { + queryInsightsService.addRecord(record); + runUntilTimeoutOrFinish(deterministicTaskQueue, 5000); + assertEquals(1, queryInsightsService.getTopNStoreSize(MetricType.LATENCY)); + assertEquals(1, queryInsightsService.getTopNHistoryStoreSize(MetricType.LATENCY)); } + assertEquals(0, queryInsightsService.getTopNRecords(MetricType.LATENCY, false).size()); + } + + public void testRollingWindowWithHistory() { + // Create 2 records with starting Now and last 10 minutes + final List records = QueryInsightsTestUtils.generateQueryInsightRecords( + 2, + 2, + System.currentTimeMillis() - 1000 * 60 * 10, + 1000 * 60 * 10 + ); + queryInsightsService.setWindowSize(TimeValue.timeValueMinutes(3)); + queryInsightsService.addRecord(records.get(0)); + runUntilTimeoutOrFinish(deterministicTaskQueue, 5000); + assertEquals(1, queryInsightsService.getTopNStoreSize(MetricType.LATENCY)); + assertEquals(0, queryInsightsService.getTopNHistoryStoreSize(MetricType.LATENCY)); + queryInsightsService.addRecord(records.get(1)); + runUntilTimeoutOrFinish(deterministicTaskQueue, 5000); + assertEquals(1, queryInsightsService.getTopNStoreSize(MetricType.LATENCY)); + assertEquals(0, queryInsightsService.getTopNHistoryStoreSize(MetricType.LATENCY)); + assertEquals(1, queryInsightsService.getTopNRecords(MetricType.LATENCY, true).size()); + } - public void doStop() { - super.doStop(); + public void testSmallWindowClearOutdatedData() { + final List records = QueryInsightsTestUtils.generateQueryInsightRecords( + 2, + 2, + System.currentTimeMillis(), + 1000 * 60 * 20 + ); + queryInsightsService.setWindowSize(TimeValue.timeValueMinutes(10)); + + for (SearchQueryRecord record : records) { + queryInsightsService.addRecord(record); } + runUntilTimeoutOrFinish(deterministicTaskQueue, 5000); + assertTrue(queryInsightsService.getTopNRecords(MetricType.LATENCY, false).size() <= 1); } - @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(); - 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); - dummyQueryInsightsService = new DummyQueryInsightsService(threadPool, clusterService, client, exporter); - when(threadPool.scheduleWithFixedDelay(any(), any(), any())).thenReturn(new Scheduler.Cancellable() { - @Override - public boolean cancel() { - return false; - } + public void testSmallNSize() { + final List records = QueryInsightsTestUtils.generateQueryInsightRecords(10); + queryInsightsService.setTopNSize(1); - @Override - public boolean isCancelled() { - return false; - } - }); + for (SearchQueryRecord record : records) { + queryInsightsService.addRecord(record); + } + runUntilTimeoutOrFinish(deterministicTaskQueue, 5000); + assertEquals(1, queryInsightsService.getTopNRecords(MetricType.LATENCY, false).size()); + } + + public void testSmallNSizeWithCPU() { + final List records = QueryInsightsTestUtils.generateQueryInsightRecords(10); + queryInsightsService.setTopNSize(1); + + for (SearchQueryRecord record : records) { + queryInsightsService.addRecord(record); + } + runUntilTimeoutOrFinish(deterministicTaskQueue, 5000); + assertEquals(1, queryInsightsService.getTopNRecords(MetricType.CPU, false).size()); } - public void testIngestDataWhenFeatureNotEnabled() { - dummyQueryInsightsService.setEnableCollect(false); - final List records = QueryInsightsTestUtils.generateQueryInsightRecords(1); - dummyQueryInsightsService.ingestQueryData(records.get(0)); - assertThrows(IllegalArgumentException.class, () -> { dummyQueryInsightsService.getQueryData(); }); + public void testSmallNSizeWithJVM() { + final List records = QueryInsightsTestUtils.generateQueryInsightRecords(10); + queryInsightsService.setTopNSize(1); + + for (SearchQueryRecord record : records) { + queryInsightsService.addRecord(record); + } + runUntilTimeoutOrFinish(deterministicTaskQueue, 5000); + assertEquals(1, queryInsightsService.getTopNRecords(MetricType.JVM, false).size()); + } + + public void testValidateTopNSize() { + assertThrows( + IllegalArgumentException.class, + () -> { queryInsightsService.validateTopNSize(QueryInsightsSettings.MAX_N_SIZE + 1); } + ); } - public void testIngestDataWhenFeatureEnabled() { - dummyQueryInsightsService.setEnableCollect(true); - final List records = QueryInsightsTestUtils.generateQueryInsightRecords(1); - dummyQueryInsightsService.ingestQueryData(records.get(0)); - assertEquals(records.get(0), dummyQueryInsightsService.getQueryData().get(0)); + public void testGetTopQueriesWhenNotEnabled() { + queryInsightsService.enableCollection(MetricType.LATENCY, false); + assertThrows(IllegalArgumentException.class, () -> { queryInsightsService.getTopNRecords(MetricType.LATENCY, false); }); } - public void testClearAllData() { - dummyQueryInsightsService.setEnableCollect(true); - dummyQueryInsightsService.setEnableCollect(true); - final List records = QueryInsightsTestUtils.generateQueryInsightRecords(1); - dummyQueryInsightsService.ingestQueryData(records.get(0)); - dummyQueryInsightsService.clearAllData(); - assertEquals(0, dummyQueryInsightsService.getQueryData().size()); + public void testValidateWindowSize() { + assertThrows(IllegalArgumentException.class, () -> { + queryInsightsService.validateWindowSize( + new TimeValue(QueryInsightsSettings.MAX_WINDOW_SIZE.getSeconds() + 1, TimeUnit.SECONDS) + ); + }); + assertThrows(IllegalArgumentException.class, () -> { + queryInsightsService.validateWindowSize( + new TimeValue(QueryInsightsSettings.MIN_WINDOW_SIZE.getSeconds() - 1, TimeUnit.SECONDS) + ); + }); + assertThrows(IllegalArgumentException.class, () -> { queryInsightsService.validateWindowSize(new TimeValue(2, TimeUnit.DAYS)); }); } - public void testDoStartAndStop() throws IOException { - dummyQueryInsightsService.setEnableCollect(true); - dummyQueryInsightsService.setEnableExport(true); - dummyQueryInsightsService.doStart(); - verify(threadPool, times(1)).scheduleWithFixedDelay(any(), any(), any()); - dummyQueryInsightsService.doStop(); - verify(exporter, times(1)).export(any()); + 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/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/model/SearchQueryRecordTests.java b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/model/SearchQueryRecordTests.java new file mode 100644 index 0000000000000..793d5878e2300 --- /dev/null +++ b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/model/SearchQueryRecordTests.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.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.Arrays; +import java.util.HashSet; +import java.util.List; +import java.util.Set; + +/** + * Granular tests for the {@link SearchQueryRecord} class. + */ +public class SearchQueryRecordTests 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 (SearchQueryRecord record : records) { + copiedRecords.add(roundTripRecord(record)); + } + assertTrue(QueryInsightsTestUtils.checkRecordsEquals(records, copiedRecords)); + + } + + public void testAllMetricTypes() { + Set allMetrics = MetricType.allMetricTypes(); + Set expected = new HashSet<>(Arrays.asList(MetricType.LATENCY, MetricType.CPU, MetricType.JVM)); + assertEquals(expected, allMetrics); + } + + public void testCompare() { + SearchQueryRecord record1 = QueryInsightsTestUtils.createFixedSearchQueryRecord(); + SearchQueryRecord record2 = QueryInsightsTestUtils.createFixedSearchQueryRecord(); + assertEquals(0, SearchQueryRecord.compare(record1, record2, MetricType.LATENCY)); + } + + public void testEqual() { + SearchQueryRecord record1 = QueryInsightsTestUtils.createFixedSearchQueryRecord(); + SearchQueryRecord record2 = QueryInsightsTestUtils.createFixedSearchQueryRecord(); + assertEquals(record1, record2); + } + + /** + * Serialize and deserialize a SearchQueryRecord. + * @param record A SearchQueryRecord to serialize. + * @return The deserialized, "round-tripped" record. + */ + private static SearchQueryRecord roundTripRecord(SearchQueryRecord record) throws Exception { + try (BytesStreamOutput out = new BytesStreamOutput()) { + record.writeTo(out); + try (StreamInput in = out.bytes().streamInput()) { + return new SearchQueryRecord(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() {