-
Notifications
You must be signed in to change notification settings - Fork 64
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
Add new configuration options for the job logging #147
base: master
Are you sure you want to change the base?
Conversation
'processed' => true, | ||
'failed' => true, | ||
'failed_details' => true, | ||
], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd go with one setting for all, after looking at the logs I'm not sure it's worth having different options.
How about jobs_logs
true/false?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking more a global log_level that controls all the output, or maybe individual keys for the different components? Like injecting secrets.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
log_level
sounds good (super clear), but as a user I'd be wondering what the levels are, and what they do. That's added choice on the user, which adds complexity.
Considering the logs in question, what would be the levels? On and off? That's kinda like a boolean?
i.e. I'm not against it, but I have trouble seeing what that means in practice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was initially thinking the eight RFC 5424 levels (debug, info, notice, warning, error, critical, alert, emergency).
But maybe something like this would be easier:
'log' => [
'init' => true,
'queue' => true,
],
But of course ALWAYS log any kind or failure or warning.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't the whole point of log levels that apps have a standardized way or logging messages and the administrator can control which logs he cares about?
Yes.
Our only problem is the init
stuff from #143, because the LogManager
hasn't been booted, yet. But maybe there is a way around this @georgeboot.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah completely missed that 🤭
Yeah we should be able to buffer the bootup messages somehow, until we have a hard crash or booted logger.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you feel like tackling this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any update on this... can we maybe accept my PR for now if you don't want to tackle the more complex approach. I'm getting thousands of "Processed Job" logs on my dev where Bref is not even doing anything and it's driving me mad
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd vote to make a simple and single 'log jobs' setting, default true.
It would be great if Laravel would allow you to set the severity threshold per package instead of just global. But afaiaa that's not a thing.
This PR adds the ability to turn off the logging for jobs while keeping the defaults to the current behaviour