Add support to cluster stats API for enabling/disabling mapping_stats…

[SwethaGuptha]

… and analysis_stats at request level.

Description

Continuing on #14447 to make analysis_stats and mapping_stats information optional in _cluster/stats API but limiting the scope to request level only. Two request params namely include_mapping_stats and include_analysis_stats has been introduced to enable/disable these stats info from the cluster stats API. By default these stats info will be part of the API response.

REST request formats to exclude the information for

• analysis_stats - localhost:9200/_cluster/stats?include_mapping_stats=false
• mapping_stats - localhost:9200/_cluster/stats?include_analysis_stats=false
• analysis_stats and mapping_stats - localhost:9200/_cluster/stats?include_analysis_stats=false&include_mapping_stats=false

Related Issues

Resolves #[Issue number to be closed when this PR is merged]
#14447

Check List

• Functionality includes testing.
• API changes companion pull request created, if applicable.
• Public documentation issue/PR created, if applicable.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[github-actions]

❌ Gradle check result for 839fbe0: null

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[github-actions]

❕ Gradle check result for 79d1b90: UNSTABLE

• TEST FAILURES:

      1 org.opensearch.http.SearchRestCancellationIT.testAutomaticCancellationMultiSearchDuringFetchPhase

Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

[github-actions]

✅ Gradle check result for 8f4e25f: SUCCESS

[codecov]

Codecov Report

Attention: Patch coverage is 82.75862% with 5 lines in your changes missing coverage. Please review.

Project coverage is 71.82%. Comparing base (ed65482) to head (65b6b00).
Report is 354 commits behind head on main.

Files with missing lines	Patch %	Lines
...tion/admin/cluster/stats/ClusterStatsResponse.java	66.66%	1 Missing and 4 partials ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main   #15377      +/-   ##
============================================
- Coverage     71.96%   71.82%   -0.14%     
+ Complexity    63304    63205      -99     
============================================
  Files          5219     5219              
  Lines        296033   296062      +29     
  Branches      42756    42760       +4     
============================================
- Hits         213026   212654     -372     
- Misses        65520    65970     +450     
+ Partials      17487    17438      -49     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[mgodwan]

APIs like Index Stats, Node Stats already provide a way to handle filtering the required stats through URI Path. Is it possible to implement a similar logic here?

e.g. index stats /{index}/_stats/{metric}
e.g. node stats /_nodes/{nodeId}/stats/{metric}/{index_metric}

We can add a similar route like /_cluster/stats/{metrics}/{sub_metrics} where metrics can be nodes or indices or both. And sub metrics can be specific details within stats like mappings, analysis

This should help maintain a common pattern across different stats APIS

[SwethaGuptha]

Thanks for the inputs @mgodwan but the use-case here is little different. In the the URI path approach client can fetch stats for a specific metric whereas for this issue we want to add support for filtering out some information from the overall response that client does not need.

[mgodwan]

we want to add support for filtering out some information from the overall response that client does not need.

The motivation behind this PR seems to be #14447 which mentions Reduced Overhead and Fine-Grained Control as the benefits

Hence, opting-in to fetch required metrics still seem to be a way which we should explore to handle as it aligns with these benefits while keeping the API signature streamlined across different stats APIs.

e.g. if new fields are added to the cluster stats response in a new OS version, users may start to see the overhead similar to what is mentioned in the parent issue due to some new computation happening. Opt-in mechanism like URI path ensures users only get what they want from cluster stats if they are sensitive to performance of these APIs or are concerned about computation overhead of these on the cluster. And as existing APIs work, if no filter is provided, it defaults to ALL which means all stats should be returned, and continues to maintain a backward compatible way for this existing API as well.

[SwethaGuptha]

I get the concerns, but I don't understand how we can use the URI paths to selective exclude some stats from the default response. For APIs like _cluster/health we support request params include_defaults to additionally fetch default settings in addition to the default response. Conversely, in this use-case request params is being used to exclude some information from the default response.

By default the value of the params is set to true which ensures the default behavior of API remains unchanged and is backward compatible. If the confusion is arising from the choice of param names, would exclude_mapping_stats and exclude_analysis_stats be a better logical choice?

[shwetathareja]

@mgodwan I would prefer the uri based approach. But, like @SwethaGuptha mentioned we need to maintain backward compatibility so can't turn it off by default. Now, we can explore the URI approach where metric can excluded/ included in via sub-metric

[sumitasr]

Have we discussed these param names anywhere?

[SwethaGuptha]

No there was no separate discussion for these param names. It can be part of this PR itself? open for suggestions.

[github-actions]

❌ Gradle check result for 5d8aacc: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[github-actions]

❌ Gradle check result for 849c840: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[github-actions]

❌ Gradle check result for edc3eae: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[SwethaGuptha]

❌ Gradle check result for edc3eae: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

Flaky test #15204

[github-actions]

❕ Gradle check result for 65b6b00: UNSTABLE

Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

[opensearch-trigger-bot]

This PR is stalled because it has been open for 30 days with no activity.

[opensearch-trigger-bot]

This PR is stalled because it has been open for 30 days with no activity.