Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue #432: Standardize the messages for all the criteria tests #453

Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
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
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() {
CherfaElyes marked this conversation as resolved.
Show resolved Hide resolved
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) {
CherfaElyes marked this conversation as resolved.
Show resolved Hide resolved
this.criterion = criterion;
return this;
}
}
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
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
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