Watchdog variables could be sent as additional GELF properties

Thanks, I'll take a look at this patch.

Hi, sorry for the delay. The patch seems OK, but I can see a problem if one of the keys in $entry['variables'] gets set to one of the fields we're already setting (e.g. "Uid" or "Request_uri"), then this would overwrite them.

I suppose one way around this is to do the loop to set variables before we set the "standard" fields. Thoughts?

Hi Elliot, I think my concerns would be alleviated if we simply move the block setting the additional fields to be above where we set some "default" additional fields.

I'd also like to add a small bit of text to the README explaining the usage of setting these additional fields. Would you mind taking a stab at that and re-rolling the patch?

Why not allow these variables to override the default ones? Doing it this way allows for me to force/override some of the default variables.

If we're going to allow overriding of stuff like "server host", etc., then I'd rather see that done as part of the admin configuration. Allowing calling modules to overwrite these variables seems like we're opening ourselves up to some potential bad coding mistakes. I'd rather see the GELF module set some standard "additional" fields, and then allow users to add their own custom fields as desired.

Do you have some strong need to override some of the variables as it is currently?

Variables in watchdog can be used as placeholders for message. So they don't have to be added to gelf structure.
Why not to add a special variable in $variable array ?

      if (is_array($entry['variables']) && array_key_exists("gelf_fields",$entry['variables'])) {
        foreach ($entry['variables']["gelf_fields"] as $key => $value) {
          $gelf->setAdditional($key, $value);
        }
      }

Yes but $variable is used for placeholders like (from user.module)

watchdog('user', 'Login attempt failed for %user.', array('%user' => $form_state['values']['name']));

Do you want to add an new field "%user" to Graylog ? I'm not sure.
So, if think this is better to add a new var with all desired graylog's fields like this

watchdog('user', 'Login attempt failed for %user.', array('%user' => $form_state['values']['name'], 'gelf_fields' => array("myfield" => "myvalue")));

So only myfield will be added to graylog.

I think a better way to handle the watchdog variables is to prefix the key with something (wv_ for example) so that they don't collide with the additional parameters we're setting manually. Just placing the block at the top means we'll never record the values that do clash and that seems like a bad thing.

I've attached a new patch with the proposed change for D7.