Hi guys,
----- Original Message -----
> >
> >
> > I've only tested this on RHEL 7 / PCP 3.10.6 / Python 2.7 so use at your
> > own risk on other platforms.
> >
> > 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!
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"
... this is a bit more object-oriented (& pythonic I guess), and will allow
additional fields to be added without changing config format. "pswpin/s,,,10"
also has an implied ordering (which we'd now do away with), and redundant use
of commas/empty-string to set defaults.
Error reporting will be "nicer" too, because we'd have a label in the config
to use in any error messages (both config parsing time & fetching/reporting
values later).
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"
alluser = kernel.all.cpu.alluser
alluser.formula = "kernel.all.cpu.user + kernel.all.cpu.nice"
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'
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 - in fact,
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.
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.
cheers.
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).
--
Nathan
|