librenms / librenms

Community-based GPL-licensed network monitoring system
https://www.librenms.org
Other
3.77k stars 2.26k forks source link

Only flow graphs from nfsen, no traffic/packets graphs #2710

Closed wiad closed 8 years ago

wiad commented 8 years ago

I succeeded in integrating our nfsen data into our librenms installation, but when I looked closer I realized that even though each row of graphs were titled differently ("Flows", "Packets" and "Traffic") they all contained the flow graphs. I looked at the code (html/includes/print-device-graph.php) and it seems to me that this snippet:

if (empty($graph_array['type'])) {
    $graph_array['type'] = $graph_type;
}

will only work correctly the first time it is called, at least in this example with the nfsen implementation. In html/pages/device/nfsen.inc.php the above code is referenced three times, once for each graph type (flows, packets, traffic). The first time it is called, the $graph_type variable holds the value "Flows" and hence the array $graph_array['type'] will be set to this. The second time around the $graph_type variable holds the value "Packets", but the array will not be assigned this value since it is already holding the "Flows" value. So that if statement is not working for me.

Not sure if I'm misreading the code or how other people are using this, but to me this looks wrong. I worked around it by setting $graph_array['type'] to the correct value directly in html/pages/device/nfsen.inc.php.

paulgear commented 8 years ago

@wiad Would you mind reverting your fix and trying this patch?

diff --git a/html/includes/print-graphrow.inc.php b/html/includes/print-graphrow.inc.php
index 9b98a80..a35107b 100644
--- a/html/includes/print-graphrow.inc.php
+++ b/html/includes/print-graphrow.inc.php
@@ -76,3 +76,4 @@ foreach ($periods as $period) {
         echo "</div>";
     }
 }
+unset($graph_array);
wiad commented 8 years ago

Works!

paulgear commented 8 years ago

Fixed with PR#2783