Fix: Updates UT to check the node.left.count metric

Signed-off-by: Bhumika Sharma <[email protected]>

Author: Bhumika Sharma

server/src/main/java/org/opensearch/cluster/coordination/Coordinator.java

@@ -378,7 +378,9 @@ private void removeNode(DiscoveryNode discoveryNode, String reason) {
                 clusterManagerMetrics.incrementCounter(
                     clusterManagerMetrics.nodeLeftCounter,
                     1.0,
-                    Optional.ofNullable(Tags.create().addTag(FOLLOWER_NODE_ID_TAG, discoveryNode.getId()).addTag(REASON_TAG, reasonToPublish))
+                    Optional.ofNullable(
+                        Tags.create().addTag(FOLLOWER_NODE_ID_TAG, discoveryNode.getId()).addTag(REASON_TAG, reasonToPublish)
+                    )
                 );
             }
         }

server/src/test/java/org/opensearch/cluster/coordination/CoordinatorTests.java

@@ -57,13 +57,17 @@
 import org.opensearch.gateway.GatewayService;
 import org.opensearch.monitor.StatusInfo;
 import org.opensearch.test.MockLogAppender;
+import org.opensearch.test.telemetry.TestInMemoryCounter;
+import org.opensearch.test.telemetry.TestInMemoryMetricsRegistry;
 
 import java.io.IOException;
 import java.util.Arrays;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.function.Function;
@@ -250,6 +254,25 @@ public void testUnhealthyNodesGetsRemoved() {
                 assertThat(lastCommittedConfiguration + " should be 3 nodes", lastCommittedConfiguration.getNodeIds().size(), equalTo(3));
                 assertFalse(lastCommittedConfiguration.getNodeIds().contains(newNode1.getId()));
                 assertFalse(lastCommittedConfiguration.getNodeIds().contains(newNode2.getId()));
+
+                TestInMemoryMetricsRegistry clusterManagerMetricsRegistry = leader.getMetricsRegistry();
+                TestInMemoryCounter nodeLeftCounter = clusterManagerMetricsRegistry.getCounterStore().get("node.left.count");
+                assertNotNull("node.left.count counter should be present", nodeLeftCounter);
+                ConcurrentHashMap<Map<String, ?>, Double> counterValuesByTags = nodeLeftCounter.getCounterValueForTags();
+
+                // Check for newNode1
+                Map<String, Object> tags1 = new HashMap<>();
+                tags1.put("follower_node_id", newNode1.getId());
+                tags1.put("reason", "health.check.fail");
+                assertTrue(counterValuesByTags.containsKey(tags1));
+                assertEquals(Double.valueOf(1.0), counterValuesByTags.get(tags1));
+
+                // Check for newNode2
+                Map<String, Object> tags2 = new HashMap<>();
+                tags2.put("follower_node_id", newNode2.getId());
+                tags2.put("reason", "health.check.fail");
+                assertTrue(counterValuesByTags.containsKey(tags2));
+                assertEquals(Double.valueOf(1.0), counterValuesByTags.get(tags2));
             }
         }
     }

server/src/test/java/org/opensearch/cluster/coordination/FollowersCheckerTests.java

@@ -48,13 +48,13 @@
 import org.opensearch.core.transport.TransportResponse.Empty;
 import org.opensearch.monitor.NodeHealthService;
 import org.opensearch.monitor.StatusInfo;
-import org.opensearch.telemetry.TestInMemoryMetricsRegistry;
 import org.opensearch.telemetry.metrics.MetricsRegistry;
 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;
 import org.opensearch.test.OpenSearchTestCase;
+import org.opensearch.test.telemetry.TestInMemoryMetricsRegistry;
 import org.opensearch.test.transport.CapturingTransport;
 import org.opensearch.test.transport.MockTransport;
 import org.opensearch.threadpool.ThreadPool.Names;

server/src/test/java/org/opensearch/cluster/coordination/LeaderCheckerTests.java

@@ -45,11 +45,11 @@
 import org.opensearch.core.transport.TransportResponse;
 import org.opensearch.core.transport.TransportResponse.Empty;
 import org.opensearch.monitor.StatusInfo;
-import org.opensearch.telemetry.TestInMemoryMetricsRegistry;
 import org.opensearch.telemetry.tracing.noop.NoopTracer;
 import org.opensearch.test.EqualsHashCodeTestUtils;
 import org.opensearch.test.EqualsHashCodeTestUtils.CopyFunction;
 import org.opensearch.test.OpenSearchTestCase;
+import org.opensearch.test.telemetry.TestInMemoryMetricsRegistry;
 import org.opensearch.test.transport.CapturingTransport;
 import org.opensearch.test.transport.MockTransport;
 import org.opensearch.threadpool.ThreadPool.Names;

test/framework/build.gradle

@@ -39,6 +39,7 @@ dependencies {
   api project(':libs:opensearch-nio')
   api project(":server")
   api project(":libs:opensearch-cli")
+  api project(":libs:opensearch-telemetry")
   api project(":test:telemetry")
   api "com.carrotsearch.randomizedtesting:randomizedtesting-runner:${versions.randomizedrunner}"
   api "junit:junit:${versions.junit}"

test/framework/src/main/java/org/opensearch/cluster/coordination/AbstractCoordinatorTestCase.java

@@ -96,6 +96,7 @@
 import org.opensearch.test.OpenSearchTestCase;
 import org.opensearch.test.disruption.DisruptableMockTransport;
 import org.opensearch.test.disruption.DisruptableMockTransport.ConnectionStatus;
+import org.opensearch.test.telemetry.TestInMemoryMetricsRegistry;
 import org.opensearch.threadpool.Scheduler;
 import org.opensearch.threadpool.ThreadPool;
 import org.opensearch.transport.TransportInterceptor;
@@ -1059,6 +1060,7 @@ class ClusterNode {
             private RepositoriesService repositoriesService;
             private RemoteStoreNodeService remoteStoreNodeService;
             List<BiConsumer<DiscoveryNode, ClusterState>> extraJoinValidators = new ArrayList<>();
+            private TestInMemoryMetricsRegistry metricsRegistry = new TestInMemoryMetricsRegistry();
 
             ClusterNode(int nodeIndex, boolean clusterManagerEligible, Settings nodeSettings, NodeHealthService nodeHealthService) {
                 this(
@@ -1188,7 +1190,7 @@ protected Optional<DisruptableMockTransport> getDisruptableMockTransport(Transpo
                     nodeHealthService,
                     persistedStateRegistry,
                     remoteStoreNodeService,
-                    new ClusterManagerMetrics(NoopMetricsRegistry.INSTANCE),
+                    new ClusterManagerMetrics(metricsRegistry),
                     null
                 );
                 coordinator.setNodeConnectionsService(nodeConnectionsService);
@@ -1211,6 +1213,10 @@ protected Optional<DisruptableMockTransport> getDisruptableMockTransport(Transpo
                 coordinator.startInitialJoin();
             }
 
+            public TestInMemoryMetricsRegistry getMetricsRegistry() {
+                return metricsRegistry;
+            }
+
             void close() {
                 assertThat("must add nodes to a cluster before closing them", clusterNodes, hasItem(ClusterNode.this));
                 onNode(() -> {

server/src/test/java/org/opensearch/telemetry/TestInMemoryCounter.java renamed to test/telemetry/src/main/java/org/opensearch/test/telemetry/TestInMemoryCounter.java

@@ -6,12 +6,12 @@
  * compatible open source license.
  */
 
-package org.opensearch.telemetry;
+package org.opensearch.test.telemetry;
 
 import org.opensearch.telemetry.metrics.Counter;
 import org.opensearch.telemetry.metrics.tags.Tags;
 
-import java.util.HashMap;
+import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.atomic.AtomicInteger;
 
@@ -24,13 +24,26 @@
 public class TestInMemoryCounter implements Counter {
 
     private AtomicInteger counterValue = new AtomicInteger(0);
-    private ConcurrentHashMap<HashMap<String, ?>, Double> counterValueForTags = new ConcurrentHashMap<>();
+    private final ConcurrentHashMap<Map<String, ?>, Double> counterValueForTags = new ConcurrentHashMap<>();
 
+    /**
+     * Constructor.
+     */
+    public TestInMemoryCounter() {}
+
+    /**
+     * returns the counter value.
+     * @return
+     */
     public Integer getCounterValue() {
         return this.counterValue.get();
     }
 
-    public ConcurrentHashMap<HashMap<String, ?>, Double> getCounterValueForTags() {
+    /**
+     * returns the counter value tags
+     * @return
+     */
+    public ConcurrentHashMap<Map<String, ?>, Double> getCounterValueForTags() {
         return this.counterValueForTags;
     }
 
@@ -41,12 +54,12 @@ public void add(double value) {
 
     @Override
     public synchronized void add(double value, Tags tags) {
-        HashMap<String, ?> hashMap = (HashMap<String, ?>) tags.getTagsMap();
-        if (counterValueForTags.get(hashMap) == null) {
-            counterValueForTags.put(hashMap, value);
+        Map<String, ?> tagsMap = tags.getTagsMap();
+        if (counterValueForTags.get(tagsMap) == null) {
+            counterValueForTags.put(tagsMap, value);
         } else {
-            value = counterValueForTags.get(hashMap) + value;
-            counterValueForTags.put(hashMap, value);
+            value = counterValueForTags.get(tagsMap) + value;
+            counterValueForTags.put(tagsMap, value);
         }
     }
 }

server/src/test/java/org/opensearch/telemetry/TestInMemoryHistogram.java renamed to test/telemetry/src/main/java/org/opensearch/test/telemetry/TestInMemoryHistogram.java

@@ -6,7 +6,7 @@
  * compatible open source license.
  */
 
-package org.opensearch.telemetry;
+package org.opensearch.test.telemetry;
 
 import org.opensearch.telemetry.metrics.Histogram;
 import org.opensearch.telemetry.metrics.tags.Tags;
@@ -26,10 +26,23 @@ public class TestInMemoryHistogram implements Histogram {
     private AtomicInteger histogramValue = new AtomicInteger(0);
     private ConcurrentHashMap<HashMap<String, ?>, Double> histogramValueForTags = new ConcurrentHashMap<>();
 
+    /**
+     * Constructor.
+     */
+    public TestInMemoryHistogram() {}
+
+    /**
+     * Returns the Histogram value.
+     * @return
+     */
     public Integer getHistogramValue() {
         return this.histogramValue.get();
     }
 
+    /**
+     * Returns the Histogram value for tags
+     * @return
+     */
     public ConcurrentHashMap<HashMap<String, ?>, Double> getHistogramValueForTags() {
         return this.histogramValueForTags;
     }

server/src/test/java/org/opensearch/telemetry/TestInMemoryMetricsRegistry.java renamed to test/telemetry/src/main/java/org/opensearch/test/telemetry/TestInMemoryMetricsRegistry.java

@@ -6,7 +6,7 @@
  * compatible open source license.
  */
 
-package org.opensearch.telemetry;
+package org.opensearch.test.telemetry;
 
 import org.opensearch.telemetry.metrics.Counter;
 import org.opensearch.telemetry.metrics.Histogram;
@@ -29,10 +29,23 @@ public class TestInMemoryMetricsRegistry implements MetricsRegistry {
     private ConcurrentHashMap<String, TestInMemoryCounter> counterStore = new ConcurrentHashMap<>();
     private ConcurrentHashMap<String, TestInMemoryHistogram> histogramStore = new ConcurrentHashMap<>();
 
+    /**
+     * Constructor.
+     */
+    public TestInMemoryMetricsRegistry() {}
+
+    /**
+     * Returns counterStore
+     * @return
+     */
     public ConcurrentHashMap<String, TestInMemoryCounter> getCounterStore() {
         return this.counterStore;
     }
 
+    /**
+     * Returns the histogramStore.
+     * @return
+     */
     public ConcurrentHashMap<String, TestInMemoryHistogram> getHistogramStore() {
         return this.histogramStore;
     }

test/telemetry/src/main/java/org/opensearch/test/telemetry/package-info.java

@@ -0,0 +1,10 @@
+/*
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * The OpenSearch Contributors require contributions made to
+ * this file be licensed under the Apache-2.0 license or a
+ * compatible open source license.
+ */
+
+/** Base opensearch package. */
+package org.opensearch.test.telemetry;