From: Michael Tremer <michael.tremer@ipfire.org>
To: development@lists.ipfire.org
Subject: Re: [PATCH] GUI: Periodically reload RRD graphs
Date: Wed, 10 Mar 2021 13:49:58 +0000 [thread overview]
Message-ID: <CED2ADAA-B1D2-421D-A6AD-3C86A47D8F71@ipfire.org> (raw)
In-Reply-To: <0C074083-06C7-42B9-981C-57161C343D30@ipfire.org>
[-- Attachment #1: Type: text/plain, Size: 20063 bytes --]
Hello,
> On 10 Mar 2021, at 12:29, Jonatan Schlag <jonatan.schlag(a)ipfire.org> wrote:
>
> Hi,
>
>> Am 08.03.2021 um 20:21 schrieb Leo-Andres Hofmann <hofmann(a)leo-andres.de>:
>>
>> The RRD graphs are embedded in an iframe tag.
> Do know why? Just wondering why we put an iFrame tag around the image. Should it no be possible to reload the image without an iFrame?
Very good question. I think this is because we needed a way to change only the image and JS wasn’t an option back then. The <a> tag allowed to specify a target which changed the content of the iframe.
I would absolutely prefer to get rid of it and just use one or multiple img tags. Those would also make styling easier.
>> Therefore it is possible
>> to refresh them periodically without reloading or changing the pages.
>> This patch adds a graph reload javascript to the ipfire main theme.
>>
>> Signed-off-by: Leo-Andres Hofmann <hofmann(a)leo-andres.de>
>> ---
>>
> ⬇️ This part which is not really part of the commit message, can be put before an patch using the —compose option :-).
Yes this is brilliant :)
>> Hi,
>>
>> This is more of a nice-to-have, an idea I would like to present.
>> As I was working on the QoS graphs, I noticed that all RRD graphs are embedded in an iframe.
>> I wrote a little javascript that periodically reloads these graphs.
>
> Would it not be sufficient to reload only the hourly graph? So all others have a range where I would not assume somebody to monitor them “live“.
>> The refresh interval is based on the graph's time range, i.e. hourly graphs are refreshed more often.
>> Reloading should stop in case of a network error, but I haven't fully tested this yet.
>>
>> This patch adds a checkbox to the GUI settings (default: disabled), translations, ...
>> If you just want to try the javascript, you can simply paste it to the html <head> in
>> html/themes/ipfire/include/functions.pl. Be sure to escape the '$' then.
>>
>> Best regards
>> Leo
>>
>> doc/language_issues.en | 1 +
>> doc/language_issues.es | 1 +
>> doc/language_issues.fr | 1 +
>> doc/language_issues.it | 1 +
>> doc/language_issues.nl | 1 +
>> doc/language_issues.pl | 1 +
>> doc/language_issues.ru | 1 +
>> doc/language_issues.tr | 1 +
>> doc/language_missings | 7 ++
>> html/cgi-bin/gui.cgi | 16 ++++
>> html/html/themes/ipfire/include/functions.pl | 7 ++
>> .../themes/ipfire/include/js/reloadGraphs.js | 81 +++++++++++++++++++
>> langs/de/cgi-bin/de.pl | 1 +
>> langs/en/cgi-bin/en.pl | 1 +
>> 14 files changed, 121 insertions(+)
>> create mode 100644 html/html/themes/ipfire/include/js/reloadGraphs.js
>>
>> diff --git a/doc/language_issues.en b/doc/language_issues.en
>> index 7819541c2..9edc57d19 100644
>> --- a/doc/language_issues.en
>> +++ b/doc/language_issues.en
>> @@ -1446,6 +1446,7 @@ WARNING: untranslated string: password not set = Password not set.
>> WARNING: untranslated string: password too short = Password is too short.
>> WARNING: untranslated string: passwords do not match = Passwords do not match.
>> WARNING: untranslated string: percentage = Percentage
>> +WARNING: untranslated string: periodically reload graphs = Periodically refresh RRD graphs
>> WARNING: untranslated string: persistent = Persistent
>> WARNING: untranslated string: pfs yes no = Perfect Forward Secrecy (PFS)
>> WARNING: untranslated string: pkcs12 file password = PKCS12 File Password
>> diff --git a/doc/language_issues.es b/doc/language_issues.es
>> index 952321fbe..3d623302c 100644
>> --- a/doc/language_issues.es
>> +++ b/doc/language_issues.es
>> @@ -1318,6 +1318,7 @@ WARNING: untranslated string: pakfire tree = Repository
>> WARNING: untranslated string: pakfire tree stable = Stable
>> WARNING: untranslated string: pakfire tree testing = Testing
>> WARNING: untranslated string: pakfire tree unstable = Unstable
>> +WARNING: untranslated string: periodically reload graphs = Periodically refresh RRD graphs
>> WARNING: untranslated string: please reboot to apply your changes = Please reboot to apply your changes
>> WARNING: untranslated string: pptp netconfig = My Net Config
>> WARNING: untranslated string: pptp peer = Peer
>> diff --git a/doc/language_issues.fr b/doc/language_issues.fr
>> index 9fe5ca21c..e3a38df4e 100644
>> --- a/doc/language_issues.fr
>> +++ b/doc/language_issues.fr
>> @@ -914,6 +914,7 @@ WARNING: untranslated string: guardian logtarget_syslog = unknown string
>> WARNING: untranslated string: guardian no entries = unknown string
>> WARNING: untranslated string: guardian service = unknown string
>> WARNING: untranslated string: pakfire ago = ago.
>> +WARNING: untranslated string: periodically reload graphs = Periodically refresh RRD graphs
>> WARNING: untranslated string: route config changed = unknown string
>> WARNING: untranslated string: routing config added = unknown string
>> WARNING: untranslated string: routing config changed = unknown string
>> diff --git a/doc/language_issues.it b/doc/language_issues.it
>> index 29c9c7d58..a66c693b0 100644
>> --- a/doc/language_issues.it
>> +++ b/doc/language_issues.it
>> @@ -1108,6 +1108,7 @@ WARNING: untranslated string: pakfire tree = Repository
>> WARNING: untranslated string: pakfire tree stable = Stable
>> WARNING: untranslated string: pakfire tree testing = Testing
>> WARNING: untranslated string: pakfire tree unstable = Unstable
>> +WARNING: untranslated string: periodically reload graphs = Periodically refresh RRD graphs
>> WARNING: untranslated string: please reboot to apply your changes = Please reboot to apply your changes
>> WARNING: untranslated string: pptp netconfig = My Net Config
>> WARNING: untranslated string: pptp peer = Peer
>> diff --git a/doc/language_issues.nl b/doc/language_issues.nl
>> index d6cad0dee..a2b978ccc 100644
>> --- a/doc/language_issues.nl
>> +++ b/doc/language_issues.nl
>> @@ -1140,6 +1140,7 @@ WARNING: untranslated string: pakfire tree = Repository
>> WARNING: untranslated string: pakfire tree stable = Stable
>> WARNING: untranslated string: pakfire tree testing = Testing
>> WARNING: untranslated string: pakfire tree unstable = Unstable
>> +WARNING: untranslated string: periodically reload graphs = Periodically refresh RRD graphs
>> WARNING: untranslated string: please reboot to apply your changes = Please reboot to apply your changes
>> WARNING: untranslated string: pptp netconfig = My Net Config
>> WARNING: untranslated string: pptp peer = Peer
>> diff --git a/doc/language_issues.pl b/doc/language_issues.pl
>> index b8fba938f..7f2b72cc0 100644
>> --- a/doc/language_issues.pl
>> +++ b/doc/language_issues.pl
>> @@ -1327,6 +1327,7 @@ WARNING: untranslated string: pakfire tree = Repository
>> WARNING: untranslated string: pakfire tree stable = Stable
>> WARNING: untranslated string: pakfire tree testing = Testing
>> WARNING: untranslated string: pakfire tree unstable = Unstable
>> +WARNING: untranslated string: periodically reload graphs = Periodically refresh RRD graphs
>> WARNING: untranslated string: please reboot to apply your changes = Please reboot to apply your changes
>> WARNING: untranslated string: pptp netconfig = My Net Config
>> WARNING: untranslated string: pptp peer = Peer
>> diff --git a/doc/language_issues.ru b/doc/language_issues.ru
>> index d507911f1..f988580cf 100644
>> --- a/doc/language_issues.ru
>> +++ b/doc/language_issues.ru
>> @@ -1322,6 +1322,7 @@ WARNING: untranslated string: pakfire tree = Repository
>> WARNING: untranslated string: pakfire tree stable = Stable
>> WARNING: untranslated string: pakfire tree testing = Testing
>> WARNING: untranslated string: pakfire tree unstable = Unstable
>> +WARNING: untranslated string: periodically reload graphs = Periodically refresh RRD graphs
>> WARNING: untranslated string: please reboot to apply your changes = Please reboot to apply your changes
>> WARNING: untranslated string: pptp netconfig = My Net Config
>> WARNING: untranslated string: pptp peer = Peer
>> diff --git a/doc/language_issues.tr b/doc/language_issues.tr
>> index 04343d527..dc0fd98f0 100644
>> --- a/doc/language_issues.tr
>> +++ b/doc/language_issues.tr
>> @@ -1021,6 +1021,7 @@ WARNING: untranslated string: pakfire tree = Repository
>> WARNING: untranslated string: pakfire tree stable = Stable
>> WARNING: untranslated string: pakfire tree testing = Testing
>> WARNING: untranslated string: pakfire tree unstable = Unstable
>> +WARNING: untranslated string: periodically reload graphs = Periodically refresh RRD graphs
>> WARNING: untranslated string: please reboot to apply your changes = Please reboot to apply your changes
>> WARNING: untranslated string: processor vulnerability mitigations = Processor Vulnerability Mitigations
>> WARNING: untranslated string: ptr = PTR
>> diff --git a/doc/language_missings b/doc/language_missings
>> index f10c4f242..89808f3b5 100644
>> --- a/doc/language_missings
>> +++ b/doc/language_missings
>> @@ -669,6 +669,7 @@
>> < pakfire tree stable
>> < pakfire tree testing
>> < pakfire tree unstable
>> +< periodically reload graphs
>> < please reboot to apply your changes
>> < pptp netconfig
>> < pptp peer
>> @@ -922,6 +923,7 @@
>> < dhcp valid range required when deny known clients checked
>> < g.dtm
>> < g.lite
>> +< periodically reload graphs
>> < token
>> < token not set
>> < upload fcdsl.o
>> @@ -1198,6 +1200,7 @@
>> < pakfire tree stable
>> < pakfire tree testing
>> < pakfire tree unstable
>> +< periodically reload graphs
>> < please reboot to apply your changes
>> < pptp netconfig
>> < pptp peer
>> @@ -1632,6 +1635,7 @@
>> < pakfire tree stable
>> < pakfire tree testing
>> < pakfire tree unstable
>> +< periodically reload graphs
>> < please reboot to apply your changes
>> < pptp netconfig
>> < pptp peer
>> @@ -2407,6 +2411,7 @@
>> < pakfire tree stable
>> < pakfire tree testing
>> < pakfire tree unstable
>> +< periodically reload graphs
>> < please reboot to apply your changes
>> < pptp netconfig
>> < pptp peer
>> @@ -3297,6 +3302,7 @@
>> < pakfire tree stable
>> < pakfire tree testing
>> < pakfire tree unstable
>> +< periodically reload graphs
>> < please reboot to apply your changes
>> < pptp netconfig
>> < pptp peer
>> @@ -3660,6 +3666,7 @@
>> < pakfire tree stable
>> < pakfire tree testing
>> < pakfire tree unstable
>> +< periodically reload graphs
>> < please reboot to apply your changes
>> < processor vulnerability mitigations
>> < ptr
>> diff --git a/html/cgi-bin/gui.cgi b/html/cgi-bin/gui.cgi
>> index f06b0f923..a5255dbce 100644
>> --- a/html/cgi-bin/gui.cgi
>> +++ b/html/cgi-bin/gui.cgi
>> @@ -37,6 +37,7 @@ my $errormessage='';
>>
>>
>> $cgiparams{'SPEED'} = 'off';
>> +$cgiparams{'RELOADGRAPHS'} = 'off';
>> $cgiparams{'WINDOWWITHHOSTNAME'} = 'off';
>> $cgiparams{'REFRESHINDEX'} = 'off';
>> $cgiparams{'ACTION'} = '';
>> @@ -88,6 +89,7 @@ if ($cgiparams{'ACTION'} eq "$Lang::tr{'save'}")
>> $mainsettings{'WINDOWWITHHOSTNAME'} = $cgiparams{'WINDOWWITHHOSTNAME'};
>> $mainsettings{'PPPUPDOWNBEEP'} = $cgiparams{'PPPUPDOWNBEEP'};
>> $mainsettings{'SPEED'} = $cgiparams{'SPEED'};
>> + $mainsettings{'RELOADGRAPHS'} = $cgiparams{'RELOADGRAPHS'};
>> $mainsettings{'THEME'} = $cgiparams{'theme'};
>> $mainsettings{'REFRESHINDEX'} = $cgiparams{'REFRESHINDEX'};
>> &General::writehash("${General::swroot}/main/settings", \%mainsettings);
>> @@ -125,6 +127,11 @@ if ($cgiparams{'ACTION'} eq "$Lang::tr{'save'}")
>> # by manually setting the var to off
>> $cgiparams{'SPEED'} = 'on';
>> }
>> + if($mainsettings{'RELOADGRAPHS'}) {
>> + $cgiparams{'RELOADGRAPHS'} = $mainsettings{'RELOADGRAPHS'};
>> + } else {
>> + $cgiparams{'RELOADGRAPHS'} = 'off';
>> + }
>> }
>>
>> # Default settings
>> @@ -134,6 +141,7 @@ if ($cgiparams{'ACTION'} eq "$Lang::tr{'restore defaults'}")
>> $cgiparams{'PPPUPDOWNBEEP'} = 'on';
>> $cgiparams{'REFRESHINDEX'} = 'off';
>> $cgiparams{'SPEED'} = 'on';
>> + $cgiparams{'RELOADGRAPHS'} = 'off';
>> $cgiparams{'THEME'} = 'ipfire';
>> }
>>
>> @@ -153,6 +161,10 @@ $checked{'SPEED'}{'off'} = '';
>> $checked{'SPEED'}{'on'} = '';
>> $checked{'SPEED'}{$cgiparams{'SPEED'}} = "checked='checked'";
>>
>> +$checked{'RELOADGRAPHS'}{'off'} = '';
>> +$checked{'RELOADGRAPHS'}{'on'} = '';
>> +$checked{'RELOADGRAPHS'}{$cgiparams{'RELOADGRAPHS'}} = "checked='checked'";
>> +
>> &Header::openpage($Lang::tr{'gui settings'}, 1, '');
>> &Header::openbigbox('100%', 'left', '', $errormessage);
>>
>> @@ -179,6 +191,10 @@ print <<END
>> <td><input type='checkbox' name='SPEED' $checked{'SPEED'}{'on'} /></td>
>> <td>$Lang::tr{'show ajax speedmeter in footer'}</td>
>> </tr>
>> +<tr>
>> + <td><input type='checkbox' name='RELOADGRAPHS' $checked{'RELOADGRAPHS'}{'on'} /></td>
>> + <td>$Lang::tr{'periodically reload graphs'}</td>
>> +</tr>
>> <tr>
>> <td> </td>
>> <td>$Lang::tr{'languagepurpose'}</td>
Does this actually need configuration or can we not simply enable this for everyone?
What is the downside? If the browser has JS disabled, simply nothing would happen.
>> diff --git a/html/html/themes/ipfire/include/functions.pl b/html/html/themes/ipfire/include/functions.pl
>> index 9aec77497..7f3401119 100644
>> --- a/html/html/themes/ipfire/include/functions.pl
>> +++ b/html/html/themes/ipfire/include/functions.pl
>> @@ -147,6 +147,13 @@ END
>> ;
>> }
>>
>> +if ($settings{'RELOADGRAPHS'} eq 'on') {
>> +print <<END
>> + <script src="/themes/ipfire/include/js/reloadGraphs.js"></script>
>> +END
>> +;
>> +}
>> +
>> print <<END
>> </head>
>> <body>
>> diff --git a/html/html/themes/ipfire/include/js/reloadGraphs.js b/html/html/themes/ipfire/include/js/reloadGraphs.js
>> new file mode 100644
>> index 000000000..c0c8bec9e
>> --- /dev/null
>> +++ b/html/html/themes/ipfire/include/js/reloadGraphs.js
>> @@ -0,0 +1,81 @@
>> +/*#############################################################################
>> +# #
>> +# IPFire.org - A linux based firewall #
>> +# Copyright (C) 2007-2021 IPFire Team <info(a)ipfire.org> #
>> +# #
>> +# This program is free software: you can redistribute it and/or modify #
>> +# it under the terms of the GNU General Public License as published by #
>> +# the Free Software Foundation, either version 3 of the License, or #
>> +# (at your option) any later version. #
>> +# #
>> +# This program is distributed in the hope that it will be useful, #
>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of #
>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the #
>> +# GNU General Public License for more details. #
>> +# #
>> +# You should have received a copy of the GNU General Public License #
>> +# along with this program. If not, see <http://www.gnu.org/licenses/>. #
>> +# #
>> +#############################################################################*/
>> +
>> +var graphs_reloadTimers = new Map();
>> +
>> +//graph onload event: graph is now loaded, start reload timer
>> +function graphs_onloadEvent(graphObj, startTimer = true) {
>> + if(! graphObj) {
>> + return; //graph iframe object is missing
>> + }
>> +
>> + //get graph parameters from iframe url: [url]?[graph name]?[time range]
>> + let url = graphObj.contentWindow.location.href;
>> + if((! url) || (url === 'about:blank')) {
>> + url = graphObj.src;
>> + }
>> + url = url.split('?', 3);
>> + let graphRange = url.pop();
>> + let graphName = url.pop();
>> +
>> + if((! graphName) || (! graphRange)) {
>> + return; //url parameters are missing
>> + }
>> +
>> + //clear pending reload timeout to prevent multiple timer starts
>> + if(graphs_reloadTimers.has(graphName)) {
>> + clearTimeout(graphs_reloadTimers.get(graphName));
>> + graphs_reloadTimers.delete(graphName);
>> + }
>> +
>> + if(startTimer) {
>> + //determine reload interval
>> + let interval = 60; //default: 60s
>> + if(graphRange === 'hour') {
>> + interval = 10;
>> + } else if(graphRange === 'month' || graphRange === 'year') {
> Do not think that’s this makes any sense here. Should not change a pixel in this graphs :-). Why is day not listed here ?
>> + interval = 180;
>> + }
>> +
>> + //start timeout and store timer id
>> + let timer = setTimeout(function(graphFrame, timerRef) {
>> + graphs_reloadTimers.delete(timerRef); //clear expired timer
>> +
>> + //reload graph iframe, ignore cache
>> + graphFrame.contentWindow.location.reload(true);
>> + }, interval * 1000, graphObj, graphName);
>> + graphs_reloadTimers.set(graphName, timer);
>> + }
>> +}
>> +
>
> Would it no be easier to attach this directly to a graph so inside the function which creates the graph, should be makegraphs() or something like that?
>> +//document.ready, must fire before iframes load
>> +$(function() {
>> + //attach onload event to graphs
>> + $('iframe.graph').each(function() {
>> + $(this).on({
>> + load: function() {
>> + graphs_onloadEvent(this);
>> + },
>> + error: function() { //stop reload on error
>> + graphs_onloadEvent(this, false);
>> + }
>> + });
>> + });
>> +});
>> diff --git a/langs/de/cgi-bin/de.pl b/langs/de/cgi-bin/de.pl
>> index b3c2a69da..00e6b9853 100644
>> --- a/langs/de/cgi-bin/de.pl
>> +++ b/langs/de/cgi-bin/de.pl
>> @@ -2007,6 +2007,7 @@
>> 'pc add' => 'PC hinzufügen',
>> 'pdc options' => 'PDC Optionen',
>> 'percentage' => 'Prozent',
>> +'periodically reload graphs' => 'RRD Graphen periodisch neu laden',
>> 'persistent' => 'Dauerhaft',
>> 'pfs yes no' => 'Perfect Forward Secrecy (PFS)',
>> 'phase1 group' => 'Phase1 Gruppe',
>> diff --git a/langs/en/cgi-bin/en.pl b/langs/en/cgi-bin/en.pl
>> index 9767be178..ed9e4996a 100644
>> --- a/langs/en/cgi-bin/en.pl
>> +++ b/langs/en/cgi-bin/en.pl
>> @@ -2037,6 +2037,7 @@
>> 'pc add' => 'Add workstation',
>> 'pdc options' => 'PDC options',
>> 'percentage' => 'Percentage',
>> +'periodically reload graphs' => 'Periodically refresh RRD graphs',
>> 'persistent' => 'Persistent',
>> 'pfs yes no' => 'Perfect Forward Secrecy (PFS)',
>> 'phase1 group' => 'Phase1 Group',
>> --
>> 2.27.0.windows.1
>>
>
> Some generic thoughts. Would it not be better to remove the iFrame and make the links for hour/day/month/year firing some JavaScripts which reloads/changes the graphs? And then when we click hour start this script to reload the graphs periodically?
Agreed.
I very much like the idea and I think this is an opportunity to clean up a little bit more. I will let you two sort out the technical things and how often is best to reload :)
> Thanks for the nice patch. It is easy to review :-)
>
> Greetings Jonatan
-Michael
next prev parent reply other threads:[~2021-03-10 13:49 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-03-08 19:21 Leo-Andres Hofmann
2021-03-10 12:29 ` Jonatan Schlag
2021-03-10 13:49 ` Michael Tremer [this message]
2021-03-12 13:41 ` Leo Hofmann
2021-03-14 11:44 ` Jonatan Schlag
2021-03-15 16:13 ` Aw: " Bernhard Bitsch
2021-03-16 15:30 ` Michael Tremer
2021-03-16 15:30 ` Michael Tremer
2021-03-16 12:29 ` Michael Tremer
[not found] <513a2083-77a3-bd4c-f1cc-d461625c0554@leo-andres.de>
2021-03-16 8:37 ` Adolf Belka
2021-03-16 15:35 ` Michael Tremer
2021-03-16 15:34 ` Michael Tremer
2021-03-18 20:20 ` Leo Hofmann
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CED2ADAA-B1D2-421D-A6AD-3C86A47D8F71@ipfire.org \
--to=michael.tremer@ipfire.org \
--cc=development@lists.ipfire.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox