ffnord / ffmap-backend

THIS PROJECT DOESN'T HAVE A MAINTAINER!
Other
20 stars 56 forks source link

support node down areas for graphs over longer timespan #83

Closed ecsv closed 8 years ago

ecsv commented 8 years ago

It was noticed that the downtime marks are missing for graphs over longer timespans (e.g. over 7 days instead of just 1d). This seems to be caused by the averaging function of the rrdtool.

Right now the down time area is defined as

'CDEF:d=clients,UN,maxc,UN,1,maxc,IF,*',

So d is defined as max(1,maxc) whenever clients is not defined. But clients seems to become a defined value when the averaging happens.

Right now I am using upstate as workaround for this:

 'CDEF:d=upstate,UN,0,upstate,IF,0,EQ,clients,UN,MAX,maxc,UN,1,maxc,IF,1,MAX,*'

This might have other downsides and thus I haven't created a pull request for this change. The full testcall used right now is:

rrdtool 'graph' /opt/freifunk/meshviewer/data/nodes/test.png \
    '-s' '-7d' \
    '-w' '800' \
    '-h' '400' \
    'DEF:clients=/opt/freifunk/meshviewer/data/nodedb/f4f26dc1f80a.rrd:clients:AVERAGE' \
    'DEF:upstate=/opt/freifunk/meshviewer/data/nodedb/f4f26dc1f80a.rrd:upstate:AVERAGE' \
    'VDEF:maxc=clients,MAXIMUM' \
    'CDEF:c=0,clients,ADDNAN' \
    'CDEF:d=upstate,UN,0,upstate,IF,0,EQ,clients,UN,MAX,maxc,UN,1,maxc,IF,1,MAX,*' \
    'AREA:c#0F0:up\\l' \
    'AREA:d#F00:down\\l' \
    'LINE1:c#00F:clients connected\\l'

Here are is an example from node 60e327fb5176 (before the change, after the change)

original patched testdata.zip

ecsv commented 8 years ago

@sunbox this is most likely the problem which was spotted on the FFC/FFV meshviewer installation

SunboX commented 8 years ago

👍

jplitza commented 8 years ago

I'd consider using the upstate as semantically cleaner, and always assumed it would be used for exactly this purpose. However, your new command still incorporates whether clients is unknown, right? Why is that?

And please use pull requests if you propose code. Opening a pull request doesn't necessarily mean “This is finished, please pull”, especially if you explicitly state that the code isn't supposed to be merged yet, but it eases code review greatly. Github is all about not having to deal with patch files anymore.

Edit: And @SunboX, please don't add comments just for a thumbs up. You can add a reaction for that using the smiley button on the upper right part of every comment/issue/whatever.

ecsv commented 8 years ago

@jplitza This was not meant as code contribution but to discuss what I consider to be wrong.

And yes, clients could be dropped. Still had it in my test because I don't know if there is some strange situation where clients is required

'CDEF:d=upstate,UN,0,upstate,IF,0,EQ,maxc,UN,1,maxc,IF,1,MAX,*'
jplitza commented 8 years ago

Let's further discuss in #86, closing here.