Skip to content

Commit e2df8ad

Browse files
ansjcyansjcy
committed
refactor the code to make it more maintainable
Signed-off-by: Chenyang Ji <[email protected]>
1 parent 112a611 commit e2df8ad

File tree

14 files changed

+299
-125
lines changed

14 files changed

+299
-125
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
2222
- [Remote State] Add async remote state deletion task running on an interval, configurable by a setting ([#13995](https://github.com/opensearch-project/OpenSearch/pull/13995))
2323
- Add remote routing table for remote state publication with experimental feature flag ([#13304](https://github.com/opensearch-project/OpenSearch/pull/13304))
2424
- Add support for query level resource usage tracking ([#13172](https://github.com/opensearch-project/OpenSearch/pull/13172))
25+
- [Query Insights] Add cpu and memory metrics to top n queries ([#13739](https://github.com/opensearch-project/OpenSearch/pull/13739))
2526

2627
### Dependencies
2728
- Bump `com.github.spullara.mustache.java:compiler` from 0.9.10 to 0.9.13 ([#13329](https://github.com/opensearch-project/OpenSearch/pull/13329), [#13559](https://github.com/opensearch-project/OpenSearch/pull/13559))

‎plugins/query-insights/src/main/java/org/opensearch/plugin/insights/QueryInsightsPlugin.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,9 +115,11 @@ public List<Setting<?>> getSettings() {
115115
QueryInsightsSettings.TOP_N_CPU_QUERIES_ENABLED,
116116
QueryInsightsSettings.TOP_N_CPU_QUERIES_SIZE,
117117
QueryInsightsSettings.TOP_N_CPU_QUERIES_WINDOW_SIZE,
118+
QueryInsightsSettings.TOP_N_CPU_EXPORTER_SETTINGS,
118119
QueryInsightsSettings.TOP_N_MEMORY_QUERIES_ENABLED,
119120
QueryInsightsSettings.TOP_N_MEMORY_QUERIES_SIZE,
120-
QueryInsightsSettings.TOP_N_MEMORY_QUERIES_WINDOW_SIZE
121+
QueryInsightsSettings.TOP_N_MEMORY_QUERIES_WINDOW_SIZE,
122+
QueryInsightsSettings.TOP_N_MEMORY_EXPORTER_SETTINGS
121123
);
122124
}
123125
}

‎plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/exporter/QueryInsightsExporterFactory.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
import java.util.Locale;
2020
import java.util.Set;
2121

22-
import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.DEFAULT_TOP_N_LATENCY_QUERIES_INDEX_PATTERN;
22+
import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.DEFAULT_TOP_N_QUERIES_INDEX_PATTERN;
2323
import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.DEFAULT_TOP_QUERIES_EXPORTER_TYPE;
2424
import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.EXPORTER_TYPE;
2525
import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.EXPORT_INDEX;
@@ -71,7 +71,7 @@ public void validateExporterConfig(final Settings settings) throws IllegalArgume
7171
}
7272
switch (type) {
7373
case LOCAL_INDEX:
74-
final String indexPattern = settings.get(EXPORT_INDEX, DEFAULT_TOP_N_LATENCY_QUERIES_INDEX_PATTERN);
74+
final String indexPattern = settings.get(EXPORT_INDEX, DEFAULT_TOP_N_QUERIES_INDEX_PATTERN);
7575
if (indexPattern.length() == 0) {
7676
throw new IllegalArgumentException("Empty index pattern configured for the exporter");
7777
}

‎plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/listener/QueryInsightsListener.java

Lines changed: 29 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -32,15 +32,9 @@
3232
import java.util.Map;
3333
import java.util.concurrent.TimeUnit;
3434

35-
import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.TOP_N_CPU_QUERIES_ENABLED;
36-
import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.TOP_N_CPU_QUERIES_SIZE;
37-
import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.TOP_N_CPU_QUERIES_WINDOW_SIZE;
38-
import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.TOP_N_LATENCY_QUERIES_ENABLED;
39-
import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.TOP_N_LATENCY_QUERIES_SIZE;
40-
import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.TOP_N_LATENCY_QUERIES_WINDOW_SIZE;
41-
import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.TOP_N_MEMORY_QUERIES_ENABLED;
42-
import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.TOP_N_MEMORY_QUERIES_SIZE;
43-
import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.TOP_N_MEMORY_QUERIES_WINDOW_SIZE;
35+
import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.getTopNEnabledSetting;
36+
import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.getTopNSizeSetting;
37+
import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.getTopNWindowSizeSetting;
4438

4539
/**
4640
* The listener for query insights services.
@@ -67,63 +61,30 @@ public final class QueryInsightsListener extends SearchRequestOperationsListener
6761
public QueryInsightsListener(final ClusterService clusterService, final QueryInsightsService queryInsightsService) {
6862
this.clusterService = clusterService;
6963
this.queryInsightsService = queryInsightsService;
70-
clusterService.getClusterSettings()
71-
.addSettingsUpdateConsumer(TOP_N_LATENCY_QUERIES_ENABLED, v -> this.setEnableTopQueries(MetricType.LATENCY, v));
72-
clusterService.getClusterSettings()
73-
.addSettingsUpdateConsumer(
74-
TOP_N_LATENCY_QUERIES_SIZE,
75-
v -> this.queryInsightsService.getTopQueriesService(MetricType.LATENCY).setTopNSize(v),
76-
v -> this.queryInsightsService.getTopQueriesService(MetricType.LATENCY).validateTopNSize(v)
77-
);
78-
clusterService.getClusterSettings()
79-
.addSettingsUpdateConsumer(
80-
TOP_N_LATENCY_QUERIES_WINDOW_SIZE,
81-
v -> this.queryInsightsService.getTopQueriesService(MetricType.LATENCY).setWindowSize(v),
82-
v -> this.queryInsightsService.getTopQueriesService(MetricType.LATENCY).validateWindowSize(v)
83-
);
84-
clusterService.getClusterSettings()
85-
.addSettingsUpdateConsumer(TOP_N_CPU_QUERIES_ENABLED, v -> this.setEnableTopQueries(MetricType.CPU, v));
86-
clusterService.getClusterSettings()
87-
.addSettingsUpdateConsumer(
88-
TOP_N_CPU_QUERIES_SIZE,
89-
v -> this.queryInsightsService.getTopQueriesService(MetricType.CPU).setTopNSize(v),
90-
v -> this.queryInsightsService.getTopQueriesService(MetricType.CPU).validateTopNSize(v)
91-
);
92-
clusterService.getClusterSettings()
93-
.addSettingsUpdateConsumer(
94-
TOP_N_CPU_QUERIES_WINDOW_SIZE,
95-
v -> this.queryInsightsService.getTopQueriesService(MetricType.CPU).setWindowSize(v),
96-
v -> this.queryInsightsService.getTopQueriesService(MetricType.CPU).validateWindowSize(v)
97-
);
98-
clusterService.getClusterSettings()
99-
.addSettingsUpdateConsumer(TOP_N_MEMORY_QUERIES_ENABLED, v -> this.setEnableTopQueries(MetricType.MEMORY, v));
100-
clusterService.getClusterSettings()
101-
.addSettingsUpdateConsumer(
102-
TOP_N_MEMORY_QUERIES_SIZE,
103-
v -> this.queryInsightsService.getTopQueriesService(MetricType.MEMORY).setTopNSize(v),
104-
v -> this.queryInsightsService.getTopQueriesService(MetricType.MEMORY).validateTopNSize(v)
105-
);
106-
clusterService.getClusterSettings()
107-
.addSettingsUpdateConsumer(
108-
TOP_N_MEMORY_QUERIES_WINDOW_SIZE,
109-
v -> this.queryInsightsService.getTopQueriesService(MetricType.MEMORY).setWindowSize(v),
110-
v -> this.queryInsightsService.getTopQueriesService(MetricType.MEMORY).validateWindowSize(v)
111-
);
112-
this.setEnableTopQueries(MetricType.LATENCY, clusterService.getClusterSettings().get(TOP_N_LATENCY_QUERIES_ENABLED));
113-
this.queryInsightsService.getTopQueriesService(MetricType.LATENCY)
114-
.setTopNSize(clusterService.getClusterSettings().get(TOP_N_LATENCY_QUERIES_SIZE));
115-
this.queryInsightsService.getTopQueriesService(MetricType.LATENCY)
116-
.setWindowSize(clusterService.getClusterSettings().get(TOP_N_LATENCY_QUERIES_WINDOW_SIZE));
117-
this.setEnableTopQueries(MetricType.CPU, clusterService.getClusterSettings().get(TOP_N_CPU_QUERIES_ENABLED));
118-
this.queryInsightsService.getTopQueriesService(MetricType.CPU)
119-
.setTopNSize(clusterService.getClusterSettings().get(TOP_N_CPU_QUERIES_SIZE));
120-
this.queryInsightsService.getTopQueriesService(MetricType.CPU)
121-
.setWindowSize(clusterService.getClusterSettings().get(TOP_N_CPU_QUERIES_WINDOW_SIZE));
122-
this.setEnableTopQueries(MetricType.MEMORY, clusterService.getClusterSettings().get(TOP_N_MEMORY_QUERIES_ENABLED));
123-
this.queryInsightsService.getTopQueriesService(MetricType.MEMORY)
124-
.setTopNSize(clusterService.getClusterSettings().get(TOP_N_MEMORY_QUERIES_SIZE));
125-
this.queryInsightsService.getTopQueriesService(MetricType.MEMORY)
126-
.setWindowSize(clusterService.getClusterSettings().get(TOP_N_MEMORY_QUERIES_WINDOW_SIZE));
64+
// Setting endpoints set up for top n queries, including enabling top n queries, window size and top n size
65+
// Expected metricTypes are Latency, CPU and Memory.
66+
for (MetricType type : MetricType.allMetricTypes()) {
67+
clusterService.getClusterSettings()
68+
.addSettingsUpdateConsumer(getTopNEnabledSetting(type), v -> this.setEnableTopQueries(type, v));
69+
clusterService.getClusterSettings()
70+
.addSettingsUpdateConsumer(
71+
getTopNSizeSetting(type),
72+
v -> this.queryInsightsService.setTopNSize(type, v),
73+
v -> this.queryInsightsService.validateTopNSize(type, v)
74+
);
75+
clusterService.getClusterSettings()
76+
.addSettingsUpdateConsumer(
77+
getTopNWindowSizeSetting(type),
78+
v -> this.queryInsightsService.setWindowSize(type, v),
79+
v -> this.queryInsightsService.validateWindowSize(type, v)
80+
);
81+
82+
this.setEnableTopQueries(type, clusterService.getClusterSettings().get(getTopNEnabledSetting(type)));
83+
this.queryInsightsService.validateTopNSize(type, clusterService.getClusterSettings().get(getTopNSizeSetting(type)));
84+
this.queryInsightsService.setTopNSize(type, clusterService.getClusterSettings().get(getTopNSizeSetting(type)));
85+
this.queryInsightsService.validateWindowSize(type, clusterService.getClusterSettings().get(getTopNWindowSizeSetting(type)));
86+
this.queryInsightsService.setWindowSize(type, clusterService.getClusterSettings().get(getTopNWindowSizeSetting(type)));
87+
}
12788
}
12889

12990
/**
@@ -175,6 +136,7 @@ public void onRequestStart(SearchRequestContext searchRequestContext) {}
175136
public void onRequestEnd(final SearchPhaseContext context, final SearchRequestContext searchRequestContext) {
176137
constructSearchQueryRecord(context, searchRequestContext);
177138
}
139+
178140
@Override
179141
public void onRequestFailure(final SearchPhaseContext context, final SearchRequestContext searchRequestContext) {
180142
constructSearchQueryRecord(context, searchRequestContext);
@@ -220,7 +182,7 @@ private void constructSearchQueryRecord(final SearchPhaseContext context, final
220182
attributes.put(Attribute.TOTAL_SHARDS, context.getNumShards());
221183
attributes.put(Attribute.INDICES, request.indices());
222184
attributes.put(Attribute.PHASE_LATENCY_MAP, searchRequestContext.phaseTookMap());
223-
attributes.put(Attribute.TASKS_RESOURCE_USAGES, tasksResourceUsages);
185+
attributes.put(Attribute.TASK_RESOURCE_USAGES, tasksResourceUsages);
224186

225187
Map<String, Object> labels = new HashMap<>();
226188
// Retrieve user provided label if exists

‎plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/QueryInsightsService.java

Lines changed: 82 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212
import org.opensearch.common.inject.Inject;
1313
import org.opensearch.common.lifecycle.AbstractLifecycleComponent;
1414
import org.opensearch.common.settings.ClusterSettings;
15+
import org.opensearch.common.settings.Settings;
16+
import org.opensearch.common.unit.TimeValue;
1517
import org.opensearch.plugin.insights.core.exporter.QueryInsightsExporterFactory;
1618
import org.opensearch.plugin.insights.rules.model.MetricType;
1719
import org.opensearch.plugin.insights.rules.model.SearchQueryRecord;
@@ -27,7 +29,7 @@
2729
import java.util.Map;
2830
import java.util.concurrent.LinkedBlockingQueue;
2931

30-
import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.TOP_N_LATENCY_EXPORTER_SETTINGS;
32+
import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.getExporterSettings;
3133

3234
/**
3335
* Service responsible for gathering, analyzing, storing and exporting
@@ -86,11 +88,13 @@ public QueryInsightsService(final ClusterSettings clusterSettings, final ThreadP
8688
enableCollect.put(metricType, false);
8789
topQueriesServices.put(metricType, new TopQueriesService(metricType, threadPool, queryInsightsExporterFactory));
8890
}
89-
clusterSettings.addSettingsUpdateConsumer(
90-
TOP_N_LATENCY_EXPORTER_SETTINGS,
91-
(settings -> getTopQueriesService(MetricType.LATENCY).setExporter(settings)),
92-
(settings -> getTopQueriesService(MetricType.LATENCY).validateExporterConfig(settings))
93-
);
91+
for (MetricType type : MetricType.allMetricTypes()) {
92+
clusterSettings.addSettingsUpdateConsumer(
93+
getExporterSettings(type),
94+
(settings -> setExporter(type, settings)),
95+
(settings -> validateExporterConfig(type, settings))
96+
);
97+
}
9498
}
9599

96100
/**
@@ -177,6 +181,78 @@ public boolean isEnabled() {
177181
return false;
178182
}
179183

184+
/**
185+
* Validate the window size config for a metricType
186+
*
187+
* @param type {@link MetricType}
188+
* @param windowSize {@link TimeValue}
189+
*/
190+
public void validateWindowSize(final MetricType type, final TimeValue windowSize) {
191+
if (topQueriesServices.containsKey(type)) {
192+
topQueriesServices.get(type).validateWindowSize(windowSize);
193+
}
194+
}
195+
196+
/**
197+
* Set window size for a metricType
198+
*
199+
* @param type {@link MetricType}
200+
* @param windowSize {@link TimeValue}
201+
*/
202+
public void setWindowSize(final MetricType type, final TimeValue windowSize) {
203+
if (topQueriesServices.containsKey(type)) {
204+
topQueriesServices.get(type).setWindowSize(windowSize);
205+
}
206+
}
207+
208+
/**
209+
* Validate the top n size config for a metricType
210+
*
211+
* @param type {@link MetricType}
212+
* @param topNSize top n size
213+
*/
214+
public void validateTopNSize(final MetricType type, final int topNSize) {
215+
if (topQueriesServices.containsKey(type)) {
216+
topQueriesServices.get(type).validateTopNSize(topNSize);
217+
}
218+
}
219+
220+
/**
221+
* Set the top n size config for a metricType
222+
*
223+
* @param type {@link MetricType}
224+
* @param topNSize top n size
225+
*/
226+
public void setTopNSize(final MetricType type, final int topNSize) {
227+
if (topQueriesServices.containsKey(type)) {
228+
topQueriesServices.get(type).setTopNSize(topNSize);
229+
}
230+
}
231+
232+
/**
233+
* Set the exporter config for a metricType
234+
*
235+
* @param type {@link MetricType}
236+
* @param settings exporter settings
237+
*/
238+
public void setExporter(final MetricType type, final Settings settings) {
239+
if (topQueriesServices.containsKey(type)) {
240+
topQueriesServices.get(type).setExporter(settings);
241+
}
242+
}
243+
244+
/**
245+
* Validate the exporter config for a metricType
246+
*
247+
* @param type {@link MetricType}
248+
* @param settings exporter settings
249+
*/
250+
public void validateExporterConfig(final MetricType type, final Settings settings) {
251+
if (topQueriesServices.containsKey(type)) {
252+
topQueriesServices.get(type).validateExporterConfig(settings);
253+
}
254+
}
255+
180256
@Override
181257
protected void doStart() {
182258
if (isEnabled()) {

‎plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/TopQueriesService.java

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@
3535
import java.util.stream.Collectors;
3636
import java.util.stream.Stream;
3737

38-
import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.DEFAULT_TOP_N_LATENCY_QUERIES_INDEX_PATTERN;
38+
import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.DEFAULT_TOP_N_QUERIES_INDEX_PATTERN;
3939
import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.DEFAULT_TOP_QUERIES_EXPORTER_TYPE;
4040
import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.EXPORTER_TYPE;
4141
import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.EXPORT_INDEX;
@@ -218,10 +218,7 @@ public void setExporter(final Settings settings) {
218218
if (settings.get(EXPORTER_TYPE) != null) {
219219
SinkType expectedType = SinkType.parse(settings.get(EXPORTER_TYPE, DEFAULT_TOP_QUERIES_EXPORTER_TYPE));
220220
if (exporter != null && expectedType == SinkType.getSinkTypeFromExporter(exporter)) {
221-
queryInsightsExporterFactory.updateExporter(
222-
exporter,
223-
settings.get(EXPORT_INDEX, DEFAULT_TOP_N_LATENCY_QUERIES_INDEX_PATTERN)
224-
);
221+
queryInsightsExporterFactory.updateExporter(exporter, settings.get(EXPORT_INDEX, DEFAULT_TOP_N_QUERIES_INDEX_PATTERN));
225222
} else {
226223
try {
227224
queryInsightsExporterFactory.closeExporter(this.exporter);
@@ -230,7 +227,7 @@ public void setExporter(final Settings settings) {
230227
}
231228
this.exporter = queryInsightsExporterFactory.createExporter(
232229
SinkType.parse(settings.get(EXPORTER_TYPE, DEFAULT_TOP_QUERIES_EXPORTER_TYPE)),
233-
settings.get(EXPORT_INDEX, DEFAULT_TOP_N_LATENCY_QUERIES_INDEX_PATTERN)
230+
settings.get(EXPORT_INDEX, DEFAULT_TOP_N_QUERIES_INDEX_PATTERN)
234231
);
235232
}
236233
} else {

‎plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model/Attribute.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ public enum Attribute {
4747
/**
4848
* Tasks level resource usages in this request
4949
*/
50-
TASKS_RESOURCE_USAGES,
50+
TASK_RESOURCE_USAGES,
5151
/**
5252
* Custom search request labels
5353
*/

0 commit comments

Comments
 (0)