Hi,
On 2015-09-25 09:43, Nathan Scott wrote:
>>>
>>> https://myllynen.fedorapeople.org/pmrep.py
>>> https://myllynen.fedorapeople.org/pmrep.conf
>>>
>>> Comments and feedback are welcome.
>>
>> I downloaded and tried quickly. Everything seems to work as you
>> described. I'll play around more as I have time, but I think the
>> functionality of specifying groups of metrics for reuse will be really
>> useful!
>
> +1 - this is looking like a nice step forward Marko, very very promising!
thanks for the encouraging feedback.
> Few suggestions on first reading, mainly related to the config file...
>
> 1. This syntax (from sample rep.conf) I'm finding a bit cryptic:
> mem.vmstat.pswpin=pswpin/s,,,10
> mem.vmstat.pswpout=pswpout/s,,,10
>
> I wonder if turning this inside-out might make it easier and more readable,
> keeping the ini-style format but using labels for metrics, as in
>
> pswpin = mem.vmstat.pswpin
> pswpin.width = 10
> pswpin.label = "pswpin/s"
Great idea! The syntax was something I came up with very early on and
stick with it while concentrating on everything else. I think the
comma-separated syntax would still be useful to be supported to allow
adding customized metrics more quickly on the command line.
I'm a bit busy with something else at the moment but I'll try to look at
this in a week or two.
> 2. The derived metrics syntax might become a bit simpler too this way? e.g.
> in
> the example config
>
> derived=mem.util.allcache=mem.util.cached+mem.util.slab,kernel.all.cpu.alluser=kernel.all.cpu.user+kernel.all.cpu.nice
>
> might be more clearly written as
>
> cached = mem.util.allcache
> cached.formula = "mem.util.cached + mem.util.slab"
Yep, looks clean and clear.
> 3. over to the script itself, some of the TODO items:
> # XXX allow defining instances to display
>
> This one might become easier with the above syntax tweak? e.g.
> nfsreads = nfs.server.reqs
> nfsreads.instance = 'read'
> nfswrites = nfs.server.reqs
> nfswrites.instance = 'write'
Yes, looks good.
> and this one interests me greatly... :)
> # XXX modularize code to allow custom output plugins
>
> I think what you will want to end up with here is an API
Yeah. I'd like to see something that's easy for casual administrators
and other non-programmers to use but also something slightly more
sophisticated than the current code.
> it would be great if most of the code in the script was abstracted
> out into a module, and pmrep(1) uses that API - custom plugins would
> just be scripts like pmrep, but a bit more specialised.
Perhaps we could have a couple of real modules written to validate the
interface, for example the already provided CSV and stdout and the
perhaps pcp2graphite or pcp2zabbix (which would now be near-trivial with
pmrep + zbxsend, see https://pypi.python.org/pypi/zbxsend).
> You'll find pcp.pmcc provides good convenience classes that you could
> use in building such an API (it has a grouping concept too & there's a
> bunch of tests already, so you might find it easier to extend both the
> code & tests :) - so, I really encourage you to look closely there and
> see if it can be used, early on, rather than implementing a separate
> API/module down the track for this - lets design it in from day one.
I looked at this a bit more, basics look good but some parts don't have
any in-tree users at all, so perhaps we'd need a bit of hands-on
experience to see how well these fit together.
> ps: oh, several small coding things, but lets talk through the above
> first. The pmDebug issue - should be fully supported via pmOptions;
> so no code needed in your script (just noticed your comment there re
> not being able to set it? shouldn't need to, libpcp does it all).
The pmDebug snippet was actually just a copypaste from pcp2graphite, I
checked that -D9 worked and didn't pay any attention to it later on. So
if you could adjust pcp2graphite as needed, I'll then copypaste from it
again :-)
Thanks,
--
Marko Myllynen
|