-
Notifications
You must be signed in to change notification settings - Fork 15
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
Review logging (dev and prod settings) #124
Conversation
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.
Thanks, this should help greatly when debugging the platform. We'll need to decide what to do precisely with prod settings, but otherwise 👍
45d47a4
to
b887d27
Compare
Prod examplesWithout printing SQL queries and without printing worker workspaces: server:
Note: the different log formats for the requests/apps is due to the usage of uwsgi. worker
Dev examplesserver
worker
|
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'm more than fine with the changes, but maybe someone who participated in creating the previous conf should weigh in.
Need to be rebased @samlesu |
- add a log for each request done to the ledger - no more calls to root logger (logging.info(...)), use a module logger instead: it will allow better logs configuration and tuning - review debug log configuration: - remove mail and file handlers: they are not used in dev mode - add substrapp logger - set custom configuration for external library when needed (hfc, celery, ...) - add settings/env variables to: - disable SQL query logss - disable worker printing tuple workerspace
- remove unused handlers
6294957
to
648f917
Compare
Thanks, done. |
I let you merge if you think it's the log level you need to debug :) |
Could you be more precise? Do you think we should increase or decrease some logger levels in dev or prod settings ? |
Nope, all I mean is, if you do not need to make any change to the log level feel free to merge ! I don't know if you're still ajusting the log level :) |
Thanks for the clarification ! I think this PR is great start to improve our logs in dev and production. It will have to be updated according to our needs. I will be happy to merge once I have an approval from either you, @inalgnu, or @GuillaumeCisco before merging. |
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.
thank you @samlesu 👍
@@ -182,6 +182,8 @@ def container_format_log(container_name, container_logs): | |||
|
|||
|
|||
def list_files(startpath): | |||
if not settings.TASK['LIST_WORKSPACE']: | |||
return |
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.
We should never return nothing.
Why not simply use if settings.TASK['LIST_WORKSPACE']:
logging.info(...)
), use a module logger instead: it will allow better logging configuration and tuningcelery, ...)