From 4a19407148c0209a3963d649c68735d05afc8f2a Mon Sep 17 00:00:00 2001 From: Khushboo Rajput <59671881+khushbr@users.noreply.github.com> Date: Tue, 10 Oct 2023 14:39:06 -0700 Subject: [PATCH] Remove redundant ClusterManagerThrottlingMetricsCollector (#582) * Remove redundant ClusterManagerThrottlingMetricsCollector Signed-off-by: Khushboo Rajput * Fix typo in ClusterManagerServiceEventMetrics Signed-off-by: Khushboo Rajput --------- Signed-off-by: Khushboo Rajput (cherry picked from commit 9e2b749d317d2cedd98b847f09c7f31f7c94c765) --- build.gradle | 1 - .../PerformanceAnalyzerPlugin.java | 4 - .../ClusterManagerServiceEventMetrics.java | 2 +- ...sterManagerThrottlingMetricsCollector.java | 168 ------------------ .../performanceanalyzer/util/Utils.java | 2 - ...anagerThrottlingMetricsCollectorTests.java | 55 ------ 6 files changed, 1 insertion(+), 231 deletions(-) delete mode 100644 src/main/java/org/opensearch/performanceanalyzer/collectors/ClusterManagerThrottlingMetricsCollector.java delete mode 100644 src/test/java/org/opensearch/performanceanalyzer/collectors/ClusterManagerThrottlingMetricsCollectorTests.java diff --git a/build.gradle b/build.gradle index ab67f267..bac56115 100644 --- a/build.gradle +++ b/build.gradle @@ -229,7 +229,6 @@ jacocoTestReport { ], exclude: [ '**/FaultDetectionMetricsCollector.class', - '**/ClusterManagerThrottlingMetricsCollector.class', '**/ClusterSettingsManager.class', ]) }) diff --git a/src/main/java/org/opensearch/performanceanalyzer/PerformanceAnalyzerPlugin.java b/src/main/java/org/opensearch/performanceanalyzer/PerformanceAnalyzerPlugin.java index 4eedc307..f2fe6cc4 100644 --- a/src/main/java/org/opensearch/performanceanalyzer/PerformanceAnalyzerPlugin.java +++ b/src/main/java/org/opensearch/performanceanalyzer/PerformanceAnalyzerPlugin.java @@ -47,7 +47,6 @@ import org.opensearch.performanceanalyzer.collectors.ClusterApplierServiceStatsCollector; import org.opensearch.performanceanalyzer.collectors.ClusterManagerServiceEventMetrics; import org.opensearch.performanceanalyzer.collectors.ClusterManagerServiceMetrics; -import org.opensearch.performanceanalyzer.collectors.ClusterManagerThrottlingMetricsCollector; import org.opensearch.performanceanalyzer.collectors.ElectionTermCollector; import org.opensearch.performanceanalyzer.collectors.FaultDetectionMetricsCollector; import org.opensearch.performanceanalyzer.collectors.NodeDetailsCollector; @@ -216,9 +215,6 @@ public PerformanceAnalyzerPlugin(final Settings settings, final java.nio.file.Pa performanceAnalyzerController, configOverridesWrapper)); scheduledMetricCollectorsExecutor.addScheduledMetricCollector( new ShardStateCollector(performanceAnalyzerController, configOverridesWrapper)); - scheduledMetricCollectorsExecutor.addScheduledMetricCollector( - new ClusterManagerThrottlingMetricsCollector( - performanceAnalyzerController, configOverridesWrapper)); scheduledMetricCollectorsExecutor.addScheduledMetricCollector( new ClusterApplierServiceStatsCollector( performanceAnalyzerController, configOverridesWrapper)); diff --git a/src/main/java/org/opensearch/performanceanalyzer/collectors/ClusterManagerServiceEventMetrics.java b/src/main/java/org/opensearch/performanceanalyzer/collectors/ClusterManagerServiceEventMetrics.java index c90c02ab..8af21bc6 100644 --- a/src/main/java/org/opensearch/performanceanalyzer/collectors/ClusterManagerServiceEventMetrics.java +++ b/src/main/java/org/opensearch/performanceanalyzer/collectors/ClusterManagerServiceEventMetrics.java @@ -160,7 +160,7 @@ public void collectMetrics(long startTime) { | IllegalAccessException | ClassNotFoundException e) { LOG.debug( - "[ {} ] Exception raised while getting Cluster Manager throttling metrics: {} ", + "[ {} ] Exception raised while getting Cluster Manager Service Event metrics: {} ", this::getCollectorName, e::getMessage); StatsCollector.instance().logException(CLUSTER_MANAGER_METRICS_ERROR); diff --git a/src/main/java/org/opensearch/performanceanalyzer/collectors/ClusterManagerThrottlingMetricsCollector.java b/src/main/java/org/opensearch/performanceanalyzer/collectors/ClusterManagerThrottlingMetricsCollector.java deleted file mode 100644 index 92b4d005..00000000 --- a/src/main/java/org/opensearch/performanceanalyzer/collectors/ClusterManagerThrottlingMetricsCollector.java +++ /dev/null @@ -1,168 +0,0 @@ -/* - * Copyright OpenSearch Contributors - * SPDX-License-Identifier: Apache-2.0 - */ - -package org.opensearch.performanceanalyzer.collectors; - -import static org.opensearch.performanceanalyzer.commons.stats.metrics.StatExceptionCode.CLUSTER_MANAGER_THROTTLING_COLLECTOR_ERROR; -import static org.opensearch.performanceanalyzer.commons.stats.metrics.StatMetrics.CLUSTER_MANAGER_THROTTLING_COLLECTOR_EXECUTION_TIME; - -import com.fasterxml.jackson.annotation.JsonProperty; -import java.lang.reflect.InvocationTargetException; -import java.lang.reflect.Method; -import java.util.Objects; -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; -import org.opensearch.cluster.service.ClusterManagerService; -import org.opensearch.performanceanalyzer.OpenSearchResources; -import org.opensearch.performanceanalyzer.commons.collectors.MetricStatus; -import org.opensearch.performanceanalyzer.commons.collectors.PerformanceAnalyzerMetricsCollector; -import org.opensearch.performanceanalyzer.commons.collectors.StatsCollector; -import org.opensearch.performanceanalyzer.commons.config.overrides.ConfigOverridesWrapper; -import org.opensearch.performanceanalyzer.commons.metrics.AllMetrics; -import org.opensearch.performanceanalyzer.commons.metrics.MetricsConfiguration; -import org.opensearch.performanceanalyzer.commons.metrics.MetricsProcessor; -import org.opensearch.performanceanalyzer.commons.metrics.PerformanceAnalyzerMetrics; -import org.opensearch.performanceanalyzer.commons.stats.ServiceMetrics; -import org.opensearch.performanceanalyzer.commons.stats.metrics.StatMetrics; -import org.opensearch.performanceanalyzer.config.PerformanceAnalyzerController; - -public class ClusterManagerThrottlingMetricsCollector extends PerformanceAnalyzerMetricsCollector - implements MetricsProcessor { - - public static final int SAMPLING_TIME_INTERVAL = - MetricsConfiguration.CONFIG_MAP.get(ClusterManagerThrottlingMetricsCollector.class) - .samplingInterval; - private static final Logger LOG = - LogManager.getLogger(ClusterManagerThrottlingMetricsCollector.class); - - private static final int KEYS_PATH_LENGTH = 0; - private static final String CLUSTER_MANAGER_THROTTLING_RETRY_LISTENER_PATH = - "org.opensearch.action.support.master.MasterThrottlingRetryListener"; - private static final String THROTTLED_PENDING_TASK_COUNT_METHOD_NAME = - "numberOfThrottledPendingTasks"; - private static final String RETRYING_TASK_COUNT_METHOD_NAME = "getRetryingTasksCount"; - private final StringBuilder value; - private final PerformanceAnalyzerController controller; - private final ConfigOverridesWrapper configOverridesWrapper; - - public ClusterManagerThrottlingMetricsCollector( - PerformanceAnalyzerController controller, - ConfigOverridesWrapper configOverridesWrapper) { - super( - SAMPLING_TIME_INTERVAL, - "ClusterManagerThrottlingMetricsCollector", - CLUSTER_MANAGER_THROTTLING_COLLECTOR_EXECUTION_TIME, - CLUSTER_MANAGER_THROTTLING_COLLECTOR_ERROR); - value = new StringBuilder(); - this.controller = controller; - this.configOverridesWrapper = configOverridesWrapper; - } - - @Override - public void collectMetrics(long startTime) { - if (!controller.isCollectorEnabled(configOverridesWrapper, getCollectorName())) { - return; - } - if (Objects.isNull(OpenSearchResources.INSTANCE.getClusterService()) - || Objects.isNull( - OpenSearchResources.INSTANCE - .getClusterService() - .getClusterManagerService())) { - return; - } - - if (!isClusterManagerThrottlingFeatureAvailable()) { - LOG.debug("ClusterManager Throttling Feature is not available for this domain"); - ServiceMetrics.COMMONS_STAT_METRICS_AGGREGATOR.updateStat( - StatMetrics.CLUSTER_MANAGER_THROTTLING_COLLECTOR_NOT_AVAILABLE, 1); - return; - } - - value.setLength(0); - value.append(PerformanceAnalyzerMetrics.getJsonCurrentMilliSeconds()) - .append(PerformanceAnalyzerMetrics.sMetricNewLineDelimitor); - try { - value.append( - new ClusterManagerThrottlingMetrics( - getRetryingPendingTaskCount(), - getTotalClusterManagerThrottledTaskCount()) - .serialize()); - } catch (ClassNotFoundException - | NoSuchMethodException - | InvocationTargetException - | IllegalAccessException e) { - LOG.debug( - "[ {} ] Exception raised while getting Cluster Manager throttling metrics: {} ", - this::getCollectorName, - e::getMessage); - StatsCollector.instance().logException(CLUSTER_MANAGER_THROTTLING_COLLECTOR_ERROR); - return; - } - - saveMetricValues(value.toString(), startTime); - } - - private boolean isClusterManagerThrottlingFeatureAvailable() { - try { - Class.forName(CLUSTER_MANAGER_THROTTLING_RETRY_LISTENER_PATH); - ClusterManagerService.class.getMethod(THROTTLED_PENDING_TASK_COUNT_METHOD_NAME); - } catch (ClassNotFoundException | NoSuchMethodException e) { - return false; - } - return true; - } - - private long getTotalClusterManagerThrottledTaskCount() - throws NoSuchMethodException, InvocationTargetException, IllegalAccessException { - Method method = - ClusterManagerService.class.getMethod(THROTTLED_PENDING_TASK_COUNT_METHOD_NAME); - return (long) - method.invoke( - OpenSearchResources.INSTANCE - .getClusterService() - .getClusterManagerService()); - } - - private long getRetryingPendingTaskCount() - throws ClassNotFoundException, NoSuchMethodException, InvocationTargetException, - IllegalAccessException { - Method method = - Class.forName(CLUSTER_MANAGER_THROTTLING_RETRY_LISTENER_PATH) - .getMethod(RETRYING_TASK_COUNT_METHOD_NAME); - return (long) method.invoke(null); - } - - @Override - public String getMetricsPath(long startTime, String... keysPath) { - if (keysPath.length != KEYS_PATH_LENGTH) { - throw new RuntimeException("keys length should be " + KEYS_PATH_LENGTH); - } - - return PerformanceAnalyzerMetrics.generatePath( - startTime, PerformanceAnalyzerMetrics.sClusterManagerThrottledTasksPath); - } - - public static class ClusterManagerThrottlingMetrics extends MetricStatus { - private final long retryingTaskCount; - private final long throttledPendingTasksCount; - - public ClusterManagerThrottlingMetrics( - long retryingTaskCount, long throttledPendingTasksCount) { - this.retryingTaskCount = retryingTaskCount; - this.throttledPendingTasksCount = throttledPendingTasksCount; - } - - @JsonProperty(AllMetrics.ClusterManagerThrottlingValue.Constants.RETRYING_TASK_COUNT) - public long getRetryingTaskCount() { - return retryingTaskCount; - } - - @JsonProperty( - AllMetrics.ClusterManagerThrottlingValue.Constants.THROTTLED_PENDING_TASK_COUNT) - public long getThrottledPendingTasksCount() { - return throttledPendingTasksCount; - } - } -} diff --git a/src/main/java/org/opensearch/performanceanalyzer/util/Utils.java b/src/main/java/org/opensearch/performanceanalyzer/util/Utils.java index d2c7d3c8..3c65e090 100644 --- a/src/main/java/org/opensearch/performanceanalyzer/util/Utils.java +++ b/src/main/java/org/opensearch/performanceanalyzer/util/Utils.java @@ -40,8 +40,6 @@ public static void configureMetrics() { MetricsConfiguration.CONFIG_MAP.put(ClusterManagerServiceMetrics.class, cdefault); MetricsConfiguration.CONFIG_MAP.put(FaultDetectionMetricsCollector.class, cdefault); MetricsConfiguration.CONFIG_MAP.put(ShardStateCollector.class, cdefault); - MetricsConfiguration.CONFIG_MAP.put( - ClusterManagerThrottlingMetricsCollector.class, cdefault); MetricsConfiguration.CONFIG_MAP.put(ClusterApplierServiceStatsCollector.class, cdefault); MetricsConfiguration.CONFIG_MAP.put(ElectionTermCollector.class, cdefault); MetricsConfiguration.CONFIG_MAP.put(ShardIndexingPressureMetricsCollector.class, cdefault); diff --git a/src/test/java/org/opensearch/performanceanalyzer/collectors/ClusterManagerThrottlingMetricsCollectorTests.java b/src/test/java/org/opensearch/performanceanalyzer/collectors/ClusterManagerThrottlingMetricsCollectorTests.java deleted file mode 100644 index 31f5a623..00000000 --- a/src/test/java/org/opensearch/performanceanalyzer/collectors/ClusterManagerThrottlingMetricsCollectorTests.java +++ /dev/null @@ -1,55 +0,0 @@ -/* - * Copyright OpenSearch Contributors - * SPDX-License-Identifier: Apache-2.0 - */ - -package org.opensearch.performanceanalyzer.collectors; - -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; - -import java.util.ArrayList; -import java.util.List; -import org.junit.Test; -import org.mockito.Mockito; -import org.opensearch.performanceanalyzer.CustomMetricsLocationTestBase; -import org.opensearch.performanceanalyzer.commons.config.overrides.ConfigOverridesWrapper; -import org.opensearch.performanceanalyzer.commons.event_process.Event; -import org.opensearch.performanceanalyzer.commons.metrics.MetricsConfiguration; -import org.opensearch.performanceanalyzer.commons.metrics.PerformanceAnalyzerMetrics; -import org.opensearch.performanceanalyzer.config.PerformanceAnalyzerController; - -public class ClusterManagerThrottlingMetricsCollectorTests extends CustomMetricsLocationTestBase { - - @Test - public void testClusterManagerThrottlingMetrics() { - MetricsConfiguration.CONFIG_MAP.put( - ClusterManagerThrottlingMetricsCollector.class, MetricsConfiguration.cdefault); - System.setProperty("performanceanalyzer.metrics.log.enabled", "False"); - - long startTimeInMills = 1153721339; - PerformanceAnalyzerController controller = - Mockito.mock(PerformanceAnalyzerController.class); - ConfigOverridesWrapper configOverrides = Mockito.mock(ConfigOverridesWrapper.class); - Mockito.when( - controller.isCollectorEnabled( - configOverrides, "ClusterManagerThrottlingMetricsCollector")) - .thenReturn(true); - ClusterManagerThrottlingMetricsCollector throttlingMetricsCollectorCollector = - new ClusterManagerThrottlingMetricsCollector(controller, configOverrides); - throttlingMetricsCollectorCollector.saveMetricValues("testMetric", startTimeInMills); - - List metrics = new ArrayList<>(); - PerformanceAnalyzerMetrics.metricQueue.drainTo(metrics); - assertEquals(1, metrics.size()); - assertEquals("testMetric", metrics.get(0).value); - - try { - throttlingMetricsCollectorCollector.saveMetricValues( - "throttled_pending_tasks", startTimeInMills, "123"); - assertTrue("Negative scenario test: Should have been a RuntimeException", true); - } catch (RuntimeException ex) { - // - expecting exception...1 values passed; 0 expected - } - } -}