Skip to content

Commit 04f88e7

Browse files
DaveCTurnerDaveCTurner
authored
Clean up some MockLogAppender usages (#92748)
No need for the explict start/add...remove/stop any more.
1 parent 2bb710b commit 04f88e7

File tree

11 files changed

+88
-283
lines changed

11 files changed

+88
-283
lines changed

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

Lines changed: 37 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@
3535
import org.elasticsearch.cluster.routing.allocation.decider.ThrottlingAllocationDecider;
3636
import org.elasticsearch.common.Priority;
3737
import org.elasticsearch.common.io.FileSystemUtils;
38-
import org.elasticsearch.common.logging.Loggers;
3938
import org.elasticsearch.common.settings.Settings;
4039
import org.elasticsearch.core.IOUtils;
4140
import org.elasticsearch.core.TimeValue;
@@ -402,7 +401,7 @@ public void testRerouteExplain() {
402401
assertThat(explanation.decisions().type(), equalTo(Decision.Type.YES));
403402
}
404403

405-
public void testMessageLogging() throws Exception {
404+
public void testMessageLogging() {
406405
final Settings settings = Settings.builder()
407406
.put(EnableAllocationDecider.CLUSTER_ROUTING_ALLOCATION_ENABLE_SETTING.getKey(), Allocation.NONE.name())
408407
.put(EnableAllocationDecider.CLUSTER_ROUTING_REBALANCE_ENABLE_SETTING.getKey(), EnableAllocationDecider.Rebalance.NONE.name())
@@ -431,10 +430,7 @@ public void testMessageLogging() throws Exception {
431430
.execute()
432431
.actionGet();
433432

434-
Logger actionLogger = LogManager.getLogger(TransportClusterRerouteAction.class);
435-
436433
MockLogAppender dryRunMockLog = new MockLogAppender();
437-
dryRunMockLog.start();
438434
dryRunMockLog.addExpectation(
439435
new MockLogAppender.UnseenEventExpectation(
440436
"no completed message logged on dry run",
@@ -443,28 +439,26 @@ public void testMessageLogging() throws Exception {
443439
"allocated an empty primary*"
444440
)
445441
);
446-
Loggers.addAppender(actionLogger, dryRunMockLog);
447-
448-
AllocationCommand dryRunAllocation = new AllocateEmptyPrimaryAllocationCommand(indexName, 0, nodeName1, true);
449-
ClusterRerouteResponse dryRunResponse = client().admin()
450-
.cluster()
451-
.prepareReroute()
452-
.setExplain(randomBoolean())
453-
.setDryRun(true)
454-
.add(dryRunAllocation)
455-
.execute()
456-
.actionGet();
457-
458-
// during a dry run, messages exist but are not logged or exposed
459-
assertThat(dryRunResponse.getExplanations().getYesDecisionMessages(), hasSize(1));
460-
assertThat(dryRunResponse.getExplanations().getYesDecisionMessages().get(0), containsString("allocated an empty primary"));
461442

462-
dryRunMockLog.assertAllExpectationsMatched();
463-
dryRunMockLog.stop();
464-
Loggers.removeAppender(actionLogger, dryRunMockLog);
443+
try (var ignored = dryRunMockLog.capturing(TransportClusterRerouteAction.class)) {
444+
AllocationCommand dryRunAllocation = new AllocateEmptyPrimaryAllocationCommand(indexName, 0, nodeName1, true);
445+
ClusterRerouteResponse dryRunResponse = client().admin()
446+
.cluster()
447+
.prepareReroute()
448+
.setExplain(randomBoolean())
449+
.setDryRun(true)
450+
.add(dryRunAllocation)
451+
.execute()
452+
.actionGet();
453+
454+
// during a dry run, messages exist but are not logged or exposed
455+
assertThat(dryRunResponse.getExplanations().getYesDecisionMessages(), hasSize(1));
456+
assertThat(dryRunResponse.getExplanations().getYesDecisionMessages().get(0), containsString("allocated an empty primary"));
457+
458+
dryRunMockLog.assertAllExpectationsMatched();
459+
}
465460

466461
MockLogAppender allocateMockLog = new MockLogAppender();
467-
allocateMockLog.start();
468462
allocateMockLog.addExpectation(
469463
new MockLogAppender.SeenEventExpectation(
470464
"message for first allocate empty primary",
@@ -481,26 +475,25 @@ public void testMessageLogging() throws Exception {
481475
"allocated an empty primary*" + nodeName2 + "*"
482476
)
483477
);
484-
Loggers.addAppender(actionLogger, allocateMockLog);
485-
486-
AllocationCommand yesDecisionAllocation = new AllocateEmptyPrimaryAllocationCommand(indexName, 0, nodeName1, true);
487-
AllocationCommand noDecisionAllocation = new AllocateEmptyPrimaryAllocationCommand("noexist", 1, nodeName2, true);
488-
ClusterRerouteResponse response = client().admin()
489-
.cluster()
490-
.prepareReroute()
491-
.setExplain(true) // so we get a NO decision back rather than an exception
492-
.add(yesDecisionAllocation)
493-
.add(noDecisionAllocation)
494-
.execute()
495-
.actionGet();
496-
497-
assertThat(response.getExplanations().getYesDecisionMessages(), hasSize(1));
498-
assertThat(response.getExplanations().getYesDecisionMessages().get(0), containsString("allocated an empty primary"));
499-
assertThat(response.getExplanations().getYesDecisionMessages().get(0), containsString(nodeName1));
500-
501-
allocateMockLog.assertAllExpectationsMatched();
502-
allocateMockLog.stop();
503-
Loggers.removeAppender(actionLogger, allocateMockLog);
478+
try (var ignored = allocateMockLog.capturing(TransportClusterRerouteAction.class)) {
479+
480+
AllocationCommand yesDecisionAllocation = new AllocateEmptyPrimaryAllocationCommand(indexName, 0, nodeName1, true);
481+
AllocationCommand noDecisionAllocation = new AllocateEmptyPrimaryAllocationCommand("noexist", 1, nodeName2, true);
482+
ClusterRerouteResponse response = client().admin()
483+
.cluster()
484+
.prepareReroute()
485+
.setExplain(true) // so we get a NO decision back rather than an exception
486+
.add(yesDecisionAllocation)
487+
.add(noDecisionAllocation)
488+
.execute()
489+
.actionGet();
490+
491+
assertThat(response.getExplanations().getYesDecisionMessages(), hasSize(1));
492+
assertThat(response.getExplanations().getYesDecisionMessages().get(0), containsString("allocated an empty primary"));
493+
assertThat(response.getExplanations().getYesDecisionMessages().get(0), containsString(nodeName1));
494+
495+
allocateMockLog.assertAllExpectationsMatched();
496+
}
504497
}
505498

506499
public void testClusterRerouteWithBlocks() {

‎server/src/internalClusterTest/java/org/elasticsearch/discovery/single/SingleNodeDiscoveryIT.java‎

Lines changed: 6 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,10 @@
99
package org.elasticsearch.discovery.single;
1010

1111
import org.apache.logging.log4j.Level;
12-
import org.apache.logging.log4j.LogManager;
13-
import org.apache.logging.log4j.Logger;
1412
import org.apache.logging.log4j.core.LogEvent;
1513
import org.elasticsearch.cluster.ClusterState;
1614
import org.elasticsearch.cluster.coordination.JoinHelper;
1715
import org.elasticsearch.cluster.service.ClusterService;
18-
import org.elasticsearch.common.logging.Loggers;
1916
import org.elasticsearch.common.settings.Settings;
2017
import org.elasticsearch.node.Node;
2118
import org.elasticsearch.test.ESIntegTestCase;
@@ -105,7 +102,6 @@ public Path nodeConfigPath(int nodeOrdinal) {
105102

106103
public void testCannotJoinNodeWithSingleNodeDiscovery() throws Exception {
107104
MockLogAppender mockAppender = new MockLogAppender();
108-
mockAppender.start();
109105
mockAppender.addExpectation(
110106
new MockLogAppender.SeenEventExpectation("test", JoinHelper.class.getCanonicalName(), Level.INFO, "failed to join") {
111107

@@ -158,20 +154,13 @@ public Path nodeConfigPath(int nodeOrdinal) {
158154
"other",
159155
Arrays.asList(getTestTransportPlugin(), MockHttpTransport.TestPlugin.class),
160156
Function.identity()
161-
)
157+
);
158+
var ignored = mockAppender.capturing(JoinHelper.class)
162159
) {
163-
164-
Logger clusterLogger = LogManager.getLogger(JoinHelper.class);
165-
Loggers.addAppender(clusterLogger, mockAppender);
166-
try {
167-
other.beforeTest(random());
168-
final ClusterState first = internalCluster().getInstance(ClusterService.class).state();
169-
assertThat(first.nodes().getSize(), equalTo(1));
170-
assertBusy(() -> mockAppender.assertAllExpectationsMatched());
171-
} finally {
172-
Loggers.removeAppender(clusterLogger, mockAppender);
173-
mockAppender.stop();
174-
}
160+
other.beforeTest(random());
161+
final ClusterState first = internalCluster().getInstance(ClusterService.class).state();
162+
assertThat(first.nodes().getSize(), equalTo(1));
163+
assertBusy(mockAppender::assertAllExpectationsMatched);
175164
}
176165
}
177166

‎server/src/test/java/org/elasticsearch/cluster/NodeConnectionsServiceTests.java‎

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
package org.elasticsearch.cluster;
1010

1111
import org.apache.logging.log4j.Level;
12-
import org.apache.logging.log4j.LogManager;
1312
import org.elasticsearch.Build;
1413
import org.elasticsearch.Version;
1514
import org.elasticsearch.action.ActionListener;
@@ -21,7 +20,6 @@
2120
import org.elasticsearch.common.UUIDs;
2221
import org.elasticsearch.common.component.Lifecycle;
2322
import org.elasticsearch.common.component.LifecycleListener;
24-
import org.elasticsearch.common.logging.Loggers;
2523
import org.elasticsearch.common.settings.Settings;
2624
import org.elasticsearch.common.transport.BoundTransportAddress;
2725
import org.elasticsearch.common.transport.TransportAddress;
@@ -357,9 +355,7 @@ public void testDebugLogging() {
357355
transportService.disconnectFromNode(disconnectedNode);
358356
}
359357
MockLogAppender appender = new MockLogAppender();
360-
try {
361-
appender.start();
362-
Loggers.addAppender(LogManager.getLogger("org.elasticsearch.cluster.NodeConnectionsService"), appender);
358+
try (var ignored = appender.capturing(NodeConnectionsService.class)) {
363359
for (DiscoveryNode targetNode : targetNodes) {
364360
if (disconnectedNodes.contains(targetNode)) {
365361
appender.addExpectation(
@@ -400,10 +396,8 @@ public void testDebugLogging() {
400396

401397
runTasksUntil(deterministicTaskQueue, CLUSTER_NODE_RECONNECT_INTERVAL_SETTING.get(Settings.EMPTY).millis());
402398
appender.assertAllExpectationsMatched();
403-
} finally {
404-
Loggers.removeAppender(LogManager.getLogger("org.elasticsearch.cluster.NodeConnectionsService"), appender);
405-
appender.stop();
406399
}
400+
407401
for (DiscoveryNode disconnectedNode : disconnectedNodes) {
408402
transportService.disconnectFromNode(disconnectedNode);
409403
}
@@ -414,9 +408,7 @@ public void testDebugLogging() {
414408
transportService.disconnectFromNode(disconnectedNode);
415409
}
416410
appender = new MockLogAppender();
417-
try {
418-
appender.start();
419-
Loggers.addAppender(LogManager.getLogger("org.elasticsearch.cluster.NodeConnectionsService"), appender);
411+
try (var ignored = appender.capturing(NodeConnectionsService.class)) {
420412
for (DiscoveryNode targetNode : targetNodes) {
421413
if (disconnectedNodes.contains(targetNode) && newTargetNodes.get(targetNode.getId()) != null) {
422414
appender.addExpectation(
@@ -497,9 +489,6 @@ public void testDebugLogging() {
497489
service.connectToNodes(newTargetNodes, () -> {});
498490
deterministicTaskQueue.runAllRunnableTasks();
499491
appender.assertAllExpectationsMatched();
500-
} finally {
501-
Loggers.removeAppender(LogManager.getLogger("org.elasticsearch.cluster.NodeConnectionsService"), appender);
502-
appender.stop();
503492
}
504493
}
505494

0 commit comments

Comments
 (0)