Skip to content

Commit

Permalink
Address review comments
Browse files Browse the repository at this point in the history 
Signed-off-by: Siddhant Deshmukh <[email protected]>
  • Loading branch information
@deshsidd
deshsidd committed Oct 17, 2023
1 parent c2f0447 commit c042369
Show file tree
Hide file tree
Showing 5 changed files with 38 additions and 38 deletions.
3 changes: 1 addition & 2 deletions server/src/main/java/org/opensearch/action/search/SearchQueryCategorizer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,7 @@ private void logQueryShape(QueryBuilder topLevelQueryBuilder) {
}
QueryShapeVisitor shapeVisitor = new QueryShapeVisitor();
topLevelQueryBuilder.visit(shapeVisitor);
String indentedQueryShape = shapeVisitor.prettyPrintTree(" ");
log.debug("Query shape : " + indentedQueryShape);
log.debug("Query shape : {}", shapeVisitor.prettyPrintTree(" "));
}

}
23 changes: 12 additions & 11 deletions server/src/main/java/org/opensearch/action/search/SearchQueryCategorizingVisitor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
* Increments the counters related to Search Query type.
*/
public class SearchQueryCategorizingVisitor implements QueryBuilderVisitor {
public static final String LEVEL_TAG = "level";
private final int level;
private final SearchQueryCounters searchQueryCounters;

Expand All @@ -42,27 +43,27 @@ private SearchQueryCategorizingVisitor(SearchQueryCounters counters, int level)

public void accept(QueryBuilder qb) {
if (qb instanceof BoolQueryBuilder) {
searchQueryCounters.boolCounter.add(1, Tags.create().addTag("level", level));
searchQueryCounters.boolCounter.add(1, Tags.create().addTag(LEVEL_TAG, level));
} else if (qb instanceof FunctionScoreQueryBuilder) {
searchQueryCounters.functionScoreCounter.add(1, Tags.create().addTag("level", level));
searchQueryCounters.functionScoreCounter.add(1, Tags.create().addTag(LEVEL_TAG, level));
} else if (qb instanceof MatchQueryBuilder) {
searchQueryCounters.matchCounter.add(1, Tags.create().addTag("level", level));
searchQueryCounters.matchCounter.add(1, Tags.create().addTag(LEVEL_TAG, level));
} else if (qb instanceof MatchPhraseQueryBuilder) {
searchQueryCounters.matchPhrasePrefixCounter.add(1, Tags.create().addTag("level", level));
searchQueryCounters.matchPhrasePrefixCounter.add(1, Tags.create().addTag(LEVEL_TAG, level));
} else if (qb instanceof MultiMatchQueryBuilder) {
searchQueryCounters.multiMatchCounter.add(1, Tags.create().addTag("level", level));
searchQueryCounters.multiMatchCounter.add(1, Tags.create().addTag(LEVEL_TAG, level));
} else if (qb instanceof QueryStringQueryBuilder) {
searchQueryCounters.queryStringQueryCounter.add(1, Tags.create().addTag("level", level));
searchQueryCounters.queryStringQueryCounter.add(1, Tags.create().addTag(LEVEL_TAG, level));
} else if (qb instanceof RangeQueryBuilder) {
searchQueryCounters.rangeCounter.add(1, Tags.create().addTag("level", level));
searchQueryCounters.rangeCounter.add(1, Tags.create().addTag(LEVEL_TAG, level));
} else if (qb instanceof RegexpQueryBuilder) {
searchQueryCounters.regexCounter.add(1, Tags.create().addTag("level", level));
searchQueryCounters.regexCounter.add(1, Tags.create().addTag(LEVEL_TAG, level));
} else if (qb instanceof TermQueryBuilder) {
searchQueryCounters.termCounter.add(1, Tags.create().addTag("level", level));
searchQueryCounters.termCounter.add(1, Tags.create().addTag(LEVEL_TAG, level));
} else if (qb instanceof WildcardQueryBuilder) {
searchQueryCounters.wildcardCounter.add(1, Tags.create().addTag("level", level));
searchQueryCounters.wildcardCounter.add(1, Tags.create().addTag(LEVEL_TAG, level));
} else {
searchQueryCounters.otherQueryCounter.add(1, Tags.create().addTag("level", level));
searchQueryCounters.otherQueryCounter.add(1, Tags.create().addTag(LEVEL_TAG, level));
}
}

Expand Down
30 changes: 15 additions & 15 deletions server/src/main/java/org/opensearch/action/search/SearchQueryCounters.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,77 +38,77 @@ public class SearchQueryCounters {
public SearchQueryCounters(MetricsRegistry metricsRegistry) {
this.metricsRegistry = metricsRegistry;
this.aggCounter = metricsRegistry.createCounter(
"aggSearchQueryCounter",
"search.query.type.agg.count",
"Counter for the number of top level agg search queries",
"0"
);
this.boolCounter = metricsRegistry.createCounter(
"boolSearchQueryCounter",
"search.query.type.bool.count",
"Counter for the number of top level and nested bool search queries",
"0"
);
this.functionScoreCounter = metricsRegistry.createCounter(
"functionScoreSearchQueryCounter",
"search.query.type.functionscore.count",
"Counter for the number of top level and nested function score search queries",
"0"
);
this.matchCounter = metricsRegistry.createCounter(
"matchSearchQueryCounter",
"search.query.type.match.count",
"Counter for the number of top level and nested match search queries",
"0"
);
this.matchPhrasePrefixCounter = metricsRegistry.createCounter(
"matchPhrasePrefixSearchQueryCounter",
"search.query.type.matchphrase.count",
"Counter for the number of top level and nested match phrase prefix search queries",
"0"
);
this.multiMatchCounter = metricsRegistry.createCounter(
"multiMatchSearchQueryCounter",
"search.query.type.multimatch.count",
"Counter for the number of top level and nested multi match search queries",
"0"
);
this.otherQueryCounter = metricsRegistry.createCounter(
"otherSearchQueryCounter",
"search.query.type.other.count",
"Counter for the number of top level and nested search queries that do not match any other categories",
"0"
);
this.queryStringQueryCounter = metricsRegistry.createCounter(
"queryStringQuerySearchQueryCounter",
"search.query.type.querystringquery.count",
"Counter for the number of top level and nested queryStringQuery search queries",
"0"
);
this.rangeCounter = metricsRegistry.createCounter(
"rangeSearchQueryCounter",
"search.query.type.range.count",
"Counter for the number of top level and nested range search queries",
"0"
);
this.regexCounter = metricsRegistry.createCounter(
"regexSearchQueryCounter",
"search.query.type.regex.count",
"Counter for the number of top level and nested regex search queries",
"0"
);
this.skippedCounter = metricsRegistry.createCounter(
"skippedQueryCounter",
"search.query.type.skipped.count",
"Counter for the number queries skipped due to error",
"0"
);
this.sortCounter = metricsRegistry.createCounter(
"sortSearchQueryCounter",
"search.query.type.sort.count",
"Counter for the number of top level sort search queries",
"0"
);
this.termCounter = metricsRegistry.createCounter(
"termSearchQueryCounter",
"search.query.type.term.count",
"Counter for the number of top level and nested term search queries",
"0"
);
this.totalCounter = metricsRegistry.createCounter(
"totalSearchQueryCounter",
"search.query.type.total.count",
"Counter for the number of top level and nested search queries",
"0"
);
this.wildcardCounter = metricsRegistry.createCounter(
"wildcardSearchQueryCounter",
"search.query.type.wildcard.count",
"Counter for the number of top level and nested wildcard search queries",
"0"
);
Expand Down
18 changes: 9 additions & 9 deletions server/src/main/java/org/opensearch/action/search/TransportSearchAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -138,8 +138,8 @@ public class TransportSearchAction extends HandledTransportAction<SearchRequest,
Property.NodeScope
);

public static final Setting<Boolean> SEARCH_QUERY_CATEGORIZATION_ENABLED_SETTING = Setting.boolSetting(
"search.query.categorization.enabled",
public static final Setting<Boolean> SEARCH_QUERY_METRICS_ENABLED_SETTING = Setting.boolSetting(
"search.query.metrics.enabled",
false,
Setting.Property.NodeScope,
Setting.Property.Dynamic
Expand Down Expand Up @@ -185,7 +185,7 @@ public class TransportSearchAction extends HandledTransportAction<SearchRequest,

private volatile boolean isRequestStatsEnabled;

private volatile boolean isSearchQueryCategorizationEnabled;
private volatile boolean searchQueryCategorizationEnabled;

private final SearchRequestStats searchRequestStats;

Expand Down Expand Up @@ -227,14 +227,14 @@ public TransportSearchAction(
clusterService.getClusterSettings().addSettingsUpdateConsumer(SEARCH_REQUEST_STATS_ENABLED, this::setIsRequestStatsEnabled);
this.searchRequestStats = searchRequestStats;
this.metricsRegistry = metricsRegistry;
this.isSearchQueryCategorizationEnabled = clusterService.getClusterSettings().get(SEARCH_QUERY_CATEGORIZATION_ENABLED_SETTING);
this.searchQueryCategorizationEnabled = clusterService.getClusterSettings().get(SEARCH_QUERY_METRICS_ENABLED_SETTING);
clusterService.getClusterSettings()
.addSettingsUpdateConsumer(SEARCH_QUERY_CATEGORIZATION_ENABLED_SETTING, this::setIsSearchQueryCategorizationEnabled);
.addSettingsUpdateConsumer(SEARCH_QUERY_METRICS_ENABLED_SETTING, this::setIsSearchQueryCategorizationEnabled);
}

private void setIsSearchQueryCategorizationEnabled(boolean isSearchQueryCategorizationEnabled) {
this.isSearchQueryCategorizationEnabled = isSearchQueryCategorizationEnabled;
if (this.isSearchQueryCategorizationEnabled && this.searchQueryCategorizer == null) {
this.searchQueryCategorizationEnabled = isSearchQueryCategorizationEnabled;
if (this.searchQueryCategorizationEnabled && this.searchQueryCategorizer == null) {
this.searchQueryCategorizer = new SearchQueryCategorizer(metricsRegistry);
}
}
Expand Down Expand Up @@ -515,11 +515,11 @@ private void executeRequest(
return;
}

if (isSearchQueryCategorizationEnabled) {
if (searchQueryCategorizationEnabled) {
try {
searchQueryCategorizer.categorize(searchRequest.source());
} catch (Exception e) {
logger.error("Error while trying to categorize the query.");
logger.error("Error while trying to categorize the query.", e);
}
}

Expand Down
2 changes: 1 addition & 1 deletion server/src/main/java/org/opensearch/common/settings/ClusterSettings.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,7 @@ public void apply(Settings value, Settings current, Settings previous) {
TransportSearchAction.SHARD_COUNT_LIMIT_SETTING,
TransportSearchAction.SEARCH_CANCEL_AFTER_TIME_INTERVAL_SETTING,
TransportSearchAction.SEARCH_REQUEST_STATS_ENABLED,
TransportSearchAction.SEARCH_QUERY_CATEGORIZATION_ENABLED_SETTING,
TransportSearchAction.SEARCH_QUERY_METRICS_ENABLED_SETTING,
TransportSearchAction.SEARCH_PHASE_TOOK_ENABLED,
RemoteClusterService.REMOTE_CLUSTER_SKIP_UNAVAILABLE,
SniffConnectionStrategy.REMOTE_CONNECTIONS_PER_CLUSTER,
Expand Down

0 comments on commit c042369

Please sign in to comment.