Refactor and cleanup the processor graph

Author: murrant

LibreNMS/Data/Graphing/Device/ProcessorGraph.php

@@ -29,30 +29,34 @@
 use App\Facades\LibrenmsConfig;
 use App\Facades\Rrd;
 use App\Models\Device;
-use Illuminate\Support\Facades\DB;
+use Illuminate\Support\Collection;
 use Illuminate\Support\Facades\Gate;
 use LibreNMS\Data\Graphing\AbstractGraph;
 use LibreNMS\Data\Graphing\Builders\MultiLineGraphBuilder;
 use LibreNMS\Data\Graphing\Builders\MultiSimplexSeparatedGraphBuilder;
-use LibreNMS\Exceptions\RrdGraphException;
-use LibreNMS\Util\Rewrite;
 
 class ProcessorGraph extends AbstractGraph
 {
     public string $type = 'device';
     public string $subtype = 'processor';
 
     private Device $device;
+    private Collection $processors;
 
-
-    public function __construct(private readonly array $vars = [])
-    {
+    public function __construct(
+        private readonly array $vars = [],
+    ) {
         $this->device = DeviceCache::get($this->vars['device'] ?? null);
+        $this->processors = $this->device->exists ? $this->device->processors : new Collection;
     }
 
     public function authorize(): bool
     {
-        return Gate::allows('view', $this->device);
+        if ($processor = $this->processors->first()) {
+            return Gate::allows('view', $processor);
+        }
+
+        return $this->device->exists && Gate::allows('view', $this->device);
     }
 
     public function getGraphTitle(): string
@@ -62,42 +66,30 @@ public function getGraphTitle(): string
 
     public function getRrdFiles(): array
     {
-        $procs = DB::table('processors')->where('device_id', $this->device->device_id)->get();
         $files = [];
-        foreach ($procs as $proc) {
-            $rrd_filename = Rrd::name($this->device->hostname, ['processor', $proc->processor_type, $proc->processor_index]);
-            if (Rrd::checkRrdExists($rrd_filename)) {
-                $files[] = $rrd_filename;
-            }
+        foreach ($this->processors as $proc) {
+            $files[] = Rrd::name($this->device->hostname, ['processor', $proc->processor_type, $proc->processor_index]);
         }
         return $files;
     }
 
     public function definition(array $vars = []): array
     {
-        $this->authorize();
-
-        $procs = DB::table('processors')->where('device_id', $this->device->device_id)->get();
-        if ($procs->isEmpty()) {
-            throw new RrdGraphException('No Processors');
+        if ($this->processors->isEmpty()) {
+            throw new \LibreNMS\Exceptions\RrdGraphException('No Processors');
         }
 
-        $rrd_list = [];
-        foreach ($procs as $proc) {
+        $vars = array_merge($this->vars, $vars);
+
+        // Count how many files actually exist on disk for the layout
+        $rrd_count = 0;
+        foreach ($this->processors as $proc) {
             $rrd_filename = Rrd::name($this->device->hostname, ['processor', $proc->processor_type, $proc->processor_index]);
             if (Rrd::checkRrdExists($rrd_filename)) {
-                $descr = Rewrite::shortHrDevice($proc->processor_descr);
-                $rrd_list[] = [
-                    'filename' => $rrd_filename,
-                    'descr' => $descr,
-                    'ds' => 'usage',
-                ];
+                $rrd_count++;
             }
         }
 
-        $vars = array_merge($this->vars, $vars);
-
-        // Check if stacked processor graph is configured for this OS
         if (LibrenmsConfig::getOsSetting($this->device->os, 'processor_stacked')) {
             $builder = (new MultiSimplexSeparatedGraphBuilder())
                 ->unitText('Load %')
@@ -106,37 +98,29 @@ public function definition(array $vars = []): array
                 ->colours('oranges')
                 ->scaleMin(0)
                 ->scaleMax(100)
-                ->divider(count($rrd_list))
+                ->divider(max(1, $rrd_count))
                 ->textOrig()
                 ->noTotal();
-
-            foreach ($rrd_list as $rrd) {
-                $builder->addDataset(
-                    filename: $rrd['filename'],
-                    ds: $rrd['ds'],
-                    description: $rrd['descr']
-                );
-            }
-
-            return $builder->build($vars);
+        } else {
+            $builder = (new MultiLineGraphBuilder())
+                ->unitText('Load %')
+                ->units('')
+                ->totalUnits('%')
+                ->colours('mixed')
+                ->scaleMin(0)
+                ->scaleMax(100)
+                ->noTotal();
         }
 
-        $builder = (new MultiLineGraphBuilder())
-            ->unitText('Load %')
-            ->units('')
-            ->totalUnits('%')
-            ->colours('mixed')
-            ->scaleMin(0)
-            ->scaleMax(100)
-            ->noTotal();
-
-        foreach ($rrd_list as $rrd) {
-            $builder->addDataset(
-                filename: $rrd['filename'],
-                ds: $rrd['ds'],
-                description: $rrd['descr'],
-                area: true
-            );
+        foreach ($this->processors as $proc) {
+            $rrd_filename = Rrd::name($this->device->hostname, ['processor', $proc->processor_type, $proc->processor_index]);
+            if (Rrd::checkRrdExists($rrd_filename)) {
+                if ($builder instanceof MultiLineGraphBuilder) {
+                    $builder->addDataset($rrd_filename, 'usage', $proc->getFormattedDescription(), area: true);
+                } else {
+                    $builder->addDataset($rrd_filename, 'usage', $proc->getFormattedDescription());
+                }
+            }
         }
 
         return $builder->build($vars);

LibreNMS/Data/Graphing/LegacyGraph.php

@@ -79,7 +79,7 @@ private function load(array $vars = []): void
         include_once base_path('includes/rewrites.php');
 
         if (! $this->device && isset($this->vars['device'])) {
-            $this->device = DeviceCache::get(is_numeric($this->vars['device']) ? $this->vars['device'] : getidbyname($this->vars['device']));
+            $this->device = DeviceCache::get($this->vars['device']);
         }
 
         if ($this->device) {

LibreNMS/Util/Rewrite.php

@@ -405,21 +405,4 @@ public static function celsiusToFahrenheit(float $celsius): float
     {
         return round($celsius * 1.8 + 32, 2);
     }
-
-    public static function shortHrDevice(string $descr): string
-    {
-        $rewrite_hrDevice = [
-            'GenuineIntel:' => '',
-            'AuthenticAMD:' => '',
-            'Intel(R)' => '',
-            'CPU' => '',
-            '(R)' => '',
-            '  ' => ' ',
-        ];
-
-        $descr = str_replace(array_keys($rewrite_hrDevice), array_values($rewrite_hrDevice), $descr);
-        $descr = preg_replace('/\ +/', ' ', $descr);
-
-        return trim($descr);
-    }
 }

app/Models/Processor.php

@@ -18,7 +18,7 @@ class Processor extends DeviceRelatedModel
      *
      * @return string
      */
-    public function getFormattedDescription()
+    public function getFormattedDescription(): string
     {
         $bad_descr = [
             'GenuineIntel:',
@@ -32,8 +32,6 @@ public function getFormattedDescription()
         $descr = str_replace($bad_descr, '', $this->processor_descr);
 
         // reduce extra spaces
-        $descr = str_replace('  ', ' ', $descr);
-
-        return $descr;
+        return str_replace('  ', ' ', $descr);
     }
 }