From 08173af4ac099459c4e3c6bea28783777492aa47 Mon Sep 17 00:00:00 2001 From: Siddhant Deshmukh Date: Mon, 24 Jun 2024 13:27:53 -0700 Subject: [PATCH 01/22] Move query categorization changes to query insights plugin Signed-off-by: Siddhant Deshmukh --- .../plugin/insights/QueryInsightsPlugin.java | 12 ++++-- .../core/categorizer}/QueryShapeVisitor.java | 6 ++- .../SearchQueryAggregationCategorizer.java | 2 +- .../categorizer}/SearchQueryCategorizer.java | 16 ++++++-- .../SearchQueryCategorizingVisitor.java | 2 +- .../categorizer}/SearchQueryCounters.java | 4 +- .../core/listener/QueryInsightsListener.java | 2 +- .../core/service/QueryInsightsService.java | 38 ++++++++++++++++++- .../settings/QueryCategorizationSettings.java | 21 ++++++++++ .../insights/QueryInsightsTestUtils.java | 7 +++- .../categorizor}/QueryShapeVisitorTests.java | 13 +++++-- .../SearchQueryCategorizerTests.java | 8 ++-- .../listener/QueryInsightsListenerTests.java | 2 +- .../service/QueryInsightsServiceTests.java | 4 +- .../action/search/TransportSearchAction.java | 30 --------------- 15 files changed, 112 insertions(+), 55 deletions(-) rename {server/src/main/java/org/opensearch/index/query => plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer}/QueryShapeVisitor.java (94%) rename {server/src/main/java/org/opensearch/action/search => plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer}/SearchQueryAggregationCategorizer.java (97%) rename {server/src/main/java/org/opensearch/action/search => plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer}/SearchQueryCategorizer.java (84%) rename {server/src/main/java/org/opensearch/action/search => plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer}/SearchQueryCategorizingVisitor.java (95%) rename {server/src/main/java/org/opensearch/action/search => plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer}/SearchQueryCounters.java (96%) create mode 100644 plugins/query-insights/src/main/java/org/opensearch/plugin/insights/settings/QueryCategorizationSettings.java rename {server/src/test/java/org/opensearch/index/query => plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/categorizor}/QueryShapeVisitorTests.java (70%) rename {server/src/test/java/org/opensearch/action/search => plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/categorizor}/SearchQueryCategorizerTests.java (98%) 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 bba676436c39a..526adace7be56 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 @@ -9,6 +9,7 @@ package org.opensearch.plugin.insights; import org.opensearch.action.ActionRequest; +import org.opensearch.action.search.TransportSearchAction; import org.opensearch.client.Client; import org.opensearch.cluster.metadata.IndexNameExpressionResolver; import org.opensearch.cluster.node.DiscoveryNodes; @@ -33,10 +34,13 @@ import org.opensearch.plugin.insights.settings.QueryInsightsSettings; import org.opensearch.plugins.ActionPlugin; import org.opensearch.plugins.Plugin; +import org.opensearch.plugins.TelemetryAwarePlugin; import org.opensearch.repositories.RepositoriesService; import org.opensearch.rest.RestController; import org.opensearch.rest.RestHandler; import org.opensearch.script.ScriptService; +import org.opensearch.telemetry.metrics.MetricsRegistry; +import org.opensearch.telemetry.tracing.Tracer; import org.opensearch.threadpool.ExecutorBuilder; import org.opensearch.threadpool.ScalingExecutorBuilder; import org.opensearch.threadpool.ThreadPool; @@ -49,7 +53,7 @@ /** * Plugin class for Query Insights. */ -public class QueryInsightsPlugin extends Plugin implements ActionPlugin { +public class QueryInsightsPlugin extends Plugin implements ActionPlugin, TelemetryAwarePlugin { /** * Default constructor */ @@ -67,10 +71,12 @@ public Collection createComponents( final NodeEnvironment nodeEnvironment, final NamedWriteableRegistry namedWriteableRegistry, final IndexNameExpressionResolver indexNameExpressionResolver, - final Supplier repositoriesServiceSupplier + final Supplier repositoriesServiceSupplier, + final Tracer tracer, + final MetricsRegistry metricsRegistry ) { // create top n queries service - final QueryInsightsService queryInsightsService = new QueryInsightsService(clusterService.getClusterSettings(), threadPool, client); + final QueryInsightsService queryInsightsService = new QueryInsightsService(clusterService.getClusterSettings(), threadPool, client, metricsRegistry); return List.of(queryInsightsService, new QueryInsightsListener(clusterService, queryInsightsService)); } diff --git a/server/src/main/java/org/opensearch/index/query/QueryShapeVisitor.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer/QueryShapeVisitor.java similarity index 94% rename from server/src/main/java/org/opensearch/index/query/QueryShapeVisitor.java rename to plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer/QueryShapeVisitor.java index 3ba13bc7a2da4..92724c9cc57b2 100644 --- a/server/src/main/java/org/opensearch/index/query/QueryShapeVisitor.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer/QueryShapeVisitor.java @@ -6,10 +6,12 @@ * compatible open source license. */ -package org.opensearch.index.query; +package org.opensearch.plugin.insights.core.categorizer; import org.apache.lucene.search.BooleanClause; import org.opensearch.common.SetOnce; +import org.opensearch.index.query.QueryBuilder; +import org.opensearch.index.query.QueryBuilderVisitor; import java.util.ArrayList; import java.util.EnumMap; @@ -55,7 +57,7 @@ public QueryBuilderVisitor getChildVisitor(BooleanClause.Occur occur) { return childVisitorWrapper; } - String toJson() { + public String toJson() { StringBuilder outputBuilder = new StringBuilder("{\"type\":\"").append(queryType.get()).append("\""); for (Map.Entry> entry : childVisitors.entrySet()) { outputBuilder.append(",\"").append(entry.getKey().name().toLowerCase(Locale.ROOT)).append("\"["); diff --git a/server/src/main/java/org/opensearch/action/search/SearchQueryAggregationCategorizer.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer/SearchQueryAggregationCategorizer.java similarity index 97% rename from server/src/main/java/org/opensearch/action/search/SearchQueryAggregationCategorizer.java rename to plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer/SearchQueryAggregationCategorizer.java index 607ccf182851b..94a9b9182e9d7 100644 --- a/server/src/main/java/org/opensearch/action/search/SearchQueryAggregationCategorizer.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer/SearchQueryAggregationCategorizer.java @@ -6,7 +6,7 @@ * compatible open source license. */ -package org.opensearch.action.search; +package org.opensearch.plugin.insights.core.categorizer; import org.opensearch.search.aggregations.AggregationBuilder; import org.opensearch.search.aggregations.PipelineAggregationBuilder; diff --git a/server/src/main/java/org/opensearch/action/search/SearchQueryCategorizer.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer/SearchQueryCategorizer.java similarity index 84% rename from server/src/main/java/org/opensearch/action/search/SearchQueryCategorizer.java rename to plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer/SearchQueryCategorizer.java index ffaae5b08772f..cd8f3b3e978bf 100644 --- a/server/src/main/java/org/opensearch/action/search/SearchQueryCategorizer.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer/SearchQueryCategorizer.java @@ -6,13 +6,14 @@ * compatible open source license. */ -package org.opensearch.action.search; +package org.opensearch.plugin.insights.core.categorizer; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.opensearch.index.query.QueryBuilder; import org.opensearch.index.query.QueryBuilderVisitor; -import org.opensearch.index.query.QueryShapeVisitor; +import org.opensearch.plugin.insights.rules.model.Attribute; +import org.opensearch.plugin.insights.rules.model.SearchQueryRecord; import org.opensearch.search.aggregations.AggregatorFactories; import org.opensearch.search.builder.SearchSourceBuilder; import org.opensearch.search.sort.SortBuilder; @@ -26,11 +27,11 @@ * Class to categorize the search queries based on the type and increment the relevant counters. * Class also logs the query shape. */ -final class SearchQueryCategorizer { +public final class SearchQueryCategorizer { private static final Logger log = LogManager.getLogger(SearchQueryCategorizer.class); - final SearchQueryCounters searchQueryCounters; + public final SearchQueryCounters searchQueryCounters; final SearchQueryAggregationCategorizer searchQueryAggregationCategorizer; @@ -39,6 +40,13 @@ public SearchQueryCategorizer(MetricsRegistry metricsRegistry) { searchQueryAggregationCategorizer = new SearchQueryAggregationCategorizer(searchQueryCounters); } + public void consumeRecords(List records) { + for (SearchQueryRecord record : records) { + SearchSourceBuilder source = (SearchSourceBuilder) record.getAttributes().get(Attribute.SOURCE); + categorize(source); + } + } + public void categorize(SearchSourceBuilder source) { QueryBuilder topLevelQueryBuilder = source.query(); logQueryShape(topLevelQueryBuilder); diff --git a/server/src/main/java/org/opensearch/action/search/SearchQueryCategorizingVisitor.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer/SearchQueryCategorizingVisitor.java similarity index 95% rename from server/src/main/java/org/opensearch/action/search/SearchQueryCategorizingVisitor.java rename to plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer/SearchQueryCategorizingVisitor.java index 31f83dbef9dc9..ed6411145376d 100644 --- a/server/src/main/java/org/opensearch/action/search/SearchQueryCategorizingVisitor.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer/SearchQueryCategorizingVisitor.java @@ -6,7 +6,7 @@ * compatible open source license. */ -package org.opensearch.action.search; +package org.opensearch.plugin.insights.core.categorizer; import org.apache.lucene.search.BooleanClause; import org.opensearch.index.query.QueryBuilder; diff --git a/server/src/main/java/org/opensearch/action/search/SearchQueryCounters.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer/SearchQueryCounters.java similarity index 96% rename from server/src/main/java/org/opensearch/action/search/SearchQueryCounters.java rename to plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer/SearchQueryCounters.java index a8a7e352b89dc..42569b2978379 100644 --- a/server/src/main/java/org/opensearch/action/search/SearchQueryCounters.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer/SearchQueryCounters.java @@ -6,7 +6,7 @@ * compatible open source license. */ -package org.opensearch.action.search; +package org.opensearch.plugin.insights.core.categorizer; import org.opensearch.index.query.QueryBuilder; import org.opensearch.telemetry.metrics.Counter; @@ -20,7 +20,7 @@ /** * Class contains all the Counters related to search query types. */ -final class SearchQueryCounters { +public final class SearchQueryCounters { private static final String LEVEL_TAG = "level"; private static final String UNIT = "1"; private final MetricsRegistry metricsRegistry; diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/listener/QueryInsightsListener.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/listener/QueryInsightsListener.java index a1f810ad5987c..a1b43a171c6a2 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/listener/QueryInsightsListener.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/listener/QueryInsightsListener.java @@ -178,7 +178,7 @@ private void constructSearchQueryRecord(final SearchPhaseContext context, final } Map attributes = new HashMap<>(); attributes.put(Attribute.SEARCH_TYPE, request.searchType().toString().toLowerCase(Locale.ROOT)); - attributes.put(Attribute.SOURCE, request.source().toString(FORMAT_PARAMS)); + attributes.put(Attribute.SOURCE, request.source()); attributes.put(Attribute.TOTAL_SHARDS, context.getNumShards()); attributes.put(Attribute.INDICES, request.indices()); attributes.put(Attribute.PHASE_LATENCY_MAP, searchRequestContext.phaseTookMap()); diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/QueryInsightsService.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/QueryInsightsService.java index c63430a1a726c..e81670a1d842d 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/QueryInsightsService.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/QueryInsightsService.java @@ -8,16 +8,20 @@ package org.opensearch.plugin.insights.core.service; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; import org.opensearch.client.Client; import org.opensearch.common.inject.Inject; import org.opensearch.common.lifecycle.AbstractLifecycleComponent; import org.opensearch.common.settings.ClusterSettings; import org.opensearch.common.settings.Settings; import org.opensearch.common.unit.TimeValue; +import org.opensearch.plugin.insights.core.categorizer.SearchQueryCategorizer; import org.opensearch.plugin.insights.core.exporter.QueryInsightsExporterFactory; import org.opensearch.plugin.insights.rules.model.MetricType; import org.opensearch.plugin.insights.rules.model.SearchQueryRecord; import org.opensearch.plugin.insights.settings.QueryInsightsSettings; +import org.opensearch.telemetry.metrics.MetricsRegistry; import org.opensearch.threadpool.Scheduler; import org.opensearch.threadpool.ThreadPool; @@ -29,6 +33,7 @@ import java.util.Map; import java.util.concurrent.LinkedBlockingQueue; +import static org.opensearch.plugin.insights.settings.QueryCategorizationSettings.SEARCH_QUERY_METRICS_ENABLED_SETTING; import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.getExporterSettings; /** @@ -38,6 +43,9 @@ * @opensearch.internal */ public class QueryInsightsService extends AbstractLifecycleComponent { + + private static final Logger logger = LogManager.getLogger(QueryInsightsService.class); + /** * The internal OpenSearch thread pool that execute async processing and exporting tasks */ @@ -69,6 +77,12 @@ public class QueryInsightsService extends AbstractLifecycleComponent { */ final QueryInsightsExporterFactory queryInsightsExporterFactory; + private volatile boolean searchQueryMetricsEnabled; + + private SearchQueryCategorizer searchQueryCategorizer; + + private MetricsRegistry metricsRegistry; + /** * Constructor of the QueryInsightsService * @@ -77,7 +91,7 @@ public class QueryInsightsService extends AbstractLifecycleComponent { * @param client OS client */ @Inject - public QueryInsightsService(final ClusterSettings clusterSettings, final ThreadPool threadPool, final Client client) { + public QueryInsightsService(final ClusterSettings clusterSettings, final ThreadPool threadPool, final Client client, final MetricsRegistry metricsRegistry) { enableCollect = new HashMap<>(); queryRecordsQueue = new LinkedBlockingQueue<>(QueryInsightsSettings.QUERY_RECORD_QUEUE_CAPACITY); this.threadPool = threadPool; @@ -95,6 +109,19 @@ public QueryInsightsService(final ClusterSettings clusterSettings, final ThreadP (settings -> validateExporterConfig(type, settings)) ); } + + this.searchQueryMetricsEnabled = clusterSettings.get(SEARCH_QUERY_METRICS_ENABLED_SETTING); + clusterSettings + .addSettingsUpdateConsumer(SEARCH_QUERY_METRICS_ENABLED_SETTING, this::setSearchQueryMetricsEnabled); + + this.metricsRegistry = metricsRegistry; + } + + private void setSearchQueryMetricsEnabled(boolean searchQueryMetricsEnabled) { + this.searchQueryMetricsEnabled = searchQueryMetricsEnabled; + if ((this.searchQueryMetricsEnabled == true) && this.searchQueryCategorizer == null) { + this.searchQueryCategorizer = new SearchQueryCategorizer(metricsRegistry); + } } /** @@ -135,6 +162,15 @@ public void drainRecords() { topQueriesServices.get(metricType).consumeRecords(records); } } + + if (searchQueryMetricsEnabled) { + try { + searchQueryCategorizer.consumeRecords(records); + } catch (Exception e) { + logger.error("Error while trying to categorize the queries.", e); + } + } + } /** diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/settings/QueryCategorizationSettings.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/settings/QueryCategorizationSettings.java new file mode 100644 index 0000000000000..1220d58294bc0 --- /dev/null +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/settings/QueryCategorizationSettings.java @@ -0,0 +1,21 @@ +/* + * 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; + +public class QueryCategorizationSettings { + public static final Setting SEARCH_QUERY_METRICS_ENABLED_SETTING = Setting.boolSetting( + "search.query.metrics.enabled", + false, + Setting.Property.NodeScope, + Setting.Property.Dynamic + ); + +} 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 7fa4e9841c20e..dd5df4821ce29 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 @@ -19,6 +19,7 @@ import org.opensearch.plugin.insights.rules.model.MetricType; import org.opensearch.plugin.insights.rules.model.SearchQueryRecord; import org.opensearch.plugin.insights.settings.QueryInsightsSettings; +import org.opensearch.search.builder.SearchSourceBuilder; import org.opensearch.test.VersionUtils; import java.io.IOException; @@ -74,9 +75,13 @@ public static List generateQueryInsightRecords(int lower, int for (int j = 0; j < countOfPhases; ++j) { phaseLatencyMap.put(randomAlphaOfLengthBetween(5, 10), randomLong()); } + + SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder(); + searchSourceBuilder.size(20); // Set the size parameter as needed + Map attributes = new HashMap<>(); attributes.put(Attribute.SEARCH_TYPE, SearchType.QUERY_THEN_FETCH.toString().toLowerCase(Locale.ROOT)); - attributes.put(Attribute.SOURCE, "{\"size\":20}"); + attributes.put(Attribute.SOURCE, searchSourceBuilder); attributes.put(Attribute.TOTAL_SHARDS, randomIntBetween(1, 100)); attributes.put(Attribute.INDICES, randomArray(1, 3, Object[]::new, () -> randomAlphaOfLengthBetween(5, 10))); attributes.put(Attribute.PHASE_LATENCY_MAP, phaseLatencyMap); diff --git a/server/src/test/java/org/opensearch/index/query/QueryShapeVisitorTests.java b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/categorizor/QueryShapeVisitorTests.java similarity index 70% rename from server/src/test/java/org/opensearch/index/query/QueryShapeVisitorTests.java rename to plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/categorizor/QueryShapeVisitorTests.java index 18b814aec61c2..8e3947f0f3902 100644 --- a/server/src/test/java/org/opensearch/index/query/QueryShapeVisitorTests.java +++ b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/categorizor/QueryShapeVisitorTests.java @@ -6,11 +6,18 @@ * compatible open source license. */ -package org.opensearch.index.query; +package org.opensearch.plugin.insights.core.categorizor; +import org.opensearch.index.query.BoolQueryBuilder; +import org.opensearch.index.query.ConstantScoreQueryBuilder; +import org.opensearch.index.query.MatchQueryBuilder; +import org.opensearch.index.query.QueryBuilder; +import org.opensearch.index.query.RangeQueryBuilder; +import org.opensearch.index.query.RegexpQueryBuilder; +import org.opensearch.index.query.TermQueryBuilder; +import org.opensearch.index.query.TermsQueryBuilder; import org.opensearch.test.OpenSearchTestCase; - -import static org.junit.Assert.assertEquals; +import org.opensearch.plugin.insights.core.categorizer.QueryShapeVisitor; public final class QueryShapeVisitorTests extends OpenSearchTestCase { public void testQueryShapeVisitor() { diff --git a/server/src/test/java/org/opensearch/action/search/SearchQueryCategorizerTests.java b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/categorizor/SearchQueryCategorizerTests.java similarity index 98% rename from server/src/test/java/org/opensearch/action/search/SearchQueryCategorizerTests.java rename to plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/categorizor/SearchQueryCategorizerTests.java index 4878a463729f9..41be96e1ec5c1 100644 --- a/server/src/test/java/org/opensearch/action/search/SearchQueryCategorizerTests.java +++ b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/categorizor/SearchQueryCategorizerTests.java @@ -6,8 +6,10 @@ * compatible open source license. */ -package org.opensearch.action.search; +package org.opensearch.plugin.insights.core.categorizor; +import org.junit.Before; +import org.mockito.ArgumentCaptor; import org.opensearch.index.query.BoolQueryBuilder; import org.opensearch.index.query.BoostingQueryBuilder; import org.opensearch.index.query.MatchNoneQueryBuilder; @@ -20,6 +22,7 @@ import org.opensearch.index.query.TermQueryBuilder; import org.opensearch.index.query.WildcardQueryBuilder; import org.opensearch.index.query.functionscore.FunctionScoreQueryBuilder; +import org.opensearch.plugin.insights.core.categorizer.SearchQueryCategorizer; import org.opensearch.search.aggregations.bucket.range.RangeAggregationBuilder; import org.opensearch.search.aggregations.bucket.terms.MultiTermsAggregationBuilder; import org.opensearch.search.aggregations.support.MultiTermsValuesSourceConfig; @@ -30,12 +33,9 @@ import org.opensearch.telemetry.metrics.MetricsRegistry; import org.opensearch.telemetry.metrics.tags.Tags; import org.opensearch.test.OpenSearchTestCase; -import org.junit.Before; import java.util.Arrays; -import org.mockito.ArgumentCaptor; - import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; diff --git a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/listener/QueryInsightsListenerTests.java b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/listener/QueryInsightsListenerTests.java index 86de44c680188..9ce4adfd1f506 100644 --- a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/listener/QueryInsightsListenerTests.java +++ b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/listener/QueryInsightsListenerTests.java @@ -136,7 +136,7 @@ public void testOnRequestEnd() throws InterruptedException { assertEquals(timestamp.longValue(), generatedRecord.getTimestamp()); assertEquals(numberOfShards, generatedRecord.getAttributes().get(Attribute.TOTAL_SHARDS)); assertEquals(searchType.toString().toLowerCase(Locale.ROOT), generatedRecord.getAttributes().get(Attribute.SEARCH_TYPE)); - assertEquals(searchSourceBuilder.toString(), generatedRecord.getAttributes().get(Attribute.SOURCE)); + assertEquals(searchSourceBuilder, generatedRecord.getAttributes().get(Attribute.SOURCE)); Map labels = (Map) generatedRecord.getAttributes().get(Attribute.LABELS); assertEquals("userLabel", labels.get(Task.X_OPAQUE_ID)); } diff --git a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/service/QueryInsightsServiceTests.java b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/service/QueryInsightsServiceTests.java index 75a5768f50681..1b511786865c5 100644 --- a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/service/QueryInsightsServiceTests.java +++ b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/service/QueryInsightsServiceTests.java @@ -15,6 +15,8 @@ import org.opensearch.plugin.insights.rules.model.MetricType; import org.opensearch.plugin.insights.rules.model.SearchQueryRecord; import org.opensearch.plugin.insights.settings.QueryInsightsSettings; +import org.opensearch.telemetry.metrics.MetricsRegistry; +import org.opensearch.telemetry.metrics.noop.NoopMetricsRegistry; import org.opensearch.test.OpenSearchTestCase; import org.opensearch.threadpool.ThreadPool; import org.junit.Before; @@ -35,7 +37,7 @@ public void setup() { Settings settings = settingsBuilder.build(); ClusterSettings clusterSettings = new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); QueryInsightsTestUtils.registerAllQueryInsightsSettings(clusterSettings); - queryInsightsService = new QueryInsightsService(clusterSettings, threadPool, client); + queryInsightsService = new QueryInsightsService(clusterSettings, threadPool, client, NoopMetricsRegistry.INSTANCE); queryInsightsService.enableCollection(MetricType.LATENCY, true); queryInsightsService.enableCollection(MetricType.CPU, true); queryInsightsService.enableCollection(MetricType.MEMORY, true); 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 6e380775355a2..a2cf559a229a2 100644 --- a/server/src/main/java/org/opensearch/action/search/TransportSearchAction.java +++ b/server/src/main/java/org/opensearch/action/search/TransportSearchAction.java @@ -143,13 +143,6 @@ public class TransportSearchAction extends HandledTransportAction SEARCH_QUERY_METRICS_ENABLED_SETTING = Setting.boolSetting( - "search.query.metrics.enabled", - false, - Setting.Property.NodeScope, - Setting.Property.Dynamic - ); - // cluster level setting for timeout based search cancellation. If search request level parameter is present then that will take // precedence over the cluster setting value public static final String SEARCH_CANCEL_AFTER_TIME_INTERVAL_SETTING_KEY = "search.cancel_after_time_interval"; @@ -181,12 +174,7 @@ public class TransportSearchAction extends HandledTransportAction buildPerIndexAliasFilter( SearchRequest request, ClusterState clusterState, @@ -473,14 +451,6 @@ private void executeRequest( } ActionListener requestTransformListener = ActionListener.wrap(sr -> { - if (searchQueryMetricsEnabled) { - try { - searchQueryCategorizer.categorize(sr.source()); - } catch (Exception e) { - logger.error("Error while trying to categorize the query.", e); - } - } - ActionListener rewriteListener = buildRewriteListener( sr, task, From 9e8c75d7c440714bd82503e0c2a37a5eded8324e Mon Sep 17 00:00:00 2001 From: Siddhant Deshmukh Date: Mon, 24 Jun 2024 13:39:52 -0700 Subject: [PATCH 02/22] Add changelog Signed-off-by: Siddhant Deshmukh --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c7cfbba928da9..496065596cd68 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Make the class CommunityIdProcessor final ([#14448](https://github.com/opensearch-project/OpenSearch/pull/14448)) - Allow @InternalApi annotation on classes not meant to be constructed outside of the OpenSearch core ([#14575](https://github.com/opensearch-project/OpenSearch/pull/14575)) - Add @InternalApi annotation to japicmp exclusions ([#14597](https://github.com/opensearch-project/OpenSearch/pull/14597)) +- Move query categorization changes to query insights plugin ([#14528](https://github.com/opensearch-project/OpenSearch/pull/14528)) ### Deprecated From 4af171d13aebebc9e361911f49bc45bb1104d612 Mon Sep 17 00:00:00 2001 From: Siddhant Deshmukh Date: Mon, 24 Jun 2024 13:59:24 -0700 Subject: [PATCH 03/22] Fix build Signed-off-by: Siddhant Deshmukh --- .../java/org/opensearch/common/settings/ClusterSettings.java | 1 - 1 file changed, 1 deletion(-) 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 233a8d732d178..a9feb3c6fe64f 100644 --- a/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java +++ b/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java @@ -403,7 +403,6 @@ public void apply(Settings value, Settings current, Settings previous) { SearchService.DEFAULT_ALLOW_PARTIAL_SEARCH_RESULTS, TransportSearchAction.SHARD_COUNT_LIMIT_SETTING, TransportSearchAction.SEARCH_CANCEL_AFTER_TIME_INTERVAL_SETTING, - TransportSearchAction.SEARCH_QUERY_METRICS_ENABLED_SETTING, TransportSearchAction.SEARCH_PHASE_TOOK_ENABLED, SearchRequestStats.SEARCH_REQUEST_STATS_ENABLED, RemoteClusterService.REMOTE_CLUSTER_SKIP_UNAVAILABLE, From efa8184a6ed77a2b44874ee0a1c07dbc1c0d8f4f Mon Sep 17 00:00:00 2001 From: Siddhant Deshmukh Date: Mon, 24 Jun 2024 16:19:11 -0700 Subject: [PATCH 04/22] Javadocs Signed-off-by: Siddhant Deshmukh --- .../core/categorizer/QueryShapeVisitor.java | 9 ++++++++ .../SearchQueryAggregationCategorizer.java | 8 +++++++ .../categorizer/SearchQueryCategorizer.java | 17 ++++++++++++++- .../core/categorizer/SearchQueryCounters.java | 21 +++++++++++++++++++ .../core/service/QueryInsightsService.java | 1 + .../settings/QueryCategorizationSettings.java | 6 ++++++ 6 files changed, 61 insertions(+), 1 deletion(-) diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer/QueryShapeVisitor.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer/QueryShapeVisitor.java index 92724c9cc57b2..f967b05dc7258 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer/QueryShapeVisitor.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer/QueryShapeVisitor.java @@ -57,6 +57,10 @@ public QueryBuilderVisitor getChildVisitor(BooleanClause.Occur occur) { return childVisitorWrapper; } + /** + * Convert query builder tree to json + * @return + */ public String toJson() { StringBuilder outputBuilder = new StringBuilder("{\"type\":\"").append(queryType.get()).append("\""); for (Map.Entry> entry : childVisitors.entrySet()) { @@ -75,6 +79,11 @@ public String toJson() { return outputBuilder.toString(); } + /** + * Pretty print the query builder tree + * @param indent indent size + * @return + */ public String prettyPrintTree(String indent) { StringBuilder outputBuilder = new StringBuilder(indent).append(queryType.get()).append("\n"); for (Map.Entry> entry : childVisitors.entrySet()) { diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer/SearchQueryAggregationCategorizer.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer/SearchQueryAggregationCategorizer.java index 94a9b9182e9d7..31f12f57be562 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer/SearchQueryAggregationCategorizer.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer/SearchQueryAggregationCategorizer.java @@ -22,10 +22,18 @@ public class SearchQueryAggregationCategorizer { private static final String TYPE_TAG = "type"; private final SearchQueryCounters searchQueryCounters; + /** + * Constructor for SearchQueryAggregationCategorizer + * @param searchQueryCounters + */ public SearchQueryAggregationCategorizer(SearchQueryCounters searchQueryCounters) { this.searchQueryCounters = searchQueryCounters; } + /** + * Increment aggregation related counters + * @param aggregatorFactories input aggregations + */ public void incrementSearchQueryAggregationCounters(Collection aggregatorFactories) { for (AggregationBuilder aggregationBuilder : aggregatorFactories) { incrementCountersRecursively(aggregationBuilder); diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer/SearchQueryCategorizer.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer/SearchQueryCategorizer.java index cd8f3b3e978bf..96b91b6fd5bb0 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer/SearchQueryCategorizer.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer/SearchQueryCategorizer.java @@ -31,15 +31,26 @@ public final class SearchQueryCategorizer { private static final Logger log = LogManager.getLogger(SearchQueryCategorizer.class); + /** + * Contains all the search query counters + */ public final SearchQueryCounters searchQueryCounters; final SearchQueryAggregationCategorizer searchQueryAggregationCategorizer; + /** + * Constructor for SearchQueryCategorizor + * @param metricsRegistry + */ public SearchQueryCategorizer(MetricsRegistry metricsRegistry) { searchQueryCounters = new SearchQueryCounters(metricsRegistry); searchQueryAggregationCategorizer = new SearchQueryAggregationCategorizer(searchQueryCounters); } + /** + * Consume records and increment counters for the records + * @param records records to consume + */ public void consumeRecords(List records) { for (SearchQueryRecord record : records) { SearchSourceBuilder source = (SearchSourceBuilder) record.getAttributes().get(Attribute.SOURCE); @@ -47,6 +58,10 @@ public void consumeRecords(List records) { } } + /** + * Increment categorizations counters for the given source search query + * @param source search query source + */ public void categorize(SearchSourceBuilder source) { QueryBuilder topLevelQueryBuilder = source.query(); logQueryShape(topLevelQueryBuilder); @@ -58,7 +73,7 @@ public void categorize(SearchSourceBuilder source) { private void incrementQuerySortCounters(List> sorts) { if (sorts != null && sorts.size() > 0) { for (ListIterator> it = sorts.listIterator(); it.hasNext();) { - SortBuilder sortBuilder = it.next(); + SortBuilder sortBuilder = it.next(); String sortOrder = sortBuilder.order().toString(); searchQueryCounters.sortCounter.add(1, Tags.create().addTag("sort_order", sortOrder)); } diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer/SearchQueryCounters.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer/SearchQueryCounters.java index 42569b2978379..8c7c91f794f16 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer/SearchQueryCounters.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer/SearchQueryCounters.java @@ -24,12 +24,28 @@ public final class SearchQueryCounters { private static final String LEVEL_TAG = "level"; private static final String UNIT = "1"; private final MetricsRegistry metricsRegistry; + /** + * Aggregation counter + */ public final Counter aggCounter; + /** + * Counter for all other query types (catch all) + */ public final Counter otherQueryCounter; + /** + * Counter for sort + */ public final Counter sortCounter; private final Map, Counter> queryHandlers; + /** + * Counter name to Counter object map + */ public final ConcurrentHashMap nameToQueryTypeCounters; + /** + * Constructor + * @param metricsRegistry opentelemetry metrics registry + */ public SearchQueryCounters(MetricsRegistry metricsRegistry) { this.metricsRegistry = metricsRegistry; this.nameToQueryTypeCounters = new ConcurrentHashMap<>(); @@ -52,6 +68,11 @@ public SearchQueryCounters(MetricsRegistry metricsRegistry) { } + /** + * Increment counter + * @param queryBuilder query builder + * @param level level of query builder, 0 being highest level + */ public void incrementCounter(QueryBuilder queryBuilder, int level) { String uniqueQueryCounterName = queryBuilder.getName(); diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/QueryInsightsService.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/QueryInsightsService.java index e81670a1d842d..7ee5e3cff4af6 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/QueryInsightsService.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/QueryInsightsService.java @@ -89,6 +89,7 @@ public class QueryInsightsService extends AbstractLifecycleComponent { * @param clusterSettings OpenSearch cluster level settings * @param threadPool The OpenSearch thread pool to run async tasks * @param client OS client + * @param metricsRegistry Opentelemetry Metrics registry */ @Inject public QueryInsightsService(final ClusterSettings clusterSettings, final ThreadPool threadPool, final Client client, final MetricsRegistry metricsRegistry) { diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/settings/QueryCategorizationSettings.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/settings/QueryCategorizationSettings.java index 1220d58294bc0..a0d8873c4bb62 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/settings/QueryCategorizationSettings.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/settings/QueryCategorizationSettings.java @@ -10,7 +10,13 @@ import org.opensearch.common.settings.Setting; +/** + * Settings for Query Categorization + */ public class QueryCategorizationSettings { + /** + * Enabling search query metrics + */ public static final Setting SEARCH_QUERY_METRICS_ENABLED_SETTING = Setting.boolSetting( "search.query.metrics.enabled", false, From 916c8b9f3aa1dccdd46b6a950973fe757677362c Mon Sep 17 00:00:00 2001 From: Siddhant Deshmukh Date: Mon, 24 Jun 2024 16:33:17 -0700 Subject: [PATCH 05/22] Add javadocs Signed-off-by: Siddhant Deshmukh --- .../plugin/insights/core/categorizer/QueryShapeVisitor.java | 5 +++++ .../insights/settings/QueryCategorizationSettings.java | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer/QueryShapeVisitor.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer/QueryShapeVisitor.java index f967b05dc7258..f11d0fb36cafa 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer/QueryShapeVisitor.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer/QueryShapeVisitor.java @@ -94,4 +94,9 @@ public String prettyPrintTree(String indent) { } return outputBuilder.toString(); } + + /** + * Default constructor + */ + public QueryShapeVisitor() {} } diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/settings/QueryCategorizationSettings.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/settings/QueryCategorizationSettings.java index a0d8873c4bb62..a5a65af93448e 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/settings/QueryCategorizationSettings.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/settings/QueryCategorizationSettings.java @@ -24,4 +24,9 @@ public class QueryCategorizationSettings { Setting.Property.Dynamic ); + /** + * Default constructor + */ + public QueryCategorizationSettings() {} + } From 8b331061a16461b0b7192b11e6f349a4a3f3182e Mon Sep 17 00:00:00 2001 From: Siddhant Deshmukh Date: Mon, 24 Jun 2024 16:42:26 -0700 Subject: [PATCH 06/22] Add package javadocs Signed-off-by: Siddhant Deshmukh --- .../insights/core/categorizer/package-info.java | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer/package-info.java diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer/package-info.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer/package-info.java new file mode 100644 index 0000000000000..84d5e47af4520 --- /dev/null +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer/package-info.java @@ -0,0 +1,16 @@ +/* + * 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. + */ + +/** + * Query Insights exporter + */ + +/** + * Query Insights search query categorizor to collect metrics related to search queries + */ +package org.opensearch.plugin.insights.core.categorizer; From 78f0733fbb4eed0e0c53a220e83cc8d7bce2e5eb Mon Sep 17 00:00:00 2001 From: Siddhant Deshmukh Date: Mon, 24 Jun 2024 19:48:45 -0700 Subject: [PATCH 07/22] Fix build Signed-off-by: Siddhant Deshmukh --- .../plugin/insights/core/categorizer/QueryShapeVisitor.java | 4 ++-- .../core/categorizer/SearchQueryAggregationCategorizer.java | 2 +- .../insights/core/categorizer/SearchQueryCategorizer.java | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer/QueryShapeVisitor.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer/QueryShapeVisitor.java index f11d0fb36cafa..2f3f729e79752 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer/QueryShapeVisitor.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer/QueryShapeVisitor.java @@ -59,7 +59,7 @@ public QueryBuilderVisitor getChildVisitor(BooleanClause.Occur occur) { /** * Convert query builder tree to json - * @return + * @return json query builder tree as a string */ public String toJson() { StringBuilder outputBuilder = new StringBuilder("{\"type\":\"").append(queryType.get()).append("\""); @@ -82,7 +82,7 @@ public String toJson() { /** * Pretty print the query builder tree * @param indent indent size - * @return + * @return Query builder tree as a pretty string */ public String prettyPrintTree(String indent) { StringBuilder outputBuilder = new StringBuilder(indent).append(queryType.get()).append("\n"); diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer/SearchQueryAggregationCategorizer.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer/SearchQueryAggregationCategorizer.java index 31f12f57be562..876eec3813f5f 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer/SearchQueryAggregationCategorizer.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer/SearchQueryAggregationCategorizer.java @@ -24,7 +24,7 @@ public class SearchQueryAggregationCategorizer { /** * Constructor for SearchQueryAggregationCategorizer - * @param searchQueryCounters + * @param searchQueryCounters Object containing all query counters */ public SearchQueryAggregationCategorizer(SearchQueryCounters searchQueryCounters) { this.searchQueryCounters = searchQueryCounters; diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer/SearchQueryCategorizer.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer/SearchQueryCategorizer.java index 96b91b6fd5bb0..c2a0713fa6c83 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer/SearchQueryCategorizer.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer/SearchQueryCategorizer.java @@ -40,7 +40,7 @@ public final class SearchQueryCategorizer { /** * Constructor for SearchQueryCategorizor - * @param metricsRegistry + * @param metricsRegistry opentelemetry metrics registry */ public SearchQueryCategorizer(MetricsRegistry metricsRegistry) { searchQueryCounters = new SearchQueryCounters(metricsRegistry); From 71000ef6f0418c8fed33bddaef3c2e613fe0b406 Mon Sep 17 00:00:00 2001 From: Siddhant Deshmukh Date: Mon, 24 Jun 2024 20:08:23 -0700 Subject: [PATCH 08/22] Spotless java check Signed-off-by: Siddhant Deshmukh --- .../plugin/insights/QueryInsightsPlugin.java | 8 ++++++-- .../insights/core/service/QueryInsightsService.java | 10 +++++++--- .../core/categorizor/QueryShapeVisitorTests.java | 2 +- .../core/categorizor/SearchQueryCategorizerTests.java | 5 +++-- .../core/service/QueryInsightsServiceTests.java | 1 - 5 files changed, 17 insertions(+), 9 deletions(-) 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 526adace7be56..9d9900f57dd84 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 @@ -9,7 +9,6 @@ package org.opensearch.plugin.insights; import org.opensearch.action.ActionRequest; -import org.opensearch.action.search.TransportSearchAction; import org.opensearch.client.Client; import org.opensearch.cluster.metadata.IndexNameExpressionResolver; import org.opensearch.cluster.node.DiscoveryNodes; @@ -76,7 +75,12 @@ public Collection createComponents( final MetricsRegistry metricsRegistry ) { // create top n queries service - final QueryInsightsService queryInsightsService = new QueryInsightsService(clusterService.getClusterSettings(), threadPool, client, metricsRegistry); + final QueryInsightsService queryInsightsService = new QueryInsightsService( + clusterService.getClusterSettings(), + threadPool, + client, + metricsRegistry + ); return List.of(queryInsightsService, new QueryInsightsListener(clusterService, queryInsightsService)); } diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/QueryInsightsService.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/QueryInsightsService.java index 7ee5e3cff4af6..4adf47e6531a3 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/QueryInsightsService.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/QueryInsightsService.java @@ -92,7 +92,12 @@ public class QueryInsightsService extends AbstractLifecycleComponent { * @param metricsRegistry Opentelemetry Metrics registry */ @Inject - public QueryInsightsService(final ClusterSettings clusterSettings, final ThreadPool threadPool, final Client client, final MetricsRegistry metricsRegistry) { + public QueryInsightsService( + final ClusterSettings clusterSettings, + final ThreadPool threadPool, + final Client client, + final MetricsRegistry metricsRegistry + ) { enableCollect = new HashMap<>(); queryRecordsQueue = new LinkedBlockingQueue<>(QueryInsightsSettings.QUERY_RECORD_QUEUE_CAPACITY); this.threadPool = threadPool; @@ -112,8 +117,7 @@ public QueryInsightsService(final ClusterSettings clusterSettings, final ThreadP } this.searchQueryMetricsEnabled = clusterSettings.get(SEARCH_QUERY_METRICS_ENABLED_SETTING); - clusterSettings - .addSettingsUpdateConsumer(SEARCH_QUERY_METRICS_ENABLED_SETTING, this::setSearchQueryMetricsEnabled); + clusterSettings.addSettingsUpdateConsumer(SEARCH_QUERY_METRICS_ENABLED_SETTING, this::setSearchQueryMetricsEnabled); this.metricsRegistry = metricsRegistry; } diff --git a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/categorizor/QueryShapeVisitorTests.java b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/categorizor/QueryShapeVisitorTests.java index 8e3947f0f3902..2b6fd6f1daec1 100644 --- a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/categorizor/QueryShapeVisitorTests.java +++ b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/categorizor/QueryShapeVisitorTests.java @@ -16,8 +16,8 @@ import org.opensearch.index.query.RegexpQueryBuilder; import org.opensearch.index.query.TermQueryBuilder; import org.opensearch.index.query.TermsQueryBuilder; -import org.opensearch.test.OpenSearchTestCase; import org.opensearch.plugin.insights.core.categorizer.QueryShapeVisitor; +import org.opensearch.test.OpenSearchTestCase; public final class QueryShapeVisitorTests extends OpenSearchTestCase { public void testQueryShapeVisitor() { diff --git a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/categorizor/SearchQueryCategorizerTests.java b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/categorizor/SearchQueryCategorizerTests.java index 41be96e1ec5c1..e78fc7bd23f8d 100644 --- a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/categorizor/SearchQueryCategorizerTests.java +++ b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/categorizor/SearchQueryCategorizerTests.java @@ -8,8 +8,6 @@ package org.opensearch.plugin.insights.core.categorizor; -import org.junit.Before; -import org.mockito.ArgumentCaptor; import org.opensearch.index.query.BoolQueryBuilder; import org.opensearch.index.query.BoostingQueryBuilder; import org.opensearch.index.query.MatchNoneQueryBuilder; @@ -33,9 +31,12 @@ import org.opensearch.telemetry.metrics.MetricsRegistry; import org.opensearch.telemetry.metrics.tags.Tags; import org.opensearch.test.OpenSearchTestCase; +import org.junit.Before; import java.util.Arrays; +import org.mockito.ArgumentCaptor; + import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; diff --git a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/service/QueryInsightsServiceTests.java b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/service/QueryInsightsServiceTests.java index 1b511786865c5..1c81d516a0f97 100644 --- a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/service/QueryInsightsServiceTests.java +++ b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/service/QueryInsightsServiceTests.java @@ -15,7 +15,6 @@ import org.opensearch.plugin.insights.rules.model.MetricType; import org.opensearch.plugin.insights.rules.model.SearchQueryRecord; import org.opensearch.plugin.insights.settings.QueryInsightsSettings; -import org.opensearch.telemetry.metrics.MetricsRegistry; import org.opensearch.telemetry.metrics.noop.NoopMetricsRegistry; import org.opensearch.test.OpenSearchTestCase; import org.opensearch.threadpool.ThreadPool; From f2d8c3f03201e2875a1a77cb0e83525326e929b5 Mon Sep 17 00:00:00 2001 From: Siddhant Deshmukh Date: Tue, 25 Jun 2024 18:14:59 -0700 Subject: [PATCH 09/22] Pass source object to query record rather than source string Signed-off-by: Siddhant Deshmukh --- .../insights/rules/model/Attribute.java | 68 +++++++++++++++++++ .../rules/model/SearchQueryRecord.java | 5 +- .../insights/QueryInsightsPluginTests.java | 2 + .../insights/QueryInsightsTestUtils.java | 2 + 4 files changed, 75 insertions(+), 2 deletions(-) diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model/Attribute.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model/Attribute.java index dcdb085fdc6fa..9e7c29f896abe 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model/Attribute.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model/Attribute.java @@ -8,11 +8,16 @@ package org.opensearch.plugin.insights.rules.model; +import org.apache.lucene.util.ArrayUtil; import org.opensearch.core.common.io.stream.StreamInput; import org.opensearch.core.common.io.stream.StreamOutput; +import org.opensearch.search.builder.SearchSourceBuilder; import java.io.IOException; +import java.util.Collections; +import java.util.HashMap; import java.util.Locale; +import java.util.Map; /** * Valid attributes for a search query record @@ -75,6 +80,69 @@ static void writeTo(final StreamOutput out, final Attribute attribute) throws IO out.writeString(attribute.toString()); } + /** + * Write Attribute value to a StreamOutput + * @param out the StreamOutput to write + * @param attributeValue the Attribute value to write + */ + public static void writeValueTo(StreamOutput out, Object attributeValue) throws IOException { + if (attributeValue instanceof SearchSourceBuilder) { + ((SearchSourceBuilder) attributeValue).writeTo(out); + } else { + out.writeGenericValue(attributeValue); + } + } + + /** + * Read attribute value from the input stream given the Attribute type + * + * @param in the streaminput to read + * @param attribute attribute type to differentiate between Source and others + * @return parse value + * @throws IOException + */ + public static Object readAttributeValue(StreamInput in, Attribute attribute) throws IOException { + if (attribute == Attribute.SOURCE) { + SearchSourceBuilder builder = new SearchSourceBuilder(in); + return builder; + } else { + return in.readGenericValue(); + } + } + + /** + * Read attribute map from the input stream + * + * @param in the streaminput to read + * @return parsed attribute map + * @throws IOException + */ + public static Map readAttributeMap(StreamInput in) throws IOException { + int size = readArraySize(in); + if (size == 0) { + return Collections.emptyMap(); + } + Map map = new HashMap<>(size); + + for (int i = 0; i < size; i++) { + Attribute key = readFromStream(in); + Object value = readAttributeValue(in, key); + map.put(key, value); + } + return map; + } + + private static int readArraySize(StreamInput in) throws IOException { + final int arraySize = in.readVInt(); + if (arraySize > ArrayUtil.MAX_ARRAY_LENGTH) { + throw new IllegalStateException("array length must be <= to " + ArrayUtil.MAX_ARRAY_LENGTH + " but was: " + arraySize); + } + if (arraySize < 0) { + throw new NegativeArraySizeException("array size must be positive but was: " + arraySize); + } + return arraySize; + } + @Override public String toString() { return this.name().toLowerCase(Locale.ROOT); diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model/SearchQueryRecord.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model/SearchQueryRecord.java index fec00a680ae58..76989f3ec96bc 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model/SearchQueryRecord.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model/SearchQueryRecord.java @@ -45,7 +45,7 @@ public SearchQueryRecord(final StreamInput in) throws IOException, ClassCastExce measurements = new HashMap<>(); in.readMap(MetricType::readFromStream, StreamInput::readGenericValue) .forEach(((metricType, o) -> measurements.put(metricType, metricType.parseValue(o)))); - this.attributes = in.readMap(Attribute::readFromStream, StreamInput::readGenericValue); + this.attributes = Attribute.readAttributeMap(in); } /** @@ -134,7 +134,8 @@ public XContentBuilder toXContent(final XContentBuilder builder, final ToXConten public void writeTo(final StreamOutput out) throws IOException { out.writeLong(timestamp); out.writeMap(measurements, (stream, metricType) -> MetricType.writeTo(out, metricType), StreamOutput::writeGenericValue); - out.writeMap(attributes, (stream, attribute) -> Attribute.writeTo(out, attribute), StreamOutput::writeGenericValue); + out.writeMap(attributes, (stream, attribute) -> Attribute.writeTo(out, attribute), (stream, attributeValue) -> + Attribute.writeValueTo(out, attributeValue)); } /** 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 2efe9085a39ee..382ea3c5fa535 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 @@ -83,6 +83,8 @@ public void testCreateComponent() { null, null, null, + null, + null, null ); assertEquals(2, components.size()); 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 dd5df4821ce29..a2dd8822921c8 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 @@ -18,6 +18,7 @@ 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.plugin.insights.settings.QueryCategorizationSettings; import org.opensearch.plugin.insights.settings.QueryInsightsSettings; import org.opensearch.search.builder.SearchSourceBuilder; import org.opensearch.test.VersionUtils; @@ -206,5 +207,6 @@ public static void registerAllQueryInsightsSettings(ClusterSettings clusterSetti clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_MEMORY_QUERIES_SIZE); clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_MEMORY_QUERIES_WINDOW_SIZE); clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_MEMORY_EXPORTER_SETTINGS); + clusterSettings.registerSetting(QueryCategorizationSettings.SEARCH_QUERY_METRICS_ENABLED_SETTING); } } From 3706eda92ad2885e5d454cb386896958d595db57 Mon Sep 17 00:00:00 2001 From: Siddhant Deshmukh Date: Tue, 25 Jun 2024 18:26:25 -0700 Subject: [PATCH 10/22] Spotless java Signed-off-by: Siddhant Deshmukh --- .../opensearch/plugin/insights/rules/model/Attribute.java | 2 +- .../plugin/insights/rules/model/SearchQueryRecord.java | 7 +++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model/Attribute.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model/Attribute.java index 9e7c29f896abe..d8d850fbb6003 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model/Attribute.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model/Attribute.java @@ -131,7 +131,7 @@ public static Map readAttributeMap(StreamInput in) throws IOE } return map; } - + private static int readArraySize(StreamInput in) throws IOException { final int arraySize = in.readVInt(); if (arraySize > ArrayUtil.MAX_ARRAY_LENGTH) { diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model/SearchQueryRecord.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model/SearchQueryRecord.java index 76989f3ec96bc..a6e6b4a9051f0 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model/SearchQueryRecord.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model/SearchQueryRecord.java @@ -134,8 +134,11 @@ public XContentBuilder toXContent(final XContentBuilder builder, final ToXConten public void writeTo(final StreamOutput out) throws IOException { out.writeLong(timestamp); out.writeMap(measurements, (stream, metricType) -> MetricType.writeTo(out, metricType), StreamOutput::writeGenericValue); - out.writeMap(attributes, (stream, attribute) -> Attribute.writeTo(out, attribute), (stream, attributeValue) -> - Attribute.writeValueTo(out, attributeValue)); + out.writeMap( + attributes, + (stream, attribute) -> Attribute.writeTo(out, attribute), + (stream, attributeValue) -> Attribute.writeValueTo(out, attributeValue) + ); } /** From 5c0e83ec8b5ced019eb173ccda0b661a293e7553 Mon Sep 17 00:00:00 2001 From: Siddhant Deshmukh Date: Tue, 25 Jun 2024 21:26:17 -0700 Subject: [PATCH 11/22] Javadoc Signed-off-by: Siddhant Deshmukh --- .../org/opensearch/plugin/insights/rules/model/Attribute.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model/Attribute.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model/Attribute.java index d8d850fbb6003..b6b4b111aab81 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model/Attribute.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model/Attribute.java @@ -99,7 +99,7 @@ public static void writeValueTo(StreamOutput out, Object attributeValue) throws * @param in the streaminput to read * @param attribute attribute type to differentiate between Source and others * @return parse value - * @throws IOException + * @throws IOException IOException */ public static Object readAttributeValue(StreamInput in, Attribute attribute) throws IOException { if (attribute == Attribute.SOURCE) { @@ -115,7 +115,7 @@ public static Object readAttributeValue(StreamInput in, Attribute attribute) thr * * @param in the streaminput to read * @return parsed attribute map - * @throws IOException + * @throws IOException IOException */ public static Map readAttributeMap(StreamInput in) throws IOException { int size = readArraySize(in); From 8ff9e2130fc204912d57a0bc010351266048f520 Mon Sep 17 00:00:00 2001 From: Siddhant Deshmukh Date: Thu, 27 Jun 2024 16:14:32 -0700 Subject: [PATCH 12/22] Add setting Signed-off-by: Siddhant Deshmukh --- .../org/opensearch/plugin/insights/QueryInsightsPlugin.java | 4 +++- .../plugin/insights/core/categorizer/package-info.java | 4 ---- 2 files changed, 3 insertions(+), 5 deletions(-) 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 9d9900f57dd84..ba33bbba1a1e7 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 @@ -30,6 +30,7 @@ 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.QueryCategorizationSettings; import org.opensearch.plugin.insights.settings.QueryInsightsSettings; import org.opensearch.plugins.ActionPlugin; import org.opensearch.plugins.Plugin; @@ -129,7 +130,8 @@ public List> getSettings() { QueryInsightsSettings.TOP_N_MEMORY_QUERIES_ENABLED, QueryInsightsSettings.TOP_N_MEMORY_QUERIES_SIZE, QueryInsightsSettings.TOP_N_MEMORY_QUERIES_WINDOW_SIZE, - QueryInsightsSettings.TOP_N_MEMORY_EXPORTER_SETTINGS + QueryInsightsSettings.TOP_N_MEMORY_EXPORTER_SETTINGS, + QueryCategorizationSettings.SEARCH_QUERY_METRICS_ENABLED_SETTING ); } } diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer/package-info.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer/package-info.java index 84d5e47af4520..a59f4490c40e0 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer/package-info.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer/package-info.java @@ -6,10 +6,6 @@ * compatible open source license. */ -/** - * Query Insights exporter - */ - /** * Query Insights search query categorizor to collect metrics related to search queries */ From ac7fd90567b548d568bdeef19cc77a9d88e78861 Mon Sep 17 00:00:00 2001 From: Siddhant Deshmukh Date: Fri, 28 Jun 2024 16:56:11 -0700 Subject: [PATCH 13/22] Address review comments Signed-off-by: Siddhant Deshmukh --- .../core/service/QueryInsightsService.java | 2 +- .../categorizer/QueryShapeVisitor.java | 2 +- .../SearchQueryAggregationCategorizer.java | 6 +-- .../categorizer/SearchQueryCategorizer.java | 12 +++-- .../SearchQueryCategorizingVisitor.java | 2 +- .../categorizer/SearchQueryCounters.java | 30 +++++++++-- .../categorizer/package-info.java | 2 +- .../categorizor/QueryShapeVisitorTests.java | 4 +- .../SearchQueryCategorizerTests.java | 52 +++++++++---------- 9 files changed, 66 insertions(+), 46 deletions(-) rename plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/{ => service}/categorizer/QueryShapeVisitor.java (98%) rename plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/{ => service}/categorizer/SearchQueryAggregationCategorizer.java (88%) rename plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/{ => service}/categorizer/SearchQueryCategorizer.java (90%) rename plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/{ => service}/categorizer/SearchQueryCategorizingVisitor.java (95%) rename plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/{ => service}/categorizer/SearchQueryCounters.java (78%) rename plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/{ => service}/categorizer/package-info.java (82%) rename plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/{ => service}/categorizor/QueryShapeVisitorTests.java (92%) rename plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/{ => service}/categorizor/SearchQueryCategorizerTests.java (71%) diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/QueryInsightsService.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/QueryInsightsService.java index 4adf47e6531a3..f0f504447cf82 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/QueryInsightsService.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/QueryInsightsService.java @@ -16,7 +16,7 @@ import org.opensearch.common.settings.ClusterSettings; import org.opensearch.common.settings.Settings; import org.opensearch.common.unit.TimeValue; -import org.opensearch.plugin.insights.core.categorizer.SearchQueryCategorizer; +import org.opensearch.plugin.insights.core.service.categorizer.SearchQueryCategorizer; import org.opensearch.plugin.insights.core.exporter.QueryInsightsExporterFactory; import org.opensearch.plugin.insights.rules.model.MetricType; import org.opensearch.plugin.insights.rules.model.SearchQueryRecord; diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer/QueryShapeVisitor.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/categorizer/QueryShapeVisitor.java similarity index 98% rename from plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer/QueryShapeVisitor.java rename to plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/categorizer/QueryShapeVisitor.java index 2f3f729e79752..626e05dcabb4e 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer/QueryShapeVisitor.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/categorizer/QueryShapeVisitor.java @@ -6,7 +6,7 @@ * compatible open source license. */ -package org.opensearch.plugin.insights.core.categorizer; +package org.opensearch.plugin.insights.core.service.categorizer; import org.apache.lucene.search.BooleanClause; import org.opensearch.common.SetOnce; diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer/SearchQueryAggregationCategorizer.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/categorizer/SearchQueryAggregationCategorizer.java similarity index 88% rename from plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer/SearchQueryAggregationCategorizer.java rename to plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/categorizer/SearchQueryAggregationCategorizer.java index 876eec3813f5f..f252795aaca3e 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer/SearchQueryAggregationCategorizer.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/categorizer/SearchQueryAggregationCategorizer.java @@ -6,7 +6,7 @@ * compatible open source license. */ -package org.opensearch.plugin.insights.core.categorizer; +package org.opensearch.plugin.insights.core.service.categorizer; import org.opensearch.search.aggregations.AggregationBuilder; import org.opensearch.search.aggregations.PipelineAggregationBuilder; @@ -43,7 +43,7 @@ public void incrementSearchQueryAggregationCounters(Collection subAggregations = aggregationBuilder.getSubAggregations(); @@ -57,7 +57,7 @@ private void incrementCountersRecursively(AggregationBuilder aggregationBuilder) Collection pipelineAggregations = aggregationBuilder.getPipelineAggregations(); for (PipelineAggregationBuilder pipelineAggregation : pipelineAggregations) { String pipelineAggregationType = pipelineAggregation.getType(); - searchQueryCounters.aggCounter.add(1, Tags.create().addTag(TYPE_TAG, pipelineAggregationType)); + searchQueryCounters.incrementAggCounter(1, Tags.create().addTag(TYPE_TAG, pipelineAggregationType)); } } } diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer/SearchQueryCategorizer.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/categorizer/SearchQueryCategorizer.java similarity index 90% rename from plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer/SearchQueryCategorizer.java rename to plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/categorizer/SearchQueryCategorizer.java index c2a0713fa6c83..dbbfec69dc600 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer/SearchQueryCategorizer.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/categorizer/SearchQueryCategorizer.java @@ -6,7 +6,7 @@ * compatible open source license. */ -package org.opensearch.plugin.insights.core.categorizer; +package org.opensearch.plugin.insights.core.service.categorizer; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -34,7 +34,7 @@ public final class SearchQueryCategorizer { /** * Contains all the search query counters */ - public final SearchQueryCounters searchQueryCounters; + private final SearchQueryCounters searchQueryCounters; final SearchQueryAggregationCategorizer searchQueryAggregationCategorizer; @@ -72,10 +72,9 @@ public void categorize(SearchSourceBuilder source) { private void incrementQuerySortCounters(List> sorts) { if (sorts != null && sorts.size() > 0) { - for (ListIterator> it = sorts.listIterator(); it.hasNext();) { - SortBuilder sortBuilder = it.next(); + for (SortBuilder sortBuilder : sorts) { String sortOrder = sortBuilder.order().toString(); - searchQueryCounters.sortCounter.add(1, Tags.create().addTag("sort_order", sortOrder)); + searchQueryCounters.incrementSortCounter(1, Tags.create().addTag("sort_order", sortOrder)); } } } @@ -105,4 +104,7 @@ private void logQueryShape(QueryBuilder topLevelQueryBuilder) { log.trace("Query shape : {}", shapeVisitor.prettyPrintTree(" ")); } + public SearchQueryCounters getSearchQueryCounters() { + return this.searchQueryCounters; + } } diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer/SearchQueryCategorizingVisitor.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/categorizer/SearchQueryCategorizingVisitor.java similarity index 95% rename from plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer/SearchQueryCategorizingVisitor.java rename to plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/categorizer/SearchQueryCategorizingVisitor.java index ed6411145376d..d4762e6967881 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer/SearchQueryCategorizingVisitor.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/categorizer/SearchQueryCategorizingVisitor.java @@ -6,7 +6,7 @@ * compatible open source license. */ -package org.opensearch.plugin.insights.core.categorizer; +package org.opensearch.plugin.insights.core.service.categorizer; import org.apache.lucene.search.BooleanClause; import org.opensearch.index.query.QueryBuilder; diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer/SearchQueryCounters.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/categorizer/SearchQueryCounters.java similarity index 78% rename from plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer/SearchQueryCounters.java rename to plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/categorizer/SearchQueryCounters.java index 8c7c91f794f16..bcbbc442d6a04 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer/SearchQueryCounters.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/categorizer/SearchQueryCounters.java @@ -6,7 +6,7 @@ * compatible open source license. */ -package org.opensearch.plugin.insights.core.categorizer; +package org.opensearch.plugin.insights.core.service.categorizer; import org.opensearch.index.query.QueryBuilder; import org.opensearch.telemetry.metrics.Counter; @@ -27,20 +27,20 @@ public final class SearchQueryCounters { /** * Aggregation counter */ - public final Counter aggCounter; + private final Counter aggCounter; /** * Counter for all other query types (catch all) */ - public final Counter otherQueryCounter; + private final Counter otherQueryCounter; /** * Counter for sort */ - public final Counter sortCounter; + private final Counter sortCounter; private final Map, Counter> queryHandlers; /** * Counter name to Counter object map */ - public final ConcurrentHashMap nameToQueryTypeCounters; + private final ConcurrentHashMap nameToQueryTypeCounters; /** * Constructor @@ -80,6 +80,26 @@ public void incrementCounter(QueryBuilder queryBuilder, int level) { counter.add(1, Tags.create().addTag(LEVEL_TAG, level)); } + public void incrementAggCounter(double value, Tags tags) { + aggCounter.add(value, tags); + } + + public void incrementSortCounter(double value, Tags tags) { + sortCounter.add(value, tags); + } + + public Counter getAggCounter() { + return aggCounter; + } + + public Counter getSortCounter() { + return sortCounter; + } + + public Counter getCounterByQueryBuilderName(String queryBuilderName) { + return nameToQueryTypeCounters.get(queryBuilderName); + } + private Counter createQueryCounter(String counterName) { Counter counter = metricsRegistry.createCounter( "search.query.type." + counterName + ".count", diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer/package-info.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/categorizer/package-info.java similarity index 82% rename from plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer/package-info.java rename to plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/categorizer/package-info.java index a59f4490c40e0..3bcbc7d4e31ee 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/categorizer/package-info.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/categorizer/package-info.java @@ -9,4 +9,4 @@ /** * Query Insights search query categorizor to collect metrics related to search queries */ -package org.opensearch.plugin.insights.core.categorizer; +package org.opensearch.plugin.insights.core.service.categorizer; diff --git a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/categorizor/QueryShapeVisitorTests.java b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/service/categorizor/QueryShapeVisitorTests.java similarity index 92% rename from plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/categorizor/QueryShapeVisitorTests.java rename to plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/service/categorizor/QueryShapeVisitorTests.java index 2b6fd6f1daec1..bffe287ed8c25 100644 --- a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/categorizor/QueryShapeVisitorTests.java +++ b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/service/categorizor/QueryShapeVisitorTests.java @@ -6,7 +6,7 @@ * compatible open source license. */ -package org.opensearch.plugin.insights.core.categorizor; +package org.opensearch.plugin.insights.core.service.categorizor; import org.opensearch.index.query.BoolQueryBuilder; import org.opensearch.index.query.ConstantScoreQueryBuilder; @@ -16,7 +16,7 @@ import org.opensearch.index.query.RegexpQueryBuilder; import org.opensearch.index.query.TermQueryBuilder; import org.opensearch.index.query.TermsQueryBuilder; -import org.opensearch.plugin.insights.core.categorizer.QueryShapeVisitor; +import org.opensearch.plugin.insights.core.service.categorizer.QueryShapeVisitor; import org.opensearch.test.OpenSearchTestCase; public final class QueryShapeVisitorTests extends OpenSearchTestCase { diff --git a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/categorizor/SearchQueryCategorizerTests.java b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/service/categorizor/SearchQueryCategorizerTests.java similarity index 71% rename from plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/categorizor/SearchQueryCategorizerTests.java rename to plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/service/categorizor/SearchQueryCategorizerTests.java index e78fc7bd23f8d..6987ebee6b868 100644 --- a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/categorizor/SearchQueryCategorizerTests.java +++ b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/service/categorizor/SearchQueryCategorizerTests.java @@ -6,7 +6,7 @@ * compatible open source license. */ -package org.opensearch.plugin.insights.core.categorizor; +package org.opensearch.plugin.insights.core.service.categorizor; import org.opensearch.index.query.BoolQueryBuilder; import org.opensearch.index.query.BoostingQueryBuilder; @@ -20,7 +20,7 @@ import org.opensearch.index.query.TermQueryBuilder; import org.opensearch.index.query.WildcardQueryBuilder; import org.opensearch.index.query.functionscore.FunctionScoreQueryBuilder; -import org.opensearch.plugin.insights.core.categorizer.SearchQueryCategorizer; +import org.opensearch.plugin.insights.core.service.categorizer.SearchQueryCategorizer; import org.opensearch.search.aggregations.bucket.range.RangeAggregationBuilder; import org.opensearch.search.aggregations.bucket.terms.MultiTermsAggregationBuilder; import org.opensearch.search.aggregations.support.MultiTermsValuesSourceConfig; @@ -75,14 +75,12 @@ public void testAggregationsQuery() { searchQueryCategorizer.categorize(sourceBuilder); - verify(searchQueryCategorizer.searchQueryCounters.aggCounter).add(eq(1.0d), any(Tags.class)); + verify(searchQueryCategorizer.getSearchQueryCounters().getAggCounter()).add(eq(1.0d), any(Tags.class)); - // capture the arguments passed to the aggCounter.add method ArgumentCaptor valueCaptor = ArgumentCaptor.forClass(Double.class); ArgumentCaptor tagsCaptor = ArgumentCaptor.forClass(Tags.class); - // Verify that aggCounter.add was called with the expected arguments - verify(searchQueryCategorizer.searchQueryCounters.aggCounter).add(valueCaptor.capture(), tagsCaptor.capture()); + verify(searchQueryCategorizer.getSearchQueryCounters().getAggCounter()).add(valueCaptor.capture(), tagsCaptor.capture()); double actualValue = valueCaptor.getValue(); String actualTag = (String) tagsCaptor.getValue().getTagsMap().get("type"); @@ -98,8 +96,8 @@ public void testBoolQuery() { searchQueryCategorizer.categorize(sourceBuilder); - verify(searchQueryCategorizer.searchQueryCounters.nameToQueryTypeCounters.get("bool")).add(eq(1.0d), any(Tags.class)); - verify(searchQueryCategorizer.searchQueryCounters.nameToQueryTypeCounters.get("match")).add(eq(1.0d), any(Tags.class)); + verify(searchQueryCategorizer.getSearchQueryCounters().getCounterByQueryBuilderName("bool")).add(eq(1.0d), any(Tags.class)); + verify(searchQueryCategorizer.getSearchQueryCounters().getCounterByQueryBuilderName("match")).add(eq(1.0d), any(Tags.class)); } public void testFunctionScoreQuery() { @@ -109,7 +107,7 @@ public void testFunctionScoreQuery() { searchQueryCategorizer.categorize(sourceBuilder); - verify(searchQueryCategorizer.searchQueryCounters.nameToQueryTypeCounters.get("function_score")).add(eq(1.0d), any(Tags.class)); + verify(searchQueryCategorizer.getSearchQueryCounters().getCounterByQueryBuilderName("function_score")).add(eq(1.0d), any(Tags.class)); } public void testMatchQuery() { @@ -119,7 +117,7 @@ public void testMatchQuery() { searchQueryCategorizer.categorize(sourceBuilder); - verify(searchQueryCategorizer.searchQueryCounters.nameToQueryTypeCounters.get("match")).add(eq(1.0d), any(Tags.class)); + verify(searchQueryCategorizer.getSearchQueryCounters().getCounterByQueryBuilderName("match")).add(eq(1.0d), any(Tags.class)); } public void testMatchPhraseQuery() { @@ -129,7 +127,7 @@ public void testMatchPhraseQuery() { searchQueryCategorizer.categorize(sourceBuilder); - verify(searchQueryCategorizer.searchQueryCounters.nameToQueryTypeCounters.get("match_phrase")).add(eq(1.0d), any(Tags.class)); + verify(searchQueryCategorizer.getSearchQueryCounters().getCounterByQueryBuilderName("match_phrase")).add(eq(1.0d), any(Tags.class)); } public void testMultiMatchQuery() { @@ -139,7 +137,7 @@ public void testMultiMatchQuery() { searchQueryCategorizer.categorize(sourceBuilder); - verify(searchQueryCategorizer.searchQueryCounters.nameToQueryTypeCounters.get("multi_match")).add(eq(1.0d), any(Tags.class)); + verify(searchQueryCategorizer.getSearchQueryCounters().getCounterByQueryBuilderName("multi_match")).add(eq(1.0d), any(Tags.class)); } public void testOtherQuery() { @@ -153,9 +151,9 @@ public void testOtherQuery() { searchQueryCategorizer.categorize(sourceBuilder); - verify(searchQueryCategorizer.searchQueryCounters.nameToQueryTypeCounters.get("boosting")).add(eq(1.0d), any(Tags.class)); - verify(searchQueryCategorizer.searchQueryCounters.nameToQueryTypeCounters.get("match_none")).add(eq(1.0d), any(Tags.class)); - verify(searchQueryCategorizer.searchQueryCounters.nameToQueryTypeCounters.get("term")).add(eq(1.0d), any(Tags.class)); + verify(searchQueryCategorizer.getSearchQueryCounters().getCounterByQueryBuilderName("boosting")).add(eq(1.0d), any(Tags.class)); + verify(searchQueryCategorizer.getSearchQueryCounters().getCounterByQueryBuilderName("match_none")).add(eq(1.0d), any(Tags.class)); + verify(searchQueryCategorizer.getSearchQueryCounters().getCounterByQueryBuilderName("term")).add(eq(1.0d), any(Tags.class)); } public void testQueryStringQuery() { @@ -166,7 +164,7 @@ public void testQueryStringQuery() { searchQueryCategorizer.categorize(sourceBuilder); - verify(searchQueryCategorizer.searchQueryCounters.nameToQueryTypeCounters.get("query_string")).add(eq(1.0d), any(Tags.class)); + verify(searchQueryCategorizer.getSearchQueryCounters().getCounterByQueryBuilderName("query_string")).add(eq(1.0d), any(Tags.class)); } public void testRangeQuery() { @@ -178,7 +176,7 @@ public void testRangeQuery() { searchQueryCategorizer.categorize(sourceBuilder); - verify(searchQueryCategorizer.searchQueryCounters.nameToQueryTypeCounters.get("range")).add(eq(1.0d), any(Tags.class)); + verify(searchQueryCategorizer.getSearchQueryCounters().getCounterByQueryBuilderName("range")).add(eq(1.0d), any(Tags.class)); } public void testRegexQuery() { @@ -187,7 +185,7 @@ public void testRegexQuery() { searchQueryCategorizer.categorize(sourceBuilder); - verify(searchQueryCategorizer.searchQueryCounters.nameToQueryTypeCounters.get("regexp")).add(eq(1.0d), any(Tags.class)); + verify(searchQueryCategorizer.getSearchQueryCounters().getCounterByQueryBuilderName("regexp")).add(eq(1.0d), any(Tags.class)); } public void testSortQuery() { @@ -198,8 +196,8 @@ public void testSortQuery() { searchQueryCategorizer.categorize(sourceBuilder); - verify(searchQueryCategorizer.searchQueryCounters.nameToQueryTypeCounters.get("match")).add(eq(1.0d), any(Tags.class)); - verify(searchQueryCategorizer.searchQueryCounters.sortCounter, times(2)).add(eq(1.0d), any(Tags.class)); + verify(searchQueryCategorizer.getSearchQueryCounters().getCounterByQueryBuilderName("match")).add(eq(1.0d), any(Tags.class)); + verify(searchQueryCategorizer.getSearchQueryCounters().getSortCounter(), times(2)).add(eq(1.0d), any(Tags.class)); } public void testTermQuery() { @@ -209,7 +207,7 @@ public void testTermQuery() { searchQueryCategorizer.categorize(sourceBuilder); - verify(searchQueryCategorizer.searchQueryCounters.nameToQueryTypeCounters.get("term")).add(eq(1.0d), any(Tags.class)); + verify(searchQueryCategorizer.getSearchQueryCounters().getCounterByQueryBuilderName("term")).add(eq(1.0d), any(Tags.class)); } public void testWildcardQuery() { @@ -219,7 +217,7 @@ public void testWildcardQuery() { searchQueryCategorizer.categorize(sourceBuilder); - verify(searchQueryCategorizer.searchQueryCounters.nameToQueryTypeCounters.get("wildcard")).add(eq(1.0d), any(Tags.class)); + verify(searchQueryCategorizer.getSearchQueryCounters().getCounterByQueryBuilderName("wildcard")).add(eq(1.0d), any(Tags.class)); } public void testComplexQuery() { @@ -237,10 +235,10 @@ public void testComplexQuery() { searchQueryCategorizer.categorize(sourceBuilder); - verify(searchQueryCategorizer.searchQueryCounters.nameToQueryTypeCounters.get("term")).add(eq(1.0d), any(Tags.class)); - verify(searchQueryCategorizer.searchQueryCounters.nameToQueryTypeCounters.get("match")).add(eq(1.0d), any(Tags.class)); - verify(searchQueryCategorizer.searchQueryCounters.nameToQueryTypeCounters.get("regexp")).add(eq(1.0d), any(Tags.class)); - verify(searchQueryCategorizer.searchQueryCounters.nameToQueryTypeCounters.get("bool")).add(eq(1.0d), any(Tags.class)); - verify(searchQueryCategorizer.searchQueryCounters.aggCounter).add(eq(1.0d), any(Tags.class)); + verify(searchQueryCategorizer.getSearchQueryCounters().getCounterByQueryBuilderName("term")).add(eq(1.0d), any(Tags.class)); + verify(searchQueryCategorizer.getSearchQueryCounters().getCounterByQueryBuilderName("match")).add(eq(1.0d), any(Tags.class)); + verify(searchQueryCategorizer.getSearchQueryCounters().getCounterByQueryBuilderName("regexp")).add(eq(1.0d), any(Tags.class)); + verify(searchQueryCategorizer.getSearchQueryCounters().getCounterByQueryBuilderName("bool")).add(eq(1.0d), any(Tags.class)); + verify(searchQueryCategorizer.getSearchQueryCounters().getAggCounter()).add(eq(1.0d), any(Tags.class)); } } From 32ab63aee3fc842165e4c81afe7fcbfc9937639b Mon Sep 17 00:00:00 2001 From: Siddhant Deshmukh Date: Fri, 28 Jun 2024 17:01:21 -0700 Subject: [PATCH 14/22] Add categorization to the enabled check for insights service Signed-off-by: Siddhant Deshmukh --- .../plugin/insights/core/service/QueryInsightsService.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/QueryInsightsService.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/QueryInsightsService.java index f0f504447cf82..8dde7ec9588ee 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/QueryInsightsService.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/QueryInsightsService.java @@ -209,7 +209,7 @@ public boolean isCollectionEnabled(final MetricType metricType) { } /** - * Check if query insights service is enabled + * Check if query insights service is enabled including TopN and categorization. * * @return if query insights service is enabled */ @@ -219,7 +219,7 @@ public boolean isEnabled() { return true; } } - return false; + return searchQueryMetricsEnabled; } /** From dc1fefbd0d217e934106729cf0987797c6ded920 Mon Sep 17 00:00:00 2001 From: Siddhant Deshmukh Date: Fri, 28 Jun 2024 19:09:45 -0700 Subject: [PATCH 15/22] Spotless apply Signed-off-by: Siddhant Deshmukh --- .../plugin/insights/core/service/QueryInsightsService.java | 2 +- .../core/service/categorizer/SearchQueryCategorizer.java | 1 - .../service/categorizor/SearchQueryCategorizerTests.java | 5 ++++- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/QueryInsightsService.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/QueryInsightsService.java index 8dde7ec9588ee..da30d37e557b7 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/QueryInsightsService.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/QueryInsightsService.java @@ -16,8 +16,8 @@ import org.opensearch.common.settings.ClusterSettings; import org.opensearch.common.settings.Settings; import org.opensearch.common.unit.TimeValue; -import org.opensearch.plugin.insights.core.service.categorizer.SearchQueryCategorizer; import org.opensearch.plugin.insights.core.exporter.QueryInsightsExporterFactory; +import org.opensearch.plugin.insights.core.service.categorizer.SearchQueryCategorizer; import org.opensearch.plugin.insights.rules.model.MetricType; import org.opensearch.plugin.insights.rules.model.SearchQueryRecord; import org.opensearch.plugin.insights.settings.QueryInsightsSettings; diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/categorizer/SearchQueryCategorizer.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/categorizer/SearchQueryCategorizer.java index dbbfec69dc600..c7d64e4dff7d5 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/categorizer/SearchQueryCategorizer.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/categorizer/SearchQueryCategorizer.java @@ -21,7 +21,6 @@ import org.opensearch.telemetry.metrics.tags.Tags; import java.util.List; -import java.util.ListIterator; /** * Class to categorize the search queries based on the type and increment the relevant counters. diff --git a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/service/categorizor/SearchQueryCategorizerTests.java b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/service/categorizor/SearchQueryCategorizerTests.java index 6987ebee6b868..3c2618fa7eb94 100644 --- a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/service/categorizor/SearchQueryCategorizerTests.java +++ b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/service/categorizor/SearchQueryCategorizerTests.java @@ -107,7 +107,10 @@ public void testFunctionScoreQuery() { searchQueryCategorizer.categorize(sourceBuilder); - verify(searchQueryCategorizer.getSearchQueryCounters().getCounterByQueryBuilderName("function_score")).add(eq(1.0d), any(Tags.class)); + verify(searchQueryCategorizer.getSearchQueryCounters().getCounterByQueryBuilderName("function_score")).add( + eq(1.0d), + any(Tags.class) + ); } public void testMatchQuery() { From 0906513b3430eb67c1ec61681d141dd1f043503a Mon Sep 17 00:00:00 2001 From: Siddhant Deshmukh Date: Mon, 1 Jul 2024 10:26:40 -0700 Subject: [PATCH 16/22] Fix settings test Signed-off-by: Siddhant Deshmukh --- .../opensearch/plugin/insights/QueryInsightsPluginTests.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) 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 382ea3c5fa535..570b9fc0a33c3 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 @@ -18,6 +18,7 @@ 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.QueryCategorizationSettings; import org.opensearch.plugin.insights.settings.QueryInsightsSettings; import org.opensearch.plugins.ActionPlugin; import org.opensearch.rest.RestHandler; @@ -65,7 +66,8 @@ public void testGetSettings() { QueryInsightsSettings.TOP_N_MEMORY_QUERIES_ENABLED, QueryInsightsSettings.TOP_N_MEMORY_QUERIES_SIZE, QueryInsightsSettings.TOP_N_MEMORY_QUERIES_WINDOW_SIZE, - QueryInsightsSettings.TOP_N_MEMORY_EXPORTER_SETTINGS + QueryInsightsSettings.TOP_N_MEMORY_EXPORTER_SETTINGS, + QueryCategorizationSettings.SEARCH_QUERY_METRICS_ENABLED_SETTING ), queryInsightsPlugin.getSettings() ); From be6527b95c96fafe672b1840d266296d84b2388e Mon Sep 17 00:00:00 2001 From: Siddhant Deshmukh Date: Mon, 1 Jul 2024 10:49:44 -0700 Subject: [PATCH 17/22] Javadoc Signed-off-by: Siddhant Deshmukh --- .../categorizer/SearchQueryCategorizer.java | 4 ++++ .../categorizer/SearchQueryCounters.java | 23 +++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/categorizer/SearchQueryCategorizer.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/categorizer/SearchQueryCategorizer.java index c7d64e4dff7d5..a2776f3acffeb 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/categorizer/SearchQueryCategorizer.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/categorizer/SearchQueryCategorizer.java @@ -103,6 +103,10 @@ private void logQueryShape(QueryBuilder topLevelQueryBuilder) { log.trace("Query shape : {}", shapeVisitor.prettyPrintTree(" ")); } + /** + * Get search query counters + * @return + */ public SearchQueryCounters getSearchQueryCounters() { return this.searchQueryCounters; } diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/categorizer/SearchQueryCounters.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/categorizer/SearchQueryCounters.java index bcbbc442d6a04..640491b9d30a5 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/categorizer/SearchQueryCounters.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/categorizer/SearchQueryCounters.java @@ -80,22 +80,45 @@ public void incrementCounter(QueryBuilder queryBuilder, int level) { counter.add(1, Tags.create().addTag(LEVEL_TAG, level)); } + /** + * Increment aggregate counter + * @param value value to increment + * @param tags tags + */ public void incrementAggCounter(double value, Tags tags) { aggCounter.add(value, tags); } + /** + * Increment sort counter + * @param value value to increment + * @param tags tags + */ public void incrementSortCounter(double value, Tags tags) { sortCounter.add(value, tags); } + /** + * Get aggregation counter + * @return + */ public Counter getAggCounter() { return aggCounter; } + /** + * Get sort counter + * @return + */ public Counter getSortCounter() { return sortCounter; } + /** + * Get counter based on the query builder name + * @param queryBuilderName query builder name + * @return counter + */ public Counter getCounterByQueryBuilderName(String queryBuilderName) { return nameToQueryTypeCounters.get(queryBuilderName); } From eb8ebd85c7e05733e69f145c9f85228d45bc46f9 Mon Sep 17 00:00:00 2001 From: Siddhant Deshmukh Date: Mon, 1 Jul 2024 11:09:56 -0700 Subject: [PATCH 18/22] Java doc Signed-off-by: Siddhant Deshmukh --- .../core/service/categorizer/SearchQueryCategorizer.java | 2 +- .../core/service/categorizer/SearchQueryCounters.java | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/categorizer/SearchQueryCategorizer.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/categorizer/SearchQueryCategorizer.java index a2776f3acffeb..834c8ed117ffa 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/categorizer/SearchQueryCategorizer.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/categorizer/SearchQueryCategorizer.java @@ -105,7 +105,7 @@ private void logQueryShape(QueryBuilder topLevelQueryBuilder) { /** * Get search query counters - * @return + * @return search query counters */ public SearchQueryCounters getSearchQueryCounters() { return this.searchQueryCounters; diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/categorizer/SearchQueryCounters.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/categorizer/SearchQueryCounters.java index 640491b9d30a5..7a272b613e703 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/categorizer/SearchQueryCounters.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/categorizer/SearchQueryCounters.java @@ -100,7 +100,7 @@ public void incrementSortCounter(double value, Tags tags) { /** * Get aggregation counter - * @return + * @return aggregation counter */ public Counter getAggCounter() { return aggCounter; @@ -108,7 +108,7 @@ public Counter getAggCounter() { /** * Get sort counter - * @return + * @return sort counter */ public Counter getSortCounter() { return sortCounter; From d419ca5d20c50e199cb4da893471350024924198 Mon Sep 17 00:00:00 2001 From: Siddhant Deshmukh Date: Tue, 2 Jul 2024 12:56:50 -0700 Subject: [PATCH 19/22] Add unit test and change serialization Signed-off-by: Siddhant Deshmukh --- .../core/service/QueryInsightsService.java | 27 +++++++++++++------ .../rules/model/SearchQueryRecord.java | 6 +---- .../service/QueryInsightsServiceTests.java | 21 +++++++++++++++ 3 files changed, 41 insertions(+), 13 deletions(-) diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/QueryInsightsService.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/QueryInsightsService.java index da30d37e557b7..b0afd66968b1f 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/QueryInsightsService.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/QueryInsightsService.java @@ -122,13 +122,6 @@ public QueryInsightsService( this.metricsRegistry = metricsRegistry; } - private void setSearchQueryMetricsEnabled(boolean searchQueryMetricsEnabled) { - this.searchQueryMetricsEnabled = searchQueryMetricsEnabled; - if ((this.searchQueryMetricsEnabled == true) && this.searchQueryCategorizer == null) { - this.searchQueryCategorizer = new SearchQueryCategorizer(metricsRegistry); - } - } - /** * Ingest the query data into in-memory stores * @@ -175,7 +168,6 @@ public void drainRecords() { logger.error("Error while trying to categorize the queries.", e); } } - } /** @@ -282,6 +274,25 @@ public void setExporter(final MetricType type, final Settings settings) { } } + /** + * Set search query metrics enabled to enable collection of search query categorization metrics + * @param searchQueryMetricsEnabled boolean flag + */ + public void setSearchQueryMetricsEnabled(boolean searchQueryMetricsEnabled) { + this.searchQueryMetricsEnabled = searchQueryMetricsEnabled; + if ((this.searchQueryMetricsEnabled == true) && this.searchQueryCategorizer == null) { + this.searchQueryCategorizer = new SearchQueryCategorizer(metricsRegistry); + } + } + + public boolean isSearchQueryMetricsEnabled() { + return this.searchQueryMetricsEnabled; + } + + public SearchQueryCategorizer getSearchQueryCategorizer() { + return this.searchQueryCategorizer; + } + /** * Validate the exporter config for a metricType * diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model/SearchQueryRecord.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model/SearchQueryRecord.java index a6e6b4a9051f0..1d8d931ad9866 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model/SearchQueryRecord.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model/SearchQueryRecord.java @@ -134,11 +134,7 @@ public XContentBuilder toXContent(final XContentBuilder builder, final ToXConten public void writeTo(final StreamOutput out) throws IOException { out.writeLong(timestamp); out.writeMap(measurements, (stream, metricType) -> MetricType.writeTo(out, metricType), StreamOutput::writeGenericValue); - out.writeMap( - attributes, - (stream, attribute) -> Attribute.writeTo(out, attribute), - (stream, attributeValue) -> Attribute.writeValueTo(out, attributeValue) - ); + out.writeMap(attributes, (stream, attribute) -> Attribute.writeTo(out, attribute), StreamOutput::writeGenericValue); } /** diff --git a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/service/QueryInsightsServiceTests.java b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/service/QueryInsightsServiceTests.java index 1c81d516a0f97..68be62148e4ce 100644 --- a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/service/QueryInsightsServiceTests.java +++ b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/service/QueryInsightsServiceTests.java @@ -63,4 +63,25 @@ public void testClose() { fail("No exception expected when closing query insights service"); } } + + public void testSearchQueryMetricsEnabled() { + // Initially, searchQueryMetricsEnabled should be false and searchQueryCategorizer should be null + assertFalse(queryInsightsService.isSearchQueryMetricsEnabled()); + assertNull(queryInsightsService.getSearchQueryCategorizer()); + + // Enable search query metrics + queryInsightsService.setSearchQueryMetricsEnabled(true); + + // Assert that searchQueryMetricsEnabled is true and searchQueryCategorizer is initialized + assertTrue(queryInsightsService.isSearchQueryMetricsEnabled()); + assertNotNull(queryInsightsService.getSearchQueryCategorizer()); + + // Disable search query metrics + queryInsightsService.setSearchQueryMetricsEnabled(false); + + // Assert that searchQueryMetricsEnabled is false and searchQueryCategorizer is not null + assertFalse(queryInsightsService.isSearchQueryMetricsEnabled()); + assertNotNull(queryInsightsService.getSearchQueryCategorizer()); + + } } From d6b29c7981a28478b6360e0e1bbbe1bc74735ce0 Mon Sep 17 00:00:00 2001 From: Siddhant Deshmukh Date: Tue, 2 Jul 2024 13:11:13 -0700 Subject: [PATCH 20/22] Javadoc Signed-off-by: Siddhant Deshmukh --- .../core/listener/QueryInsightsListener.java | 9 ++++++--- .../core/service/QueryInsightsService.java | 16 ++++++++++++---- 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/listener/QueryInsightsListener.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/listener/QueryInsightsListener.java index a1b43a171c6a2..7d9f9d4d046ba 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/listener/QueryInsightsListener.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/listener/QueryInsightsListener.java @@ -96,11 +96,14 @@ public QueryInsightsListener(final ClusterService clusterService, final QueryIns * @param enabled boolean */ public void setEnableTopQueries(final MetricType metricType, final boolean enabled) { - boolean isAllMetricsDisabled = !queryInsightsService.isEnabled(); + boolean isAllMetricsDisabled = !queryInsightsService.isTopNEnabledForAnyMetric(); + boolean isQueryMetricsDisabled = !queryInsightsService.isSearchQueryMetricsEnabled(); this.queryInsightsService.enableCollection(metricType, enabled); + if (!enabled) { - // disable QueryInsightsListener only if all metrics collections are disabled now. - if (!queryInsightsService.isEnabled()) { + // disable QueryInsightsListener only if all metrics collections are disabled now + // and search query metrics is disabled. + if (!queryInsightsService.isTopNEnabledForAnyMetric() && isQueryMetricsDisabled) { super.setEnabled(false); this.queryInsightsService.stop(); } diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/QueryInsightsService.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/QueryInsightsService.java index b0afd66968b1f..cd8ac1589b911 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/QueryInsightsService.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/QueryInsightsService.java @@ -201,17 +201,17 @@ public boolean isCollectionEnabled(final MetricType metricType) { } /** - * Check if query insights service is enabled including TopN and categorization. + * Check if Top N feature is enabled for query insights service for any metric type * * @return if query insights service is enabled */ - public boolean isEnabled() { + public boolean isTopNEnabledForAnyMetric() { for (MetricType t : MetricType.allMetricTypes()) { if (isCollectionEnabled(t)) { return true; } } - return searchQueryMetricsEnabled; + return false; } /** @@ -285,10 +285,18 @@ public void setSearchQueryMetricsEnabled(boolean searchQueryMetricsEnabled) { } } + /** + * Is search query metrics feature enabled. + * @return boolean flag + */ public boolean isSearchQueryMetricsEnabled() { return this.searchQueryMetricsEnabled; } + /** + * Get search query categorizer object + * @return SearchQueryCategorizer object + */ public SearchQueryCategorizer getSearchQueryCategorizer() { return this.searchQueryCategorizer; } @@ -307,7 +315,7 @@ public void validateExporterConfig(final MetricType type, final Settings setting @Override protected void doStart() { - if (isEnabled()) { + if (isTopNEnabledForAnyMetric()) { scheduledFuture = threadPool.scheduleWithFixedDelay( this::drainRecords, QueryInsightsSettings.QUERY_RECORD_QUEUE_DRAIN_INTERVAL, From ead3ae2d2494ee99839647e3eb8d399feda460db Mon Sep 17 00:00:00 2001 From: Siddhant Deshmukh Date: Wed, 3 Jul 2024 16:38:19 -0700 Subject: [PATCH 21/22] Fix entrypoint check Signed-off-by: Siddhant Deshmukh --- .../plugin/insights/core/listener/QueryInsightsListener.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/listener/QueryInsightsListener.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/listener/QueryInsightsListener.java index 7d9f9d4d046ba..bbb4a9b40fb8b 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/listener/QueryInsightsListener.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/listener/QueryInsightsListener.java @@ -109,8 +109,9 @@ public void setEnableTopQueries(final MetricType metricType, final boolean enabl } } else { super.setEnabled(true); - // restart QueryInsightsListener only if none of metrics collections is enabled before. - if (isAllMetricsDisabled) { + // restart QueryInsightsListener only if none of metrics collections is enabled before and + // search query metrics is disabled before. + if (isAllMetricsDisabled && isQueryMetricsDisabled) { this.queryInsightsService.stop(); this.queryInsightsService.start(); } From 9b80e9055dda29d3aa0ca7f83fffa9ea21e03fe4 Mon Sep 17 00:00:00 2001 From: Siddhant Deshmukh Date: Wed, 3 Jul 2024 17:46:11 -0700 Subject: [PATCH 22/22] Address review comments Signed-off-by: Siddhant Deshmukh --- .../core/listener/QueryInsightsListener.java | 8 ++--- .../core/service/QueryInsightsService.java | 16 +++------- .../categorizer/SearchQueryCategorizer.java | 31 +++++++++++++++---- .../rules/model/SearchQueryRecord.java | 2 +- .../SearchQueryCategorizerTests.java | 2 +- 5 files changed, 35 insertions(+), 24 deletions(-) diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/listener/QueryInsightsListener.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/listener/QueryInsightsListener.java index bbb4a9b40fb8b..68aab6b084b83 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/listener/QueryInsightsListener.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/listener/QueryInsightsListener.java @@ -96,14 +96,12 @@ public QueryInsightsListener(final ClusterService clusterService, final QueryIns * @param enabled boolean */ public void setEnableTopQueries(final MetricType metricType, final boolean enabled) { - boolean isAllMetricsDisabled = !queryInsightsService.isTopNEnabledForAnyMetric(); - boolean isQueryMetricsDisabled = !queryInsightsService.isSearchQueryMetricsEnabled(); - this.queryInsightsService.enableCollection(metricType, enabled); + boolean isInsightsServiceDisabled = !queryInsightsService.isEnabled(); if (!enabled) { // disable QueryInsightsListener only if all metrics collections are disabled now // and search query metrics is disabled. - if (!queryInsightsService.isTopNEnabledForAnyMetric() && isQueryMetricsDisabled) { + if (isInsightsServiceDisabled) { super.setEnabled(false); this.queryInsightsService.stop(); } @@ -111,7 +109,7 @@ public void setEnableTopQueries(final MetricType metricType, final boolean enabl super.setEnabled(true); // restart QueryInsightsListener only if none of metrics collections is enabled before and // search query metrics is disabled before. - if (isAllMetricsDisabled && isQueryMetricsDisabled) { + if (isInsightsServiceDisabled) { this.queryInsightsService.stop(); this.queryInsightsService.start(); } diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/QueryInsightsService.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/QueryInsightsService.java index cd8ac1589b911..5c7357ef887ab 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/QueryInsightsService.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/QueryInsightsService.java @@ -81,8 +81,6 @@ public class QueryInsightsService extends AbstractLifecycleComponent { private SearchQueryCategorizer searchQueryCategorizer; - private MetricsRegistry metricsRegistry; - /** * Constructor of the QueryInsightsService * @@ -117,9 +115,8 @@ public QueryInsightsService( } this.searchQueryMetricsEnabled = clusterSettings.get(SEARCH_QUERY_METRICS_ENABLED_SETTING); + this.searchQueryCategorizer = SearchQueryCategorizer.getInstance(metricsRegistry); clusterSettings.addSettingsUpdateConsumer(SEARCH_QUERY_METRICS_ENABLED_SETTING, this::setSearchQueryMetricsEnabled); - - this.metricsRegistry = metricsRegistry; } /** @@ -201,17 +198,17 @@ public boolean isCollectionEnabled(final MetricType metricType) { } /** - * Check if Top N feature is enabled for query insights service for any metric type + * Check if any feature of Query Insights service is enabled, right now includes Top N and Categorization. * * @return if query insights service is enabled */ - public boolean isTopNEnabledForAnyMetric() { + public boolean isEnabled() { for (MetricType t : MetricType.allMetricTypes()) { if (isCollectionEnabled(t)) { return true; } } - return false; + return this.searchQueryMetricsEnabled; } /** @@ -280,9 +277,6 @@ public void setExporter(final MetricType type, final Settings settings) { */ public void setSearchQueryMetricsEnabled(boolean searchQueryMetricsEnabled) { this.searchQueryMetricsEnabled = searchQueryMetricsEnabled; - if ((this.searchQueryMetricsEnabled == true) && this.searchQueryCategorizer == null) { - this.searchQueryCategorizer = new SearchQueryCategorizer(metricsRegistry); - } } /** @@ -315,7 +309,7 @@ public void validateExporterConfig(final MetricType type, final Settings setting @Override protected void doStart() { - if (isTopNEnabledForAnyMetric()) { + if (isEnabled()) { scheduledFuture = threadPool.scheduleWithFixedDelay( this::drainRecords, QueryInsightsSettings.QUERY_RECORD_QUEUE_DRAIN_INTERVAL, diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/categorizer/SearchQueryCategorizer.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/categorizer/SearchQueryCategorizer.java index 834c8ed117ffa..851912a6ffea3 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/categorizer/SearchQueryCategorizer.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/categorizer/SearchQueryCategorizer.java @@ -36,16 +36,33 @@ public final class SearchQueryCategorizer { private final SearchQueryCounters searchQueryCounters; final SearchQueryAggregationCategorizer searchQueryAggregationCategorizer; + private static SearchQueryCategorizer instance; /** * Constructor for SearchQueryCategorizor * @param metricsRegistry opentelemetry metrics registry */ - public SearchQueryCategorizer(MetricsRegistry metricsRegistry) { + private SearchQueryCategorizer(MetricsRegistry metricsRegistry) { searchQueryCounters = new SearchQueryCounters(metricsRegistry); searchQueryAggregationCategorizer = new SearchQueryAggregationCategorizer(searchQueryCounters); } + /** + * Get singleton instance of SearchQueryCategorizer + * @param metricsRegistry metric registry + * @return singleton instance + */ + public static SearchQueryCategorizer getInstance(MetricsRegistry metricsRegistry) { + if (instance == null) { + synchronized (SearchQueryCategorizer.class) { + if (instance == null) { + instance = new SearchQueryCategorizer(metricsRegistry); + } + } + } + return instance; + } + /** * Consume records and increment counters for the records * @param records records to consume @@ -95,12 +112,14 @@ private void incrementQueryTypeCounters(QueryBuilder topLevelQueryBuilder) { } private void logQueryShape(QueryBuilder topLevelQueryBuilder) { - if (topLevelQueryBuilder == null) { - return; + if (log.isTraceEnabled()) { + if (topLevelQueryBuilder == null) { + return; + } + QueryShapeVisitor shapeVisitor = new QueryShapeVisitor(); + topLevelQueryBuilder.visit(shapeVisitor); + log.trace("Query shape : {}", shapeVisitor.prettyPrintTree(" ")); } - QueryShapeVisitor shapeVisitor = new QueryShapeVisitor(); - topLevelQueryBuilder.visit(shapeVisitor); - log.trace("Query shape : {}", shapeVisitor.prettyPrintTree(" ")); } /** diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model/SearchQueryRecord.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model/SearchQueryRecord.java index 1d8d931ad9866..fec00a680ae58 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model/SearchQueryRecord.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model/SearchQueryRecord.java @@ -45,7 +45,7 @@ public SearchQueryRecord(final StreamInput in) throws IOException, ClassCastExce measurements = new HashMap<>(); in.readMap(MetricType::readFromStream, StreamInput::readGenericValue) .forEach(((metricType, o) -> measurements.put(metricType, metricType.parseValue(o)))); - this.attributes = Attribute.readAttributeMap(in); + this.attributes = in.readMap(Attribute::readFromStream, StreamInput::readGenericValue); } /** diff --git a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/service/categorizor/SearchQueryCategorizerTests.java b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/service/categorizor/SearchQueryCategorizerTests.java index 3c2618fa7eb94..e511ef3b54d81 100644 --- a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/service/categorizor/SearchQueryCategorizerTests.java +++ b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/service/categorizor/SearchQueryCategorizerTests.java @@ -58,7 +58,7 @@ public void setup() { when(metricsRegistry.createCounter(any(String.class), any(String.class), any(String.class))).thenAnswer( invocation -> mock(Counter.class) ); - searchQueryCategorizer = new SearchQueryCategorizer(metricsRegistry); + searchQueryCategorizer = SearchQueryCategorizer.getInstance(metricsRegistry); } public void testAggregationsQuery() {