[syslog-ng] [PATCH 1/2] [dbparser] inherit nvpairs from parent in correlation action

[Balint Kovacs]

On 11/10/2012 04:18 PM, Gergely Nagy wrote:
> balint.kovacs at balabit.com writes:
>
>> This patch implements an inherit-name-value-pairs attribute for
>> message type actions is patterndb correlation. In case this is set to
>> TRUE, the triggering log message is cloned and all name-value pairs
>> are automagically copied to the emitted message. The values set in the
>> values XML subtree overwrite the original values. In case it is set to
>> FALSE or unspecified, a new message is generated and only the values
>> set manually are added to it.
> This looks simple, and is backwards compatible, I like it. Picked it to
> my feature/3.4/dbparser/corellation-improvements branch for further
> testing.
Kewl, thanks.

> See my comments below, though.
>
>> diff --git a/modules/dbparser/patterndb.c b/modules/dbparser/patterndb.c
>> index 8720ea0..168a24c 100644
>> --- a/modules/dbparser/patterndb.c
>> +++ b/modules/dbparser/patterndb.c
> [...]
>> +void
>> +pdb_action_set_inheritance(PDBAction *self, const gchar *inherit_nvpairs, GError **error)
>> +{
>> +  if (strcmp(inherit_nvpairs, "TRUE") == 0)
>> +    self->inherit_nvpairs = TRUE;
>> +  else if (strcmp(inherit_nvpairs, "FALSE") == 0)
>> +    self->inherit_nvpairs = FALSE;
>> +  else
>> +    g_set_error(error, 0, 1, "Unknown inheritance type: %s", inherit_nvpairs);
>> +}
> Although there is no similar thing in patterndb yet (boolean flags of
> this kind), the xs:boolean type of XML can be "true", "false", "0", or
> "1", so we should - in my opinion - accept all of those, case
> insensitively. A simple way would be something like:
>
> if (inherit_nvpairs[0] == 't' || inherit_nvpairs[0] == 'T' ||
>      inherit_nvpairs[0] == '1')
>    self->inerhit_nvpairs = TRUE;
> else if (inherit_nvpairs[0] == 'f' || inherit_nvpairs[0] == 'F' ||
>           inherit_nvpairs[0] == '0')
>    self->inherit_nvpairs = FALSE;
> else
>    g_set_error(error, 0, 1, "Unknown inheritance type: %s", inherit_nvpairs);
>
> What do you think?
>
> This'd allow things like inherit-name-value-pairs="FOR SURE" (and parse
> it as false), but I can live with that, as long as the valid cases are
> parsed correctly.
>
> This should be added to the XSD aswell, so that patterndb files using it
> can still pass validation.
>
> [...]
Thanks for pointing this out, this is of course more compatible, 
intuitive, so in short - better.

>> +                  if (action->inherit_nvpairs)
>> +                    {
>> +                      LogPathOptions path_options = LOG_PATH_OPTIONS_INIT;
>> +                      path_options.ack_needed = FALSE;
>> +                      genmsg = log_msg_clone_cow(msg, &path_options);
>> +                    }
>> +                  else
>> +                    {
>> +                      genmsg = log_msg_new_empty();
>> +                      genmsg->flags |= LF_LOCAL;
>> +                      genmsg->timestamps[LM_TS_STAMP] = msg->timestamps[LM_TS_STAMP];
>> +                    }
> As you mentioned in person, valgrind was a bit upset about the
> log_msg_clone_cow() here, I'll investigate that once I've written some
> test cases.
>
> Which leads us to the next point: tests! I wants them. But since I need
> to learn patterndb aswell, I'll write them.
>
Great, thanks again, I'm quite short on time these days. Let me know if 
I can help.

Balint