SOLR-15823: Broadcast V2 logging level changes to named nodes and migrate the Admin UI to V2

changelog/unreleased/SOLR-15823.yml

@@ -0,0 +1,7 @@
+title: V2 logging API can broadcast log level changes to multiple nodes, and the Admin UI logging screen now uses it.
+type: added
+authors:
+  - name: Nick Shanin
+links:
+  - name: SOLR-15823
+    url: https://issues.apache.org/jira/browse/SOLR-15823

solr/api/src/java/org/apache/solr/client/api/endpoint/NodeLoggingApis.java

@@ -42,8 +42,13 @@ public interface NodeLoggingApis {
   @Path("/levels")
   @Operation(
       summary = "Set one or more logger levels on the target node.",
+      description =
+          "If the 'nodes' parameter is provided, the level changes are instead applied to "
+              + "each of the named nodes (or to every live node, if 'nodes' is 'all'), and the "
+              + "response reports the per-node results.",
       tags = {"logging"})
-  LoggingResponse modifyLocalLogLevel(List<LogLevelChange> requestBody);
+  LoggingResponse modifyLocalLogLevel(
+      @QueryParam("nodes") String nodes, List<LogLevelChange> requestBody);
 
   @GET
   @Path("/messages")

solr/api/src/java/org/apache/solr/client/api/model/LoggingResponse.java

@@ -16,10 +16,42 @@
  */
 package org.apache.solr.client.api.model;
 
+import com.fasterxml.jackson.annotation.JsonAnyGetter;
+import com.fasterxml.jackson.annotation.JsonAnySetter;
 import com.fasterxml.jackson.annotation.JsonProperty;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
 
 /** Generic logging response that includes the name of the log watcher (e.g. "Log4j2") */
 public class LoggingResponse extends SolrJerseyResponse {
   @JsonProperty("watcher")
   public String watcherName;
+
+  /**
+   * Per-node results of a request broadcast with the 'nodes' parameter, keyed by node name in "live
+   * node" format (e.g. "someHost:8983_solr"). Serialized inline, as top-level fields named by node,
+   * mirroring {@link NodeSystemResponse#remoteNodeData}. Empty for requests that were not
+   * broadcast.
+   */
+  // Object, not LoggingResponse, since @JsonAnySetter below also feeds this map raw values.
+  public Map<String, Object> remoteNodeData = new LinkedHashMap<>();
+
+  @JsonAnyGetter
+  public Map<String, Object> remoteNodeData() {
+    return remoteNodeData;
+  }
+
+  @JsonAnySetter
+  public void setRemoteNodeResponse(String field, Object value) {
+    remoteNodeData.put(field, value);
+  }
+
+  /**
+   * Nodes that were asked to apply a broadcast request (via the 'nodes' parameter) but did not
+   * return a response, e.g. because they timed out or errored. Null for requests that were not
+   * broadcast, and empty when every requested node responded.
+   */
+  @JsonProperty("failedNodes")
+  public List<String> failedNodes;
 }

solr/core/src/java/org/apache/solr/handler/admin/LoggingHandler.java

@@ -75,7 +75,7 @@ public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp) throw
     String[] set = params.getParams("set");
     if (set != null) {
       final List<LogLevelChange> changes = NodeLogging.parseLogLevelChanges(set);
-      squashV2Response(rsp, loggingApi.modifyLocalLogLevel(changes));
+      squashV2Response(rsp, loggingApi.modifyLocalLogLevel(null, changes));
     }
 
     String since = req.getParams().get("since");

solr/core/src/java/org/apache/solr/handler/admin/api/NodeLogging.java

@@ -24,7 +24,10 @@
 import jakarta.inject.Inject;
 import java.lang.invoke.MethodHandles;
 import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
 import java.util.List;
+import java.util.Set;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.stream.Collectors;
 import org.apache.solr.api.JerseyResource;
@@ -36,17 +39,19 @@
 import org.apache.solr.client.api.model.LogMessagesResponse;
 import org.apache.solr.client.api.model.LoggingResponse;
 import org.apache.solr.client.api.model.SetThresholdRequestBody;
+import org.apache.solr.client.solrj.request.LoggingApi;
 import org.apache.solr.common.SolrDocumentList;
 import org.apache.solr.common.SolrException;
 import org.apache.solr.core.CoreContainer;
+import org.apache.solr.handler.admin.proxy.V2SolrRequestBasedProxy;
 import org.apache.solr.jersey.PermissionName;
 import org.apache.solr.logging.LogWatcher;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-// TODO Add support for 'nodes' param once SOLR-16738 is completed.
 /**
- * V2 APIs for getting or setting log levels on an individual node.
+ * V2 APIs for getting or setting log levels on an individual node, or for broadcasting log level
+ * changes across nodes.
  *
  * <p>These APIs ('/api/node/logging' and descendants) are analogous to the v1 /admin/info/logging.
  */
@@ -84,20 +89,74 @@ public ListLevelsResponse listAllLoggersAndLevels() {
 
   @Override
   @PermissionName(CONFIG_EDIT_PERM)
-  public LoggingResponse modifyLocalLogLevel(List<LogLevelChange> requestBody) {
+  public LoggingResponse modifyLocalLogLevel(String nodes, List<LogLevelChange> requestBody) {
     ensureLogWatcherEnabled();
     final LoggingResponse response = instantiateLoggingResponse(LoggingResponse.class);
 
     if (requestBody == null) {
       throw new SolrException(BAD_REQUEST, "Missing request body");
     }
 
+    if (nodes != null && !nodes.isEmpty()) {
+      proxyLevelChangesToNodes(response, nodes, requestBody);
+      return response;
+    }
+
     for (LogLevelChange change : requestBody) {
       watcher.setLogLevel(change.logger, change.level);
     }
     return response;
   }
 
+  /**
+   * Applies the given log level changes on other nodes, mirroring how {@link GetNodeSystemInfo}
+   * fans its request out. The receiving node does not apply the changes locally; it is covered only
+   * if the resolved node set includes it, in which case it calls itself over HTTP.
+   */
+  private void proxyLevelChangesToNodes(
+      LoggingResponse response, String nodes, List<LogLevelChange> requestBody) {
+    if (coreContainer == null || coreContainer.getZkController() == null) {
+      throw new SolrException(
+          BAD_REQUEST, "The 'nodes' parameter is only supported in SolrCloud mode");
+    }
+    try {
+      final var req = new LoggingApi.ModifyLocalLogLevel();
+      requestBody.forEach(req::addLogLevelChange);
+      req.setNodes(nodes);
+      final var reqProxy =
+          new V2SolrRequestBasedProxy<LoggingResponse>(coreContainer, req) {
+            @Override
+            public void processTypedProxiedResponse(
+                String nodeName, LoggingResponse proxiedResponse) {
+              response.remoteNodeData.put(nodeName, proxiedResponse);
+            }
+          };
+      final Collection<String> destinationNodes = reqProxy.getDestinationNodes();
+      // Fail before sending anything if a named node is not part of the cluster; otherwise the
+      // broadcast would fail partway through, after earlier nodes already applied the change.
+      final Set<String> liveNodes =
+          coreContainer.getZkController().zkStateReader.getClusterState().getLiveNodes();
+      final List<String> unknownNodes =
+          destinationNodes.stream().filter(node -> !liveNodes.contains(node)).sorted().toList();
+      if (!unknownNodes.isEmpty()) {
+        throw new SolrException(
+            BAD_REQUEST, "Requested nodes are not part of the cluster: " + unknownNodes);
+      }
+      reqProxy.proxyRequest();
+      // The proxy logs and skips nodes that error or time out; surface those nodes here so a
+      // partial broadcast is visible in the response instead of silent.
+      final var failedNodes = new ArrayList<>(destinationNodes);
+      failedNodes.removeAll(response.remoteNodeData.keySet());
+      Collections.sort(failedNodes);
+      response.failedNodes = failedNodes;
+    } catch (SolrException e) {
+      throw e;
+    } catch (Exception e) {
+      throw new SolrException(
+          SolrException.ErrorCode.SERVER_ERROR, "Error occurred while proxying to other nodes", e);
+    }
+  }
+
   @Override
   @PermissionName(CONFIG_READ_PERM)
   public LogMessagesResponse fetchLocalLogMessages(Long boundingTimeMillis) {

solr/core/src/test/org/apache/solr/handler/admin/api/NodeLoggingAPITest.java

@@ -21,17 +21,31 @@
 import static org.hamcrest.Matchers.containsInAnyOrder;
 import static org.hamcrest.Matchers.instanceOf;
 import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
+import java.lang.reflect.Field;
 import java.util.List;
+import java.util.Set;
+import java.util.concurrent.CompletableFuture;
 import org.apache.solr.SolrTestCase;
 import org.apache.solr.client.api.model.LogLevelChange;
+import org.apache.solr.client.api.model.LoggingResponse;
 import org.apache.solr.client.api.model.SetThresholdRequestBody;
+import org.apache.solr.client.solrj.jetty.HttpJettySolrClient;
+import org.apache.solr.cloud.ZkController;
 import org.apache.solr.common.SolrDocument;
 import org.apache.solr.common.SolrDocumentList;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.cloud.ClusterState;
+import org.apache.solr.common.cloud.ZkStateReader;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.common.util.SuppressForbidden;
 import org.apache.solr.core.CoreContainer;
 import org.apache.solr.logging.LogWatcher;
 import org.apache.solr.logging.LoggerInfo;
@@ -84,13 +98,104 @@ public void testReliesOnLogWatcherToListLogLevels() {
   public void testReliesOnLogWatcherToModifyLogLevels() {
     final var responseBody =
         new NodeLogging(mockCoreContainer)
-            .modifyLocalLogLevel(List.of(new LogLevelChange("o.a.s.Foo", "WARN")));
+            .modifyLocalLogLevel(null, List.of(new LogLevelChange("o.a.s.Foo", "WARN")));
 
     assertNotNull(responseBody);
     assertNull("Expected error to be null but was " + responseBody.error, responseBody.error);
     verify(mockLogWatcher).setLogLevel("o.a.s.Foo", "WARN");
   }
 
+  @Test
+  public void testModifyLogLevelsWithoutNodesReportsNoBroadcastResults() {
+    final var responseBody =
+        new NodeLogging(mockCoreContainer)
+            .modifyLocalLogLevel(null, List.of(new LogLevelChange("o.a.s.Foo", "WARN")));
+
+    assertTrue(
+        "Expected no per-node results for a local request, but was " + responseBody.remoteNodeData,
+        responseBody.remoteNodeData.isEmpty());
+    assertNull(
+        "Expected failedNodes to be unset for a local request, but was " + responseBody.failedNodes,
+        responseBody.failedNodes);
+  }
+
+  @Test
+  public void testModifyLogLevelsWithNodesFailsOutsideSolrCloud() {
+    // The mock CoreContainer has no ZkController stubbed, as on a standalone (user-managed)
+    // node, so NodeLogging sees a null ZkController here.
+    try {
+      new NodeLogging(mockCoreContainer)
+          .modifyLocalLogLevel("all", List.of(new LogLevelChange("o.a.s.Foo", "WARN")));
+      fail("Expected a SolrException when 'nodes' is used outside SolrCloud mode");
+    } catch (SolrException e) {
+      assertEquals(SolrException.ErrorCode.BAD_REQUEST.code, e.code());
+      assertTrue(
+          "Expected the error to name the 'nodes' parameter, but was: " + e.getMessage(),
+          e.getMessage().contains("'nodes'"));
+    }
+    verify(mockLogWatcher, never()).setLogLevel(any(), any());
+  }
+
+  @Test
+  @SuppressForbidden(
+      reason =
+          "places a stubbed ZkStateReader on a ZkController mock; production reads the public"
+              + " final field directly, so there is no setter or getter to stub")
+  public void testModifyLogLevelsWithNodesReportsNodesThatDidNotRespond() throws Exception {
+    final String respondingNode = "host1:8983_solr";
+    final String failingNode = "host2:8983_solr";
+
+    final ZkController mockZkController = mock(ZkController.class);
+    final ZkStateReader mockZkStateReader = mock(ZkStateReader.class);
+    final ClusterState mockClusterState = mock(ClusterState.class);
+    // NodeLogging and the request proxy read ZkController.zkStateReader directly (a public
+    // final field) rather than calling a getter, so the stubbed reader has to be placed on
+    // the mock reflectively.
+    final Field zkStateReaderField = ZkController.class.getField("zkStateReader");
+    zkStateReaderField.setAccessible(true);
+    zkStateReaderField.set(mockZkController, mockZkStateReader);
+
+    when(mockCoreContainer.getZkController()).thenReturn(mockZkController);
+    when(mockZkController.getCoreContainer()).thenReturn(mockCoreContainer);
+    when(mockZkStateReader.getClusterState()).thenReturn(mockClusterState);
+    when(mockClusterState.getLiveNodes()).thenReturn(Set.of(respondingNode, failingNode));
+    when(mockZkStateReader.getBaseUrlForNodeName(respondingNode))
+        .thenReturn("http://host1:8983/solr");
+    when(mockZkStateReader.getBaseUrlForNodeName(failingNode)).thenReturn("http://host2:8983/solr");
+
+    // One node answers with a logging response; the other's request fails, the way the proxy
+    // sees a node that errors or times out.
+    final HttpJettySolrClient mockClient = mock(HttpJettySolrClient.class);
+    when(mockCoreContainer.getDefaultHttpSolrClient()).thenReturn(mockClient);
+    final NamedList<Object> proxiedResponse = new NamedList<>();
+    proxiedResponse.add("response", new LoggingResponse());
+    doAnswer(
+            invocation -> {
+              if (invocation.getArgument(0).equals("http://host1:8983/solr")) {
+                return CompletableFuture.completedFuture(proxiedResponse);
+              }
+              return CompletableFuture.failedFuture(new RuntimeException("simulated node failure"));
+            })
+        .when(mockClient)
+        .requestWithBaseUrl(anyString(), any());
+
+    final var responseBody =
+        new NodeLogging(mockCoreContainer)
+            .modifyLocalLogLevel(
+                respondingNode + "," + failingNode,
+                List.of(new LogLevelChange("o.a.s.Foo", "WARN")));
+
+    assertEquals(
+        "Expected only the responding node to report a result",
+        Set.of(respondingNode),
+        responseBody.remoteNodeData.keySet());
+    assertEquals(
+        "Expected the node whose request failed to be reported",
+        List.of(failingNode),
+        responseBody.failedNodes);
+    verify(mockLogWatcher, never()).setLogLevel(any(), any());
+  }
+
   private SolrDocumentList logMessageDocList(String... logMessages) {
     final var docList = new SolrDocumentList();
     for (String logMessage : logMessages) {