Serialize NodesStatsRequest as set of strings (#53235)

* Add unit tests before refactoring

* Convert boolean fields to set of strings

In order to make nodes stats plugins pluggable, we need to make the
NodesStatsRequest class capable of carrying a flexible list of metrics
rather than a fixed list of boolean flags. This commit changes the
internal storage of the class without changing its serialization.

* Rename test class to match conventions

* Change serialization of NodesStatsRequest

* Set up BWC before merging

* Singularize enum name

Author: williamrandolph

build.gradle

@@ -220,8 +220,8 @@ task verifyVersions {
  * after the backport of the backcompat code is complete.
  */
 
-boolean bwc_tests_enabled = true
-final String bwc_tests_disabled_issue = "" /* place a PR link here when committing bwc changes */
+boolean bwc_tests_enabled = false
+final String bwc_tests_disabled_issue = "https://github.com/elastic/elasticsearch/pull/53235" /* place a PR link here when committing bwc changes */
 if (bwc_tests_enabled == false) {
   if (bwc_tests_disabled_issue.isEmpty()) {
     throw new GradleException("bwc_tests_disabled_issue must be set when bwc_tests_enabled == false")

server/src/main/java/org/elasticsearch/action/admin/cluster/node/stats/NodesStatsRequest.java

@@ -19,51 +19,51 @@
 
 package org.elasticsearch.action.admin.cluster.node.stats;
 
+import org.elasticsearch.Version;
 import org.elasticsearch.action.admin.indices.stats.CommonStatsFlags;
 import org.elasticsearch.action.support.nodes.BaseNodesRequest;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 
 import java.io.IOException;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Set;
+import java.util.stream.Collectors;
 
 /**
  * A request to get node (cluster) level stats.
  */
 public class NodesStatsRequest extends BaseNodesRequest<NodesStatsRequest> {
 
     private CommonStatsFlags indices = new CommonStatsFlags();
-    private boolean os;
-    private boolean process;
-    private boolean jvm;
-    private boolean threadPool;
-    private boolean fs;
-    private boolean transport;
-    private boolean http;
-    private boolean breaker;
-    private boolean script;
-    private boolean discovery;
-    private boolean ingest;
-    private boolean adaptiveSelection;
+    private final Set<String> requestedMetrics = new HashSet<>();
 
     public NodesStatsRequest() {
         super((String[]) null);
     }
 
     public NodesStatsRequest(StreamInput in) throws IOException {
         super(in);
+
         indices = new CommonStatsFlags(in);
-        os = in.readBoolean();
-        process = in.readBoolean();
-        jvm = in.readBoolean();
-        threadPool = in.readBoolean();
-        fs = in.readBoolean();
-        transport = in.readBoolean();
-        http = in.readBoolean();
-        breaker = in.readBoolean();
-        script = in.readBoolean();
-        discovery = in.readBoolean();
-        ingest = in.readBoolean();
-        adaptiveSelection = in.readBoolean();
+        requestedMetrics.clear();
+        if (in.getVersion().before(Version.V_7_7_0)) {
+            addOrRemoveMetric(in.readBoolean(), Metric.OS.metricName());
+            addOrRemoveMetric(in.readBoolean(), Metric.PROCESS.metricName());
+            addOrRemoveMetric(in.readBoolean(), Metric.JVM.metricName());
+            addOrRemoveMetric(in.readBoolean(), Metric.THREAD_POOL.metricName());
+            addOrRemoveMetric(in.readBoolean(), Metric.FS.metricName());
+            addOrRemoveMetric(in.readBoolean(), Metric.TRANSPORT.metricName());
+            addOrRemoveMetric(in.readBoolean(), Metric.HTTP.metricName());
+            addOrRemoveMetric(in.readBoolean(), Metric.BREAKER.metricName());
+            addOrRemoveMetric(in.readBoolean(), Metric.SCRIPT.metricName());
+            addOrRemoveMetric(in.readBoolean(), Metric.DISCOVERY.metricName());
+            addOrRemoveMetric(in.readBoolean(), Metric.INGEST.metricName());
+            addOrRemoveMetric(in.readBoolean(), Metric.ADAPTIVE_SELECTION.metricName());
+        } else {
+            requestedMetrics.addAll(in.readStringList());
+        }
     }
 
     /**
@@ -79,18 +79,7 @@ public NodesStatsRequest(String... nodesIds) {
      */
     public NodesStatsRequest all() {
         this.indices.all();
-        this.os = true;
-        this.process = true;
-        this.jvm = true;
-        this.threadPool = true;
-        this.fs = true;
-        this.transport = true;
-        this.http = true;
-        this.breaker = true;
-        this.script = true;
-        this.discovery = true;
-        this.ingest = true;
-        this.adaptiveSelection = true;
+        this.requestedMetrics.addAll(Metric.allMetrics());
         return this;
     }
 
@@ -99,18 +88,7 @@ public NodesStatsRequest all() {
      */
     public NodesStatsRequest clear() {
         this.indices.clear();
-        this.os = false;
-        this.process = false;
-        this.jvm = false;
-        this.threadPool = false;
-        this.fs = false;
-        this.transport = false;
-        this.http = false;
-        this.breaker = false;
-        this.script = false;
-        this.discovery = false;
-        this.ingest = false;
-        this.adaptiveSelection = false;
+        this.requestedMetrics.clear();
         return this;
     }
 
@@ -139,180 +117,234 @@ public NodesStatsRequest indices(boolean indices) {
      * Should the node OS be returned.
      */
     public boolean os() {
-        return this.os;
+        return Metric.OS.containedIn(requestedMetrics);
     }
 
     /**
      * Should the node OS be returned.
      */
     public NodesStatsRequest os(boolean os) {
-        this.os = os;
+        addOrRemoveMetric(os, Metric.OS.metricName());
         return this;
     }
 
     /**
      * Should the node Process be returned.
      */
     public boolean process() {
-        return this.process;
+        return Metric.PROCESS.containedIn(requestedMetrics);
     }
 
     /**
      * Should the node Process be returned.
      */
     public NodesStatsRequest process(boolean process) {
-        this.process = process;
+        addOrRemoveMetric(process, Metric.PROCESS.metricName());
         return this;
     }
 
     /**
      * Should the node JVM be returned.
      */
     public boolean jvm() {
-        return this.jvm;
+        return Metric.JVM.containedIn(requestedMetrics);
     }
 
     /**
      * Should the node JVM be returned.
      */
     public NodesStatsRequest jvm(boolean jvm) {
-        this.jvm = jvm;
+        addOrRemoveMetric(jvm, Metric.JVM.metricName());
         return this;
     }
 
     /**
      * Should the node Thread Pool be returned.
      */
     public boolean threadPool() {
-        return this.threadPool;
+        return Metric.THREAD_POOL.containedIn(requestedMetrics);
     }
 
     /**
      * Should the node Thread Pool be returned.
      */
     public NodesStatsRequest threadPool(boolean threadPool) {
-        this.threadPool = threadPool;
+        addOrRemoveMetric(threadPool, Metric.THREAD_POOL.metricName());
         return this;
     }
 
     /**
      * Should the node file system stats be returned.
      */
     public boolean fs() {
-        return this.fs;
+        return Metric.FS.containedIn(requestedMetrics);
     }
 
     /**
      * Should the node file system stats be returned.
      */
     public NodesStatsRequest fs(boolean fs) {
-        this.fs = fs;
+        addOrRemoveMetric(fs, Metric.FS.metricName());
         return this;
     }
 
     /**
      * Should the node Transport be returned.
      */
     public boolean transport() {
-        return this.transport;
+        return Metric.TRANSPORT.containedIn(requestedMetrics);
     }
 
     /**
      * Should the node Transport be returned.
      */
     public NodesStatsRequest transport(boolean transport) {
-        this.transport = transport;
+        addOrRemoveMetric(transport, Metric.TRANSPORT.metricName());
         return this;
     }
 
     /**
      * Should the node HTTP be returned.
      */
     public boolean http() {
-        return this.http;
+        return Metric.HTTP.containedIn(requestedMetrics);
     }
 
     /**
      * Should the node HTTP be returned.
      */
     public NodesStatsRequest http(boolean http) {
-        this.http = http;
+        addOrRemoveMetric(http, Metric.HTTP.metricName());
         return this;
     }
 
     public boolean breaker() {
-        return this.breaker;
+        return Metric.BREAKER.containedIn(requestedMetrics);
     }
 
     /**
      * Should the node's circuit breaker stats be returned.
      */
     public NodesStatsRequest breaker(boolean breaker) {
-        this.breaker = breaker;
+        addOrRemoveMetric(breaker, Metric.BREAKER.metricName());
         return this;
     }
 
     public boolean script() {
-        return script;
+        return Metric.SCRIPT.containedIn(requestedMetrics);
     }
 
     public NodesStatsRequest script(boolean script) {
-        this.script = script;
+        addOrRemoveMetric(script, Metric.SCRIPT.metricName());
         return this;
     }
 
 
     public boolean discovery() {
-        return this.discovery;
+        return Metric.DISCOVERY.containedIn(requestedMetrics);
     }
 
     /**
      * Should the node's discovery stats be returned.
      */
     public NodesStatsRequest discovery(boolean discovery) {
-        this.discovery = discovery;
+        addOrRemoveMetric(discovery, Metric.DISCOVERY.metricName());
         return this;
     }
 
     public boolean ingest() {
-        return ingest;
+        return Metric.INGEST.containedIn(requestedMetrics);
     }
 
     /**
      * Should ingest statistics be returned.
      */
     public NodesStatsRequest ingest(boolean ingest) {
-        this.ingest = ingest;
+        addOrRemoveMetric(ingest, Metric.INGEST.metricName());
         return this;
     }
 
     public boolean adaptiveSelection() {
-        return adaptiveSelection;
+        return Metric.ADAPTIVE_SELECTION.containedIn(requestedMetrics);
     }
 
     /**
      * Should adaptiveSelection statistics be returned.
      */
     public NodesStatsRequest adaptiveSelection(boolean adaptiveSelection) {
-        this.adaptiveSelection = adaptiveSelection;
+        addOrRemoveMetric(adaptiveSelection, Metric.ADAPTIVE_SELECTION.metricName());
         return this;
     }
 
+    /**
+     * Helper method for adding and removing metrics.
+     * @param includeMetric Whether or not to include a metric.
+     * @param metricName Name of the metric to include or remove.
+     */
+    private void addOrRemoveMetric(boolean includeMetric, String metricName) {
+        if (includeMetric) {
+            requestedMetrics.add(metricName);
+        } else {
+            requestedMetrics.remove(metricName);
+        }
+    }
+
     @Override
     public void writeTo(StreamOutput out) throws IOException {
         super.writeTo(out);
         indices.writeTo(out);
-        out.writeBoolean(os);
-        out.writeBoolean(process);
-        out.writeBoolean(jvm);
-        out.writeBoolean(threadPool);
-        out.writeBoolean(fs);
-        out.writeBoolean(transport);
-        out.writeBoolean(http);
-        out.writeBoolean(breaker);
-        out.writeBoolean(script);
-        out.writeBoolean(discovery);
-        out.writeBoolean(ingest);
-        out.writeBoolean(adaptiveSelection);
+        if (out.getVersion().before(Version.V_7_7_0)) {
+            out.writeBoolean(Metric.OS.containedIn(requestedMetrics));
+            out.writeBoolean(Metric.PROCESS.containedIn(requestedMetrics));
+            out.writeBoolean(Metric.JVM.containedIn(requestedMetrics));
+            out.writeBoolean(Metric.THREAD_POOL.containedIn(requestedMetrics));
+            out.writeBoolean(Metric.FS.containedIn(requestedMetrics));
+            out.writeBoolean(Metric.TRANSPORT.containedIn(requestedMetrics));
+            out.writeBoolean(Metric.HTTP.containedIn(requestedMetrics));
+            out.writeBoolean(Metric.BREAKER.containedIn(requestedMetrics));
+            out.writeBoolean(Metric.SCRIPT.containedIn(requestedMetrics));
+            out.writeBoolean(Metric.DISCOVERY.containedIn(requestedMetrics));
+            out.writeBoolean(Metric.INGEST.containedIn(requestedMetrics));
+            out.writeBoolean(Metric.ADAPTIVE_SELECTION.containedIn(requestedMetrics));
+        } else {
+            out.writeStringArray(requestedMetrics.toArray(String[]::new));
+        }
+    }
+
+    /**
+     * An enumeration of the "core" sections of metrics that may be requested
+     * from the nodes stats endpoint. Eventually this list will be pluggable.
+     */
+    private enum Metric {
+        OS("os"),
+        PROCESS("process"),
+        JVM("jvm"),
+        THREAD_POOL("threadPool"),
+        FS("fs"),
+        TRANSPORT("transport"),
+        HTTP("http"),
+        BREAKER("breaker"),
+        SCRIPT("script"),
+        DISCOVERY("discovery"),
+        INGEST("ingest"),
+        ADAPTIVE_SELECTION("adaptiveSelection");
+
+        private String metricName;
+
+        Metric(String name) {
+            this.metricName = name;
+        }
+
+        String metricName() {
+            return this.metricName;
+        }
+
+        boolean containedIn(Set<String> metricNames) {
+            return metricNames.contains(this.metricName());
+        }
+
+        static Set<String> allMetrics() {
+            return Arrays.stream(values()).map(Metric::metricName).collect(Collectors.toSet());
+        }
     }
 }