[nick-boss-tech]

🤖 AI text below 🤖 (posted on behalf of Nick Shanin)

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

What happens today

The V2 logging endpoint applies level changes only on the node that receives the request, so the Admin UI still sets levels through V1 in SolrCloud mode.

The V2 logging endpoint PUT /api/node/logging/levels only ever applies level changes on the node that receives the request (NodeLogging.java:87-99); NodeLogging carries a TODO to add a nodes parameter once SOLR-16738 lands (NodeLogging.java:47), and that proxy machinery has since landed (the node system info endpoint uses it (GetNodeSystemInfo.java:70-83)). Because of the gap, the Admin UI logging screen still sets levels through the V1 handler in SolrCloud mode (logging.js:164-175), since only V1 understands nodes=all.

The V1 handler has a second route into the same code: it applies level changes whenever a request carries set parameters (LoggingHandler.java:75-79), including on GET requests. Eric Pugh noted on the ticket (2026-10-05) that he stumbled over this while testing: he went looking for his POST or PUT in the browser's network log and realized the GET was changing the log level. This PR does not change that behavior; every GET works exactly as it does today.

What this change does

The set-level endpoint accepts a nodes parameter and fans the change out to the named nodes, reporting one result per node and naming any that did not respond.

The set-level endpoint accepts a nodes query parameter (NodeLoggingApis.java:41-51). When it is present, NodeLogging fans the request out to the named nodes (or every live node for nodes=all) through V2SolrRequestBasedProxy (NodeLogging.java:123-133), mirroring GetNodeSystemInfo (GetNodeSystemInfo.java:70-83), and the response carries one result per node, keyed by node name, plus a failedNodes list naming any requested nodes that did not respond (LoggingResponse.java:31-56). A named node that is not part of the cluster is rejected with a 400 before anything is sent (NodeLogging.java:134-145), so an unknown name cannot half-apply the change. A node that leaves the live set while a broadcast is in flight is a narrower case the pre-check cannot cover; it is in Limits. Using nodes on a standalone node is also a 400, not an NPE (NodeLogging.java:118-121). When nodes is absent the local-only path is unchanged (NodeLogging.java:100-108), and, again mirroring system info, the receiving node does not also apply the change locally when nodes is present; nodes=all covers it because the resolved set includes it (NodeLogging.java:111-114). The Admin UI logging screen now calls the V2 endpoint in both modes (nodes: "all" only in cloud mode) (logging.js:163-174), and the V1 Logging factory is deleted (commit a76e30e48db). The reference guide's logging page documents the parameter (configuring-logging.adoc:105-116).

Proof

The new cloud test passes 4 of 4 here and fails 3 of 4 on base code, and the API and V1 handler tests pass.

Verified 2026-10-05 at head 85d8df8. The head 1f60cd3 adds one reference-guide sentence after that.

• NodeLoggingNodesSolrCloudTest (new, 2-node cluster) (NodeLoggingNodesSolrCloudTest.java): 4/4 with this change. Against base production code the same tests fail 3 of 4 for the stated reason: no broadcast happens, the response covers only the receiving node, and an unknown node name is silently treated as a local request. The fourth test, which pins the no-nodes local response shape, passes on base too. Assertions run against the response body, since the test cluster's nodes share one JVM and log levels are JVM-wide, so level state is not observable per node. The broadcast case also checks the listing afterwards and finds the new level in effect (JVM-wide is the only way it is observable in this cluster), then restores the logger to unset.
• NodeLoggingAPITest (NodeLoggingAPITest.java): 8/8, including the standalone nodes guard, the unchanged local response shape, and a broadcast case in which one node's proxied request fails: the response carries the answering node's result and names the failed node in failedNodes.
• LoggingHandlerTest (LoggingHandlerTest.java): 1/1 (the V1 handler passes a null nodes (LoggingHandler.java:77-78), so V1 does not broadcast twice).
• :solr:core:check and :solr:api:check -x test pass; Error Prone compile is clean. The generated SolrJ client exposes setNodes, and the proxy forwards the request with the nodes parameter removed (NodeLogging.java:123-125, V2SolrRequestBasedProxy.java:58-71), which the cloud test exercises end to end.
• The Selenium suites for this screen passed at 100ad2e (run 2026-10-05 on Windows with Chrome 153, -Ptests.selenium=true); the two commits since then add tests and one reference-guide sentence, and the UI code is unchanged: AdminUiLoggingScreenTest (AdminUiLoggingScreenTest.java) 3/3 and AdminUiLoggingStandaloneTest (AdminUiLoggingStandaloneTest.java) 1/1. Both include testChangeLogLevelViaUi, which clicks a level in the UI and waits for the server to report it, so the UI's request is exercised end to end in cloud and standalone mode.

Choices to check

Three calls are posed: one PR or a split, the response shape on partial failure, and how far the nodes parameter should extend.

1. One PR, or split? This PR lands the API change and the UI migration together, as the ticket's acceptance criteria list both. Our position: one PR, because the UI switch is small and is the point of the API change. We are happy to split it into an API PR and a UI PR that follows if reviewers prefer.
2. Broadcast response shape on partial failure. We return a per-node result mirroring the system info response, with nodes that did not respond named in failedNodes; the alternative is a single overall status. Our position: the per-node shape, because the caller needs to know which nodes still run the old levels. Happy to change the shape if another form is preferred.
3. Scope of nodes. Only the set-level PUT honors nodes here, as the ticket names. The other endpoints (the level and message GETs and the message threshold PUT (NodeLoggingApis.java:34-65)) could take it too. One caution from the ticket discussion: in V1 the GET is itself a level-changing route when it carries set parameters (LoggingHandler.java:75-79), which Eric Pugh flagged after stumbling over it in testing, and the ticket's verb-split question (changes on PUT or POST, retrieval on GET) is still open. We kept the ticket's scope, left every GET behaving exactly as it does today, and did not let this PR become the place that settles the GET question. Our plan for the read side: the levels GET, which is the read half of the screen this PR migrates and the one place V1 with nodes=all already aggregates, is open as #5031, a separate follow-up PR stacked on an earlier head of this PR; the messages GET and the threshold PUT stay out unless maintainers ask for them, and the follow-up PR names them in its Limits. If you would rather see the levels GET in this PR, say so and we will fold it in.

Were these the right calls?

Limits

A broadcast is authorized once, on the receiving node, so a rule that is not cluster-wide is effectively widened, and a partial failure is not surfaced in the Admin UI.

• Authorization for a broadcast is checked exactly once, on the node that receives the request: this endpoint requires config-edit permission there (NodeLogging.java:91-92). Node-to-node calls then go through the container's internal client with PKI authentication (RemoteRequestProxy.java:135-137), the same path the system info read uses. On that path the proxied requests present the sending node's own identity rather than the original caller's, and a node that receives one does not re-authorize it (PKIAuthenticationPlugin.java:371-372): the receiving node runs no config-edit check of its own for a change the broadcast delivers to it. A permission rule that is not cluster-wide is therefore effectively widened by the broadcast: a caller allowed config-edit on the receiving node alone can change levels on every node the request names. The broadcast has not been tested on a cluster with authentication and authorization enabled.
• When a broadcast only partly succeeds, the Admin UI logging screen does not surface the nodes that failed; it refreshes exactly as it did under V1 (logging.js:171-174). For the other nodes that matches how V1 behaved, but not for the node the browser is talking to: V1 applied the change on that node locally before it broadcast (LoggingHandler.java:75-79; the broadcast call is at LoggingHandler.java:95), so that node always changed. Under V2 it changes only through the broadcast's call to itself, so if the self-call fails, the node is named in failedNodes, the UI ignores the list, and the screen keeps showing the old level. API callers do get the failedNodes list.
• A node that is live when the request is validated but stops responding during the broadcast is reported in failedNodes after the fact (NodeLogging.java:145-151); the change may already have been applied on the nodes that did respond. There is no rollback, matching how the V1 broadcast behaved. A node that drops out of the live set mid-request is a harder version of the same case: the proxy re-checks membership as it sends (RemoteRequestProxy.java:121-131) and fails the request at that point, after earlier nodes may already have applied the change, and the caller gets that error rather than a failedNodes list.

Changelog: changelog/unreleased/SOLR-15823.yml

AI assistance

AI agents assisted with research, implementation, review, and drafting. Nick Shanin directed the work and takes responsibility for this contribution.

[epugh]

@nick-boss-tech can you look at my changes? I have some curl command sthat claude gave me, Ill do a bit of testing by hand in the morning. Then on to 5031....

Also, thiere is #5110 that also overlaps with this. I'm hoping that it won't matter the order of this or 5110 in getting merged..

[nick-boss-tech]

🤖 AI text below 🤖 (posted on behalf of Nick Shanin)

Thanks, Eric. I read both of your commits: the extraction and the move into RemoteRequestProxy look right, and the logging path keeps its behavior, including the same 400 for an unknown node and the same failedNodes content. On #5110 I agree the order does not matter; after your second commit the shared class also carries failedNodes, so it is no longer byte-identical to #5110's version, and whoever rebases second resolves that one file mechanically.

I pushed one commit on top, f8b0bbc: /node/system now copies the proxy's failed nodes into its response, so the reporting half of your second commit lands there too.

The Crave failure at your head was unrelated to either change: both failures were teardown failures from one event in CoreContainer.shutdown(), where a close-pool timeout skips the closes after it and leaks the container's metrics contexts into the next suite in the fork. I filed it as SOLR-18564 and opened the fix as #5192.