[Dnsmasq-discuss] ubus and metrics

Simon Kelley simon at thekelleys.org.uk
Mon Apr 23 23:07:30 BST 2018


That's great. Thank you.

Can I make another request? It would be good (and consistent) to have a
dnsmasq option for UBus analogous to --enable-dbus, called --enable-ubus
so that dnsmasq doesn't try and connect to the Ubus unless that's
explicitly requested, even if the code is compiled in. Like the DBUS
version, it should raise a fatal error if the options is set but the
UBus code is not compiled in.


Cheers,


Simon.

On 21/04/18 17:40, Julian Kornberger wrote:
> On 20.04.2018 23:04, Simon Kelley wrote:
>> 1) Conditional compilation.
> HAVE_METRICS has been removed.
> 
>> 2) In config.h, you document HAVE_UBUS, but don't include it in the
>> features string, whilst you include HAVE_METRIC in the features string,
>> but don't document it.
> Has been added.
> 
>> 3) The documentation for the GetMetrics DBus method is almost
>> non-existent. The documentation for the UBus interface is entirely
>> non-existent.
> I asked the original author to write some documentation.
> 
>> 4) The list of metrics is in metrics.h and the list of their names is in
>> metrics.c and the two have to be kept in sync. Can this be done some way
>> that at least keeps the two in the same source file, and preferably
>> allows the addition of a new metric by adding one line?
> I don't have any idea how to achieve this. I added a hint to the enum.
> 
>> 5) What is the motivation for the metrics you're collecting? Is that
>> actually useful information? What uses it?
> Sometimes there are clients that behave in a strange way. Like acquiring
> a same lease every second and ignoring the reply. Metrics help to debug
> these kind of problems. If you observe a specific message counter to
> increase rapidly (like NAKs), then something might be wrong in your
> network.
> 
> You get a graph like this:
> https://grafana.ffhb.de/d/YYHhL-Wmz/dhcp?orgId=1&from=1524212492387&to=1524253637769
> 
> 
>> 6)
>> Some files have
>>
>> METRICS_INCREMENT(<metric>);
>>
>> and some have
>>
>> #ifdef HAVE_METRICS
>>       metrics[<metric>]++;
>> #endif
>>
>> Any good reason for that?
> METRICS_INCREMENT is now used everwhere.
> 
>> 7) Apart from reading metrics, there seems to be code to broadcast some
>> events over UBus. But no justification for doing this and no
>> documentation of what is done.
>>
>> 8) The convention in dnsmasq is that global data structures are part of
>> the "daemon" structure, so
>>
>> metrics[<metric>]
>>
>> should be
>>
>> daemon->metrics[metric]
>>
>> and the array should be allocated and initialised appropriately.
> Has been changed
> 
>> 9) Dnsmasq already collects some metrics about DNS forwarder operation,
>> which are dumped to the log when the process receives SIGUSR1. Should
>> this new metrics system embrace and extend the existing one, rather than
>> ignoring it?
> I removed the old metric variables and added new fields to the metrics
> struct.
> ubus and dbus now also return the already existing metrics.
> 
> Regards
> Julian Kornberger
> 



More information about the Dnsmasq-discuss mailing list