Skip to content
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 SysLogger for new implementation. #17171

Merged
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
af3353e
Add SysLogger.
xincunli-sonic Nov 14, 2023
ce22e71
Fix SysLogger Path
xincunli-sonic Nov 15, 2023
cfbdcec
improve format.
xincunli-sonic Nov 16, 2023
72e0524
Update daemon_base.py
xincunli-sonic Nov 16, 2023
badb1e8
Update daemon_base.py
xincunli-sonic Nov 16, 2023
e1b211d
remove syslogger
xincunli-sonic Nov 16, 2023
c1a1813
add back sysloger for lower case file name.
xincunli-sonic Nov 16, 2023
c163d58
fix mapping.
xincunli-sonic Nov 16, 2023
6d29d33
Update daemon_base.py
xincunli-sonic Dec 16, 2023
46028ae
Update daemon_base.py
xincunli-sonic Dec 16, 2023
28d1de4
Update daemon_base.py
xincunli-sonic Jan 5, 2024
d918a64
Update syslogger.py
xincunli-sonic Jan 5, 2024
47e42ec
Update daemon_base.py to change use_syslogger default as true.
xincunli-sonic Feb 2, 2024
42619fd
Update syslogger.py
xincunli-sonic Feb 2, 2024
a0fb65e
Remove address.
xincunli-sonic Feb 14, 2024
1f50042
fix parameter name
xincunli-sonic Feb 14, 2024
1e6ec55
Remove unused import
xincunli-sonic Feb 15, 2024
304cd32
Fix log level
xincunli-sonic Feb 15, 2024
f09c880
Add PID in syslogger formatter
xincunli-sonic Mar 29, 2024
972cc6d
Change to logging's priority constant
xincunli-sonic Apr 19, 2024
26b5de3
Update syslogger.py
xincunli-sonic Apr 22, 2024
0a0bd01
Update daemon_base.py
xincunli-sonic Apr 23, 2024
d4982f1
Update syslogger.py
xincunli-sonic May 6, 2024
3607f8f
Fix UT
xincunli-sonic May 6, 2024
d9d9675
Merge branch 'master' into xincun/add-new-syslogger
xincunli-sonic May 6, 2024
78c8359
Update test_system_health.py
xincunli-sonic May 6, 2024
3471d48
Fix UT
xincunli-sonic May 7, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 70 additions & 0 deletions src/sonic-py-common/sonic_py_common/SysLogger.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
import os
import socket
import sys
import logging
from logging.handlers import SysLogHandler


class SysLogger:
"""
SysLogger class for Python applications using SysLogHandler
"""

# Mapping syslog priorities to logging module levels
LOG_LEVEL_MAP = {
'LOG_ERR': logging.ERROR,
'LOG_WARNING': logging.WARNING,
'LOG_NOTICE': logging.INFO,
'LOG_INFO': logging.INFO,
'LOG_DEBUG': logging.DEBUG
}

DEFAULT_LOG_FACILITY = SysLogHandler.LOG_USER
DEFAULT_LOG_LEVEL = SysLogHandler.LOG_INFO

def __init__(self, log_identifier=None, log_facility=DEFAULT_LOG_FACILITY, log_level=DEFAULT_LOG_LEVEL):
if log_identifier is None:
log_identifier = os.path.basename(sys.argv[0])

# Initialize SysLogger
self.logger = logging.getLogger(log_identifier)
self.logger.setLevel(log_level)
handler = SysLogHandler(address="/dev/log", facility=log_facility, socktype=socket.SOCK_DGRAM)
formatter = logging.Formatter("%(name)s: %(message)s")
handler.setFormatter(formatter)
self.logger.addHandler(handler)

# Set the default minimum log priority to 'LOG_DEBUG'
self.set_min_log_priority('LOG_DEBUG')

def set_min_log_priority(self, priority):
"""
Sets the minimum log priority level. All log messages
with a priority lower than this will not be logged
"""
self._min_log_level = self.LOG_LEVEL_MAP[priority]

# Methods for logging messages
def log(self, priority, msg, also_print_to_console=False):
log_level = self.LOG_LEVEL_MAP[priority]
xincunli-sonic marked this conversation as resolved.
Show resolved Hide resolved
if log_level >= self._min_log_level:
self.logger.log(log_level, msg)

if also_print_to_console:
print(msg)

# Convenience methods
def log_error(self, msg, also_print_to_console=False):
self.log('LOG_ERR', msg, also_print_to_console)

def log_warning(self, msg, also_print_to_console=False):
self.log('LOG_WARNING', msg, also_print_to_console)

def log_notice(self, msg, also_print_to_console=False):
self.log('LOG_NOTICE', msg, also_print_to_console)

def log_info(self, msg, also_print_to_console=False):
self.log('LOG_INFO', msg, also_print_to_console)

def log_debug(self, msg, also_print_to_console=False):
self.log('LOG_DEBUG', msg, also_print_to_console)
43 changes: 37 additions & 6 deletions src/sonic-py-common/sonic_py_common/daemon_base.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,20 @@
import signal
import sys
import syslog

from . import device_info
from .general import load_module_from_source
from .logger import Logger
from SysLogger import SysLogger

# Mapping syslog priorities to SysLogger's priority.
LOG_PRIORITY_MAP = {
syslog.LOG_ERR: 'LOG_ERR',
syslog.LOG_WARNING: 'LOG_WARNING',
syslog.LOG_NOTICE: 'LOG_NOTICE',
syslog.LOG_INFO: 'LOG_INFO',
syslog.LOG_DEBUG: 'LOG_DEBUG'
}

#
# Constants ====================================================================
Expand All @@ -21,6 +32,7 @@
# Helper functions =============================================================
#


def db_connect(db_name, namespace=EMPTY_NAMESPACE):
from swsscommon import swsscommon
return swsscommon.DBConnector(db_name, REDIS_TIMEOUT_MSECS, True, namespace)
Expand All @@ -32,12 +44,17 @@ def db_connect(db_name, namespace=EMPTY_NAMESPACE):


class DaemonBase(Logger):
xincunli-sonic marked this conversation as resolved.
Show resolved Hide resolved
def __init__(self, log_identifier):
super(DaemonBase, self).__init__(
log_identifier=log_identifier,
log_facility=Logger.LOG_FACILITY_DAEMON,
log_option=(Logger.LOG_OPTION_NDELAY | Logger.LOG_OPTION_PID)
)
def __init__(self, log_identifier, use_syslogger=False):
xincunli-sonic marked this conversation as resolved.
Show resolved Hide resolved
super().__init__()
self.use_syslogger = use_syslogger
if self.use_syslogger:
self.logger_instance = SysLogger(log_identifier)
xincunli-sonic marked this conversation as resolved.
Show resolved Hide resolved
else:
self.logger_instance = Logger(
log_identifier=log_identifier,
log_facility=Logger.LOG_FACILITY_DAEMON,
log_option=(Logger.LOG_OPTION_NDELAY | Logger.LOG_OPTION_PID)
)

# Register our default signal handlers, unless the signal already has a
# handler registered, most likely from a subclass implementation
Expand All @@ -48,6 +65,20 @@ def __init__(self, log_identifier):
if not signal.getsignal(signal.SIGTERM):
signal.signal(signal.SIGTERM, self.signal_handler)

def log(self, priority, msg, also_print_to_console=False):
xincunli-sonic marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@xincunli-sonic IMHO, this is a hack and in general a bad architecture. The method you override, was not designed to be overridable. Also, the logger class is used in many other places, not just only a various python demons implementation. This means, that the issue appears to be in many places. Ideally, there should be two different implementations, which share the same interface (e.g., LoggerBase[defines logger interface API], SystemLogger[uses native syslog], SyslogLogger[uses syslog handler], FileLogger[uses file handler]). But that will lead up to a major code changes. I would suggest to abandon having a separate SyslogLogger implementation and just fix the original one, preserving all the defined set of APIs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is intending to be reduce the scope of new change. Here is the consideration

  1. Most of daemon process will inherit from Daemon_Base with old logger implementation. If we use the new SysLogger, then it could be used for all daemons.
  2. What we want to do would be only apply new SysLogger to some new Daemon process, like ycabled etc.
  3. The override function, you can think it like factory pattern, which depends on use_syslogger flag as optional parameter, if it is true then it will use new SysLogger, otherwise it keeps the old logger and behavior will not change.

if self._min_log_priority >= priority:
if self.use_syslogger:
# Using SysLogger.
logging_priority = LOG_PRIORITY_MAP.get(priority, 'LOG_INFO')
self.logger_instance.log(logging_priority, msg)
else:
# Send message to syslog
self.logger_instance.log(priority, msg)

# Send message to console
if also_print_to_console:
print(msg)

# Default signal handler; can be overridden by subclass
def signal_handler(self, sig, frame):
if sig == signal.SIGHUP:
Expand Down
Loading