SOLR-16458: Node system info V2 api jersey resource

[igiguere]

https://issues.apache.org/jira/browse/SOLR-16458

Jira Ticket

The Excel spreadsheet links to SOLR-16458 for the V1 /solr/admin/info/system and V2 /api/node/system, although the ticket does not mention those URLs.

From the checklist below : "I have created a Jira issue and added the issue ID to my pull request title." - m'well, no. But keeping track of these V2 tickets is probably difficult enough without adding one more.

Sorry about the mix-up in the commit messages. I originally landed on the wrong ticket.

Description

Implementation of a Jersey resource to support getting the system info. This new resource should replace the home-brew "Endpoint" V2 resource.

DRAFT:

• Should we keep the URL path /node/system as suggested in the Excel spreadsheet? This PR suggests /node/info/system. If we keep the same URL path as the "old" V2, then the home-brew "Endpoint" will be deleted.
• AdminHandlersProxy does not support V2, so this PR does not test parameter "nodes". Ref: PR SOLR-16738: Refactor special-casing out of AdminHandlersProxy  #3991, mentioned in PR SOLR-17436: Create a v2 equivalent for /admin/metrics #4057
• I didn't like that the system info returned in the single-node response was not separate from the response header (i.e.: fields "node"(name), "mode", "core_root"... at the same level as "responseHeader").
  -- This PR wrap the system info in "nodeInfo":
  <response>
<lst name="responseHeader"><!-- ... --></lst>
<lst name="nodeInfo">
<str name="node">localhost:8983_solr</str>
<!-- ... -->
</lst>
</response>
  -- Ideally, IMHO, the "node" field should be named "name" (i.e.: the node name), and then the wrapper "nodeInfo" could be just "node"
  -- AdminHandlersProxy wraps all proxied responses into a NamedList where the name is the node name (host:port_solr). This creates a structure with unpredictable (or not easily predictable) keys. If we do adopt a response with the "nodeInfo" wrapper, then, the proxied responses could be added to the list of "nodeInfo"... In a specific implementation of AdminHandlersProxy (ref.SOLR-16738: Refactor special-casing out of AdminHandlersProxy  #3991). This is all a lot of changes, breaking changes for whoever is parsing the response already. Opinions?
• TODO: Clean-up documentation. Path /admin/info/system is mentionned in 5 pages... And there's SOLR-11918 ?

Solution

Add NodeSystemInfoApi (in solr-api), implemented in GetNodeSystemInfo.

Class NodeSystemInfoProvider contains code to provide the system info, copied from SystemInfoHandler.

Clean-up SystemInfoHandler to use GetNodeSystemInfo.

Tests

Add unit tests for NodeSystemInfoProvider (note that the test class for SystemInfoHandler was actually only testing a method now found in NodeSystemInfoProvider).

Add unit tests for GetNodeSystemInfo.

Functional tests on a local instance (dev-slim, started with the "cloud" example)

Checklist

Please review the following and check all that apply:

• I have reviewed the guidelines for How to Contribute and my code conforms to the standards described there to the best of my ability.
• I have created a Jira issue and added the issue ID to my pull request title.
• I have given Solr maintainers access to contribute to my PR branch. (optional but recommended, not available for branches on forks living under an organisation)
• I have developed this patch against the main branch.
• I have run ./gradlew check.
• I have added tests for my changes.
• I have added documentation for the Reference Guide
• I have added a changelog entry for my change

[gerlowskija]

First off, thanks again for sharing this PR and tackling another v2 API!

Should we keep the URL path /node/system [...] This PR wrap the system info in "nodeInfo" [...] the "node" field should be named "name" [...] a lot of changes, breaking changes

I agree with some of your concerns about the "cosmetics" of the request and API-response as they are today. We're free to change v2 APIs and their responses as much as we'd like. But v1 APIs are required to be backwards-compatible: breaking changes can only occur on major-version upgrades. That's not to say that v1 can't be changed, just that if v1 has a "breaking change" then it can only go to 'main', not branch_10x. So we've got some options depending on how much we care about updating v1 vs. v2-only.

What we've done in a few places elsewhere is make whatever improvements we want to the v2 response format, and then if the v1 codepath calls v2 code have the v1 code do some manual massaging to reshape the response into something that meets backwards compatibility. It's not pretty, but it meets the requirements and is code that will eventually go away when we are able to deprecate and remove some of these v1 endpoints. So that's probably what I'd recommend for these sort of smaller response-changes.

I'm somewhat tempted, as I read your comments, to suggest overhauling this endpoint entirely and making it look radically different in our v2 API. Something like splitting it up into a few smaller paths, like /node/jvm, /node/resources, /node/version, etc. But it's probably better to get things ported over to v2 in the current format and then reevaluate? Idk - curious if you have any thoughts on that.

[igiguere]

First off, thanks again for sharing this PR and tackling another v2 API!

Should we keep the URL path /node/system [...] This PR wrap the system info in "nodeInfo" [...] the "node" field should be named "name" [...] a lot of changes, breaking changes

I agree with some of your concerns about the "cosmetics" of the request and API-response as they are today. We're free to change v2 APIs and their responses as much as we'd like. But v1 APIs are required to be backwards-compatible: breaking changes can only occur on major-version upgrades. That's not to say that v1 can't be changed, just that if v1 has a "breaking change" then it can only go to 'main', not branch_10x. So we've got some options depending on how much we care about updating v1 vs. v2-only.

What we've done in a few places elsewhere is make whatever improvements we want to the v2 response format, and then if the v1 codepath calls v2 code have the v1 code do some manual massaging to reshape the response into something that meets backwards compatibility. It's not pretty, but it meets the requirements and is code that will eventually go away when we are able to deprecate and remove some of these v1 endpoints. So that's probably what I'd recommend for these sort of smaller response-changes.

I'm somewhat tempted, as I read your comments, to suggest overhauling this endpoint entirely and making it look radically different in our v2 API. Something like splitting it up into a few smaller paths, like /node/jvm, /node/resources, /node/version, etc. But it's probably better to get things ported over to v2 in the current format and then reevaluate? Idk - curious if you have any thoughts on that.

Thanks for the feedback, @gerlowskija
I will make sure that the V1 response is back-compatible.

As for splitting the new V2 response into smaller paths, it could be a path parameter, at least for "block" of info, like "jvm", "gpu", what else? I'm not sure what /node/version would cover, since there's the Solr version, Lucene version, JVM version. So that might be a query param, like a filter, to get the version of everything that has a version.