This is the mail archive of the
systemtap@sourceware.org
mailing list for the systemtap project.
Re: logger PMDA review
- From: Nathan Scott <nathans at aconex dot com>
- To: David Smith <dsmith at redhat dot com>
- Cc: Systemtap List <systemtap at sources dot redhat dot com>, pcp <pcp at oss dot sgi dot com>
- Date: Mon, 18 Apr 2011 09:21:37 +1000 (EST)
- Subject: Re: logger PMDA review
----- Original Message -----
> After mucking around a bit, I've got a working version of my PCP
> logger
> PMDA. You can see the result by looking at:
> ...
> I'd appreciate any comments on the code. Thanks for the help.
Here's my random notes from a quick look through, some overlap
with Ken's...
Install
pmda_interface=4 -> pmdaproc.sh doesn't handle 5?
(can see a case wrt help text - was that it?)
check for valid PMNS names, I stupidly used xxx-yyyy.
(looks like pmda code checks, but maybe Install
could too?) echo $name | grep "^[A-Za-z][A-Za-z0-9]*$"
reserve a domain number - done
Sanity check (pminfo -dfmtT logger)
- no help text (not only just not dynamic ones)
- add a metric exporting name/path mapping?
- add a counter metric with nevents seen?
logger.c
- cannot call exit(1) - in dso case will terminate pmcd
- should have a default config file? dso cannot work atm
(or remove DSO case)
- not sure what param_string intention is? (WIP?)
event.c
- exit on failure of one file is a bit extreme, if
other files ok and file temporarily / not yet available.
(exit vs DSO issue as well).
- comment typo "the we", "which logfiles is up to date"
- use of gettimeofday() - might be better to have option of
parsing a timestamp from the line(s) of file and using that?
- "We can't really use select() on the logfiles... if normal file,"
"select will (continually) return EOF ... So, now we read data "
"inside the event fetch routine."
- pmdaweblog? (see below)
- should definately seek to end of file at the start (commented out
at the moment), else could read gigabytes of data on startup for
large logfiles.
util.c
- start_cmd should be popen(2)? more portable (works on Windows)
(maybe not, due to need to kill it? -- __pmProcessCreate?)
percontext.c
- wonder if some of this should become generic libpcp_pmda code,
similarities with pmdasample's needs.
In general:
- refer pmdaweblog ... similar sort of thing, predates events by ~10
years or so, and is a bit old in the tooth, but code in there might
be useful for reference. the select loop in particular, and also
the regex's pulling out timestamps (IIRC).
- might want to revisit using dynamic metric names versus metric
instances, dynamic names generally a bit more complicated to code.
(removes need to restrict/check names in Install too, although they
do need to be unique)
--
Nathan