[
https://issues.apache.org/jira/browse/SOLR-15823?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18123559#comment-18123559
]
Nick Shanin commented on SOLR-15823:
------------------------------------
🤖 *AI text below* 🤖 *(posted on behalf of Nick Shanin)*
A note on how we are approaching this ticket. This is our first big PR
contribution to Solr, so we split the work into two pull requests to keep each
piece manageable to review.
The first PR, #5030 ([https://github.com/apache/solr/pull/5030]), adds the
`nodes` parameter to the V2 set-level endpoint, broadcasting through the same
proxy machinery the node system info endpoint already uses, migrates the Admin
UI logging screen to the V2 endpoint in both modes, and retires the V1
`Logging` factory. Its UI change is verified end to end: the Selenium suites
for the logging screen pass in cloud and standalone mode.
The second PR, #5031 ([https://github.com/apache/solr/pull/5031]), is stacked
on the first and adds the same `nodes` broadcast to the levels GET, the read
half of the same screen; it is meant to land after #5030. The messages GET and
the message threshold PUT are unchanged for now; we are glad to extend `nodes`
to them as follow-ups if that is wanted.
Eric, we saw your note about the V1 GET changing levels when it carries `set`
parameters. Neither PR changes that behavior. If the maintainers want that
looked at as its own follow-up, we are glad to take it on; it seems separate
from the `nodes` work here.
> Split v2 /node/logging API into separate GET and PUT APIs and deal with
> single node/all nodes logic
> ---------------------------------------------------------------------------------------------------
>
> Key: SOLR-15823
> URL: https://issues.apache.org/jira/browse/SOLR-15823
> Project: Solr
> Issue Type: Bug
> Components: v2 API
> Reporter: Jason Gerlowski
> Priority: Major
> Labels: V2, pull-request-available
> Time Spent: 20m
> Remaining Estimate: 0h
>
> Currently, the /v2/node/logging API (and it's v1 counterpart:
> /solr/admin/info/logging) use the same API to both set and get log-level
> information.
> In the v2 world at least, this could be split up to depend on the HTTP verb,
> using POST or PUT for changes to log-levels, and GET for retrieval of
> log-level information.
> Doing so would be more consistent with the HTTP-verb-aware design of the v2
> API. Practically though, it'd also help us get around a limitation of the
> current annotation framework, where each API can only be governed by a single
> {{PermissionNameProvider.Name}} value. Splitting the APIs into different
> verbs lets us govern them with the appropriate permissions in v2-land.
> We also need to deal with the single node/multiple nodes thing.
> Â
> NodeLoggingApis ('/api/node/logging/...', implemented by NodeLogging.java)
> only ever
> operates on the receiving node. There's no way to broadcast a log-level
> change to every
> live node via v2 today, so the Admin UI's "set log level" feature
> (LoggingLevelController
> in logging.js) still falls back to the legacy v1
> '/admin/info/logging?nodes=all' handler
> in SolrCloud mode (see the "Intentionally still v1" comment in logging.js and
> the TODO in
> NodeLogging.java).
> Â
> SOLR-16738 already added v2-API-compatible proxy support in general
> (V2SolrRequestBasedProxy), and GetNodeSystemInfo (NodeSystemInfoApi, also
> under '/api/node')
> already uses it to support a 'nodes' query param that fans a request out to a
> specified set
> of nodes (or 'all') and aggregates the responses. That's the exact pattern
> needed here;
> NodeLoggingApis just hasn't been wired up to it yet.
> Proposed change:
> - Add a 'nodes' query param to NodeLoggingApis.modifyLocalLogLevel (and
> NodeLoggingApis
> Â interface method signature), following the same shape as
> NodeSystemInfoApi.getNodeSystemInfo.
> - In NodeLogging.java, when 'nodes' is present, proxy the request via
> V2SolrRequestBasedProxy
> Â the same way GetNodeSystemInfo.proxyToNodes(...) does, instead of (or in
> addition to) applying
> Â the change locally.
> - Update the generated SolrJ/js-client (LoggingApi) accordingly - should be
> automatic from the
> Â OpenAPI annotation change.
> - Once this lands, LoggingLevelController.setLevel in logging.js can drop its
> SolrCloud-mode v1
> Â branch entirely and always call LoggingV2.modifyLocalLogLevel with
> nodes:'all' (or omitted in
> Â standalone mode, as already handled by SOLR-18317) - retiring the 'Logging'
> v1 $resource
> Â factory in services.js for good.
> Acceptance criteria:
> - PUT /api/node/logging/levels?nodes=all applies the given log-level
> change(s) to every live
> Â node and aggregates success/failure like GetNodeSystemInfo does.
> - PUT /api/node/logging/levels with no 'nodes' param continues to behave
> exactly as it does
> Â today (local node only) - no regression for existing callers.
> - The Admin UI's logging-levels screen is updated to use
> LoggingV2.modifyLocalLogLevel
> Â unconditionally, and the v1 'Logging' factory in services.js is removed.
> - Existing LoggingHandlerTest / AdminUiLoggingScreenTest /
> AdminUiLoggingStandaloneTest
> Â (SOLR-18317) continue to pass.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
