Move logging config options into the log module

We learned a lesson in Nova - it's best to declare and use config
options within a single module if possible. Globally declared and
use config options grow like weeds and it becomes harder to find out
if, where and how individual options are used.

Strangely, in cfg itself, we randomly declare a bunch of logging options
which are only used within the openstack.common.log module - let's move
the options there and remove the CommonConfigOpts class before they
become part of the API we commit to when oslo-config is released.

A minor detail in the patch - the logfile and logdir options are already
deprecated in favour of log_file and log_dir, but we never got around to
removing all other traces of the deprecated options.

Change-Id: I3913ea54465658d93dc56e014dfe5d911b0541d6

4 files changed, 123 insertions, 142 deletions

diff --git a/openstack/common/cfg.py b/openstack/common/cfg.py
index 8c965c1..0d8e6da 100644
--- a/openstack/common/cfg.py
+++ b/openstack/common/cfg.py
@@ -217,7 +217,7 @@ log files::
         ...
      ]
 
-This module also contains a global instance of the CommonConfigOpts class
+This module also contains a global instance of the ConfigOpts class
 in order to support a common usage pattern in OpenStack::
 
     from openstack.common import cfg
@@ -1728,64 +1728,4 @@ class ConfigOpts(collections.Mapping):
             return value
 
 
-class CommonConfigOpts(ConfigOpts):
-
-    DEFAULT_LOG_FORMAT = "%(asctime)s %(levelname)8s [%(name)s] %(message)s"
-    DEFAULT_LOG_DATE_FORMAT = "%Y-%m-%d %H:%M:%S"
-
-    common_cli_opts = [
-        BoolOpt('debug',
-                short='d',
-                default=False,
-                help='Print debugging output (set logging level to '
-                     'DEBUG instead of default WARNING level).'),
-        BoolOpt('verbose',
-                short='v',
-                default=False,
-                help='Print more verbose output (set logging level to '
-                     'INFO instead of default WARNING level).'),
-    ]
-
-    logging_cli_opts = [
-        StrOpt('log-config',
-               metavar='PATH',
-               help='If this option is specified, the logging configuration '
-                    'file specified is used and overrides any other logging '
-                    'options specified. Please see the Python logging module '
-                    'documentation for details on logging configuration '
-                    'files.'),
-        StrOpt('log-format',
-               default=DEFAULT_LOG_FORMAT,
-               metavar='FORMAT',
-               help='A logging.Formatter log message format string which may '
-                    'use any of the available logging.LogRecord attributes. '
-                    'Default: %(default)s'),
-        StrOpt('log-date-format',
-               default=DEFAULT_LOG_DATE_FORMAT,
-               metavar='DATE_FORMAT',
-               help='Format string for %%(asctime)s in log records. '
-                    'Default: %(default)s'),
-        StrOpt('log-file',
-               metavar='PATH',
-               deprecated_name='logfile',
-               help='(Optional) Name of log file to output to. '
-                    'If not set, logging will go to stdout.'),
-        StrOpt('log-dir',
-               deprecated_name='logdir',
-               help='(Optional) The directory to keep log files in '
-                    '(will be prepended to --log-file)'),
-        BoolOpt('use-syslog',
-                default=False,
-                help='Use syslog for logging.'),
-        StrOpt('syslog-log-facility',
-               default='LOG_USER',
-               help='syslog facility to receive log lines')
-    ]
-
-    def __init__(self):
-        super(CommonConfigOpts, self).__init__()
-        self.register_cli_opts(self.common_cli_opts)
-        self.register_cli_opts(self.logging_cli_opts)
-
-
-CONF = CommonConfigOpts()
+CONF = ConfigOpts()
diff --git a/openstack/common/log.py b/openstack/common/log.py
index 0be34a5..fad5731 100644
--- a/openstack/common/log.py
+++ b/openstack/common/log.py
@@ -47,6 +47,67 @@ from openstack.common import local
 from openstack.common import notifier
 
 
+_DEFAULT_LOG_FORMAT = "%(asctime)s %(levelname)8s [%(name)s] %(message)s"
+_DEFAULT_LOG_DATE_FORMAT = "%Y-%m-%d %H:%M:%S"
+
+common_cli_opts = [
+    cfg.BoolOpt('debug',
+                short='d',
+                default=False,
+                help='Print debugging output (set logging level to '
+                     'DEBUG instead of default WARNING level).'),
+    cfg.BoolOpt('verbose',
+                short='v',
+                default=False,
+                help='Print more verbose output (set logging level to '
+                     'INFO instead of default WARNING level).'),
+]
+
+logging_cli_opts = [
+    cfg.StrOpt('log-config',
+               metavar='PATH',
+               help='If this option is specified, the logging configuration '
+                    'file specified is used and overrides any other logging '
+                    'options specified. Please see the Python logging module '
+                    'documentation for details on logging configuration '
+                    'files.'),
+    cfg.StrOpt('log-format',
+               default=_DEFAULT_LOG_FORMAT,
+               metavar='FORMAT',
+               help='A logging.Formatter log message format string which may '
+                    'use any of the available logging.LogRecord attributes. '
+                    'Default: %(default)s'),
+    cfg.StrOpt('log-date-format',
+               default=_DEFAULT_LOG_DATE_FORMAT,
+               metavar='DATE_FORMAT',
+               help='Format string for %%(asctime)s in log records. '
+                    'Default: %(default)s'),
+    cfg.StrOpt('log-file',
+               metavar='PATH',
+               deprecated_name='logfile',
+               help='(Optional) Name of log file to output to. '
+                    'If not set, logging will go to stdout.'),
+    cfg.StrOpt('log-dir',
+               deprecated_name='logdir',
+               help='(Optional) The directory to keep log files in '
+                    '(will be prepended to --log-file)'),
+    cfg.BoolOpt('use-syslog',
+                default=False,
+                help='Use syslog for logging.'),
+    cfg.StrOpt('syslog-log-facility',
+               default='LOG_USER',
+               help='syslog facility to receive log lines')
+]
+
+generic_log_opts = [
+    cfg.BoolOpt('use_stderr',
+                default=True,
+                help='Log output to standard error'),
+    cfg.StrOpt('logfile_mode',
+               default='0644',
+               help='Default file mode used when creating log files'),
+]
+
 log_opts = [
     cfg.StrOpt('logging_context_format_string',
                default='%(asctime)s.%(msecs)03d %(levelname)s %(name)s '
@@ -94,24 +155,9 @@ log_opts = [
                     'format it like this'),
 ]
 
-
-generic_log_opts = [
-    cfg.StrOpt('logdir',
-               default=None,
-               help='Log output to a per-service log file in named directory'),
-    cfg.StrOpt('logfile',
-               default=None,
-               help='Log output to a named file'),
-    cfg.BoolOpt('use_stderr',
-                default=True,
-                help='Log output to standard error'),
-    cfg.StrOpt('logfile_mode',
-               default='0644',
-               help='Default file mode used when creating log files'),
-]
-
-
 CONF = cfg.CONF
+CONF.register_cli_opts(common_cli_opts)
+CONF.register_cli_opts(logging_cli_opts)
 CONF.register_opts(generic_log_opts)
 CONF.register_opts(log_opts)
 
@@ -149,8 +195,8 @@ def _get_binary_name():
 
 
 def _get_log_file_path(binary=None):
-    logfile = CONF.log_file or CONF.logfile
-    logdir = CONF.log_dir or CONF.logdir
+    logfile = CONF.log_file
+    logdir = CONF.log_dir
 
     if logfile and not logdir:
         return logfile
diff --git a/tests/unit/test_cfg.py b/tests/unit/test_cfg.py
index b2c14ec..8eaaf67 100644
--- a/tests/unit/test_cfg.py
+++ b/tests/unit/test_cfg.py
@@ -1573,62 +1573,6 @@ class OptDumpingTestCase(BaseTestCase):
                           ])
 
 
-class CommonOptsTestCase(BaseTestCase):
-
-    def setUp(self):
-        super(CommonOptsTestCase, self).setUp()
-        self.conf = CommonConfigOpts()
-
-    def test_print_help(self):
-        f = StringIO.StringIO()
-        self.conf([])
-        self.conf.print_help(file=f)
-        self.assertTrue('debug' in f.getvalue())
-        self.assertTrue('verbose' in f.getvalue())
-        self.assertTrue('log-config' in f.getvalue())
-        self.assertTrue('log-format' in f.getvalue())
-
-    def test_debug_verbose(self):
-        self.conf(['--debug', '--verbose'])
-
-        self.assertEquals(self.conf.debug, True)
-        self.assertEquals(self.conf.verbose, True)
-
-    def test_logging_opts(self):
-        self.conf([])
-
-        self.assertTrue(self.conf.log_config is None)
-        self.assertTrue(self.conf.log_file is None)
-        self.assertTrue(self.conf.log_dir is None)
-
-        self.assertEquals(self.conf.log_format,
-                          CommonConfigOpts.DEFAULT_LOG_FORMAT)
-        self.assertEquals(self.conf.log_date_format,
-                          CommonConfigOpts.DEFAULT_LOG_DATE_FORMAT)
-
-        self.assertEquals(self.conf.use_syslog, False)
-
-    def test_log_file(self):
-        log_file = '/some/path/foo-bar.log'
-        self.conf(['--log-file', log_file])
-        self.assertEquals(self.conf.log_file, log_file)
-
-    def test_logfile_deprecated(self):
-        logfile = '/some/other/path/foo-bar.log'
-        self.conf(['--logfile', logfile])
-        self.assertEquals(self.conf.log_file, logfile)
-
-    def test_log_dir(self):
-        log_dir = '/some/path/'
-        self.conf(['--log-dir', log_dir])
-        self.assertEquals(self.conf.log_dir, log_dir)
-
-    def test_logdir_deprecated(self):
-        logdir = '/some/other/path/'
-        self.conf(['--logdir', logdir])
-        self.assertEquals(self.conf.log_dir, logdir)
-
-
 class ConfigParserTestCase(unittest.TestCase):
     def test_no_section(self):
         with tempfile.NamedTemporaryFile() as tmpfile:
diff --git a/tests/unit/test_log.py b/tests/unit/test_log.py
index a2ba31a..451fb4f 100644
--- a/tests/unit/test_log.py
+++ b/tests/unit/test_log.py
@@ -1,6 +1,7 @@
 import cStringIO
 import exceptions
 import logging
+import StringIO
 import subprocess
 import sys
 import textwrap
@@ -74,22 +75,22 @@ class LoggerTestCase(test_utils.BaseTestCase):
 
 class LogHandlerTestCase(test_utils.BaseTestCase):
     def test_log_path_logdir(self):
-        self.config(logdir='/some/path', logfile=None)
+        self.config(log_dir='/some/path', log_file=None)
         self.assertEquals(log._get_log_file_path(binary='foo-bar'),
                           '/some/path/foo-bar.log')
 
     def test_log_path_logfile(self):
-        self.config(logfile='/some/path/foo-bar.log')
+        self.config(log_file='/some/path/foo-bar.log')
         self.assertEquals(log._get_log_file_path(binary='foo-bar'),
                           '/some/path/foo-bar.log')
 
     def test_log_path_none(self):
-        self.config(logdir=None, logfile=None)
+        self.config(log_dir=None, log_file=None)
         self.assertTrue(log._get_log_file_path(binary='foo-bar') is None)
 
     def test_log_path_logfile_overrides_logdir(self):
-        self.config(logdir='/some/other/path',
-                    logfile='/some/path/foo-bar.log')
+        self.config(log_dir='/some/other/path',
+                    log_file='/some/path/foo-bar.log')
         self.assertEquals(log._get_log_file_path(binary='foo-bar'),
                           '/some/path/foo-bar.log')
 
@@ -328,3 +329,53 @@ class SetDefaultsTestCase(test_utils.BaseTestCase):
         log.set_defaults(logging_context_format_string=my_default)
         self.conf([])
         self.assertEquals(self.conf.logging_context_format_string, my_default)
+
+
+class LogConfigOptsTestCase(test_utils.BaseTestCase):
+
+    def test_print_help(self):
+        f = StringIO.StringIO()
+        CONF([])
+        CONF.print_help(file=f)
+        self.assertTrue('debug' in f.getvalue())
+        self.assertTrue('verbose' in f.getvalue())
+        self.assertTrue('log-config' in f.getvalue())
+        self.assertTrue('log-format' in f.getvalue())
+
+    def test_debug_verbose(self):
+        CONF(['--debug', '--verbose'])
+
+        self.assertEquals(CONF.debug, True)
+        self.assertEquals(CONF.verbose, True)
+
+    def test_logging_opts(self):
+        CONF([])
+
+        self.assertTrue(CONF.log_config is None)
+        self.assertTrue(CONF.log_file is None)
+        self.assertTrue(CONF.log_dir is None)
+
+        self.assertEquals(CONF.log_format, log._DEFAULT_LOG_FORMAT)
+        self.assertEquals(CONF.log_date_format, log._DEFAULT_LOG_DATE_FORMAT)
+
+        self.assertEquals(CONF.use_syslog, False)
+
+    def test_log_file(self):
+        log_file = '/some/path/foo-bar.log'
+        CONF(['--log-file', log_file])
+        self.assertEquals(CONF.log_file, log_file)
+
+    def test_logfile_deprecated(self):
+        logfile = '/some/other/path/foo-bar.log'
+        CONF(['--logfile', logfile])
+        self.assertEquals(CONF.log_file, logfile)
+
+    def test_log_dir(self):
+        log_dir = '/some/path/'
+        CONF(['--log-dir', log_dir])
+        self.assertEquals(CONF.log_dir, log_dir)
+
+    def test_logdir_deprecated(self):
+        logdir = '/some/other/path/'
+        CONF(['--logdir', logdir])
+        self.assertEquals(CONF.log_dir, logdir)