Make SetupToolsLogging use tools logfile
authorHelga Velroyen <helgav@google.com>
Tue, 15 Dec 2015 10:33:01 +0000 (11:33 +0100)
committerHelga Velroyen <helgav@google.com>
Thu, 7 Jan 2016 13:33:17 +0000 (14:33 +0100)
This patch makes SetupToolsLogging use a dedicated
log file (tools.log) instead of spamming node-daemon.log.
Logging tools' output to node-daemon.log is not really
the correct thing to do, because tools can be called
without any node daemon running and thus it makes more
sense that they have their own log file.

This patch also makes SetupToolsLogging use the SetupTools
function, because nearly all functionality added before is
actually subsumed by SetupTools (like printing the
threadname). Only a slight modification of the log level
handling had to be added.

Signed-off-by: Helga Velroyen <helgav@google.com>
Reviewed-by: Klaus Aehlig <aehlig@google.com>

lib/pathutils.py
lib/utils/log.py
test/py/ganeti.utils.log_unittest.py

index fbedb73..78e321a 100644 (file)
@@ -194,3 +194,4 @@ def GetLogFilename(daemon_name):
 LOG_WATCHER = GetLogFilename("watcher")
 LOG_COMMANDS = GetLogFilename("commands")
 LOG_BURNIN = GetLogFilename("burnin")
 LOG_WATCHER = GetLogFilename("watcher")
 LOG_COMMANDS = GetLogFilename("commands")
 LOG_BURNIN = GetLogFilename("burnin")
+LOG_TOOLS = GetLogFilename("tools")
index 3703221..c39e96a 100644 (file)
 import os.path
 import logging
 import logging.handlers
 import os.path
 import logging
 import logging.handlers
-from cStringIO import StringIO
 
 from ganeti import constants
 from ganeti import compat
 
 from ganeti import constants
 from ganeti import compat
+from ganeti import pathutils
 
 
 class _ReopenableLogHandler(logging.handlers.BaseRotatingHandler):
 
 
 class _ReopenableLogHandler(logging.handlers.BaseRotatingHandler):
@@ -188,7 +188,8 @@ def _ReopenLogFiles(handlers):
 
 def SetupLogging(logfile, program, debug=0, stderr_logging=False,
                  multithreaded=False, syslog=constants.SYSLOG_USAGE,
 
 def SetupLogging(logfile, program, debug=0, stderr_logging=False,
                  multithreaded=False, syslog=constants.SYSLOG_USAGE,
-                 console_logging=False, root_logger=None):
+                 console_logging=False, root_logger=None,
+                 verbose=True):
   """Configures the logging module.
 
   @type logfile: str
   """Configures the logging module.
 
   @type logfile: str
@@ -212,6 +213,8 @@ def SetupLogging(logfile, program, debug=0, stderr_logging=False,
       the system console if logging fails
   @type root_logger: logging.Logger
   @param root_logger: Root logger to use (for unittests)
       the system console if logging fails
   @type root_logger: logging.Logger
   @param root_logger: Root logger to use (for unittests)
+  @type verbose: boolean
+  @param verbose: whether to log at 'info' level already (logfile logging only)
   @raise EnvironmentError: if we can't open the log file and
       syslog/stderr logging is disabled
   @rtype: callable
   @raise EnvironmentError: if we can't open the log file and
       syslog/stderr logging is disabled
   @rtype: callable
@@ -267,8 +270,10 @@ def SetupLogging(logfile, program, debug=0, stderr_logging=False,
       logfile_handler.setFormatter(formatter)
       if debug:
         logfile_handler.setLevel(logging.DEBUG)
       logfile_handler.setFormatter(formatter)
       if debug:
         logfile_handler.setLevel(logging.DEBUG)
-      else:
+      elif verbose:
         logfile_handler.setLevel(logging.INFO)
         logfile_handler.setLevel(logging.INFO)
+      else:
+        logfile_handler.setLevel(logging.WARN)
       root_logger.addHandler(logfile_handler)
       reopen_handlers.append(logfile_handler)
     except EnvironmentError:
       root_logger.addHandler(logfile_handler)
       reopen_handlers.append(logfile_handler)
     except EnvironmentError:
@@ -282,11 +287,13 @@ def SetupLogging(logfile, program, debug=0, stderr_logging=False,
 
 
 def SetupToolLogging(debug, verbose, threadname=False,
 
 
 def SetupToolLogging(debug, verbose, threadname=False,
-                     _root_logger=None, _stream=None):
+                     toolname=None):
   """Configures the logging module for tools.
 
   """Configures the logging module for tools.
 
-  All log messages are sent to stderr.
+  All log messages are sent to the tools.log logfile.
 
 
+  @type toolname: string
+  @param toolname: name of the tool that's logging
   @type debug: boolean
   @param debug: Disable log message filtering
   @type verbose: boolean
   @type debug: boolean
   @param debug: Disable log message filtering
   @type verbose: boolean
@@ -295,32 +302,18 @@ def SetupToolLogging(debug, verbose, threadname=False,
   @param threadname: Whether to include thread name in output
 
   """
   @param threadname: Whether to include thread name in output
 
   """
-  if _root_logger is None:
-    root_logger = logging.getLogger("")
-  else:
-    root_logger = _root_logger
-
-  fmt = StringIO()
-  fmt.write("%(asctime)s:")
+  if not toolname:
+    toolname = "unspecified_tool"
 
 
-  if threadname:
-    fmt.write(" %(threadName)s")
-
-  if debug or verbose:
-    fmt.write(" %(levelname)s")
-
-  fmt.write(" %(message)s")
-
-  formatter = logging.Formatter(fmt.getvalue())
-
-  stderr_handler = logging.StreamHandler(_stream)
-  stderr_handler.setFormatter(formatter)
+  # 'SetupLogging' takes a quite unintuitive 'debug' option that
+  # is '0' for 'log higher than debug level' and '1' for
+  # 'log at NOSET' level. Hence this conversion.
+  debug_int = 0
   if debug:
   if debug:
-    stderr_handler.setLevel(logging.NOTSET)
-  elif verbose:
-    stderr_handler.setLevel(logging.INFO)
-  else:
-    stderr_handler.setLevel(logging.WARNING)
+    debug_int = 1
 
 
-  root_logger.setLevel(logging.NOTSET)
-  root_logger.addHandler(stderr_handler)
+  SetupLogging(pathutils.LOG_TOOLS,
+               program=toolname,
+               debug=debug_int,
+               multithreaded=threadname,
+               verbose=verbose)
index a5d98e9..c568b96 100755 (executable)
@@ -204,70 +204,5 @@ class TestSetupLogging(unittest.TestCase):
     self.assertTrue(utils.ReadFile(logfile2).endswith("This is a test\n"))
 
 
     self.assertTrue(utils.ReadFile(logfile2).endswith("This is a test\n"))
 
 
-class TestSetupToolLogging(unittest.TestCase):
-  def test(self):
-    error_name = logging.getLevelName(logging.ERROR)
-    warn_name = logging.getLevelName(logging.WARNING)
-    info_name = logging.getLevelName(logging.INFO)
-    debug_name = logging.getLevelName(logging.DEBUG)
-
-    for debug in [False, True]:
-      for verbose in [False, True]:
-        logger = logging.Logger("TestLogger")
-        buf = StringIO()
-
-        utils.SetupToolLogging(debug, verbose, _root_logger=logger, _stream=buf)
-
-        logger.error("level=error")
-        logger.warning("level=warning")
-        logger.info("level=info")
-        logger.debug("level=debug")
-
-        lines = buf.getvalue().splitlines()
-
-        self.assertTrue(compat.all(line.count(":") == 3 for line in lines))
-
-        messages = [line.split(":", 3)[-1].strip() for line in lines]
-
-        if debug:
-          self.assertEqual(messages, [
-            "%s level=error" % error_name,
-            "%s level=warning" % warn_name,
-            "%s level=info" % info_name,
-            "%s level=debug" % debug_name,
-            ])
-        elif verbose:
-          self.assertEqual(messages, [
-            "%s level=error" % error_name,
-            "%s level=warning" % warn_name,
-            "%s level=info" % info_name,
-            ])
-        else:
-          self.assertEqual(messages, [
-            "level=error",
-            "level=warning",
-            ])
-
-  def testThreadName(self):
-    thread_name = threading.currentThread().getName()
-
-    for enable_threadname in [False, True]:
-      logger = logging.Logger("TestLogger")
-      buf = StringIO()
-
-      utils.SetupToolLogging(True, True, threadname=enable_threadname,
-                             _root_logger=logger, _stream=buf)
-
-      logger.debug("test134042376")
-
-      lines = buf.getvalue().splitlines()
-      self.assertEqual(len(lines), 1)
-
-      if enable_threadname:
-        self.assertTrue((" %s " % thread_name) in lines[0])
-      else:
-        self.assertTrue(thread_name not in lines[0])
-
-
 if __name__ == "__main__":
   testutils.GanetiTestProgram()
 if __name__ == "__main__":
   testutils.GanetiTestProgram()