hedii / laravel-gelf-logger

A package to send gelf logs to a gelf compatible backend like graylog
MIT License
125 stars 33 forks source link

Breaking due to "id" field #33

Closed fidelix closed 2 years ago

fidelix commented 2 years ago

https://github.com/bzikarsky/gelf-php/blob/master/src/Gelf/MessageValidator.php#L69

Unfortunately this started breaking my queue workers.

The gelf version should be 1.1, and because a lot of exceptions are being thrown, there should be a default handler for this, to rename the "id" additionalField to something like _id instead of throwing exceptions.

hedii commented 2 years ago

Hi, Processors described in the README config file are just for that. Simply write a custom processor. Feel free to reopen or submit a PR if you need.

hedii commented 2 years ago

@fidelix Can you share the exceptions that are being thrown, and maybe your implementation and config?

There should be none, because we use the IgnoreErrorTransportWrapper (ignore any kind of errors) https://github.com/hedii/laravel-gelf-logger/blob/049976a51030071648c1752ba4157ae5b0b0bbfa/src/GelfLoggerFactory.php#L60-L67

fidelix commented 2 years ago

@hedii this is the exception:

https://github.com/bzikarsky/gelf-php/blob/master/src/Gelf/MessageValidator.php#L69

Pretty easy to reproduce, you just need to log a message with the "id" field: Log::warning("Something", ['id' => 1]);

Using gelf 1.1 would solve this particular exception, apparently.

hedii commented 2 years ago

You can now use RenameIdFieldProcessor if you want to prevent exceptions using an id key.

Check the updated config example in the README.

Also I don't get your because a lot of exceptions are being thrown

Are they related to this package?

fidelix commented 2 years ago

There were a couple of other exceptions being thrown (and the requests failing), I think due to a failure in graylog, which was out of storage or something.

I don't think they're related to this package, but I was surprised to find that a logging driver would break the application in these circumstances.

Thanks a ton for the commit, btw. I will use that.