From b082ae1b9ff25bec3c2afe25787785d8687370d7 Mon Sep 17 00:00:00 2001 From: Chenyang Ji Date: Wed, 13 Dec 2023 16:41:38 -0800 Subject: [PATCH] refactor query insight components into plugin Signed-off-by: Chenyang Ji --- gradle/missing-javadoc.gradle | 1 + plugins/query-insights/build.gradle | 51 ++++++ .../plugin/insights/QueryInsightsPlugin.java | 104 ++++++++++++ .../core/exporter}/QueryInsightExporter.java | 14 +- .../QueryInsightLocalIndexExporter.java | 75 ++++----- .../insights/core/exporter}/package-info.java | 6 +- .../listener/SearchQueryLatencyListener.java | 90 +++++++++++ .../insights/core/listener/package-info.java | 12 ++ .../core/service}/QueryInsightService.java | 40 ++--- .../service/TopQueriesByLatencyService.java | 124 ++++++++++++++ .../insights/core/service/package-info.java | 12 ++ .../plugin}/insights/package-info.java | 6 +- .../insights/rules/action/package-info.java | 12 ++ .../rules/action}/top_queries/TopQueries.java | 13 +- .../action}/top_queries/TopQueriesAction.java | 3 +- .../top_queries/TopQueriesRequest.java | 11 +- .../top_queries/TopQueriesResponse.java | 14 +- .../action/top_queries/package-info.java | 12 ++ .../model}/SearchQueryLatencyRecord.java | 14 +- .../rules/model}/SearchQueryRecord.java | 42 +++-- .../insights/rules/model/package-info.java | 12 ++ .../rules/resthandler/package-info.java | 12 ++ .../top_queries}/RestTopQueriesAction.java | 50 +++--- .../resthandler/top_queries/package-info.java | 12 ++ .../rules/transport/package-info.java | 12 ++ .../TransportTopQueriesAction.java | 23 +-- .../transport/top_queries/package-info.java | 12 ++ .../settings/QueryInsightsSettings.java | 72 +++++++++ .../insights/settings/package-info.java | 12 ++ .../mappings/top_n_queries_record.json | 0 .../insights/QueryInsightsPluginTests.java | 19 +++ .../SearchQueryLatencyListenerTests.java | 73 +++++---- .../top_queries/TopQueriesRequestTests.java | 2 +- .../action}/top_queries/TopQueriesTests.java | 5 +- .../RestTopQueriesActionTests.java | 6 +- .../org/opensearch/action/ActionModule.java | 9 -- .../top_queries/TopQueriesRequestBuilder.java | 38 ----- .../search/QueryLatencyInsightService.java | 153 ------------------ .../search/SearchQueryLatencyListener.java | 58 ------- .../action/search/SearchRequest.java | 2 +- .../action/search/SearchRequestContext.java | 6 +- .../SearchRequestOperationsListener.java | 23 +-- .../action/search/SearchRequestSlowLog.java | 10 +- .../action/search/SearchRequestStats.java | 6 +- .../action/search/TransportSearchAction.java | 6 +- .../opensearch/client/ClusterAdminClient.java | 26 --- .../java/org/opensearch/client/Requests.java | 24 --- .../client/support/AbstractClient.java | 19 --- .../common/settings/ClusterSettings.java | 8 +- .../SearchRequestListenerManagerTests.java | 6 +- .../snapshots/SnapshotResiliencyTests.java | 2 - 51 files changed, 823 insertions(+), 551 deletions(-) create mode 100644 plugins/query-insights/build.gradle create mode 100644 plugins/query-insights/src/main/java/org/opensearch/plugin/insights/QueryInsightsPlugin.java rename {server/src/main/java/org/opensearch/action/search => plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/exporter}/QueryInsightExporter.java (81%) rename {server/src/main/java/org/opensearch/action/search => plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/exporter}/QueryInsightLocalIndexExporter.java (73%) rename {server/src/main/java/org/opensearch/action/admin/cluster/insights/top_queries => plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/exporter}/package-info.java (65%) create mode 100644 plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/listener/SearchQueryLatencyListener.java create mode 100644 plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/listener/package-info.java rename {server/src/main/java/org/opensearch/action/search => plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service}/QueryInsightService.java (78%) create mode 100644 plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/TopQueriesByLatencyService.java create mode 100644 plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/package-info.java rename {server/src/main/java/org/opensearch/action/admin/cluster => plugins/query-insights/src/main/java/org/opensearch/plugin}/insights/package-info.java (69%) create mode 100644 plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/package-info.java rename {server/src/main/java/org/opensearch/action/admin/cluster/insights => plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action}/top_queries/TopQueries.java (82%) rename {server/src/main/java/org/opensearch/action/admin/cluster/insights => plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action}/top_queries/TopQueriesAction.java (90%) rename {server/src/main/java/org/opensearch/action/admin/cluster/insights => plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action}/top_queries/TopQueriesRequest.java (91%) rename {server/src/main/java/org/opensearch/action/admin/cluster/insights => plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action}/top_queries/TopQueriesResponse.java (91%) create mode 100644 plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/package-info.java rename {server/src/main/java/org/opensearch/action/search => plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model}/SearchQueryLatencyRecord.java (88%) rename {server/src/main/java/org/opensearch/action/search => plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model}/SearchQueryRecord.java (81%) create mode 100644 plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model/package-info.java create mode 100644 plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/resthandler/package-info.java rename {server/src/main/java/org/opensearch/rest/action/admin/cluster => plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/resthandler/top_queries}/RestTopQueriesAction.java (55%) create mode 100644 plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/resthandler/top_queries/package-info.java create mode 100644 plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/transport/package-info.java rename {server/src/main/java/org/opensearch/action/admin/cluster/insights => plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/transport}/top_queries/TransportTopQueriesAction.java (77%) create mode 100644 plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/transport/top_queries/package-info.java create mode 100644 plugins/query-insights/src/main/java/org/opensearch/plugin/insights/settings/QueryInsightsSettings.java create mode 100644 plugins/query-insights/src/main/java/org/opensearch/plugin/insights/settings/package-info.java rename {server/src/main/resources/org/opensearch/action/search => plugins/query-insights/src/main/resources}/mappings/top_n_queries_record.json (100%) create mode 100644 plugins/query-insights/src/test/java/org/opensearch/plugin/insights/QueryInsightsPluginTests.java rename {server/src/test/java/org/opensearch/action/search => plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/listener}/SearchQueryLatencyListenerTests.java (63%) rename {server/src/test/java/org/opensearch/action/admin/cluster/insights => plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/action}/top_queries/TopQueriesRequestTests.java (95%) rename {server/src/test/java/org/opensearch/action/admin/cluster/insights => plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/action}/top_queries/TopQueriesTests.java (96%) rename {server/src/test/java/org/opensearch/rest/action/admin/cluster => plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/resthandler/top_queries}/RestTopQueriesActionTests.java (89%) delete mode 100644 server/src/main/java/org/opensearch/action/admin/cluster/insights/top_queries/TopQueriesRequestBuilder.java delete mode 100644 server/src/main/java/org/opensearch/action/search/QueryLatencyInsightService.java delete mode 100644 server/src/main/java/org/opensearch/action/search/SearchQueryLatencyListener.java diff --git a/gradle/missing-javadoc.gradle b/gradle/missing-javadoc.gradle index e9a6d798b8323..d3fb9f82c3715 100644 --- a/gradle/missing-javadoc.gradle +++ b/gradle/missing-javadoc.gradle @@ -141,6 +141,7 @@ configure([ project(":plugins:mapper-annotated-text"), project(":plugins:mapper-murmur3"), project(":plugins:mapper-size"), + project(":plugins:query-insights"), project(":plugins:repository-azure"), project(":plugins:repository-gcs"), project(":plugins:repository-hdfs"), diff --git a/plugins/query-insights/build.gradle b/plugins/query-insights/build.gradle new file mode 100644 index 0000000000000..e65abd810010d --- /dev/null +++ b/plugins/query-insights/build.gradle @@ -0,0 +1,51 @@ +/* + * 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. + * + * Modifications Copyright OpenSearch Contributors. See + * GitHub history for details. + */ + +//apply plugin: 'opensearch.java-rest-test' +//apply plugin: 'opensearch.internal-cluster-test' + +import org.opensearch.gradle.testclusters.RunTask +import org.opensearch.gradle.test.RestIntegTestTask +apply plugin: 'opensearch.testclusters' + +opensearchplugin { + description 'OpenSearch Query Insights Plugin.' + classname 'org.opensearch.plugin.insights.QueryInsightsPlugin' +} + +dependencies { +} + +task integTest(type: RestIntegTestTask) { + description = "Run tests against a cluster" + testClassesDirs = sourceSets.test.output.classesDirs + classpath = sourceSets.test.runtimeClasspath +} +tasks.named("check").configure { dependsOn(integTest) } + +integTest { + // The --debug-jvm command-line option makes the cluster debuggable; this makes the tests debuggable + if (System.getProperty("test.debug") != null) { + jvmArgs '-agentlib:jdwp=transport=dt_socket,server=y,suspend=y,address=*:5005' + } +} + +testClusters.integTest { + testDistribution = "INTEG_TEST" + + // This installs our plugin into the testClusters + plugin(project.tasks.bundlePlugin.archiveFile) +} + +run { + useCluster testClusters.integTest +} + 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 new file mode 100644 index 0000000000000..aeedb3a92e121 --- /dev/null +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/QueryInsightsPlugin.java @@ -0,0 +1,104 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.plugin.insights; + +import org.opensearch.action.ActionRequest; +import org.opensearch.client.Client; +import org.opensearch.cluster.metadata.IndexNameExpressionResolver; +import org.opensearch.cluster.node.DiscoveryNodes; +import org.opensearch.cluster.service.ClusterService; +import org.opensearch.common.settings.ClusterSettings; +import org.opensearch.common.settings.IndexScopedSettings; +import org.opensearch.common.settings.Setting; +import org.opensearch.common.settings.Settings; +import org.opensearch.common.settings.SettingsFilter; +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.rules.action.top_queries.TopQueriesAction; +import org.opensearch.plugin.insights.rules.resthandler.top_queries.RestTopQueriesAction; +import org.opensearch.plugin.insights.rules.transport.top_queries.TransportTopQueriesAction; +import org.opensearch.plugin.insights.settings.QueryInsightsSettings; +import org.opensearch.plugins.ActionPlugin; +import org.opensearch.plugins.EnginePlugin; +import org.opensearch.plugins.Plugin; +import org.opensearch.plugins.SearchPlugin; +import org.opensearch.repositories.RepositoriesService; +import org.opensearch.rest.RestController; +import org.opensearch.rest.RestHandler; +import org.opensearch.script.ScriptService; +import org.opensearch.threadpool.ThreadPool; +import org.opensearch.watcher.ResourceWatcherService; + +import java.util.Collection; +import java.util.List; +import java.util.function.Supplier; + +/** + * Plugin class for Query Insights. + */ +public class QueryInsightsPlugin extends Plugin implements ActionPlugin, SearchPlugin, EnginePlugin { + /** + * Default constructor + */ + public QueryInsightsPlugin() {} + + @Override + public Collection createComponents( + Client client, + ClusterService clusterService, + ThreadPool threadPool, + ResourceWatcherService resourceWatcherService, + ScriptService scriptService, + NamedXContentRegistry xContentRegistry, + Environment environment, + NodeEnvironment nodeEnvironment, + NamedWriteableRegistry namedWriteableRegistry, + IndexNameExpressionResolver indexNameExpressionResolver, + 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); + } + + @Override + public List getRestHandlers( + Settings settings, + RestController restController, + ClusterSettings clusterSettings, + IndexScopedSettings indexScopedSettings, + SettingsFilter settingsFilter, + IndexNameExpressionResolver indexNameExpressionResolver, + Supplier nodesInCluster + ) { + return List.of(new RestTopQueriesAction()); + } + + @Override + public List> getActions() { + return List.of(new ActionPlugin.ActionHandler<>(TopQueriesAction.INSTANCE, TransportTopQueriesAction.class)); + } + + @Override + public List> getSettings() { + return List.of( + // Settings for top N queries + QueryInsightsSettings.TOP_N_LATENCY_QUERIES_ENABLED, + QueryInsightsSettings.TOP_N_LATENCY_QUERIES_SIZE, + QueryInsightsSettings.TOP_N_LATENCY_QUERIES_WINDOW_SIZE + ); + } +} diff --git a/server/src/main/java/org/opensearch/action/search/QueryInsightExporter.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/exporter/QueryInsightExporter.java similarity index 81% rename from server/src/main/java/org/opensearch/action/search/QueryInsightExporter.java rename to plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/exporter/QueryInsightExporter.java index a323f27c9bbd7..3bf5f70758e89 100644 --- a/server/src/main/java/org/opensearch/action/search/QueryInsightExporter.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/exporter/QueryInsightExporter.java @@ -6,9 +6,10 @@ * compatible open source license. */ -package org.opensearch.action.search; +package org.opensearch.plugin.insights.core.exporter; import org.opensearch.common.unit.TimeValue; +import org.opensearch.plugin.insights.rules.model.SearchQueryRecord; import java.util.List; @@ -19,24 +20,19 @@ * * @opensearch.internal */ -abstract class QueryInsightExporter> { +public abstract class QueryInsightExporter> { private boolean enabled = false; /** The export interval of this exporter, default to 60 seconds */ - private TimeValue exportInterval = TimeValue.timeValueSeconds(60); - - /** - * Initial set up the exporter - */ - abstract void setup() throws Exception; + private TimeValue exportInterval = TimeValue.timeValueSeconds(5); /** * Export the data with the exporter. * * @param records the data to export */ - abstract void export(List records) throws Exception; + public abstract void export(List records) throws Exception; public boolean getEnabled() { return enabled; diff --git a/server/src/main/java/org/opensearch/action/search/QueryInsightLocalIndexExporter.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/exporter/QueryInsightLocalIndexExporter.java similarity index 73% rename from server/src/main/java/org/opensearch/action/search/QueryInsightLocalIndexExporter.java rename to plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/exporter/QueryInsightLocalIndexExporter.java index b65fba1e69e03..56570a045bc17 100644 --- a/server/src/main/java/org/opensearch/action/search/QueryInsightLocalIndexExporter.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/exporter/QueryInsightLocalIndexExporter.java @@ -6,7 +6,7 @@ * compatible open source license. */ -package org.opensearch.action.search; +package org.opensearch.plugin.insights.core.exporter; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -28,11 +28,13 @@ 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; /** @@ -42,7 +44,7 @@ * * @opensearch.internal */ -public class QueryInsightLocalIndexExporter> extends QueryInsightExporter{ +public class QueryInsightLocalIndexExporter> extends QueryInsightExporter { private static final Logger log = LogManager.getLogger(QueryInsightLocalIndexExporter.class); @@ -60,23 +62,15 @@ public QueryInsightLocalIndexExporter( ClusterService clusterService, Client client, String localIndexName, - InputStream localIndexMapping) { + InputStream localIndexMapping + ) { this.setEnabled(enabled); this.clusterService = clusterService; this.client = client; this.localIndexName = localIndexName; this.localIndexMapping = localIndexMapping; - try { - setup(); - } catch (IOException e) { - log.error(String.format("failed to set up with error %s.", e)); - } } - @Override - public void setup() throws IOException {} - - /** * Export the data to the predefined local OpenSearch Index * @@ -96,21 +90,26 @@ public synchronized void export(List records) throws IOException { @Override public void onResponse(CreateIndexResponse response) { if (response.isAcknowledged()) { - log.debug(String.format("successfully initialized local index %s for query insight.", localIndexName)); + log.debug(String.format(Locale.ROOT, "successfully initialized local index %s for query insight.", localIndexName)); try { bulkRecord(records); } catch (IOException e) { - log.error(String.format("fail to ingest query insight data to local index, error: %s", e)); + log.error(String.format(Locale.ROOT, "fail to ingest query insight data to local index, error: %s", e)); } - } - else { - log.error(String.format("request to created local index %s for query insight not acknowledged.", localIndexName)); + } else { + log.error( + String.format( + Locale.ROOT, + "request to created local index %s for query insight not acknowledged.", + localIndexName + ) + ); } } @Override public void onFailure(Exception e) { - log.error(String.format("error creating local index for query insight: %s", e)); + log.error(String.format(Locale.ROOT, "error creating local index for query insight: %s", e)); } }); } @@ -133,9 +132,8 @@ private boolean checkIfIndexExists() { * @throws IOException if an error occurs */ private synchronized void initLocalIndex(ActionListener listener) throws IOException { - CreateIndexRequest createIndexRequest = new CreateIndexRequest(this.localIndexName).mapping( - getIndexMappings() - ).settings(Settings.builder().put("index.hidden", false).build()); + CreateIndexRequest createIndexRequest = new CreateIndexRequest(this.localIndexName).mapping(getIndexMappings()) + .settings(Settings.builder().put("index.hidden", false).build()); client.admin().indices().create(createIndexRequest, listener); } @@ -157,11 +155,7 @@ private synchronized void dropLocalIndex(ActionListener li * @throws IOException if an error occurs */ private String getIndexMappings() throws IOException { - return new String( - Objects.requireNonNull(this.localIndexMapping) - .readAllBytes(), - Charset.defaultCharset() - ); + return new String(Objects.requireNonNull(this.localIndexMapping).readAllBytes(), Charset.defaultCharset()); } /** @@ -171,27 +165,26 @@ private String getIndexMappings() throws IOException { * @throws IOException if an error occurs */ private synchronized void bulkRecord(List records) throws IOException { - BulkRequest bulkRequest = new BulkRequest() - .setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE).timeout(TimeValue.timeValueSeconds(60)); + BulkRequest bulkRequest = new BulkRequest().setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE) + .timeout(TimeValue.timeValueSeconds(60)); for (T record : records) { bulkRequest.add( - new IndexRequest(localIndexName) - .source(record.toXContent(XContentFactory.jsonBuilder(), ToXContent.EMPTY_PARAMS)) + new IndexRequest(localIndexName).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("successfully ingest data for %s! ", localIndexName)); - } - else { - log.error(String.format("error when ingesting data for %s", localIndexName)); + log.debug(String.format(Locale.ROOT, "successfully ingest data for %s! ", localIndexName)); + } else { + log.error(String.format(Locale.ROOT, "error when ingesting data for %s", localIndexName)); } } + @Override public void onFailure(Exception e) { - log.error(String.format("failed to ingest data for %s, %s", localIndexName, e)); + log.error(String.format(Locale.ROOT, "failed to ingest data for %s, %s", localIndexName, e)); } }); } @@ -203,8 +196,7 @@ public void onFailure(Exception e) { * @throws IOException if an error occurs */ private synchronized void indexRecord(T record) throws IOException { - IndexRequest indexRequest = new IndexRequest(localIndexName) - .setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE) + IndexRequest indexRequest = new IndexRequest(localIndexName).setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE) .source(record.toXContent(XContentFactory.jsonBuilder(), ToXContent.EMPTY_PARAMS)) .timeout(TimeValue.timeValueSeconds(60)); @@ -212,16 +204,15 @@ private synchronized void indexRecord(T record) throws IOException { @Override public void onResponse(IndexResponse response) { if (response.status().equals(RestStatus.CREATED) || response.status().equals(RestStatus.OK)) { - log.debug(String.format("successfully indexed data for %s ", localIndexName)); - } - else { - log.error(String.format("failed to index data for %s", localIndexName)); + log.debug(String.format(Locale.ROOT, "successfully indexed data for %s ", localIndexName)); + } else { + log.error(String.format(Locale.ROOT, "failed to index data for %s", localIndexName)); } } @Override public void onFailure(Exception e) { - log.error(String.format("failed to index data for %s, error: %s", localIndexName, e)); + log.error(String.format(Locale.ROOT, "failed to index data for %s, error: %s", localIndexName, e)); } }); } diff --git a/server/src/main/java/org/opensearch/action/admin/cluster/insights/top_queries/package-info.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/exporter/package-info.java similarity index 65% rename from server/src/main/java/org/opensearch/action/admin/cluster/insights/top_queries/package-info.java rename to plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/exporter/package-info.java index 170b56f2c16a6..3ccbfc5c4cb24 100644 --- a/server/src/main/java/org/opensearch/action/admin/cluster/insights/top_queries/package-info.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/exporter/package-info.java @@ -6,5 +6,7 @@ * compatible open source license. */ -/** Top n queries transport handlers. */ -package org.opensearch.action.admin.cluster.insights.top_queries; +/** + * 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/SearchQueryLatencyListener.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/listener/SearchQueryLatencyListener.java new file mode 100644 index 0000000000000..5bda8f1d9e751 --- /dev/null +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/listener/SearchQueryLatencyListener.java @@ -0,0 +1,90 @@ +/* + * 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_SIZE; +import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.TOP_N_LATENCY_QUERIES_WINDOW_SIZE; + +/** + * The listener for top N queries by latency + * + * @opensearch.internal + */ +public final class SearchQueryLatencyListener extends SearchRequestOperationsListener { + private static final ToXContent.Params FORMAT_PARAMS = new ToXContent.MapParams(Collections.singletonMap("pretty", "false")); + + private static final Logger log = LogManager.getLogger(SearchQueryLatencyListener.class); + + private final TopQueriesByLatencyService topQueriesByLatencyService; + + @Inject + public SearchQueryLatencyListener(ClusterService clusterService, TopQueriesByLatencyService topQueriesByLatencyService) { + this.topQueriesByLatencyService = topQueriesByLatencyService; + clusterService.getClusterSettings().addSettingsUpdateConsumer(TOP_N_LATENCY_QUERIES_ENABLED, this::setEnabled); + clusterService.getClusterSettings().addSettingsUpdateConsumer(TOP_N_LATENCY_QUERIES_SIZE, this.topQueriesByLatencyService::setTopNSize); + clusterService.getClusterSettings().addSettingsUpdateConsumer(TOP_N_LATENCY_QUERIES_WINDOW_SIZE, this.topQueriesByLatencyService::setWindowSize); + } + + @Override + public void setEnabled(boolean enabled){ + super.setEnabled(enabled); + this.topQueriesByLatencyService.setEnabled(enabled); + } + + @Override + public boolean getEnabled(){ + return super.getEnabled(); + } + + @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() + ); + } catch (Exception e) { + log.error(String.format(Locale.ROOT, "fail to ingest query insight data, error: %s", e)); + } + } +} diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/listener/package-info.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/listener/package-info.java new file mode 100644 index 0000000000000..3cb9cacf7fd1c --- /dev/null +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/listener/package-info.java @@ -0,0 +1,12 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +/** + * Listeners for Query Insights + */ +package org.opensearch.plugin.insights.core.listener; diff --git a/server/src/main/java/org/opensearch/action/search/QueryInsightService.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/QueryInsightService.java similarity index 78% rename from server/src/main/java/org/opensearch/action/search/QueryInsightService.java rename to plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/QueryInsightService.java index dd26850dfb496..ffe945d6f3d1b 100644 --- a/server/src/main/java/org/opensearch/action/search/QueryInsightService.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/QueryInsightService.java @@ -6,13 +6,15 @@ * compatible open source license. */ -package org.opensearch.action.search; +package org.opensearch.plugin.insights.core.service; 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.plugin.insights.core.exporter.QueryInsightExporter; +import org.opensearch.plugin.insights.rules.model.SearchQueryRecord; import org.opensearch.threadpool.Scheduler; import org.opensearch.threadpool.ThreadPool; @@ -20,6 +22,7 @@ import java.util.Collection; import java.util.Collections; import java.util.List; +import java.util.Locale; /** * Service responsible for gathering, analyzing, storing and exporting data related to @@ -48,14 +51,8 @@ public abstract class QueryInsightService, S exte private final ThreadPool threadPool; private volatile Scheduler.Cancellable scheduledFuture; - - public static final String TOP_N_QUERIES_PREFIX = "search.top_n_queries"; @Inject - public QueryInsightService( - ThreadPool threadPool, - @Nullable S store, - @Nullable E exporter - ) { + public QueryInsightService(ThreadPool threadPool, @Nullable S store, @Nullable E exporter) { this.threadPool = threadPool; this.store = store; this.exporter = exporter; @@ -115,15 +112,7 @@ public boolean getEnabled() { @Override protected void doStart() { if (exporter != null && exporter.getEnabled()) { - scheduledFuture = threadPool.scheduleWithFixedDelay(() -> { - List topQueries = getQueryData(); - try { - exporter.export(topQueries); - log.debug(String.format("finish exporting query insight data to sink %s", topQueries)); - } catch (Exception e) { - throw new RuntimeException(String.format("failed to export query insight data to sink, error: %s", e)); - } - }, exporter.getExportInterval(), ThreadPool.Names.GENERIC); + scheduledFuture = threadPool.scheduleWithFixedDelay(this::doExportAndClear, exporter.getExportInterval(), ThreadPool.Names.GENERIC); } } @@ -134,10 +123,23 @@ protected void doStart() { protected void doStop() { if (scheduledFuture != null) { scheduledFuture.cancel(); + if (exporter != null && exporter.getEnabled()) { + doExportAndClear(); + } } } - @Override - protected void doClose() { + private void doExportAndClear() { + List storedData = getQueryData(); + try { + exporter.export(storedData); + clearAllData(); + log.debug(String.format(Locale.ROOT, "finish exporting query insight data to sink %s", storedData)); + } catch (Exception e) { + throw new RuntimeException(String.format(Locale.ROOT, "failed to export query insight data to sink, error: %s", e)); + } } + + @Override + protected void doClose() {} } diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/TopQueriesByLatencyService.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/TopQueriesByLatencyService.java new file mode 100644 index 0000000000000..96368126d738d --- /dev/null +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/TopQueriesByLatencyService.java @@ -0,0 +1,124 @@ +/* + * 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.QueryInsightLocalIndexExporter; +import org.opensearch.plugin.insights.rules.model.SearchQueryLatencyRecord; +import org.opensearch.threadpool.ThreadPool; + +import java.util.Locale; +import java.util.Map; +import java.util.concurrent.PriorityBlockingQueue; + +/** + * Service responsible for gathering, analyzing, storing and exporting + * top N queries with high latency data for search queries + * + * @opensearch.internal + */ +public class TopQueriesByLatencyService extends QueryInsightService< + SearchQueryLatencyRecord, + PriorityBlockingQueue, + QueryInsightLocalIndexExporter> { + private static final Logger log = LogManager.getLogger(TopQueriesByLatencyService.class); + + /** 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 */ + public static final int DEFAULT_TOP_N_SIZE = 3; + + private int topNSize = DEFAULT_TOP_N_SIZE; + + private TimeValue windowSize = TimeValue.timeValueSeconds(DEFAULT_WINDOW_SIZE); + + @Inject + public TopQueriesByLatencyService(ThreadPool threadPool, ClusterService clusterService, Client client) { + super(threadPool, new PriorityBlockingQueue<>(), null); + } + + /** + * 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 + */ + public void ingestQueryData( + final Long timestamp, + final SearchType searchType, + final String source, + final int totalShards, + final String[] indices, + final Map propertyMap, + final Map phaseLatencyMap + ) { + 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; + } + super.ingestQueryData( + new SearchQueryLatencyRecord(timestamp, searchType, source, totalShards, indices, propertyMap, phaseLatencyMap) + ); + // remove top elements for fix sizing priority queue + if (this.store.size() > this.getTopNSize()) { + this.store.poll(); + } + log.debug(String.format(Locale.ROOT, "successfully ingested: %s", this.store)); + } + + @Override + public void clearOutdatedData() { + store.removeIf(record -> record.getTimestamp() < System.currentTimeMillis() - windowSize.getMillis()); + } + + public void setTopNSize(int size) { + this.topNSize = size; + } + + public void setWindowSize(TimeValue windowSize) { + this.windowSize = windowSize; + } + + public int getTopNSize() { + return this.topNSize; + } + + public TimeValue getWindowSize() { + return this.windowSize; + } + +} diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/package-info.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/package-info.java new file mode 100644 index 0000000000000..5068f28234f6d --- /dev/null +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/package-info.java @@ -0,0 +1,12 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +/** + * Service Classes for Query Insights + */ +package org.opensearch.plugin.insights.core.service; diff --git a/server/src/main/java/org/opensearch/action/admin/cluster/insights/package-info.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/package-info.java similarity index 69% rename from server/src/main/java/org/opensearch/action/admin/cluster/insights/package-info.java rename to plugins/query-insights/src/main/java/org/opensearch/plugin/insights/package-info.java index 392958c22afe8..04d1f9bfff7e1 100644 --- a/server/src/main/java/org/opensearch/action/admin/cluster/insights/package-info.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/package-info.java @@ -6,5 +6,7 @@ * compatible open source license. */ -/** insights transport handlers. */ -package org.opensearch.action.admin.cluster.insights; +/** + * Base Package of Query Insights + */ +package org.opensearch.plugin.insights; diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/package-info.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/package-info.java new file mode 100644 index 0000000000000..9b6b5856f7d27 --- /dev/null +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/package-info.java @@ -0,0 +1,12 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +/** + * Transport Actions, Requests and Responses for Query Insights + */ +package org.opensearch.plugin.insights.rules.action; diff --git a/server/src/main/java/org/opensearch/action/admin/cluster/insights/top_queries/TopQueries.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueries.java similarity index 82% rename from server/src/main/java/org/opensearch/action/admin/cluster/insights/top_queries/TopQueries.java rename to plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueries.java index 44db5e30788fb..1bb250e5d57b7 100644 --- a/server/src/main/java/org/opensearch/action/admin/cluster/insights/top_queries/TopQueries.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueries.java @@ -6,18 +6,16 @@ * compatible open source license. */ +package org.opensearch.plugin.insights.rules.action.top_queries; -package org.opensearch.action.admin.cluster.insights.top_queries; - -import org.opensearch.action.search.SearchQueryLatencyRecord; 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.ToXContent; import org.opensearch.core.xcontent.ToXContentObject; import org.opensearch.core.xcontent.XContentBuilder; +import org.opensearch.plugin.insights.rules.model.SearchQueryLatencyRecord; import java.io.IOException; import java.util.List; @@ -39,16 +37,13 @@ public TopQueries(StreamInput in) throws IOException { latencyRecords = in.readList(SearchQueryLatencyRecord::new); } - public TopQueries( - DiscoveryNode node, - @Nullable List latencyRecords - ) { + public TopQueries(DiscoveryNode node, @Nullable List latencyRecords) { super(node); this.latencyRecords = latencyRecords; } @Override - public XContentBuilder toXContent(XContentBuilder builder, ToXContent.Params params) throws IOException { + public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { if (latencyRecords != null) { for (SearchQueryLatencyRecord record : latencyRecords) { record.toXContent(builder, params); diff --git a/server/src/main/java/org/opensearch/action/admin/cluster/insights/top_queries/TopQueriesAction.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesAction.java similarity index 90% rename from server/src/main/java/org/opensearch/action/admin/cluster/insights/top_queries/TopQueriesAction.java rename to plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesAction.java index 614abeb370ca8..69abf001d18d9 100644 --- a/server/src/main/java/org/opensearch/action/admin/cluster/insights/top_queries/TopQueriesAction.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesAction.java @@ -6,8 +6,7 @@ * compatible open source license. */ - -package org.opensearch.action.admin.cluster.insights.top_queries; +package org.opensearch.plugin.insights.rules.action.top_queries; import org.opensearch.action.ActionType; diff --git a/server/src/main/java/org/opensearch/action/admin/cluster/insights/top_queries/TopQueriesRequest.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesRequest.java similarity index 91% rename from server/src/main/java/org/opensearch/action/admin/cluster/insights/top_queries/TopQueriesRequest.java rename to plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesRequest.java index 24e389da59442..435f5188a4e0b 100644 --- a/server/src/main/java/org/opensearch/action/admin/cluster/insights/top_queries/TopQueriesRequest.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesRequest.java @@ -6,8 +6,7 @@ * compatible open source license. */ - -package org.opensearch.action.admin.cluster.insights.top_queries; +package org.opensearch.plugin.insights.rules.action.top_queries; import org.opensearch.action.support.nodes.BaseNodesRequest; import org.opensearch.common.annotation.PublicApi; @@ -16,13 +15,14 @@ import java.io.IOException; import java.util.Arrays; +import java.util.Locale; import java.util.Set; import java.util.stream.Collectors; /** * A request to get cluster/node level top queries information. * - * @opensearch.api + * @opensearch.internal */ @PublicApi(since = "1.0.0") public class TopQueriesRequest extends BaseNodesRequest { @@ -59,15 +59,14 @@ public Metric getMetricType() { * Set the type of requested metrics */ public TopQueriesRequest setMetricType(String metricType) { - metricType = metricType.toUpperCase(); - if (Metric.allMetrics().contains(metricType) == false) { + 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); diff --git a/server/src/main/java/org/opensearch/action/admin/cluster/insights/top_queries/TopQueriesResponse.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesResponse.java similarity index 91% rename from server/src/main/java/org/opensearch/action/admin/cluster/insights/top_queries/TopQueriesResponse.java rename to plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesResponse.java index c9d76cf3a1dd0..efa89538551c7 100644 --- a/server/src/main/java/org/opensearch/action/admin/cluster/insights/top_queries/TopQueriesResponse.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesResponse.java @@ -6,11 +6,9 @@ * compatible open source license. */ - -package org.opensearch.action.admin.cluster.insights.top_queries; +package org.opensearch.plugin.insights.rules.action.top_queries; import org.opensearch.action.FailedNodeException; -import org.opensearch.action.search.SearchQueryLatencyRecord; import org.opensearch.action.support.nodes.BaseNodesResponse; import org.opensearch.cluster.ClusterName; import org.opensearch.common.annotation.PublicApi; @@ -19,6 +17,7 @@ import org.opensearch.core.common.io.stream.StreamOutput; import org.opensearch.core.xcontent.ToXContentFragment; import org.opensearch.core.xcontent.XContentBuilder; +import org.opensearch.plugin.insights.rules.model.SearchQueryLatencyRecord; import java.io.IOException; import java.util.Collection; @@ -29,7 +28,7 @@ /** * Transport response for cluster/node level top queries information. * - * @opensearch.api + * @opensearch.internal */ @PublicApi(since = "1.0.0") public class TopQueriesResponse extends BaseNodesResponse implements ToXContentFragment { @@ -57,8 +56,9 @@ protected void writeNodesTo(StreamOutput out, List nodes) throws IOE @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { List results = getNodes(); + builder.startObject(); toClusterLevelResult(builder, params, results); - return builder; + return builder.endObject(); } @Override @@ -85,7 +85,9 @@ public String toString() { private void toClusterLevelResult(XContentBuilder builder, Params params, List results) throws IOException { List all_records = results.stream() .map(TopQueries::getLatencyRecords) - .flatMap(Collection::stream).sorted(Collections.reverseOrder()).collect(Collectors.toList()); + .flatMap(Collection::stream) + .sorted(Collections.reverseOrder()) + .collect(Collectors.toList()); builder.startArray(CLUSTER_LEVEL_RESULTS_KEY); for (SearchQueryLatencyRecord record : all_records) { record.toXContent(builder, params); diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/package-info.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/package-info.java new file mode 100644 index 0000000000000..3cc7900e5ce7d --- /dev/null +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/package-info.java @@ -0,0 +1,12 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +/** + * Transport Actions, Requests and Responses for Top N Queries + */ +package org.opensearch.plugin.insights.rules.action.top_queries; diff --git a/server/src/main/java/org/opensearch/action/search/SearchQueryLatencyRecord.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model/SearchQueryLatencyRecord.java similarity index 88% rename from server/src/main/java/org/opensearch/action/search/SearchQueryLatencyRecord.java rename to plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model/SearchQueryLatencyRecord.java index 0357714c4d76e..5a08dfa462700 100644 --- a/server/src/main/java/org/opensearch/action/search/SearchQueryLatencyRecord.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model/SearchQueryLatencyRecord.java @@ -6,8 +6,9 @@ * compatible open source license. */ -package org.opensearch.action.search; +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; @@ -40,15 +41,8 @@ public SearchQueryLatencyRecord( final String[] indices, final Map propertyMap, final Map phaseLatencyMap - ) { - super( - timestamp, - searchType, - source, - totalShards, - indices, - propertyMap, - phaseLatencyMap.values().stream().mapToLong(x -> x).sum()); + ) { + super(timestamp, searchType, source, totalShards, indices, propertyMap, phaseLatencyMap.values().stream().mapToLong(x -> x).sum()); this.phaseLatencyMap = phaseLatencyMap; } diff --git a/server/src/main/java/org/opensearch/action/search/SearchQueryRecord.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model/SearchQueryRecord.java similarity index 81% rename from server/src/main/java/org/opensearch/action/search/SearchQueryRecord.java rename to plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model/SearchQueryRecord.java index e935918016c84..0c875105b856e 100644 --- a/server/src/main/java/org/opensearch/action/search/SearchQueryRecord.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model/SearchQueryRecord.java @@ -6,15 +6,20 @@ * compatible open source license. */ -package org.opensearch.action.search; +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.ToXContentObject; import org.opensearch.core.xcontent.XContentBuilder; +import org.opensearch.plugin.insights.core.listener.SearchQueryLatencyListener; import java.io.IOException; +import java.util.Locale; import java.util.Map; /** @@ -23,7 +28,13 @@ * @param The value type associated with the record * @opensearch.internal */ -public abstract class SearchQueryRecord > implements Comparable>, Writeable, ToXContentObject { +public abstract class SearchQueryRecord> + implements + Comparable>, + Writeable, + ToXContentObject { + + private static final Logger log = LogManager.getLogger(SearchQueryRecord.class); protected static final String TIMESTAMP = "timestamp"; protected static final String SEARCH_TYPE = "searchType"; protected static final String SOURCE = "source"; @@ -47,14 +58,14 @@ public abstract class SearchQueryRecord > imple private T value; - public SearchQueryRecord(final StreamInput in) throws IOException { + public SearchQueryRecord(final StreamInput in) throws IOException, ClassCastException { this.timestamp = in.readLong(); - this.searchType = SearchType.fromString(in.readString().toLowerCase()); + 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.value = (T) in.readGenericValue(); + this.value = castToValue(in.readGenericValue()); } public SearchQueryRecord( @@ -65,15 +76,8 @@ public SearchQueryRecord( final String[] indices, final Map propertyMap, final T value - ) { - this ( - timestamp, - searchType, - source, - totalShards, - indices, - propertyMap - ); + ) { + this(timestamp, searchType, source, totalShards, indices, propertyMap); this.value = value; } @@ -154,6 +158,16 @@ public int compareTo(SearchQueryRecord otherRecord) { return value.compareTo(otherRecord.getValue()); } + @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; + } + } + @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { builder.startObject(); diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model/package-info.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model/package-info.java new file mode 100644 index 0000000000000..c59ec1550f54b --- /dev/null +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model/package-info.java @@ -0,0 +1,12 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +/** + * Data Models for Query Insight Records + */ +package org.opensearch.plugin.insights.rules.model; diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/resthandler/package-info.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/resthandler/package-info.java new file mode 100644 index 0000000000000..3787f05f65552 --- /dev/null +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/resthandler/package-info.java @@ -0,0 +1,12 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +/** + * Rest Handlers for Query Insights + */ +package org.opensearch.plugin.insights.rules.resthandler; diff --git a/server/src/main/java/org/opensearch/rest/action/admin/cluster/RestTopQueriesAction.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/resthandler/top_queries/RestTopQueriesAction.java similarity index 55% rename from server/src/main/java/org/opensearch/rest/action/admin/cluster/RestTopQueriesAction.java rename to plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/resthandler/top_queries/RestTopQueriesAction.java index 3c7029e982f70..874df86ebb4fc 100644 --- a/server/src/main/java/org/opensearch/rest/action/admin/cluster/RestTopQueriesAction.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/resthandler/top_queries/RestTopQueriesAction.java @@ -6,24 +6,29 @@ * compatible open source license. */ +package org.opensearch.plugin.insights.rules.resthandler.top_queries; -package org.opensearch.rest.action.admin.cluster; - -import org.opensearch.action.admin.cluster.insights.top_queries.TopQueriesRequest; import org.opensearch.client.node.NodeClient; import org.opensearch.common.settings.Settings; import org.opensearch.core.common.Strings; +import org.opensearch.core.rest.RestStatus; +import org.opensearch.core.xcontent.ToXContent; +import org.opensearch.plugin.insights.rules.action.top_queries.TopQueriesAction; +import org.opensearch.plugin.insights.rules.action.top_queries.TopQueriesRequest; +import org.opensearch.plugin.insights.rules.action.top_queries.TopQueriesResponse; import org.opensearch.rest.BaseRestHandler; +import org.opensearch.rest.BytesRestResponse; +import org.opensearch.rest.RestChannel; import org.opensearch.rest.RestRequest; -import org.opensearch.rest.action.RestActions.NodesResponseRestListener; +import org.opensearch.rest.RestResponse; +import org.opensearch.rest.action.RestResponseListener; import java.io.IOException; import java.util.List; import java.util.Locale; import java.util.Set; -import static java.util.Arrays.asList; -import static java.util.Collections.unmodifiableList; +import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.TOP_QUERIES_BASE_URI; import static org.opensearch.rest.RestRequest.Method.GET; /** @@ -39,11 +44,9 @@ public RestTopQueriesAction() {} @Override public List routes() { - return unmodifiableList( - asList( - new Route(GET, "/_insights/top_queries"), - new Route(GET, "/_insights/top_queries/{nodeId}") - ) + return List.of( + new Route(GET, TOP_QUERIES_BASE_URI), + new Route(GET, String.format(Locale.ROOT, "%s/{nodeId}", TOP_QUERIES_BASE_URI)) ); } @@ -57,20 +60,20 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC final TopQueriesRequest topQueriesRequest = prepareRequest(request); topQueriesRequest.timeout(request.param("timeout")); - return channel -> client.admin().cluster().topQueries(topQueriesRequest, new NodesResponseRestListener<>(channel)); + return channel -> client.execute( + TopQueriesAction.INSTANCE, + topQueriesRequest, + topQueriesResponse(channel, request.method()) + + ); } static TopQueriesRequest prepareRequest(final RestRequest request) { String[] nodesIds = Strings.splitStringByCommaToArray(request.param("nodeId")); - String metricType = request.param("type", TopQueriesRequest.Metric.LATENCY.metricName()).toUpperCase(); + String metricType = request.param("type", TopQueriesRequest.Metric.LATENCY.metricName()).toUpperCase(Locale.ROOT); if (!ALLOWED_METRICS.contains(metricType)) { throw new IllegalArgumentException( - String.format( - Locale.ROOT, - "request [%s] contains invalid metric type [%s]", - request.path(), - metricType - ) + String.format(Locale.ROOT, "request [%s] contains invalid metric type [%s]", request.path(), metricType) ); } TopQueriesRequest topQueriesRequest = new TopQueriesRequest(nodesIds); @@ -87,4 +90,13 @@ protected Set responseParams() { public boolean canTripCircuitBreaker() { return false; } + + private RestResponseListener topQueriesResponse(RestChannel channel, RestRequest.Method restMethod) { + return new RestResponseListener<>(channel) { + @Override + public RestResponse buildResponse(TopQueriesResponse response) throws Exception { + return new BytesRestResponse(RestStatus.OK, response.toXContent(channel.newBuilder(), ToXContent.EMPTY_PARAMS)); + } + }; + } } diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/resthandler/top_queries/package-info.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/resthandler/top_queries/package-info.java new file mode 100644 index 0000000000000..087cf7d765f8c --- /dev/null +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/resthandler/top_queries/package-info.java @@ -0,0 +1,12 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +/** + * Rest Handlers for Top N Queries + */ +package org.opensearch.plugin.insights.rules.resthandler.top_queries; diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/transport/package-info.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/transport/package-info.java new file mode 100644 index 0000000000000..f3a1c70b9af57 --- /dev/null +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/transport/package-info.java @@ -0,0 +1,12 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +/** + * Transport Actions for Query Insights. + */ +package org.opensearch.plugin.insights.rules.transport; diff --git a/server/src/main/java/org/opensearch/action/admin/cluster/insights/top_queries/TransportTopQueriesAction.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/transport/top_queries/TransportTopQueriesAction.java similarity index 77% rename from server/src/main/java/org/opensearch/action/admin/cluster/insights/top_queries/TransportTopQueriesAction.java rename to plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/transport/top_queries/TransportTopQueriesAction.java index 62f8750f6e0bb..0048a989cb0dd 100644 --- a/server/src/main/java/org/opensearch/action/admin/cluster/insights/top_queries/TransportTopQueriesAction.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/transport/top_queries/TransportTopQueriesAction.java @@ -6,24 +6,28 @@ * compatible open source license. */ - -package org.opensearch.action.admin.cluster.insights.top_queries; +package org.opensearch.plugin.insights.rules.transport.top_queries; import org.opensearch.OpenSearchException; import org.opensearch.action.FailedNodeException; -import org.opensearch.action.search.QueryLatencyInsightService; import org.opensearch.action.support.ActionFilters; import org.opensearch.action.support.nodes.TransportNodesAction; import org.opensearch.cluster.service.ClusterService; import org.opensearch.common.inject.Inject; import org.opensearch.core.common.io.stream.StreamInput; import org.opensearch.core.common.io.stream.StreamOutput; +import org.opensearch.plugin.insights.core.service.TopQueriesByLatencyService; +import org.opensearch.plugin.insights.rules.action.top_queries.TopQueries; +import org.opensearch.plugin.insights.rules.action.top_queries.TopQueriesAction; +import org.opensearch.plugin.insights.rules.action.top_queries.TopQueriesRequest; +import org.opensearch.plugin.insights.rules.action.top_queries.TopQueriesResponse; import org.opensearch.threadpool.ThreadPool; import org.opensearch.transport.TransportRequest; import org.opensearch.transport.TransportService; import java.io.IOException; import java.util.List; +import java.util.Locale; /** * Transport action for cluster/node level top queries information. @@ -36,14 +40,14 @@ public class TransportTopQueriesAction extends TransportNodesAction< TransportTopQueriesAction.NodeRequest, TopQueries> { - private final QueryLatencyInsightService queryLatencyInsightService; + private final TopQueriesByLatencyService topQueriesByLatencyService; @Inject public TransportTopQueriesAction( ThreadPool threadPool, ClusterService clusterService, TransportService transportService, - QueryLatencyInsightService queryLatencyInsightService, + TopQueriesByLatencyService topQueriesByLatencyService, ActionFilters actionFilters ) { super( @@ -57,7 +61,7 @@ public TransportTopQueriesAction( ThreadPool.Names.GENERIC, TopQueries.class ); - this.queryLatencyInsightService = queryLatencyInsightService; + this.topQueriesByLatencyService = topQueriesByLatencyService; } @Override @@ -83,12 +87,9 @@ protected TopQueries newNodeResponse(StreamInput in) throws IOException { protected TopQueries nodeOperation(NodeRequest nodeRequest) { TopQueriesRequest request = nodeRequest.request; if (request.getMetricType() == TopQueriesRequest.Metric.LATENCY) { - return new TopQueries( - clusterService.localNode(), - queryLatencyInsightService.getQueryData() - ); + return new TopQueries(clusterService.localNode(), topQueriesByLatencyService.getQueryData()); } else { - throw new OpenSearchException(String.format("invalid metric type %s", request.getMetricType())); + 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/rules/transport/top_queries/package-info.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/transport/top_queries/package-info.java new file mode 100644 index 0000000000000..54da0980deff8 --- /dev/null +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/transport/top_queries/package-info.java @@ -0,0 +1,12 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +/** + * Transport Actions for Top N Queries. + */ +package org.opensearch.plugin.insights.rules.transport.top_queries; diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/settings/QueryInsightsSettings.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/settings/QueryInsightsSettings.java new file mode 100644 index 0000000000000..4ec741fef03cc --- /dev/null +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/settings/QueryInsightsSettings.java @@ -0,0 +1,72 @@ +/* + * 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.settings; + +import org.opensearch.common.settings.Setting; +import org.opensearch.common.unit.TimeValue; + +import java.util.concurrent.TimeUnit; +import static org.opensearch.plugin.insights.core.service.TopQueriesByLatencyService.DEFAULT_TOP_N_SIZE; +import static org.opensearch.plugin.insights.core.service.TopQueriesByLatencyService.DEFAULT_WINDOW_SIZE; + +/** + * Settings for Query Insights Plugin + * + * @opensearch.api + * @opensearch.experimental + */ +public class QueryInsightsSettings { + /** + * Query Insights base uri + */ + public static final String PLUGINS_BASE_URI = "/_insights"; + + /** + * Settings for Top Queries + * + */ + public static final String TOP_QUERIES_BASE_URI = PLUGINS_BASE_URI + "/top_queries"; + public static final String TOP_N_QUERIES_SETTING_PREFIX = "search.top_n_queries"; + + public static final String TOP_N_LATENCY_QUERIES_PREFIX = TOP_N_QUERIES_SETTING_PREFIX + ".latency"; + /** + * Boolean setting for enabling top queries by latency. + */ + public static final Setting TOP_N_LATENCY_QUERIES_ENABLED = Setting.boolSetting( + TOP_N_LATENCY_QUERIES_PREFIX + ".enabled", + false, + Setting.Property.Dynamic, + Setting.Property.NodeScope + ); + + /** + * Int setting to define the top n size for top queries by latency. + */ + public static final Setting TOP_N_LATENCY_QUERIES_SIZE = Setting.intSetting( + TOP_N_LATENCY_QUERIES_PREFIX + ".top_n_size", + DEFAULT_TOP_N_SIZE, + Setting.Property.Dynamic, + Setting.Property.NodeScope + ); + + /** + * Time setting to define the window size in seconds for top queries by latency. + */ + public static final Setting TOP_N_LATENCY_QUERIES_WINDOW_SIZE = Setting.positiveTimeSetting( + TOP_N_LATENCY_QUERIES_PREFIX + ".window_size", + new TimeValue(DEFAULT_WINDOW_SIZE, TimeUnit.SECONDS), + Setting.Property.NodeScope, + Setting.Property.Dynamic + ); + + /** + * Default constructor + */ + public QueryInsightsSettings() {} +} diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/settings/package-info.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/settings/package-info.java new file mode 100644 index 0000000000000..f3152bbf966cb --- /dev/null +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/settings/package-info.java @@ -0,0 +1,12 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +/** + * Settings for Query Insights Plugin + */ +package org.opensearch.plugin.insights.settings; diff --git a/server/src/main/resources/org/opensearch/action/search/mappings/top_n_queries_record.json b/plugins/query-insights/src/main/resources/mappings/top_n_queries_record.json similarity index 100% rename from server/src/main/resources/org/opensearch/action/search/mappings/top_n_queries_record.json rename to plugins/query-insights/src/main/resources/mappings/top_n_queries_record.json 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 new file mode 100644 index 0000000000000..45255b055984f --- /dev/null +++ b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/QueryInsightsPluginTests.java @@ -0,0 +1,19 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.plugin.insights; + +import org.opensearch.test.OpenSearchTestCase; +import org.junit.Assert; + +public class QueryInsightsPluginTests extends OpenSearchTestCase { + + public void testDummy() { + Assert.assertEquals(1, 1); + } +} diff --git a/server/src/test/java/org/opensearch/action/search/SearchQueryLatencyListenerTests.java b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/listener/SearchQueryLatencyListenerTests.java similarity index 63% rename from server/src/test/java/org/opensearch/action/search/SearchQueryLatencyListenerTests.java rename to plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/listener/SearchQueryLatencyListenerTests.java index afa97c8eda1e7..604bc35c2f7cb 100644 --- a/server/src/test/java/org/opensearch/action/search/SearchQueryLatencyListenerTests.java +++ b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/listener/SearchQueryLatencyListenerTests.java @@ -6,12 +6,22 @@ * compatible open source license. */ -package org.opensearch.action.search; +package org.opensearch.plugin.insights.core.listener; +import org.opensearch.action.search.SearchRequestContext; +import org.opensearch.action.search.SearchPhaseContext; +import org.opensearch.action.search.SearchType; +import org.opensearch.action.search.SearchRequest; + +import org.opensearch.cluster.service.ClusterService; +import org.opensearch.common.settings.ClusterSettings; +import org.opensearch.common.settings.Settings; +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.opensearch.plugin.insights.core.service.TopQueriesByLatencyService; import java.util.ArrayList; import java.util.HashMap; @@ -20,7 +30,6 @@ import java.util.concurrent.CountDownLatch; import java.util.concurrent.Phaser; -import static org.mockito.Mockito.any; import static org.mockito.Mockito.anyMap; import static org.mockito.Mockito.eq; import static org.mockito.Mockito.mock; @@ -34,7 +43,17 @@ public void testOnRequestEnd() { final SearchRequestContext searchRequestContext = mock(SearchRequestContext.class); final SearchPhaseContext searchPhaseContext = mock(SearchPhaseContext.class); final SearchRequest searchRequest = mock(SearchRequest.class); - final QueryLatencyInsightService queryLatencyInsightService = mock(QueryLatencyInsightService.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); + + ClusterService clusterService = new ClusterService(settings, clusterSettings, null); Long timestamp = System.currentTimeMillis() - 100L; SearchType searchType = SearchType.QUERY_THEN_FETCH; @@ -56,15 +75,8 @@ public void testOnRequestEnd() { int numberOfShards = 10; + SearchQueryLatencyListener searchQueryLatencyListener = new SearchQueryLatencyListener(clusterService, topQueriesByLatencyService); - SearchQueryLatencyListener searchQueryLatencyListener = new SearchQueryLatencyListener(queryLatencyInsightService); - final List searchListenersList = new ArrayList<>( - List.of(searchQueryLatencyListener) - ); - - when(searchRequestContext.getSearchRequestOperationsListener()).thenReturn( - new SearchRequestOperationsListener.CompositeListener(searchListenersList, logger) - ); when(searchRequest.getOrCreateAbsoluteStartMillis()).thenReturn(timestamp); when(searchRequest.searchType()).thenReturn(searchType); when(searchRequest.source()).thenReturn(searchSourceBuilder); @@ -74,9 +86,9 @@ public void testOnRequestEnd() { when(searchPhaseContext.getNumShards()).thenReturn(numberOfShards); - searchRequestContext.getSearchRequestOperationsListener().onRequestEnd(searchPhaseContext, searchRequestContext); + searchQueryLatencyListener.onRequestEnd(searchPhaseContext, searchRequestContext); - verify(queryLatencyInsightService, times(1)).ingestQueryData( + verify(topQueriesByLatencyService, times(1)).ingestQueryData( eq(timestamp), eq(searchType), eq(searchSourceBuilder.toString()), @@ -88,9 +100,20 @@ public void testOnRequestEnd() { } public void testConcurrentOnRequestEnd() throws InterruptedException { + final SearchRequestContext searchRequestContext = mock(SearchRequestContext.class); final SearchPhaseContext searchPhaseContext = mock(SearchPhaseContext.class); final SearchRequest searchRequest = mock(SearchRequest.class); - final QueryLatencyInsightService queryLatencyInsightService = mock(QueryLatencyInsightService.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); + + ClusterService clusterService = new ClusterService(settings, clusterSettings, null); + Long timestamp = System.currentTimeMillis() - 100L; SearchType searchType = SearchType.QUERY_THEN_FETCH; @@ -112,16 +135,13 @@ public void testConcurrentOnRequestEnd() throws InterruptedException { int numberOfShards = 10; - - SearchQueryLatencyListener searchQueryLatencyListener = new SearchQueryLatencyListener(queryLatencyInsightService); - final List searchListenersList = new ArrayList<>( - List.of(searchQueryLatencyListener) - ); + final List searchListenersList = new ArrayList<>(); when(searchRequest.getOrCreateAbsoluteStartMillis()).thenReturn(timestamp); when(searchRequest.searchType()).thenReturn(searchType); when(searchRequest.source()).thenReturn(searchSourceBuilder); when(searchRequest.indices()).thenReturn(indices); + when(searchRequestContext.phaseTookMap()).thenReturn(phaseLatencyMap); when(searchPhaseContext.getRequest()).thenReturn(searchRequest); when(searchPhaseContext.getNumShards()).thenReturn(numberOfShards); @@ -131,23 +151,18 @@ public void testConcurrentOnRequestEnd() throws InterruptedException { Phaser phaser = new Phaser(numRequests + 1); CountDownLatch countDownLatch = new CountDownLatch(numRequests); - ArrayList searchRequestContexts = new ArrayList<>(); for (int i = 0; i < numRequests; i++) { - SearchRequestContext searchRequestContext = new SearchRequestContext( - new SearchRequestOperationsListener.CompositeListener(searchListenersList, logger) + searchListenersList.add( + new SearchQueryLatencyListener(clusterService, topQueriesByLatencyService) ); - phaseLatencyMap.forEach(searchRequestContext::updatePhaseTookMap); - searchRequestContexts.add(searchRequestContext); } for (int i = 0; i < numRequests; i++) { int finalI = i; threads[i] = new Thread(() -> { phaser.arriveAndAwaitAdvance(); - SearchRequestContext thisContext = searchRequestContexts.get(finalI); - thisContext - .getSearchRequestOperationsListener() - .onRequestEnd(searchPhaseContext, thisContext); + SearchQueryLatencyListener thisListener = searchListenersList.get(finalI); + thisListener.onRequestEnd(searchPhaseContext, searchRequestContext); countDownLatch.countDown(); }); threads[i].start(); @@ -155,7 +170,7 @@ public void testConcurrentOnRequestEnd() throws InterruptedException { phaser.arriveAndAwaitAdvance(); countDownLatch.await(); - verify(queryLatencyInsightService, times(numRequests)).ingestQueryData( + verify(topQueriesByLatencyService, times(numRequests)).ingestQueryData( eq(timestamp), eq(searchType), eq(searchSourceBuilder.toString()), diff --git a/server/src/test/java/org/opensearch/action/admin/cluster/insights/top_queries/TopQueriesRequestTests.java b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesRequestTests.java similarity index 95% rename from server/src/test/java/org/opensearch/action/admin/cluster/insights/top_queries/TopQueriesRequestTests.java rename to plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesRequestTests.java index ae63031fab835..188bf7b931383 100644 --- a/server/src/test/java/org/opensearch/action/admin/cluster/insights/top_queries/TopQueriesRequestTests.java +++ b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesRequestTests.java @@ -7,7 +7,7 @@ */ -package org.opensearch.action.admin.cluster.insights.top_queries; +package org.opensearch.plugin.insights.rules.action.top_queries; import org.opensearch.common.io.stream.BytesStreamOutput; import org.opensearch.core.common.io.stream.StreamInput; diff --git a/server/src/test/java/org/opensearch/action/admin/cluster/insights/top_queries/TopQueriesTests.java b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesTests.java similarity index 96% rename from server/src/test/java/org/opensearch/action/admin/cluster/insights/top_queries/TopQueriesTests.java rename to plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesTests.java index c1a849436a93f..8e1fd77b4feba 100644 --- a/server/src/test/java/org/opensearch/action/admin/cluster/insights/top_queries/TopQueriesTests.java +++ b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesTests.java @@ -7,9 +7,8 @@ */ -package org.opensearch.action.admin.cluster.insights.top_queries; +package org.opensearch.plugin.insights.rules.action.top_queries; -import org.opensearch.action.search.SearchQueryLatencyRecord; import org.opensearch.action.search.SearchType; import org.opensearch.cluster.node.DiscoveryNode; import org.opensearch.common.io.stream.BytesStreamOutput; @@ -19,6 +18,8 @@ import org.opensearch.test.OpenSearchTestCase; import org.opensearch.test.VersionUtils; +import org.opensearch.plugin.insights.rules.model.SearchQueryLatencyRecord; + import java.io.IOException; import java.util.ArrayList; import java.util.HashMap; diff --git a/server/src/test/java/org/opensearch/rest/action/admin/cluster/RestTopQueriesActionTests.java b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/resthandler/top_queries/RestTopQueriesActionTests.java similarity index 89% rename from server/src/test/java/org/opensearch/rest/action/admin/cluster/RestTopQueriesActionTests.java rename to plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/resthandler/top_queries/RestTopQueriesActionTests.java index 583b58f03fe0c..f0092a48ccc95 100644 --- a/server/src/test/java/org/opensearch/rest/action/admin/cluster/RestTopQueriesActionTests.java +++ b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/resthandler/top_queries/RestTopQueriesActionTests.java @@ -7,9 +7,9 @@ */ -package org.opensearch.rest.action.admin.cluster; +package org.opensearch.plugin.insights.rules.resthandler.top_queries; -import org.opensearch.action.admin.cluster.insights.top_queries.TopQueriesRequest; +import org.opensearch.plugin.insights.rules.action.top_queries.TopQueriesRequest; import org.opensearch.rest.RestRequest; import org.opensearch.test.OpenSearchTestCase; import org.opensearch.test.rest.FakeRestRequest; @@ -17,7 +17,7 @@ import java.util.HashMap; import java.util.Map; -import static org.opensearch.rest.action.admin.cluster.RestTopQueriesAction.ALLOWED_METRICS; +import static org.opensearch.plugin.insights.rules.resthandler.top_queries.RestTopQueriesAction.ALLOWED_METRICS; public class RestTopQueriesActionTests extends OpenSearchTestCase { diff --git a/server/src/main/java/org/opensearch/action/ActionModule.java b/server/src/main/java/org/opensearch/action/ActionModule.java index a2817778b144a..46775466aa615 100644 --- a/server/src/main/java/org/opensearch/action/ActionModule.java +++ b/server/src/main/java/org/opensearch/action/ActionModule.java @@ -48,8 +48,6 @@ import org.opensearch.action.admin.cluster.decommission.awareness.put.TransportDecommissionAction; import org.opensearch.action.admin.cluster.health.ClusterHealthAction; import org.opensearch.action.admin.cluster.health.TransportClusterHealthAction; -import org.opensearch.action.admin.cluster.insights.top_queries.TopQueriesAction; -import org.opensearch.action.admin.cluster.insights.top_queries.TransportTopQueriesAction; import org.opensearch.action.admin.cluster.node.hotthreads.NodesHotThreadsAction; import org.opensearch.action.admin.cluster.node.hotthreads.TransportNodesHotThreadsAction; import org.opensearch.action.admin.cluster.node.info.NodesInfoAction; @@ -361,7 +359,6 @@ import org.opensearch.rest.action.admin.cluster.RestRestoreRemoteStoreAction; import org.opensearch.rest.action.admin.cluster.RestRestoreSnapshotAction; import org.opensearch.rest.action.admin.cluster.RestSnapshotsStatusAction; -import org.opensearch.rest.action.admin.cluster.RestTopQueriesAction; import org.opensearch.rest.action.admin.cluster.RestVerifyRepositoryAction; import org.opensearch.rest.action.admin.cluster.dangling.RestDeleteDanglingIndexAction; import org.opensearch.rest.action.admin.cluster.dangling.RestImportDanglingIndexAction; @@ -765,9 +762,6 @@ public void reg actions.register(GetSearchPipelineAction.INSTANCE, GetSearchPipelineTransportAction.class); actions.register(DeleteSearchPipelineAction.INSTANCE, DeleteSearchPipelineTransportAction.class); - // Query Insight Actions - actions.register(TopQueriesAction.INSTANCE, TransportTopQueriesAction.class); - return unmodifiableMap(actions.getRegistry()); } @@ -980,9 +974,6 @@ public void initRestHandlers(Supplier nodesInCluster) { registerHandler.accept(new RestGetDecommissionStateAction()); registerHandler.accept(new RestRemoteStoreStatsAction()); registerHandler.accept(new RestRestoreRemoteStoreAction()); - - // Query insights API - registerHandler.accept(new RestTopQueriesAction()); } @Override diff --git a/server/src/main/java/org/opensearch/action/admin/cluster/insights/top_queries/TopQueriesRequestBuilder.java b/server/src/main/java/org/opensearch/action/admin/cluster/insights/top_queries/TopQueriesRequestBuilder.java deleted file mode 100644 index e88e9aacc9c9d..0000000000000 --- a/server/src/main/java/org/opensearch/action/admin/cluster/insights/top_queries/TopQueriesRequestBuilder.java +++ /dev/null @@ -1,38 +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.action.admin.cluster.insights.top_queries; - -import org.opensearch.action.support.nodes.NodesOperationRequestBuilder; -import org.opensearch.client.OpenSearchClient; -import org.opensearch.common.annotation.PublicApi; - -/** - * Builder class for requesting cluster/node level top queries information. - * - * @opensearch.api - */ -@PublicApi(since = "1.0.0") -public class TopQueriesRequestBuilder extends NodesOperationRequestBuilder { - - public TopQueriesRequestBuilder(OpenSearchClient client, TopQueriesAction action) { - super(client, action, new TopQueriesRequest()); - } - - /** - * set metric type for the request - * - * @param metric Name of metric as a string. - * @return This, for request chaining. - */ - public TopQueriesRequestBuilder setMetricType(String metric) { - request.setMetricType(metric); - return this; - } -} diff --git a/server/src/main/java/org/opensearch/action/search/QueryLatencyInsightService.java b/server/src/main/java/org/opensearch/action/search/QueryLatencyInsightService.java deleted file mode 100644 index e87b0cd8d5d2f..0000000000000 --- a/server/src/main/java/org/opensearch/action/search/QueryLatencyInsightService.java +++ /dev/null @@ -1,153 +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.action.search; - -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; -import org.opensearch.client.Client; -import org.opensearch.cluster.service.ClusterService; -import org.opensearch.common.inject.Inject; -import org.opensearch.common.settings.Setting; -import org.opensearch.common.unit.TimeValue; -import org.opensearch.threadpool.ThreadPool; - -import java.util.Map; -import java.util.concurrent.PriorityBlockingQueue; - -/** - * Service responsible for gathering, analyzing, storing and exporting - * top N queries with high latency data for search queries - * - * @opensearch.internal - */ -public class QueryLatencyInsightService extends QueryInsightService< - SearchQueryLatencyRecord, - PriorityBlockingQueue, - QueryInsightLocalIndexExporter - > { - private static final Logger log = LogManager.getLogger(QueryLatencyInsightService.class); - - /** Default window size in seconds to keep the top N queries with latency data in query insight store */ - private static final int DEFAULT_WINDOW_SIZE = 60; - - /** Default top N size to keep the data in query insight store */ - private static final int DEFAULT_TOP_N_SIZE = 3; - public static final String TOP_N_LATENCY_QUERIES_PREFIX = TOP_N_QUERIES_PREFIX + ".latency"; - public static final Setting TOP_N_LATENCY_QUERIES_ENABLED = Setting.boolSetting( - TOP_N_LATENCY_QUERIES_PREFIX + ".enabled", - false, - Setting.Property.Dynamic, - Setting.Property.NodeScope - ); - public static final Setting TOP_N_LATENCY_QUERIES_SIZE = Setting.intSetting( - TOP_N_LATENCY_QUERIES_PREFIX + ".top_n_size", - DEFAULT_TOP_N_SIZE, - Setting.Property.Dynamic, - Setting.Property.NodeScope - ); - public static final Setting TOP_N_LATENCY_QUERIES_WINDOW_SIZE = Setting.intSetting( - TOP_N_LATENCY_QUERIES_PREFIX + ".window_size", - DEFAULT_WINDOW_SIZE, - Setting.Property.Dynamic, - Setting.Property.NodeScope - ); - - private int topNSize = DEFAULT_TOP_N_SIZE; - - private TimeValue windowSize = TimeValue.timeValueSeconds(DEFAULT_WINDOW_SIZE); - - - @Inject - public QueryLatencyInsightService( - ClusterService clusterService, - Client client, - ThreadPool threadPool - ) { - super( - threadPool, - new PriorityBlockingQueue<>(), - null - ); - - clusterService.getClusterSettings() - .addSettingsUpdateConsumer(TOP_N_LATENCY_QUERIES_ENABLED, this::setEnabled); - clusterService.getClusterSettings() - .addSettingsUpdateConsumer(TOP_N_LATENCY_QUERIES_SIZE, this::setTopNSize); - clusterService.getClusterSettings() - .addSettingsUpdateConsumer(TOP_N_LATENCY_QUERIES_WINDOW_SIZE, this::setWindowSize); - } - - /** - * 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 - */ - public void ingestQueryData( - final Long timestamp, - final SearchType searchType, - final String source, - final int totalShards, - final String[] indices, - final Map propertyMap, - final Map phaseLatencyMap - ) { - if (timestamp <= 0) { - log.error(String.format("Invalid timestamp %s when ingesting query data to compute top n queries with latency", timestamp)); - return; - } - if (totalShards <= 0) { - log.error(String.format("Invalid totalShards %s when ingesting query data to compute top n queries with latency", totalShards)); - return; - } - super.ingestQueryData(new SearchQueryLatencyRecord( - timestamp, - searchType, - source, - totalShards, - indices, - propertyMap, - phaseLatencyMap - )); - // remove top elements for fix sizing priority queue - if (this.store.size() > this.getTopNSize()) { - this.store.poll(); - } - log.debug(String.format("successfully ingested: %s", this.store)); - } - - @Override - public void clearOutdatedData() { - store.removeIf(record -> - record.getTimestamp() < System.currentTimeMillis() - windowSize.getMillis() - ); - } - - public void setTopNSize(int size) { - this.topNSize = size; - } - - public void setWindowSize(int windowSize) { - this.windowSize = TimeValue.timeValueSeconds(windowSize); - } - - public int getTopNSize() { - return this.topNSize; - } - - public TimeValue getWindowSize() { - return this.windowSize; - } - -} diff --git a/server/src/main/java/org/opensearch/action/search/SearchQueryLatencyListener.java b/server/src/main/java/org/opensearch/action/search/SearchQueryLatencyListener.java deleted file mode 100644 index b403b12442234..0000000000000 --- a/server/src/main/java/org/opensearch/action/search/SearchQueryLatencyListener.java +++ /dev/null @@ -1,58 +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.action.search; - -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; -import org.opensearch.common.inject.Inject; -import org.opensearch.core.xcontent.ToXContent; - -import java.util.Collections; -import java.util.HashMap; - -/** - * 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 final QueryLatencyInsightService queryLatencyInsightService; - - @Inject - public SearchQueryLatencyListener(QueryLatencyInsightService queryLatencyInsightService) { - this.queryLatencyInsightService = queryLatencyInsightService; - } - - @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(); - queryLatencyInsightService.ingestQueryData( - request.getOrCreateAbsoluteStartMillis(), - request.searchType(), - request.source().toString(FORMAT_PARAMS), - context.getNumShards(), - request.indices(), - new HashMap<>(), - searchRequestContext.phaseTookMap() - ); - } -} diff --git a/server/src/main/java/org/opensearch/action/search/SearchRequest.java b/server/src/main/java/org/opensearch/action/search/SearchRequest.java index 96cea17ff4972..f738c182c06da 100644 --- a/server/src/main/java/org/opensearch/action/search/SearchRequest.java +++ b/server/src/main/java/org/opensearch/action/search/SearchRequest.java @@ -359,7 +359,7 @@ boolean isFinalReduce() { * request. When created through {@link #subSearchRequest(SearchRequest, String[], String, long, boolean)}, this method returns * the provided current time, otherwise it will return {@link System#currentTimeMillis()}. */ - long getOrCreateAbsoluteStartMillis() { + public long getOrCreateAbsoluteStartMillis() { return absoluteStartMillis == DEFAULT_ABSOLUTE_START_MILLIS ? System.currentTimeMillis() : absoluteStartMillis; } diff --git a/server/src/main/java/org/opensearch/action/search/SearchRequestContext.java b/server/src/main/java/org/opensearch/action/search/SearchRequestContext.java index 674363600b251..8addfea06c2f7 100644 --- a/server/src/main/java/org/opensearch/action/search/SearchRequestContext.java +++ b/server/src/main/java/org/opensearch/action/search/SearchRequestContext.java @@ -24,7 +24,7 @@ * @opensearch.internal */ @InternalApi -class SearchRequestContext { +public class SearchRequestContext { private final SearchRequestOperationsListener searchRequestOperationsListener; private long absoluteStartNanos; private final Map phaseTookMap; @@ -53,7 +53,7 @@ void updatePhaseTookMap(String phaseName, Long tookTime) { this.phaseTookMap.put(phaseName, tookTime); } - Map phaseTookMap() { + public Map phaseTookMap() { return phaseTookMap; } @@ -68,7 +68,7 @@ void setAbsoluteStartNanos(long absoluteStartNanos) { /** * Request start time in nanos */ - long getAbsoluteStartNanos() { + public long getAbsoluteStartNanos() { return absoluteStartNanos; } diff --git a/server/src/main/java/org/opensearch/action/search/SearchRequestOperationsListener.java b/server/src/main/java/org/opensearch/action/search/SearchRequestOperationsListener.java index 0c0dc4d0dc853..b377c35139477 100644 --- a/server/src/main/java/org/opensearch/action/search/SearchRequestOperationsListener.java +++ b/server/src/main/java/org/opensearch/action/search/SearchRequestOperationsListener.java @@ -30,21 +30,22 @@ protected SearchRequestOperationsListener(boolean enabled) { this.enabled = enabled; } - abstract void onPhaseStart(SearchPhaseContext context); + protected abstract void onPhaseStart(SearchPhaseContext context); - abstract void onPhaseEnd(SearchPhaseContext context, SearchRequestContext searchRequestContext); + protected abstract void onPhaseEnd(SearchPhaseContext context, SearchRequestContext searchRequestContext); - abstract void onPhaseFailure(SearchPhaseContext context); + protected abstract void onPhaseFailure(SearchPhaseContext context); - void onRequestStart(SearchRequestContext searchRequestContext) {} + protected void onRequestStart(SearchRequestContext searchRequestContext) {} - void onRequestEnd(SearchPhaseContext context, SearchRequestContext searchRequestContext) {} + protected void onRequestEnd(SearchPhaseContext context, SearchRequestContext searchRequestContext) {} - boolean getEnabled() { + + protected boolean getEnabled() { return enabled; } - void setEnabled(boolean enabled) { + protected void setEnabled(boolean enabled) { this.enabled = enabled; } @@ -65,7 +66,7 @@ static final class CompositeListener extends SearchRequestOperationsListener { } @Override - void onPhaseStart(SearchPhaseContext context) { + protected void onPhaseStart(SearchPhaseContext context) { for (SearchRequestOperationsListener listener : listeners) { try { listener.onPhaseStart(context); @@ -76,7 +77,7 @@ void onPhaseStart(SearchPhaseContext context) { } @Override - void onPhaseEnd(SearchPhaseContext context, SearchRequestContext searchRequestContext) { + protected void onPhaseEnd(SearchPhaseContext context, SearchRequestContext searchRequestContext) { for (SearchRequestOperationsListener listener : listeners) { try { listener.onPhaseEnd(context, searchRequestContext); @@ -87,7 +88,7 @@ void onPhaseEnd(SearchPhaseContext context, SearchRequestContext searchRequestCo } @Override - void onPhaseFailure(SearchPhaseContext context) { + protected void onPhaseFailure(SearchPhaseContext context) { for (SearchRequestOperationsListener listener : listeners) { try { listener.onPhaseFailure(context); @@ -98,7 +99,7 @@ void onPhaseFailure(SearchPhaseContext context) { } @Override - void onRequestStart(SearchRequestContext searchRequestContext) { + protected void onRequestStart(SearchRequestContext searchRequestContext) { for (SearchRequestOperationsListener listener : listeners) { try { listener.onRequestStart(searchRequestContext); diff --git a/server/src/main/java/org/opensearch/action/search/SearchRequestSlowLog.java b/server/src/main/java/org/opensearch/action/search/SearchRequestSlowLog.java index c15ef38789e65..05210618a6d47 100644 --- a/server/src/main/java/org/opensearch/action/search/SearchRequestSlowLog.java +++ b/server/src/main/java/org/opensearch/action/search/SearchRequestSlowLog.java @@ -134,19 +134,19 @@ public SearchRequestSlowLog(ClusterService clusterService) { } @Override - void onPhaseStart(SearchPhaseContext context) {} + protected void onPhaseStart(SearchPhaseContext context) {} @Override - void onPhaseEnd(SearchPhaseContext context, SearchRequestContext searchRequestContext) {} + protected void onPhaseEnd(SearchPhaseContext context, SearchRequestContext searchRequestContext) {} @Override - void onPhaseFailure(SearchPhaseContext context) {} + protected void onPhaseFailure(SearchPhaseContext context) {} @Override - void onRequestStart(SearchRequestContext searchRequestContext) {} + protected void onRequestStart(SearchRequestContext searchRequestContext) {} @Override - void onRequestEnd(SearchPhaseContext context, SearchRequestContext searchRequestContext) { + protected void onRequestEnd(SearchPhaseContext context, SearchRequestContext searchRequestContext) { long tookInNanos = System.nanoTime() - searchRequestContext.getAbsoluteStartNanos(); if (warnThreshold >= 0 && tookInNanos > warnThreshold && level.isLevelEnabledFor(SlowLogLevel.WARN)) { diff --git a/server/src/main/java/org/opensearch/action/search/SearchRequestStats.java b/server/src/main/java/org/opensearch/action/search/SearchRequestStats.java index 58dbbb1a31e97..217f3a7c5e6ba 100644 --- a/server/src/main/java/org/opensearch/action/search/SearchRequestStats.java +++ b/server/src/main/java/org/opensearch/action/search/SearchRequestStats.java @@ -57,12 +57,12 @@ public long getPhaseMetric(SearchPhaseName searchPhaseName) { } @Override - void onPhaseStart(SearchPhaseContext context) { + protected void onPhaseStart(SearchPhaseContext context) { phaseStatsMap.get(context.getCurrentPhase().getSearchPhaseName()).current.inc(); } @Override - void onPhaseEnd(SearchPhaseContext context, SearchRequestContext searchRequestContext) { + protected void onPhaseEnd(SearchPhaseContext context, SearchRequestContext searchRequestContext) { StatsHolder phaseStats = phaseStatsMap.get(context.getCurrentPhase().getSearchPhaseName()); phaseStats.current.dec(); phaseStats.total.inc(); @@ -70,7 +70,7 @@ void onPhaseEnd(SearchPhaseContext context, SearchRequestContext searchRequestCo } @Override - void onPhaseFailure(SearchPhaseContext context) { + protected void onPhaseFailure(SearchPhaseContext context) { phaseStatsMap.get(context.getCurrentPhase().getSearchPhaseName()).current.dec(); } diff --git a/server/src/main/java/org/opensearch/action/search/TransportSearchAction.java b/server/src/main/java/org/opensearch/action/search/TransportSearchAction.java index e1cd7d075c355..8394854e27d36 100644 --- a/server/src/main/java/org/opensearch/action/search/TransportSearchAction.java +++ b/server/src/main/java/org/opensearch/action/search/TransportSearchAction.java @@ -343,10 +343,10 @@ void setEnabled(boolean enabledAtClusterLevel, SearchRequest searchRequest) { } @Override - void onPhaseStart(SearchPhaseContext context) {} + protected void onPhaseStart(SearchPhaseContext context) {} @Override - void onPhaseEnd(SearchPhaseContext context, SearchRequestContext searchRequestContext) { + protected void onPhaseEnd(SearchPhaseContext context, SearchRequestContext searchRequestContext) { phaseStatsMap.put( context.getCurrentPhase().getSearchPhaseName(), TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - context.getCurrentPhase().getStartTimeInNanos()) @@ -354,7 +354,7 @@ void onPhaseEnd(SearchPhaseContext context, SearchRequestContext searchRequestCo } @Override - void onPhaseFailure(SearchPhaseContext context) {} + protected void onPhaseFailure(SearchPhaseContext context) {} public Long getPhaseTookTime(SearchPhaseName searchPhaseName) { return phaseStatsMap.get(searchPhaseName); diff --git a/server/src/main/java/org/opensearch/client/ClusterAdminClient.java b/server/src/main/java/org/opensearch/client/ClusterAdminClient.java index e3b579b84f53d..05f09c1a6e661 100644 --- a/server/src/main/java/org/opensearch/client/ClusterAdminClient.java +++ b/server/src/main/java/org/opensearch/client/ClusterAdminClient.java @@ -47,9 +47,6 @@ import org.opensearch.action.admin.cluster.health.ClusterHealthRequest; import org.opensearch.action.admin.cluster.health.ClusterHealthRequestBuilder; import org.opensearch.action.admin.cluster.health.ClusterHealthResponse; -import org.opensearch.action.admin.cluster.insights.top_queries.TopQueriesRequest; -import org.opensearch.action.admin.cluster.insights.top_queries.TopQueriesRequestBuilder; -import org.opensearch.action.admin.cluster.insights.top_queries.TopQueriesResponse; import org.opensearch.action.admin.cluster.node.hotthreads.NodesHotThreadsRequest; import org.opensearch.action.admin.cluster.node.hotthreads.NodesHotThreadsRequestBuilder; import org.opensearch.action.admin.cluster.node.hotthreads.NodesHotThreadsResponse; @@ -945,27 +942,4 @@ public interface ClusterAdminClient extends OpenSearchClient { * Deletes a stored search pipeline */ ActionFuture deleteSearchPipeline(DeleteSearchPipelineRequest request); - - /** - * Top Queries of the cluster. - * - * @param request The top queries request - * @return The result future - * @see org.opensearch.client.Requests#topQueriesRequest(String...) - */ - ActionFuture topQueries(TopQueriesRequest request); - - /** - * Top Queries of the cluster. - * - * @param request The top queries request - * @param listener A listener to be notified with a result - * @see org.opensearch.client.Requests#topQueriesRequest(String...) - */ - void topQueries(TopQueriesRequest request, ActionListener listener); - - /** - * Top Queries of the cluster. - */ - TopQueriesRequestBuilder prepareTopQueries(String... nodesIds); } diff --git a/server/src/main/java/org/opensearch/client/Requests.java b/server/src/main/java/org/opensearch/client/Requests.java index e0a8f3c583733..3607590826007 100644 --- a/server/src/main/java/org/opensearch/client/Requests.java +++ b/server/src/main/java/org/opensearch/client/Requests.java @@ -36,7 +36,6 @@ import org.opensearch.action.admin.cluster.decommission.awareness.get.GetDecommissionStateRequest; import org.opensearch.action.admin.cluster.decommission.awareness.put.DecommissionRequest; import org.opensearch.action.admin.cluster.health.ClusterHealthRequest; -import org.opensearch.action.admin.cluster.insights.top_queries.TopQueriesRequest; import org.opensearch.action.admin.cluster.node.info.NodesInfoRequest; import org.opensearch.action.admin.cluster.node.stats.NodesStatsRequest; import org.opensearch.action.admin.cluster.node.tasks.cancel.CancelTasksRequest; @@ -609,27 +608,4 @@ public static GetDecommissionStateRequest getDecommissionStateRequest() { public static DeleteDecommissionStateRequest deleteDecommissionStateRequest() { return new DeleteDecommissionStateRequest(); } - - - /** - * Creates a cluster level top queries request - * - * @return The top queries request - * @see org.opensearch.client.ClusterAdminClient#topQueries(TopQueriesRequest) - */ - public static TopQueriesRequest topQueriesRequest() { - return new TopQueriesRequest(); - } - - - /** - * Creates a top queries request against one or more nodes. Pass {@code null} or an empty array for all nodes. - * - * @param nodesIds The nodes ids to get top queries for - * @return The top queries request - * @see org.opensearch.client.ClusterAdminClient#topQueries(TopQueriesRequest) - */ - public static TopQueriesRequest topQueriesRequest(String... nodesIds) { - return new TopQueriesRequest(nodesIds); - } } diff --git a/server/src/main/java/org/opensearch/client/support/AbstractClient.java b/server/src/main/java/org/opensearch/client/support/AbstractClient.java index b6f0755f9c4d0..786bfa38bb19c 100644 --- a/server/src/main/java/org/opensearch/client/support/AbstractClient.java +++ b/server/src/main/java/org/opensearch/client/support/AbstractClient.java @@ -56,10 +56,6 @@ import org.opensearch.action.admin.cluster.health.ClusterHealthRequest; import org.opensearch.action.admin.cluster.health.ClusterHealthRequestBuilder; import org.opensearch.action.admin.cluster.health.ClusterHealthResponse; -import org.opensearch.action.admin.cluster.insights.top_queries.TopQueriesAction; -import org.opensearch.action.admin.cluster.insights.top_queries.TopQueriesRequest; -import org.opensearch.action.admin.cluster.insights.top_queries.TopQueriesRequestBuilder; -import org.opensearch.action.admin.cluster.insights.top_queries.TopQueriesResponse; import org.opensearch.action.admin.cluster.node.hotthreads.NodesHotThreadsAction; import org.opensearch.action.admin.cluster.node.hotthreads.NodesHotThreadsRequest; import org.opensearch.action.admin.cluster.node.hotthreads.NodesHotThreadsRequestBuilder; @@ -1514,21 +1510,6 @@ public void deleteSearchPipeline(DeleteSearchPipelineRequest request, ActionList public ActionFuture deleteSearchPipeline(DeleteSearchPipelineRequest request) { return execute(DeleteSearchPipelineAction.INSTANCE, request); } - - @Override - public ActionFuture topQueries(final TopQueriesRequest request) { - return execute(TopQueriesAction.INSTANCE, request); - } - - @Override - public void topQueries(final TopQueriesRequest request, final ActionListener listener) { - execute(TopQueriesAction.INSTANCE, request, listener); - } - - @Override - public TopQueriesRequestBuilder prepareTopQueries(String... nodesIds) { - return new TopQueriesRequestBuilder(this, TopQueriesAction.INSTANCE).setNodesIds(nodesIds); - } } static class IndicesAdmin implements IndicesAdminClient { diff --git a/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java b/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java index 2c8dd35238586..814e7b6619106 100644 --- a/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java +++ b/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java @@ -35,7 +35,6 @@ import org.opensearch.action.admin.cluster.configuration.TransportAddVotingConfigExclusionsAction; import org.opensearch.action.admin.indices.close.TransportCloseIndexAction; import org.opensearch.action.search.CreatePitController; -import org.opensearch.action.search.QueryLatencyInsightService; import org.opensearch.action.search.SearchRequestListenerManager; import org.opensearch.action.search.SearchRequestSlowLog; import org.opensearch.action.search.SearchRequestStats; @@ -704,12 +703,7 @@ public void apply(Settings value, Settings current, Settings previous) { AdmissionControlSettings.ADMISSION_CONTROL_TRANSPORT_LAYER_MODE, CpuBasedAdmissionControllerSettings.CPU_BASED_ADMISSION_CONTROLLER_TRANSPORT_LAYER_MODE, CpuBasedAdmissionControllerSettings.INDEXING_CPU_USAGE_LIMIT, - CpuBasedAdmissionControllerSettings.SEARCH_CPU_USAGE_LIMIT, - - // Cluster insight settings - QueryLatencyInsightService.TOP_N_LATENCY_QUERIES_ENABLED, - QueryLatencyInsightService.TOP_N_LATENCY_QUERIES_WINDOW_SIZE, - QueryLatencyInsightService.TOP_N_LATENCY_QUERIES_SIZE + CpuBasedAdmissionControllerSettings.SEARCH_CPU_USAGE_LIMIT ) ) ); diff --git a/server/src/test/java/org/opensearch/action/search/SearchRequestListenerManagerTests.java b/server/src/test/java/org/opensearch/action/search/SearchRequestListenerManagerTests.java index f8098610268f5..9a208b72f4b78 100644 --- a/server/src/test/java/org/opensearch/action/search/SearchRequestListenerManagerTests.java +++ b/server/src/test/java/org/opensearch/action/search/SearchRequestListenerManagerTests.java @@ -130,13 +130,13 @@ public void testStandardListenerAndTimeProviderDisabled() throws NoSuchFieldExce public SearchRequestOperationsListener createTestSearchRequestOperationsListener() { return new SearchRequestOperationsListener() { @Override - void onPhaseStart(SearchPhaseContext context) {} + protected void onPhaseStart(SearchPhaseContext context) {} @Override - void onPhaseEnd(SearchPhaseContext context, SearchRequestContext searchRequestContext) {} + protected void onPhaseEnd(SearchPhaseContext context, SearchRequestContext searchRequestContext) {} @Override - void onPhaseFailure(SearchPhaseContext context) {} + protected void onPhaseFailure(SearchPhaseContext context) {} }; } } diff --git a/server/src/test/java/org/opensearch/snapshots/SnapshotResiliencyTests.java b/server/src/test/java/org/opensearch/snapshots/SnapshotResiliencyTests.java index 91265325358bf..e631d3e67a484 100644 --- a/server/src/test/java/org/opensearch/snapshots/SnapshotResiliencyTests.java +++ b/server/src/test/java/org/opensearch/snapshots/SnapshotResiliencyTests.java @@ -86,11 +86,9 @@ import org.opensearch.action.bulk.TransportShardBulkAction; import org.opensearch.action.index.IndexRequest; import org.opensearch.action.resync.TransportResyncReplicationAction; -import org.opensearch.action.search.QueryLatencyInsightService; import org.opensearch.action.search.SearchAction; import org.opensearch.action.search.SearchExecutionStatsCollector; import org.opensearch.action.search.SearchPhaseController; -import org.opensearch.action.search.SearchQueryLatencyListener; import org.opensearch.action.search.SearchRequest; import org.opensearch.action.search.SearchRequestListenerManager; import org.opensearch.action.search.SearchResponse;