Skip to content

Commit

Permalink
Do not request "search_pipelines" metrics by default in NodesInfoRequ…
Browse files Browse the repository at this point in the history
…est (opensearch-project#12497)

This commit introduces a breaking change:

NodesInfoRequest does not request all metrics by default.

There are metrics there are not included in the default set of metrics. Right
now "search_pipelines" metric is not included in the default set of metrics.

Signed-off-by: Lukáš Vlček <[email protected]>
  • Loading branch information
lukas-vlcek authored Mar 12, 2024
1 parent e6eec36 commit f873d0e
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 11 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
- Add task completion count in search backpressure stats API ([#10028](https://github.com/opensearch-project/OpenSearch/pull/10028/))
- Deprecate CamelCase `PathHierarchy` tokenizer name in favor to lowercase `path_hierarchy` ([#10894](https://github.com/opensearch-project/OpenSearch/pull/10894))
- Switched to more reliable OpenSearch Lucene snapshot location([#11728](https://github.com/opensearch-project/OpenSearch/pull/11728))
- Breaking change: Do not request "search_pipelines" metrics by default in NodesInfoRequest ([#12497](https://github.com/opensearch-project/OpenSearch/pull/12497))

### Deprecated

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@
@PublicApi(since = "1.0.0")
public class NodesInfoRequest extends BaseNodesRequest<NodesInfoRequest> {

private Set<String> requestedMetrics = Metric.allMetrics();
private Set<String> requestedMetrics = Metric.defaultMetrics();

/**
* Create a new NodeInfoRequest from a {@link StreamInput} object.
Expand All @@ -73,7 +73,7 @@ public NodesInfoRequest(StreamInput in) throws IOException {
*/
public NodesInfoRequest(String... nodesIds) {
super(nodesIds);
all();
defaultMetrics();
}

/**
Expand All @@ -85,13 +85,24 @@ public NodesInfoRequest clear() {
}

/**
* Sets to return all the data.
* Sets to return data for all the metrics.
* See {@link Metric}
*/
public NodesInfoRequest all() {
requestedMetrics.addAll(Metric.allMetrics());
return this;
}

/**
* Sets to return data for default metrics only.
* See {@link Metric}
* See {@link Metric#defaultMetrics()}.
*/
public NodesInfoRequest defaultMetrics() {
requestedMetrics.addAll(Metric.defaultMetrics());
return this;
}

/**
* Get the names of requested metrics
*/
Expand Down Expand Up @@ -156,7 +167,7 @@ public void writeTo(StreamOutput out) throws IOException {

/**
* An enumeration of the "core" sections of metrics that may be requested
* from the nodes information endpoint. Eventually this list list will be
* from the nodes information endpoint. Eventually this list will be
* pluggable.
*/
public enum Metric {
Expand Down Expand Up @@ -187,8 +198,25 @@ boolean containedIn(Set<String> metricNames) {
return metricNames.contains(this.metricName());
}

/**
* Return all available metrics.
* See {@link Metric}
*/
public static Set<String> allMetrics() {
return Arrays.stream(values()).map(Metric::metricName).collect(Collectors.toSet());
}

/**
* Return "the default" set of metrics.
* Similar to {@link #allMetrics()} except {@link Metric#SEARCH_PIPELINES} metric is not included.
* <br>
* The motivation to define the default set of metrics was to keep the default response
* size at bay. Metrics that are NOT included in the default set were typically introduced later
* and are considered to contain specific type of information that is not usually useful unless you
* know that you really need it.
*/
public static Set<String> defaultMetrics() {
return allMetrics().stream().filter(metric -> !(metric.equals(SEARCH_PIPELINES.metricName()))).collect(Collectors.toSet());
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ protected void clusterManagerOperation(
ClusterState state,
ActionListener<AcknowledgedResponse> listener
) throws Exception {
NodesInfoRequest nodesInfoRequest = new NodesInfoRequest();
NodesInfoRequest nodesInfoRequest = new NodesInfoRequest().clear().addMetric(NodesInfoRequest.Metric.SEARCH_PIPELINES.metricName());
client.admin().cluster().nodesInfo(nodesInfoRequest, ActionListener.wrap(nodeInfos -> {
Map<DiscoveryNode, SearchPipelineInfo> searchPipelineInfos = new HashMap<>();
for (NodeInfo nodeInfo : nodeInfos.getNodes()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,15 +86,18 @@ public void testRemoveSingleMetric() throws Exception {
}

/**
* Test that a newly constructed NodesInfoRequestObject requests all of the
* possible metrics defined in {@link NodesInfoRequest.Metric}.
* Test that a newly constructed NodesInfoRequestObject does not request all the
* possible metrics defined in {@link NodesInfoRequest.Metric} but only the default metrics
* according to {@link NodesInfoRequest.Metric#defaultMetrics()}.
*/
public void testNodesInfoRequestDefaults() {
NodesInfoRequest defaultNodesInfoRequest = new NodesInfoRequest(randomAlphaOfLength(8));
NodesInfoRequest allMetricsNodesInfoRequest = new NodesInfoRequest(randomAlphaOfLength(8));
allMetricsNodesInfoRequest.all();
NodesInfoRequest requestOOTB = new NodesInfoRequest(randomAlphaOfLength(8));
NodesInfoRequest requestAll = new NodesInfoRequest(randomAlphaOfLength(8)).all();
NodesInfoRequest requestDefault = new NodesInfoRequest(randomAlphaOfLength(8)).defaultMetrics();

assertThat(defaultNodesInfoRequest.requestedMetrics(), equalTo(allMetricsNodesInfoRequest.requestedMetrics()));
assertTrue(requestAll.requestedMetrics().size() > requestOOTB.requestedMetrics().size());
assertTrue(requestDefault.requestedMetrics().size() == requestOOTB.requestedMetrics().size());
assertThat(requestOOTB.requestedMetrics(), equalTo(requestDefault.requestedMetrics()));
}

/**
Expand All @@ -107,6 +110,21 @@ public void testNodesInfoRequestAll() throws Exception {
assertThat(request.requestedMetrics(), equalTo(NodesInfoRequest.Metric.allMetrics()));
}

/**
* Test that the {@link NodesInfoRequest#defaultMetrics()} method enables default metrics.
*/
public void testNodesInfoRequestDefault() {
NodesInfoRequest request = new NodesInfoRequest("node");
request.defaultMetrics();

assertEquals(11, request.requestedMetrics().size());
assertThat(request.requestedMetrics(), equalTo(NodesInfoRequest.Metric.defaultMetrics()));
assertTrue(request.requestedMetrics().contains(NodesInfoRequest.Metric.JVM.metricName()));
assertTrue(request.requestedMetrics().contains(NodesInfoRequest.Metric.AGGREGATIONS.metricName()));
// search_pipelines metrics are not included
assertFalse(request.requestedMetrics().contains(NodesInfoRequest.Metric.SEARCH_PIPELINES.metricName()));
}

/**
* Test that the {@link NodesInfoRequest#clear()} method disables all metrics.
*/
Expand Down

0 comments on commit f873d0e

Please sign in to comment.