Merge pull request #225 from sentrysoftware/feature/issue-203-refacto…

…r-extensions-code-to-check-protocol-efficiently Issue #203 : Refactor extensions code to check protocol efficiently
sentrysoftware · Jun 5, 2024 · ad4b861 · ad4b861
2 parents c2d6fe2 + 9dd2034
commit ad4b861
Show file tree

Hide file tree

Showing 21 changed files with 258 additions and 415 deletions.
diff --git a/...gine/src/main/java/org/sentrysoftware/metricshub/engine/extension/IProtocolExtension.java b/...gine/src/main/java/org/sentrysoftware/metricshub/engine/extension/IProtocolExtension.java
@@ -89,8 +89,9 @@ public interface IProtocolExtension {
 	 * telemetry manager.
 	 *
 	 * @param telemetryManager The telemetry manager to use for monitoring.
+	 * @return Optional.of(true) if the protocol check succeeds, Optional.of(false) otherwise.
 	 */
-	void checkProtocol(TelemetryManager telemetryManager);
+	Optional<Boolean> checkProtocol(TelemetryManager telemetryManager);
 
 	/**
 	 * Executes a source operation based on the given source and configuration within the telemetry manager.
@@ -141,4 +142,11 @@ public interface IProtocolExtension {
 	 */
 	IConfiguration buildConfiguration(String configurationType, JsonNode jsonNode, UnaryOperator<char[]> decrypt)
 		throws InvalidConfigurationException;
+
+	/**
+	 * Returns the identifier for protocol extension.
+	 *
+	 * @return The protocol identifier as a string.
+	 */
+	String getIdentifier();
 }
diff --git a/...va/org/sentrysoftware/metricshub/engine/strategy/collect/ProtocolHealthCheckStrategy.java b/...va/org/sentrysoftware/metricshub/engine/strategy/collect/ProtocolHealthCheckStrategy.java
@@ -24,11 +24,11 @@
 import java.util.List;
 import lombok.Builder;
 import lombok.NonNull;
-import lombok.extern.slf4j.Slf4j;
 import org.sentrysoftware.metricshub.engine.client.ClientsExecutor;
 import org.sentrysoftware.metricshub.engine.extension.ExtensionManager;
 import org.sentrysoftware.metricshub.engine.extension.IProtocolExtension;
 import org.sentrysoftware.metricshub.engine.strategy.AbstractStrategy;
+import org.sentrysoftware.metricshub.engine.telemetry.MetricFactory;
 import org.sentrysoftware.metricshub.engine.telemetry.TelemetryManager;
 
 /**
@@ -42,7 +42,6 @@
  * responding or not.
  * </p>
  */
-@Slf4j
 public class ProtocolHealthCheckStrategy extends AbstractStrategy {
 
 	/**
@@ -78,14 +77,23 @@ public ProtocolHealthCheckStrategy(
 
 	@Override
 	public void run() {
-		// Retrieve the hostname
-		final String hostname = telemetryManager.getHostConfiguration().getHostname();
-
-		log.info("Hostname {} - Performing protocol health check.", hostname);
-
 		// Call the extensions to check the protocol health
 		final List<IProtocolExtension> protocolExtensions = extensionManager.findProtocolCheckExtensions(telemetryManager);
-		protocolExtensions.forEach(protocolExtension -> protocolExtension.checkProtocol(telemetryManager));
+		// CHECKSTYLE:OFF
+		protocolExtensions.forEach(protocolExtension ->
+			protocolExtension
+				.checkProtocol(telemetryManager)
+				.ifPresent(isUp ->
+					new MetricFactory()
+						.collectNumberMetric(
+							telemetryManager.getEndpointHostMonitor(),
+							"metricshub.host.up{protocol=\"" + protocolExtension.getIdentifier() + "\"}",
+							isUp ? UP : DOWN,
+							telemetryManager.getStrategyTime()
+						)
+				)
+		);
+		// CHECKSTYLE:ON
 	}
 
 	@Override

diff --git a/...rg/sentrysoftware/metricshub/engine/strategy/collect/ProtocolHealthCheckStrategyTest.java b/...rg/sentrysoftware/metricshub/engine/strategy/collect/ProtocolHealthCheckStrategyTest.java
@@ -1,8 +1,6 @@
 package org.sentrysoftware.metricshub.engine.strategy.collect;
 
 import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
-import static org.mockito.ArgumentMatchers.any;
-import static org.mockito.Mockito.doNothing;
 import static org.mockito.Mockito.doReturn;
 import static org.sentrysoftware.metricshub.engine.common.helpers.KnownMonitorType.HOST;
 import static org.sentrysoftware.metricshub.engine.constants.Constants.HOSTNAME;
@@ -82,8 +80,6 @@ void testCheckHealth() throws Exception {
 			.when(protocolExtensionMock)
 			.isValidConfiguration(telemetryManager.getHostConfiguration().getConfigurations().get(TestConfiguration.class));
 
-		doNothing().when(protocolExtensionMock).checkProtocol(any(TelemetryManager.class));
-
 		// Create a new protocol health check strategy
 		final ProtocolHealthCheckStrategy healthCheckStrategy = new ProtocolHealthCheckStrategy(
 			telemetryManager,

diff --git a/...p-extension/src/main/java/org/sentrysoftware/metricshub/extension/http/HttpExtension.java b/...p-extension/src/main/java/org/sentrysoftware/metricshub/extension/http/HttpExtension.java
@@ -28,6 +28,7 @@
 import com.fasterxml.jackson.databind.json.JsonMapper;
 import com.fasterxml.jackson.dataformat.yaml.YAMLFactory;
 import java.util.Map;
+import java.util.Optional;
 import java.util.Set;
 import java.util.function.UnaryOperator;
 import lombok.extern.slf4j.Slf4j;
@@ -41,8 +42,6 @@
 import org.sentrysoftware.metricshub.engine.extension.IProtocolExtension;
 import org.sentrysoftware.metricshub.engine.strategy.detection.CriterionTestResult;
 import org.sentrysoftware.metricshub.engine.strategy.source.SourceTable;
-import org.sentrysoftware.metricshub.engine.telemetry.MetricFactory;
-import org.sentrysoftware.metricshub.engine.telemetry.Monitor;
 import org.sentrysoftware.metricshub.engine.telemetry.TelemetryManager;
 import org.sentrysoftware.metricshub.extension.http.utils.HttpRequest;
 
@@ -53,6 +52,11 @@
 @Slf4j
 public class HttpExtension implements IProtocolExtension {
 
+	/**
+	 * The identifier for the Http protocol.
+	 */
+	private static final String IDENTIFIER = "http";
+
 	private HttpRequestExecutor httpRequestExecutor;
 
 	/**
@@ -62,21 +66,6 @@ public HttpExtension() {
 		httpRequestExecutor = new HttpRequestExecutor();
 	}
 
-	/**
-	 * Protocol up status value '1.0'
-	 */
-	public static final Double UP = 1.0;
-
-	/**
-	 * Protocol down status value '0.0'
-	 */
-	public static final Double DOWN = 0.0;
-
-	/**
-	 * Up metric name format that will be saved by the metric factory
-	 */
-	public static final String HTTP_UP_METRIC = "metricshub.host.up{protocol=\"http\"}";
-
 	@Override
 	public boolean isValidConfiguration(IConfiguration configuration) {
 		return configuration instanceof HttpConfiguration;
@@ -98,13 +87,10 @@ public Set<Class<? extends Criterion>> getSupportedCriteria() {
 	}
 
 	@Override
-	public void checkProtocol(TelemetryManager telemetryManager) {
+	public Optional<Boolean> checkProtocol(TelemetryManager telemetryManager) {
 		// Retrieve the hostname
 		final String hostname = telemetryManager.getHostConfiguration().getHostname();
 
-		// Retrieve the host endpoint monitor
-		final Monitor hostMonitor = telemetryManager.getEndpointHostMonitor();
-
 		// Create and set the HTTP result to null
 		String httpResult = null;
 
@@ -116,9 +102,10 @@ public void checkProtocol(TelemetryManager telemetryManager) {
 
 		// Stop the HTTP health check if there is not an HTTP configuration
 		if (httpConfiguration == null) {
-			return;
+			return Optional.empty();
 		}
 
+		log.info("Hostname {} - Performing {} protocol health check.", hostname, getIdentifier());
 		log.info("Hostname {} - Checking HTTP protocol status. Sending GET request to '/'.", hostname);
 
 		// Execute HTTP test request
@@ -142,16 +129,7 @@ public void checkProtocol(TelemetryManager telemetryManager) {
 			);
 		}
 
-		// Generate a metric from the Http result
-		// CHECKSTYLE:OFF
-		new MetricFactory()
-			.collectNumberMetric(
-				hostMonitor,
-				HTTP_UP_METRIC,
-				httpResult != null ? UP : DOWN,
-				telemetryManager.getStrategyTime()
-			);
-		// CHECKSTYLE:ON
+		return Optional.of(httpResult != null);
 	}
 
 	@Override
@@ -188,7 +166,7 @@ public CriterionTestResult processCriterion(
 
 	@Override
 	public boolean isSupportedConfigurationType(String configurationType) {
-		return "http".equalsIgnoreCase(configurationType);
+		return IDENTIFIER.equalsIgnoreCase(configurationType);
 	}
 
 	@Override
@@ -233,4 +211,9 @@ public static JsonMapper newObjectMapper() {
 			.configure(DeserializationFeature.FAIL_ON_INVALID_SUBTYPE, false)
 			.build();
 	}
+
+	@Override
+	public String getIdentifier() {
+		return IDENTIFIER;
+	}
 }
diff --git a/...tension/src/test/java/org/sentrysoftware/metricshub/extension/http/HttpExtensionTest.java b/...tension/src/test/java/org/sentrysoftware/metricshub/extension/http/HttpExtensionTest.java
@@ -11,7 +11,6 @@
 import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.doThrow;
 import static org.sentrysoftware.metricshub.engine.common.helpers.KnownMonitorType.HOST;
-import static org.sentrysoftware.metricshub.extension.http.HttpExtension.HTTP_UP_METRIC;
 
 import com.fasterxml.jackson.databind.node.BooleanNode;
 import com.fasterxml.jackson.databind.node.IntNode;
@@ -21,6 +20,7 @@
 import java.io.IOException;
 import java.util.HashMap;
 import java.util.Map;
+import java.util.Optional;
 import java.util.UUID;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
@@ -108,9 +108,10 @@ void testCheckHttpDownHealth() {
 			.when(httpRequestExecutorMock)
 			.executeHttp(any(HttpRequest.class), anyBoolean(), any(TelemetryManager.class));
 
-		httpExtension.checkProtocol(telemetryManager);
+		Optional<Boolean> result = httpExtension.checkProtocol(telemetryManager);
 
-		assertEquals(HttpExtension.DOWN, telemetryManager.getEndpointHostMonitor().getMetric(HTTP_UP_METRIC).getValue());
+		// Assert the result
+		assertFalse(result.get());
 	}
 
 	@Test
@@ -122,9 +123,10 @@ void testCheckHttpUpHealth() {
 			.when(httpRequestExecutorMock)
 			.executeHttp(any(HttpRequest.class), anyBoolean(), any(TelemetryManager.class));
 
-		httpExtension.checkProtocol(telemetryManager);
+		Optional<Boolean> result = httpExtension.checkProtocol(telemetryManager);
 
-		assertEquals(HttpExtension.UP, telemetryManager.getEndpointHostMonitor().getMetric(HTTP_UP_METRIC).getValue());
+		// Assert the result
+		assertTrue(result.get());
 	}
 
 	@Test

diff --git a/...i-extension/src/main/java/org/sentrysoftware/metricshub/extension/ipmi/IpmiExtension.java b/...i-extension/src/main/java/org/sentrysoftware/metricshub/extension/ipmi/IpmiExtension.java
@@ -28,6 +28,7 @@
 import com.fasterxml.jackson.databind.json.JsonMapper;
 import com.fasterxml.jackson.dataformat.yaml.YAMLFactory;
 import java.util.Map;
+import java.util.Optional;
 import java.util.Set;
 import java.util.function.UnaryOperator;
 import lombok.extern.slf4j.Slf4j;
@@ -44,29 +45,17 @@
 import org.sentrysoftware.metricshub.engine.extension.IProtocolExtension;
 import org.sentrysoftware.metricshub.engine.strategy.detection.CriterionTestResult;
 import org.sentrysoftware.metricshub.engine.strategy.source.SourceTable;
-import org.sentrysoftware.metricshub.engine.telemetry.MetricFactory;
-import org.sentrysoftware.metricshub.engine.telemetry.Monitor;
 import org.sentrysoftware.metricshub.engine.telemetry.TelemetryManager;
 
 @Slf4j
 public class IpmiExtension implements IProtocolExtension {
 
-	private IpmiRequestExecutor ipmiRequestExecutor = new IpmiRequestExecutor();
-
 	/**
-	 * Protocol up status value '1.0'
+	 * The identifier for the Ipmi protocol.
 	 */
-	public static final Double UP = 1.0;
+	private static final String IDENTIFIER = "ipmi";
 
-	/**
-	 * Protocol down status value '0.0'
-	 */
-	public static final Double DOWN = 0.0;
-
-	/**
-	 * IPMI Up metric
-	 */
-	public static final String IPMI_UP_METRIC = "metricshub.host.up{protocol=\"ipmi\"}";
+	private IpmiRequestExecutor ipmiRequestExecutor = new IpmiRequestExecutor();
 
 	@Override
 	public boolean isValidConfiguration(IConfiguration configuration) {
@@ -89,13 +78,10 @@ public Set<Class<? extends Criterion>> getSupportedCriteria() {
 	}
 
 	@Override
-	public void checkProtocol(TelemetryManager telemetryManager) {
+	public Optional<Boolean> checkProtocol(TelemetryManager telemetryManager) {
 		// Retrieve the hostname
 		String hostname = telemetryManager.getHostConfiguration().getHostname();
 
-		// Retrieve the host endpoint monitor
-		final Monitor hostMonitor = telemetryManager.getEndpointHostMonitor();
-
 		// Create and set the IPMI result to null
 		String ipmiResult = null;
 
@@ -107,9 +93,10 @@ public void checkProtocol(TelemetryManager telemetryManager) {
 
 		// Stop the IPMI health check if there is not an IPMI configuration
 		if (ipmiConfiguration == null) {
-			return;
+			return Optional.empty();
 		}
 
+		log.info("Hostname {} - Performing {} protocol health check.", hostname, getIdentifier());
 		log.info(
 			"Hostname {} - Checking IPMI protocol status. Sending a IPMI 'Get Chassis Status As String Result' request.",
 			hostname
@@ -135,17 +122,7 @@ public void checkProtocol(TelemetryManager telemetryManager) {
 				e
 			);
 		}
-
-		// Generate a metric from the IPMI result
-		// CHECKSTYLE:OFF
-		new MetricFactory()
-			.collectNumberMetric(
-				hostMonitor,
-				IPMI_UP_METRIC,
-				ipmiResult != null ? UP : DOWN,
-				telemetryManager.getStrategyTime()
-			);
-		// CHECKSTYLE:ON
+		return Optional.of(ipmiResult != null);
 	}
 
 	@Override
@@ -226,7 +203,7 @@ public CriterionTestResult processCriterion(
 
 	@Override
 	public boolean isSupportedConfigurationType(String configurationType) {
-		return "ipmi".equalsIgnoreCase(configurationType);
+		return IDENTIFIER.equalsIgnoreCase(configurationType);
 	}
 
 	@Override
@@ -271,4 +248,9 @@ public static JsonMapper newObjectMapper() {
 			.configure(DeserializationFeature.FAIL_ON_INVALID_SUBTYPE, false)
 			.build();
 	}
+
+	@Override
+	public String getIdentifier() {
+		return IDENTIFIER;
+	}
 }
diff --git a/metricshub-ipmi-extension/src/test/java/org/sentrysoftware/IpmiExtensionTest.java b/metricshub-ipmi-extension/src/test/java/org/sentrysoftware/IpmiExtensionTest.java
@@ -8,16 +8,14 @@
 import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.doThrow;
 import static org.sentrysoftware.metricshub.engine.common.helpers.KnownMonitorType.HOST;
-import static org.sentrysoftware.metricshub.engine.strategy.collect.ProtocolHealthCheckStrategy.DOWN;
-import static org.sentrysoftware.metricshub.engine.strategy.collect.ProtocolHealthCheckStrategy.UP;
-import static org.sentrysoftware.metricshub.extension.ipmi.IpmiExtension.IPMI_UP_METRIC;
 
 import com.fasterxml.jackson.databind.node.BooleanNode;
 import com.fasterxml.jackson.databind.node.JsonNodeFactory;
 import com.fasterxml.jackson.databind.node.ObjectNode;
 import com.fasterxml.jackson.databind.node.TextNode;
 import java.util.HashMap;
 import java.util.Map;
+import java.util.Optional;
 import java.util.UUID;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.TimeoutException;
@@ -106,9 +104,9 @@ void testCheckIpmiUpHealth() {
 				.thenReturn(SUCCESS_RESPONSE);
 
 			// Start the IPMI Health Check strategy
-			ipmiExtension.checkProtocol(telemetryManager);
+			Optional<Boolean> result = ipmiExtension.checkProtocol(telemetryManager);
 
-			assertEquals(UP, telemetryManager.getEndpointHostMonitor().getMetric(IPMI_UP_METRIC).getValue());
+			assertTrue(result.get());
 		}
 	}
 
@@ -123,9 +121,9 @@ void testCheckIpmiDownHealth() {
 				.thenReturn(null);
 
 			// Start the IPMI Health Check strategy
-			ipmiExtension.checkProtocol(telemetryManager);
+			Optional<Boolean> result = ipmiExtension.checkProtocol(telemetryManager);
 
-			assertEquals(DOWN, telemetryManager.getEndpointHostMonitor().getMetric(IPMI_UP_METRIC).getValue());
+			assertFalse(result.get());
 		}
 	}