Issue #203: Refactor-Extensions-code-to-check-protocol-efficiently

Modify checkProtocol method
sentrysoftware · Jun 4, 2024 · 974c845 · 974c845
1 parent 125162c
commit 974c845
Show file tree

Hide file tree

Showing 20 changed files with 127 additions and 112 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,9 +89,9 @@ public interface IProtocolExtension {
 	 * telemetry manager.
 	 *
 	 * @param telemetryManager The telemetry manager to use for monitoring.
-	 * @return true if the protocol check succeeds, false otherwise.
+	 * @return Optional.of(true) if the protocol check succeeds, Optional.of(false) otherwise.
 	 */
-	boolean checkProtocol(TelemetryManager telemetryManager);
+	Optional<Boolean> checkProtocol(TelemetryManager telemetryManager);
 
 	/**
 	 * Executes a source operation based on the given source and configuration within the telemetry manager.

diff --git a/...va/org/sentrysoftware/metricshub/engine/strategy/collect/ProtocolHealthCheckStrategy.java b/...va/org/sentrysoftware/metricshub/engine/strategy/collect/ProtocolHealthCheckStrategy.java
@@ -22,6 +22,7 @@
  */
 
 import java.util.List;
+import java.util.Optional;
 import lombok.Builder;
 import lombok.NonNull;
 import org.sentrysoftware.metricshub.engine.client.ClientsExecutor;
@@ -80,12 +81,21 @@ public void run() {
 		// Call the extensions to check the protocol health
 		final List<IProtocolExtension> protocolExtensions = extensionManager.findProtocolCheckExtensions(telemetryManager);
 		protocolExtensions.forEach(protocolExtension -> {
+			Optional<Boolean> protocolCheckResult = protocolExtension.checkProtocol(telemetryManager);
+
+			double metricValue;
+			if (!protocolCheckResult.isPresent()) {
+				metricValue = DOWN;
+			} else {
+				metricValue = protocolCheckResult.get() ? UP : DOWN;
+			}
+
 			// CHECKSTYLE:OFF
 			new MetricFactory()
 				.collectNumberMetric(
 					telemetryManager.getEndpointHostMonitor(),
 					"metricshub.host.up{protocol=\"" + protocolExtension.getIdentifier() + "\"}",
-					protocolExtension.checkProtocol(telemetryManager) ? UP : DOWN,
+					metricValue,
 					telemetryManager.getStrategyTime()
 				);
 			// CHECKSTYLE:OFF

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;
@@ -86,12 +87,10 @@ public Set<Class<? extends Criterion>> getSupportedCriteria() {
 	}
 
 	@Override
-	public boolean checkProtocol(TelemetryManager telemetryManager) {
+	public Optional<Boolean> checkProtocol(TelemetryManager telemetryManager) {
 		// Retrieve the hostname
 		final String hostname = telemetryManager.getHostConfiguration().getHostname();
 
-		log.info("Hostname {} - Performing protocol health check.", hostname);
-
 		// Create and set the HTTP result to null
 		String httpResult = null;
 
@@ -103,9 +102,10 @@ public boolean checkProtocol(TelemetryManager telemetryManager) {
 
 		// Stop the HTTP health check if there is not an HTTP configuration
 		if (httpConfiguration == null) {
-			return false;
+			return Optional.of(false);
 		}
 
+		log.info("Hostname {} - Performing protocol health check.", hostname);
 		log.info("Hostname {} - Checking HTTP protocol status. Sending GET request to '/'.", hostname);
 
 		// Execute HTTP test request
@@ -129,7 +129,7 @@ public boolean checkProtocol(TelemetryManager telemetryManager) {
 			);
 		}
 
-		return httpResult != null;
+		return Optional.of(httpResult != null);
 	}
 
 	@Override

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
@@ -20,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;
@@ -107,10 +108,10 @@ void testCheckHttpDownHealth() {
 			.when(httpRequestExecutorMock)
 			.executeHttp(any(HttpRequest.class), anyBoolean(), any(TelemetryManager.class));
 
-		boolean result = httpExtension.checkProtocol(telemetryManager);
+		Optional<Boolean> result = httpExtension.checkProtocol(telemetryManager);
 
 		// Assert the result
-		assertFalse(result);
+		assertFalse(result.get());
 	}
 
 	@Test
@@ -122,10 +123,10 @@ void testCheckHttpUpHealth() {
 			.when(httpRequestExecutorMock)
 			.executeHttp(any(HttpRequest.class), anyBoolean(), any(TelemetryManager.class));
 
-		boolean result = httpExtension.checkProtocol(telemetryManager);
+		Optional<Boolean> result = httpExtension.checkProtocol(telemetryManager);
 
 		// Assert the result
-		assertTrue(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;
@@ -77,12 +78,10 @@ public Set<Class<? extends Criterion>> getSupportedCriteria() {
 	}
 
 	@Override
-	public boolean checkProtocol(TelemetryManager telemetryManager) {
+	public Optional<Boolean> checkProtocol(TelemetryManager telemetryManager) {
 		// Retrieve the hostname
 		String hostname = telemetryManager.getHostConfiguration().getHostname();
 
-		log.info("Hostname {} - Performing protocol health check.", hostname);
-
 		// Create and set the IPMI result to null
 		String ipmiResult = null;
 
@@ -94,9 +93,10 @@ public boolean checkProtocol(TelemetryManager telemetryManager) {
 
 		// Stop the IPMI health check if there is not an IPMI configuration
 		if (ipmiConfiguration == null) {
-			return false;
+			return Optional.empty();
 		}
 
+		log.info("Hostname {} - Performing protocol health check.", hostname);
 		log.info(
 			"Hostname {} - Checking IPMI protocol status. Sending a IPMI 'Get Chassis Status As String Result' request.",
 			hostname
@@ -122,7 +122,7 @@ public boolean checkProtocol(TelemetryManager telemetryManager) {
 				e
 			);
 		}
-		return ipmiResult != null;
+		return Optional.of(ipmiResult != null);
 	}
 
 	@Override

diff --git a/metricshub-ipmi-extension/src/test/java/org/sentrysoftware/IpmiExtensionTest.java b/metricshub-ipmi-extension/src/test/java/org/sentrysoftware/IpmiExtensionTest.java
@@ -15,6 +15,7 @@
 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;
@@ -103,9 +104,9 @@ void testCheckIpmiUpHealth() {
 				.thenReturn(SUCCESS_RESPONSE);
 
 			// Start the IPMI Health Check strategy
-			boolean result = ipmiExtension.checkProtocol(telemetryManager);
+			Optional<Boolean> result = ipmiExtension.checkProtocol(telemetryManager);
 
-			assertTrue(result);
+			assertTrue(result.get());
 		}
 	}
 
@@ -120,9 +121,9 @@ void testCheckIpmiDownHealth() {
 				.thenReturn(null);
 
 			// Start the IPMI Health Check strategy
-			boolean result = ipmiExtension.checkProtocol(telemetryManager);
+			Optional<Boolean> result = ipmiExtension.checkProtocol(telemetryManager);
 
-			assertFalse(result);
+			assertFalse(result.get());
 		}
 	}
 

diff --git a/...n/src/main/java/org/sentrysoftware/metricshub/extension/oscommand/OsCommandExtension.java b/...n/src/main/java/org/sentrysoftware/metricshub/extension/oscommand/OsCommandExtension.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;
@@ -98,15 +99,13 @@ public Set<Class<? extends Criterion>> getSupportedCriteria() {
 	}
 
 	@Override
-	public boolean checkProtocol(TelemetryManager telemetryManager) {
+	public Optional<Boolean> checkProtocol(TelemetryManager telemetryManager) {
 		// Create and set the SSH result to null
 		Double sshResult = UP;
 
 		// Retrieve the hostname
 		String hostname = telemetryManager.getHostConfiguration().getHostname();
 
-		log.info("Hostname {} - Performing protocol health check.", hostname);
-
 		// Retrieve SSH Configuration
 		final SshConfiguration sshConfiguration = (SshConfiguration) telemetryManager
 			.getHostConfiguration()
@@ -115,9 +114,10 @@ public boolean checkProtocol(TelemetryManager telemetryManager) {
 
 		// Stop the SSH health check if there is not any SSH configuration
 		if (sshConfiguration == null || !telemetryManager.getHostProperties().isMustCheckSshStatus()) {
-			return false;
+			return Optional.empty();
 		}
 
+		log.info("Hostname {} - Performing protocol health check.", hostname);
 		log.info("Hostname {} - Checking SSH protocol status. Sending an SSH 'echo test' command.", hostname);
 
 		// Execute Local test
@@ -129,8 +129,7 @@ public boolean checkProtocol(TelemetryManager telemetryManager) {
 			sshResult = remoteSshTest(hostname, sshResult, sshConfiguration);
 		}
 
-		//Return true if sshResult is UP, else false
-		return sshResult == UP;
+		return Optional.of(sshResult == UP);
 	}
 
 	@Override

diff --git a/...c/test/java/org/sentrysoftware/metricshub/extension/oscommand/OsCommandExtensionTest.java b/...c/test/java/org/sentrysoftware/metricshub/extension/oscommand/OsCommandExtensionTest.java
@@ -23,6 +23,7 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 import java.util.Set;
 import java.util.stream.Stream;
 import org.junit.jupiter.api.Test;
@@ -322,10 +323,10 @@ void testCheckSshHealthLocally() {
 				.thenReturn(SUCCESS_RESPONSE);
 
 			// Start the SSH Health Check strategy
-			boolean result = osCommandExtension.checkProtocol(telemetryManager);
+			Optional<Boolean> result = osCommandExtension.checkProtocol(telemetryManager);
 
 			// Assert the result
-			assertTrue(result);
+			assertTrue(result.get());
 		}
 
 		try (MockedStatic<OsCommandService> staticOsCommandHelper = Mockito.mockStatic(OsCommandService.class)) {
@@ -334,10 +335,10 @@ void testCheckSshHealthLocally() {
 				.thenReturn(null);
 
 			// Start the SSH Health Check strategy
-			boolean result = osCommandExtension.checkProtocol(telemetryManager);
+			Optional<Boolean> result = osCommandExtension.checkProtocol(telemetryManager);
 
 			// Assert the result
-			assertFalse(result);
+			assertFalse(result.get());
 		}
 	}
 
@@ -359,10 +360,10 @@ void testCheckSshUpHealthRemotely() {
 				.thenReturn(SUCCESS_RESPONSE);
 
 			// Start the SSH Health Check strategy
-			boolean result = osCommandExtension.checkProtocol(telemetryManager);
+			Optional<Boolean> result = osCommandExtension.checkProtocol(telemetryManager);
 
 			// Assert the result
-			assertTrue(result);
+			assertTrue(result.get());
 		}
 
 		try (MockedStatic<OsCommandService> staticOsCommandHelper = Mockito.mockStatic(OsCommandService.class)) {
@@ -373,10 +374,10 @@ void testCheckSshUpHealthRemotely() {
 				.thenReturn(null);
 
 			// Start the SSH Health Check strategy
-			boolean result = osCommandExtension.checkProtocol(telemetryManager);
+			Optional<Boolean> result = osCommandExtension.checkProtocol(telemetryManager);
 
 			// Assert the result
-			assertFalse(result);
+			assertFalse(result.get());
 		}
 	}
 
@@ -403,10 +404,10 @@ void testCheckSshUpHealthBothLocallyAndRemotely() {
 				.thenReturn(SUCCESS_RESPONSE);
 
 			// Start the SSH Health Check strategy
-			boolean result = osCommandExtension.checkProtocol(telemetryManager);
+			Optional<Boolean> result = osCommandExtension.checkProtocol(telemetryManager);
 
 			// Assert the result
-			assertTrue(result);
+			assertTrue(result.get());
 		}
 
 		// Local commands not working
@@ -422,10 +423,10 @@ void testCheckSshUpHealthBothLocallyAndRemotely() {
 				.thenReturn(null);
 
 			// Start the SSH Health Check strategy
-			boolean result = osCommandExtension.checkProtocol(telemetryManager);
+			Optional<Boolean> result = osCommandExtension.checkProtocol(telemetryManager);
 
 			// Assert the result
-			assertFalse(result);
+			assertFalse(result.get());
 		}
 		// remote command not working
 		try (MockedStatic<OsCommandService> staticOsCommandHelper = Mockito.mockStatic(OsCommandService.class)) {
@@ -440,10 +441,10 @@ void testCheckSshUpHealthBothLocallyAndRemotely() {
 				.thenReturn(SUCCESS_RESPONSE);
 
 			// Start the SSH Health Check strategy
-			boolean result = osCommandExtension.checkProtocol(telemetryManager);
+			Optional<Boolean> result = osCommandExtension.checkProtocol(telemetryManager);
 
 			// Assert the result
-			assertFalse(result);
+			assertFalse(result.get());
 		}
 
 		// Both local and remote commands not working, but not throwing exceptions
@@ -458,10 +459,10 @@ void testCheckSshUpHealthBothLocallyAndRemotely() {
 				.when(() -> OsCommandService.runLocalCommand(anyString(), anyLong(), any()))
 				.thenReturn(null);
 
-			boolean result = osCommandExtension.checkProtocol(telemetryManager);
+			Optional<Boolean> result = osCommandExtension.checkProtocol(telemetryManager);
 
 			// Assert the result
-			assertFalse(result);
+			assertFalse(result.get());
 		}
 	}
 
@@ -476,10 +477,10 @@ void testCheckSshNoHealthWhenMustCheckFalse() {
 		telemetryManager.getHostProperties().setOsCommandExecutesRemotely(true);
 
 		// Start the SSH Health Check strategy
-		boolean result = osCommandExtension.checkProtocol(telemetryManager);
+		Optional<Boolean> result = osCommandExtension.checkProtocol(telemetryManager);
 
 		// Assert the result
-		assertFalse(result);
+		assertFalse(result.get());
 	}
 
 	@Test
@@ -493,10 +494,10 @@ void testCheckSshNoHealthWhenNoConfiguration() {
 		telemetryManager.getHostProperties().setOsCommandExecutesRemotely(true);
 
 		// Start the SSH Health Check strategy
-		boolean result = osCommandExtension.checkProtocol(telemetryManager);
+		Optional<Boolean> result = osCommandExtension.checkProtocol(telemetryManager);
 
 		// Assert the result
-		assertFalse(result);
+		assertFalse(result.get());
 	}
 
 	@Test