From 1895133d56d966b31cedb1521f1b5e9788a2f8c9 Mon Sep 17 00:00:00 2001 From: Chenyang Ji Date: Mon, 5 Feb 2024 18:23:31 -0800 Subject: [PATCH] rebase from master to pick up query insights plugin changes Signed-off-by: Chenyang Ji --- .../QueryInsightsPluginTransportIT.java | 4 +-- .../plugin/insights/QueryInsightsPlugin.java | 10 ++++-- .../insights/QueryInsightsPluginTests.java | 22 ++++++++++-- .../insights/QueryInsightsTestUtils.java | 34 +++++++++++++++++++ .../RestTopQueriesActionTests.java | 10 +++++- 5 files changed, 71 insertions(+), 9 deletions(-) diff --git a/plugins/query-insights/src/internalClusterTest/java/org/opensearch/plugin/insights/QueryInsightsPluginTransportIT.java b/plugins/query-insights/src/internalClusterTest/java/org/opensearch/plugin/insights/QueryInsightsPluginTransportIT.java index 711faa9323520..04e715444f50a 100644 --- a/plugins/query-insights/src/internalClusterTest/java/org/opensearch/plugin/insights/QueryInsightsPluginTransportIT.java +++ b/plugins/query-insights/src/internalClusterTest/java/org/opensearch/plugin/insights/QueryInsightsPluginTransportIT.java @@ -81,7 +81,7 @@ public void testGetTopQueriesWhenFeatureDisabled() { TopQueriesResponse response = OpenSearchIntegTestCase.client().execute(TopQueriesAction.INSTANCE, request).actionGet(); Assert.assertNotEquals(0, response.failures().size()); Assert.assertEquals( - "Cannot get top n queries when [search.top_n_queries.latency.enabled] is not enabled.", + "Cannot get top n queries for [latency] when it is not enabled.", response.failures().get(0).getCause().getCause().getMessage() ); } @@ -121,7 +121,7 @@ public void testUpdateRecordWhenFeatureDisabledThenEnabled() throws ExecutionExc TopQueriesResponse response = OpenSearchIntegTestCase.client().execute(TopQueriesAction.INSTANCE, request).actionGet(); Assert.assertNotEquals(0, response.failures().size()); Assert.assertEquals( - "Cannot get top n queries when [search.top_n_queries.latency.enabled] is not enabled.", + "Cannot get top n queries for [latency] when it is not enabled.", response.failures().get(0).getCause().getCause().getMessage() ); diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/QueryInsightsPlugin.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/QueryInsightsPlugin.java index 0acd7e2dcac0d..4d7e0d486068a 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/QueryInsightsPlugin.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/QueryInsightsPlugin.java @@ -25,7 +25,11 @@ import org.opensearch.core.xcontent.NamedXContentRegistry; import org.opensearch.env.Environment; import org.opensearch.env.NodeEnvironment; +import org.opensearch.plugin.insights.core.listener.QueryInsightsListener; import org.opensearch.plugin.insights.core.service.QueryInsightsService; +import org.opensearch.plugin.insights.rules.action.top_queries.TopQueriesAction; +import org.opensearch.plugin.insights.rules.resthandler.top_queries.RestTopQueriesAction; +import org.opensearch.plugin.insights.rules.transport.top_queries.TransportTopQueriesAction; import org.opensearch.plugin.insights.settings.QueryInsightsSettings; import org.opensearch.plugins.ActionPlugin; import org.opensearch.plugins.Plugin; @@ -67,7 +71,7 @@ public Collection createComponents( ) { // create top n queries service final QueryInsightsService queryInsightsService = new QueryInsightsService(threadPool); - return List.of(queryInsightsService); + return List.of(queryInsightsService, new QueryInsightsListener(clusterService, queryInsightsService)); } @Override @@ -92,12 +96,12 @@ public List getRestHandlers( final IndexNameExpressionResolver indexNameExpressionResolver, final Supplier nodesInCluster ) { - return List.of(); + return List.of(new RestTopQueriesAction()); } @Override public List> getActions() { - return List.of(); + return List.of(new ActionPlugin.ActionHandler<>(TopQueriesAction.INSTANCE, TransportTopQueriesAction.class)); } @Override diff --git a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/QueryInsightsPluginTests.java b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/QueryInsightsPluginTests.java index f6d95450855de..273b69e483e8c 100644 --- a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/QueryInsightsPluginTests.java +++ b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/QueryInsightsPluginTests.java @@ -14,11 +14,16 @@ import org.opensearch.common.settings.ClusterSettings; import org.opensearch.common.settings.Settings; import org.opensearch.core.action.ActionResponse; +import org.opensearch.plugin.insights.core.listener.QueryInsightsListener; import org.opensearch.plugin.insights.core.service.QueryInsightsService; +import org.opensearch.plugin.insights.rules.action.top_queries.TopQueriesAction; +import org.opensearch.plugin.insights.rules.resthandler.top_queries.RestTopQueriesAction; import org.opensearch.plugin.insights.settings.QueryInsightsSettings; import org.opensearch.plugins.ActionPlugin; import org.opensearch.rest.RestHandler; import org.opensearch.test.OpenSearchTestCase; +import org.opensearch.threadpool.ExecutorBuilder; +import org.opensearch.threadpool.ScalingExecutorBuilder; import org.opensearch.threadpool.ThreadPool; import org.junit.Before; @@ -74,18 +79,29 @@ public void testCreateComponent() { null, null ); - assertEquals(1, components.size()); + assertEquals(2, components.size()); assertTrue(components.get(0) instanceof QueryInsightsService); + assertTrue(components.get(1) instanceof QueryInsightsListener); + } + + public void testGetExecutorBuilders() { + Settings.Builder settingsBuilder = Settings.builder(); + Settings settings = settingsBuilder.build(); + List> executorBuilders = queryInsightsPlugin.getExecutorBuilders(settings); + assertEquals(1, executorBuilders.size()); + assertTrue(executorBuilders.get(0) instanceof ScalingExecutorBuilder); } public void testGetRestHandlers() { List components = queryInsightsPlugin.getRestHandlers(Settings.EMPTY, null, null, null, null, null, null); - assertEquals(0, components.size()); + assertEquals(1, components.size()); + assertTrue(components.get(0) instanceof RestTopQueriesAction); } public void testGetActions() { List> components = queryInsightsPlugin.getActions(); - assertEquals(0, components.size()); + assertEquals(1, components.size()); + assertTrue(components.get(0).getAction() instanceof TopQueriesAction); } } diff --git a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/QueryInsightsTestUtils.java b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/QueryInsightsTestUtils.java index 04f49d5fbc7dd..870ef5b9c8be9 100644 --- a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/QueryInsightsTestUtils.java +++ b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/QueryInsightsTestUtils.java @@ -9,12 +9,15 @@ package org.opensearch.plugin.insights; import org.opensearch.action.search.SearchType; +import org.opensearch.cluster.node.DiscoveryNode; import org.opensearch.common.util.Maps; import org.opensearch.core.xcontent.ToXContent; import org.opensearch.core.xcontent.XContentBuilder; +import org.opensearch.plugin.insights.rules.action.top_queries.TopQueries; import org.opensearch.plugin.insights.rules.model.Attribute; import org.opensearch.plugin.insights.rules.model.MetricType; import org.opensearch.plugin.insights.rules.model.SearchQueryRecord; +import org.opensearch.test.VersionUtils; import java.io.IOException; import java.util.ArrayList; @@ -26,7 +29,11 @@ import java.util.Set; import java.util.TreeSet; +import static java.util.Collections.emptyMap; +import static java.util.Collections.emptySet; import static org.opensearch.common.xcontent.XContentFactory.jsonBuilder; +import static org.opensearch.test.OpenSearchTestCase.buildNewFakeTransportAddress; +import static org.opensearch.test.OpenSearchTestCase.random; import static org.opensearch.test.OpenSearchTestCase.randomAlphaOfLengthBetween; import static org.opensearch.test.OpenSearchTestCase.randomArray; import static org.opensearch.test.OpenSearchTestCase.randomDouble; @@ -79,6 +86,33 @@ public static List generateQueryInsightRecords(int lower, int return records; } + public static TopQueries createRandomTopQueries() { + DiscoveryNode node = new DiscoveryNode( + "node_for_top_queries_test", + buildNewFakeTransportAddress(), + emptyMap(), + emptySet(), + VersionUtils.randomVersion(random()) + ); + List records = generateQueryInsightRecords(10); + + return new TopQueries(node, records); + } + + public static TopQueries createFixedTopQueries() { + DiscoveryNode node = new DiscoveryNode( + "node_for_top_queries_test", + buildNewFakeTransportAddress(), + emptyMap(), + emptySet(), + VersionUtils.randomVersion(random()) + ); + List records = new ArrayList<>(); + records.add(createFixedSearchQueryRecord()); + + return new TopQueries(node, records); + } + public static SearchQueryRecord createFixedSearchQueryRecord() { long timestamp = 1706574180000L; Map measurements = Map.of(MetricType.LATENCY, 1L); diff --git a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/resthandler/top_queries/RestTopQueriesActionTests.java b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/resthandler/top_queries/RestTopQueriesActionTests.java index 627d794731eed..ac19fa2a7348f 100644 --- a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/resthandler/top_queries/RestTopQueriesActionTests.java +++ b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/resthandler/top_queries/RestTopQueriesActionTests.java @@ -9,11 +9,13 @@ package org.opensearch.plugin.insights.rules.resthandler.top_queries; import org.opensearch.plugin.insights.rules.action.top_queries.TopQueriesRequest; +import org.opensearch.rest.RestHandler; import org.opensearch.rest.RestRequest; import org.opensearch.test.OpenSearchTestCase; import org.opensearch.test.rest.FakeRestRequest; import java.util.HashMap; +import java.util.List; import java.util.Locale; import java.util.Map; @@ -24,7 +26,6 @@ public class RestTopQueriesActionTests extends OpenSearchTestCase { public void testEmptyNodeIdsValidType() { Map params = new HashMap<>(); params.put("type", randomFrom(ALLOWED_METRICS)); - RestRequest restRequest = buildRestRequest(params); TopQueriesRequest actual = RestTopQueriesAction.prepareRequest(restRequest); assertEquals(0, actual.nodesIds().length); @@ -53,6 +54,13 @@ public void testInValidType() { ); } + public void testGetRoutes() { + RestTopQueriesAction action = new RestTopQueriesAction(); + List routes = action.routes(); + assertEquals(2, routes.size()); + assertEquals("query_insights_top_queries_action", action.getName()); + } + private FakeRestRequest buildRestRequest(Map params) { return new FakeRestRequest.Builder(xContentRegistry()).withMethod(RestRequest.Method.GET) .withPath("/_insights/top_queries")