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 #427: Add status information to connector monitor #431

Merged

Conversation

MedMaalej
Copy link
Contributor

  • Add buildStatusInformation in AbstractStrategy
  • Set StatusInformation in the legacyTextParams attribute (map) of the connector monitor
  • Update unit tests

* Update validateConnectorDetectionCriteria to put StatusInformation in the legacyTextParameters monitor attribute map of the connector monitor
* Add unit tests assertions in DiscoveryStrategyTest, SimpleStrategyTest and CollectStrategyTest
* Update validateConnectorDetectionCriteria
* Update unit tests
…ure/issue-427-Add-statutusInformation-to-connector-monitor
@MedMaalej
Copy link
Contributor Author

This feature was tested locally using the connector WinStorageSpaces. Here is a metricshub log showing the StatusInformation field in legacyTextParams of the connector monitor.

successful

// 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" +
"SnmpGetNextCriterion(super=SnmpCriterion(super=Criterion(type=snmpGetNext, forceSerialization=false), oid=1.3.6.1.4.1.795.10.1.1.3.1.1, expectedResult=null))\n" +
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please @MedMaalej update the toString() method on DeviceTypeCriterion, HttpCriterion, ProcessCriterion, ProductRequirementsCriterion and SnmpCriterion. Please use the same pattern as on CommandLineCriterion. Thanks.

@iguitton iguitton changed the title Issue #427: Add statutus information to connector monitor Issue #427: Add status information to connector monitor Sep 30, 2024
* Override toString method in DeviceTypeCriterion, HttpCriterion, ProcessCriterion, ProductRequirementsCriterion and SnmpCriterion
* Update unit tests
// 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" +
"SnmpGetNextCriterion(super=- OID: 1.3.6.1.4.1.795.10.1.1.3.1.1)\n" +
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry we need to call super.toString() in toString() of SnmpGetNextCriterion and SnmpGetCriterion .

.map(criterionResult -> {
final String result = criterionResult.getResult();
final String message = criterionResult.getMessage();
return String.format(
Copy link
Member

@NassimBtk NassimBtk Sep 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Each extension provides the messages in a specific format. We need to normalize all the messages. It is a huge work but we should do that to get normalized messages for BMC or even for OTEL logs.
Created issue: #432

@NassimBtk
Copy link
Member

Also @MedMaalej how is the StatusInformation parameter displayed using the CLI?

…ure/issue-427-Add-statutusInformation-to-connector-monitor
* Update CLI display of text parameters
* Update unit tests
…ure/issue-427-Add-statutusInformation-to-connector-monitor
@MedMaalej MedMaalej force-pushed the feature/issue-427-Add-statutusInformation-to-connector-monitor branch from fd80d40 to 3a7de5a Compare October 7, 2024 09:37
@@ -41,6 +43,7 @@
import org.sentrysoftware.metricshub.engine.telemetry.metric.NumberMetric;

@ExtendWith(MockitoExtension.class)
@Disabled
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed disabled.

@@ -46,6 +48,7 @@
import org.sentrysoftware.metricshub.engine.telemetry.TelemetryManager;

@ExtendWith(MockitoExtension.class)
@Disabled
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove disabled.

@@ -53,6 +54,7 @@
import org.sentrysoftware.metricshub.engine.telemetry.metric.NumberMetric;

@ExtendWith(MockitoExtension.class)
@Disabled
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove disabled.

* Remove disabled annotation from strategy tests
@NassimBtk NassimBtk merged commit b3c5524 into main Oct 8, 2024
2 checks passed
@NassimBtk NassimBtk deleted the feature/issue-427-Add-statutusInformation-to-connector-monitor branch October 8, 2024 10:32
@NassimBtk NassimBtk added this to the v0.9.07 milestone Oct 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request java
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add StatusInformation as internal text parameter to the connector monitor
2 participants