addressed comments

Signed-off-by: Atharva Sharma <[email protected]>
opensearch-project · Sep 2, 2024 · 4f3f508 · 4f3f508
1 parent 5ace5ac
commit 4f3f508
Show file tree

Hide file tree

Showing 5 changed files with 10 additions and 176 deletions.
diff --git a/src/main/java/org/opensearch/performanceanalyzer/PerformanceAnalyzerPlugin.java b/src/main/java/org/opensearch/performanceanalyzer/PerformanceAnalyzerPlugin.java
@@ -59,7 +59,6 @@
 import org.opensearch.performanceanalyzer.collectors.telemetry.RTFDisksCollector;
 import org.opensearch.performanceanalyzer.collectors.telemetry.RTFHeapMetricsCollector;
 import org.opensearch.performanceanalyzer.collectors.telemetry.RTFNodeStatsAllShardsMetricsCollector;
-import org.opensearch.performanceanalyzer.collectors.telemetry.RTFOSMetricsCollector;
 import org.opensearch.performanceanalyzer.collectors.telemetry.RTFThreadPoolMetricsCollector;
 import org.opensearch.performanceanalyzer.commons.OSMetricsGeneratorFactory;
 import org.opensearch.performanceanalyzer.commons.collectors.DisksCollector;
@@ -240,8 +239,6 @@ private void scheduleTelemetryCollectors() {
         scheduledMetricCollectorsExecutor.addScheduledMetricCollector(
                 new RTFCacheConfigMetricsCollector(
                         performanceAnalyzerController, configOverridesWrapper));
-        scheduledMetricCollectorsExecutor.addScheduledMetricCollector(
-                new RTFOSMetricsCollector(performanceAnalyzerController, configOverridesWrapper));
     }
 
     private void scheduleRcaCollectors() {

diff --git a/...g/opensearch/performanceanalyzer/collectors/telemetry/RTFCacheConfigMetricsCollector.java b/...g/opensearch/performanceanalyzer/collectors/telemetry/RTFCacheConfigMetricsCollector.java
@@ -29,15 +29,12 @@
 import org.opensearch.performanceanalyzer.commons.stats.metrics.StatExceptionCode;
 import org.opensearch.performanceanalyzer.commons.stats.metrics.StatMetrics;
 import org.opensearch.performanceanalyzer.config.PerformanceAnalyzerController;
-import org.opensearch.telemetry.metrics.Histogram;
 import org.opensearch.telemetry.metrics.MetricsRegistry;
 import org.opensearch.telemetry.metrics.tags.Tags;
 
 public class RTFCacheConfigMetricsCollector extends PerformanceAnalyzerMetricsCollector
         implements TelemetryCollector {
     private MetricsRegistry metricsRegistry;
-    private boolean metricsInitialised;
-    private Histogram cacheMaxSizeMetrics;
     private static final Logger LOG = LogManager.getLogger(RTFCacheConfigMetricsCollector.class);
     private PerformanceAnalyzerController performanceAnalyzerController;
     private ConfigOverridesWrapper configOverridesWrapper;
@@ -51,7 +48,6 @@ public RTFCacheConfigMetricsCollector(
                 "RTFCacheConfigMetricsCollector",
                 StatMetrics.RTF_CACHE_CONFIG_METRICS_COLLECTOR_EXECUTION_TIME,
                 StatExceptionCode.RTF_CACHE_CONFIG_METRICS_COLLECTOR_ERROR);
-        this.metricsInitialised = false;
         this.performanceAnalyzerController = performanceAnalyzerController;
         this.configOverridesWrapper = configOverridesWrapper;
     }
@@ -77,7 +73,6 @@ configOverridesWrapper, getCollectorName())) {
         }
 
         LOG.debug("Executing collect metrics for RTFCacheConfigMetricsCollector");
-        initialiseMetricsIfNeeded();
         CacheMaxSizeStatus fieldDataCacheMaxSizeStatus =
                 AccessController.doPrivileged(
                         (PrivilegedAction<CacheMaxSizeStatus>)
@@ -99,8 +94,7 @@ configOverridesWrapper, getCollectorName())) {
                                         LOG.debug(
                                                 "Error occurred while fetching fieldDataCacheMaxSizeStatus: "
                                                         + e.getMessage());
-                                        return new CacheMaxSizeStatus(
-                                                FIELD_DATA_CACHE.toString(), -1L);
+                                        return null;
                                     }
                                 });
 
@@ -135,16 +129,12 @@ configOverridesWrapper, getCollectorName())) {
                                         LOG.debug(
                                                 "Error occurred while fetching shardRequestCacheMaxSizeStatus: "
                                                         + e.getMessage());
-                                        return new CacheMaxSizeStatus(
-                                                SHARD_REQUEST_CACHE.toString(), -1L);
+                                        return null;
                                     }
                                 });
 
         if (fieldDataCacheMaxSizeStatus != null
                 && fieldDataCacheMaxSizeStatus.getCacheMaxSize() > 0) {
-            LOG.info(
-                    "fieldDataCacheMaxSizeStatus.getCacheMaxSize() value : "
-                            + fieldDataCacheMaxSizeStatus.getCacheMaxSize());
             recordMetrics(fieldDataCacheMaxSizeStatus);
         }
 
@@ -155,25 +145,17 @@ configOverridesWrapper, getCollectorName())) {
     }
 
     private void recordMetrics(CacheMaxSizeStatus cacheMaxSizeStatus) {
-        cacheMaxSizeMetrics.record(
-                cacheMaxSizeStatus.getCacheMaxSize(),
+        metricsRegistry.createGauge(
+                RTFMetrics.CacheConfigValue.Constants.CACHE_MAX_SIZE_VALUE,
+                "Cache Max Size metrics",
+                RTFMetrics.MetricUnits.BYTE.toString(),
+                () -> (double) cacheMaxSizeStatus.getCacheMaxSize(),
                 Tags.create()
                         .addTag(
                                 RTFMetrics.CacheConfigDimension.Constants.TYPE_VALUE,
                                 cacheMaxSizeStatus.getCacheType()));
     }
 
-    private void initialiseMetricsIfNeeded() {
-        if (metricsInitialised == false) {
-            cacheMaxSizeMetrics =
-                    metricsRegistry.createHistogram(
-                            RTFMetrics.CacheConfigValue.Constants.CACHE_MAX_SIZE_VALUE,
-                            "CacheMaxSizeMetrics",
-                            RTFMetrics.MetricUnits.BYTE.toString());
-            metricsInitialised = true;
-        }
-    }
-
     static class CacheMaxSizeStatus extends MetricStatus {
 
         private final String cacheType;

diff --git a/...n/java/org/opensearch/performanceanalyzer/collectors/telemetry/RTFOSMetricsCollector.java b/...n/java/org/opensearch/performanceanalyzer/collectors/telemetry/RTFOSMetricsCollector.java
diff --git a/src/main/java/org/opensearch/performanceanalyzer/util/Utils.java b/src/main/java/org/opensearch/performanceanalyzer/util/Utils.java
@@ -51,8 +51,9 @@ public static void configureMetrics() {
         MetricsConfiguration.CONFIG_MAP.put(RTFHeapMetricsCollector.class, cdefault);
         MetricsConfiguration.CONFIG_MAP.put(RTFNodeStatsAllShardsMetricsCollector.class, cdefault);
         MetricsConfiguration.CONFIG_MAP.put(RTFThreadPoolMetricsCollector.class, cdefault);
-        MetricsConfiguration.CONFIG_MAP.put(RTFOSMetricsCollector.class, cdefault);
-        MetricsConfiguration.CONFIG_MAP.put(RTFCacheConfigMetricsCollector.class, cdefault);
+        MetricsConfiguration.CONFIG_MAP.put(
+                RTFCacheConfigMetricsCollector.class,
+                new MetricsConfiguration.MetricConfig(60000, 0));
     }
 
     // These methods are utility functions for the Node Stat Metrics Collectors. These methods are

diff --git a/...a/org/opensearch/performanceanalyzer/collectors/telemetry/RTFOSMetricsCollectorTests.java b/...a/org/opensearch/performanceanalyzer/collectors/telemetry/RTFOSMetricsCollectorTests.java