nick-boss-tech opened a new pull request, #5030: URL: https://github.com/apache/solr/pull/5030
🤖 *AI text below* 🤖 *(posted on behalf of Nick Shanin)* https://issues.apache.org/jira/browse/SOLR-15823 ## What happens today 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](https://github.com/apache/solr/blob/e432df19c4a50b9d54d7fba545397b859cc54f98/solr/core/src/java/org/apache/solr/handler/admin/api/NodeLogging.java#L87-L99)); `NodeLogging` carries a TODO to add a `nodes` parameter once SOLR-16738 lands ([NodeLogging.java:47](https://github.com/apache/solr/blob/e432df19c4a50b9d54d7fba545397b859cc54f98/solr/core/src/java/org/apache/solr/handler/admin/api/NodeLogging.java#L47)), and that proxy machinery has since landed (the node system info endpoint uses it ([GetNodeSystemInfo.java:70-83](https://github.com/apache/solr/blob/100ad2e0df69ff800c269d1db15d26b6d027d1a8/solr/core/src/java/org/apache/solr/handler/admin/api/GetNodeSystemInfo.java#L70-L83))). Because of the gap, the Admin UI logging screen still sets levels through the V1 handler in SolrCloud mode ([logging.js:164-175](https://github.com/apach e/solr/blob/e432df19c4a50b9d54d7fba545397b859cc54f98/solr/webapp/web/js/angular/controllers/logging.js#L164-L175)), 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](https://github.com/apache/solr/blob/100ad2e0df69ff800c269d1db15d26b6d027d1a8/solr/core/src/java/org/apache/solr/handler/admin/LoggingHandler.java#L75-L79)), 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` query parameter ([NodeLoggingApis.java:41-51](https://github.com/apache/solr/blob/100ad2e0df69ff800c269d1db15d26b6d027d1a8/solr/api/src/java/org/apache/solr/client/api/endpoint/NodeLoggingApis.java#L41-L51)). 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](https://github.com/apache/solr/blob/100ad2e0df69ff800c269d1db15d26b6d027d1a8/solr/core/src/java/org/apache/solr/handler/admin/api/NodeLogging.java#L123-L133)), mirroring `GetNodeSystemInfo` ([GetNodeSystemInfo.java:70-83](https://github.com/apache/solr/blob/100ad2e0df69ff800c269d1db15d26b6d027d1a8/solr/core/src/java/org/apache/solr/handler/admin/api/GetNodeSystemInfo.java#L70-L83)), 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:34-56](https://github.c om/apache/solr/blob/100ad2e0df69ff800c269d1db15d26b6d027d1a8/solr/api/src/java/org/apache/solr/client/api/model/LoggingResponse.java#L34-L56)). A named node that is not part of the cluster is rejected with a 400 before anything is sent ([NodeLogging.java:134-145](https://github.com/apache/solr/blob/100ad2e0df69ff800c269d1db15d26b6d027d1a8/solr/core/src/java/org/apache/solr/handler/admin/api/NodeLogging.java#L134-L145)), so a broadcast cannot half-apply and then fail. Using `nodes` on a standalone node is also a 400, not an NPE ([NodeLogging.java:118-121](https://github.com/apache/solr/blob/100ad2e0df69ff800c269d1db15d26b6d027d1a8/solr/core/src/java/org/apache/solr/handler/admin/api/NodeLogging.java#L118-L121)). When `nodes` is absent the local-only path is unchanged ([NodeLogging.java:100-108](https://github.com/apache/solr/blob/100ad2e0df69ff800c269d1db15d26b6d027d1a8/solr/core/src/java/org/apache/solr/handler/admin/api/NodeLogging.java#L100-L108)), 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](https://github.com/apache/solr/blob/100ad2e0df69ff800c269d1db15d26b6d027d1a8/solr/core/src/java/org/apache/solr/handler/admin/api/NodeLogging.java#L111-L114)). The Admin UI logging screen now calls the V2 endpoint in both modes (`nodes: "all"` only in cloud mode) ([logging.js:163-174](https://github.com/apache/solr/blob/100ad2e0df69ff800c269d1db15d26b6d027d1a8/solr/webapp/web/js/angular/controllers/logging.js#L163-L174)), and the V1 `Logging` factory is deleted ([commit a76e30e48db](https://github.com/apache/solr/commit/a76e30e48db11b95592ce0d2cded98553097232d)). The reference guide's logging page documents the parameter ([configuring-logging.adoc:105-116](https://github.com/apache/solr/blob/100ad2e0df69ff800c269d1db15d26b6d027d1a8/solr/solr-ref-guide/modules/deployment-guide/pages/configuring-logging.adoc#L105-L1 16)). ## Proof Verified 2026-10-05 at head 100ad2e0df69f (base e432df19c4a). * `NodeLoggingNodesSolrCloudTest` (new, 2-node cluster) ([NodeLoggingNodesSolrCloudTest.java](https://github.com/apache/solr/blob/100ad2e0df69ff800c269d1db15d26b6d027d1a8/solr/core/src/test/org/apache/solr/handler/admin/api/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. * `NodeLoggingAPITest` ([NodeLoggingAPITest.java](https://github.com/apache/solr/blob/100ad2e0df69ff800c269d1db15d26b6d027d1a8/solr/core/src/test/org/apache/solr/handler/admin/api/NodeLoggingAPITest.java)): 7/7, including the standalone `nodes` guard and the unchanged local response shape. * `LoggingHandlerTest` ([LoggingHandlerTest.java](https://github.com/apache/solr/blob/100ad2e0df69ff800c269d1db15d26b6d027d1a8/solr/core/src/test/org/apache/solr/handler/admin/LoggingHandlerTest.java)): 1/1 (the V1 handler passes a null `nodes` ([LoggingHandler.java:77-78](https://github.com/apache/solr/blob/100ad2e0df69ff800c269d1db15d26b6d027d1a8/solr/core/src/java/org/apache/solr/handler/admin/LoggingHandler.java#L77-L78)), 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 forwards the request body through the proxy ([NodeLogging.java:123-125](https://github.com/apache/solr/blob/100ad2e0df69ff800c269d1db15d26b6d027d1a8/solr/core/src/java/org/apache/solr/handler/admin/api/NodeLogging.java#L123-L125), [V2SolrRequestBasedProxy.java:58-71](https://github.com/apache/solr/blob/100ad2e0df69ff800c269d1db15d26b6d027d1a8/solr/core/src/java/org/apache/solr/handler/admin/proxy/V2SolrRequestBasedProxy.java#L58-L71)), which the cloud test exercises end to end. * The Selenium suites for this screen pass at this head (run 2026-10-05 on Windows with Chrome 153, `-Ptests.selenium=true`): `AdminUiLoggingScreenTest` ([AdminUiLoggingScreenTest.java](https://github.com/apache/solr/blob/100ad2e0df69ff800c269d1db15d26b6d027d1a8/solr/webapp/src/test/org/apache/solr/webapp/AdminUiLoggingScreenTest.java)) 3/3 and `AdminUiLoggingStandaloneTest` ([AdminUiLoggingStandaloneTest.java](https://github.com/apache/solr/blob/100ad2e0df69ff800c269d1db15d26b6d027d1a8/solr/webapp/src/test/org/apache/solr/webapp/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 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:53-65](https://github.com/apache/solr/blob/100ad2e0df69ff800c269d1db15d26b6d027d1a8/solr/api/src/java/org/apache/solr/client/api/endpoint/NodeLoggingApis.java#L53-L65))) 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](https://github.com/apache/solr/blob/100ad2e0df69ff800c269d1db15d26b6d027d1a8/solr/core/src/java/org/apache/solr/handler/admin/LoggingHandler.java#L75-L79)), 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 being prepared as a separate follow-up PR stacked on this one; 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 * Node-to-node calls go through the container's internal client with PKI authentication ([RemoteRequestProxy.java:135-137](https://github.com/apache/solr/blob/100ad2e0df69ff800c269d1db15d26b6d027d1a8/solr/core/src/java/org/apache/solr/handler/admin/proxy/RemoteRequestProxy.java#L135-L137)), 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](https://github.com/apache/solr/blob/100ad2e0df69ff800c269d1db15d26b6d027d1a8/solr/core/src/java/org/apache/solr/security/PKIAuthenticationPlugin.java#L371-L372)). We have not tested the broadcast 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 as it did under V1 ([logging.js:171-174](https://github.com/apache/solr/blob/100ad2e0df69ff800c269d1db15d26b6d027d1a8/solr/webapp/web/js/angular/controllers/logging.js#L171-L174)), which behaved the same way. 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](https://github.com/apache/solr/blob/100ad2e0df69ff800c269d1db15d26b6d027d1a8/solr/core/src/java/org/apache/solr/handler/admin/api/NodeLogging.java#L145-L151)); the change may already have been applied on the nodes that did respond. There is no rollback, matching how the V1 broadcast behaved. 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. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
