Merge all cpu in one graph in device overview #2470

Merged
merged 5 commits into from Nov 24, 2015

Projects

None yet

3 participants

@Alucardfh

Hi,

This provide a way to hide all separate graphs in the device overview and replace it by the average cpu usage.

This make the overview easier to read and render the page a bit faster as there is less graphs to draw.

This is an optional feature, disabled by default.

Just add in your config.php :

$config['cpu_details_overview'] = false;

This is related to issue #2218

cpuovreview

cpuovreviewchange

Louis Bailleul added some commits Nov 20, 2015
Louis Bailleul Add possibility to merge all cpu in one graph in the device overview a2bc1a4
Louis Bailleul Fix formatting 8921c75
Louis Bailleul Replace tab by space ee99ee8
Louis Bailleul Add the number of cpu between the type and percent graph
439193c
@laf
Member
laf commented Nov 20, 2015

I've not checked any code yet but can you update this to have it ON by default. It's horrible so I don't expect people will miss it. It makes the UI better this way.

@laf laf added the WebUI label Nov 20, 2015
@laf
Member
laf commented Nov 20, 2015

Had a chance to check through this, some comments below and also inline with your code:

@laf laf and 1 other commented on an outdated diff Nov 20, 2015
html/pages/device/overview/processors.inc.php
foreach ($processors as $proc) {
$text_descr = rewrite_entity_descr($proc['processor_descr']);
// disable short hrDeviceDescr. need to make this prettier.
// $text_descr = short_hrDeviceDescr($proc['processor_descr']);
$percent = $proc['processor_usage'];
- $background = get_percentage_colours($percent);
- $graph_colour = str_replace('#', '', $row_colour);
@laf
laf Nov 20, 2015 Member

Should this line and the above be moved? Are these variables not used in both parts of the if / else block?

@Alucardfh
Alucardfh Nov 20, 2015

Only the $background is used in both part, as actualy $graph_colour doesn't seem to be used.

But it make sense to compute the background colour on the final precentage and not on the last one.
I'll move this to the else block.

I need to clean up the variables that are not used.

@laf laf and 1 other commented on an outdated diff Nov 20, 2015
html/pages/device/overview/processors.inc.php
- $background = get_percentage_colours($percent);
- $graph_colour = str_replace('#', '', $row_colour);
+ if ($config['cpu_details_overview'] === true)
+ {
+
+ $background = get_percentage_colours($percent);
+ $graph_colour = str_replace('#', '', $row_colour);
+
+ $graph_array = array();
+ $graph_array['height'] = '100';
+ $graph_array['width'] = '210';
+ $graph_array['to'] = $config['time']['now'];
+ $graph_array['id'] = $proc['processor_id'];
+ $graph_array['type'] = $graph_type;
+ $graph_array['from'] = $config['time']['day'];
+ $graph_array['legend'] = 'no';
@laf
laf Nov 20, 2015 Member

We now have a lot of duplicated variables which could just be set before the if check. Can you consolidate them together and then just overwrite where you need.

@Alucardfh
Alucardfh Nov 20, 2015

I agree, it would be cleaner to have less duplicate, I'll see to consolidate everything above and just override what is needed.

@laf laf commented on an outdated diff Nov 20, 2015
html/pages/device/overview/processors.inc.php
- $graph_array = array();
+ }//end foreach
+
+ if ($config['cpu_details_overview'] !== true)
@laf
laf Nov 20, 2015 Member

Can you not change this to be === false?

@laf laf and 1 other commented on an outdated diff Nov 20, 2015
html/pages/device/overview/processors.inc.php
$graph_array['height'] = '100';
- $graph_array['width'] = '210';
+ $graph_array['width'] = '485';
@laf
laf Nov 20, 2015 Member

What's the reason for changing the width?

@Alucardfh
Alucardfh Nov 20, 2015

The width change is to make the graph of a normal size instead of the mini graph when all cpu are displayed.

@laf laf and 1 other commented on an outdated diff Nov 20, 2015
html/pages/device/overview/processors.inc.php
$graph_array['to'] = $config['time']['now'];
- $graph_array['id'] = $proc['processor_id'];
- $graph_array['type'] = $graph_type;
+ $graph_array['device'] = $device['device_id'];
+ $graph_array['type'] = 'device_processor';
@laf
laf Nov 20, 2015 Member

Are these changes necessary?

@Alucardfh
Alucardfh Nov 20, 2015

Yes as it is not the same graph type, the old one use "processor_usage" which works with the id of the cpus but the new way works with the device ID and the "device_processor" graph type.

@laf laf commented on an outdated diff Nov 20, 2015
html/pages/device/overview/processors.inc.php
$link = generate_url($link_array);
- $overlib_content = generate_overlib_content($graph_array, $device['hostname'].' - '.$text_descr);
-
- $graph_array['width'] = 80;
- $graph_array['height'] = 20;
- $graph_array['bg'] = 'ffffff00';
- // the 00 at the end makes the area transparent.
- $minigraph = generate_lazy_graph_tag($graph_array);
+ $graph_array['width'] = '210';
@laf
laf Nov 20, 2015 Member

Width has been changed, what's the reason for that?

@Alucardfh

I'll work on the comments when I get time (probably Monday).

@laf
Member
laf commented Nov 20, 2015

No problem. Thanks for the work :)

Louis Bailleul Cleanup and reorganize code
Make the average cpu the default
a563442
@Alucardfh

I have reorganized the code and trimmed what wasn't used.

I also switched it on by default.

@laf laf merged commit d3f3b1d into librenms:master Nov 24, 2015

1 of 3 checks passed

Scrutinizer Analysis Timed out
Details
Scrutinizer Tests Timed out
Details
Auto-Deploy Build finished. No test results found.
Details
@Rosiak Rosiak referenced this pull request Dec 11, 2015
Closed

Overview tab. #2218

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment