On Wed, 2011-04-13 at 14:52 -0500, David Smith wrote:
> After mucking around a bit, I've got a working version of my PCP logger
> PMDA. You can see the result by looking at:
>
> <http://sources.redhat.com/git/gitweb.cgi?p=systemtap.git;a=tree;f=pcp/src/pmdas/logger;h=b69c63b346157a2a6f857ac80cbf1eda5f7be824;hb=HEAD>
>
> Or you can do a git pull (and look at pcp/src/pmdas/logger in the
> resulting tree):
> # git clone git://sources.redhat.com/git/systemtap.git
>
> When installed, the PMDA creates a config file containing a NAME and
> PATH for logfiles to monitor. If PATH ends in '|', the filename is
> interpreted as a command that pipes output to the PMDA.
>
> Each logfile's data appears under logger.perfile.NAME. Each event
> stream can have multiple clients attached to it.
>
> I'd appreciate any comments on the code. Thanks for the help.
David,
Overall this looks like a worthwhile contribution, so looks good.
I've downloaded the source, built the code and taken it for a bit of a
play. Nothing too deep, but here is my feedback
1. building and installing the DSO variant of the PMDA is not going to
work here, as you need configfile from the command line ... so just make
it a daemon-only PMDA
2. usage text does not quite match code
3. diagnostics are a bit too verbose ... can use the existing mechanisms
to include diagsnostics, disable them by default and allow -Dappl0 or
-Dappl1 or -Dappl2 or any combination thereof on the command line to
enable diagnostics (or more exotically allow 'em to be turned on and off
via pmstore and a storable metric)
4. I'm not sure how the "catchup" logic was intended to work with an
existing file ... I used /var/log/messages, and the agent delivered 4096
byte chunks from the start of the file which did not honour any sort of
newline boundaries, and does not chop the input into one event per line
(which may be by design, I'm not sure) ... similarly in non-catchup
mode, multiple new lines in the logfile are returned as a single
record ... perhaps there needs to be some concept of a record delimiter
(newline by default) for the input logfiles?
5. I don't think the multi-client logic is correct, at least in the
"catchup" case, where repeated pminfo -x invocations deliver progressive
4096 byte chunks (even though these are different clients) and two
concurrent pmevent processes do not appear to return the same data (as I
would have expected) and one of them sometimes sees "No events" event
when there is lots of catchup still to be done.
6. Install should use pmda_interface=5 ... I've fixed the bug in
pmdaproc.sh
7. My PMNS after install is ...
logger.numclients
logger.numlogfiles
logger.param_string
logger.perfile.syslog.records
logger.perfile.syslog.numclients
and I think this might be a little more user friendly ...
logger.config.numclients
logger.config.numlogfiles
logger.config.param_string
logger.syslog.records
logger.config.syslog.numclients
although this does mean guarding against the PMNS name of "config" in
the configfile ... perhaps this is not worth changing
I've attached a suggested patch for issues 1., 2., 3. and 6.
Hope this helps.
Cheers, Ken.
patch.logger
Description: Text Data
|