Add SysLogger for new implementation. #17171

xincunli-sonic · 2023-11-14T20:03:25Z

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.

How did you verify/test it?

Run code snippet

import SysLogger
from daemon_base import DaemonBase

# 1. SysLogger
SYSLOG_IDENTIFIER = "TestSysLogger"

# Global logger instance
log = SysLogger.SysLogger(SYSLOG_IDENTIFIER)

log.log_info("This is a info message")
log.log_notice("This is a notice message")
log.log_warning("This is a warning message")
log.log_debug("This is a debug message")
log.log_error("This is a error message")

log.log_info("This is a info message", also_print_to_console=True)
log.log_notice("This is a notice message", also_print_to_console=True)
log.log_warning("This is a warning message", also_print_to_console=True)
log.log_debug("This is a debug message", also_print_to_console=True)
log.log_error("This is a error message", also_print_to_console=True)


# 2. Daemonbase with default syslog.

log2 = DaemonBase(SYSLOG_IDENTIFIER)

log2.log_info("log2: This is a info message")
log2.log_notice("log2: This is a notice message")
log2.log_warning("log2: This is a warning message")
log2.log_debug("log2: This is a debug message")
log2.log_error("log2: This is a error message")

log2.log_info("log2: This is a info message", also_print_to_console=True)
log2.log_notice("log2: This is a notice message", also_print_to_console=True)
log2.log_warning("log2: This is a warning message", also_print_to_console=True)
log2.log_debug("log2: This is a debug message", also_print_to_console=True)
log2.log_error("log2: This is a error message", also_print_to_console=True)

# 3. Daemonbase with SysLogger.

log3 = DaemonBase(SYSLOG_IDENTIFIER, use_syslogger=True)

log3.log_info("log3: This is a info message")
log3.log_notice("log3: This is a notice message")
log3.log_warning("log3: This is a warning message")
log3.log_debug("log3: This is a debug message")
log3.log_error("log3: This is a error message")

log3.log_info("log3: This is a info message", also_print_to_console=True)
log3.log_notice("log3: This is a notice message", also_print_to_console=True)
log3.log_warning("log3: This is a warning message", also_print_to_console=True)
log3.log_debug("log3: This is a debug message", also_print_to_console=True)
log3.log_error("log3: This is a error message", also_print_to_console=True)

Sample output

Jan  5 20:28:18 xincun-dev-vm TestSysLogger: This is a info message
Jan  5 20:28:18 xincun-dev-vm TestSysLogger: This is a notice message
Jan  5 20:28:18 xincun-dev-vm TestSysLogger: This is a warning message
Jan  5 20:28:18 xincun-dev-vm TestSysLogger: This is a error message
Jan  5 20:28:18 xincun-dev-vm TestSysLogger: This is a info message
Jan  5 20:28:18 xincun-dev-vm TestSysLogger: This is a notice message
Jan  5 20:28:18 xincun-dev-vm TestSysLogger: This is a warning message
Jan  5 20:28:18 xincun-dev-vm TestSysLogger: This is a error message
Jan  5 20:28:18 xincun-dev-vm TestSysLogger[967382]: log2: This is a notice message
Jan  5 20:28:18 xincun-dev-vm TestSysLogger[967382]: log2: This is a warning message
Jan  5 20:28:18 xincun-dev-vm TestSysLogger[967382]: log2: This is a error message
Jan  5 20:28:18 xincun-dev-vm TestSysLogger[967382]: log2: This is a notice message
Jan  5 20:28:18 xincun-dev-vm TestSysLogger[967382]: log2: This is a warning message
Jan  5 20:28:18 xincun-dev-vm TestSysLogger[967382]: log2: This is a error message
Jan  5 20:28:18 xincun-dev-vm TestSysLogger: log3: This is a info message
Jan  5 20:28:18 xincun-dev-vm TestSysLogger: log3: This is a notice message
Jan  5 20:28:18 xincun-dev-vm TestSysLogger: log3: This is a warning message
Jan  5 20:28:18 xincun-dev-vm TestSysLogger: log3: This is a error message
Jan  5 20:28:18 xincun-dev-vm TestSysLogger: log3: This is a info message
Jan  5 20:28:18 xincun-dev-vm TestSysLogger: log3: This is a notice message
Jan  5 20:28:18 xincun-dev-vm TestSysLogger: log3: This is a warning message
Jan  5 20:28:18 xincun-dev-vm TestSysLogger: log3: This is a error message

Which release branch to backport (provide reason below if selected)

202205
202305
202311

Signed-off-by: Xincun Li [email protected]

prgeor · 2023-11-15T15:11:27Z

@nazariig could you help review?

Fix path issue.

src/sonic-py-common/sonic_py_common/daemon_base.py

src/sonic-py-common/sonic_py_common/SysLogger.py

nazariig · 2023-11-21T16:29:07Z

src/sonic-py-common/sonic_py_common/daemon_base.py

@@ -48,6 +66,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 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.

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

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.

What we want to do would be only apply new SysLogger to some new Daemon process, like ycabled etc.

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.

src/sonic-py-common/sonic_py_common/syslogger.py

src/sonic-py-common/sonic_py_common/daemon_base.py

bingwang-ms · 2024-02-13T23:21:38Z

/azp run

azure-pipelines · 2024-02-13T23:21:44Z

Commenter does not have sufficient privileges for PR 17171 in repo sonic-net/sonic-buildimage

qiluo-msft · 2024-02-13T23:25:52Z

/azp run Azure.sonic-buildimage

azure-pipelines · 2024-02-13T23:26:06Z

Azure Pipelines successfully started running 1 pipeline(s).

src/sonic-py-common/sonic_py_common/syslogger.py

azure-pipelines · 2024-05-01T16:46:36Z

Azure Pipelines successfully started running 1 pipeline(s).

xincunli-sonic · 2024-05-02T00:24:31Z

/azpw run

mssonicbld · 2024-05-02T00:24:33Z

/AzurePipelines run

azure-pipelines · 2024-05-02T00:24:47Z

Azure Pipelines successfully started running 1 pipeline(s).

xincunli-sonic · 2024-05-02T17:29:39Z

/azpw run

mssonicbld · 2024-05-02T17:29:41Z

/AzurePipelines run

azure-pipelines · 2024-05-02T17:29:56Z

Azure Pipelines successfully started running 1 pipeline(s).

liat-grozovik · 2024-05-06T06:49:10Z

@xincunli-sonic could you please help to prioritize this PR feedback and checkers?
this infra is required for additional PR related here as well and we should have it on 202405

Junchao-Mellanox · 2024-05-06T08:34:37Z

Hi @xincunli-sonic , I suppose the checker failure is related to the code change. The unit test tried to connect to a unix socket of a logger server, but the unix socket does not exist.

--- Logging error ---
Traceback (most recent call last):
  File "/usr/lib/python3.11/logging/handlers.py", line 1007, in emit
    self.socket.send(msg)
OSError: [Errno 9] Bad file descriptor

Junchao-Mellanox · 2024-05-09T01:13:06Z

Hi @prgeor , could you please kindly review this?

prgeor · 2024-05-15T00:19:23Z

@liat-grozovik can you merge?

liat-grozovik · 2024-05-15T05:50:23Z

i removed the backport ask for old branches as 202205 and 202305. if this is must lets discuss.
i assume this is not needed for 202311 as well.

### **Issue** Fix the issue sonic-net/sonic-buildimage#21290 No info log found in syslog on 202405 image for caclmgrd ### **Work item tracking** - Microsoft ADO **(number only)**: 30611546 ### Why did it happen RP sonic-net/sonic-buildimage#17171, introduced a new Class SysLogger, DaemonBase will choose SysLogger by default PR sonic-net/sonic-buildimage#19232, it added noticed level and make it to be default level which suppresses INFO logs. `caclmgr.set_min_log_priority_info()` it sets min log priority to info, this function is in Logger class, SysLogger doesn't have this function. But DaemonBase still inherits Logger which implements set_min_log_priority_info, that's why even caclmgrd called this function, it didn't throw exception. But it didn't make INFO level effect in SysLogger which is actually used in caclmgrd Even change to use Logger by setting `use_syslogger=False`, it still doesn't work. The root cause is that it added a new instance for logger, `self.logger_instance`, any instance inherited from DaemonBase class can't change the debug level, the level they changed is their own instance, not the self.logger_instance's level. ### **How to fix** The solution here for caclmgrd is to choose logger.Logger class instead of DaemonBase. ### **How to verify it** Test it on 202405

Add SysLogger.

af3353e

xincunli-sonic requested a review from prgeor November 14, 2023 20:12

Fix SysLogger Path

ce22e71

prgeor requested a review from nazariig November 15, 2023 15:11

xincunli-sonic and others added 2 commits November 15, 2023 16:46

improve format.

cfbdcec

Update daemon_base.py

72e0524

Fix path issue.

nazariig reviewed Nov 16, 2023

View reviewed changes

src/sonic-py-common/sonic_py_common/daemon_base.py Outdated Show resolved Hide resolved

nazariig reviewed Nov 16, 2023

View reviewed changes

src/sonic-py-common/sonic_py_common/SysLogger.py Outdated Show resolved Hide resolved

xincunli-sonic and others added 4 commits November 16, 2023 10:11

Update daemon_base.py

badb1e8

remove syslogger

e1b211d

add back sysloger for lower case file name.

c1a1813

fix mapping.

c163d58

nazariig reviewed Nov 21, 2023

View reviewed changes

nazariig approved these changes Dec 11, 2023

View reviewed changes

xincunli-sonic added 4 commits December 16, 2023 15:58

Update daemon_base.py

6d29d33

Update daemon_base.py

46028ae

Update daemon_base.py

28d1de4

Update syslogger.py

d918a64

prgeor reviewed Feb 1, 2024

View reviewed changes

src/sonic-py-common/sonic_py_common/syslogger.py Outdated Show resolved Hide resolved

prgeor reviewed Feb 1, 2024

View reviewed changes

src/sonic-py-common/sonic_py_common/daemon_base.py Outdated Show resolved Hide resolved

xincunli-sonic added 2 commits February 2, 2024 12:03

Update daemon_base.py to change use_syslogger default as true.

47e42ec

Update syslogger.py

42619fd

xincunli-sonic added 2 commits February 14, 2024 09:59

Remove address.

a0fb65e

fix parameter name

1f50042

prgeor reviewed Feb 15, 2024

View reviewed changes

src/sonic-py-common/sonic_py_common/syslogger.py Outdated Show resolved Hide resolved

xincunli-sonic and others added 2 commits May 6, 2024 09:02

Update syslogger.py

d4982f1

Fix UT

3607f8f

xincunli-sonic requested a review from lguohan as a code owner May 6, 2024 21:12

xincunli-sonic and others added 3 commits May 6, 2024 15:18

Merge branch 'master' into xincun/add-new-syslogger

d9d9675

Update test_system_health.py

78c8359

Fix UT

3471d48

prgeor approved these changes May 15, 2024

View reviewed changes

liat-grozovik merged commit 166a9fb into sonic-net:master May 15, 2024
19 checks passed

xincunli-sonic mentioned this pull request May 20, 2024

Fix CCmisApi identifier overrides ycable identifier in syslog. sonic-net/sonic-platform-common#409

Closed

3 tasks

ZhaohuiS mentioned this pull request Dec 27, 2024

Fix the issue of no INFO level syslog for caclmgrd #21291

Closed

12 tasks

This was referenced Jan 8, 2025

INFO level logs disappear in caclmgrd #21290

Closed

Fix the issue of no info log in syslog for caclmgrd sonic-net/sonic-host-services#200

Merged

mssonicbld mentioned this pull request Jan 9, 2025

[action] [PR:200] Fix the issue of no info log in syslog for caclmgrd sonic-net/sonic-host-services#201

Merged

mssonicbld mentioned this pull request Feb 7, 2025

[action] [PR:200] Fix the issue of no info log in syslog for caclmgrd sonic-net/sonic-host-services#214

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add SysLogger for new implementation. #17171

Add SysLogger for new implementation. #17171

xincunli-sonic commented Nov 14, 2023 •

edited by liat-grozovik

Loading

prgeor commented Nov 15, 2023

nazariig Nov 21, 2023

xincunli-sonic Nov 30, 2023

bingwang-ms commented Feb 13, 2024

azure-pipelines bot commented Feb 13, 2024

qiluo-msft commented Feb 13, 2024

azure-pipelines bot commented Feb 13, 2024

azure-pipelines bot commented May 1, 2024

xincunli-sonic commented May 2, 2024

mssonicbld commented May 2, 2024

azure-pipelines bot commented May 2, 2024

xincunli-sonic commented May 2, 2024

mssonicbld commented May 2, 2024

azure-pipelines bot commented May 2, 2024

liat-grozovik commented May 6, 2024

Junchao-Mellanox commented May 6, 2024

Junchao-Mellanox commented May 9, 2024

prgeor commented May 15, 2024

liat-grozovik commented May 15, 2024 •

edited

Loading

Add SysLogger for new implementation. #17171

Add SysLogger for new implementation. #17171

Conversation

xincunli-sonic commented Nov 14, 2023 • edited by liat-grozovik Loading

prgeor commented Nov 15, 2023

nazariig Nov 21, 2023

Choose a reason for hiding this comment

xincunli-sonic Nov 30, 2023

Choose a reason for hiding this comment

bingwang-ms commented Feb 13, 2024

azure-pipelines bot commented Feb 13, 2024

qiluo-msft commented Feb 13, 2024

azure-pipelines bot commented Feb 13, 2024

azure-pipelines bot commented May 1, 2024

xincunli-sonic commented May 2, 2024

mssonicbld commented May 2, 2024

azure-pipelines bot commented May 2, 2024

xincunli-sonic commented May 2, 2024

mssonicbld commented May 2, 2024

azure-pipelines bot commented May 2, 2024

liat-grozovik commented May 6, 2024

Junchao-Mellanox commented May 6, 2024

Junchao-Mellanox commented May 9, 2024

prgeor commented May 15, 2024

liat-grozovik commented May 15, 2024 • edited Loading

xincunli-sonic commented Nov 14, 2023 •

edited by liat-grozovik

Loading

liat-grozovik commented May 15, 2024 •

edited

Loading