Add additional F5 sensor support #4642

Merged
merged 24 commits into from Oct 13, 2016

Projects

None yet

6 participants

@boudreau
Contributor
boudreau commented Sep 27, 2016 edited

Please note

Please read this information carefully. You can run ./scripts/pre-commit.php to check your code before submitting.

boudreau added some commits Sep 26, 2016
@boudreau boudreau added recent F5 mibs ver 11.6 5352553
@boudreau boudreau F5 temp discovery module done 0fad272
@boudreau boudreau changed desc of sensors in DB c8d29a9
@boudreau boudreau added fanspeed and Power states 975adf9
@boudreau boudreau Added other sensors states
79388cf
@murrant

Please check the comments I made in all places.

I just made this PR, should be an ok example for keeping things tidy.
#4640

+ }
+ }
+
+ foreach (array_keys($temp) as $index) {
@murrant
murrant Sep 27, 2016 Contributor

You can do foreach($temp as $index => $data) here.

+
+ foreach (array_keys($temp) as $index) {
+ $descr = "sysChassisPowerSupplyStatus.".$temp[$index]['sysChassisPowerSupplyIndex'];
+ $current = $temp[$index]['sysChassisPowerSupplyStatus'];
@murrant
murrant Sep 27, 2016 Contributor

and $current = $data['sysChassisPowerSupplyStatus']; here

+ $current = $temp[$index]['sysChassisPowerSupplyStatus'];
+ $sensorType = 'f5';
+ $oid = '1.3.6.1.4.1.3375.2.1.3.2.2.2.1.2.'.$index;
+ $oid_status = snmp_get($device, $oid, '-Oqv');
@murrant
murrant Sep 27, 2016 Contributor

Remove this line, it is unused and causes many extra snmp_get() calls

@laf laf changed the title from Issue 4599 to Add additional F5 sensor support Sep 27, 2016
@laf
Member
laf commented Sep 27, 2016

Also lots of formatting issues, please see our docs about running pre-commit checks.

@laf laf added Conflict Blocker and removed Conflict labels Sep 27, 2016
@boudreau boudreau Update f5.inc.php
2651669
boudreau added some commits Sep 28, 2016
@boudreau boudreau modified for CS Style and recommendation
61394b4
@boudreau boudreau Update defaults.inc.php
9d87695
includes/defaults.inc.php
@@ -703,7 +703,9 @@
$config['poller_modules']['cisco-mac-accounting'] = 1;
$config['poller_modules']['cipsec-tunnels'] = 1;
$config['poller_modules']['cisco-ace-loadbalancer'] = 1;
+$config['poller_modules']['F5-loadbalancer'] = 1;
@laf
laf Sep 28, 2016 Member

Not sure you've added these in. They aren't poller modules.

includes/defaults.inc.php
$config['poller_modules']['cisco-ace-serverfarms'] = 1;
+$config['poller_modules']['F5-serverfarms'] = 1;
@laf
laf Sep 28, 2016 Member

Same goes for this.

+ $split_oid = explode('.', $oid);
+ $index = $split_oid[(count($split_oid) - 1)];
+ $descr = 'Fan Speed '.$index;
+ $oid = '1.3.6.1.4.1.3375.2.1.3.2.1.2.1.3.'.$index;
@laf
laf Sep 28, 2016 Member

This should start with a . - .1.3.6

+ $descr = "sysChassisPowerSupplyStatus.".$temp[$index]['sysChassisPowerSupplyIndex'];
+ $current = $data['sysChassisPowerSupplyStatus'];
+ $sensorType = 'f5';
+ $oid = '1.3.6.1.4.1.3375.2.1.3.2.2.2.1.2.'.$index;
@laf
laf Sep 28, 2016 Member

This should start with a . - .1.3.6

+ $descr = "sysCmFailoverStatusId.".$temp1[$index]['sysCmFailoverStatusId'];
+ $current = $temp1[$index]['sysCmFailoverStatusId'];
+ $sensorType = 'f5';
+ $oid = '1.3.6.1.4.1.3375.2.1.14.3.1.'.$index;
@laf
laf Sep 28, 2016 Member

This should start with a . - .1.3.6

+ $descr = "sysCmFailoverStatusColor.".$index;
+ $current = $temp1[$index]['sysCmFailoverStatusColor'];
+ $sensorType = 'f5';
+ $oid = '1.3.6.1.4.1.3375.2.1.14.3.3.'.$index;
@laf
laf Sep 28, 2016 Member

This should start with a . - .1.3.6

+ $descr = "Fan Speed Status ".$index;
+ $current = $temp3[$index]['sysChassisFanStatus'];
+ $sensorType = 'f5';
+ $oid = '1.3.6.1.4.1.3375.2.1.3.2.1.2.1.2.'.$index;
@laf
laf Sep 28, 2016 Member

This should start with a . - .1.3.6

boudreau added some commits Sep 29, 2016
@boudreau boudreau Update f5.inc.php
4d80d8f
@boudreau boudreau Update f5.inc.php
86b74ca
@boudreau boudreau missing '.' at start of the OIDs
a409ae2
@boudreau
Contributor
boudreau commented Sep 29, 2016 edited

Hi,

Sorry, got mixed up with another project, no changes should be applied on the includes/defaults.inc.phphttps://github.com/librenms/librenms/pull/4642#pullrequestreview-2036461: file.

I added the missing ‘.’ on the OIDs.

Thanks

Yves

@Rosiak
Contributor
Rosiak commented Oct 1, 2016

Sorry to bother again, but would it be possible to clean up the state sensor code?
You should be able to use includes/discovery/sensors/states/cisco.inc.php as an example.

@laf laf removed the Blocker label Oct 3, 2016
+ $index = $split_oid[(count($split_oid) - 1)];
+ $descr = 'Fan Speed '.$index;
+ $oid = '.1.3.6.1.4.1.3375.2.1.3.2.1.2.1.3.'.$index;
+ $fanspeed = (snmp_get($device, $oid, '-Oqv') / $divisor);
@laf
laf Oct 3, 2016 Member

Haven't we got fanspeed from the sysChassisFanSpeed walk?

@boudreau
boudreau Oct 11, 2016 Contributor

Not sure, I'm following?
I did the same code structure as in dell.inc.php in includes/discovery/sensors/fanspeeds.
It also looks the same in the lmsensors.inc.php. They snmp_walk and then it snmp_get ?

+ $descr = "sysCmFailoverStatusId.".$temp1[$index]['sysCmFailoverStatusId'];
+ $current = $temp1[$index]['sysCmFailoverStatusId'];
+ $sensorType = 'f5';
+ $oid = '.1.3.6.1.4.1.3375.2.1.14.3.1.'.$index;
@laf
laf Oct 3, 2016 Member

Again don't we have this value from the walk already?

+ $current = $temp1[$index]['sysCmFailoverStatusColor'];
+ $sensorType = 'f5';
+ $oid = '.1.3.6.1.4.1.3375.2.1.14.3.3.'.$index;
+ $oid_status = snmp_get($device, $oid, '-Oqev');
@laf
laf Oct 3, 2016 Member

Don't we have that from the snmpwalk of sysCmFailoverStatusColor

+ $sensorType = 'f5';
+ $oid = '.1.3.6.1.4.1.3375.2.1.3.2.1.2.1.2.'.$index;
+ $oid_status = snmp_get($device, $oid, '-Oqv');
+ discover_sensor($valid['sensor'], 'state', $device, $oid, $index, $state_name, $descr, '1', '1', null, null, null, null, $current, 'snmp', $index);
@laf
laf Oct 3, 2016 Member

Don't we have this value already from the snmpwalk?

- $entphydata = dbFetchRows("SELECT `entPhysicalIndex`, `entPhysicalClass`, `entPhysicalName` FROM `entPhysical` WHERE `device_id` = ? AND `entPhysicalClass` REGEXP 'module|sensor' ORDER BY `entPhysicalIndex`", array(
- $device['device_id']
- ));
+ $entphydata = dbFetchRows(
@laf
laf Oct 3, 2016 Member

The formatting of this seems unnecessary.

@@ -36,10 +38,12 @@
foreach ($comware_oids as $index => $entry) {
if (is_numeric($entry['hh3cTransceiverTemperature']) && $entry['hh3cTransceiverTemperature'] != 2147483647) {
$oid = '.1.3.6.1.4.1.25506.2.70.1.1.1.15.' . $index;
- $dbquery = dbFetchRows("SELECT `ifDescr` FROM `ports` WHERE `ifIndex`= ? AND `device_id` = ? AND `ifAdminStatus` = 'up'", array(
- $index,
+ $dbquery = dbFetchRows(
@laf
laf Oct 3, 2016 Member

The formatting of this seems unnecessary.

+<?php
+
+if ($device['os'] == 'f5') {
+ $f5 = null;
@laf
laf Oct 3, 2016 Member

should be $f5 = array()``or better yet$f5_chassis = array();`

+ $current = $f5[$index]['sysChassisTempTemperature'];
+ $sensorType = 'f5';
+ $oid = '.1.3.6.1.4.1.3375.2.1.3.2.3.2.1.2.'.$index;
+ $low_limit = null;
@laf
laf Oct 3, 2016 Member

This seems unnecessary to set all these variables when they are only ever null, just pass null to the function.

+ }
+
+ // Get the CPU Temperature values
+ $f5cpu = null;
@laf
laf Oct 3, 2016 Member

Should be $f5_cpu = array();

+ // Get the CPU Temperature values
+ $f5cpu = null;
+ // $f5cpu = snmpwalk_cache_oid($device, 'sysCpuSensorTemperature', array(), 'F5-BIGIP-SYSTEM-MIB');
+ $f5cpu = snmpwalk_cache_multi_oid($device, 'sysCpuSensorTemperature', array(), 'F5-BIGIP-SYSTEM-MIB');
@laf
laf Oct 3, 2016 Member

Seems more appropriate to walk sysCpuSensorEntry so you don't need the other two snmp_get()'s

@laf
Member
laf commented Oct 3, 2016

I've put some comments in this. Please also remove the .txt extension from all MIBs.

@laf laf added the Blocker label Oct 3, 2016
@laf
Member
laf commented Oct 11, 2016

bump @boudreau

@scrutinizer-notifier

The inspection completed: 10 new issues

@laf laf added New-Device and removed Blocker labels Oct 13, 2016
@laf
laf approved these changes Oct 13, 2016 View changes
@laf laf merged commit 5669dce into librenms:master Oct 13, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@boudreau boudreau deleted the boudreau:issue-4599 branch Oct 19, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment