Skip to content

Commit dfe0e9a

Browse files
committed
Do not request "search_pipelines" metrics by default in NodesInfoRequest
This commit introduces a breaking change: NodesInfoRequest does not request all metrics by default. There are metrics there are not included in the default set of metrics. Right now "search_pipelines" metric is not included in the default set of metrics. Signed-off-by: Lukáš Vlček <[email protected]>
1 parent 3125b94 commit dfe0e9a

File tree

4 files changed

+58
-11
lines changed

4 files changed

+58
-11
lines changed

CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
6464
- Add task completion count in search backpressure stats API ([#10028](https://github.com/opensearch-project/OpenSearch/pull/10028/))
6565
- Deprecate CamelCase `PathHierarchy` tokenizer name in favor to lowercase `path_hierarchy` ([#10894](https://github.com/opensearch-project/OpenSearch/pull/10894))
6666
- Switched to more reliable OpenSearch Lucene snapshot location([#11728](https://github.com/opensearch-project/OpenSearch/pull/11728))
67+
- Breaking change: Do not request "search_pipelines" metrics by default in NodesInfoRequest ([#12497](https://github.com/opensearch-project/OpenSearch/pull/12497))
6768

6869
### Deprecated
6970

server/src/main/java/org/opensearch/action/admin/cluster/node/info/NodesInfoRequest.java

+32-4
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@
5353
@PublicApi(since = "1.0.0")
5454
public class NodesInfoRequest extends BaseNodesRequest<NodesInfoRequest> {
5555

56-
private Set<String> requestedMetrics = Metric.allMetrics();
56+
private Set<String> requestedMetrics = Metric.defaultMetrics();
5757

5858
/**
5959
* Create a new NodeInfoRequest from a {@link StreamInput} object.
@@ -73,7 +73,7 @@ public NodesInfoRequest(StreamInput in) throws IOException {
7373
*/
7474
public NodesInfoRequest(String... nodesIds) {
7575
super(nodesIds);
76-
all();
76+
defaultMetrics();
7777
}
7878

7979
/**
@@ -85,13 +85,24 @@ public NodesInfoRequest clear() {
8585
}
8686

8787
/**
88-
* Sets to return all the data.
88+
* Sets to return data for all the metrics.
89+
* See {@link Metric}
8990
*/
9091
public NodesInfoRequest all() {
9192
requestedMetrics.addAll(Metric.allMetrics());
9293
return this;
9394
}
9495

96+
/**
97+
* Sets to return data for default metrics only.
98+
* See {@link Metric}
99+
* See {@link Metric#defaultMetrics()}.
100+
*/
101+
public NodesInfoRequest defaultMetrics() {
102+
requestedMetrics.addAll(Metric.defaultMetrics());
103+
return this;
104+
}
105+
95106
/**
96107
* Get the names of requested metrics
97108
*/
@@ -156,7 +167,7 @@ public void writeTo(StreamOutput out) throws IOException {
156167

157168
/**
158169
* An enumeration of the "core" sections of metrics that may be requested
159-
* from the nodes information endpoint. Eventually this list list will be
170+
* from the nodes information endpoint. Eventually this list will be
160171
* pluggable.
161172
*/
162173
public enum Metric {
@@ -187,8 +198,25 @@ boolean containedIn(Set<String> metricNames) {
187198
return metricNames.contains(this.metricName());
188199
}
189200

201+
/**
202+
* Return all available metrics.
203+
* See {@link Metric}
204+
*/
190205
public static Set<String> allMetrics() {
191206
return Arrays.stream(values()).map(Metric::metricName).collect(Collectors.toSet());
192207
}
208+
209+
/**
210+
* Return "the default" set of metrics.
211+
* Similar to {@link #allMetrics()} except {@link Metric#SEARCH_PIPELINES} metric is not included.
212+
* <br>
213+
* The motivation to define the default set of metrics was to keep the default response
214+
* size at bay. Metrics that are NOT included in the default set were typically introduced later
215+
* and are considered to contain specific type of information that is not usually useful unless you
216+
* know that you really need it.
217+
*/
218+
public static Set<String> defaultMetrics() {
219+
return allMetrics().stream().filter(metric -> !(metric.equals(SEARCH_PIPELINES.metricName()))).collect(Collectors.toSet());
220+
}
193221
}
194222
}

server/src/main/java/org/opensearch/action/search/PutSearchPipelineTransportAction.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ protected void clusterManagerOperation(
8282
ClusterState state,
8383
ActionListener<AcknowledgedResponse> listener
8484
) throws Exception {
85-
NodesInfoRequest nodesInfoRequest = new NodesInfoRequest();
85+
NodesInfoRequest nodesInfoRequest = new NodesInfoRequest().clear().addMetric(NodesInfoRequest.Metric.SEARCH_PIPELINES.metricName());
8686
client.admin().cluster().nodesInfo(nodesInfoRequest, ActionListener.wrap(nodeInfos -> {
8787
Map<DiscoveryNode, SearchPipelineInfo> searchPipelineInfos = new HashMap<>();
8888
for (NodeInfo nodeInfo : nodeInfos.getNodes()) {

server/src/test/java/org/opensearch/action/admin/cluster/node/info/NodesInfoRequestTests.java

+24-6
Original file line numberDiff line numberDiff line change
@@ -86,15 +86,18 @@ public void testRemoveSingleMetric() throws Exception {
8686
}
8787

8888
/**
89-
* Test that a newly constructed NodesInfoRequestObject requests all of the
90-
* possible metrics defined in {@link NodesInfoRequest.Metric}.
89+
* Test that a newly constructed NodesInfoRequestObject does not request all the
90+
* possible metrics defined in {@link NodesInfoRequest.Metric} but only the default metrics
91+
* according to {@link NodesInfoRequest.Metric#defaultMetrics()}.
9192
*/
9293
public void testNodesInfoRequestDefaults() {
93-
NodesInfoRequest defaultNodesInfoRequest = new NodesInfoRequest(randomAlphaOfLength(8));
94-
NodesInfoRequest allMetricsNodesInfoRequest = new NodesInfoRequest(randomAlphaOfLength(8));
95-
allMetricsNodesInfoRequest.all();
94+
NodesInfoRequest requestOOTB = new NodesInfoRequest(randomAlphaOfLength(8));
95+
NodesInfoRequest requestAll = new NodesInfoRequest(randomAlphaOfLength(8)).all();
96+
NodesInfoRequest requestDefault = new NodesInfoRequest(randomAlphaOfLength(8)).defaultMetrics();
9697

97-
assertThat(defaultNodesInfoRequest.requestedMetrics(), equalTo(allMetricsNodesInfoRequest.requestedMetrics()));
98+
assertTrue(requestAll.requestedMetrics().size() > requestOOTB.requestedMetrics().size());
99+
assertTrue(requestDefault.requestedMetrics().size() == requestOOTB.requestedMetrics().size());
100+
assertThat(requestOOTB.requestedMetrics(), equalTo(requestDefault.requestedMetrics()));
98101
}
99102

100103
/**
@@ -107,6 +110,21 @@ public void testNodesInfoRequestAll() throws Exception {
107110
assertThat(request.requestedMetrics(), equalTo(NodesInfoRequest.Metric.allMetrics()));
108111
}
109112

113+
/**
114+
* Test that the {@link NodesInfoRequest#defaultMetrics()} method enables default metrics.
115+
*/
116+
public void testNodesInfoRequestDefault() {
117+
NodesInfoRequest request = new NodesInfoRequest("node");
118+
request.defaultMetrics();
119+
120+
assertEquals(11, request.requestedMetrics().size());
121+
assertThat(request.requestedMetrics(), equalTo(NodesInfoRequest.Metric.defaultMetrics()));
122+
assertTrue(request.requestedMetrics().contains(NodesInfoRequest.Metric.JVM.metricName()));
123+
assertTrue(request.requestedMetrics().contains(NodesInfoRequest.Metric.AGGREGATIONS.metricName()));
124+
// search_pipelines metrics are not included
125+
assertFalse(request.requestedMetrics().contains(NodesInfoRequest.Metric.SEARCH_PIPELINES.metricName()));
126+
}
127+
110128
/**
111129
* Test that the {@link NodesInfoRequest#clear()} method disables all metrics.
112130
*/

0 commit comments

Comments
 (0)