From 568e79db1b3e6567b77cefe8e7e6e50fb3a4948d Mon Sep 17 00:00:00 2001 From: William Brafford Date: Fri, 6 Mar 2020 14:10:06 -0500 Subject: [PATCH 01/14] Add set-based API to NodesInfoRequest This commit begins the work of removing the "hard-coded" metric getters and setters from the NodesInfoRequest classes. We start by providing new flexible getters and setters. We then update the test classes to remove the old getters, and then remove those getters. --- .../cluster/node/info/NodesInfoRequest.java | 105 +++++++----------- .../node/info/TransportNodesInfoAction.java | 15 ++- .../org/elasticsearch/node/NodeService.java | 1 + .../admin/cluster/RestNodesInfoAction.java | 12 +- .../node/info/NodesInfoRequestTests.java | 63 +---------- .../cluster/RestNodesInfoActionTests.java | 55 ++++----- 6 files changed, 80 insertions(+), 171 deletions(-) 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 4ea78a17fd0e9..2946e9a4ff9f8 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 @@ -26,6 +26,7 @@ import java.io.IOException; import java.util.Arrays; +import java.util.Collection; import java.util.Set; import java.util.stream.Collectors; @@ -88,6 +89,41 @@ public NodesInfoRequest all() { return this; } + /** + * Get the names of requested metrics + */ + public Set requestedMetrics() { + return Set.copyOf(requestedMetrics); + } + + /** + * Add metric + */ + public void addMetric(String metric) { + requestedMetrics.add(metric); + } + + /** + * Add collection of metrics + */ + public void addMetrics(Collection metrics) { + requestedMetrics.addAll(metrics); + } + + /** + * Remove metric + */ + public void removeMetric(String metric) { + requestedMetrics.remove(metric); + } + + /** + * Remove collection of metrics + */ + public void removeMetrics(Collection metrics) { + requestedMetrics.removeAll(metrics); + } + /** * Should the node settings be returned. */ @@ -103,13 +139,6 @@ public NodesInfoRequest settings(boolean settings) { return this; } - /** - * Should the node OS be returned. - */ - public boolean os() { - return Metrics.OS.containedIn(requestedMetrics); - } - /** * Should the node OS be returned. */ @@ -118,13 +147,6 @@ public NodesInfoRequest os(boolean os) { return this; } - /** - * Should the node Process be returned. - */ - public boolean process() { - return Metrics.PROCESS.containedIn(requestedMetrics); - } - /** * Should the node Process be returned. */ @@ -133,13 +155,6 @@ public NodesInfoRequest process(boolean process) { return this; } - /** - * Should the node JVM be returned. - */ - public boolean jvm() { - return Metrics.JVM.containedIn(requestedMetrics); - } - /** * Should the node JVM be returned. */ @@ -148,13 +163,6 @@ public NodesInfoRequest jvm(boolean jvm) { 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. */ @@ -163,13 +171,6 @@ public NodesInfoRequest threadPool(boolean threadPool) { return this; } - /** - * Should the node Transport be returned. - */ - public boolean transport() { - return Metrics.TRANSPORT.containedIn(requestedMetrics); - } - /** * Should the node Transport be returned. */ @@ -178,13 +179,6 @@ public NodesInfoRequest transport(boolean transport) { return this; } - /** - * Should the node HTTP be returned. - */ - public boolean http() { - return Metrics.HTTP.containedIn(requestedMetrics); - } - /** * Should the node HTTP be returned. */ @@ -203,13 +197,6 @@ public NodesInfoRequest plugins(boolean plugins) { 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 @@ -219,13 +206,6 @@ public NodesInfoRequest ingest(boolean ingest) { return this; } - /** - * @return true if information about ingest is requested - */ - 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 @@ -235,13 +215,6 @@ public NodesInfoRequest indices(boolean indices) { 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. @@ -281,12 +254,12 @@ public void writeTo(StreamOutput out) throws IOException { * from the nodes information endpoint. Eventually this list list will be * pluggable. */ - enum Metrics { + public enum Metrics { SETTINGS("settings"), OS("os"), PROCESS("process"), JVM("jvm"), - THREAD_POOL("threadPool"), + THREAD_POOL("thread_pool"), TRANSPORT("transport"), HTTP("http"), PLUGINS("plugins"), @@ -307,7 +280,7 @@ boolean containedIn(Set metricNames) { return metricNames.contains(this.metricName()); } - static Set allMetrics() { + public static Set allMetrics() { return Arrays.stream(values()).map(Metrics::metricName).collect(Collectors.toSet()); } } 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..9e31b7e28b397 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("settings"), + metrics.contains("os"), + metrics.contains("process"), + metrics.contains("jvm"), + metrics.contains("threadPool"), + metrics.contains("transport"), + metrics.contains("http"), + metrics.contains("plugins"), + metrics.contains("ingest"), + metrics.contains("indices")); } public static class NodeInfoRequest extends BaseNodeRequest { diff --git a/server/src/main/java/org/elasticsearch/node/NodeService.java b/server/src/main/java/org/elasticsearch/node/NodeService.java index 895be6fca60d6..a822b16bbce63 100644 --- a/server/src/main/java/org/elasticsearch/node/NodeService.java +++ b/server/src/main/java/org/elasticsearch/node/NodeService.java @@ -85,6 +85,7 @@ public class NodeService implements Closeable { clusterService.addStateApplier(ingestService); } + // TODO: this should take a set of strings argument rather than a bunch of boolean arguments public NodeInfo info(boolean settings, boolean os, boolean process, boolean jvm, boolean threadPool, boolean transport, boolean http, boolean plugin, boolean ingest, boolean indices) { return new NodeInfo(Version.CURRENT, Build.CURRENT, transportService.getLocalNode(), 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..ffbf228333873 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.Metrics.allMetrics(); private final SettingsFilter settingsFilter; 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..ac2f520210d84 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,6 +23,7 @@ import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.test.ESTestCase; +import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.equalTo; /** @@ -34,24 +35,12 @@ 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 { 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.addMetrics(randomSubsetOf(NodesInfoRequest.Metrics.allMetrics())); NodesInfoRequest deserializedRequest = roundTripRequest(request); - assertRequestsEqual(request, deserializedRequest); + assertThat(request.requestedMetrics(), equalTo(deserializedRequest.requestedMetrics())); } /** @@ -63,53 +52,29 @@ public void testNodesInfoRequestDefaults() { 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 */ 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.Metrics.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 */ 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()); } /** @@ -125,20 +90,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..ce69262d31533 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 @@ -26,65 +26,67 @@ import java.util.ArrayList; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; import static org.elasticsearch.rest.action.admin.cluster.RestNodesInfoAction.ALLOWED_METRICS; +import static org.hamcrest.Matchers.equalTo; 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,11 +99,11 @@ 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); @@ -114,30 +116,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); + // if the metrics list contains + assertThat(new HashSet<>(metrics), equalTo(nodesInfoRequest.requestedMetrics())); } - + 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)); } } From 6fe9d71c21542eb279c8c994134fa325b44b0d09 Mon Sep 17 00:00:00 2001 From: William Brafford Date: Fri, 6 Mar 2020 15:42:28 -0500 Subject: [PATCH 02/14] Protect against unknown metric names Throw an IllegalStateException if an unknown metric is added or removed. Replace some of the infrequently used setters. --- .../cluster/node/info/NodesInfoRequest.java | 40 +++++-------------- .../node/info/NodesInfoRequestBuilder.java | 28 ++++++++----- .../admin/cluster/RestNodesInfoAction.java | 11 +---- .../node/info/NodesInfoRequestTests.java | 25 ++++++++++++ 4 files changed, 53 insertions(+), 51 deletions(-) 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 2946e9a4ff9f8..eed8f5aeb6bf2 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 @@ -100,6 +100,9 @@ public Set requestedMetrics() { * Add metric */ public void addMetric(String metric) { + if (Metrics.allMetrics().contains(metric) == false) { + throw new IllegalStateException("Used an illegal metric: " + metric); + } requestedMetrics.add(metric); } @@ -107,6 +110,9 @@ public void addMetric(String metric) { * Add collection of metrics */ public void addMetrics(Collection metrics) { + if (Metrics.allMetrics().containsAll(metrics) == false) { + throw new IllegalStateException("Used an illegal metric: " + metrics); + } requestedMetrics.addAll(metrics); } @@ -114,23 +120,12 @@ public void addMetrics(Collection metrics) { * Remove metric */ public void removeMetric(String metric) { + if (Metrics.allMetrics().contains(metric) == false) { + throw new IllegalStateException("Used an illegal metric: " + metric); + } requestedMetrics.remove(metric); } - /** - * Remove collection of metrics - */ - public void removeMetrics(Collection metrics) { - requestedMetrics.removeAll(metrics); - } - - /** - * Should the node settings be returned. - */ - public boolean settings() { - return Metrics.SETTINGS.containedIn(requestedMetrics); - } - /** * Should the node settings be returned. */ @@ -171,14 +166,6 @@ public NodesInfoRequest threadPool(boolean threadPool) { return this; } - /** - * Should the node Transport be returned. - */ - public NodesInfoRequest transport(boolean transport) { - addOrRemoveMetric(transport, Metrics.TRANSPORT.metricName()); - return this; - } - /** * Should the node HTTP be returned. */ @@ -206,15 +193,6 @@ public NodesInfoRequest ingest(boolean ingest) { return this; } - /** - * 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()); - return this; - } - /** * Helper method for adding and removing metrics. * @param includeMetric Whether or not to include a metric. 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..3e92b75391f9d 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 @@ -48,7 +48,7 @@ public NodesInfoRequestBuilder all() { * Should the node settings be returned. */ public NodesInfoRequestBuilder setSettings(boolean settings) { - request.settings(settings); + addOrRemoveMetric(settings, NodesInfoRequest.Metrics.SETTINGS); return this; } @@ -56,7 +56,7 @@ public NodesInfoRequestBuilder setSettings(boolean settings) { * Should the node OS info be returned. */ public NodesInfoRequestBuilder setOs(boolean os) { - request.os(os); + addOrRemoveMetric(os, NodesInfoRequest.Metrics.OS); return this; } @@ -64,7 +64,7 @@ public NodesInfoRequestBuilder setOs(boolean os) { * Should the node OS process be returned. */ public NodesInfoRequestBuilder setProcess(boolean process) { - request.process(process); + addOrRemoveMetric(process, NodesInfoRequest.Metrics.PROCESS); return this; } @@ -72,7 +72,7 @@ public NodesInfoRequestBuilder setProcess(boolean process) { * Should the node JVM info be returned. */ public NodesInfoRequestBuilder setJvm(boolean jvm) { - request.jvm(jvm); + addOrRemoveMetric(jvm, NodesInfoRequest.Metrics.JVM); return this; } @@ -80,7 +80,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.Metrics.THREAD_POOL); return this; } @@ -88,7 +88,7 @@ public NodesInfoRequestBuilder setThreadPool(boolean threadPool) { * Should the node Transport info be returned. */ public NodesInfoRequestBuilder setTransport(boolean transport) { - request.transport(transport); + addOrRemoveMetric(transport, NodesInfoRequest.Metrics.TRANSPORT); return this; } @@ -96,7 +96,7 @@ public NodesInfoRequestBuilder setTransport(boolean transport) { * Should the node HTTP info be returned. */ public NodesInfoRequestBuilder setHttp(boolean http) { - request.http(http); + addOrRemoveMetric(http, NodesInfoRequest.Metrics.HTTP); return this; } @@ -104,7 +104,7 @@ public NodesInfoRequestBuilder setHttp(boolean http) { * Should the node plugins info be returned. */ public NodesInfoRequestBuilder setPlugins(boolean plugins) { - request().plugins(plugins); + addOrRemoveMetric(plugins, NodesInfoRequest.Metrics.PLUGINS); return this; } @@ -112,7 +112,7 @@ public NodesInfoRequestBuilder setPlugins(boolean plugins) { * Should the node ingest info be returned. */ public NodesInfoRequestBuilder setIngest(boolean ingest) { - request().ingest(ingest); + addOrRemoveMetric(ingest, NodesInfoRequest.Metrics.INGEST); return this; } @@ -120,7 +120,15 @@ public NodesInfoRequestBuilder setIngest(boolean ingest) { * Should the node indices info be returned. */ public NodesInfoRequestBuilder setIndices(boolean indices) { - request().indices(indices); + addOrRemoveMetric(indices, NodesInfoRequest.Metrics.INDICES); return this; } + + private void addOrRemoveMetric(boolean includeMetric, NodesInfoRequest.Metrics metric) { + if (includeMetric) { + request.addMetric(metric.metricName()); + } else { + request.removeMetric(metric.metricName()); + } + } } 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 ffbf228333873..a8f561b04a302 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 @@ -98,16 +98,7 @@ 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")); + nodesInfoRequest.addMetrics(metrics); } return nodesInfoRequest; } 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 ac2f520210d84..0598c7c886e9f 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,8 +23,12 @@ 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.startsWith; /** * Granular tests for the {@link NodesInfoRequest} class. Higher-level tests @@ -77,6 +81,27 @@ public void testNodesInfoRequestClear() throws Exception { assertThat(request.requestedMetrics(), empty()); } + /** + * Test that (for now) we can only add metrics from a set of known metrics. + */ + public void testUnknownMetricsRejected() { + String unknownMetric = "unknown_metric1"; + Set unknownMetrics = new HashSet<>(); + unknownMetrics.add(unknownMetric); + unknownMetrics.addAll(randomSubsetOf(NodesInfoRequest.Metrics.allMetrics())); + + NodesInfoRequest request = new NodesInfoRequest(); + + IllegalStateException exception = expectThrows(IllegalStateException.class, () -> request.addMetric(unknownMetric)); + assertThat(exception.getMessage(), equalTo("Used an illegal metric: " + unknownMetric)); + + exception = expectThrows(IllegalStateException.class, () -> request.removeMetric(unknownMetric)); + assertThat(exception.getMessage(), equalTo("Used an illegal metric: " + unknownMetric)); + + exception = expectThrows(IllegalStateException.class, () -> request.addMetrics(unknownMetrics)); + assertThat(exception.getMessage(), startsWith("Used an illegal metric: ")); + } + /** * Serialize and deserialize a request. * @param request A request to serialize. From a3035ff77ccb069a7d61e5f5efbce315160c6b74 Mon Sep 17 00:00:00 2001 From: William Brafford Date: Fri, 6 Mar 2020 17:09:54 -0500 Subject: [PATCH 03/14] Remove old getters and some magic strings --- .../cluster/node/info/NodesInfoRequest.java | 108 ++++-------------- .../node/info/TransportNodesInfoAction.java | 20 ++-- .../ingest/PutPipelineTransportAction.java | 4 +- .../rest/action/cat/RestNodeAttrsAction.java | 3 +- .../rest/action/cat/RestNodesAction.java | 6 +- .../rest/action/cat/RestPluginsAction.java | 3 +- .../rest/action/cat/RestThreadPoolAction.java | 4 +- 7 files changed, 46 insertions(+), 102 deletions(-) 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 eed8f5aeb6bf2..76b81ecac7149 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 @@ -49,16 +49,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(), Metrics.SETTINGS.metricName()); + optionallyAddMetric(in.readBoolean(), Metrics.OS.metricName()); + optionallyAddMetric(in.readBoolean(), Metrics.PROCESS.metricName()); + optionallyAddMetric(in.readBoolean(), Metrics.JVM.metricName()); + optionallyAddMetric(in.readBoolean(), Metrics.THREAD_POOL.metricName()); + optionallyAddMetric(in.readBoolean(), Metrics.TRANSPORT.metricName()); + optionallyAddMetric(in.readBoolean(), Metrics.HTTP.metricName()); + optionallyAddMetric(in.readBoolean(), Metrics.PLUGINS.metricName()); + optionallyAddMetric(in.readBoolean(), Metrics.INGEST.metricName()); + optionallyAddMetric(in.readBoolean(), Metrics.INDICES.metricName()); } else { requestedMetrics.addAll(Arrays.asList(in.readStringArray())); } @@ -99,110 +99,46 @@ public Set requestedMetrics() { /** * Add metric */ - public void addMetric(String metric) { + public NodesInfoRequest addMetric(String metric) { if (Metrics.allMetrics().contains(metric) == false) { throw new IllegalStateException("Used an illegal metric: " + metric); } requestedMetrics.add(metric); + return this; } /** * Add collection of metrics */ - public void addMetrics(Collection metrics) { + public NodesInfoRequest addMetrics(Collection metrics) { if (Metrics.allMetrics().containsAll(metrics) == false) { throw new IllegalStateException("Used an illegal metric: " + metrics); } requestedMetrics.addAll(metrics); + return this; } /** * Remove metric */ - public void removeMetric(String metric) { + public NodesInfoRequest removeMetric(String metric) { if (Metrics.allMetrics().contains(metric) == false) { throw new IllegalStateException("Used an illegal metric: " + metric); } requestedMetrics.remove(metric); - } - - /** - * Should the node settings be returned. - */ - public NodesInfoRequest settings(boolean settings) { - addOrRemoveMetric(settings, Metrics.SETTINGS.metricName()); - return this; - } - - /** - * 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 NodesInfoRequest process(boolean process) { - addOrRemoveMetric(process, Metrics.PROCESS.metricName()); - return this; - } - - /** - * 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 NodesInfoRequest threadPool(boolean threadPool) { - addOrRemoveMetric(threadPool, Metrics.THREAD_POOL.metricName()); - return this; - } - - /** - * 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()); - return this; - } - - /** - * Should information about ingest be returned - * @param ingest true if you want info - */ - public NodesInfoRequest ingest(boolean ingest) { - addOrRemoveMetric(ingest, Metrics.INGEST.metricName()); - return this; - } - - /** - * 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); } } @@ -250,7 +186,7 @@ public enum Metrics { this.metricName = name; } - String metricName() { + public String metricName() { return this.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 9e31b7e28b397..e5d6a366428c7 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 @@ -72,16 +72,16 @@ protected NodeInfo nodeOperation(NodeInfoRequest nodeRequest, Task task) { NodesInfoRequest request = nodeRequest.request; Set metrics = request.requestedMetrics(); return nodeService.info( - metrics.contains("settings"), - metrics.contains("os"), - metrics.contains("process"), - metrics.contains("jvm"), - metrics.contains("threadPool"), - metrics.contains("transport"), - metrics.contains("http"), - metrics.contains("plugins"), - metrics.contains("ingest"), - metrics.contains("indices")); + metrics.contains(NodesInfoRequest.Metrics.SETTINGS.metricName()), + metrics.contains(NodesInfoRequest.Metrics.OS.metricName()), + metrics.contains(NodesInfoRequest.Metrics.PROCESS.metricName()), + metrics.contains(NodesInfoRequest.Metrics.JVM.metricName()), + metrics.contains(NodesInfoRequest.Metrics.THREAD_POOL.metricName()), + metrics.contains(NodesInfoRequest.Metrics.TRANSPORT.metricName()), + metrics.contains(NodesInfoRequest.Metrics.HTTP.metricName()), + metrics.contains(NodesInfoRequest.Metrics.PLUGINS.metricName()), + metrics.contains(NodesInfoRequest.Metrics.INGEST.metricName()), + metrics.contains(NodesInfoRequest.Metrics.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..996fa446ca8bc 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.Metrics.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/cat/RestNodeAttrsAction.java b/server/src/main/java/org/elasticsearch/rest/action/cat/RestNodeAttrsAction.java index 13c2a5c581eb3..3f7d2ae607a25 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.Metrics.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..4bd7f3b0dbdb2 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() + .addMetric(NodesInfoRequest.Metrics.JVM.metricName()) + .addMetric(NodesInfoRequest.Metrics.OS.metricName()) + .addMetric(NodesInfoRequest.Metrics.PROCESS.metricName()) + .addMetric(NodesInfoRequest.Metrics.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..682c4180f383f 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.Metrics.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..ca82dbcb07e46 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() + .addMetric(NodesInfoRequest.Metrics.PROCESS.metricName()) + .addMetric(NodesInfoRequest.Metrics.THREAD_POOL.metricName()); client.admin().cluster().nodesInfo(nodesInfoRequest, new RestActionListener(channel) { @Override public void processResponse(final NodesInfoResponse nodesInfoResponse) { From 4e5083dc625a56cc8d880a84b8d1d74e280c6f95 Mon Sep 17 00:00:00 2001 From: William Brafford Date: Mon, 9 Mar 2020 14:03:22 -0400 Subject: [PATCH 04/14] Cleanup --- .../rest/action/admin/cluster/RestNodesInfoAction.java | 2 ++ .../rest/action/admin/cluster/RestNodesInfoActionTests.java | 1 - 2 files changed, 2 insertions(+), 1 deletion(-) 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 a8f561b04a302..5fec4474511f0 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 @@ -98,6 +98,8 @@ static NodesInfoRequest prepareRequest(final RestRequest request) { nodesInfoRequest.all(); } else { nodesInfoRequest.clear(); + // disregard unknown metrics + metrics.retainAll(ALLOWED_METRICS); nodesInfoRequest.addMetrics(metrics); } return nodesInfoRequest; 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 ce69262d31533..051f2e7514dfd 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 @@ -118,7 +118,6 @@ private FakeRestRequest buildRestRequest(Map params) { } private void assertMetrics(List metrics, NodesInfoRequest nodesInfoRequest) { - // if the metrics list contains assertThat(new HashSet<>(metrics), equalTo(nodesInfoRequest.requestedMetrics())); } From aba731fc2e729b682f4405df2a59338506d6dddd Mon Sep 17 00:00:00 2001 From: William Brafford Date: Mon, 9 Mar 2020 17:58:42 -0400 Subject: [PATCH 05/14] Add test coverage --- .../cluster/RestNodesInfoActionTests.java | 33 ++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) 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 051f2e7514dfd..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,6 +20,7 @@ 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; @@ -29,9 +30,12 @@ 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 { @@ -109,6 +113,32 @@ public void testExplicitAllMetrics() { 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) @@ -118,7 +148,8 @@ private FakeRestRequest buildRestRequest(Map params) { } private void assertMetrics(List metrics, NodesInfoRequest nodesInfoRequest) { - assertThat(new HashSet<>(metrics), equalTo(nodesInfoRequest.requestedMetrics())); + Set validRequestedMetrics = Sets.intersection(new HashSet<>(metrics), ALLOWED_METRICS); + assertThat(nodesInfoRequest.requestedMetrics(), equalTo(validRequestedMetrics)); } private void assertAllMetricsTrue(NodesInfoRequest nodesInfoRequest) { From 112679a5431e9bd1e3a8e3095568c591845d9790 Mon Sep 17 00:00:00 2001 From: William Brafford Date: Tue, 10 Mar 2020 16:20:27 -0400 Subject: [PATCH 06/14] Singularize enum name --- .../cluster/node/info/NodesInfoRequest.java | 56 +++++++++---------- .../node/info/NodesInfoRequestBuilder.java | 22 ++++---- .../node/info/TransportNodesInfoAction.java | 20 +++---- .../ingest/PutPipelineTransportAction.java | 2 +- .../admin/cluster/RestNodesInfoAction.java | 2 +- .../rest/action/cat/RestNodeAttrsAction.java | 2 +- .../rest/action/cat/RestNodesAction.java | 8 +-- .../rest/action/cat/RestPluginsAction.java | 2 +- .../rest/action/cat/RestThreadPoolAction.java | 4 +- .../node/info/NodesInfoRequestTests.java | 8 +-- 10 files changed, 63 insertions(+), 63 deletions(-) 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 76b81ecac7149..b491e970eddb9 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 @@ -35,7 +35,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. @@ -49,16 +49,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 - optionallyAddMetric(in.readBoolean(), Metrics.SETTINGS.metricName()); - optionallyAddMetric(in.readBoolean(), Metrics.OS.metricName()); - optionallyAddMetric(in.readBoolean(), Metrics.PROCESS.metricName()); - optionallyAddMetric(in.readBoolean(), Metrics.JVM.metricName()); - optionallyAddMetric(in.readBoolean(), Metrics.THREAD_POOL.metricName()); - optionallyAddMetric(in.readBoolean(), Metrics.TRANSPORT.metricName()); - optionallyAddMetric(in.readBoolean(), Metrics.HTTP.metricName()); - optionallyAddMetric(in.readBoolean(), Metrics.PLUGINS.metricName()); - optionallyAddMetric(in.readBoolean(), Metrics.INGEST.metricName()); - optionallyAddMetric(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())); } @@ -85,7 +85,7 @@ public NodesInfoRequest clear() { * Sets to return all the data. */ public NodesInfoRequest all() { - requestedMetrics.addAll(Metrics.allMetrics()); + requestedMetrics.addAll(Metric.allMetrics()); return this; } @@ -100,7 +100,7 @@ public Set requestedMetrics() { * Add metric */ public NodesInfoRequest addMetric(String metric) { - if (Metrics.allMetrics().contains(metric) == false) { + if (Metric.allMetrics().contains(metric) == false) { throw new IllegalStateException("Used an illegal metric: " + metric); } requestedMetrics.add(metric); @@ -111,7 +111,7 @@ public NodesInfoRequest addMetric(String metric) { * Add collection of metrics */ public NodesInfoRequest addMetrics(Collection metrics) { - if (Metrics.allMetrics().containsAll(metrics) == false) { + if (Metric.allMetrics().containsAll(metrics) == false) { throw new IllegalStateException("Used an illegal metric: " + metrics); } requestedMetrics.addAll(metrics); @@ -122,7 +122,7 @@ public NodesInfoRequest addMetrics(Collection metrics) { * Remove metric */ public NodesInfoRequest removeMetric(String metric) { - if (Metrics.allMetrics().contains(metric) == false) { + if (Metric.allMetrics().contains(metric) == false) { throw new IllegalStateException("Used an illegal metric: " + metric); } requestedMetrics.remove(metric); @@ -148,16 +148,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)); } @@ -168,7 +168,7 @@ public void writeTo(StreamOutput out) throws IOException { * from the nodes information endpoint. Eventually this list list will be * pluggable. */ - public enum Metrics { + public enum Metric { SETTINGS("settings"), OS("os"), PROCESS("process"), @@ -182,7 +182,7 @@ public enum Metrics { private String metricName; - Metrics(String name) { + Metric(String name) { this.metricName = name; } @@ -195,7 +195,7 @@ boolean containedIn(Set metricNames) { } public static Set allMetrics() { - return Arrays.stream(values()).map(Metrics::metricName).collect(Collectors.toSet()); + 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 3e92b75391f9d..7e2b400cd3527 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 @@ -48,7 +48,7 @@ public NodesInfoRequestBuilder all() { * Should the node settings be returned. */ public NodesInfoRequestBuilder setSettings(boolean settings) { - addOrRemoveMetric(settings, NodesInfoRequest.Metrics.SETTINGS); + addOrRemoveMetric(settings, NodesInfoRequest.Metric.SETTINGS); return this; } @@ -56,7 +56,7 @@ public NodesInfoRequestBuilder setSettings(boolean settings) { * Should the node OS info be returned. */ public NodesInfoRequestBuilder setOs(boolean os) { - addOrRemoveMetric(os, NodesInfoRequest.Metrics.OS); + addOrRemoveMetric(os, NodesInfoRequest.Metric.OS); return this; } @@ -64,7 +64,7 @@ public NodesInfoRequestBuilder setOs(boolean os) { * Should the node OS process be returned. */ public NodesInfoRequestBuilder setProcess(boolean process) { - addOrRemoveMetric(process, NodesInfoRequest.Metrics.PROCESS); + addOrRemoveMetric(process, NodesInfoRequest.Metric.PROCESS); return this; } @@ -72,7 +72,7 @@ public NodesInfoRequestBuilder setProcess(boolean process) { * Should the node JVM info be returned. */ public NodesInfoRequestBuilder setJvm(boolean jvm) { - addOrRemoveMetric(jvm, NodesInfoRequest.Metrics.JVM); + addOrRemoveMetric(jvm, NodesInfoRequest.Metric.JVM); return this; } @@ -80,7 +80,7 @@ public NodesInfoRequestBuilder setJvm(boolean jvm) { * Should the node thread pool info be returned. */ public NodesInfoRequestBuilder setThreadPool(boolean threadPool) { - addOrRemoveMetric(threadPool, NodesInfoRequest.Metrics.THREAD_POOL); + addOrRemoveMetric(threadPool, NodesInfoRequest.Metric.THREAD_POOL); return this; } @@ -88,7 +88,7 @@ public NodesInfoRequestBuilder setThreadPool(boolean threadPool) { * Should the node Transport info be returned. */ public NodesInfoRequestBuilder setTransport(boolean transport) { - addOrRemoveMetric(transport, NodesInfoRequest.Metrics.TRANSPORT); + addOrRemoveMetric(transport, NodesInfoRequest.Metric.TRANSPORT); return this; } @@ -96,7 +96,7 @@ public NodesInfoRequestBuilder setTransport(boolean transport) { * Should the node HTTP info be returned. */ public NodesInfoRequestBuilder setHttp(boolean http) { - addOrRemoveMetric(http, NodesInfoRequest.Metrics.HTTP); + addOrRemoveMetric(http, NodesInfoRequest.Metric.HTTP); return this; } @@ -104,7 +104,7 @@ public NodesInfoRequestBuilder setHttp(boolean http) { * Should the node plugins info be returned. */ public NodesInfoRequestBuilder setPlugins(boolean plugins) { - addOrRemoveMetric(plugins, NodesInfoRequest.Metrics.PLUGINS); + addOrRemoveMetric(plugins, NodesInfoRequest.Metric.PLUGINS); return this; } @@ -112,7 +112,7 @@ public NodesInfoRequestBuilder setPlugins(boolean plugins) { * Should the node ingest info be returned. */ public NodesInfoRequestBuilder setIngest(boolean ingest) { - addOrRemoveMetric(ingest, NodesInfoRequest.Metrics.INGEST); + addOrRemoveMetric(ingest, NodesInfoRequest.Metric.INGEST); return this; } @@ -120,11 +120,11 @@ public NodesInfoRequestBuilder setIngest(boolean ingest) { * Should the node indices info be returned. */ public NodesInfoRequestBuilder setIndices(boolean indices) { - addOrRemoveMetric(indices, NodesInfoRequest.Metrics.INDICES); + addOrRemoveMetric(indices, NodesInfoRequest.Metric.INDICES); return this; } - private void addOrRemoveMetric(boolean includeMetric, NodesInfoRequest.Metrics metric) { + private void addOrRemoveMetric(boolean includeMetric, NodesInfoRequest.Metric metric) { if (includeMetric) { request.addMetric(metric.metricName()); } else { 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 e5d6a366428c7..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 @@ -72,16 +72,16 @@ protected NodeInfo nodeOperation(NodeInfoRequest nodeRequest, Task task) { NodesInfoRequest request = nodeRequest.request; Set metrics = request.requestedMetrics(); return nodeService.info( - metrics.contains(NodesInfoRequest.Metrics.SETTINGS.metricName()), - metrics.contains(NodesInfoRequest.Metrics.OS.metricName()), - metrics.contains(NodesInfoRequest.Metrics.PROCESS.metricName()), - metrics.contains(NodesInfoRequest.Metrics.JVM.metricName()), - metrics.contains(NodesInfoRequest.Metrics.THREAD_POOL.metricName()), - metrics.contains(NodesInfoRequest.Metrics.TRANSPORT.metricName()), - metrics.contains(NodesInfoRequest.Metrics.HTTP.metricName()), - metrics.contains(NodesInfoRequest.Metrics.PLUGINS.metricName()), - metrics.contains(NodesInfoRequest.Metrics.INGEST.metricName()), - metrics.contains(NodesInfoRequest.Metrics.INDICES.metricName())); + 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 996fa446ca8bc..c51f09c2da0dc 100644 --- a/server/src/main/java/org/elasticsearch/action/ingest/PutPipelineTransportAction.java +++ b/server/src/main/java/org/elasticsearch/action/ingest/PutPipelineTransportAction.java @@ -75,7 +75,7 @@ protected void masterOperation(Task task, PutPipelineRequest request, ClusterSta throws Exception { NodesInfoRequest nodesInfoRequest = new NodesInfoRequest(); nodesInfoRequest.clear() - .addMetric(NodesInfoRequest.Metrics.INGEST.metricName()); + .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 5fec4474511f0..1a578848e6e8e 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,7 +36,7 @@ import static org.elasticsearch.rest.RestRequest.Method.GET; public class RestNodesInfoAction extends BaseRestHandler { - static final Set ALLOWED_METRICS = NodesInfoRequest.Metrics.allMetrics(); + static final Set ALLOWED_METRICS = NodesInfoRequest.Metric.allMetrics(); private final SettingsFilter settingsFilter; 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 3f7d2ae607a25..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 @@ -68,7 +68,7 @@ public RestChannelConsumer doCatRequest(final RestRequest request, final NodeCli public void processResponse(final ClusterStateResponse clusterStateResponse) { NodesInfoRequest nodesInfoRequest = new NodesInfoRequest(); nodesInfoRequest.clear() - .addMetric(NodesInfoRequest.Metrics.PROCESS.metricName()); + .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 4bd7f3b0dbdb2..fbde611921c4d 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 @@ -98,10 +98,10 @@ public RestChannelConsumer doCatRequest(final RestRequest request, final NodeCli public void processResponse(final ClusterStateResponse clusterStateResponse) { NodesInfoRequest nodesInfoRequest = new NodesInfoRequest(); nodesInfoRequest.clear() - .addMetric(NodesInfoRequest.Metrics.JVM.metricName()) - .addMetric(NodesInfoRequest.Metrics.OS.metricName()) - .addMetric(NodesInfoRequest.Metrics.PROCESS.metricName()) - .addMetric(NodesInfoRequest.Metrics.HTTP.metricName()); + .addMetric(NodesInfoRequest.Metric.JVM.metricName()) + .addMetric(NodesInfoRequest.Metric.OS.metricName()) + .addMetric(NodesInfoRequest.Metric.PROCESS.metricName()) + .addMetric(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 682c4180f383f..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 @@ -68,7 +68,7 @@ public RestChannelConsumer doCatRequest(final RestRequest request, final NodeCli public void processResponse(final ClusterStateResponse clusterStateResponse) throws Exception { NodesInfoRequest nodesInfoRequest = new NodesInfoRequest(); nodesInfoRequest.clear() - .addMetric(NodesInfoRequest.Metrics.PLUGINS.metricName()); + .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 ca82dbcb07e46..3c6b08f005334 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 @@ -81,8 +81,8 @@ public RestChannelConsumer doCatRequest(final RestRequest request, final NodeCli public void processResponse(final ClusterStateResponse clusterStateResponse) { NodesInfoRequest nodesInfoRequest = new NodesInfoRequest(); nodesInfoRequest.clear() - .addMetric(NodesInfoRequest.Metrics.PROCESS.metricName()) - .addMetric(NodesInfoRequest.Metrics.THREAD_POOL.metricName()); + .addMetric(NodesInfoRequest.Metric.PROCESS.metricName()) + .addMetric(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 0598c7c886e9f..4c761cc6b74be 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 @@ -42,14 +42,14 @@ public class NodesInfoRequestTests extends ESTestCase { */ public void testMetricsSetters() throws Exception { NodesInfoRequest request = new NodesInfoRequest(randomAlphaOfLength(8)); - request.addMetrics(randomSubsetOf(NodesInfoRequest.Metrics.allMetrics())); + request.addMetrics(randomSubsetOf(NodesInfoRequest.Metric.allMetrics())); NodesInfoRequest deserializedRequest = roundTripRequest(request); assertThat(request.requestedMetrics(), equalTo(deserializedRequest.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)); @@ -67,7 +67,7 @@ public void testNodesInfoRequestAll() throws Exception { NodesInfoRequest request = new NodesInfoRequest("node"); request.all(); - assertThat(request.requestedMetrics(), equalTo(NodesInfoRequest.Metrics.allMetrics())); + assertThat(request.requestedMetrics(), equalTo(NodesInfoRequest.Metric.allMetrics())); } /** @@ -88,7 +88,7 @@ public void testUnknownMetricsRejected() { String unknownMetric = "unknown_metric1"; Set unknownMetrics = new HashSet<>(); unknownMetrics.add(unknownMetric); - unknownMetrics.addAll(randomSubsetOf(NodesInfoRequest.Metrics.allMetrics())); + unknownMetrics.addAll(randomSubsetOf(NodesInfoRequest.Metric.allMetrics())); NodesInfoRequest request = new NodesInfoRequest(); From 9d001d1a3bcf3cdd0a6716e5b041301ac29a42a0 Mon Sep 17 00:00:00 2001 From: William Brafford Date: Tue, 10 Mar 2020 16:32:27 -0400 Subject: [PATCH 07/14] Remove TODO comment --- server/src/main/java/org/elasticsearch/node/NodeService.java | 1 - 1 file changed, 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/node/NodeService.java b/server/src/main/java/org/elasticsearch/node/NodeService.java index a822b16bbce63..895be6fca60d6 100644 --- a/server/src/main/java/org/elasticsearch/node/NodeService.java +++ b/server/src/main/java/org/elasticsearch/node/NodeService.java @@ -85,7 +85,6 @@ public class NodeService implements Closeable { clusterService.addStateApplier(ingestService); } - // TODO: this should take a set of strings argument rather than a bunch of boolean arguments public NodeInfo info(boolean settings, boolean os, boolean process, boolean jvm, boolean threadPool, boolean transport, boolean http, boolean plugin, boolean ingest, boolean indices) { return new NodeInfo(Version.CURRENT, Build.CURRENT, transportService.getLocalNode(), From f3baadaa2f5600e087f8966949e6041731894b63 Mon Sep 17 00:00:00 2001 From: William Brafford Date: Tue, 10 Mar 2020 16:49:53 -0400 Subject: [PATCH 08/14] Add an addMetrics(String...) method Based on some of the refactoring I'm doing, it seems like it's convenient to be have an addMetrics(String ...) method. Also add some more test coverage. --- .../cluster/node/info/NodesInfoRequest.java | 8 ++++++ .../rest/action/cat/RestNodesAction.java | 10 +++---- .../rest/action/cat/RestThreadPoolAction.java | 6 ++-- .../node/info/NodesInfoRequestTests.java | 28 ++++++++++++++++++- 4 files changed, 43 insertions(+), 9 deletions(-) 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 b491e970eddb9..8da5fdfe91611 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 @@ -118,6 +118,14 @@ public NodesInfoRequest addMetrics(Collection metrics) { return this; } + /** + * Add String array of metrics + */ + public NodesInfoRequest addMetrics(String... metrics) { + addMetrics(Arrays.stream(metrics).collect(Collectors.toSet())); + return this; + } + /** * Remove metric */ 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 fbde611921c4d..978589e28005c 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,11 +97,11 @@ public RestChannelConsumer doCatRequest(final RestRequest request, final NodeCli @Override public void processResponse(final ClusterStateResponse clusterStateResponse) { NodesInfoRequest nodesInfoRequest = new NodesInfoRequest(); - nodesInfoRequest.clear() - .addMetric(NodesInfoRequest.Metric.JVM.metricName()) - .addMetric(NodesInfoRequest.Metric.OS.metricName()) - .addMetric(NodesInfoRequest.Metric.PROCESS.metricName()) - .addMetric(NodesInfoRequest.Metric.HTTP.metricName()); + 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/RestThreadPoolAction.java b/server/src/main/java/org/elasticsearch/rest/action/cat/RestThreadPoolAction.java index 3c6b08f005334..b56901b5e610d 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,9 +80,9 @@ public RestChannelConsumer doCatRequest(final RestRequest request, final NodeCli @Override public void processResponse(final ClusterStateResponse clusterStateResponse) { NodesInfoRequest nodesInfoRequest = new NodesInfoRequest(); - nodesInfoRequest.clear() - .addMetric(NodesInfoRequest.Metric.PROCESS.metricName()) - .addMetric(NodesInfoRequest.Metric.THREAD_POOL.metricName()); + 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 4c761cc6b74be..ae720bc6274a7 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 @@ -28,6 +28,8 @@ import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.in; +import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.startsWith; /** @@ -40,13 +42,37 @@ public class NodesInfoRequestTests extends ESTestCase { * Make sure that we can set, serialize, and deserialize arbitrary sets * of metrics. */ - public void testMetricsSetters() throws Exception { + public void testAddMetricsSet() throws Exception { NodesInfoRequest request = new NodesInfoRequest(randomAlphaOfLength(8)); request.addMetrics(randomSubsetOf(NodesInfoRequest.Metric.allMetrics())); 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.all(); + String metric = randomFrom(NodesInfoRequest.Metric.allMetrics()); + request.removeMetric(metric); + + NodesInfoRequest deserializedRequest = roundTripRequest(request); + 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.Metric}. From 4c6c40d29d8b09fab0accc5375d8cc08b0a453a6 Mon Sep 17 00:00:00 2001 From: William Brafford Date: Wed, 11 Mar 2020 09:43:04 -0400 Subject: [PATCH 09/14] Add TODO for further work --- .../action/admin/cluster/node/info/NodesInfoRequestBuilder.java | 1 + 1 file changed, 1 insertion(+) 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 7e2b400cd3527..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) { From 24f252b8f292331b0eecbea49b7584650a13a00e Mon Sep 17 00:00:00 2001 From: William Brafford Date: Fri, 13 Mar 2020 09:45:04 -0400 Subject: [PATCH 10/14] Remove redundant convenience method --- .../admin/cluster/node/info/NodesInfoRequest.java | 8 -------- .../rest/action/cat/RestNodesAction.java | 12 +++++++----- .../rest/action/cat/RestThreadPoolAction.java | 9 ++++++--- 3 files changed, 13 insertions(+), 16 deletions(-) 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 8da5fdfe91611..b491e970eddb9 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 @@ -118,14 +118,6 @@ public NodesInfoRequest addMetrics(Collection metrics) { return this; } - /** - * Add String array of metrics - */ - public NodesInfoRequest addMetrics(String... metrics) { - addMetrics(Arrays.stream(metrics).collect(Collectors.toSet())); - return this; - } - /** * Remove metric */ 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 978589e28005c..d48dea93416a5 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 @@ -64,6 +64,7 @@ import java.util.List; import java.util.Locale; import java.util.stream.Collectors; +import java.util.stream.Stream; import static org.elasticsearch.rest.RestRequest.Method.GET; @@ -97,11 +98,12 @@ public RestChannelConsumer doCatRequest(final RestRequest request, final NodeCli @Override public void processResponse(final ClusterStateResponse clusterStateResponse) { NodesInfoRequest nodesInfoRequest = new NodesInfoRequest(); - nodesInfoRequest.clear().addMetrics( - NodesInfoRequest.Metric.JVM.metricName(), - NodesInfoRequest.Metric.OS.metricName(), - NodesInfoRequest.Metric.PROCESS.metricName(), - NodesInfoRequest.Metric.HTTP.metricName()); + nodesInfoRequest.clear().addMetrics(Stream.of( + NodesInfoRequest.Metric.JVM.metricName(), + NodesInfoRequest.Metric.OS.metricName(), + NodesInfoRequest.Metric.PROCESS.metricName(), + NodesInfoRequest.Metric.HTTP.metricName() + ).collect(Collectors.toSet())); 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/RestThreadPoolAction.java b/server/src/main/java/org/elasticsearch/rest/action/cat/RestThreadPoolAction.java index b56901b5e610d..09ad9d8c71142 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 @@ -46,6 +46,8 @@ import java.util.Map; import java.util.Set; import java.util.TreeMap; +import java.util.stream.Collectors; +import java.util.stream.Stream; import static org.elasticsearch.rest.RestRequest.Method.GET; @@ -80,9 +82,10 @@ public RestChannelConsumer doCatRequest(final RestRequest request, final NodeCli @Override public void processResponse(final ClusterStateResponse clusterStateResponse) { NodesInfoRequest nodesInfoRequest = new NodesInfoRequest(); - nodesInfoRequest.clear().addMetrics( - NodesInfoRequest.Metric.PROCESS.metricName(), - NodesInfoRequest.Metric.THREAD_POOL.metricName()); + nodesInfoRequest.clear().addMetrics(Stream.of( + NodesInfoRequest.Metric.PROCESS.metricName(), + NodesInfoRequest.Metric.THREAD_POOL.metricName() + ).collect(Collectors.toSet())); client.admin().cluster().nodesInfo(nodesInfoRequest, new RestActionListener(channel) { @Override public void processResponse(final NodesInfoResponse nodesInfoResponse) { From c8780fdc001683746b6544edff42f921f5f325cb Mon Sep 17 00:00:00 2001 From: William Brafford Date: Mon, 16 Mar 2020 12:58:04 -0400 Subject: [PATCH 11/14] Make setter variatic instead of set-based --- .../cluster/node/info/NodesInfoRequest.java | 21 ++++++++++++---- .../admin/cluster/RestNodesInfoAction.java | 2 +- .../rest/action/cat/RestNodesAction.java | 5 ++-- .../rest/action/cat/RestThreadPoolAction.java | 5 ++-- .../node/info/NodesInfoRequestTests.java | 24 +++++++++++-------- 5 files changed, 36 insertions(+), 21 deletions(-) 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 b491e970eddb9..2b0884944b5ae 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,7 +27,10 @@ import java.io.IOException; import java.util.Arrays; import java.util.Collection; +import java.util.HashSet; import java.util.Set; +import java.util.SortedSet; +import java.util.TreeSet; import java.util.stream.Collectors; /** @@ -110,11 +113,21 @@ public NodesInfoRequest addMetric(String metric) { /** * Add collection of metrics */ - public NodesInfoRequest addMetrics(Collection metrics) { - if (Metric.allMetrics().containsAll(metrics) == false) { - throw new IllegalStateException("Used an illegal metric: " + metrics); + public NodesInfoRequest addMetrics(String... metrics) { + SortedSet illegalMetrics = new TreeSet<>(); + Set validMetrics = new HashSet<>(); + for (String metric : metrics) { + if (Metric.allMetrics().contains(metric)) { + validMetrics.add(metric); + } else { + illegalMetrics.add(metric); + } } - requestedMetrics.addAll(metrics); + if (illegalMetrics.isEmpty() == false) { + String plural = illegalMetrics.size() == 1 ? "" : "s"; + throw new IllegalStateException("Used illegal metric" + plural + ": " + illegalMetrics); + } + requestedMetrics.addAll(validMetrics); return this; } 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 1a578848e6e8e..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 @@ -100,7 +100,7 @@ static NodesInfoRequest prepareRequest(final RestRequest request) { nodesInfoRequest.clear(); // disregard unknown metrics metrics.retainAll(ALLOWED_METRICS); - nodesInfoRequest.addMetrics(metrics); + nodesInfoRequest.addMetrics(metrics.toArray(String[]::new)); } return nodesInfoRequest; } 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 d48dea93416a5..a1d9c12fbaf52 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 @@ -98,12 +98,11 @@ public RestChannelConsumer doCatRequest(final RestRequest request, final NodeCli @Override public void processResponse(final ClusterStateResponse clusterStateResponse) { NodesInfoRequest nodesInfoRequest = new NodesInfoRequest(); - nodesInfoRequest.clear().addMetrics(Stream.of( + nodesInfoRequest.clear().addMetrics( NodesInfoRequest.Metric.JVM.metricName(), NodesInfoRequest.Metric.OS.metricName(), NodesInfoRequest.Metric.PROCESS.metricName(), - NodesInfoRequest.Metric.HTTP.metricName() - ).collect(Collectors.toSet())); + 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/RestThreadPoolAction.java b/server/src/main/java/org/elasticsearch/rest/action/cat/RestThreadPoolAction.java index 09ad9d8c71142..8f9cd72172072 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 @@ -82,10 +82,9 @@ public RestChannelConsumer doCatRequest(final RestRequest request, final NodeCli @Override public void processResponse(final ClusterStateResponse clusterStateResponse) { NodesInfoRequest nodesInfoRequest = new NodesInfoRequest(); - nodesInfoRequest.clear().addMetrics(Stream.of( + nodesInfoRequest.clear().addMetrics( NodesInfoRequest.Metric.PROCESS.metricName(), - NodesInfoRequest.Metric.THREAD_POOL.metricName() - ).collect(Collectors.toSet())); + 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 ae720bc6274a7..dbc66d56d1bde 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 @@ -30,7 +30,6 @@ import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.in; import static org.hamcrest.Matchers.not; -import static org.hamcrest.Matchers.startsWith; /** * Granular tests for the {@link NodesInfoRequest} class. Higher-level tests @@ -44,7 +43,7 @@ public class NodesInfoRequestTests extends ESTestCase { */ public void testAddMetricsSet() throws Exception { NodesInfoRequest request = new NodesInfoRequest(randomAlphaOfLength(8)); - request.addMetrics(randomSubsetOf(NodesInfoRequest.Metric.allMetrics())); + randomSubsetOf(NodesInfoRequest.Metric.allMetrics()).forEach(request::addMetric); NodesInfoRequest deserializedRequest = roundTripRequest(request); assertThat(request.requestedMetrics(), equalTo(deserializedRequest.requestedMetrics())); } @@ -111,21 +110,26 @@ public void testNodesInfoRequestClear() throws Exception { * Test that (for now) we can only add metrics from a set of known metrics. */ public void testUnknownMetricsRejected() { - String unknownMetric = "unknown_metric1"; + String unknownMetric1 = "unknown_metric1"; + String unknownMetric2 = "unknown_metric2"; Set unknownMetrics = new HashSet<>(); - unknownMetrics.add(unknownMetric); + unknownMetrics.add(unknownMetric1); unknownMetrics.addAll(randomSubsetOf(NodesInfoRequest.Metric.allMetrics())); NodesInfoRequest request = new NodesInfoRequest(); - IllegalStateException exception = expectThrows(IllegalStateException.class, () -> request.addMetric(unknownMetric)); - assertThat(exception.getMessage(), equalTo("Used an illegal metric: " + unknownMetric)); + IllegalStateException exception = expectThrows(IllegalStateException.class, () -> request.addMetric(unknownMetric1)); + assertThat(exception.getMessage(), equalTo("Used an illegal metric: " + unknownMetric1)); - exception = expectThrows(IllegalStateException.class, () -> request.removeMetric(unknownMetric)); - assertThat(exception.getMessage(), equalTo("Used an illegal metric: " + unknownMetric)); + exception = expectThrows(IllegalStateException.class, () -> request.removeMetric(unknownMetric1)); + assertThat(exception.getMessage(), equalTo("Used an illegal metric: " + unknownMetric1)); - exception = expectThrows(IllegalStateException.class, () -> request.addMetrics(unknownMetrics)); - assertThat(exception.getMessage(), startsWith("Used an illegal metric: ")); + 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(String.format("Used illegal metrics: [%s, %s]", unknownMetric1, unknownMetric2))); } /** From d3665f95c6d7a37e3d0246b6c9d577264a56ddd3 Mon Sep 17 00:00:00 2001 From: William Brafford Date: Mon, 16 Mar 2020 14:16:27 -0400 Subject: [PATCH 12/14] Fixup comments and imports --- .../action/admin/cluster/node/info/NodesInfoRequest.java | 3 +-- .../elasticsearch/rest/action/cat/RestNodesAction.java | 1 - .../rest/action/cat/RestThreadPoolAction.java | 2 -- .../admin/cluster/node/info/NodesInfoRequestTests.java | 8 +++----- 4 files changed, 4 insertions(+), 10 deletions(-) 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 2b0884944b5ae..c1a87ce543522 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 @@ -26,7 +26,6 @@ import java.io.IOException; import java.util.Arrays; -import java.util.Collection; import java.util.HashSet; import java.util.Set; import java.util.SortedSet; @@ -111,7 +110,7 @@ public NodesInfoRequest addMetric(String metric) { } /** - * Add collection of metrics + * Add multiple metrics */ public NodesInfoRequest addMetrics(String... metrics) { SortedSet illegalMetrics = new TreeSet<>(); 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 a1d9c12fbaf52..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 @@ -64,7 +64,6 @@ import java.util.List; import java.util.Locale; import java.util.stream.Collectors; -import java.util.stream.Stream; import static org.elasticsearch.rest.RestRequest.Method.GET; 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 8f9cd72172072..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 @@ -46,8 +46,6 @@ import java.util.Map; import java.util.Set; import java.util.TreeMap; -import java.util.stream.Collectors; -import java.util.stream.Stream; import static org.elasticsearch.rest.RestRequest.Method.GET; 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 dbc66d56d1bde..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 @@ -85,8 +85,7 @@ public void testNodesInfoRequestDefaults() { } /** - * Test that the {@link NodesInfoRequest#all()} method sets all of the - * metrics to {@code true}. + * Test that the {@link NodesInfoRequest#all()} method enables all metrics. */ public void testNodesInfoRequestAll() throws Exception { NodesInfoRequest request = new NodesInfoRequest("node"); @@ -96,8 +95,7 @@ public void testNodesInfoRequestAll() throws Exception { } /** - * Test that the {@link NodesInfoRequest#clear()} method sets all of the - * metrics to {@code false}. + * Test that the {@link NodesInfoRequest#clear()} method disables all metrics. */ public void testNodesInfoRequestClear() throws Exception { NodesInfoRequest request = new NodesInfoRequest("node"); @@ -129,7 +127,7 @@ public void testUnknownMetricsRejected() { unknownMetrics.add(unknownMetric2); exception = expectThrows(IllegalStateException.class, () -> request.addMetrics(unknownMetrics.toArray(String[]::new))); - assertThat(exception.getMessage(), equalTo(String.format("Used illegal metrics: [%s, %s]", unknownMetric1, unknownMetric2))); + assertThat(exception.getMessage(), equalTo("Used illegal metrics: [" + unknownMetric1 + ", " + unknownMetric2 + "]")); } /** From 824e5715eea170f3a5e3e67c2f11a076a2044dec Mon Sep 17 00:00:00 2001 From: William Brafford Date: Tue, 24 Mar 2020 21:06:23 -0400 Subject: [PATCH 13/14] Streamline set-checking logic --- .../cluster/node/info/NodesInfoRequest.java | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) 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 c1a87ce543522..1b723834f9d88 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 @@ -113,20 +113,13 @@ public NodesInfoRequest addMetric(String metric) { * Add multiple metrics */ public NodesInfoRequest addMetrics(String... metrics) { - SortedSet illegalMetrics = new TreeSet<>(); - Set validMetrics = new HashSet<>(); - for (String metric : metrics) { - if (Metric.allMetrics().contains(metric)) { - validMetrics.add(metric); - } else { - illegalMetrics.add(metric); - } + 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); } - if (illegalMetrics.isEmpty() == false) { - String plural = illegalMetrics.size() == 1 ? "" : "s"; - throw new IllegalStateException("Used illegal metric" + plural + ": " + illegalMetrics); - } - requestedMetrics.addAll(validMetrics); + requestedMetrics.addAll(metricsSet); return this; } From 014385cb49ed787884b7d756459c9b00385c7174 Mon Sep 17 00:00:00 2001 From: William Brafford Date: Tue, 24 Mar 2020 22:09:40 -0400 Subject: [PATCH 14/14] Remove unused import --- .../action/admin/cluster/node/info/NodesInfoRequest.java | 1 - 1 file changed, 1 deletion(-) 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 1b723834f9d88..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 @@ -26,7 +26,6 @@ import java.io.IOException; import java.util.Arrays; -import java.util.HashSet; import java.util.Set; import java.util.SortedSet; import java.util.TreeSet;