From 79d045851d57f9a997cbaf1f6fb19a20490a60c0 Mon Sep 17 00:00:00 2001 From: Siddhant Deshmukh Date: Wed, 13 Mar 2024 15:19:59 -0700 Subject: [PATCH 1/3] Make search query counters dynamic to support all query types (#12601) (cherry picked from commit abe270f3b54c9e89d9375067509b9696a43d699a) Signed-off-by: Siddhant Deshmukh --- CHANGELOG.md | 1 + .../action/search/SearchQueryCounters.java | 95 +++++-------------- .../search/SearchQueryCategorizerTests.java | 41 ++++---- 3 files changed, 44 insertions(+), 93 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 74839b817383c..ab03e10442e69 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Added cluster setting cluster.restrict.index.replication_type to restrict setting of index setting replication type ([#10866](https://github.com/opensearch-project/OpenSearch/pull/10866)) - Add cluster state stats ([#10670](https://github.com/opensearch-project/OpenSearch/pull/10670)) - [Remote cluster state] Restore cluster state version during remote state auto restore ([#10853](https://github.com/opensearch-project/OpenSearch/pull/10853)) +- Make search query counters dynamic to support all query types ([#12601](https://github.com/opensearch-project/OpenSearch/pull/12601)) ### Dependencies - Bumps jetty version to 9.4.52.v20230823 to fix GMS-2023-1857 ([#9822](https://github.com/opensearch-project/OpenSearch/pull/9822)) diff --git a/server/src/main/java/org/opensearch/action/search/SearchQueryCounters.java b/server/src/main/java/org/opensearch/action/search/SearchQueryCounters.java index 7e0259af07701..b564bd3c0fdfa 100644 --- a/server/src/main/java/org/opensearch/action/search/SearchQueryCounters.java +++ b/server/src/main/java/org/opensearch/action/search/SearchQueryCounters.java @@ -8,8 +8,14 @@ package org.opensearch.action.search; +import org.opensearch.index.query.QueryBuilder; import org.opensearch.telemetry.metrics.Counter; import org.opensearch.telemetry.metrics.MetricsRegistry; +import org.opensearch.telemetry.metrics.tags.Tags; + +import java.util.HashMap; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; /** * Class contains all the Counters related to search query types. @@ -17,101 +23,44 @@ final class SearchQueryCounters { private static final String UNIT = "1"; private final MetricsRegistry metricsRegistry; - - // Counters related to Query types public final Counter aggCounter; - public final Counter boolCounter; - public final Counter functionScoreCounter; - public final Counter matchCounter; - public final Counter matchPhrasePrefixCounter; - public final Counter multiMatchCounter; public final Counter otherQueryCounter; - public final Counter queryStringQueryCounter; - public final Counter rangeCounter; - public final Counter regexCounter; - public final Counter sortCounter; - public final Counter skippedCounter; - public final Counter termCounter; - public final Counter totalCounter; - public final Counter wildcardCounter; + public final ConcurrentHashMap nameToQueryTypeCounters; public SearchQueryCounters(MetricsRegistry metricsRegistry) { this.metricsRegistry = metricsRegistry; + this.nameToQueryTypeCounters = new ConcurrentHashMap<>(); this.aggCounter = metricsRegistry.createCounter( "search.query.type.agg.count", "Counter for the number of top level agg search queries", UNIT ); - this.boolCounter = metricsRegistry.createCounter( - "search.query.type.bool.count", - "Counter for the number of top level and nested bool search queries", - UNIT - ); - this.functionScoreCounter = metricsRegistry.createCounter( - "search.query.type.functionscore.count", - "Counter for the number of top level and nested function score search queries", - UNIT - ); - this.matchCounter = metricsRegistry.createCounter( - "search.query.type.match.count", - "Counter for the number of top level and nested match search queries", - UNIT - ); - this.matchPhrasePrefixCounter = metricsRegistry.createCounter( - "search.query.type.matchphrase.count", - "Counter for the number of top level and nested match phrase prefix search queries", - UNIT - ); - this.multiMatchCounter = metricsRegistry.createCounter( - "search.query.type.multimatch.count", - "Counter for the number of top level and nested multi match search queries", - UNIT - ); this.otherQueryCounter = metricsRegistry.createCounter( "search.query.type.other.count", "Counter for the number of top level and nested search queries that do not match any other categories", UNIT ); - this.queryStringQueryCounter = metricsRegistry.createCounter( - "search.query.type.querystringquery.count", - "Counter for the number of top level and nested queryStringQuery search queries", - UNIT - ); - this.rangeCounter = metricsRegistry.createCounter( - "search.query.type.range.count", - "Counter for the number of top level and nested range search queries", - UNIT - ); - this.regexCounter = metricsRegistry.createCounter( - "search.query.type.regex.count", - "Counter for the number of top level and nested regex search queries", - UNIT - ); - this.skippedCounter = metricsRegistry.createCounter( - "search.query.type.skipped.count", - "Counter for the number queries skipped due to error", - UNIT - ); this.sortCounter = metricsRegistry.createCounter( "search.query.type.sort.count", "Counter for the number of top level sort search queries", UNIT ); - this.termCounter = metricsRegistry.createCounter( - "search.query.type.term.count", - "Counter for the number of top level and nested term search queries", - UNIT - ); - this.totalCounter = metricsRegistry.createCounter( - "search.query.type.total.count", - "Counter for the number of top level and nested search queries", - UNIT - ); - this.wildcardCounter = metricsRegistry.createCounter( - "search.query.type.wildcard.count", - "Counter for the number of top level and nested wildcard search queries", + } + + public void incrementCounter(QueryBuilder queryBuilder, int level) { + String uniqueQueryCounterName = queryBuilder.getName(); + + Counter counter = nameToQueryTypeCounters.computeIfAbsent(uniqueQueryCounterName, k -> createQueryCounter(k)); + counter.add(1, Tags.create().addTag(LEVEL_TAG, level)); + } + + private Counter createQueryCounter(String counterName) { + Counter counter = metricsRegistry.createCounter( + "search.query.type." + counterName + ".count", + "Counter for the number of top level and nested " + counterName + " search queries", UNIT ); + return counter; } } diff --git a/server/src/test/java/org/opensearch/action/search/SearchQueryCategorizerTests.java b/server/src/test/java/org/opensearch/action/search/SearchQueryCategorizerTests.java index a2e301143d694..6636af1531999 100644 --- a/server/src/test/java/org/opensearch/action/search/SearchQueryCategorizerTests.java +++ b/server/src/test/java/org/opensearch/action/search/SearchQueryCategorizerTests.java @@ -81,8 +81,8 @@ public void testBoolQuery() { searchQueryCategorizer.categorize(sourceBuilder); - Mockito.verify(searchQueryCategorizer.searchQueryCounters.boolCounter).add(eq(1.0d), any(Tags.class)); - Mockito.verify(searchQueryCategorizer.searchQueryCounters.matchCounter).add(eq(1.0d), any(Tags.class)); + 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)); } public void testFunctionScoreQuery() { @@ -92,7 +92,7 @@ public void testFunctionScoreQuery() { searchQueryCategorizer.categorize(sourceBuilder); - Mockito.verify(searchQueryCategorizer.searchQueryCounters.functionScoreCounter).add(eq(1.0d), any(Tags.class)); + verify(searchQueryCategorizer.searchQueryCounters.nameToQueryTypeCounters.get("function_score")).add(eq(1.0d), any(Tags.class)); } public void testMatchQuery() { @@ -102,7 +102,7 @@ public void testMatchQuery() { searchQueryCategorizer.categorize(sourceBuilder); - Mockito.verify(searchQueryCategorizer.searchQueryCounters.matchCounter).add(eq(1.0d), any(Tags.class)); + verify(searchQueryCategorizer.searchQueryCounters.nameToQueryTypeCounters.get("match")).add(eq(1.0d), any(Tags.class)); } public void testMatchPhraseQuery() { @@ -112,7 +112,7 @@ public void testMatchPhraseQuery() { searchQueryCategorizer.categorize(sourceBuilder); - Mockito.verify(searchQueryCategorizer.searchQueryCounters.matchPhrasePrefixCounter).add(eq(1.0d), any(Tags.class)); + verify(searchQueryCategorizer.searchQueryCounters.nameToQueryTypeCounters.get("match_phrase")).add(eq(1.0d), any(Tags.class)); } public void testMultiMatchQuery() { @@ -122,7 +122,7 @@ public void testMultiMatchQuery() { searchQueryCategorizer.categorize(sourceBuilder); - Mockito.verify(searchQueryCategorizer.searchQueryCounters.multiMatchCounter).add(eq(1.0d), any(Tags.class)); + verify(searchQueryCategorizer.searchQueryCounters.nameToQueryTypeCounters.get("multi_match")).add(eq(1.0d), any(Tags.class)); } public void testOtherQuery() { @@ -136,8 +136,9 @@ public void testOtherQuery() { searchQueryCategorizer.categorize(sourceBuilder); - Mockito.verify(searchQueryCategorizer.searchQueryCounters.otherQueryCounter, times(2)).add(eq(1.0d), any(Tags.class)); - Mockito.verify(searchQueryCategorizer.searchQueryCounters.termCounter).add(eq(1.0d), any(Tags.class)); + 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)); } public void testQueryStringQuery() { @@ -148,7 +149,7 @@ public void testQueryStringQuery() { searchQueryCategorizer.categorize(sourceBuilder); - Mockito.verify(searchQueryCategorizer.searchQueryCounters.queryStringQueryCounter).add(eq(1.0d), any(Tags.class)); + verify(searchQueryCategorizer.searchQueryCounters.nameToQueryTypeCounters.get("query_string")).add(eq(1.0d), any(Tags.class)); } public void testRangeQuery() { @@ -160,7 +161,7 @@ public void testRangeQuery() { searchQueryCategorizer.categorize(sourceBuilder); - Mockito.verify(searchQueryCategorizer.searchQueryCounters.rangeCounter).add(eq(1.0d), any(Tags.class)); + verify(searchQueryCategorizer.searchQueryCounters.nameToQueryTypeCounters.get("range")).add(eq(1.0d), any(Tags.class)); } public void testRegexQuery() { @@ -169,7 +170,7 @@ public void testRegexQuery() { searchQueryCategorizer.categorize(sourceBuilder); - Mockito.verify(searchQueryCategorizer.searchQueryCounters.regexCounter).add(eq(1.0d), any(Tags.class)); + verify(searchQueryCategorizer.searchQueryCounters.nameToQueryTypeCounters.get("regexp")).add(eq(1.0d), any(Tags.class)); } public void testSortQuery() { @@ -180,8 +181,8 @@ public void testSortQuery() { searchQueryCategorizer.categorize(sourceBuilder); - Mockito.verify(searchQueryCategorizer.searchQueryCounters.matchCounter).add(eq(1.0d), any(Tags.class)); - Mockito.verify(searchQueryCategorizer.searchQueryCounters.sortCounter, times(2)).add(eq(1.0d), any(Tags.class)); + 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)); } public void testTermQuery() { @@ -191,7 +192,7 @@ public void testTermQuery() { searchQueryCategorizer.categorize(sourceBuilder); - Mockito.verify(searchQueryCategorizer.searchQueryCounters.termCounter).add(eq(1.0d), any(Tags.class)); + verify(searchQueryCategorizer.searchQueryCounters.nameToQueryTypeCounters.get("term")).add(eq(1.0d), any(Tags.class)); } public void testWildcardQuery() { @@ -201,7 +202,7 @@ public void testWildcardQuery() { searchQueryCategorizer.categorize(sourceBuilder); - Mockito.verify(searchQueryCategorizer.searchQueryCounters.wildcardCounter).add(eq(1.0d), any(Tags.class)); + verify(searchQueryCategorizer.searchQueryCounters.nameToQueryTypeCounters.get("wildcard")).add(eq(1.0d), any(Tags.class)); } public void testComplexQuery() { @@ -219,10 +220,10 @@ public void testComplexQuery() { searchQueryCategorizer.categorize(sourceBuilder); - Mockito.verify(searchQueryCategorizer.searchQueryCounters.termCounter).add(eq(1.0d), any(Tags.class)); - Mockito.verify(searchQueryCategorizer.searchQueryCounters.matchCounter).add(eq(1.0d), any(Tags.class)); - Mockito.verify(searchQueryCategorizer.searchQueryCounters.regexCounter).add(eq(1.0d), any(Tags.class)); - Mockito.verify(searchQueryCategorizer.searchQueryCounters.boolCounter).add(eq(1.0d), any(Tags.class)); - Mockito.verify(searchQueryCategorizer.searchQueryCounters.aggCounter).add(eq(1.0d)); + 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)); } } From 8d264ecdd7a0ceddb4dd6848453418114420ca07 Mon Sep 17 00:00:00 2001 From: Siddhant Deshmukh Date: Tue, 23 Apr 2024 15:13:05 -0700 Subject: [PATCH 2/3] Remove unused imports Signed-off-by: Siddhant Deshmukh --- .../java/org/opensearch/action/search/SearchQueryCounters.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/server/src/main/java/org/opensearch/action/search/SearchQueryCounters.java b/server/src/main/java/org/opensearch/action/search/SearchQueryCounters.java index 438ca961f5094..fb4bbec9d0108 100644 --- a/server/src/main/java/org/opensearch/action/search/SearchQueryCounters.java +++ b/server/src/main/java/org/opensearch/action/search/SearchQueryCounters.java @@ -13,8 +13,6 @@ import org.opensearch.telemetry.metrics.MetricsRegistry; import org.opensearch.telemetry.metrics.tags.Tags; -import java.util.HashMap; -import java.util.Map; import java.util.concurrent.ConcurrentHashMap; /** From f386dcc694b035481b1725ef8c05a0d54c91f901 Mon Sep 17 00:00:00 2001 From: Siddhant Deshmukh Date: Thu, 25 Apr 2024 11:41:37 -0700 Subject: [PATCH 3/3] Trigger build Signed-off-by: Siddhant Deshmukh