Skip to content

Commit

Permalink
Add SysLogger for new implementation. (#17171)
Browse files Browse the repository at this point in the history
This PR introduces the SysLogger to address an issue where the original Logger.py could override the SYSLOG_IDENTIFIER if a new Logger instance is created.

- Why I did it?
This change was made because the SysLog was displaying NOTICE pmon#CCmisApi: instead of NOTICE pmon#ycable:. This discrepancy led to confusion during debugging and incorrect component identification in log analysis.

- How I did it
I implemented an instance-level syslog using the logging.handlers module to prevent the SYSLOG_IDENTIFIER from being overridden.
  • Loading branch information
xincunli-sonic authored May 15, 2024
1 parent a480f8d commit 166a9fb
Show file tree
Hide file tree
Showing 3 changed files with 103 additions and 8 deletions.
38 changes: 32 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,12 @@
import logging
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

#
# Constants ====================================================================
Expand Down Expand Up @@ -32,12 +35,16 @@ def db_connect(db_name, namespace=EMPTY_NAMESPACE):


class DaemonBase(Logger):
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=True):
super().__init__()
if use_syslogger:
self.logger_instance = SysLogger(log_identifier)
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 +55,25 @@ def __init__(self, log_identifier):
if not signal.getsignal(signal.SIGTERM):
signal.signal(signal.SIGTERM, self.signal_handler)

def log(self, priority, message, also_print_to_console=False):
self.logger_instance.log(priority, message, also_print_to_console)

def log_error(self, message, also_print_to_console=False):
self.logger_instance.log_error(message, also_print_to_console)

def log_warning(self, message, also_print_to_console=False):
self.logger_instance.log_warning(message, also_print_to_console)

def log_notice(self, message, also_print_to_console=False):
self.logger_instance.log_notice(message, also_print_to_console)

def log_info(self, message, also_print_to_console=False):
self.logger_instance.log_info(message, also_print_to_console)

def log_debug(self, message, also_print_to_console=False):
self.logger_instance.log_debug(message, also_print_to_console)


# Default signal handler; can be overridden by subclass
def signal_handler(self, sig, frame):
if sig == signal.SIGHUP:
Expand Down
62 changes: 62 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,62 @@
import logging
from logging.handlers import SysLogHandler
import os
import socket
import sys

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

DEFAULT_LOG_FACILITY = SysLogHandler.LOG_USER
DEFAULT_LOG_LEVEL = SysLogHandler.LOG_NOTICE

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)

# Reset all existing handlers
for handler in self.logger.handlers[:]:
self.logger.removeHandler(handler)

handler = SysLogHandler(address="/dev/log", facility=log_facility, socktype=socket.SOCK_DGRAM)
formatter = logging.Formatter('%(name)s[%(process)d]: %(message)s')
handler.setFormatter(formatter)
self.logger.addHandler(handler)

self.set_min_log_priority(log_level)

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 = priority
self.logger.setLevel(priority)

# Methods for logging messages
def log(self, priority, msg, also_print_to_console=False):
self.logger.log(priority, msg)

if also_print_to_console:
print(msg)

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

def log_warning(self, msg, also_print_to_console=False):
self.log(logging.WARNING, msg, also_print_to_console)

def log_notice(self, msg, also_print_to_console=False):
self.log(logging.INFO, msg, also_print_to_console)

def log_info(self, msg, also_print_to_console=False):
self.log(logging.INFO, msg, also_print_to_console)

def log_debug(self, msg, also_print_to_console=False):
self.log(logging.DEBUG, msg, also_print_to_console)
11 changes: 9 additions & 2 deletions src/system-health/tests/test_system_health.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,12 @@
snmp-subagent EXITED Oct 19 01:53 AM
"""
device_info.get_platform = MagicMock(return_value='unittest')

device_runtime_metadata = {"DEVICE_RUNTIME_METADATA": {"ETHERNET_PORTS_PRESENT":True}}

def no_op(*args, **kwargs):
pass # This function does nothing

def setup():
if os.path.exists(ServiceChecker.CRITICAL_PROCESS_CACHE):
os.remove(ServiceChecker.CRITICAL_PROCESS_CACHE)
Expand Down Expand Up @@ -875,7 +878,9 @@ def test_get_service_from_feature_table():


@patch('healthd.time.time')
def test_healthd_check_interval(mock_time):
@patch('healthd.HealthDaemon.log_notice', side_effect=lambda *args, **kwargs: None)
@patch('healthd.HealthDaemon.log_warning', side_effect=lambda *args, **kwargs: None)
def test_healthd_check_interval(mock_log_warning, mock_log_notice, mock_time):
daemon = HealthDaemon()
manager = MagicMock()
manager.check = MagicMock()
Expand All @@ -888,6 +893,8 @@ def test_healthd_check_interval(mock_time):
daemon.stop_event.wait.return_value = False
manager.config.interval = 60
mock_time.side_effect = [0, 3, 0, 61, 0, 1]
mock_log_notice.side_effect = no_op
mock_log_warning.side_effect = no_op
assert daemon._run_checker(manager, chassis)
daemon.stop_event.wait.assert_called_with(57)
assert daemon._run_checker(manager, chassis)
Expand Down

0 comments on commit 166a9fb

Please sign in to comment.