Replace RFCSysLogHandler by a syslog() based one

Rather than using the Python provided logging module that reimplents the
whole syslog protocol and pointing it to /dev/log (which is not
portable), we use the system calls from the syslog module, which are
portable and default to the behaviour we actually want.

Closes-Bug: 1385295
Closes-Bug: 1391428

Change-Id: I39a36316bd8778831b1bd5e7c5e2e12ede062a37

oslo_log/handlers.py

@@ -15,6 +15,9 @@ import logging
 import logging.config
 import logging.handlers
 import os
+import syslog
+
+from debtcollector import removals
 
 
 try:
@@ -36,6 +39,17 @@ def _get_binary_name():
 
 
 class RFCSysLogHandler(logging.handlers.SysLogHandler):
+    """SysLogHandler following the RFC
+
+    .. deprecated:: 1.2.0
+       Use :class:`OSSysLogHandler` instead
+    """
+
+    @removals.remove(
+        message='use oslo_log.handlers.OSSysLogHandler()',
+        version='1.2.0',
+        removal_version='?',
+    )
     def __init__(self, *args, **kwargs):
         self.binary_name = _get_binary_name()
         # Do not use super() unless type(logging.handlers.SysLogHandler)
@@ -54,6 +68,33 @@ class RFCSysLogHandler(logging.handlers.SysLogHandler):
 _AUDIT = logging.INFO + 1
 
 
+class OSSysLogHandler(logging.Handler):
+    severity_map = {
+        "CRITICAL": syslog.LOG_CRIT,
+        "DEBUG": syslog.LOG_DEBUG,
+        "ERROR": syslog.LOG_ERR,
+        "INFO": syslog.LOG_INFO,
+        "WARNING": syslog.LOG_WARNING,
+        "WARN": syslog.LOG_WARNING,
+    }
+
+    def __init__(self, facility=syslog.LOG_USER,
+                 use_syslog_rfc_format=True):
+        # Do not use super() unless type(logging.Handler) is 'type'
+        # (i.e. >= Python 2.7).
+        logging.Handler.__init__(self)
+        if use_syslog_rfc_format:
+            binary_name = _get_binary_name()
+        else:
+            binary_name = ""
+        syslog.openlog(binary_name, 0, facility)
+
+    def emit(self, record):
+        syslog.syslog(self.severity_map.get(record.levelname,
+                                            syslog.LOG_DEBUG),
+                      record.getMessage())
+
+
 class ColorHandler(logging.StreamHandler):
     LEVEL_COLORS = {
         logging.DEBUG: '\033[00;32m',  # GREEN

oslo_log/log.py

@@ -31,8 +31,8 @@ import logging
 import logging.config
 import logging.handlers
 import os
-import socket
 import sys
+import syslog
 import traceback
 
 from oslo_config import cfg
@@ -260,28 +260,30 @@ def tempest_set_log_file(filename):
     cfg.set_defaults(_options.logging_cli_opts, log_file=filename)
 
 
-def _find_facility_from_conf(conf):
-    facility_names = logging.handlers.SysLogHandler.facility_names
-    facility = getattr(logging.handlers.SysLogHandler,
-                       conf.syslog_log_facility,
-                       None)
+def _find_facility(facility):
+    # NOTE(jd): Check the validity of facilities at run time as they differ
+    # depending on the OS and Python version being used.
+    valid_facilities = [f for f in
+                        ["LOG_KERN", "LOG_USER", "LOG_MAIL",
+                         "LOG_DAEMON", "LOG_AUTH", "LOG_SYSLOG",
+                         "LOG_LPR", "LOG_NEWS", "LOG_UUCP",
+                         "LOG_CRON", "LOG_AUTHPRIV", "LOG_FTP",
+                         "LOG_LOCAL0", "LOG_LOCAL1", "LOG_LOCAL2",
+                         "LOG_LOCAL3", "LOG_LOCAL4", "LOG_LOCAL5",
+                         "LOG_LOCAL6", "LOG_LOCAL7"]
+                        if getattr(syslog, f, None)]
 
-    if facility is None and conf.syslog_log_facility in facility_names:
-        facility = facility_names.get(conf.syslog_log_facility)
+    facility = facility.upper()
 
-    if facility is None:
-        valid_facilities = facility_names.keys()
-        consts = ['LOG_AUTH', 'LOG_AUTHPRIV', 'LOG_CRON', 'LOG_DAEMON',
-                  'LOG_FTP', 'LOG_KERN', 'LOG_LPR', 'LOG_MAIL', 'LOG_NEWS',
-                  'LOG_AUTH', 'LOG_SYSLOG', 'LOG_USER', 'LOG_UUCP',
-                  'LOG_LOCAL0', 'LOG_LOCAL1', 'LOG_LOCAL2', 'LOG_LOCAL3',
-                  'LOG_LOCAL4', 'LOG_LOCAL5', 'LOG_LOCAL6', 'LOG_LOCAL7']
-        valid_facilities.extend(consts)
+    if not facility.startswith("LOG_"):
+        facility = "LOG_" + facility
+
+    if facility not in valid_facilities:
         raise TypeError(_('syslog facility must be one of: %s') %
                         ', '.join("'%s'" % fac
                                   for fac in valid_facilities))
 
-    return facility
+    return getattr(syslog, facility)
 
 
 def _setup_logging_from_conf(conf, project, version):
@@ -311,20 +313,13 @@ def _setup_logging_from_conf(conf, project, version):
         log_root.addHandler(handler)
 
     if conf.use_syslog:
-        try:
-            facility = _find_facility_from_conf(conf)
-            # TODO(bogdando) use the format provided by RFCSysLogHandler
-            #   after existing syslog format deprecation in J
-            if conf.use_syslog_rfc_format:
-                syslog = handlers.RFCSysLogHandler(address='/dev/log',
-                                                   facility=facility)
-            else:
-                syslog = logging.handlers.SysLogHandler(address='/dev/log',
-                                                        facility=facility)
-            log_root.addHandler(syslog)
-        except socket.error:
-            log_root.error('Unable to add syslog handler. Verify that syslog '
-                           'is running.')
+        facility = _find_facility(conf.syslog_log_facility)
+        # TODO(bogdando) use the format provided by RFCSysLogHandler after
+        # existing syslog format deprecation in J
+        syslog = handlers.OSSysLogHandler(
+            facility=facility,
+            use_syslog_rfc_format=conf.use_syslog_rfc_format)
+        log_root.addHandler(syslog)
 
     datefmt = conf.log_date_format
     for handler in log_root.handlers:

oslo_log/tests/unit/test_log.py

@@ -17,6 +17,7 @@
 import logging
 import os
 import sys
+import syslog
 import tempfile
 
 import mock
@@ -210,6 +211,26 @@ class SysLogHandlersTestCase(BaseTestCase):
                          expected.getMessage())
 
 
+class OSSysLogHandlerTestCase(BaseTestCase):
+    def tests_handler(self):
+        handler = handlers.OSSysLogHandler()
+        syslog.syslog = mock.Mock()
+        handler.emit(
+            logging.LogRecord("foo", logging.INFO,
+                              "path", 123, "hey!",
+                              None, None))
+        self.assertTrue(syslog.syslog.called)
+
+    def test_find_facility(self):
+        self.assertEqual(syslog.LOG_USER, log._find_facility("user"))
+        self.assertEqual(syslog.LOG_LPR, log._find_facility("LPR"))
+        self.assertEqual(syslog.LOG_LOCAL3, log._find_facility("log_local3"))
+        self.assertEqual(syslog.LOG_UUCP, log._find_facility("LOG_UUCP"))
+        self.assertRaises(TypeError,
+                          log._find_facility,
+                          "fougere")
+
+
 class LogLevelTestCase(BaseTestCase):
     def setUp(self):
         super(LogLevelTestCase, self).setUp()

requirements.txt

@@ -11,3 +11,4 @@ oslo.context>=0.2.0                     # Apache-2.0
 oslo.i18n>=1.5.0  # Apache-2.0
 oslo.utils>=1.4.0                       # Apache-2.0
 oslo.serialization>=1.4.0               # Apache-2.0
+debtcollector>=0.3.0  # Apache-2.0