Skips the counter incremet if value is zero (#764)

Signed-off-by: Gagan Juneja <[email protected]> Co-authored-by: Gagan Juneja <[email protected]> (cherry picked from commit 648f69e)
opensearch-project · Nov 17, 2024 · 6770eea · 6770eea
1 parent 99436a5
commit 6770eea
Show file tree

Hide file tree

Showing 4 changed files with 44 additions and 23 deletions.
diff --git a/src/main/java/org/opensearch/performanceanalyzer/action/PerformanceAnalyzerActionFilter.java b/src/main/java/org/opensearch/performanceanalyzer/action/PerformanceAnalyzerActionFilter.java
@@ -17,6 +17,7 @@
 import org.opensearch.core.action.ActionListener;
 import org.opensearch.core.action.ActionResponse;
 import org.opensearch.performanceanalyzer.commons.metrics.PerformanceAnalyzerMetrics;
+import org.opensearch.performanceanalyzer.commons.util.Util;
 import org.opensearch.performanceanalyzer.config.PerformanceAnalyzerController;
 import org.opensearch.tasks.Task;
 
@@ -39,7 +40,10 @@ public <Request extends ActionRequest, Response extends ActionResponse> void app
             ActionListener<Response> listener,
             ActionFilterChain<Request, Response> chain) {
 
-        if (controller.isPerformanceAnalyzerEnabled()) {
+        if (controller.isPerformanceAnalyzerEnabled()
+                && (controller.getCollectorsRunModeValue() == Util.CollectorMode.DUAL.getValue()
+                        || controller.getCollectorsRunModeValue()
+                                == Util.CollectorMode.RCA.getValue())) {
             if (request instanceof BulkRequest) {
                 PerformanceAnalyzerActionListener<Response> newListener =
                         new PerformanceAnalyzerActionListener<>();

diff --git a/...earch/performanceanalyzer/collectors/telemetry/RTFNodeStatsAllShardsMetricsCollector.java b/...earch/performanceanalyzer/collectors/telemetry/RTFNodeStatsAllShardsMetricsCollector.java
@@ -255,18 +255,34 @@ private void recordMetrics(
             nodeStatsMetricsTag.addTag(
                     RTFMetrics.CommonDimension.INDEX_UUID.toString(), shardId.getIndex().getUUID());
         }
+        populateOnlyIfNonZero(
+                cacheQueryMissMetrics, metrics.getQueryCacheMissCount(), nodeStatsMetricsTag);
+        populateOnlyIfNonZero(
+                cacheQuerySizeMetrics, metrics.getQueryCacheInBytes(), nodeStatsMetricsTag);
+        populateOnlyIfNonZero(
+                cacheQueryHitMetrics, metrics.getQueryCacheHitCount(), nodeStatsMetricsTag);
+        populateOnlyIfNonZero(
+                cacheFieldDataEvictionMetrics,
+                metrics.getFieldDataEvictions(),
+                nodeStatsMetricsTag);
+        populateOnlyIfNonZero(
+                cacheFieldDataSizeMetrics, metrics.getFieldDataInBytes(), nodeStatsMetricsTag);
+        populateOnlyIfNonZero(
+                cacheRequestEvictionMetrics,
+                metrics.getRequestCacheEvictions(),
+                nodeStatsMetricsTag);
+        populateOnlyIfNonZero(
+                cacheRequestHitMetrics, metrics.getRequestCacheHitCount(), nodeStatsMetricsTag);
+        populateOnlyIfNonZero(
+                cacheRequestMissMetrics, metrics.getRequestCacheMissCount(), nodeStatsMetricsTag);
+        populateOnlyIfNonZero(
+                cacheRequestSizeMetrics, metrics.getRequestCacheInBytes(), nodeStatsMetricsTag);
+    }
 
-        cacheQueryMissMetrics.add(metrics.getQueryCacheMissCount(), nodeStatsMetricsTag);
-        cacheQuerySizeMetrics.add(metrics.getQueryCacheInBytes(), nodeStatsMetricsTag);
-        cacheQueryHitMetrics.add(metrics.getQueryCacheHitCount(), nodeStatsMetricsTag);
-
-        cacheFieldDataEvictionMetrics.add(metrics.getFieldDataEvictions(), nodeStatsMetricsTag);
-        cacheFieldDataSizeMetrics.add(metrics.getFieldDataInBytes(), nodeStatsMetricsTag);
-
-        cacheRequestEvictionMetrics.add(metrics.getRequestCacheEvictions(), nodeStatsMetricsTag);
-        cacheRequestHitMetrics.add(metrics.getRequestCacheHitCount(), nodeStatsMetricsTag);
-        cacheRequestMissMetrics.add(metrics.getRequestCacheMissCount(), nodeStatsMetricsTag);
-        cacheRequestSizeMetrics.add(metrics.getRequestCacheInBytes(), nodeStatsMetricsTag);
+    private void populateOnlyIfNonZero(Counter counter, double value, Tags tags) {
+        if (value > 0.0) {
+            counter.add(value, tags);
+        }
     }
 
     public void populateDiffMetricValue(

diff --git a/src/main/java/org/opensearch/performanceanalyzer/util/Utils.java b/src/main/java/org/opensearch/performanceanalyzer/util/Utils.java
@@ -48,7 +48,9 @@ public static void configureMetrics() {
         MetricsConfiguration.CONFIG_MAP.put(ShardIndexingPressureMetricsCollector.class, cdefault);
         MetricsConfiguration.CONFIG_MAP.put(RTFDisksCollector.class, cdefault);
         MetricsConfiguration.CONFIG_MAP.put(RTFHeapMetricsCollector.class, cdefault);
-        MetricsConfiguration.CONFIG_MAP.put(RTFNodeStatsAllShardsMetricsCollector.class, cdefault);
+        MetricsConfiguration.CONFIG_MAP.put(
+                RTFNodeStatsAllShardsMetricsCollector.class,
+                new MetricsConfiguration.MetricConfig(60000, 0));
         MetricsConfiguration.CONFIG_MAP.put(RTFThreadPoolMetricsCollector.class, cdefault);
         MetricsConfiguration.CONFIG_MAP.put(
                 RTFCacheConfigMetricsCollector.class,

diff --git a/.../performanceanalyzer/collectors/telemetry/RTFNodeStatsAllShardsMetricsCollectorTests.java b/.../performanceanalyzer/collectors/telemetry/RTFNodeStatsAllShardsMetricsCollectorTests.java
@@ -9,7 +9,6 @@
 import static org.mockito.ArgumentMatchers.anyDouble;
 import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.Mockito.*;
-import static org.mockito.Mockito.atLeastOnce;
 
 import java.io.IOException;
 import org.junit.After;
@@ -125,14 +124,14 @@ public void testCollectMetrics() throws IOException {
         rtfNodeStatsAllShardsMetricsCollector.collectMetrics(startTimeInMills);
         verify(rtfNodeStatsAllShardsMetricsCollector, times(1))
                 .populateDiffMetricValue(any(), any(), any());
-        verify(cacheFieldDataEvictionCounter, atLeastOnce()).add(anyDouble(), any());
-        verify(cacheFieldDataSizeCounter, atLeastOnce()).add(anyDouble(), any());
-        verify(cacheQueryMissCounter, atLeastOnce()).add(anyDouble(), any());
-        verify(cacheQueryHitCounter, atLeastOnce()).add(anyDouble(), any());
-        verify(cacheQuerySizeCounter, atLeastOnce()).add(anyDouble(), any());
-        verify(cacheRequestEvictionCounter, atLeastOnce()).add(anyDouble(), any());
-        verify(cacheRequestHitCounter, atLeastOnce()).add(anyDouble(), any());
-        verify(cacheRequestMissCounter, atLeastOnce()).add(anyDouble(), any());
-        verify(cacheRequestSizeCounter, atLeastOnce()).add(anyDouble(), any());
+        verify(cacheFieldDataEvictionCounter, never()).add(anyDouble(), any());
+        verify(cacheFieldDataSizeCounter, never()).add(anyDouble(), any());
+        verify(cacheQueryMissCounter, never()).add(anyDouble(), any());
+        verify(cacheQueryHitCounter, never()).add(anyDouble(), any());
+        verify(cacheQuerySizeCounter, never()).add(anyDouble(), any());
+        verify(cacheRequestEvictionCounter, never()).add(anyDouble(), any());
+        verify(cacheRequestHitCounter, never()).add(anyDouble(), any());
+        verify(cacheRequestMissCounter, never()).add(anyDouble(), any());
+        verify(cacheRequestSizeCounter, never()).add(anyDouble(), any());
     }
 }