Skip to content

Commit aeeaa24

Browse files
Make sure listener is started when query metrics enabled (#74) (#75)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
1 parent 94f6848 commit aeeaa24

File tree

9 files changed

+206
-131
lines changed

9 files changed

+206
-131
lines changed

src/main/java/org/opensearch/plugin/insights/QueryInsightsPlugin.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ public Collection<Object> createComponents(
8181
client,
8282
metricsRegistry
8383
);
84-
return List.of(queryInsightsService, new QueryInsightsListener(clusterService, queryInsightsService));
84+
return List.of(queryInsightsService, new QueryInsightsListener(clusterService, queryInsightsService, false));
8585
}
8686

8787
@Override

src/main/java/org/opensearch/plugin/insights/core/exporter/QueryInsightsExporterFactory.java

+1
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,7 @@ public QueryInsightsExporter updateExporter(QueryInsightsExporter exporter, Stri
118118
* Close an exporter
119119
*
120120
* @param exporter the exporter to close
121+
* @throws IOException exception
121122
*/
122123
public void closeExporter(QueryInsightsExporter exporter) throws IOException {
123124
if (exporter != null) {

src/main/java/org/opensearch/plugin/insights/core/listener/QueryInsightsListener.java

+49-14
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
package org.opensearch.plugin.insights.core.listener;
1010

11+
import static org.opensearch.plugin.insights.settings.QueryCategorizationSettings.SEARCH_QUERY_METRICS_ENABLED_SETTING;
1112
import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.getTopNEnabledSetting;
1213
import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.getTopNSizeSetting;
1314
import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.getTopNWindowSizeSetting;
@@ -56,10 +57,27 @@ public final class QueryInsightsListener extends SearchRequestOperationsListener
5657
*/
5758
@Inject
5859
public QueryInsightsListener(final ClusterService clusterService, final QueryInsightsService queryInsightsService) {
60+
this(clusterService, queryInsightsService, false);
61+
}
62+
63+
/**
64+
* Constructor for QueryInsightsListener
65+
*
66+
* @param clusterService The Node's cluster service.
67+
* @param queryInsightsService The topQueriesByLatencyService associated with this listener
68+
* @param initiallyEnabled Is the listener initially enabled/disabled
69+
*/
70+
public QueryInsightsListener(
71+
final ClusterService clusterService,
72+
final QueryInsightsService queryInsightsService,
73+
boolean initiallyEnabled
74+
) {
75+
super(initiallyEnabled);
5976
this.clusterService = clusterService;
6077
this.queryInsightsService = queryInsightsService;
61-
// Setting endpoints set up for top n queries, including enabling top n queries, window size and top n size
62-
// Expected metricTypes are Latency, CPU and Memory.
78+
79+
// Setting endpoints set up for top n queries, including enabling top n queries, window size, and top n size
80+
// Expected metricTypes are Latency, CPU, and Memory.
6381
for (MetricType type : MetricType.allMetricTypes()) {
6482
clusterService.getClusterSettings()
6583
.addSettingsUpdateConsumer(getTopNEnabledSetting(type), v -> this.setEnableTopQueries(type, v));
@@ -82,31 +100,48 @@ public QueryInsightsListener(final ClusterService clusterService, final QueryIns
82100
this.queryInsightsService.validateWindowSize(type, clusterService.getClusterSettings().get(getTopNWindowSizeSetting(type)));
83101
this.queryInsightsService.setWindowSize(type, clusterService.getClusterSettings().get(getTopNWindowSizeSetting(type)));
84102
}
103+
104+
clusterService.getClusterSettings()
105+
.addSettingsUpdateConsumer(SEARCH_QUERY_METRICS_ENABLED_SETTING, v -> setSearchQueryMetricsEnabled(v));
106+
setSearchQueryMetricsEnabled(clusterService.getClusterSettings().get(SEARCH_QUERY_METRICS_ENABLED_SETTING));
85107
}
86108

87109
/**
88-
* Enable or disable top queries insights collection for {@link MetricType}
110+
* Enable or disable top queries insights collection for {@link MetricType}.
89111
* This function will enable or disable the corresponding listeners
90112
* and query insights services.
91113
*
92114
* @param metricType {@link MetricType}
93115
* @param isCurrentMetricEnabled boolean
94116
*/
95117
public void setEnableTopQueries(final MetricType metricType, final boolean isCurrentMetricEnabled) {
96-
boolean isTopNFeaturePreviouslyDisabled = !queryInsightsService.isTopNFeatureEnabled();
97118
this.queryInsightsService.enableCollection(metricType, isCurrentMetricEnabled);
98-
boolean isTopNFeatureCurrentlyDisabled = !queryInsightsService.isTopNFeatureEnabled();
119+
updateQueryInsightsState();
120+
}
99121

100-
if (isTopNFeatureCurrentlyDisabled) {
101-
super.setEnabled(false);
102-
if (!isTopNFeaturePreviouslyDisabled) {
103-
queryInsightsService.checkAndStopQueryInsights();
104-
}
105-
} else {
122+
/**
123+
* Set search query metrics enabled to enable collection of search query categorization metrics.
124+
* @param searchQueryMetricsEnabled boolean flag
125+
*/
126+
public void setSearchQueryMetricsEnabled(boolean searchQueryMetricsEnabled) {
127+
this.queryInsightsService.enableSearchQueryMetricsFeature(searchQueryMetricsEnabled);
128+
updateQueryInsightsState();
129+
}
130+
131+
/**
132+
* Update the query insights service state based on the enabled features.
133+
* If any feature is enabled, it starts the service. If no features are enabled, it stops the service.
134+
*/
135+
private void updateQueryInsightsState() {
136+
boolean anyFeatureEnabled = queryInsightsService.isAnyFeatureEnabled();
137+
138+
if (anyFeatureEnabled && !super.isEnabled()) {
106139
super.setEnabled(true);
107-
if (isTopNFeaturePreviouslyDisabled) {
108-
queryInsightsService.checkAndRestartQueryInsights();
109-
}
140+
queryInsightsService.stop(); // Ensures a clean restart
141+
queryInsightsService.start();
142+
} else if (!anyFeatureEnabled && super.isEnabled()) {
143+
super.setEnabled(false);
144+
queryInsightsService.stop();
110145
}
111146
}
112147

src/main/java/org/opensearch/plugin/insights/core/service/QueryInsightsService.java

+6-36
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88

99
package org.opensearch.plugin.insights.core.service;
1010

11-
import static org.opensearch.plugin.insights.settings.QueryCategorizationSettings.SEARCH_QUERY_METRICS_ENABLED_SETTING;
1211
import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.getExporterSettings;
1312

1413
import java.io.IOException;
@@ -111,15 +110,15 @@ public QueryInsightsService(
111110
);
112111
}
113112

114-
this.searchQueryMetricsEnabled = clusterSettings.get(SEARCH_QUERY_METRICS_ENABLED_SETTING);
115113
this.searchQueryCategorizer = SearchQueryCategorizer.getInstance(metricsRegistry);
116-
clusterSettings.addSettingsUpdateConsumer(SEARCH_QUERY_METRICS_ENABLED_SETTING, this::setSearchQueryMetricsEnabled);
114+
this.enableSearchQueryMetricsFeature(false);
117115
}
118116

119117
/**
120118
* Ingest the query data into in-memory stores
121119
*
122120
* @param record the record to ingest
121+
* @return SearchQueryRecord
123122
*/
124123
public boolean addRecord(final SearchQueryRecord record) {
125124
boolean shouldAdd = searchQueryMetricsEnabled;
@@ -228,22 +227,11 @@ public boolean isSearchQueryMetricsFeatureEnabled() {
228227
}
229228

230229
/**
231-
* Stops query insights service if no features enabled
230+
* Enable/Disable search query metrics feature.
231+
* @param enable enable/disable search query metrics feature
232232
*/
233-
public void checkAndStopQueryInsights() {
234-
if (!isAnyFeatureEnabled()) {
235-
this.stop();
236-
}
237-
}
238-
239-
/**
240-
* Restarts query insights service if any feature enabled
241-
*/
242-
public void checkAndRestartQueryInsights() {
243-
if (isAnyFeatureEnabled()) {
244-
this.stop();
245-
this.start();
246-
}
233+
public void enableSearchQueryMetricsFeature(boolean enable) {
234+
searchQueryMetricsEnabled = enable;
247235
}
248236

249237
/**
@@ -306,24 +294,6 @@ public void setExporter(final MetricType type, final Settings settings) {
306294
}
307295
}
308296

309-
/**
310-
* Set search query metrics enabled to enable collection of search query categorization metrics
311-
* @param searchQueryMetricsEnabled boolean flag
312-
*/
313-
public void setSearchQueryMetricsEnabled(boolean searchQueryMetricsEnabled) {
314-
boolean oldSearchQueryMetricsEnabled = isSearchQueryMetricsFeatureEnabled();
315-
this.searchQueryMetricsEnabled = searchQueryMetricsEnabled;
316-
if (searchQueryMetricsEnabled) {
317-
if (!oldSearchQueryMetricsEnabled) {
318-
checkAndRestartQueryInsights();
319-
}
320-
} else {
321-
if (oldSearchQueryMetricsEnabled) {
322-
checkAndStopQueryInsights();
323-
}
324-
}
325-
}
326-
327297
/**
328298
* Get search query categorizer object
329299
* @return SearchQueryCategorizer object

src/main/java/org/opensearch/plugin/insights/core/service/TopQueriesService.java

+1
Original file line numberDiff line numberDiff line change
@@ -363,6 +363,7 @@ public List<SearchQueryRecord> getTopQueriesCurrentSnapshot() {
363363

364364
/**
365365
* Close the top n queries service
366+
* @throws IOException exception
366367
*/
367368
public void close() throws IOException {
368369
queryInsightsExporterFactory.closeExporter(this.exporter);

src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesRequest.java

+1
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ public TopQueriesRequest(final MetricType metricType, final String... nodesIds)
4646

4747
/**
4848
* Get the type of requested metrics
49+
* @return MetricType for current top query service
4950
*/
5051
public MetricType getMetricType() {
5152
return metricType;

src/main/java/org/opensearch/plugin/insights/rules/model/Attribute.java

+1
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ static void writeTo(final StreamOutput out, final Attribute attribute) throws IO
8585
*
8686
* @param out the StreamOutput to write
8787
* @param attributeValue the Attribute value to write
88+
* @throws IOException exception
8889
*/
8990
@SuppressWarnings("unchecked")
9091
public static void writeValueTo(StreamOutput out, Object attributeValue) throws IOException {

0 commit comments

Comments
 (0)