Add spec for cluster stats metric and index_metric filter paths.

[SwethaGuptha]

Description

Adding specs for path /_cluster/stats/{metric}/nodes/{nodeId} and /_cluster/stats/{metric}/{index_metric}/nodes/{nodeId} in cluster/stats API. With opensearch-project/OpenSearch#15938, the cluster/stats API will have filtering support that will allow the clients to fetch only specific metrics from cluster/stats instead of all metrics.

Issues Resolved

List any issues this PR will resolve, e.g. Closes [...].

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]

Changes Analysis

Commit SHA: 3ceb510
Comparing To SHA: fce5721

API Changes

Summary

├─┬Paths
│ ├──[➕] path (1840:3)
│ └──[➕] path (1862:3)
└─┬Components
  ├──[➕] parameters (17278:7)
  ├──[➕] parameters (17288:7)
  ├──[➕] schemas (47763:7)
  ├──[➕] schemas (47792:7)
  ├─┬cluster.stats___StatsResponseBase
  │ ├─┬ALLOF
  │ │ └──[🔀] $ref (52964:13)❌ 
  │ └─┬ALLOF
  │   └──[🔀] $ref (47850:11)❌ 
  ├─┬cluster.stats___ClusterNodes
  │ ├──[➖] required (47479:11)❌ 
  │ ├──[➖] required (47480:11)❌ 
  │ ├──[➖] required (47483:11)❌ 
  │ ├──[➖] required (47485:11)❌ 
  │ ├──[➖] required (47486:11)❌ 
  │ ├──[➖] required (47487:11)❌ 
  │ ├──[➖] required (47478:11)❌ 
  │ ├──[➖] required (47481:11)❌ 
  │ ├──[➖] required (47482:11)❌ 
  │ ├──[➖] required (47484:11)❌ 
  │ └──[➖] required (47477:11)❌ 
  └─┬cluster.stats___ClusterIndices
    ├──[➖] required (47263:11)❌ 
    ├──[➖] required (47261:11)❌ 
    ├──[➖] required (47262:11)❌ 
    ├──[➖] required (47265:11)❌ 
    ├──[➖] required (47266:11)❌ 
    ├──[➖] required (47267:11)❌ 
    ├──[➖] required (47268:11)❌ 
    ├──[➖] required (47269:11)❌ 
    ├──[➖] required (47260:11)❌ 
    └──[➖] required (47264:11)❌ 

Document Element	Total Changes	Breaking Changes
components	27	25
paths	2	0

• ❌ BREAKING Changes: 25 out of 29
• Modifications: 2
• Removals: 21
• Additions: 6
• Breaking Removals: 21
• Breaking Modifications: 2

Report

The full API changes report is available at: https://github.com/opensearch-project/opensearch-api-specification/actions/runs/11826040133/artifacts/2184467167

API Coverage

	Before	After	Δ
Covered (%)	588 (57.59 %)	588 (57.59 %)	0 (0 %)
Uncovered (%)	433 (42.41 %)	433 (42.41 %)	0 (0 %)
Unknown	40	42	2

[dblock]

Looking good.

1. Bump the SHA for a more recent build of 2.18 and 3.0 in https://github.com/opensearch-project/opensearch-api-specification/blob/main/.github/workflows/test-spec.yml#L45 until tests pass.
2. Add to CHANGELOG.

[dblock]

@SwethaGuptha Looks like the build has other problems (missing other plugins like SQL and ISM). This is a known and rather annoying problem, 3.x builds don't always have everything. You should chase what's broken in OpenSearch main distribution.

I opened opensearch-project/opensearch-build#5130 to see what the infra team says about this (cc: @gaiksaya).

@gaiksaya

[SwethaGuptha]

@SwethaGuptha Looks like the build has other problems (missing other plugins like SQL and ISM). This is a known and rather annoying problem, 3.x builds don't always have everything. You should chase what's broken in OpenSearch main distribution.

I opened opensearch-project/opensearch-build#5130 to see what the infra team says about this (cc: @gaiksaya).

@gaiksaya

Thanks @dblock for creating the issue. Did the tests work for v#3.0.0 previously? I see there are some failures from core OS as well. Also,I see the latest image available for v#3.0 is from eight days ago, do you know what cadence is followed for updating the version image?

[dblock]

Thanks @dblock for creating the issue. Did the tests work for v#3.0.0 previously? I see there are some failures from core OS as well.

Yes, the failure on main right now came from #625 and will get fixed soon.

Also,I see the latest image available for v#3.0 is from eight days ago, do you know what cadence is followed for updating the version image?

I think they try to build one daily, but it doesn't always work :(

[gaiksaya]

I opened opensearch-project/opensearch-build#5130 to see what the infra team says about this (cc: @gaiksaya).

@gaiksaya

Will add more details to the issue but docker image is build using tarball. And all the distributions contains distribution manifest with all the information about a distribution and the components bundled in that distribution.

[dblock]

I opened opensearch-project/opensearch-build#5130 to see what the infra team says about this (cc: @gaiksaya).
@gaiksaya

Will add more details to the issue but docker image is build using tarball. And all the distributions contains distribution manifest with all the information about a distribution and the components bundled in that distribution.

It's just hard to tell from a distribution what the docker SHA is.

[SwethaGuptha]

@dblock Any inputs on the next steps to make progress on this PR?

[dblock]

@SwethaGuptha yes, update the SHAs to the latest available for 2.18 and 3.0 and see if all tests pass - it has to pass 2.18, but you can mark tests as < 3.0 if that one doesn't pass, for now

[dblock]

Now that 2.18 has been released this should be easier.

[github-actions]

Spec Test Coverage Analysis

Total	Tested
517	349 (67.5 %)

[SwethaGuptha]

@dblock Can you please help with the review?

[dblock]

Looks good, some cosmetic asks, please.

[dblock]

Sorry to be annoying, some more small stuff flagged here.

[dblock]

Thanks for taking this on, merged!

There's also node stats, do those have to mirror cluster stats?

 GET /_nodes/{node_id}/stats/{metric}/{index_metric}
 GET /_nodes/{node_id}/usage
 GET /_nodes/{node_id}/usage/{metric}
 GET /_nodes/{node_id}/{metric}

[nhtruong]

@dblock
So cluster.stats will have the following paths:

• /_cluster/stats
• /_cluster/stats/nodes/{node_id}
• /_cluster/stats/{metric}/nodes/{node_id}
• /_cluster/stats/{metric}/{index_metric}/nodes/{node_id}

current path-finding strat for the JS CodeGen will create:

  const path = ['/_cluster/stats/', metric, '/', index_metric, '/nodes/', node_id].filter(c => c).join('').replace('//', '/');

which works fine for most situations EXCEPT for when no path params are provided since it will yield /_cluster/stats/nodes, which is not a valid path. I've gotta update the CodeGen for JS (I really like the above solution if not for this edge case 😢 )
cc: @Xtansia in case this affects Java/.NET CodeGen too.

[dblock]

@nhtruong I think these are 2 separate APIs, one is cluster stats (/_cluster/stats), and the other is cluster/nodes stats (the other 3), would that help?

[nhtruong]

I'm referring to these 4 operations: https://github.com/opensearch-project/opensearch-api-specification/pull/639/files#diff-c68dc8d4bd22aabd667fe0978fba9a121988d77ac766b1e2a5e94091603c0734R305-R366

It's no big deal. We just can't have that one-line solution 🥲

[dblock]

@nhtruong Yes, I think they should be totally different operations, no? would that not work?

[nhtruong]

They are in the same operation-group which means they will be piped into the same API function on the client. That function has to pick a path depending on the path params provided by the user. NodeJS CodeGen is using that one-line solution which works for the vast majority of the operation groups, even the notorious nodes.info:

• /_nodes
• /_nodes/{node_id}
• /_nodes/{metric}
• /_nodes/{node_id}/{metric}

const path = ['/_nodes/', node_id, '/', metric].filter(c => c).join('').replace('//', '/');

It's not hard to switch up the path selection logic to account for this edge case.

[dblock]

They are in the same operation-group

Is this correct?

[nhtruong]

Yes, the 4 operations I stated in #639 (comment) are in cluster.stats

[SwethaGuptha]

current path-finding strat for the JS CodeGen will create:
const path = ['/_cluster/stats/', metric, '/', index_metric, '/nodes/', node_id].filter(c => c).join('').replace('//', '/');
which works fine for most situations EXCEPT for when no path params are provided since it will yield /_cluster/stats/nodes, which is not a valid path

@nhtruong Trying to understand the problem here, was never a valid path even earlier. How is handling of two new paths causing a challenge in the Code Gen?

[dblock]

Yes, the 4 operations I stated in #639 (comment) are in cluster.stats

What I am saying is that it is incorrect and that we have 2 separate APIs here, one which is cluster.stats and another which is cluster.nodes.stats.

[nhtruong]

Sorry I missed this earlier.

Trying to understand the problem here, was never a valid path even earlier. How is handling of two new paths causing a challenge in the Code Gen?

This PR wasn't the cause. Reading through this made me realize that the paths for this operation group will cause issue for the JS CodeGen specifically because of its peculiar strategy to build the path. And this is the only group with paths like these.

We also have other edge cases where the server accepts both _alias and _aliases so we endup with paths that are aliases of each other (pun intended 😄):

• /{index}/_aliases/{name}
• /{index}/_alias/{name}

Another path edge case that you're already aware of is the {node_id_or_metric} path param. All CodeGens have to parse the spec and handle these edge cases (there are more unrelated to paths) in their own language. One thing I'd like to do is create a CodeGen friendly version of the spec that has already have these edge cases processed. So, we only have to solve them once instead of repeating the same logic in different languages.