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

[Gergely Nagy]

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.

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.

[...]
> +                  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.

-- 
|8]