-
Notifications
You must be signed in to change notification settings - Fork 90
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
Logging refactor #1048
Merged
Merged
Logging refactor #1048
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
KentShikama
approved these changes
Apr 18, 2020
wittejm
approved these changes
Apr 18, 2020
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.
Awesome! I think dropping a local log file sounds good. @KentShikama, @kenichi, and myself are the only active backend developers lately so I don't think it'd be disruptive to drop it right away.
this function was using `logging.error()` which used the root logger, which was being replaced in the flask app, inside `logging.attach_logger`.
* moves `logger.setLevel` from app.py to loggers.py * splits out `colored_stdout_handler()` and `file_handler()` from `attach_logger()` - for reuse in the future
when not in development, uses DetailedFormatter on STDOUT only for running in a stand-alone, non-dev, container
* connect to syslog on host using udp * modded /etc/rsyslog.conf: module(load="imudp") input(type="imudp" port="514" device="docker0") * use local0, local1 syslog facilities for prod, staging respectively * added /etc/rsyslog.d/99-recordsponge.conf: local1.* /var/log/recordsponge/staging.log local0.* /var/log/recordsponge/prod.log * exclude the local0, local1 facilities from default syslog * modded /etc/rsyslog.d/50-default.conf: *.*;auth,authpriv,local0,local1.none -/var/log/syslog * rotate normally, keeping 4 days worth * modded /etc/logrotate.d/rsyslog: /var/log/recordsponge/staging.log /var/log/recordsponge/prod.log
kenichi
force-pushed
the
logging_refactor
branch
from
April 20, 2020 14:43
2ee3cf7
to
21c163b
Compare
wittejm
pushed a commit
that referenced
this pull request
Apr 22, 2020
* make request.error use current_app.logger this function was using `logging.error()` which used the root logger, which was being replaced in the flask app, inside `logging.attach_logger`. * slight refactor of logging functions * moves `logger.setLevel` from app.py to loggers.py * splits out `colored_stdout_handler()` and `file_handler()` from `attach_logger()` - for reuse in the future * add tier to app.config, base log config on tier when not in development, uses DetailedFormatter on STDOUT only for running in a stand-alone, non-dev, container * add syslog driver & opts to deployed containers * connect to syslog on host using udp * modded /etc/rsyslog.conf: module(load="imudp") input(type="imudp" port="514" device="docker0") * use local0, local1 syslog facilities for prod, staging respectively * added /etc/rsyslog.d/99-recordsponge.conf: local1.* /var/log/recordsponge/staging.log local0.* /var/log/recordsponge/prod.log * exclude the local0, local1 facilities from default syslog * modded /etc/rsyslog.d/50-default.conf: *.*;auth,authpriv,local0,local1.none -/var/log/syslog * rotate normally, keeping 4 days worth * modded /etc/logrotate.d/rsyslog: /var/log/recordsponge/staging.log /var/log/recordsponge/prod.log
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
address #463 ( and eventually #640 ) - changes in devops make using docker drivers for logs in deployed environments a goal. to handle error notifications, a log handler can be configured, or something similar and simple to post to a slack channel.
For local dev, this PR leaves the current file logging and colorized stdout logging in place. i do recommend eventually deprecating file logging in development.
This PR also adds TIER to app.config so it can be cased on to determine logging handlers, making production use only a non-colorized stdout.
Last commit is to use syslog docker logging drivers on the production host for standardized capture, rotation, etc. With all these changes, the staging and prod images should run everything logging to stdout, which will be routed by the docker driver into syslog, configured to deliver to /var/log/recordsponge/*.log on the host. For example, SSH into the host and run:
tail -f /var/log/recordsponge/staging.log
, browse to the dev.recordsponge.com site, and you will see the output. standard logrotate is also configured.