From 4eadb384188162260bc14d9804e97505b6ed4f22 Mon Sep 17 00:00:00 2001 From: Swetha Guptha Date: Fri, 4 Oct 2024 19:52:03 +0530 Subject: [PATCH] Add comments. Signed-off-by: Swetha Guptha --- .../cluster/stats/ClusterStatsRequest.java | 2 + .../admin/cluster/RestClusterStatsAction.java | 49 ++++++----- .../ClusterStatsRequestBuilderTests.java | 81 ------------------- .../stats/ClusterStatsResponseTests.java | 1 + 4 files changed, 27 insertions(+), 106 deletions(-) delete mode 100644 server/src/test/java/org/opensearch/action/admin/cluster/stats/ClusterStatsRequestBuilderTests.java diff --git a/server/src/main/java/org/opensearch/action/admin/cluster/stats/ClusterStatsRequest.java b/server/src/main/java/org/opensearch/action/admin/cluster/stats/ClusterStatsRequest.java index 1bfacd7cc6451..1c929881b898b 100644 --- a/server/src/main/java/org/opensearch/action/admin/cluster/stats/ClusterStatsRequest.java +++ b/server/src/main/java/org/opensearch/action/admin/cluster/stats/ClusterStatsRequest.java @@ -189,6 +189,8 @@ public int getIndex() { /** * An enumeration of the "core" sections of indices metrics that may be requested * from the cluster stats endpoint. + * + * When no value is provided for param index_metric, default filter is set to _all. */ @PublicApi(since = "3.0.0") public enum IndexMetric { diff --git a/server/src/main/java/org/opensearch/rest/action/admin/cluster/RestClusterStatsAction.java b/server/src/main/java/org/opensearch/rest/action/admin/cluster/RestClusterStatsAction.java index f10ac9e4a1f9b..47f3e048c516a 100644 --- a/server/src/main/java/org/opensearch/rest/action/admin/cluster/RestClusterStatsAction.java +++ b/server/src/main/java/org/opensearch/rest/action/admin/cluster/RestClusterStatsAction.java @@ -108,6 +108,7 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC public static ClusterStatsRequest fromRequest(final RestRequest request) { Set metrics = Strings.tokenizeByCommaToSet(request.param("metric", "_all")); + // Value for param index_metric defaults to _all when indices metric or all metrics are requested. String indicesMetricsDefaultValue = metrics.contains(Metric.INDICES.metricName()) || metrics.contains("_all") ? "_all" : null; Set indexMetrics = Strings.tokenizeByCommaToSet(request.param("index_metric", indicesMetricsDefaultValue)); String[] nodeIds = request.paramAsStringArray("nodeId", null); @@ -115,34 +116,32 @@ public static ClusterStatsRequest fromRequest(final RestRequest request) { ClusterStatsRequest clusterStatsRequest = new ClusterStatsRequest().nodesIds(nodeIds); clusterStatsRequest.timeout(request.param("timeout")); clusterStatsRequest.useAggregatedNodeLevelResponses(true); - - if (!metrics.isEmpty()) { - paramValidations(metrics, indexMetrics, request); - final Set metricsRequested = metrics.contains("_all") - ? new HashSet<>(METRIC_REQUEST_CONSUMER_MAP.keySet()) - : new HashSet<>(metrics); - Set invalidMetrics = validateAndSetRequestedMetrics(metricsRequested, METRIC_REQUEST_CONSUMER_MAP, clusterStatsRequest); - if (!invalidMetrics.isEmpty()) { + clusterStatsRequest.computeAllMetrics(false); + + paramValidations(metrics, indexMetrics, request); + final Set metricsRequested = metrics.contains("_all") + ? new HashSet<>(METRIC_REQUEST_CONSUMER_MAP.keySet()) + : new HashSet<>(metrics); + Set invalidMetrics = validateAndSetRequestedMetrics(metricsRequested, METRIC_REQUEST_CONSUMER_MAP, clusterStatsRequest); + if (!invalidMetrics.isEmpty()) { + throw new IllegalArgumentException( + unrecognizedStrings(request, invalidMetrics, METRIC_REQUEST_CONSUMER_MAP.keySet(), "metric") + ); + } + if (metricsRequested.contains(Metric.INDICES.metricName())) { + final Set indexMetricsRequested = indexMetrics.contains("_all") + ? INDEX_METRIC_TO_REQUEST_CONSUMER_MAP.keySet() + : new HashSet<>(indexMetrics); + Set invalidIndexMetrics = validateAndSetRequestedMetrics( + indexMetricsRequested, + INDEX_METRIC_TO_REQUEST_CONSUMER_MAP, + clusterStatsRequest + ); + if (!invalidIndexMetrics.isEmpty()) { throw new IllegalArgumentException( - unrecognizedStrings(request, invalidMetrics, METRIC_REQUEST_CONSUMER_MAP.keySet(), "metric") - ); - } - if (metricsRequested.contains(Metric.INDICES.metricName())) { - final Set indexMetricsRequested = indexMetrics.contains("_all") - ? INDEX_METRIC_TO_REQUEST_CONSUMER_MAP.keySet() - : new HashSet<>(indexMetrics); - Set invalidIndexMetrics = validateAndSetRequestedMetrics( - indexMetricsRequested, - INDEX_METRIC_TO_REQUEST_CONSUMER_MAP, - clusterStatsRequest + unrecognizedStrings(request, invalidIndexMetrics, INDEX_METRIC_TO_REQUEST_CONSUMER_MAP.keySet(), "index metric") ); - if (!invalidIndexMetrics.isEmpty()) { - throw new IllegalArgumentException( - unrecognizedStrings(request, invalidIndexMetrics, INDEX_METRIC_TO_REQUEST_CONSUMER_MAP.keySet(), "index metric") - ); - } } - clusterStatsRequest.computeAllMetrics(false); } return clusterStatsRequest; diff --git a/server/src/test/java/org/opensearch/action/admin/cluster/stats/ClusterStatsRequestBuilderTests.java b/server/src/test/java/org/opensearch/action/admin/cluster/stats/ClusterStatsRequestBuilderTests.java deleted file mode 100644 index 8bc14d4617171..0000000000000 --- a/server/src/test/java/org/opensearch/action/admin/cluster/stats/ClusterStatsRequestBuilderTests.java +++ /dev/null @@ -1,81 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - */ - -package org.opensearch.action.admin.cluster.stats; - -import org.opensearch.action.admin.cluster.stats.ClusterStatsRequest.IndexMetric; -import org.opensearch.action.admin.cluster.stats.ClusterStatsRequest.Metric; -import org.opensearch.test.OpenSearchTestCase; -import org.opensearch.test.client.NoOpClient; -import org.junit.After; -import org.junit.Before; - -import java.util.Set; - -public class ClusterStatsRequestBuilderTests extends OpenSearchTestCase { - - private NoOpClient testClient; - - @Override - @Before - public void setUp() throws Exception { - super.setUp(); - this.testClient = new NoOpClient(getTestName()); - } - - @Override - @After - public void tearDown() throws Exception { - this.testClient.close(); - super.tearDown(); - } - - public void testUseAggregatedNodeLevelResponses() { - ClusterStatsRequestBuilder clusterStatsRequestBuilder = new ClusterStatsRequestBuilder( - this.testClient, - ClusterStatsAction.INSTANCE - ); - clusterStatsRequestBuilder.useAggregatedNodeLevelResponses(false); - assertFalse(clusterStatsRequestBuilder.request().useAggregatedNodeLevelResponses()); - } - - public void testApplyMetricFiltering() { - ClusterStatsRequestBuilder clusterStatsRequestBuilder = new ClusterStatsRequestBuilder( - this.testClient, - ClusterStatsAction.INSTANCE - ); - assertTrue(clusterStatsRequestBuilder.request().computeAllMetrics()); - clusterStatsRequestBuilder.computeAllMetrics(false); - assertFalse(clusterStatsRequestBuilder.request().computeAllMetrics()); - } - - public void testRequestedMetrics() { - ClusterStatsRequestBuilder clusterStatsRequestBuilder = new ClusterStatsRequestBuilder( - this.testClient, - ClusterStatsAction.INSTANCE - ); - clusterStatsRequestBuilder.computeAllMetrics(false); - clusterStatsRequestBuilder.requestMetrics(Set.of(Metric.OS, Metric.JVM)); - assertFalse(clusterStatsRequestBuilder.request().computeAllMetrics()); - assertEquals(Set.of(Metric.OS, Metric.JVM), clusterStatsRequestBuilder.request().requestedMetrics()); - } - - public void testIndicesMetrics() { - ClusterStatsRequestBuilder clusterStatsRequestBuilder = new ClusterStatsRequestBuilder( - this.testClient, - ClusterStatsAction.INSTANCE - ); - clusterStatsRequestBuilder.computeAllMetrics(false); - clusterStatsRequestBuilder.requestMetrics(Set.of(Metric.INDICES, Metric.JVM)); - clusterStatsRequestBuilder.indexMetrics(Set.of(IndexMetric.MAPPINGS, IndexMetric.ANALYSIS)); - assertFalse(clusterStatsRequestBuilder.request().computeAllMetrics()); - assertEquals(Set.of(Metric.INDICES, Metric.JVM), clusterStatsRequestBuilder.request().requestedMetrics()); - assertEquals(Set.of(IndexMetric.MAPPINGS, IndexMetric.ANALYSIS), clusterStatsRequestBuilder.request().indicesMetrics()); - } - -} diff --git a/server/src/test/java/org/opensearch/action/admin/cluster/stats/ClusterStatsResponseTests.java b/server/src/test/java/org/opensearch/action/admin/cluster/stats/ClusterStatsResponseTests.java index abd607d5787dc..ad7706292d93c 100644 --- a/server/src/test/java/org/opensearch/action/admin/cluster/stats/ClusterStatsResponseTests.java +++ b/server/src/test/java/org/opensearch/action/admin/cluster/stats/ClusterStatsResponseTests.java @@ -221,6 +221,7 @@ private ClusterStatsNodeResponse createClusterStatsNodeResponse(DiscoveryNode no null, null, null, + null, null ); return new ClusterStatsNodeResponse(node, null, nodeInfo, nodeStats, shardStats);