From 07900ccc08c4c00ce211089430dfaedc8bd2ee12 Mon Sep 17 00:00:00 2001 From: PipoCanaja <38363551+PipoCanaja@users.noreply.github.com> Date: Thu, 22 Apr 2021 21:40:45 +0200 Subject: [PATCH] Discovery - Refactor num_oid computing (#12576) * Test the num_oid generation * Refactor num_oid discovery * cnt * Cache the value to avoid unnecessary snmptranslate * desc of function * typo * Negative caching as well * Negative caching * style * Str::before * Str::before * Str::before * Fix key, and model Devices * types * types * doc update * re-run tests * Use Cache class * Simplify caching and do it into oidToNumeric * Simplify caching and do it into oidToNumeric * style * Extend to 3 hours * and not ->cache_time * force re-run tests * only one OID for num_oid test * rerun tests * typo in MD * restore * Exemple * Exemple * light change * test * test * test * back to PR * phpstan + suggestions from Tony * fix firebrick num_oid Co-authored-by: Tony Murray --- LibreNMS/Device/YamlDiscovery.php | 58 ++++++++++++++----- doc/Developing/os/Health-Information.md | 28 +++++++-- includes/definitions/discovery/firebrick.yaml | 1 + includes/discovery/functions.inc.php | 19 ++---- misc/discovery_schema.json | 5 +- 5 files changed, 75 insertions(+), 36 deletions(-) diff --git a/LibreNMS/Device/YamlDiscovery.php b/LibreNMS/Device/YamlDiscovery.php index 0ac981761e..e4a6d2f90b 100644 --- a/LibreNMS/Device/YamlDiscovery.php +++ b/LibreNMS/Device/YamlDiscovery.php @@ -24,6 +24,7 @@ namespace LibreNMS\Device; +use Cache; use Illuminate\Support\Arr; use Illuminate\Support\Str; use LibreNMS\Exceptions\InvalidOidException; @@ -32,6 +33,8 @@ use LibreNMS\OS; class YamlDiscovery { + private static $cache_time = 1800; // 30 min, Used for oid translation cache + /** * @param OS $os * @param DiscoveryItem|string $class @@ -85,16 +88,7 @@ class YamlDiscovery // determine numeric oid automatically if not specified if (! isset($data['num_oid'])) { try { - d_echo('Info: Trying to find a numerical OID for ' . $data['value'] . '.'); - $search_mib = $device['dynamic_discovery']['mib']; - if (Str::contains($data['oid'], '::') && ! (Str::contains($data['value'], '::'))) { - // We should search this mib first - $exp_oid = explode('::', $data['oid']); - $search_mib = $exp_oid[0] . ':' . $search_mib; - } - $num_oid = static::oidToNumeric($data['value'], $device, $search_mib, $device['mib_dir']); - $data['num_oid'] = $num_oid . '.{{ $index }}'; - d_echo('Info: We found numerical oid for ' . $data['value'] . ': ' . $data['num_oid']); + $data['num_oid'] = static::computeNumericalOID($device, $data); } catch (\Exception $e) { d_echo('Error: We cannot find a numerical OID for ' . $data['value'] . '. Skipping this one...'); continue; @@ -129,6 +123,32 @@ class YamlDiscovery return $items; } + /** + * @param array $device Device we are working on + * @param array $data Array derived from YAML + * @return string + */ + public static function computeNumericalOID($device, $data) + { + d_echo('Info: Trying to find a numerical OID for ' . $data['value'] . '.'); + $search_mib = $device['dynamic_discovery']['mib']; + $mib_prefix_data_oid = Str::before($data['oid'], '::'); + if (! empty($mib_prefix_data_oid) && empty(Str::before($data['value'], '::'))) { + // We should search value in this mib first, as it is explicitely specified + $search_mib = $mib_prefix_data_oid . ':' . $search_mib; + } + + try { + $num_oid = static::oidToNumeric($data['value'], $device, $search_mib, $device['mib_dir']); + } catch (\Exception $e) { + throw $e; + } + + d_echo('Info: We found numerical oid for ' . $data['value'] . ': ' . $num_oid); + + return $num_oid . '.{{ $index }}'; + } + /** * @param string $name Name of the field in yaml * @param string $index index in the snmp table @@ -192,7 +212,7 @@ class YamlDiscovery $name = $discovery_data[$name]; } - if (isset($pre_cache[$discovery_data['oid']][$index][$name])) { + if (! is_array($discovery_data['oid']) && isset($pre_cache[$discovery_data['oid']][$index]) && isset($pre_cache[$discovery_data['oid']][$index][$name])) { return $pre_cache[$discovery_data['oid']][$index][$name]; } @@ -356,13 +376,21 @@ class YamlDiscovery if (self::oidIsNumeric($oid)) { return $oid; } - - foreach (explode(':', $mib) as $mib_name) { - if ($numeric_oid = snmp_translate($oid, $mib_name, $mibdir, null, $device)) { - break; + $key = 'YamlDiscovery:oidToNumeric:' . $mibdir . '/' . $mib . '/' . $oid; + if (Cache::has($key)) { + $numeric_oid = Cache::get($key); + } else { + foreach (explode(':', $mib) as $mib_name) { + $numeric_oid = snmp_translate($oid, $mib_name, $mibdir, null, $device); + if ($numeric_oid) { + break; + } } } + //Store the value + Cache::put($key, $numeric_oid, self::$cache_time); + if (empty($numeric_oid)) { throw new InvalidOidException("Unable to translate oid $oid"); } diff --git a/doc/Developing/os/Health-Information.md b/doc/Developing/os/Health-Information.md index 746d2c6dba..df2643f6fe 100644 --- a/doc/Developing/os/Health-Information.md +++ b/doc/Developing/os/Health-Information.md @@ -88,8 +88,9 @@ are as follows: - `oid` (required): This is the name of the table you want to do the snmp walk on. - `value` (optional): This is the key within the table that contains the value. If not provided willuse `oid` -- `num_oid` (optional): If not provided, this parameter should be computed - automatically by discovery process. This is the numerical OID that contains +- `num_oid` (required for PullRequests): If not provided, this parameter should be computed + automatically by discovery process. This parameter is still required to + submit a pull request. This is the numerical OID that contains `value`. This should usually include `{{ $index }}`. In case the index is a string, `{{ $index_string }}` can be used instead. - `divisor` (optional): This is the divisor to use against the returned `value`. @@ -163,13 +164,13 @@ the variable name. Example `{{ $ifName:2 }}` value: 1 ``` -> ``` op ``` can be any of the following operators : -> +`op` can be any of the following operators : + > =, !=, ==, !==, <=, >=, <, >, > starts, ends, contains, regex, in_array, not_starts, > not_ends, not_contains, not_regex, not_in_array, exists -> -> Example: + +Example: ```yaml skip_values: @@ -187,6 +188,21 @@ the variable name. Example `{{ $ifName:2 }}` value: false ``` +```yaml + temperature: + data: + - + oid: hwOpticalModuleInfoTable + value: hwEntityOpticalTemperature + descr: '{{ $entPhysicalName }}' + index: '{{ $index }}' + skip_values: + - + oid: hwEntityOpticalMode + op: '=' + value: '1' +``` + If you aren't able to use yaml to perform the sensor discovery, you will most likely need to use Advanced health discovery. diff --git a/includes/definitions/discovery/firebrick.yaml b/includes/definitions/discovery/firebrick.yaml index 03196e6c6b..082e182e07 100644 --- a/includes/definitions/discovery/firebrick.yaml +++ b/includes/definitions/discovery/firebrick.yaml @@ -14,6 +14,7 @@ modules: - oid: fbMonitoringMib value: fbMonReadingValue + num_oid: '.1.3.6.1.4.1.24693.100.1.1.1.4.{{ $index }}' descr: 'fbMonReadingName' index: 'fbMonReadingValue.{{ $index }}' low_limit: 1000 diff --git a/includes/discovery/functions.inc.php b/includes/discovery/functions.inc.php index de72639b9b..29d8b0813c 100644 --- a/includes/discovery/functions.inc.php +++ b/includes/discovery/functions.inc.php @@ -886,9 +886,9 @@ function discovery_process(&$valid, $device, $sensor_class, $pre_cache) $user_function = $data['user_func']; } // get the value for this sensor, check 'value' and 'oid', if state string, translate to a number - $data_name = isset($data['value']) ? $data['value'] : $data['oid']; // fallback to oid if value is not set + $data['value'] = isset($data['value']) ? $data['value'] : $data['oid']; // fallback to oid if value is not set - $snmp_value = $snmp_data[$data_name]; + $snmp_value = $snmp_data[$data['value']]; if (! is_numeric($snmp_value)) { if ($sensor_class === 'temperature') { // For temp sensors, try and detect fahrenheit values @@ -917,20 +917,11 @@ function discovery_process(&$valid, $device, $sensor_class, $pre_cache) // Check if we have a "num_oid" value. If not, we'll try to compute it from textual OIDs with snmptranslate. if (empty($data['num_oid'])) { try { - d_echo('Info: Trying to find a numerical OID for ' . $data_name . '.'); - $search_mib = $device['dynamic_discovery']['mib']; - if (Str::contains($data['oid'], '::') && ! (Str::contains($data_name, '::'))) { - // We should search this mib first - $exp_oid = explode('::', $data['oid']); - $search_mib = $exp_oid[0] . ':' . $search_mib; - } - $num_oid = YamlDiscovery::oidToNumeric($data_name, $device, $search_mib, $device['mib_dir']); - $data['num_oid'] = $num_oid . '.{{ $index }}'; - d_echo('Info: We found numerical oid for ' . $data_name . ': ' . $data['num_oid']); + $data['num_oid'] = YamlDiscovery::computeNumericalOID($device, $data); } catch (\Exception $e) { - d_echo('Error: We cannot find a numerical OID for ' . $data_name . '. Skipping this one...'); + d_echo('Error: We cannot find a numerical OID for ' . $data['value'] . '. Skipping this one...'); $skippedFromYaml = true; - // Cause we still don't have a num_oid + // Because we don't have a num_oid, we have no way to add this sensor. } } diff --git a/misc/discovery_schema.json b/misc/discovery_schema.json index 6e5558beb0..f6e440ce08 100644 --- a/misc/discovery_schema.json +++ b/misc/discovery_schema.json @@ -170,7 +170,8 @@ }, "additionalProperties": false, "required": [ - "oid" + "oid", + "num_oid" ] } } @@ -278,6 +279,7 @@ "additionalProperties": false, "required": [ "descr", + "num_oid", "oid", "states" ] @@ -444,6 +446,7 @@ "additionalProperties": false, "required": [ "descr", + "num_oid", "oid" ] }