fix: Add extra check to Junos DOM discovery #5582
Conversation
Rosiak
changed the title
|
Auto-Deploy finished, Test PR at http://5582.ci.librenms.org or https://5582.ci.librenms.org |
| @@ -16,7 +16,7 @@ | |||
| $multiplier = 1; | |||
| $divisor = 1000000; | |||
| foreach ($junos_oids as $index => $entry) { | |||
| if (is_numeric($entry['jnxDomCurrentTxLaserBiasCurrent'])) { | |||
| if (is_numeric($entry['jnxDomCurrentTxLaserBiasCurrent']) && $entry['jnxDomCurrentTxLaserBiasCurrent'] && $entry['jnxDomCurrentTxLaserBiasCurrentLowAlarmThreshold'] != 0) { | |||
There was a problem hiding this comment.
Is it not worth setting jnxDomCurrentTxLaserBiasCurrentLowAlarmThreshold to null so we can still discovery the sensor here?
There was a problem hiding this comment.
Was a bit fast, just added another != 0.
There was a problem hiding this comment.
That's not what I meant, here we are skipping the sensor_discovery because the low threshold is 0, we should ignore that and set it to null if it's 0 so thresholds are auto discovered
There was a problem hiding this comment.
No we are skipping the sensor discovery if $entry['jnxDomCurrentTxLaserBiasCurrent'] != 0 AND $entry['jnxDomCurrentTxLaserBiasCurrentLowAlarmThreshold'] != 0.
The idea by doing this is to catch invalid entry's like the ones pasted in the PR.
Forgive me if I'm not getting your point.
There was a problem hiding this comment.
Yes but why not only skip jnxDomCurrentTxLaserBiasCurrent? We can calculate the threshold so just null it if it's invalid and allow the sensor to be discovered.
|
Auto-Deploy finished, Test PR at http://5582.ci.librenms.org or https://5582.ci.librenms.org |
|
Auto-Deploy finished, Test PR at http://5582.ci.librenms.org or https://5582.ci.librenms.org |
|
The inspection completed: No new issues |
DO NOT DELETE THIS TEXT
Please note
Should mitigate adding the following: