diff --git a/server/src/main/java/org/opensearch/cluster/coordination/ClusterManagerMetrics.java b/server/src/main/java/org/opensearch/cluster/coordination/ClusterManagerMetrics.java index 27a269202f048..5b64787ecb8a1 100644 --- a/server/src/main/java/org/opensearch/cluster/coordination/ClusterManagerMetrics.java +++ b/server/src/main/java/org/opensearch/cluster/coordination/ClusterManagerMetrics.java @@ -26,12 +26,12 @@ public final class ClusterManagerMetrics { public final Counter followerChecksFailureCounter; public ClusterManagerMetrics(MetricsRegistry metricsRegistry) { - this.followerChecksFailureCounter = metricsRegistry.createCounter( + followerChecksFailureCounter = metricsRegistry.createCounter( "followers.checker.failure.count", "Counter for number of failed follower checks", COUNTER_METRICS_UNIT ); - this.leaderCheckFailureCounter = metricsRegistry.createCounter( + leaderCheckFailureCounter = metricsRegistry.createCounter( "leader.checker.failure.count", "Counter for number of failed leader checks", COUNTER_METRICS_UNIT diff --git a/server/src/test/java/org/opensearch/cluster/coordination/FollowersCheckerTests.java b/server/src/test/java/org/opensearch/cluster/coordination/FollowersCheckerTests.java index 229caa8cf4b52..023ddbbdbe3ad 100644 --- a/server/src/test/java/org/opensearch/cluster/coordination/FollowersCheckerTests.java +++ b/server/src/test/java/org/opensearch/cluster/coordination/FollowersCheckerTests.java @@ -134,6 +134,8 @@ protected void onSendRequest(long requestId, String action, TransportRequest req transportService.start(); transportService.acceptIncomingRequests(); + TestInMemoryMetricsRegistry metricsRegistry = new TestInMemoryMetricsRegistry(); + final FollowersChecker followersChecker = new FollowersChecker( settings, clusterSettings, @@ -143,7 +145,7 @@ protected void onSendRequest(long requestId, String action, TransportRequest req assert false : node; }, () -> new StatusInfo(StatusInfo.Status.HEALTHY, "healthy-info"), - new ClusterManagerMetrics(NoopMetricsRegistry.INSTANCE) + new ClusterManagerMetrics(metricsRegistry) ); followersChecker.setCurrentNodes(discoveryNodesHolder[0]); @@ -197,6 +199,7 @@ protected void onSendRequest(long requestId, String action, TransportRequest req followersChecker.clearCurrentNodes(); deterministicTaskQueue.runAllTasks(); assertThat(checkedNodes, empty()); + assertEquals(Integer.valueOf(0), metricsRegistry.getCounterStore().get("followers.checker.failure.count").getCounterValue()); } public void testFailsNodeThatDoesNotRespond() { @@ -313,6 +316,7 @@ public String toString() { transportService.acceptIncomingRequests(); final AtomicBoolean nodeFailed = new AtomicBoolean(); + TestInMemoryMetricsRegistry metricsRegistry = new TestInMemoryMetricsRegistry(); final FollowersChecker followersChecker = new FollowersChecker( settings, @@ -324,7 +328,7 @@ public String toString() { assertThat(reason, equalTo("disconnected")); }, () -> new StatusInfo(HEALTHY, "healthy-info"), - new ClusterManagerMetrics(NoopMetricsRegistry.INSTANCE) + new ClusterManagerMetrics(metricsRegistry) ); DiscoveryNodes discoveryNodes = DiscoveryNodes.builder().add(localNode).add(otherNode).localNodeId(localNode.getId()).build(); @@ -335,6 +339,7 @@ public String toString() { deterministicTaskQueue.runAllRunnableTasks(); assertTrue(nodeFailed.get()); assertThat(followersChecker.getFaultyNodes(), contains(otherNode)); + assertEquals(Integer.valueOf(1), metricsRegistry.getCounterStore().get("followers.checker.failure.count").getCounterValue()); } public void testFailsNodeThatIsUnhealthy() { diff --git a/server/src/test/java/org/opensearch/cluster/coordination/LeaderCheckerTests.java b/server/src/test/java/org/opensearch/cluster/coordination/LeaderCheckerTests.java index 87ba9947f90ed..2671bad0b8b9e 100644 --- a/server/src/test/java/org/opensearch/cluster/coordination/LeaderCheckerTests.java +++ b/server/src/test/java/org/opensearch/cluster/coordination/LeaderCheckerTests.java @@ -45,7 +45,6 @@ import org.opensearch.core.transport.TransportResponse.Empty; import org.opensearch.monitor.StatusInfo; import org.opensearch.telemetry.TestInMemoryMetricsRegistry; -import org.opensearch.telemetry.metrics.noop.NoopMetricsRegistry; import org.opensearch.telemetry.tracing.noop.NoopTracer; import org.opensearch.test.EqualsHashCodeTestUtils; import org.opensearch.test.EqualsHashCodeTestUtils.CopyFunction; @@ -466,7 +465,8 @@ public void testLeaderBehaviour() { transportService.start(); transportService.acceptIncomingRequests(); - final ClusterManagerMetrics clusterManagerMetrics = new ClusterManagerMetrics(NoopMetricsRegistry.INSTANCE); + final TestInMemoryMetricsRegistry metricsRegistry = new TestInMemoryMetricsRegistry(); + final ClusterManagerMetrics clusterManagerMetrics = new ClusterManagerMetrics(metricsRegistry); final LeaderChecker leaderChecker = new LeaderChecker( settings, clusterSettings, @@ -538,6 +538,7 @@ public void testLeaderBehaviour() { equalTo("rejecting leader check from [" + otherNode + "] sent to a node that is no longer the cluster-manager") ); } + assertEquals(Integer.valueOf(0), metricsRegistry.getCounterStore().get("leader.checker.failure.count").getCounterValue()); } private class CapturingTransportResponseHandler implements TransportResponseHandler { diff --git a/server/src/test/java/org/opensearch/telemetry/TestInMemoryMetricsRegistry.java b/server/src/test/java/org/opensearch/telemetry/TestInMemoryMetricsRegistry.java index 761efc0fe96aa..6d395085b12ea 100644 --- a/server/src/test/java/org/opensearch/telemetry/TestInMemoryMetricsRegistry.java +++ b/server/src/test/java/org/opensearch/telemetry/TestInMemoryMetricsRegistry.java @@ -25,13 +25,8 @@ */ public class TestInMemoryMetricsRegistry implements MetricsRegistry { - private ConcurrentHashMap counterStore; - private ConcurrentHashMap histogramStore; - - public TestInMemoryMetricsRegistry() { - this.counterStore = new ConcurrentHashMap<>(); - this.histogramStore = new ConcurrentHashMap<>(); - } + private ConcurrentHashMap counterStore = new ConcurrentHashMap<>(); + private ConcurrentHashMap histogramStore = new ConcurrentHashMap<>(); public ConcurrentHashMap getCounterStore() { return this.counterStore;