diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/node/info/NodesInfoRequest.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/node/info/NodesInfoRequest.java index 6f33f75692294..9ed72f2fb95be 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/node/info/NodesInfoRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/node/info/NodesInfoRequest.java @@ -27,6 +27,8 @@ import java.io.IOException; import java.util.Arrays; import java.util.Set; +import java.util.SortedSet; +import java.util.TreeSet; import java.util.stream.Collectors; /** @@ -34,7 +36,7 @@ */ public class NodesInfoRequest extends BaseNodesRequest { - private Set requestedMetrics = Metrics.allMetrics(); + private Set requestedMetrics = Metric.allMetrics(); /** * Create a new NodeInfoRequest from a {@link StreamInput} object. @@ -48,16 +50,16 @@ public NodesInfoRequest(StreamInput in) throws IOException { if (in.getVersion().before(Version.V_7_7_0)){ // prior to version 8.x, a NodesInfoRequest was serialized as a list // of booleans in a fixed order - addOrRemoveMetric(in.readBoolean(), Metrics.SETTINGS.metricName()); - addOrRemoveMetric(in.readBoolean(), Metrics.OS.metricName()); - addOrRemoveMetric(in.readBoolean(), Metrics.PROCESS.metricName()); - addOrRemoveMetric(in.readBoolean(), Metrics.JVM.metricName()); - addOrRemoveMetric(in.readBoolean(), Metrics.THREAD_POOL.metricName()); - addOrRemoveMetric(in.readBoolean(), Metrics.TRANSPORT.metricName()); - addOrRemoveMetric(in.readBoolean(), Metrics.HTTP.metricName()); - addOrRemoveMetric(in.readBoolean(), Metrics.PLUGINS.metricName()); - addOrRemoveMetric(in.readBoolean(), Metrics.INGEST.metricName()); - addOrRemoveMetric(in.readBoolean(), Metrics.INDICES.metricName()); + optionallyAddMetric(in.readBoolean(), Metric.SETTINGS.metricName()); + optionallyAddMetric(in.readBoolean(), Metric.OS.metricName()); + optionallyAddMetric(in.readBoolean(), Metric.PROCESS.metricName()); + optionallyAddMetric(in.readBoolean(), Metric.JVM.metricName()); + optionallyAddMetric(in.readBoolean(), Metric.THREAD_POOL.metricName()); + optionallyAddMetric(in.readBoolean(), Metric.TRANSPORT.metricName()); + optionallyAddMetric(in.readBoolean(), Metric.HTTP.metricName()); + optionallyAddMetric(in.readBoolean(), Metric.PLUGINS.metricName()); + optionallyAddMetric(in.readBoolean(), Metric.INGEST.metricName()); + optionallyAddMetric(in.readBoolean(), Metric.INDICES.metricName()); } else { requestedMetrics.addAll(Arrays.asList(in.readStringArray())); } @@ -84,174 +86,63 @@ public NodesInfoRequest clear() { * Sets to return all the data. */ public NodesInfoRequest all() { - requestedMetrics.addAll(Metrics.allMetrics()); + requestedMetrics.addAll(Metric.allMetrics()); return this; } /** - * Should the node settings be returned. + * Get the names of requested metrics */ - public boolean settings() { - return Metrics.SETTINGS.containedIn(requestedMetrics); + public Set requestedMetrics() { + return Set.copyOf(requestedMetrics); } /** - * Should the node settings be returned. + * Add metric */ - public NodesInfoRequest settings(boolean settings) { - addOrRemoveMetric(settings, Metrics.SETTINGS.metricName()); - return this; - } - - /** - * Should the node OS be returned. - */ - public boolean os() { - return Metrics.OS.containedIn(requestedMetrics); - } - - /** - * Should the node OS be returned. - */ - public NodesInfoRequest os(boolean os) { - addOrRemoveMetric(os, Metrics.OS.metricName()); - return this; - } - - /** - * Should the node Process be returned. - */ - public boolean process() { - return Metrics.PROCESS.containedIn(requestedMetrics); - } - - /** - * Should the node Process be returned. - */ - public NodesInfoRequest process(boolean process) { - addOrRemoveMetric(process, Metrics.PROCESS.metricName()); - return this; - } - - /** - * Should the node JVM be returned. - */ - public boolean jvm() { - return Metrics.JVM.containedIn(requestedMetrics); - } - - /** - * Should the node JVM be returned. - */ - public NodesInfoRequest jvm(boolean jvm) { - addOrRemoveMetric(jvm, Metrics.JVM.metricName()); - return this; - } - - /** - * Should the node Thread Pool info be returned. - */ - public boolean threadPool() { - return Metrics.THREAD_POOL.containedIn(requestedMetrics); - } - - /** - * Should the node Thread Pool info be returned. - */ - public NodesInfoRequest threadPool(boolean threadPool) { - addOrRemoveMetric(threadPool, Metrics.THREAD_POOL.metricName()); - return this; - } - - /** - * Should the node Transport be returned. - */ - public boolean transport() { - return Metrics.TRANSPORT.containedIn(requestedMetrics); - } - - /** - * Should the node Transport be returned. - */ - public NodesInfoRequest transport(boolean transport) { - addOrRemoveMetric(transport, Metrics.TRANSPORT.metricName()); - return this; - } - - /** - * Should the node HTTP be returned. - */ - public boolean http() { - return Metrics.HTTP.containedIn(requestedMetrics); - } - - /** - * Should the node HTTP be returned. - */ - public NodesInfoRequest http(boolean http) { - addOrRemoveMetric(http, Metrics.HTTP.metricName()); - return this; - } - - /** - * Should information about plugins be returned - * @param plugins true if you want info - * @return The request - */ - public NodesInfoRequest plugins(boolean plugins) { - addOrRemoveMetric(plugins, Metrics.PLUGINS.metricName()); + public NodesInfoRequest addMetric(String metric) { + if (Metric.allMetrics().contains(metric) == false) { + throw new IllegalStateException("Used an illegal metric: " + metric); + } + requestedMetrics.add(metric); return this; } /** - * @return true if information about plugins is requested - */ - public boolean plugins() { - return Metrics.PLUGINS.containedIn(requestedMetrics); - } - - /** - * Should information about ingest be returned - * @param ingest true if you want info + * Add multiple metrics */ - public NodesInfoRequest ingest(boolean ingest) { - addOrRemoveMetric(ingest, Metrics.INGEST.metricName()); + public NodesInfoRequest addMetrics(String... metrics) { + SortedSet metricsSet = new TreeSet<>(Set.of(metrics)); + if (Metric.allMetrics().containsAll(metricsSet) == false) { + metricsSet.removeAll(Metric.allMetrics()); + String plural = metricsSet.size() == 1 ? "" : "s"; + throw new IllegalStateException("Used illegal metric" + plural + ": " + metricsSet); + } + requestedMetrics.addAll(metricsSet); return this; } /** - * @return true if information about ingest is requested + * Remove metric */ - public boolean ingest() { - return Metrics.INGEST.containedIn(requestedMetrics); - } - - /** - * Should information about indices (currently just indexing buffers) be returned - * @param indices true if you want info - */ - public NodesInfoRequest indices(boolean indices) { - addOrRemoveMetric(indices, Metrics.INDICES.metricName()); + public NodesInfoRequest removeMetric(String metric) { + if (Metric.allMetrics().contains(metric) == false) { + throw new IllegalStateException("Used an illegal metric: " + metric); + } + requestedMetrics.remove(metric); return this; } /** - * @return true if information about indices (currently just indexing buffers) - */ - public boolean indices() { - return Metrics.INDICES.containedIn(requestedMetrics); - } - - /** - * Helper method for adding and removing metrics. - * @param includeMetric Whether or not to include a metric. + * Helper method for adding and removing metrics. Used when deserializing + * a NodesInfoRequest from an ordered list of booleans. + * + * @param addMetric 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) { + private void optionallyAddMetric(boolean addMetric, String metricName) { + if (addMetric) { requestedMetrics.add(metricName); - } else { - requestedMetrics.remove(metricName); } } @@ -261,16 +152,16 @@ public void writeTo(StreamOutput out) throws IOException { if (out.getVersion().before(Version.V_7_7_0)){ // prior to version 8.x, a NodesInfoRequest was serialized as a list // of booleans in a fixed order - out.writeBoolean(Metrics.SETTINGS.containedIn(requestedMetrics)); - out.writeBoolean(Metrics.OS.containedIn(requestedMetrics)); - out.writeBoolean(Metrics.PROCESS.containedIn(requestedMetrics)); - out.writeBoolean(Metrics.JVM.containedIn(requestedMetrics)); - out.writeBoolean(Metrics.THREAD_POOL.containedIn(requestedMetrics)); - out.writeBoolean(Metrics.TRANSPORT.containedIn(requestedMetrics)); - out.writeBoolean(Metrics.HTTP.containedIn(requestedMetrics)); - out.writeBoolean(Metrics.PLUGINS.containedIn(requestedMetrics)); - out.writeBoolean(Metrics.INGEST.containedIn(requestedMetrics)); - out.writeBoolean(Metrics.INDICES.containedIn(requestedMetrics)); + out.writeBoolean(Metric.SETTINGS.containedIn(requestedMetrics)); + 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.TRANSPORT.containedIn(requestedMetrics)); + out.writeBoolean(Metric.HTTP.containedIn(requestedMetrics)); + out.writeBoolean(Metric.PLUGINS.containedIn(requestedMetrics)); + out.writeBoolean(Metric.INGEST.containedIn(requestedMetrics)); + out.writeBoolean(Metric.INDICES.containedIn(requestedMetrics)); } else { out.writeStringArray(requestedMetrics.toArray(String[]::new)); } @@ -281,7 +172,7 @@ public void writeTo(StreamOutput out) throws IOException { * from the nodes information endpoint. Eventually this list list will be * pluggable. */ - enum Metrics { + public enum Metric { SETTINGS("settings"), OS("os"), PROCESS("process"), @@ -295,11 +186,11 @@ enum Metrics { private String metricName; - Metrics(String name) { + Metric(String name) { this.metricName = name; } - String metricName() { + public String metricName() { return this.metricName; } @@ -307,8 +198,8 @@ boolean containedIn(Set metricNames) { return metricNames.contains(this.metricName()); } - static Set allMetrics() { - return Arrays.stream(values()).map(Metrics::metricName).collect(Collectors.toSet()); + public static Set allMetrics() { + return Arrays.stream(values()).map(Metric::metricName).collect(Collectors.toSet()); } } } diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/node/info/NodesInfoRequestBuilder.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/node/info/NodesInfoRequestBuilder.java index 1222101616069..97b55b7fd496a 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/node/info/NodesInfoRequestBuilder.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/node/info/NodesInfoRequestBuilder.java @@ -22,6 +22,7 @@ import org.elasticsearch.action.support.nodes.NodesOperationRequestBuilder; import org.elasticsearch.client.ElasticsearchClient; +// TODO: This class's interface should match that of NodesInfoRequest public class NodesInfoRequestBuilder extends NodesOperationRequestBuilder { public NodesInfoRequestBuilder(ElasticsearchClient client, NodesInfoAction action) { @@ -48,7 +49,7 @@ public NodesInfoRequestBuilder all() { * Should the node settings be returned. */ public NodesInfoRequestBuilder setSettings(boolean settings) { - request.settings(settings); + addOrRemoveMetric(settings, NodesInfoRequest.Metric.SETTINGS); return this; } @@ -56,7 +57,7 @@ public NodesInfoRequestBuilder setSettings(boolean settings) { * Should the node OS info be returned. */ public NodesInfoRequestBuilder setOs(boolean os) { - request.os(os); + addOrRemoveMetric(os, NodesInfoRequest.Metric.OS); return this; } @@ -64,7 +65,7 @@ public NodesInfoRequestBuilder setOs(boolean os) { * Should the node OS process be returned. */ public NodesInfoRequestBuilder setProcess(boolean process) { - request.process(process); + addOrRemoveMetric(process, NodesInfoRequest.Metric.PROCESS); return this; } @@ -72,7 +73,7 @@ public NodesInfoRequestBuilder setProcess(boolean process) { * Should the node JVM info be returned. */ public NodesInfoRequestBuilder setJvm(boolean jvm) { - request.jvm(jvm); + addOrRemoveMetric(jvm, NodesInfoRequest.Metric.JVM); return this; } @@ -80,7 +81,7 @@ public NodesInfoRequestBuilder setJvm(boolean jvm) { * Should the node thread pool info be returned. */ public NodesInfoRequestBuilder setThreadPool(boolean threadPool) { - request.threadPool(threadPool); + addOrRemoveMetric(threadPool, NodesInfoRequest.Metric.THREAD_POOL); return this; } @@ -88,7 +89,7 @@ public NodesInfoRequestBuilder setThreadPool(boolean threadPool) { * Should the node Transport info be returned. */ public NodesInfoRequestBuilder setTransport(boolean transport) { - request.transport(transport); + addOrRemoveMetric(transport, NodesInfoRequest.Metric.TRANSPORT); return this; } @@ -96,7 +97,7 @@ public NodesInfoRequestBuilder setTransport(boolean transport) { * Should the node HTTP info be returned. */ public NodesInfoRequestBuilder setHttp(boolean http) { - request.http(http); + addOrRemoveMetric(http, NodesInfoRequest.Metric.HTTP); return this; } @@ -104,7 +105,7 @@ public NodesInfoRequestBuilder setHttp(boolean http) { * Should the node plugins info be returned. */ public NodesInfoRequestBuilder setPlugins(boolean plugins) { - request().plugins(plugins); + addOrRemoveMetric(plugins, NodesInfoRequest.Metric.PLUGINS); return this; } @@ -112,7 +113,7 @@ public NodesInfoRequestBuilder setPlugins(boolean plugins) { * Should the node ingest info be returned. */ public NodesInfoRequestBuilder setIngest(boolean ingest) { - request().ingest(ingest); + addOrRemoveMetric(ingest, NodesInfoRequest.Metric.INGEST); return this; } @@ -120,7 +121,15 @@ public NodesInfoRequestBuilder setIngest(boolean ingest) { * Should the node indices info be returned. */ public NodesInfoRequestBuilder setIndices(boolean indices) { - request().indices(indices); + addOrRemoveMetric(indices, NodesInfoRequest.Metric.INDICES); return this; } + + private void addOrRemoveMetric(boolean includeMetric, NodesInfoRequest.Metric metric) { + if (includeMetric) { + request.addMetric(metric.metricName()); + } else { + request.removeMetric(metric.metricName()); + } + } } diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/node/info/TransportNodesInfoAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/node/info/TransportNodesInfoAction.java index cd960d75da3a4..dcfdfedda125f 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/node/info/TransportNodesInfoAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/node/info/TransportNodesInfoAction.java @@ -34,6 +34,7 @@ import java.io.IOException; import java.util.List; +import java.util.Set; public class TransportNodesInfoAction extends TransportNodesAction metrics = request.requestedMetrics(); + return nodeService.info( + metrics.contains(NodesInfoRequest.Metric.SETTINGS.metricName()), + metrics.contains(NodesInfoRequest.Metric.OS.metricName()), + metrics.contains(NodesInfoRequest.Metric.PROCESS.metricName()), + metrics.contains(NodesInfoRequest.Metric.JVM.metricName()), + metrics.contains(NodesInfoRequest.Metric.THREAD_POOL.metricName()), + metrics.contains(NodesInfoRequest.Metric.TRANSPORT.metricName()), + metrics.contains(NodesInfoRequest.Metric.HTTP.metricName()), + metrics.contains(NodesInfoRequest.Metric.PLUGINS.metricName()), + metrics.contains(NodesInfoRequest.Metric.INGEST.metricName()), + metrics.contains(NodesInfoRequest.Metric.INDICES.metricName())); } public static class NodeInfoRequest extends BaseNodeRequest { diff --git a/server/src/main/java/org/elasticsearch/action/ingest/PutPipelineTransportAction.java b/server/src/main/java/org/elasticsearch/action/ingest/PutPipelineTransportAction.java index 7250369fae4be..c51f09c2da0dc 100644 --- a/server/src/main/java/org/elasticsearch/action/ingest/PutPipelineTransportAction.java +++ b/server/src/main/java/org/elasticsearch/action/ingest/PutPipelineTransportAction.java @@ -74,8 +74,8 @@ protected AcknowledgedResponse read(StreamInput in) throws IOException { protected void masterOperation(Task task, PutPipelineRequest request, ClusterState state, ActionListener listener) throws Exception { NodesInfoRequest nodesInfoRequest = new NodesInfoRequest(); - nodesInfoRequest.clear(); - nodesInfoRequest.ingest(true); + nodesInfoRequest.clear() + .addMetric(NodesInfoRequest.Metric.INGEST.metricName()); client.admin().cluster().nodesInfo(nodesInfoRequest, ActionListener.wrap(nodeInfos -> { Map ingestInfos = new HashMap<>(); for (NodeInfo nodeInfo : nodeInfos.getNodes()) { diff --git a/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestNodesInfoAction.java b/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestNodesInfoAction.java index 12920599fbd19..363a17978843e 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestNodesInfoAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestNodesInfoAction.java @@ -36,17 +36,7 @@ import static org.elasticsearch.rest.RestRequest.Method.GET; public class RestNodesInfoAction extends BaseRestHandler { - static final Set ALLOWED_METRICS = Sets.newHashSet( - "http", - "ingest", - "indices", - "jvm", - "os", - "plugins", - "process", - "settings", - "thread_pool", - "transport"); + static final Set ALLOWED_METRICS = NodesInfoRequest.Metric.allMetrics(); private final SettingsFilter settingsFilter; @@ -108,16 +98,9 @@ static NodesInfoRequest prepareRequest(final RestRequest request) { nodesInfoRequest.all(); } else { nodesInfoRequest.clear(); - nodesInfoRequest.settings(metrics.contains("settings")); - nodesInfoRequest.os(metrics.contains("os")); - nodesInfoRequest.process(metrics.contains("process")); - nodesInfoRequest.jvm(metrics.contains("jvm")); - nodesInfoRequest.threadPool(metrics.contains("thread_pool")); - nodesInfoRequest.transport(metrics.contains("transport")); - nodesInfoRequest.http(metrics.contains("http")); - nodesInfoRequest.plugins(metrics.contains("plugins")); - nodesInfoRequest.ingest(metrics.contains("ingest")); - nodesInfoRequest.indices(metrics.contains("indices")); + // disregard unknown metrics + metrics.retainAll(ALLOWED_METRICS); + nodesInfoRequest.addMetrics(metrics.toArray(String[]::new)); } return nodesInfoRequest; } diff --git a/server/src/main/java/org/elasticsearch/rest/action/cat/RestNodeAttrsAction.java b/server/src/main/java/org/elasticsearch/rest/action/cat/RestNodeAttrsAction.java index 13c2a5c581eb3..557266e302af5 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/cat/RestNodeAttrsAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/cat/RestNodeAttrsAction.java @@ -67,7 +67,8 @@ public RestChannelConsumer doCatRequest(final RestRequest request, final NodeCli @Override public void processResponse(final ClusterStateResponse clusterStateResponse) { NodesInfoRequest nodesInfoRequest = new NodesInfoRequest(); - nodesInfoRequest.clear().jvm(false).os(false).process(true); + nodesInfoRequest.clear() + .addMetric(NodesInfoRequest.Metric.PROCESS.metricName()); client.admin().cluster().nodesInfo(nodesInfoRequest, new RestResponseListener(channel) { @Override public RestResponse buildResponse(NodesInfoResponse nodesInfoResponse) throws Exception { diff --git a/server/src/main/java/org/elasticsearch/rest/action/cat/RestNodesAction.java b/server/src/main/java/org/elasticsearch/rest/action/cat/RestNodesAction.java index 0833a04ff45a1..f391a10b18b3f 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/cat/RestNodesAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/cat/RestNodesAction.java @@ -97,7 +97,11 @@ public RestChannelConsumer doCatRequest(final RestRequest request, final NodeCli @Override public void processResponse(final ClusterStateResponse clusterStateResponse) { NodesInfoRequest nodesInfoRequest = new NodesInfoRequest(); - nodesInfoRequest.clear().jvm(true).os(true).process(true).http(true); + nodesInfoRequest.clear().addMetrics( + NodesInfoRequest.Metric.JVM.metricName(), + NodesInfoRequest.Metric.OS.metricName(), + NodesInfoRequest.Metric.PROCESS.metricName(), + NodesInfoRequest.Metric.HTTP.metricName()); client.admin().cluster().nodesInfo(nodesInfoRequest, new RestActionListener(channel) { @Override public void processResponse(final NodesInfoResponse nodesInfoResponse) { diff --git a/server/src/main/java/org/elasticsearch/rest/action/cat/RestPluginsAction.java b/server/src/main/java/org/elasticsearch/rest/action/cat/RestPluginsAction.java index 9d4b4b08d625e..c8fa82b773567 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/cat/RestPluginsAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/cat/RestPluginsAction.java @@ -67,7 +67,8 @@ public RestChannelConsumer doCatRequest(final RestRequest request, final NodeCli @Override public void processResponse(final ClusterStateResponse clusterStateResponse) throws Exception { NodesInfoRequest nodesInfoRequest = new NodesInfoRequest(); - nodesInfoRequest.clear().plugins(true); + nodesInfoRequest.clear() + .addMetric(NodesInfoRequest.Metric.PLUGINS.metricName()); client.admin().cluster().nodesInfo(nodesInfoRequest, new RestResponseListener(channel) { @Override public RestResponse buildResponse(final NodesInfoResponse nodesInfoResponse) throws Exception { diff --git a/server/src/main/java/org/elasticsearch/rest/action/cat/RestThreadPoolAction.java b/server/src/main/java/org/elasticsearch/rest/action/cat/RestThreadPoolAction.java index 480c329808f86..f53c43565a068 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/cat/RestThreadPoolAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/cat/RestThreadPoolAction.java @@ -80,7 +80,9 @@ public RestChannelConsumer doCatRequest(final RestRequest request, final NodeCli @Override public void processResponse(final ClusterStateResponse clusterStateResponse) { NodesInfoRequest nodesInfoRequest = new NodesInfoRequest(); - nodesInfoRequest.clear().process(true).threadPool(true); + nodesInfoRequest.clear().addMetrics( + NodesInfoRequest.Metric.PROCESS.metricName(), + NodesInfoRequest.Metric.THREAD_POOL.metricName()); client.admin().cluster().nodesInfo(nodesInfoRequest, new RestActionListener(channel) { @Override public void processResponse(final NodesInfoResponse nodesInfoResponse) { diff --git a/server/src/test/java/org/elasticsearch/action/admin/cluster/node/info/NodesInfoRequestTests.java b/server/src/test/java/org/elasticsearch/action/admin/cluster/node/info/NodesInfoRequestTests.java index 7e94454102480..e0506c664e709 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/cluster/node/info/NodesInfoRequestTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/cluster/node/info/NodesInfoRequestTests.java @@ -23,7 +23,13 @@ import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.test.ESTestCase; +import java.util.HashSet; +import java.util.Set; + +import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.in; +import static org.hamcrest.Matchers.not; /** * Granular tests for the {@link NodesInfoRequest} class. Higher-level tests @@ -34,82 +40,94 @@ public class NodesInfoRequestTests extends ESTestCase { /** * Make sure that we can set, serialize, and deserialize arbitrary sets * of metrics. - * - * TODO: Once we can set values by string, use a collection rather than - * checking each and every setter in the public API */ - public void testMetricsSetters() throws Exception { + public void testAddMetricsSet() throws Exception { + NodesInfoRequest request = new NodesInfoRequest(randomAlphaOfLength(8)); + randomSubsetOf(NodesInfoRequest.Metric.allMetrics()).forEach(request::addMetric); + NodesInfoRequest deserializedRequest = roundTripRequest(request); + assertThat(request.requestedMetrics(), equalTo(deserializedRequest.requestedMetrics())); + } + + /** + * Check that we can add a metric. + */ + public void testAddSingleMetric() throws Exception { + NodesInfoRequest request = new NodesInfoRequest(randomAlphaOfLength(8)); + request.addMetric(randomFrom(NodesInfoRequest.Metric.allMetrics())); + NodesInfoRequest deserializedRequest = roundTripRequest(request); + assertThat(request.requestedMetrics(), equalTo(deserializedRequest.requestedMetrics())); + } + + /** + * Check that we can remove a metric. + */ + public void testRemoveSingleMetric() throws Exception { NodesInfoRequest request = new NodesInfoRequest(randomAlphaOfLength(8)); - request.settings(randomBoolean()); - request.os(randomBoolean()); - request.process(randomBoolean()); - request.jvm(randomBoolean()); - request.threadPool(randomBoolean()); - request.transport(randomBoolean()); - request.http(randomBoolean()); - request.plugins(randomBoolean()); - request.ingest(randomBoolean()); - request.indices(randomBoolean()); + request.all(); + String metric = randomFrom(NodesInfoRequest.Metric.allMetrics()); + request.removeMetric(metric); + NodesInfoRequest deserializedRequest = roundTripRequest(request); - assertRequestsEqual(request, deserializedRequest); + assertThat(request.requestedMetrics(), equalTo(deserializedRequest.requestedMetrics())); + assertThat(metric, not(in(request.requestedMetrics()))); } /** * Test that a newly constructed NodesInfoRequestObject requests all of the - * possible metrics defined in {@link NodesInfoRequest.Metrics}. + * possible metrics defined in {@link NodesInfoRequest.Metric}. */ public void testNodesInfoRequestDefaults() { NodesInfoRequest defaultNodesInfoRequest = new NodesInfoRequest(randomAlphaOfLength(8)); NodesInfoRequest allMetricsNodesInfoRequest = new NodesInfoRequest(randomAlphaOfLength(8)); allMetricsNodesInfoRequest.all(); - assertRequestsEqual(defaultNodesInfoRequest, allMetricsNodesInfoRequest); + assertThat(defaultNodesInfoRequest.requestedMetrics(), equalTo(allMetricsNodesInfoRequest.requestedMetrics())); } /** - * Test that the {@link NodesInfoRequest#all()} method sets all of the - * metrics to {@code true}. - * - * TODO: Once we can check values by string, use a collection rather than - * checking each and every getter in the public API + * Test that the {@link NodesInfoRequest#all()} method enables all metrics. */ public void testNodesInfoRequestAll() throws Exception { NodesInfoRequest request = new NodesInfoRequest("node"); request.all(); - assertTrue(request.settings()); - assertTrue(request.os()); - assertTrue(request.process()); - assertTrue(request.jvm()); - assertTrue(request.threadPool()); - assertTrue(request.transport()); - assertTrue(request.http()); - assertTrue(request.plugins()); - assertTrue(request.ingest()); - assertTrue(request.indices()); + assertThat(request.requestedMetrics(), equalTo(NodesInfoRequest.Metric.allMetrics())); } /** - * Test that the {@link NodesInfoRequest#clear()} method sets all of the - * metrics to {@code false}. - * - * TODO: Once we can check values by string, use a collection rather than - * checking each and every getter in the public API + * Test that the {@link NodesInfoRequest#clear()} method disables all metrics. */ public void testNodesInfoRequestClear() throws Exception { NodesInfoRequest request = new NodesInfoRequest("node"); request.clear(); - assertFalse(request.settings()); - assertFalse(request.os()); - assertFalse(request.process()); - assertFalse(request.jvm()); - assertFalse(request.threadPool()); - assertFalse(request.transport()); - assertFalse(request.http()); - assertFalse(request.plugins()); - assertFalse(request.ingest()); - assertFalse(request.indices()); + assertThat(request.requestedMetrics(), empty()); + } + + /** + * Test that (for now) we can only add metrics from a set of known metrics. + */ + public void testUnknownMetricsRejected() { + String unknownMetric1 = "unknown_metric1"; + String unknownMetric2 = "unknown_metric2"; + Set unknownMetrics = new HashSet<>(); + unknownMetrics.add(unknownMetric1); + unknownMetrics.addAll(randomSubsetOf(NodesInfoRequest.Metric.allMetrics())); + + NodesInfoRequest request = new NodesInfoRequest(); + + IllegalStateException exception = expectThrows(IllegalStateException.class, () -> request.addMetric(unknownMetric1)); + assertThat(exception.getMessage(), equalTo("Used an illegal metric: " + unknownMetric1)); + + exception = expectThrows(IllegalStateException.class, () -> request.removeMetric(unknownMetric1)); + assertThat(exception.getMessage(), equalTo("Used an illegal metric: " + unknownMetric1)); + + exception = expectThrows(IllegalStateException.class, () -> request.addMetrics(unknownMetrics.toArray(String[]::new))); + assertThat(exception.getMessage(), equalTo("Used illegal metric: [" + unknownMetric1 + "]")); + + unknownMetrics.add(unknownMetric2); + exception = expectThrows(IllegalStateException.class, () -> request.addMetrics(unknownMetrics.toArray(String[]::new))); + assertThat(exception.getMessage(), equalTo("Used illegal metrics: [" + unknownMetric1 + ", " + unknownMetric2 + "]")); } /** @@ -125,20 +143,4 @@ private static NodesInfoRequest roundTripRequest(NodesInfoRequest request) throw } } } - - private static void assertRequestsEqual(NodesInfoRequest request1, NodesInfoRequest request2) { - - // TODO: Once we can check values by string, use a collection rather than - // checking each and every getter in the public API - assertThat(request1.settings(), equalTo(request2.settings())); - assertThat(request1.os(), equalTo(request2.os())); - assertThat(request1.process(), equalTo(request2.process())); - assertThat(request1.jvm(), equalTo(request2.jvm())); - assertThat(request1.threadPool(), equalTo(request2.threadPool())); - assertThat(request1.transport(), equalTo(request2.transport())); - assertThat(request1.http(), equalTo(request2.http())); - assertThat(request1.plugins(), equalTo(request2.plugins())); - assertThat(request1.ingest(), equalTo(request2.ingest())); - assertThat(request1.indices(), equalTo(request2.indices())); - } } diff --git a/server/src/test/java/org/elasticsearch/rest/action/admin/cluster/RestNodesInfoActionTests.java b/server/src/test/java/org/elasticsearch/rest/action/admin/cluster/RestNodesInfoActionTests.java index d757ee095cdc1..456b371b0eb50 100644 --- a/server/src/test/java/org/elasticsearch/rest/action/admin/cluster/RestNodesInfoActionTests.java +++ b/server/src/test/java/org/elasticsearch/rest/action/admin/cluster/RestNodesInfoActionTests.java @@ -20,71 +20,77 @@ package org.elasticsearch.rest.action.admin.cluster; import org.elasticsearch.action.admin.cluster.node.info.NodesInfoRequest; +import org.elasticsearch.common.util.set.Sets; import org.elasticsearch.rest.RestRequest; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.rest.FakeRestRequest; import java.util.ArrayList; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import static org.elasticsearch.rest.action.admin.cluster.RestNodesInfoAction.ALLOWED_METRICS; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.hasItem; +import static org.hamcrest.Matchers.not; public class RestNodesInfoActionTests extends ESTestCase { public void testDuplicatedFiltersAreNotRemoved() { Map params = new HashMap<>(); params.put("nodeId", "_all,master:false,_all"); - + RestRequest restRequest = buildRestRequest(params); NodesInfoRequest actual = RestNodesInfoAction.prepareRequest(restRequest); assertArrayEquals(new String[] { "_all", "master:false", "_all" }, actual.nodesIds()); } - + public void testOnlyMetrics() { Map params = new HashMap<>(); int metricsCount = randomIntBetween(1, ALLOWED_METRICS.size()); List metrics = new ArrayList<>(); - + for(int i = 0; i < metricsCount; i++) { metrics.add(randomFrom(ALLOWED_METRICS)); } params.put("nodeId", String.join(",", metrics)); - + RestRequest restRequest = buildRestRequest(params); NodesInfoRequest actual = RestNodesInfoAction.prepareRequest(restRequest); assertArrayEquals(new String[] { "_all" }, actual.nodesIds()); assertMetrics(metrics, actual); } - + public void testAllMetricsSelectedWhenNodeAndMetricSpecified() { Map params = new HashMap<>(); String nodeId = randomValueOtherThanMany(ALLOWED_METRICS::contains, () -> randomAlphaOfLength(23)); String metric = randomFrom(ALLOWED_METRICS); - + params.put("nodeId", nodeId + "," + metric); RestRequest restRequest = buildRestRequest(params); - + NodesInfoRequest actual = RestNodesInfoAction.prepareRequest(restRequest); assertArrayEquals(new String[] { nodeId, metric }, actual.nodesIds()); assertAllMetricsTrue(actual); } - + public void testSeparateNodeIdsAndMetrics() { Map params = new HashMap<>(); List nodeIds = new ArrayList<>(5); List metrics = new ArrayList<>(5); - + for(int i = 0; i < 5; i++) { nodeIds.add(randomValueOtherThanMany(ALLOWED_METRICS::contains, () -> randomAlphaOfLength(23))); metrics.add(randomFrom(ALLOWED_METRICS)); } - + params.put("nodeId", String.join(",", nodeIds)); params.put("metrics", String.join(",", metrics)); RestRequest restRequest = buildRestRequest(params); - + NodesInfoRequest actual = RestNodesInfoAction.prepareRequest(restRequest); assertArrayEquals(nodeIds.toArray(), actual.nodesIds()); assertMetrics(metrics, actual); @@ -97,16 +103,42 @@ public void testExplicitAllMetrics() { for(int i = 0; i < 5; i++) { nodeIds.add(randomValueOtherThanMany(ALLOWED_METRICS::contains, () -> randomAlphaOfLength(23))); } - + params.put("nodeId", String.join(",", nodeIds)); params.put("metrics", "_all"); RestRequest restRequest = buildRestRequest(params); - + NodesInfoRequest actual = RestNodesInfoAction.prepareRequest(restRequest); assertArrayEquals(nodeIds.toArray(), actual.nodesIds()); assertAllMetricsTrue(actual); } + /** + * Test that if a user requests a non-existent metric, it's dropped from the + * request without an error. + */ + public void testNonexistentMetricsDropped() { + Map params = new HashMap<>(); + List nodeIds = new ArrayList<>(5); + List metrics = new ArrayList<>(5); + + for(int i = 0; i < 5; i++) { + nodeIds.add(randomValueOtherThanMany(ALLOWED_METRICS::contains, () -> randomAlphaOfLength(23))); + metrics.add(randomFrom(ALLOWED_METRICS)); + } + String nonAllowedMetric = randomValueOtherThanMany(ALLOWED_METRICS::contains, () -> randomAlphaOfLength(5)); + metrics.add(nonAllowedMetric); + + params.put("nodeId", String.join(",", nodeIds)); + params.put("metrics", String.join(",", metrics)); + RestRequest restRequest = buildRestRequest(params); + + NodesInfoRequest actual = RestNodesInfoAction.prepareRequest(restRequest); + assertArrayEquals(nodeIds.toArray(), actual.nodesIds()); + assertThat(actual.requestedMetrics(), not(hasItem(nonAllowedMetric))); + assertMetrics(metrics, actual); + } + private FakeRestRequest buildRestRequest(Map params) { return new FakeRestRequest.Builder(xContentRegistry()) .withMethod(RestRequest.Method.GET) @@ -114,30 +146,13 @@ private FakeRestRequest buildRestRequest(Map params) { .withParams(params) .build(); } - + private void assertMetrics(List metrics, NodesInfoRequest nodesInfoRequest) { - assertTrue((metrics.contains("http") && nodesInfoRequest.http()) || metrics.contains("http") == false); - assertTrue((metrics.contains("ingest") && nodesInfoRequest.ingest()) || metrics.contains("ingest") == false); - assertTrue((metrics.contains("indices") && nodesInfoRequest.indices()) || metrics.contains("indices") == false); - assertTrue((metrics.contains("jvm") && nodesInfoRequest.jvm()) || metrics.contains("jvm") == false); - assertTrue((metrics.contains("os") && nodesInfoRequest.os()) || metrics.contains("os") == false); - assertTrue((metrics.contains("plugins") && nodesInfoRequest.plugins()) || metrics.contains("plugins") == false); - assertTrue((metrics.contains("process") && nodesInfoRequest.process()) || metrics.contains("process") == false); - assertTrue((metrics.contains("settings") && nodesInfoRequest.settings()) || metrics.contains("settings") == false); - assertTrue((metrics.contains("thread_pool") && nodesInfoRequest.threadPool()) || metrics.contains("thread_pool") == false); - assertTrue((metrics.contains("transport") && nodesInfoRequest.transport()) || metrics.contains("transport") == false); + Set validRequestedMetrics = Sets.intersection(new HashSet<>(metrics), ALLOWED_METRICS); + assertThat(nodesInfoRequest.requestedMetrics(), equalTo(validRequestedMetrics)); } - + private void assertAllMetricsTrue(NodesInfoRequest nodesInfoRequest) { - assertTrue(nodesInfoRequest.http()); - assertTrue(nodesInfoRequest.ingest()); - assertTrue(nodesInfoRequest.indices()); - assertTrue(nodesInfoRequest.jvm()); - assertTrue(nodesInfoRequest.os()); - assertTrue(nodesInfoRequest.plugins()); - assertTrue(nodesInfoRequest.process()); - assertTrue(nodesInfoRequest.settings()); - assertTrue(nodesInfoRequest.threadPool()); - assertTrue(nodesInfoRequest.transport()); + assertThat(nodesInfoRequest.requestedMetrics(), equalTo(ALLOWED_METRICS)); } }