-
Notifications
You must be signed in to change notification settings - Fork 71
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
Unify logger.conf and the fallback code #616
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -10,7 +10,7 @@ | |||||
from leapp.utils.actorapi import get_actor_api, RequestException | ||||||
from leapp.utils.audit import Audit | ||||||
|
||||||
_logger = None | ||||||
_leapp_logger = None | ||||||
|
||||||
|
||||||
class LeappAuditHandler(logging.Handler): | ||||||
|
@@ -52,39 +52,66 @@ def _remote_emit(self, log_data): | |||||
|
||||||
|
||||||
def configure_logger(log_file=None): | ||||||
global _logger | ||||||
if not _logger: | ||||||
""" | ||||||
Configure loggers as per the description below. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should be on the same line with """ (https://www.python.org/dev/peps/pep-0257/#multi-line-docstrings) |
||||||
|
||||||
logger: root | ||||||
level: DEBUG | ||||||
handler: StreamHandler | ||||||
level: based on the debug/verbosity options | ||||||
handler: LeappAuditHandler | ||||||
level: DEBUG | ||||||
logger: urllib3 | ||||||
level: WARN | ||||||
logger: leapp | ||||||
level: NOTSET | ||||||
handler: FileHandler | ||||||
level: DEBUG | ||||||
|
||||||
:return: The 'leapp' logger | ||||||
""" | ||||||
global _leapp_logger | ||||||
if not _leapp_logger: | ||||||
_leapp_logger = logging.getLogger('leapp') | ||||||
|
||||||
log_format = '%(asctime)s.%(msecs)-3d %(levelname)-8s PID: %(process)d %(name)s: %(message)s' | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think fo ruser would be convininent to see that this time is UTC time
Suggested change
|
||||||
log_date_format = '%Y-%m-%d %H:%M:%S' | ||||||
path = os.getenv('LEAPP_LOGGER_CONFIG', '/etc/leapp/logger.conf') | ||||||
logging.Formatter.converter = time.gmtime | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please don't do this. This is a global state of all loggers. What about the one from the cookbook? (https://docs.python.org/3/howto/logging-cookbook.html#formatting-times-using-utc-gmt-via-configuration) diff --git a/etc/leapp/logger.conf b/etc/leapp/logger.conf
index ee1cf91..66102c4 100644
--- a/etc/leapp/logger.conf
+++ b/etc/leapp/logger.conf
@@ -10,7 +10,7 @@ keys=leapp_audit,stream
[formatter_leapp]
format=%(asctime)s.%(msecs)-3d %(levelname)-8s PID: %(process)d %(name)s: %(message)s
datefmt=%Y-%m-%d %H:%M:%S
-class=logging.Formatter
+class=leapp.logger.UTCFormatter
[logger_urllib3]
level=WARN |
||||||
|
||||||
root_logger = logging.getLogger() | ||||||
root_logger.setLevel(logging.DEBUG) | ||||||
|
||||||
path = os.getenv('LEAPP_LOGGER_CONFIG', '/etc/leapp/logger.conf') | ||||||
if path and os.path.isfile(path): | ||||||
logging.config.fileConfig(path) | ||||||
else: # Fall back logging configuration | ||||||
logging.Formatter.converter = time.gmtime | ||||||
logging.basicConfig( | ||||||
level=logging.ERROR, | ||||||
format=log_format, | ||||||
datefmt=log_date_format, | ||||||
stream=sys.stderr, | ||||||
) | ||||||
stderr_handler = logging.StreamHandler(sys.stderr) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No need to put sys.stderr, it is by default |
||||||
stderr_handler.setFormatter(logging.Formatter(fmt=log_format, datefmt=log_date_format)) | ||||||
stderr_handler.setLevel(logging.ERROR) | ||||||
root_logger.addHandler(stderr_handler) | ||||||
|
||||||
logging.getLogger('urllib3').setLevel(logging.WARN) | ||||||
handler = LeappAuditHandler() | ||||||
handler.setFormatter(logging.Formatter(fmt=log_format, datefmt=log_date_format)) | ||||||
logging.getLogger('leapp').addHandler(handler) | ||||||
|
||||||
audit_handler = LeappAuditHandler() | ||||||
audit_handler.setFormatter(logging.Formatter(fmt=log_format, datefmt=log_date_format)) | ||||||
audit_handler.setLevel(logging.DEBUG) | ||||||
root_logger.addHandler(audit_handler) | ||||||
bocekm marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
|
||||||
if log_file: | ||||||
file_handler = logging.FileHandler(os.path.join(get_config().get('logs', 'dir'), log_file)) | ||||||
file_handler.setFormatter(logging.Formatter(fmt=log_format, datefmt=log_date_format)) | ||||||
file_handler.setLevel(logging.DEBUG) | ||||||
logging.getLogger('leapp').addHandler(file_handler) | ||||||
_leapp_logger.addHandler(file_handler) | ||||||
|
||||||
if is_verbose(): | ||||||
for handler in logging.getLogger().handlers: | ||||||
if isinstance(handler, logging.StreamHandler): | ||||||
handler.setLevel(logging.DEBUG if is_debug() else logging.INFO) | ||||||
for handler in root_logger.handlers: | ||||||
if isinstance(handler, logging.StreamHandler): | ||||||
if is_debug(): | ||||||
handler.setLevel(logging.DEBUG) | ||||||
elif is_verbose(): | ||||||
handler.setLevel(logging.INFO) | ||||||
else: | ||||||
handler.setLevel(logging.ERROR) | ||||||
|
||||||
_logger = logging.getLogger('leapp') | ||||||
_logger.info('Logging has been initialized') | ||||||
_leapp_logger.info('Logging has been initialized') | ||||||
|
||||||
return _logger | ||||||
return _leapp_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.
there is a special docstring syntax for the definition of what being raised: