vanished bug numbers in the nagios mantis tracker

[Andreas Ericsson]

On 08/06/2012 05:12 PM, John P. Rouillard wrote:
> In message <501F8326.9000701 at op5.se>,
> Andreas Ericsson writes:
>> On 08/03/2012 05:15 PM, John P. Rouillard wrote:
>>> In message <501BDA9E.8070606 at op5.se>,
>>> Andreas Ericsson writes:
>>>> On 08/03/2012 10:27 AM, Wim Hoekman wrote:
>>>>> On 2012-08-01 10:37, Andreas Ericsson wrote:
>>>>>> This one might resolve itself in Nagios 4, when we change how checks
>>>>>> are run. I won't investigate it further until that's out anyway though.
>>>>> The release of Nagios 4 would be a great time to import the Nagios patch
>>>>> I've submitted a while ago.
>>>>> (Subject: Additional image for action_url in host and service
>>>>> definitions patch)
>>>>>
>>>>> That patch required a change to the object abi, so it was not included
>>>>> in any minor release.
>>>>
>>>> Is there a reason why we just can't make this a custom variable with
>>>> special meaning in the UI?
>>>>
>>>> I'm especially uninterested in patches that move more UI code into the
>>>> core, since that's something that really should be going away rather
>>>> than increase.
>>>
>>> One of the things that frustrated me to no end when creating the
>>> external correlation event broker
>>> (http://www.cs.umb.edu/~rouilj/sec_nagios/nagios_sec_manual.txt) was
>>> that the broker architecture provided no mechanism for the plugin to
>>> add items to the objects and provide parsers for the objects in config
>>> files.
>>
>> That would be pretty awesome, but "adding items to the object" is simply
>> not possible. C-objects are carved in stone once the code is compiled,
>> and nothing but the module will ever know or care about the extra data.
> 
> So add a field that is a pointer to a nagios maintained data store for
> each object. Where each object in the datastore is a key/value (blob
> pointer) pair. "adding items to the object" wasn't meant as add a new
> field. More along the lines of add a new item to a linked list in the
> object.
> 

Why on earth would you prefer O(n) complexity over O(1)? Do you realize
that you're requesting something that's both slower and more complex than
what I proposed?

>>> Custom variables help a little bit but introduce the need to parse the
>>> value on every event which is not acceptable. The value should be
>>> parsed once into a quickly accessible form (e.g. bitfield) and carried
>>> around with the object.
>>>
>> Well, no. You might want to add things other than flags in such variables
>> and you may well have several modules at once competing for the dubious
>> honor of putting extra junk into objects.
> 
> The object carries the data as a linked list, static array of
> pointers.... rather than as a named item in the struct. As long as the
> name for each item is unique there isn't a problem. (e.g. ec_ in my
> examples). Nagios could even provide a register function to allow
> modules to determine which prefixes are already registered so if Eric
> Charles' plugin tries to register ec_ but my event correlation plugin
> already has ec_ loaded, it gets an error message and gets to chose a
> new prefix. Heck nagios could even return a prefix code that is used
> by the module. There are many ways to handle the issue.
> 

Not really, no, because user documentation has to work too or users
won't be able to use Eric Charles' module and yours at the same time.

>>> Are there any plans to add something like:
>>>
>>>    add_new_custom_variable_property(SERVICE_OBJECT, "_ec_active_action", \
>>>       func_parse_ec_active_action);
>>>
>>>     where:
>>>         void * func_parse_ec_active_action(char * property_name,
>>>                           char * property_value)
>>>
>>>    int add_property_value(*service_object, "_ec_active_action", void * valu
>> e)
>>>
>>>    void * get_property(*service_object, "_ec_active_action");
>>>
>>> to allow the brokers to register parsers for specific custom
>>> variables?
>>>
>>
>> No, and in time for Nagios 4 I doubt we'll do that since it doesn't
>> really require a major version bump to be added and I just don't have
>> the time for it right now. Patches are ofcourse welcome, but please
>> make sure to base them on top of latest svn from sourceforge, or the
>> git repo at www.github.com/ageric/nagios
> 
> Well changing the objects to provide a location in which to store this
> info does require a version bump since it changes the layout of all
> the object structures. But once that is done, they are more or less
> futureproof.
> 
> So maybe for nagos 4 we can get a single pointer for
> broker_data. Since all pointers are the same size, defining the
> structure of the data and the api for accessing it can be done anytime
> in the 4.x series.
> 

No, but you can get an id in the object which you can then use to access
an indexed array in constant time for any kind(s) of data your module
wants to store *without* having to traverse a linked list.

In that case, looking up your extra data becomes as simple as

xd = extra_host_data[host->id];

although you'll ofcourse have to set up the indexed array before
you can use it.

> 
>>> I envision the parsed properties (returned as a void *) are stored in
>>> a linked list or array in the object struct and simply store a propery
>>> name and a pointer to some blob returned by the
>>> func_parse... function. The blob is opaque to nagios and only has
>>> meaning to the event brokers.
>>>
>>> This way brokers (and other plugins too) could add new data to the
>>> internal objects without having to break abi compatibility.
>>
>> Well, no, they can't, because if several modules use the same mechanism
>> they'll overwrite each others data or be unable to find their own.
>> Unless you mean for the objects to have some sort of marker for each
>> module, but that's just overly complex.
> 
> Not a marker for each module, but a marker for each data bit. Consider
> (pseudo code):
> 
>    struct extended_attribute {
>        string[10] identifier;
>        void * value;
>        extended_attribute * next;
>        extended_attribute * prev; /* if you want */
>    }
> 

Why a string? Do you really want to do X strcmp()'s for each object
you're dealing with?

> The extended_attributes would be a linked list made available in an
> internal nagios structure using:
> 
>      service_object {
>         void * extended_attribute;
>         all other service object fields;
>      }
> 

That will break horribly. mod_gearman won't know (and sholdn't know)
if the extended_attributes field is used by merlin already or if it's
available, or where in the list of items the mod_gearman data resides,
without looking at all of them (which can be ridiculously slow and is
far more complex than it has to be).

> Nagios provides commands to add/remove/get pointer to items from
> the extended_attribute linked list. This keeps the data with the
> object easily available to all modules (all they need is the nagios
> strcture and the identifier and need to be able to understand the
> structure of the value). All the data is publically available in the
> service, host, .... structure.
> 
> (The above could be augmented by a nagios supplied unique (for this
> running instance of nagios) number/handle that is used as part of the
> retreival process. I think I described such a mechanism when I was
> trying to get the external correlation mechanism into nagios 2 or 3.)
> 
>>> This also neatly (IMHO) works around the issues with serializing the
>>> object to disk as the custom variable mechanism can pass the info
>>> around to only those who are looking for it. In most other
>>> applications, the overhead of parsing the custom variable's value
>>> isn't a huge issue but in the event broker loop....
>>
>> In my mail to Janne Aho, I suggested adding an 'id' field to all
> 
> I'll try to find that email.
> 
>> objects. That would allow modules to utilize indexed arrays or large
>> bitvectors to mark certain properties in objects and look them up in
>> O(1) time without having to compute a hash and deal with collisions.
> 
> But doesn't that require that the object be changed for each module so
> it has it's own array or vector?
> 

No. Each module gets to keep its own data and Nagios doesn't need to
know anything at all about it (although it would be neat if it could
issue a callback and get a key/value vector when printing status data).

>> If we couple that with a mechanism allowing one module to obtain the
>> pointers of another module, any module that wants to know about
>> another module (such as for example livestatus wanting to know which
>> mod_gearman worker executed a particular check) can just grab the
>> ancillary data of that module and look it up in constant time.
>>
>> This means that either mod_gearman has to maintain a key/value vector
>> for livestatus to use (or some other common form that all modules
>> know about), or that livestatus has to know about the other module's
>> dataformat, but it's pretty much the best we can do to allow constant
>> time lookups of ancillary data.
> 
> Agreed if constant time lookup is required. I think that could work in
> my case. Would be ugly to write and annoying to troubleshoot/debug but
> it could be made to work and would provide the fastest lookup option.

How would it be annoying to troubleshoot something so moronically simple
as

  extra_data = some_pointer_array[hst->id];

and why the hell would you *need* to debug something like that? Beyond

  assert(hst->id >= 0 && hst->id < some_pointer_array_size);

there's absolutely *nothing* that can go wrong with code like that, if
we assume the compiler is somewhat sane.

> But I fail to see how that would keep the object abi static while
> allowing multiple modules to add/remove/change data.
> 

Because core nagios doesn't have to care one whit about what modules
do with the id variable. It's *only* purpose is to allow modules to
do constant-time lookups of whatever the module wants to track,
without modules having to know *or care* about other modules' data.

It would ofcourse be nifty if modules *did* know about each other's
data, assuming they explicitly request it, but that's not what you're
talking about so let's ignore that for now.

-- 
Andreas Ericsson                   andreas.ericsson at op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

Considering the successes of the wars on alcohol, poverty, drugs and
terror, I think we should give some serious thought to declaring war
on peace.

------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and 
threat landscape has changed and how IT managers can respond. Discussions 
will include endpoint security, mobile security and the latest in malware 
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/