From mboxrd@z Thu Jan  1 00:00:00 1970
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
Message-ID: <CED2ADAA-B1D2-421D-A6AD-3C86A47D8F71@ipfire.org>
In-Reply-To: <0C074083-06C7-42B9-981C-57161C343D30@ipfire.org>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="===============5854852272050483774=="
List-Id: <development.lists.ipfire.org>

--===============5854852272050483774==
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable

Hello,

> On 10 Mar 2021, at 12:29, Jonatan Schlag <jonatan.schlag(a)ipfire.org> wrot=
e:
>=20
> Hi,
>=20
>> Am 08.03.2021 um 20:21 schrieb Leo-Andres Hofmann <hofmann(a)leo-andres.de=
>:
>>=20
>> =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. Shou=
ld 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 th=
e image and JS wasn=E2=80=99t an option back then. The <a> tag allowed to spe=
cify 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 t=
ags. 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.
>>=20
>> Signed-off-by: Leo-Andres Hofmann <hofmann(a)leo-andres.de>
>> ---
>>=20
> =E2=AC=87=EF=B8=8F This part which is not really part of the commit message=
, can be put before an patch using the =E2=80=94compose option :-).

Yes this is brilliant :)

>> Hi,
>>=20
>> 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 embe=
dded in an iframe.
>> I wrote a little javascript that periodically reloads these graphs.
>=20
> Would it not be sufficient to reload only the hourly graph? So all others h=
ave a range where I would not assume somebody to monitor them =E2=80=9Clive=
=E2=80=9C.
>> The refresh interval is based on the graph's time range, i.e. hourly graph=
s are refreshed more often.
>> Reloading should stop in case of a network error, but I haven't fully test=
ed this yet.
>>=20
>> This patch adds a checkbox to the GUI settings (default: disabled), transl=
ations, ...
>> If you just want to try the javascript, you can simply paste it to the htm=
l <head> in=20
>> html/themes/ipfire/include/functions.pl. Be sure to escape the '$' then.
>>=20
>> Best regards
>> Leo
>>=20
>> 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
>>=20
>> 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 P=
assword not set.
>> WARNING: untranslated string: password too short =3D Password is too short.
>> 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 Periodically=
 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 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 =3D Repos=
itory
>> 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 Periodically=
 refresh RRD graphs
>> WARNING: untranslated string: please reboot to apply your changes =3D Plea=
se 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_syslo=
g =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 Periodically=
 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 Repos=
itory
>> 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 Periodically=
 refresh RRD graphs
>> WARNING: untranslated string: please reboot to apply your changes =3D Plea=
se 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 Repos=
itory
>> 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 Periodically=
 refresh RRD graphs
>> WARNING: untranslated string: please reboot to apply your changes =3D Plea=
se 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 Repos=
itory
>> 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 Periodically=
 refresh RRD graphs
>> WARNING: untranslated string: please reboot to apply your changes =3D Plea=
se 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 Repos=
itory
>> 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 Periodically=
 refresh RRD graphs
>> WARNING: untranslated string: please reboot to apply your changes =3D Plea=
se 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 Repos=
itory
>> 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 Periodically=
 refresh RRD graphs
>> WARNING: untranslated string: please reboot to apply your changes =3D Plea=
se reboot to apply your changes
>> WARNING: untranslated string: processor vulnerability mitigations =3D Proc=
essor 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'';
>>=20
>>=20
>> $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';
>> +    }
>> }
>>=20
>> # Default settings
>> @@ -134,6 +141,7 @@ if ($cgiparams{'ACTION'} eq "$Lang::tr{'restore defaul=
ts'}")
>>   $cgiparams{'PPPUPDOWNBEEP'} =3D 'on';
>>   $cgiparams{'REFRESHINDEX'} =3D 'off';
>>   $cgiparams{'SPEED'} =3D 'on';
>> +    $cgiparams{'RELOADGRAPHS'} =3D 'off';
>>   $cgiparams{'THEME'} =3D 'ipfire';
>> }
>>=20
>> @@ -153,6 +161,10 @@ $checked{'SPEED'}{'off'} =3D '';
>> $checked{'SPEED'}{'on'} =3D '';
>> $checked{'SPEED'}{$cgiparams{'SPEED'}} =3D "checked=3D'checked'";
>>=20
>> +$checked{'RELOADGRAPHS'}{'off'} =3D '';
>> +$checked{'RELOADGRAPHS'}{'on'} =3D '';
>> +$checked{'RELOADGRAPHS'}{$cgiparams{'RELOADGRAPHS'}} =3D "checked=3D'chec=
ked'";
>> +
>> &Header::openpage($Lang::tr{'gui settings'}, 1, '');
>> &Header::openbigbox('100%', 'left', '', $errormessage);
>>=20
>> @@ -179,6 +191,10 @@ print <<END
>>    <td><input type=3D'checkbox' name=3D'SPEED' $checked{'SPEED'}{'on'} /><=
/td>
>>    <td>$Lang::tr{'show ajax speedmeter in footer'}</td>
>> </tr>
>> +<tr>
>> +    <td><input type=3D'checkbox' name=3D'RELOADGRAPHS' $checked{'RELOADGR=
APHS'}{'on'} /></td>
>> +    <td>$Lang::tr{'periodically reload graphs'}</td>
>> +</tr>
>> <tr>
>>    <td>&nbsp;</td>
>>    <td>$Lang::tr{'languagepurpose'}</td>

Does this actually need configuration or can we not simply enable this for ev=
eryone?

What is the downside? If the browser has JS disabled, simply nothing would ha=
ppen.

>> diff --git a/html/html/themes/ipfire/include/functions.pl b/html/html/them=
es/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
>> ;
>> }
>>=20
>> +if ($settings{'RELOADGRAPHS'} eq 'on') {
>> +print <<END
>> +    <script src=3D"/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/htm=
l/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/>.  =
     #
>> +#                                                                        =
     #
>> +#########################################################################=
####*/       =20
>> +
>> +var graphs_reloadTimers =3D new Map();
>> +
>> +//graph onload event: graph is now loaded, start reload timer
>> +function graphs_onloadEvent(graphObj, startTimer =3D true) {       =20
>> +    if(! graphObj) {
>> +        return; //graph iframe object is missing
>> +    }
>> +
>> +    //get graph parameters from iframe url: [url]?[graph name]?[time rang=
e]
>> +    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 'y=
ear') {
> 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 ?=20
>> +            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);
>> +    }
>> +}
>> +
>=20
> Would it no be easier to attach this directly to a graph so inside the func=
tion 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' =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
>>=20
>=20
> 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/chan=
ges the graphs? And then when we click hour start this script to reload the g=
raphs periodically?

Agreed.

I very much like the idea and I think this is an opportunity to clean up a li=
ttle 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 :-)
>=20
> Greetings Jonatan


-Michael

--===============5854852272050483774==--