From mboxrd@z Thu Jan 1 00:00:00 1970 From: Leo Hofmann To: development@lists.ipfire.org Subject: Re: [PATCH] GUI: Periodically reload RRD graphs Date: Fri, 12 Mar 2021 14:41:50 +0100 Message-ID: <466930dd-f05e-baa0-abf5-ef039d186ab0@leo-andres.de> In-Reply-To: MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============6778556937702938558==" List-Id: --===============6778556937702938558== Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Hi, Am 10.03.2021 um 14:49 schrieb Michael Tremer: > Hello, > >> On 10 Mar 2021, at 12:29, Jonatan Schlag wro= te: >> >> Hi, >> >>> Am 08.03.2021 um 20:21 schrieb Leo-Andres Hofmann : >>> >>> =EF=BB=BFThe RRD graphs are embedded in an iframe tag. >> Do know why? Just wondering why we put an iFrame tag around the image. Sho= uld 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=E2=80=99t an option back then. The tag allowed to s= pecify 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. I would also prefer the image tag. But as far as I know, there is no way to c= hange the image without javascript. > >>> 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 >>> --- >>> >> =E2=AC=87=EF=B8=8F This part which is not really part of the commit messag= e, can be put before an patch using the =E2=80=94compose option :-). > Yes this is brilliant :) Thanks! This time I used -annotate to fit everything in one e-mail, I'll try = -compose next time. > >>> 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 emb= edded 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 =E2=80=9Clive= =E2=80=9C. I think the daily graph might be interesting as well. But for the other range= s I agree with you, they can be left out. >>> The refresh interval is based on the graph's time range, i.e. hourly grap= hs are refreshed more often. >>> Reloading should stop in case of a network error, but I haven't fully tes= ted this yet. >>> >>> This patch adds a checkbox to the GUI settings (default: disabled), trans= lations, ... >>> If you just want to try the javascript, you can simply paste it to the ht= ml 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 =3D = Password not set. >>> WARNING: untranslated string: password too short =3D Password is too shor= t. >>> WARNING: untranslated string: passwords do not match =3D Passwords do not= match. >>> WARNING: untranslated string: percentage =3D Percentage >>> +WARNING: untranslated string: periodically reload graphs =3D Periodicall= y refresh RRD graphs >>> WARNING: untranslated string: persistent =3D Persistent >>> WARNING: untranslated string: pfs yes no =3D Perfect Forward Secrecy (PFS) >>> WARNING: untranslated string: pkcs12 file password =3D PKCS12 File Passwo= rd >>> 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 =3D Repo= sitory >>> WARNING: untranslated string: pakfire tree stable =3D Stable >>> WARNING: untranslated string: pakfire tree testing =3D Testing >>> WARNING: untranslated string: pakfire tree unstable =3D Unstable >>> +WARNING: untranslated string: periodically reload graphs =3D Periodicall= y refresh RRD graphs >>> WARNING: untranslated string: please reboot to apply your changes =3D Ple= ase reboot to apply your changes >>> WARNING: untranslated string: pptp netconfig =3D My Net Config >>> WARNING: untranslated string: pptp peer =3D 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_sysl= og =3D unknown string >>> WARNING: untranslated string: guardian no entries =3D unknown string >>> WARNING: untranslated string: guardian service =3D unknown string >>> WARNING: untranslated string: pakfire ago =3D ago. >>> +WARNING: untranslated string: periodically reload graphs =3D Periodicall= y refresh RRD graphs >>> WARNING: untranslated string: route config changed =3D unknown string >>> WARNING: untranslated string: routing config added =3D unknown string >>> WARNING: untranslated string: routing config changed =3D 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 =3D Repo= sitory >>> WARNING: untranslated string: pakfire tree stable =3D Stable >>> WARNING: untranslated string: pakfire tree testing =3D Testing >>> WARNING: untranslated string: pakfire tree unstable =3D Unstable >>> +WARNING: untranslated string: periodically reload graphs =3D Periodicall= y refresh RRD graphs >>> WARNING: untranslated string: please reboot to apply your changes =3D Ple= ase reboot to apply your changes >>> WARNING: untranslated string: pptp netconfig =3D My Net Config >>> WARNING: untranslated string: pptp peer =3D 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 =3D Repo= sitory >>> WARNING: untranslated string: pakfire tree stable =3D Stable >>> WARNING: untranslated string: pakfire tree testing =3D Testing >>> WARNING: untranslated string: pakfire tree unstable =3D Unstable >>> +WARNING: untranslated string: periodically reload graphs =3D Periodicall= y refresh RRD graphs >>> WARNING: untranslated string: please reboot to apply your changes =3D Ple= ase reboot to apply your changes >>> WARNING: untranslated string: pptp netconfig =3D My Net Config >>> WARNING: untranslated string: pptp peer =3D 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 =3D Repo= sitory >>> WARNING: untranslated string: pakfire tree stable =3D Stable >>> WARNING: untranslated string: pakfire tree testing =3D Testing >>> WARNING: untranslated string: pakfire tree unstable =3D Unstable >>> +WARNING: untranslated string: periodically reload graphs =3D Periodicall= y refresh RRD graphs >>> WARNING: untranslated string: please reboot to apply your changes =3D Ple= ase reboot to apply your changes >>> WARNING: untranslated string: pptp netconfig =3D My Net Config >>> WARNING: untranslated string: pptp peer =3D 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 =3D Repo= sitory >>> WARNING: untranslated string: pakfire tree stable =3D Stable >>> WARNING: untranslated string: pakfire tree testing =3D Testing >>> WARNING: untranslated string: pakfire tree unstable =3D Unstable >>> +WARNING: untranslated string: periodically reload graphs =3D Periodicall= y refresh RRD graphs >>> WARNING: untranslated string: please reboot to apply your changes =3D Ple= ase reboot to apply your changes >>> WARNING: untranslated string: pptp netconfig =3D My Net Config >>> WARNING: untranslated string: pptp peer =3D 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 =3D Repo= sitory >>> WARNING: untranslated string: pakfire tree stable =3D Stable >>> WARNING: untranslated string: pakfire tree testing =3D Testing >>> WARNING: untranslated string: pakfire tree unstable =3D Unstable >>> +WARNING: untranslated string: periodically reload graphs =3D Periodicall= y refresh RRD graphs >>> WARNING: untranslated string: please reboot to apply your changes =3D Ple= ase reboot to apply your changes >>> WARNING: untranslated string: processor vulnerability mitigations =3D Pro= cessor Vulnerability Mitigations >>> WARNING: untranslated string: ptr =3D 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=3D''; >>> >>> >>> $cgiparams{'SPEED'} =3D 'off'; >>> +$cgiparams{'RELOADGRAPHS'} =3D 'off'; >>> $cgiparams{'WINDOWWITHHOSTNAME'} =3D 'off'; >>> $cgiparams{'REFRESHINDEX'} =3D 'off'; >>> $cgiparams{'ACTION'} =3D ''; >>> @@ -88,6 +89,7 @@ if ($cgiparams{'ACTION'} eq "$Lang::tr{'save'}") >>> $mainsettings{'WINDOWWITHHOSTNAME'} =3D $cgiparams{'WINDOWWITHHOSTNAME= '}; >>> $mainsettings{'PPPUPDOWNBEEP'} =3D $cgiparams{'PPPUPDOWNBEEP'}; >>> $mainsettings{'SPEED'} =3D $cgiparams{'SPEED'}; >>> + $mainsettings{'RELOADGRAPHS'} =3D $cgiparams{'RELOADGRAPHS'}; >>> $mainsettings{'THEME'} =3D $cgiparams{'theme'}; >>> $mainsettings{'REFRESHINDEX'} =3D $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'} =3D 'on'; >>> } >>> + if($mainsettings{'RELOADGRAPHS'}) { >>> + $cgiparams{'RELOADGRAPHS'} =3D $mainsettings{'RELOADGRAPHS'}; >>> + } else { >>> + $cgiparams{'RELOADGRAPHS'} =3D 'off'; >>> + } >>> } >>> >>> # Default settings >>> @@ -134,6 +141,7 @@ if ($cgiparams{'ACTION'} eq "$Lang::tr{'restore defau= lts'}") >>> $cgiparams{'PPPUPDOWNBEEP'} =3D 'on'; >>> $cgiparams{'REFRESHINDEX'} =3D 'off'; >>> $cgiparams{'SPEED'} =3D 'on'; >>> + $cgiparams{'RELOADGRAPHS'} =3D 'off'; >>> $cgiparams{'THEME'} =3D 'ipfire'; >>> } >>> >>> @@ -153,6 +161,10 @@ $checked{'SPEED'}{'off'} =3D ''; >>> $checked{'SPEED'}{'on'} =3D ''; >>> $checked{'SPEED'}{$cgiparams{'SPEED'}} =3D "checked=3D'checked'"; >>> >>> +$checked{'RELOADGRAPHS'}{'off'} =3D ''; >>> +$checked{'RELOADGRAPHS'}{'on'} =3D ''; >>> +$checked{'RELOADGRAPHS'}{$cgiparams{'RELOADGRAPHS'}} =3D "checked=3D'che= cked'"; >>> + >>> &Header::openpage($Lang::tr{'gui settings'}, 1, ''); >>> &Header::openbigbox('100%', 'left', '', $errormessage); >>> >>> @@ -179,6 +191,10 @@ print <>> >>> $Lang::tr{'show ajax speedmeter in footer'} >>> >>> + >>> + >>> + $Lang::tr{'periodically reload graphs'} >>> + >>> >>> >>> $Lang::tr{'languagepurpose'} > 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. I don't see any disadvantages, I just thought that some users might not like = it. > >>> diff --git a/html/html/themes/ipfire/include/functions.pl b/html/html/the= mes/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 >>> +; >>> +} >>> + >>> print <>> >>> >>> diff --git a/html/html/themes/ipfire/include/js/reloadGraphs.js b/html/ht= ml/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 = # >>> +# = # >>> +# 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 . = # >>> +# = # >>> +########################################################################= #####*/ >>> + >>> +var graphs_reloadTimers =3D new Map(); >>> + >>> +//graph onload event: graph is now loaded, start reload timer >>> +function graphs_onloadEvent(graphObj, startTimer =3D true) { >>> + if(! graphObj) { >>> + return; //graph iframe object is missing >>> + } >>> + >>> + //get graph parameters from iframe url: [url]?[graph name]?[time ran= ge] >>> + let url =3D graphObj.contentWindow.location.href; >>> + if((! url) || (url =3D=3D=3D 'about:blank')) { >>> + url =3D graphObj.src; >>> + } >>> + url =3D url.split('?', 3); >>> + let graphRange =3D url.pop(); >>> + let graphName =3D 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 =3D 60; //default: 60s >>> + if(graphRange =3D=3D=3D 'hour') { >>> + interval =3D 10; >>> + } else if(graphRange =3D=3D=3D 'month' || graphRange =3D=3D=3D '= year') { >> Do not think that=E2=80=99s this makes any sense here. Should not change a= pixel in this graphs :-). Why is day not listed here ? Daily graphs would use the default (60s). I made that up on a sunday evening,= there is certainly room for improvement here :) >>> + interval =3D 180; >>> + } >>> + >>> + //start timeout and store timer id >>> + let timer =3D 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 fun= ction which creates the graph, should be makegraphs() or something like that? Yes, but I think this was the easiest way to do it without changing the exist= ing iframe. If we remove the iframe, I would do it as you suggested. >>> +//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' =3D> 'PC hinzuf=C3=BCgen', >>> 'pdc options' =3D> 'PDC Optionen', >>> 'percentage' =3D> 'Prozent', >>> +'periodically reload graphs' =3D> 'RRD Graphen periodisch neu laden', >>> 'persistent' =3D> 'Dauerhaft', >>> 'pfs yes no' =3D> 'Perfect Forward Secrecy (PFS)', >>> 'phase1 group' =3D> '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' =3D> 'Add workstation', >>> 'pdc options' =3D> 'PDC options', >>> 'percentage' =3D> 'Percentage', >>> +'periodically reload graphs' =3D> 'Periodically refresh RRD graphs', >>> 'persistent' =3D> 'Persistent', >>> 'pfs yes no' =3D> 'Perfect Forward Secrecy (PFS)', >>> 'phase1 group' =3D> 'Phase1 Group', >>> --=20 >>> 2.27.0.windows.1 >>> >> Some generic thoughts. Would it not be better to remove the iFrame and mak= e the links for hour/day/month/year firing some JavaScripts which reloads/cha= nges 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 oft= en is best to reload :) As I mentioned above, I would also prefer to revise that. But I think users w= ithouth JS enabled will then no longer be able to change the graph range. What is you attitude towards JS in general? I think for such "bonus" features= like interactive graphs we can require JS and break compatibility, would you= agree? > >> Thanks for the nice patch. It is easy to review :-) >> >> Greetings Jonatan > > -Michael Greetings, Leo --===============6778556937702938558==--