Skip to content

Commit

Permalink
Issue #432: Standardize the messages for all the criteria tests
Browse files Browse the repository at this point in the history 
* Added a displayCriterionMessage method that displays the same message format for all the CriterionTestResult.
* Adapted unit tests.
* Tested on MetricsHub.
  • Loading branch information
@CherfaElyes
CherfaElyes committed Oct 17, 2024
1 parent eb62fbf commit 4931b3f
Show file tree
Hide file tree
Showing 7 changed files with 128 additions and 41 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
15 changes: 12 additions & 3 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,7 +282,7 @@ public CriterionTestResult process(@SpanAttribute("criterion.definition") Proces
final CriterionTestResult result = localOSVisitor.getCriterionTestResult();

if (result != null) {
return result;
return result.setCriterion(processCriterion);
} else {
return CriterionTestResult.error(
processCriterion,
Expand All @@ -302,7 +307,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 +318,7 @@ public CriterionTestResult process(
VersionHelper.getClassVersion()
)
)
.criterion(productRequirementsCriterion)
.build();
}

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

Expand Down
33 changes: 30 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,31 @@ 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();
}

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"
);
}

public CriterionTestResult setCriterion(final Criterion criterion) {
this.criterion = criterion;
return this;
}
}
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 4931b3f

Please sign in to comment.