Hi Ken,
On 2016-02-09 21:55, Ken McDonell wrote:
>
> Sorry for your pain ... it should not be like this ... 8^)
No worries, I'm still learning :)
> On 09/02/16 18:43, Marko Myllynen wrote:
>>
>> Please consider the test case below:
>> ...
>> 3) If the config file contains only some garbage like "asd" then
>> I see errors printed while in pmLoadDerivedConfig but it returns
>> zero - this looks like an obvious bug
>
> The problem here is that the argument to pmLoadDerivedConfig may be a
> file name, a directory name or a PATH-style list of file/directory
> names. And each file may contain more than one derived metric definition.
>
> So the concept of an "error" here is a bit tricky ...
> - missing file or directory in the PATH-style list ... probably not an
> error, should be silently ignored
> - file that cannot be accessed during recursive descent ... probably not
> an error
> - bad metric spec in a file ... again, not sure as there may be other
> valid definitions in the same file (consider a commonly used derived
> metric file that is expected to work mostly work even if some of the
> definitions involve metrics or PMDAs that might not be available in the
> current context, especially an archive)
Ok, this makes sense.
> This is why pmLoadDerivedConfig() returns the "will be the number of
> derived metrics loaded" (from the man page) .. in your case this would
> be 0.
OTOH the man page also states "a value less than zero in the case of an
error," perhaps that could be clarified a bit (or parts of the above
information incorporated to the man page)?
>> 4) If the config file contains a valid derived metric definition
>> then everything works until pmLookupDesc fails. Shouldn't it work
>> with derived metrics as well?
>
> This is a problem in the test code. Buried at the end of the
> pmRegisterDerived(3) man page (not the pmLoadDerivedConfig(3) man page
> unfortunately) is this caveat ...
>
> pmRegisterDerived does not apply retrospectively to any open contexts,
> so the normal use would be to make all calls to pmRegisterDerived
> (possibly via pmLoadDerivedConfig(3)) and then call pmNewContext(3).
A-ha!
> I'd welcome any suggestions as to how this important piece of
> information could be made more visible.
After reading the above now it's of course obvious.. Perhaps a short
note in the first section of pmLoadDerivedConfig(3) along the lines
"Note that pmLoadDerivedConfig needs to be called before creating a new
context" or something like that would be enough.
Thanks,
--
Marko Myllynen
|