Skip to content

Commit

Permalink
Merge pull request #453 from sentrysoftware/feature/issue-432-standar…
Browse files Browse the repository at this point in the history 
…dize-the-messages-for-all-the-criteria-tests

Issue #432: Standardize the messages for all the criteria tests
  • Loading branch information
@NassimBtk
NassimBtk authored Oct 22, 2024
2 parents 6aac57e + 8379943 commit 1bf13cc
Show file tree
Hide file tree
Showing 7 changed files with 132 additions and 40 deletions.
20 changes: 4 additions & 16 deletions ...-engine/src/main/java/org/sentrysoftware/metricshub/engine/strategy/AbstractStrategy.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
import org.sentrysoftware.metricshub.engine.extension.ExtensionManager;
import org.sentrysoftware.metricshub.engine.strategy.detection.ConnectorSelection;
import org.sentrysoftware.metricshub.engine.strategy.detection.ConnectorTestResult;
import org.sentrysoftware.metricshub.engine.strategy.detection.CriterionTestResult;
import org.sentrysoftware.metricshub.engine.strategy.source.ISourceProcessor;
import org.sentrysoftware.metricshub.engine.strategy.source.SourceProcessor;
import org.sentrysoftware.metricshub.engine.strategy.source.SourceTable;
Expand Down Expand Up @@ -441,31 +442,18 @@ protected boolean validateConnectorDetectionCriteria(final Connector currentConn
*/
protected String buildStatusInformation(final String hostname, final ConnectorTestResult testResult) {
final StringBuilder value = new StringBuilder();

final String builtTestResult = testResult
.getCriterionTestResults()
.stream()
.filter(criterionTestResult ->
!(criterionTestResult.getResult() == null && criterionTestResult.getMessage() == null)
)
.map(criterionResult -> {
final String result = criterionResult.getResult();
final String message = criterionResult.getMessage();
return String.format(
"Received Result: %s. %s",
result != null ? result : "N/A",
message != null ? message : "N/A"
);
})
.map(CriterionTestResult::displayCriterionMessage)
.collect(Collectors.joining("\n"));
value
.append(builtTestResult)
.append("\nConclusion: ")
.append("Test on ")
.append(hostname)
.append(" ")
.append(testResult.isSuccess() ? "SUCCEEDED" : "FAILED");

.append("Conclusion:\n")
.append(String.format("Test on %s %s", hostname, testResult.isSuccess() ? "SUCCEEDED" : "FAILED"));
return value.toString();
}

Expand Down
17 changes: 15 additions & 2 deletions ...main/java/org/sentrysoftware/metricshub/engine/strategy/detection/CriterionProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ public CriterionTestResult process(@SpanAttribute("criterion.definition") Device
.message("Failed OS detection operation")
.result(CONFIGURE_OS_TYPE_MESSAGE + deviceKind.name())
.success(false)
.criterion(deviceTypeCriterion)
.build();
}

Expand All @@ -134,6 +135,7 @@ public CriterionTestResult process(@SpanAttribute("criterion.definition") Device
.message(SUCCESSFUL_OS_DETECTION_MESSAGE)
.result(CONFIGURE_OS_TYPE_MESSAGE + deviceKind.name())
.success(true)
.criterion(deviceTypeCriterion)
.build();
}

Expand Down Expand Up @@ -243,6 +245,7 @@ public CriterionTestResult process(@SpanAttribute("criterion.definition") Proces
.success(true)
.message("Process presence check: No test will be performed.")
.result(null)
.criterion(processCriterion)
.build();
}

Expand All @@ -253,6 +256,7 @@ public CriterionTestResult process(@SpanAttribute("criterion.definition") Proces
.success(true)
.message("Process presence check: No test will be performed remotely.")
.result(null)
.criterion(processCriterion)
.build();
}

Expand All @@ -264,6 +268,7 @@ public CriterionTestResult process(@SpanAttribute("criterion.definition") Proces
.success(true)
.message("Process presence check: OS unknown, no test will be performed.")
.result(null)
.criterion(processCriterion)
.build();
}

Expand All @@ -277,6 +282,7 @@ public CriterionTestResult process(@SpanAttribute("criterion.definition") Proces
final CriterionTestResult result = localOSVisitor.getCriterionTestResult();

if (result != null) {
result.setCriterion(processCriterion);
return result;
} else {
return CriterionTestResult.error(
Expand All @@ -302,7 +308,7 @@ public CriterionTestResult process(
productRequirementsCriterion.getEngineVersion() == null ||
productRequirementsCriterion.getEngineVersion().isBlank()
) {
return CriterionTestResult.builder().success(true).build();
return CriterionTestResult.builder().success(true).criterion(productRequirementsCriterion).build();
}

return CriterionTestResult
Expand All @@ -313,6 +319,7 @@ public CriterionTestResult process(
VersionHelper.getClassVersion()
)
)
.criterion(productRequirementsCriterion)
.build();
}

Expand Down Expand Up @@ -355,7 +362,13 @@ public CriterionTestResult processCriterionThroughExtension(
telemetryManager
);
return maybeExtension
.map(extension -> extension.processCriterion(criterion, connectorId, telemetryManager))
.map(extension -> {
CriterionTestResult result = extension.processCriterion(criterion, connectorId, telemetryManager);
if (result != null) {
result.setCriterion(criterion);
}
return result;
})
.orElseGet(CriterionTestResult::empty);
}

Expand Down
34 changes: 31 additions & 3 deletions ...ain/java/org/sentrysoftware/metricshub/engine/strategy/detection/CriterionTestResult.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ public class CriterionTestResult {

private Throwable exception;

private Criterion criterion;

/**
* Creates an empty criterion test result.
*
Expand All @@ -68,7 +70,7 @@ public static CriterionTestResult failure(final Criterion criterion, final Strin
criterion.toString(),
result
);
return CriterionTestResult.builder().success(false).message(message).result(result).build();
return CriterionTestResult.builder().success(false).message(message).result(result).criterion(criterion).build();
}

/**
Expand All @@ -93,7 +95,7 @@ public static CriterionTestResult error(Criterion criterion, String reason, Thro
reason
);
}
return CriterionTestResult.builder().success(false).message(message).exception(t).build();
return CriterionTestResult.builder().success(false).message(message).exception(t).criterion(criterion).build();
}

/**
Expand Down Expand Up @@ -148,6 +150,32 @@ public static CriterionTestResult success(final Criterion criterion, final Strin
result
);

return CriterionTestResult.builder().success(true).message(message).result(result).build();
return CriterionTestResult.builder().success(true).message(message).result(result).criterion(criterion).build();
}

/**
* Formats and displays the criterion message in a structured, readable format.
* The output includes information about the executed criterion, its details,
* the result, and an additional message, each presented in distinct sections.
* @return a formatted string that displays the criterion name, details, result, and message.
*/
public String displayCriterionMessage() {
return String.format(
"Executed %s Criterion:\n" +
"%s\n" +
"\n" +
"Result:\n" +
"%s\n" +
"\n" +
"Message:\n" +
"====================================\n" +
"%s\n" +
"====================================\n" +
"\n",
criterion.getClass().getSimpleName(),
criterion.toString(),
result != null ? result : "N/A",
message != null ? message : "N/A"
);
}
}
31 changes: 26 additions & 5 deletions .../test/java/org/sentrysoftware/metricshub/engine/strategy/collect/CollectStrategyTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
import java.util.List;
import java.util.Map;
import java.util.Set;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.Mock;
Expand Down Expand Up @@ -212,11 +211,22 @@ void testRun() throws Exception {

// Check that StatusInformation is collected on the connector monitor
assertEquals(
"Received Result: 1.3.6.1.4.1.795.10.1.1.3.1.1.0\tASN_OCTET_STR\tTest. SnmpGetNextCriterion test succeeded:\n" +
"Executed SnmpGetNextCriterion Criterion:\n" +
"- OID: 1.3.6.1.4.1.795.10.1.1.3.1.1\n" +
"\n" +
"Result:\n" +
"1.3.6.1.4.1.795.10.1.1.3.1.1.0\tASN_OCTET_STR\tTest\n" +
"\n" +
"Message:\n" +
"====================================\n" +
"SnmpGetNextCriterion test succeeded:\n" +
"- OID: 1.3.6.1.4.1.795.10.1.1.3.1.1\n" +
"\n" +
"Result: 1.3.6.1.4.1.795.10.1.1.3.1.1.0\tASN_OCTET_STR\tTest\n" +
"Conclusion: Test on host.name SUCCEEDED",
"====================================\n" +
"\n" +
"Conclusion:\n" +
"Test on host.name SUCCEEDED",
connectorMonitor.getLegacyTextParameters().get(STATUS_INFORMATION)
);

Expand All @@ -230,12 +240,23 @@ void testRun() throws Exception {

// Check that StatusInformation is collected on the connector monitor (criterion processing failure case)
assertEquals(
"Received Result: 1.3.6.1.4.1.795.10.1.1.3.1.1.0\tASN_OCTET_STR\tTest. SnmpGetNextCriterion test ran but failed:\n" +
"Executed SnmpGetNextCriterion Criterion:\n" +
"- OID: 1.3.6.1.4.1.795.10.1.1.3.1.1\n" +
"\n" +
"Result:\n" +
"1.3.6.1.4.1.795.10.1.1.3.1.1.0\tASN_OCTET_STR\tTest\n" +
"\n" +
"Message:\n" +
"====================================\n" +
"SnmpGetNextCriterion test ran but failed:\n" +
"- OID: 1.3.6.1.4.1.795.10.1.1.3.1.1\n" +
"\n" +
"Actual result:\n" +
"1.3.6.1.4.1.795.10.1.1.3.1.1.0\tASN_OCTET_STR\tTest\n" +
"Conclusion: Test on host.name FAILED",
"====================================\n" +
"\n" +
"Conclusion:\n" +
"Test on host.name FAILED",
connectorMonitor.getLegacyTextParameters().get(STATUS_INFORMATION)
);
}
Expand Down
6 changes: 4 additions & 2 deletions .../java/org/sentrysoftware/metricshub/engine/strategy/detection/CriterionProcessorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -614,24 +614,26 @@ void testProcessDeviceTypeCriterion() {
.build();
doReturn(telemetryManager.getHostConfiguration()).when(telemetryManagerMock).getHostConfiguration();

final DeviceTypeCriterion deviceTypeCriterion = DeviceTypeCriterion.builder().build();
// Init CriterionTestResult success and failure instances
final CriterionTestResult successfulTestResult = CriterionTestResult
.builder()
.message(SUCCESSFUL_OS_DETECTION)
.result(CONFIGURED_OS_NT_MESSAGE)
.success(true)
.criterion(deviceTypeCriterion)
.build();

final CriterionTestResult failedTestResult = CriterionTestResult
.builder()
.message(FAILED_OS_DETECTION)
.result(CONFIGURED_OS_NT_MESSAGE)
.success(false)
.criterion(deviceTypeCriterion)
.build();

// Test configured NETWORK OS

final DeviceTypeCriterion deviceTypeCriterion = DeviceTypeCriterion.builder().build();
assertEquals(successfulTestResult, criterionProcessor.process(deviceTypeCriterion));

// Include NETWORK OS
Expand Down Expand Up @@ -824,6 +826,7 @@ void testProcessIPMIOutOfBand() throws Exception {
.result(SYSTEM_POWER_UP_MESSAGE)
.message(IPMI_CONNECTION_SUCCESS_WITH_IMPI_OVER_LAN_MESSAGE)
.success(true)
.criterion(ipmiCriterion)
.build(),
criterionProcessor.process(new IpmiCriterion())
);
Expand Down Expand Up @@ -856,7 +859,6 @@ void testProcessIPMIOutOfBandNullResult() throws Exception {
MY_CONNECTOR_1_NAME,
extensionManager
);

assertEquals(CriterionTestResult.empty(), criterionProcessor.process(new IpmiCriterion()));
}

Expand Down
33 changes: 26 additions & 7 deletions ...t/java/org/sentrysoftware/metricshub/engine/strategy/discovery/DiscoveryStrategyTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.sentrysoftware.metricshub.engine.common.helpers.KnownMonitorType.CONNECTOR;
import static org.sentrysoftware.metricshub.engine.common.helpers.KnownMonitorType.DISK_CONTROLLER;
import static org.sentrysoftware.metricshub.engine.common.helpers.KnownMonitorType.HOST;
Expand All @@ -26,7 +24,6 @@
import java.util.List;
import java.util.Map;
import java.util.Set;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.Mock;
Expand Down Expand Up @@ -216,11 +213,22 @@ void testRun() throws Exception {

// Check that StatusInformation is collected on the connector monitor (criterion processing success case)
assertEquals(
"Received Result: 1.3.6.1.4.1.795.10.1.1.3.1.1.0\tASN_OCTET_STR\tTest. SnmpGetNextCriterion test succeeded:\n" +
"Executed SnmpGetNextCriterion Criterion:\n" +
"- OID: 1.3.6.1.4.1.795.10.1.1.3.1.1\n" +
"\n" +
"Result:\n" +
"1.3.6.1.4.1.795.10.1.1.3.1.1.0\tASN_OCTET_STR\tTest\n" +
"\n" +
"Message:\n" +
"====================================\n" +
"SnmpGetNextCriterion test succeeded:\n" +
"- OID: 1.3.6.1.4.1.795.10.1.1.3.1.1\n" +
"\n" +
"Result: 1.3.6.1.4.1.795.10.1.1.3.1.1.0\tASN_OCTET_STR\tTest\n" +
"Conclusion: Test on host.name SUCCEEDED",
"====================================\n" +
"\n" +
"Conclusion:\n" +
"Test on host.name SUCCEEDED",
connectorMonitor.getLegacyTextParameters().get(STATUS_INFORMATION)
);

Expand All @@ -234,12 +242,23 @@ void testRun() throws Exception {

// Check that StatusInformation is collected on the connector monitor (criterion processing failure case)
assertEquals(
"Received Result: 1.3.6.1.4.1.795.10.1.1.3.1.1.0\tASN_OCTET_STR\tTest. SnmpGetNextCriterion test ran but failed:\n" +
"Executed SnmpGetNextCriterion Criterion:\n" +
"- OID: 1.3.6.1.4.1.795.10.1.1.3.1.1\n" +
"\n" +
"Result:\n" +
"1.3.6.1.4.1.795.10.1.1.3.1.1.0\tASN_OCTET_STR\tTest\n" +
"\n" +
"Message:\n" +
"====================================\n" +
"SnmpGetNextCriterion test ran but failed:\n" +
"- OID: 1.3.6.1.4.1.795.10.1.1.3.1.1\n" +
"\n" +
"Actual result:\n" +
"1.3.6.1.4.1.795.10.1.1.3.1.1.0\tASN_OCTET_STR\tTest\n" +
"Conclusion: Test on host.name FAILED",
"====================================\n" +
"\n" +
"Conclusion:\n" +
"Test on host.name FAILED",
connectorMonitor.getLegacyTextParameters().get(STATUS_INFORMATION)
);
}
Expand Down
Loading

0 comments on commit 1bf13cc

Please sign in to comment.