Make MockLogAppender itself Releasable

modules/transport-netty4/src/internalClusterTest/java/org/elasticsearch/transport/netty4/ESLoggingHandlerIT.java

@@ -10,7 +10,6 @@
 
 import org.apache.logging.log4j.Level;
 import org.elasticsearch.ESNetty4IntegTestCase;
-import org.elasticsearch.core.Releasable;
 import org.elasticsearch.core.TimeValue;
 import org.elasticsearch.test.ESIntegTestCase;
 import org.elasticsearch.test.MockLogAppender;
@@ -24,16 +23,14 @@
 public class ESLoggingHandlerIT extends ESNetty4IntegTestCase {
 
     private MockLogAppender appender;
-    private Releasable appenderRelease;
 
     public void setUp() throws Exception {
         super.setUp();
-        appender = new MockLogAppender();
-        appenderRelease = appender.capturing(ESLoggingHandler.class, TransportLogger.class, TcpTransport.class);
+        appender = MockLogAppender.capture(ESLoggingHandler.class, TransportLogger.class, TcpTransport.class);
     }
 
     public void tearDown() throws Exception {
-        appenderRelease.close();
+        appender.close();
         super.tearDown();
     }
 

qa/no-bootstrap-tests/src/test/java/org/elasticsearch/bootstrap/SpawnerNoBootstrapTests.java

@@ -206,16 +206,16 @@ private void assertControllerSpawns(final Function<Environment, Path> pluginsDir
 
         String stdoutLoggerName = "test_plugin-controller-stdout";
         String stderrLoggerName = "test_plugin-controller-stderr";
-        MockLogAppender appender = new MockLogAppender();
         Loggers.setLevel(LogManager.getLogger(stdoutLoggerName), Level.TRACE);
         Loggers.setLevel(LogManager.getLogger(stderrLoggerName), Level.TRACE);
         CountDownLatch messagesLoggedLatch = new CountDownLatch(2);
-        if (expectSpawn) {
-            appender.addExpectation(new ExpectedStreamMessage(stdoutLoggerName, "I am alive", messagesLoggedLatch));
-            appender.addExpectation(new ExpectedStreamMessage(stderrLoggerName, "I am an error", messagesLoggedLatch));
-        }
 
-        try (var ignore = appender.capturing(stdoutLoggerName, stderrLoggerName)) {
+        try (var appender = MockLogAppender.capture(stdoutLoggerName, stderrLoggerName)) {
+            if (expectSpawn) {
+                appender.addExpectation(new ExpectedStreamMessage(stdoutLoggerName, "I am alive", messagesLoggedLatch));
+                appender.addExpectation(new ExpectedStreamMessage(stderrLoggerName, "I am an error", messagesLoggedLatch));
+            }
+
             Spawner spawner = new Spawner();
             spawner.spawnNativeControllers(environment);
 

server/src/internalClusterTest/java/org/elasticsearch/cluster/allocation/ClusterRerouteIT.java

@@ -387,17 +387,16 @@ public void testMessageLogging() {
             )
             .get();
 
-        MockLogAppender dryRunMockLog = new MockLogAppender();
-        dryRunMockLog.addExpectation(
-            new MockLogAppender.UnseenEventExpectation(
-                "no completed message logged on dry run",
-                TransportClusterRerouteAction.class.getName(),
-                Level.INFO,
-                "allocated an empty primary*"
-            )
-        );
+        try (var dryRunMockLog = MockLogAppender.capture(TransportClusterRerouteAction.class)) {
+            dryRunMockLog.addExpectation(
+                new MockLogAppender.UnseenEventExpectation(
+                    "no completed message logged on dry run",
+                    TransportClusterRerouteAction.class.getName(),
+                    Level.INFO,
+                    "allocated an empty primary*"
+                )
+            );
 
-        try (var ignored = dryRunMockLog.capturing(TransportClusterRerouteAction.class)) {
             AllocationCommand dryRunAllocation = new AllocateEmptyPrimaryAllocationCommand(indexName, 0, nodeName1, true);
             ClusterRerouteResponse dryRunResponse = clusterAdmin().prepareReroute()
                 .setExplain(randomBoolean())
@@ -412,24 +411,23 @@ public void testMessageLogging() {
             dryRunMockLog.assertAllExpectationsMatched();
         }
 
-        MockLogAppender allocateMockLog = new MockLogAppender();
-        allocateMockLog.addExpectation(
-            new MockLogAppender.SeenEventExpectation(
-                "message for first allocate empty primary",
-                TransportClusterRerouteAction.class.getName(),
-                Level.INFO,
-                "allocated an empty primary*" + nodeName1 + "*"
-            )
-        );
-        allocateMockLog.addExpectation(
-            new MockLogAppender.UnseenEventExpectation(
-                "no message for second allocate empty primary",
-                TransportClusterRerouteAction.class.getName(),
-                Level.INFO,
-                "allocated an empty primary*" + nodeName2 + "*"
-            )
-        );
-        try (var ignored = allocateMockLog.capturing(TransportClusterRerouteAction.class)) {
+        try (var allocateMockLog = MockLogAppender.capture(TransportClusterRerouteAction.class)) {
+            allocateMockLog.addExpectation(
+                new MockLogAppender.SeenEventExpectation(
+                    "message for first allocate empty primary",
+                    TransportClusterRerouteAction.class.getName(),
+                    Level.INFO,
+                    "allocated an empty primary*" + nodeName1 + "*"
+                )
+            );
+            allocateMockLog.addExpectation(
+                new MockLogAppender.UnseenEventExpectation(
+                    "no message for second allocate empty primary",
+                    TransportClusterRerouteAction.class.getName(),
+                    Level.INFO,
+                    "allocated an empty primary*" + nodeName2 + "*"
+                )
+            );
 
             AllocationCommand yesDecisionAllocation = new AllocateEmptyPrimaryAllocationCommand(indexName, 0, nodeName1, true);
             AllocationCommand noDecisionAllocation = new AllocateEmptyPrimaryAllocationCommand("noexist", 1, nodeName2, true);

test/framework/src/main/java/org/elasticsearch/test/MockLogAppender.java

@@ -31,13 +31,35 @@
 /**
  * Test appender that can be used to verify that certain events were logged correctly
  */
-public class MockLogAppender {
+public class MockLogAppender implements Releasable {
 
     private static final Map<String, List<MockLogAppender>> mockAppenders = new ConcurrentHashMap<>();
     private static final RealMockAppender parent = new RealMockAppender();
+    // TODO: this can become final once the ctor is made private
+    private List<String> loggers = List.of();
     private final List<WrappedLoggingExpectation> expectations;
     private volatile boolean isAlive = true;
 
+    @Override
+    public void close() {
+        isAlive = false;
+        for (String logger : loggers) {
+            mockAppenders.compute(logger, (k, v) -> {
+                assert v != null;
+                v.remove(this);
+                return v.isEmpty() ? null : v;
+            });
+        }
+        // check that all expectations have been evaluated before this is released
+        for (WrappedLoggingExpectation expectation : expectations) {
+            assertThat(
+                "Method assertMatched() not called on LoggingExpectation instance before release: " + expectation,
+                expectation.assertMatchedCalled,
+                is(true)
+            );
+        }
+    }
+
     private static class RealMockAppender extends AbstractAppender {
 
         RealMockAppender() {
@@ -71,6 +93,11 @@ public MockLogAppender() {
         expectations = new CopyOnWriteArrayList<>();
     }
 
+    private MockLogAppender(List<String> loggers) {
+        this();
+        this.loggers = loggers;
+    }
+
     /**
      * Initialize the mock log appender with the log4j system.
      */
@@ -267,58 +294,57 @@ public String toString() {
         }
     }
 
+    public Releasable capturing(Class<?>... classes) {
+        this.loggers = Arrays.stream(classes).map(Class::getCanonicalName).toList();
+        addToMockAppenders(this, loggers);
+        return this;
+    }
+
+    public Releasable capturing(String... names) {
+        this.loggers = Arrays.asList(names);
+        addToMockAppenders(this, loggers);
+        return this;
+    }
+
     /**
      * Adds the list of class loggers to this {@link MockLogAppender}.
      *
      * Stops and runs some checks on the {@link MockLogAppender} once the returned object is released.
      */
-    public Releasable capturing(Class<?>... classes) {
-        return appendToLoggers(Arrays.stream(classes).map(Class::getCanonicalName).toList());
+    public static MockLogAppender capture(Class<?>... classes) {
+        return create(Arrays.stream(classes).map(Class::getCanonicalName).toList());
     }
 
     /**
      * Same as above except takes string class names of each logger.
      */
-    public Releasable capturing(String... names) {
-        return appendToLoggers(Arrays.asList(names));
+    public static MockLogAppender capture(String... names) {
+        return create(Arrays.asList(names));
+    }
+
+    private static MockLogAppender create(List<String> loggers) {
+        MockLogAppender appender = new MockLogAppender(loggers);
+        addToMockAppenders(appender, loggers);
+        return appender;
     }
 
-    private Releasable appendToLoggers(List<String> loggers) {
+    private static void addToMockAppenders(MockLogAppender appender, List<String> loggers) {
         for (String logger : loggers) {
             mockAppenders.compute(logger, (k, v) -> {
                 if (v == null) {
                     v = new CopyOnWriteArrayList<>();
                 }
-                v.add(this);
+                v.add(appender);
                 return v;
             });
         }
-        return () -> {
-            isAlive = false;
-            for (String logger : loggers) {
-                mockAppenders.compute(logger, (k, v) -> {
-                    assert v != null;
-                    v.remove(this);
-                    return v.isEmpty() ? null : v;
-                });
-            }
-            // check that all expectations have been evaluated before this is released
-            for (WrappedLoggingExpectation expectation : expectations) {
-                assertThat(
-                    "Method assertMatched() not called on LoggingExpectation instance before release: " + expectation,
-                    expectation.assertMatchedCalled,
-                    is(true)
-                );
-            }
-        };
     }
 
     /**
      * Executes an action and verifies expectations against the provided logger
      */
     public static void assertThatLogger(Runnable action, Class<?> loggerOwner, MockLogAppender.LoggingExpectation expectation) {
-        MockLogAppender mockAppender = new MockLogAppender();
-        try (var ignored = mockAppender.capturing(loggerOwner)) {
+        try (var mockAppender = MockLogAppender.capture(loggerOwner)) {
             mockAppender.addExpectation(expectation);
             action.run();
             mockAppender.assertAllExpectationsMatched();

test/framework/src/main/java/org/elasticsearch/transport/AbstractSimpleTransportTestCase.java

@@ -1319,8 +1319,7 @@ public void handleException(TransportException exp) {}
                 .build()
         );
 
-        MockLogAppender appender = new MockLogAppender();
-        try (var ignored = appender.capturing("org.elasticsearch.transport.TransportService.tracer")) {
+        try (var appender = MockLogAppender.capture("org.elasticsearch.transport.TransportService.tracer")) {
 
             ////////////////////////////////////////////////////////////////////////
             // tests for included action type "internal:test"