Do not request "search_pipelines" metrics by default in NodesInfoRequ…

…est (opensearch-project#12497)

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]>
Signed-off-by: Shivansh Arora <[email protected]>

CHANGELOG.md

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

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

@@ -53,7 +53,7 @@
 @PublicApi(since = "1.0.0")
 public class NodesInfoRequest extends BaseNodesRequest<NodesInfoRequest> {
 
-    private Set<String> requestedMetrics = Metric.allMetrics();
+    private Set<String> requestedMetrics = Metric.defaultMetrics();
 
     /**
      * Create a new NodeInfoRequest from a {@link StreamInput} object.
@@ -73,7 +73,7 @@ public NodesInfoRequest(StreamInput in) throws IOException {
      */
     public NodesInfoRequest(String... nodesIds) {
         super(nodesIds);
-        all();
+        defaultMetrics();
     }
 
     /**
@@ -85,13 +85,24 @@ public NodesInfoRequest clear() {
     }
 
     /**
-     * Sets to return all the data.
+     * Sets to return data for all the metrics.
+     * See {@link Metric}
      */
     public NodesInfoRequest all() {
         requestedMetrics.addAll(Metric.allMetrics());
         return this;
     }
 
+    /**
+     * Sets to return data for default metrics only.
+     * See {@link Metric}
+     * See {@link Metric#defaultMetrics()}.
+     */
+    public NodesInfoRequest defaultMetrics() {
+        requestedMetrics.addAll(Metric.defaultMetrics());
+        return this;
+    }
+
     /**
      * Get the names of requested metrics
      */
@@ -156,7 +167,7 @@ public void writeTo(StreamOutput out) throws IOException {
 
     /**
      * An enumeration of the "core" sections of metrics that may be requested
-     * from the nodes information endpoint. Eventually this list list will be
+     * from the nodes information endpoint. Eventually this list will be
      * pluggable.
      */
     public enum Metric {
@@ -187,8 +198,25 @@ boolean containedIn(Set<String> metricNames) {
             return metricNames.contains(this.metricName());
         }
 
+        /**
+         * Return all available metrics.
+         * See {@link Metric}
+         */
         public static Set<String> allMetrics() {
             return Arrays.stream(values()).map(Metric::metricName).collect(Collectors.toSet());
         }
+
+        /**
+         * Return "the default" set of metrics.
+         * Similar to {@link #allMetrics()} except {@link Metric#SEARCH_PIPELINES} metric is not included.
+         * <br>
+         * The motivation to define the default set of metrics was to keep the default response
+         * size at bay. Metrics that are NOT included in the default set were typically introduced later
+         * and are considered to contain specific type of information that is not usually useful unless you
+         * know that you really need it.
+         */
+        public static Set<String> defaultMetrics() {
+            return allMetrics().stream().filter(metric -> !(metric.equals(SEARCH_PIPELINES.metricName()))).collect(Collectors.toSet());
+        }
     }
 }

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

@@ -82,7 +82,7 @@ protected void clusterManagerOperation(
         ClusterState state,
         ActionListener<AcknowledgedResponse> listener
     ) throws Exception {
-        NodesInfoRequest nodesInfoRequest = new NodesInfoRequest();
+        NodesInfoRequest nodesInfoRequest = new NodesInfoRequest().clear().addMetric(NodesInfoRequest.Metric.SEARCH_PIPELINES.metricName());
         client.admin().cluster().nodesInfo(nodesInfoRequest, ActionListener.wrap(nodeInfos -> {
             Map<DiscoveryNode, SearchPipelineInfo> searchPipelineInfos = new HashMap<>();
             for (NodeInfo nodeInfo : nodeInfos.getNodes()) {

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

@@ -86,15 +86,18 @@ public void testRemoveSingleMetric() throws Exception {
     }
 
     /**
-     * Test that a newly constructed NodesInfoRequestObject requests all of the
-     * possible metrics defined in {@link NodesInfoRequest.Metric}.
+     * Test that a newly constructed NodesInfoRequestObject does not request all the
+     * possible metrics defined in {@link NodesInfoRequest.Metric} but only the default metrics
+     * according to {@link NodesInfoRequest.Metric#defaultMetrics()}.
      */
     public void testNodesInfoRequestDefaults() {
-        NodesInfoRequest defaultNodesInfoRequest = new NodesInfoRequest(randomAlphaOfLength(8));
-        NodesInfoRequest allMetricsNodesInfoRequest = new NodesInfoRequest(randomAlphaOfLength(8));
-        allMetricsNodesInfoRequest.all();
+        NodesInfoRequest requestOOTB = new NodesInfoRequest(randomAlphaOfLength(8));
+        NodesInfoRequest requestAll = new NodesInfoRequest(randomAlphaOfLength(8)).all();
+        NodesInfoRequest requestDefault = new NodesInfoRequest(randomAlphaOfLength(8)).defaultMetrics();
 
-        assertThat(defaultNodesInfoRequest.requestedMetrics(), equalTo(allMetricsNodesInfoRequest.requestedMetrics()));
+        assertTrue(requestAll.requestedMetrics().size() > requestOOTB.requestedMetrics().size());
+        assertTrue(requestDefault.requestedMetrics().size() == requestOOTB.requestedMetrics().size());
+        assertThat(requestOOTB.requestedMetrics(), equalTo(requestDefault.requestedMetrics()));
     }
 
     /**
@@ -107,6 +110,21 @@ public void testNodesInfoRequestAll() throws Exception {
         assertThat(request.requestedMetrics(), equalTo(NodesInfoRequest.Metric.allMetrics()));
     }
 
+    /**
+     * Test that the {@link NodesInfoRequest#defaultMetrics()} method enables default metrics.
+     */
+    public void testNodesInfoRequestDefault() {
+        NodesInfoRequest request = new NodesInfoRequest("node");
+        request.defaultMetrics();
+
+        assertEquals(11, request.requestedMetrics().size());
+        assertThat(request.requestedMetrics(), equalTo(NodesInfoRequest.Metric.defaultMetrics()));
+        assertTrue(request.requestedMetrics().contains(NodesInfoRequest.Metric.JVM.metricName()));
+        assertTrue(request.requestedMetrics().contains(NodesInfoRequest.Metric.AGGREGATIONS.metricName()));
+        // search_pipelines metrics are not included
+        assertFalse(request.requestedMetrics().contains(NodesInfoRequest.Metric.SEARCH_PIPELINES.metricName()));
+    }
+
     /**
      * Test that the {@link NodesInfoRequest#clear()} method disables all metrics.
      */