-
Notifications
You must be signed in to change notification settings - Fork 441
Improve log extension to avoid unique messages #1394
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improve log extension to avoid unique messages #1394
Conversation
Sending debug
level logs to Sentry is, in general, a bad idea. Even with a on-premise setup it will simply generate a ton of useless logs (unless you make more complex decision making about what is sent and what isn't based on maybe channel settings).
That said, I agree that if it's in context, it could be removed from the main message string.
The thing is that it's an error
level log if the processing of the message was not successful. And you cannot easily filter messages that are sent to Sentry. Please see https://github.com/php-enqueue/enqueue-dev/pull/1394/files#diff-61f6a09a5a7e5b04b1179fa9524988b42ac12eace842080d2397c699af81a373L63
Right, I see now :) Yeah, definitely looks good to me, just need tests adjusted to the new variant.
Sure, thing. Should be fine now.
Does it look good? Is there anything else I could help to get it merged and released?
Currently the log extension is adding the processed message body to the log message. This creates unique logs, and duplicates the info as it's added in the log context. It's not an issue per se, but if you use Sentry (or similar tools), you receive issues for each message handled with error.
What do you think about excluding it?