From 7c542d70afb781f6e5a7bcb5e31cc05c35c559b3 Mon Sep 17 00:00:00 2001 From: Tony Murray Date: Sun, 29 Oct 2023 22:46:04 -0500 Subject: [PATCH] SnmpQuery runtime cache (#15512) * SnmpQuery runtime cache Optionally cache the SnmpResponse from queries for the rest of the runtime. It would be nice to have cache hits for these to help prevent wasting memory caching data that is only used once. Key data must match: device, context, options, oids * Apply fixes from StyleCI * Add command to the cache key * Caching in the wrong spot, cache individual commands... * Apply fixes from StyleCI --------- Co-authored-by: StyleCI Bot --- LibreNMS/Data/Source/NetSnmpQuery.php | 63 ++++++++++++++++----- LibreNMS/Data/Source/SnmpQueryInterface.php | 10 ++++ LibreNMS/Data/Source/SnmpResponse.php | 5 ++ tests/Mocks/SnmpQueryMock.php | 13 +++++ 4 files changed, 77 insertions(+), 14 deletions(-) diff --git a/LibreNMS/Data/Source/NetSnmpQuery.php b/LibreNMS/Data/Source/NetSnmpQuery.php index 034ede3120..dc25c4458a 100644 --- a/LibreNMS/Data/Source/NetSnmpQuery.php +++ b/LibreNMS/Data/Source/NetSnmpQuery.php @@ -30,6 +30,7 @@ use App\Models\Eventlog; use App\Polling\Measure\Measurement; use DeviceCache; use Illuminate\Support\Arr; +use Illuminate\Support\Facades\Cache; use Illuminate\Support\Str; use LibreNMS\Config; use LibreNMS\Enum\Severity; @@ -82,6 +83,7 @@ class NetSnmpQuery implements SnmpQueryInterface private array|string $options = [self::DEFAULT_FLAGS]; private Device $device; private bool $abort = false; + private bool $cache = false; // defaults for net-snmp https://net-snmp.sourceforge.io/docs/man/snmpcmd.html private array $mibs = ['SNMPv2-TC', 'SNMPv2-MIB', 'IF-MIB', 'IP-MIB', 'TCP-MIB', 'UDP-MIB', 'NET-SNMP-VACM-MIB']; @@ -126,6 +128,13 @@ class NetSnmpQuery implements SnmpQueryInterface return $this; } + public function cache(): SnmpQueryInterface + { + $this->cache = true; + + return $this; + } + /** * Set a context for the snmp query * This is most commonly used to fetch alternate sets of data, such as different VRFs @@ -201,6 +210,18 @@ class NetSnmpQuery implements SnmpQueryInterface return $this; } + /** + * Output all OIDs numerically + */ + public function numericIndex(bool $numericIndex = true): SnmpQueryInterface + { + $this->options = $numericIndex + ? array_merge($this->options, ['-Ob']) + : array_diff($this->options, ['-Ob']); + + return $this; + } + /** * Hide MIB in output */ @@ -373,7 +394,8 @@ class NetSnmpQuery implements SnmpQueryInterface // if abort on failure is set, return after first failure if ($this->abort && ! $response->isValid()) { - Log::debug("SNMP failed walking $oid of " . implode(',', $oids) . ' aborting.'); + $oid_list = implode(',', array_map(fn ($group) => implode(',', $group), $oids)); + Log::debug("SNMP failed walking $oid of $oid_list aborting."); return $response; } @@ -384,25 +406,30 @@ class NetSnmpQuery implements SnmpQueryInterface private function exec(string $command, array $oids): SnmpResponse { - $measure = Measurement::start($command); + // use runtime(array) cache if requested. The 'null' driver will simply return the value without caching + $driver = $this->cache ? 'array' : 'null'; + $key = $this->cache ? $this->getCacheKey($command, $oids) : ''; - $proc = new Process($this->buildCli($command, $oids)); - $proc->setTimeout(Config::get('snmp.exec_timeout', 1200)); + return Cache::driver($driver)->rememberForever($key, function () use ($command, $oids) { + $measure = Measurement::start($command); + $proc = new Process($this->buildCli($command, $oids)); + $proc->setTimeout(Config::get('snmp.exec_timeout', 1200)); - $this->logCommand($proc->getCommandLine()); + $this->logCommand($proc->getCommandLine()); - $proc->run(); - $exitCode = $proc->getExitCode(); - $output = $proc->getOutput(); - $stderr = $proc->getErrorOutput(); + $proc->run(); + $exitCode = $proc->getExitCode(); + $output = $proc->getOutput(); + $stderr = $proc->getErrorOutput(); - // check exit code and log possible bad auth - $this->checkExitCode($exitCode, $stderr); - $this->logOutput($output, $stderr); + // check exit code and log possible bad auth + $this->checkExitCode($exitCode, $stderr); + $this->logOutput($output, $stderr); - $measure->manager()->recordSnmp($measure->end()); + $measure->manager()->recordSnmp($measure->end()); - return new SnmpResponse($output, $stderr, $exitCode); + return new SnmpResponse($output, $stderr, $exitCode); + }); } private function initCommand(string $binary, array $oids): array @@ -513,4 +540,12 @@ class NetSnmpQuery implements SnmpQueryInterface { return is_string($oid) ? explode(' ', $oid) : $oid; } + + private function getCacheKey(string $type, array $oids): string + { + $oids = implode(',', $oids); + $options = implode(',', $this->options); + + return "$type|{$this->device->hostname}|$this->context|$oids|$options"; + } } diff --git a/LibreNMS/Data/Source/SnmpQueryInterface.php b/LibreNMS/Data/Source/SnmpQueryInterface.php index 5b3bfc6a91..7f3921821a 100644 --- a/LibreNMS/Data/Source/SnmpQueryInterface.php +++ b/LibreNMS/Data/Source/SnmpQueryInterface.php @@ -46,6 +46,11 @@ interface SnmpQueryInterface */ public function deviceArray(array $device): SnmpQueryInterface; + /** + * Cache the data for the rest of the runtime (or retrieve from cache if available) + */ + public function cache(): SnmpQueryInterface; + /** * Set a context for the snmp query * This is most commonly used to fetch alternate sets of data, such as different VRFs @@ -81,6 +86,11 @@ interface SnmpQueryInterface */ public function numeric(bool $numeric = true): SnmpQueryInterface; + /** + * Output indexes only as numeric + */ + public function numericIndex(bool $numericIndex = true): SnmpQueryInterface; + /** * Hide MIB in output */ diff --git a/LibreNMS/Data/Source/SnmpResponse.php b/LibreNMS/Data/Source/SnmpResponse.php index 1ea7f6871d..02f151bf2a 100644 --- a/LibreNMS/Data/Source/SnmpResponse.php +++ b/LibreNMS/Data/Source/SnmpResponse.php @@ -286,4 +286,9 @@ class SnmpResponse // regular oid return explode('.', $key); } + + public function __sleep() + { + return ['raw', 'exitCode', 'stderr']; + } } diff --git a/tests/Mocks/SnmpQueryMock.php b/tests/Mocks/SnmpQueryMock.php index 0718d25b65..6776083077 100644 --- a/tests/Mocks/SnmpQueryMock.php +++ b/tests/Mocks/SnmpQueryMock.php @@ -73,6 +73,11 @@ class SnmpQueryMock implements SnmpQueryInterface return $this; } + public function cache(): SnmpQueryInterface + { + return $this; // ignore, always cached + } + public function context(string $context): SnmpQueryInterface { $this->context = $context; @@ -117,6 +122,14 @@ class SnmpQueryMock implements SnmpQueryInterface return $this; } + public function numericIndex(bool $numericIndex = true): SnmpQueryInterface + { + // TODO: Implement numericIndex() method + Log::error('numericIndex not implemented in SnmpQueryMock'); + + return $this; + } + public function hideMib(): SnmpQueryInterface { $this->hideMib = true;