diff --git a/etc/internal-client.conf-sample b/etc/internal-client.conf-sample index a3be2eaa41..7ded5fd8ad 100644 --- a/etc/internal-client.conf-sample +++ b/etc/internal-client.conf-sample @@ -2,7 +2,9 @@ # swift_dir = /etc/swift # user = swift # You can specify default log routing here if you want: -# log_name = swift +# Note: the 'set' syntax is necessary to override the log_name that some +# daemons specify when instantiating an internal client. +# set log_name = swift # log_facility = LOG_LOCAL0 # log_level = INFO # log_address = /dev/log diff --git a/swift/cli/container_deleter.py b/swift/cli/container_deleter.py index 27759e48cd..ecd8657d14 100644 --- a/swift/cli/container_deleter.py +++ b/swift/cli/container_deleter.py @@ -133,7 +133,7 @@ def mark_for_deletion(swift, account, container, marker, end_marker, return enqueue_deletes() -def main(): +def main(args=None): parser = argparse.ArgumentParser( description=__doc__, formatter_class=argparse.RawTextHelpFormatter) @@ -156,10 +156,11 @@ def main(): parser.add_argument( '--timestamp', type=Timestamp, default=Timestamp.now(), help='delete all objects as of this time (default: now)') - args = parser.parse_args() + args = parser.parse_args(args) swift = InternalClient( - args.config, 'Swift Container Deleter', args.request_tries) + args.config, 'Swift Container Deleter', args.request_tries, + global_conf={'log_name': 'container-deleter-ic'}) for deleted, marker in mark_for_deletion( swift, args.account, args.container, args.marker, args.end_marker, args.prefix, args.timestamp): diff --git a/swift/common/utils.py b/swift/common/utils.py index 803b1aa19f..6a28d53b31 100644 --- a/swift/common/utils.py +++ b/swift/common/utils.py @@ -2387,11 +2387,18 @@ def get_logger(conf, name=None, log_to_console=False, log_route=None, log_statsd_metric_prefix = (empty-string) :param conf: Configuration dict to read settings from - :param name: Name of the logger + :param name: This value is used to populate the ``server`` field in the log + format, as the prefix for statsd messages, and as the default + value for ``log_route``; defaults to the ``log_name`` value in + ``conf``, if it exists, or to 'swift'. :param log_to_console: Add handler which writes to console on stderr :param log_route: Route for the logging, not emitted to the log, just used - to separate logging configurations + to separate logging configurations; defaults to the value + of ``name`` or whatever ``name`` defaults to. This value + is used as the name attribute of the + ``logging.LogAdapter`` that is returned. :param fmt: Override log format + :return: an instance of ``LogAdapter`` """ if not conf: conf = {} diff --git a/swift/container/reconciler.py b/swift/container/reconciler.py index ed677b2249..30bc3501c1 100644 --- a/swift/container/reconciler.py +++ b/swift/container/reconciler.py @@ -361,6 +361,7 @@ class ContainerReconciler(Daemon): """ Move objects that are in the wrong storage policy. """ + log_route = 'container-reconciler' def __init__(self, conf, logger=None, swift=None): self.conf = conf @@ -372,13 +373,15 @@ class ContainerReconciler(Daemon): conf_path = conf.get('__file__') or \ '/etc/swift/container-reconciler.conf' self.logger = logger or get_logger( - conf, log_route='container-reconciler') + conf, log_route=self.log_route) request_tries = int(conf.get('request_tries') or 3) self.swift = swift or InternalClient( conf_path, 'Swift Container Reconciler', request_tries, - use_replication_network=True) + use_replication_network=True, + global_conf={'log_name': '%s-ic' % conf.get( + 'log_name', self.log_route)}) self.swift_dir = conf.get('swift_dir', '/etc/swift') self.stats = defaultdict(int) self.last_stat_time = time.time() diff --git a/swift/container/sharder.py b/swift/container/sharder.py index eeaa12f2a9..db6565fd5d 100644 --- a/swift/container/sharder.py +++ b/swift/container/sharder.py @@ -665,9 +665,10 @@ DEFAULT_SHARDER_CONF = vars(ContainerSharderConf()) class ContainerSharder(ContainerSharderConf, ContainerReplicator): """Shards containers.""" + log_route = 'container-sharder' def __init__(self, conf, logger=None): - logger = logger or get_logger(conf, log_route='container-sharder') + logger = logger or get_logger(conf, log_route=self.log_route) ContainerReplicator.__init__(self, conf, logger=logger) ContainerSharderConf.__init__(self, conf) ContainerSharderConf.validate_conf(self) @@ -716,7 +717,9 @@ class ContainerSharder(ContainerSharderConf, ContainerReplicator): 'Swift Container Sharder', request_tries, allow_modify_pipeline=False, - use_replication_network=True) + use_replication_network=True, + global_conf={'log_name': '%s-ic' % conf.get( + 'log_name', self.log_route)}) except (OSError, IOError) as err: if err.errno != errno.ENOENT and \ not str(err).endswith(' not found'): diff --git a/swift/container/sync.py b/swift/container/sync.py index 54acc55f7c..4c50bf7bd6 100644 --- a/swift/container/sync.py +++ b/swift/container/sync.py @@ -156,13 +156,14 @@ class ContainerSync(Daemon): :param container_ring: If None, the /container.ring.gz will be loaded. This is overridden by unit tests. """ + log_route = 'container-sync' def __init__(self, conf, container_ring=None, logger=None): #: The dict of configuration values from the [container-sync] section #: of the container-server.conf. self.conf = conf #: Logger to use for container-sync log lines. - self.logger = logger or get_logger(conf, log_route='container-sync') + self.logger = logger or get_logger(conf, log_route=self.log_route) #: Path to the local device mount points. self.devices = conf.get('devices', '/srv/node') #: Indicates whether mount points should be verified as actual mount @@ -241,7 +242,9 @@ class ContainerSync(Daemon): try: self.swift = InternalClient( internal_client_conf, 'Swift Container Sync', request_tries, - use_replication_network=True) + use_replication_network=True, + global_conf={'log_name': '%s-ic' % conf.get( + 'log_name', self.log_route)}) except (OSError, IOError) as err: if err.errno != errno.ENOENT and \ not str(err).endswith(' not found'): diff --git a/swift/obj/expirer.py b/swift/obj/expirer.py index 772b36793b..ec53407aa5 100644 --- a/swift/obj/expirer.py +++ b/swift/obj/expirer.py @@ -73,10 +73,11 @@ class ObjectExpirer(Daemon): :param conf: The daemon configuration. """ + log_route = 'object-expirer' def __init__(self, conf, logger=None, swift=None): self.conf = conf - self.logger = logger or get_logger(conf, log_route='object-expirer') + self.logger = logger or get_logger(conf, log_route=self.log_route) self.interval = float(conf.get('interval') or 300) self.tasks_per_second = float(conf.get('tasks_per_second', 50.0)) @@ -135,7 +136,9 @@ class ObjectExpirer(Daemon): request_tries = int(self.conf.get('request_tries') or 3) self.swift = swift or InternalClient( self.ic_conf_path, 'Swift Object Expirer', request_tries, - use_replication_network=True) + use_replication_network=True, + global_conf={'log_name': '%s-ic' % self.conf.get( + 'log_name', self.log_route)}) self.processes = int(self.conf.get('processes', 0)) self.process = int(self.conf.get('process', 0)) diff --git a/test/unit/cli/test_container_deleter.py b/test/unit/cli/test_container_deleter.py index 6cf91d6bd6..b7819c9dd5 100644 --- a/test/unit/cli/test_container_deleter.py +++ b/test/unit/cli/test_container_deleter.py @@ -276,3 +276,13 @@ class TestContainerDeleter(unittest.TestCase): utils.Timestamp(ts) ) ) + + def test_init_internal_client_log_name(self): + with mock.patch( + 'swift.cli.container_deleter.InternalClient') \ + as mock_ic: + container_deleter.main(['a', 'c', '--request-tries', '2']) + mock_ic.assert_called_once_with( + '/etc/swift/internal-client.conf', + 'Swift Container Deleter', 2, + global_conf={'log_name': 'container-deleter-ic'}) diff --git a/test/unit/common/test_internal_client.py b/test/unit/common/test_internal_client.py index c25b1e7ddd..cad95df210 100644 --- a/test/unit/common/test_internal_client.py +++ b/test/unit/common/test_internal_client.py @@ -281,6 +281,103 @@ class TestInternalClient(unittest.TestCase): object_ring_path) self.assertEqual(client.auto_create_account_prefix, '-') + @mock.patch('swift.common.utils.HASH_PATH_SUFFIX', new=b'endcap') + @with_tempdir + def test_load_from_config_with_global_conf(self, tempdir): + account_ring_path = os.path.join(tempdir, 'account.ring.gz') + write_fake_ring(account_ring_path) + container_ring_path = os.path.join(tempdir, 'container.ring.gz') + write_fake_ring(container_ring_path) + object_ring_path = os.path.join(tempdir, 'object.ring.gz') + write_fake_ring(object_ring_path) + + # global_conf will override the 'x = y' syntax in conf file... + conf_path = os.path.join(tempdir, 'internal_client.conf') + conf_body = """ + [DEFAULT] + swift_dir = %s + log_name = conf-file-log-name + + [pipeline:main] + pipeline = catch_errors cache proxy-server + + [app:proxy-server] + use = egg:swift#proxy + auto_create_account_prefix = - + + [filter:cache] + use = egg:swift#memcache + + [filter:catch_errors] + use = egg:swift#catch_errors + log_name = catch-errors-log-name + """ % tempdir + with open(conf_path, 'w') as f: + f.write(dedent(conf_body)) + global_conf = {'log_name': 'global-conf-log-name'} + with patch_policies([StoragePolicy(0, 'legacy', True)]): + client = internal_client.InternalClient( + conf_path, 'test', 1, global_conf=global_conf) + self.assertEqual('global-conf-log-name', client.app.logger.server) + + # ...but the 'set x = y' syntax in conf file DEFAULT section will + # override global_conf + conf_body = """ + [DEFAULT] + swift_dir = %s + set log_name = conf-file-log-name + + [pipeline:main] + pipeline = catch_errors cache proxy-server + + [app:proxy-server] + use = egg:swift#proxy + auto_create_account_prefix = - + + [filter:cache] + use = egg:swift#memcache + + [filter:catch_errors] + use = egg:swift#catch_errors + log_name = catch-errors-log-name + """ % tempdir + with open(conf_path, 'w') as f: + f.write(dedent(conf_body)) + global_conf = {'log_name': 'global-conf-log-name'} + with patch_policies([StoragePolicy(0, 'legacy', True)]): + client = internal_client.InternalClient( + conf_path, 'test', 1, global_conf=global_conf) + self.assertEqual('conf-file-log-name', client.app.logger.server) + + # ...and the 'set x = y' syntax in conf file app section will override + # DEFAULT section and global_conf + conf_body = """ + [DEFAULT] + swift_dir = %s + set log_name = conf-file-log-name + + [pipeline:main] + pipeline = catch_errors cache proxy-server + + [app:proxy-server] + use = egg:swift#proxy + auto_create_account_prefix = - + + [filter:cache] + use = egg:swift#memcache + + [filter:catch_errors] + use = egg:swift#catch_errors + set log_name = catch-errors-log-name + """ % tempdir + with open(conf_path, 'w') as f: + f.write(dedent(conf_body)) + global_conf = {'log_name': 'global-conf-log-name'} + with patch_policies([StoragePolicy(0, 'legacy', True)]): + client = internal_client.InternalClient( + conf_path, 'test', 1, global_conf=global_conf) + self.assertEqual('catch-errors-log-name', client.app.logger.server) + def test_init(self): conf_path = 'some_path' app = object() diff --git a/test/unit/common/test_utils.py b/test/unit/common/test_utils.py index 97be1251f1..a5272a8b85 100644 --- a/test/unit/common/test_utils.py +++ b/test/unit/common/test_utils.py @@ -1757,6 +1757,27 @@ class TestUtils(unittest.TestCase): self.assertEqual(sio.getvalue(), 'test1\ntest3\ntest4\ntest6\n') + def test_get_logger_name_and_route(self): + logger = utils.get_logger({}, name='name', log_route='route') + self.assertEqual('route', logger.name) + self.assertEqual('name', logger.server) + logger = utils.get_logger({'log_name': 'conf-name'}, name='name', + log_route='route') + self.assertEqual('route', logger.name) + self.assertEqual('name', logger.server) + logger = utils.get_logger({'log_name': 'conf-name'}, log_route='route') + self.assertEqual('route', logger.name) + self.assertEqual('conf-name', logger.server) + logger = utils.get_logger({'log_name': 'conf-name'}) + self.assertEqual('conf-name', logger.name) + self.assertEqual('conf-name', logger.server) + logger = utils.get_logger({}) + self.assertEqual('swift', logger.name) + self.assertEqual('swift', logger.server) + logger = utils.get_logger({}, log_route='route') + self.assertEqual('route', logger.name) + self.assertEqual('swift', logger.server) + @with_tempdir def test_get_logger_sysloghandler_plumbing(self, tempdir): orig_sysloghandler = utils.ThreadSafeSysLogHandler @@ -5509,11 +5530,16 @@ class TestStatsdLogging(unittest.TestCase): self.assertEqual(logger.logger.statsd_client._prefix, 'some-name.') self.assertEqual(logger.logger.statsd_client._default_sample_rate, 1) + logger2 = utils.get_logger({'log_statsd_host': 'some.host.com'}, + 'other-name', log_route='some-route') logger.set_statsd_prefix('some-name.more-specific') self.assertEqual(logger.logger.statsd_client._prefix, 'some-name.more-specific.') + self.assertEqual(logger2.logger.statsd_client._prefix, + 'some-name.more-specific.') logger.set_statsd_prefix('') self.assertEqual(logger.logger.statsd_client._prefix, '') + self.assertEqual(logger2.logger.statsd_client._prefix, '') def test_get_logger_statsd_client_non_defaults(self): logger = utils.get_logger({ diff --git a/test/unit/container/test_reconciler.py b/test/unit/container/test_reconciler.py index bbf8138acb..cd8c7e45db 100644 --- a/test/unit/container/test_reconciler.py +++ b/test/unit/container/test_reconciler.py @@ -896,6 +896,22 @@ class TestReconciler(unittest.TestCase): self.assertRaises(ValueError, reconciler.ContainerReconciler, conf, self.logger, self.swift) + def test_init_internal_client_log_name(self): + def _do_test_init_ic_log_name(conf, exp_internal_client_log_name): + with mock.patch( + 'swift.container.reconciler.InternalClient') \ + as mock_ic: + reconciler.ContainerReconciler(conf) + mock_ic.assert_called_once_with( + '/etc/swift/container-reconciler.conf', + 'Swift Container Reconciler', 3, + global_conf={'log_name': exp_internal_client_log_name}, + use_replication_network=True) + + _do_test_init_ic_log_name({}, 'container-reconciler-ic') + _do_test_init_ic_log_name({'log_name': 'my-container-reconciler'}, + 'my-container-reconciler-ic') + def _mock_listing(self, objects): self.swift.parse(objects) self.fake_swift = self.reconciler.swift.app diff --git a/test/unit/container/test_sharder.py b/test/unit/container/test_sharder.py index ca5f73cbb2..75a8816aa5 100644 --- a/test/unit/container/test_sharder.py +++ b/test/unit/container/test_sharder.py @@ -187,7 +187,8 @@ class TestSharder(BaseTestSharder): mock_ic.assert_called_once_with( '/etc/swift/internal-client.conf', 'Swift Container Sharder', 3, allow_modify_pipeline=False, - use_replication_network=True) + use_replication_network=True, + global_conf={'log_name': 'container-sharder-ic'}) # non-default shard_container_threshold influences other defaults conf = {'shard_container_threshold': 20000000} @@ -201,7 +202,8 @@ class TestSharder(BaseTestSharder): mock_ic.assert_called_once_with( '/etc/swift/internal-client.conf', 'Swift Container Sharder', 3, allow_modify_pipeline=False, - use_replication_network=True) + use_replication_network=True, + global_conf={'log_name': 'container-sharder-ic'}) # non-default values conf = { @@ -262,7 +264,8 @@ class TestSharder(BaseTestSharder): mock_ic.assert_called_once_with( '/etc/swift/my-sharder-ic.conf', 'Swift Container Sharder', 2, allow_modify_pipeline=False, - use_replication_network=True) + use_replication_network=True, + global_conf={'log_name': 'container-sharder-ic'}) self.assertEqual(self.logger.get_lines_for_level('warning'), [ 'Option auto_create_account_prefix is deprecated. ' 'Configure auto_create_account_prefix under the ' @@ -385,6 +388,27 @@ class TestSharder(BaseTestSharder): ContainerSharder({}) self.assertIn('kaboom', str(cm.exception)) + def test_init_internal_client_log_name(self): + def _do_test_init_ic_log_name(conf, exp_internal_client_log_name): + with mock.patch( + 'swift.container.sharder.internal_client.InternalClient') \ + as mock_ic: + with mock.patch('swift.common.db_replicator.ring.Ring') \ + as mock_ring: + mock_ring.return_value = mock.MagicMock() + mock_ring.return_value.replica_count = 3 + ContainerSharder(conf) + mock_ic.assert_called_once_with( + '/etc/swift/internal-client.conf', + 'Swift Container Sharder', 3, + allow_modify_pipeline=False, + global_conf={'log_name': exp_internal_client_log_name}, + use_replication_network=True) + + _do_test_init_ic_log_name({}, 'container-sharder-ic') + _do_test_init_ic_log_name({'log_name': 'container-sharder-6021'}, + 'container-sharder-6021-ic') + def _assert_stats(self, expected, sharder, category): # assertEqual doesn't work with a defaultdict stats = sharder.stats['sharding'][category] diff --git a/test/unit/container/test_sync.py b/test/unit/container/test_sync.py index c1859626b0..dc1bead204 100644 --- a/test/unit/container/test_sync.py +++ b/test/unit/container/test_sync.py @@ -20,7 +20,7 @@ from textwrap import dedent import mock import errno -from swift.common.utils import Timestamp +from swift.common.utils import Timestamp, readconf from test.debug_logger import debug_logger from swift.container import sync from swift.common.db import DatabaseConnectionError @@ -154,9 +154,29 @@ class TestContainerSync(unittest.TestCase): sample_conf_filename = os.path.join( os.path.dirname(test.__file__), '../etc/internal-client.conf-sample') - with open(sample_conf_filename) as sample_conf_file: - sample_conf = sample_conf_file.read() - self.assertEqual(contents, sample_conf) + actual_conf = readconf(ConfigString(contents)) + expected_conf = readconf(sample_conf_filename) + actual_conf.pop('__file__') + expected_conf.pop('__file__') + self.assertEqual(expected_conf, actual_conf) + + def test_init_internal_client_log_name(self): + def _do_test_init_ic_log_name(conf, exp_internal_client_log_name): + with mock.patch( + 'swift.container.sync.InternalClient') \ + as mock_ic: + sync.ContainerSync(conf, container_ring='dummy object') + mock_ic.assert_called_once_with( + 'conf-path', + 'Swift Container Sync', 3, + global_conf={'log_name': exp_internal_client_log_name}, + use_replication_network=True) + + _do_test_init_ic_log_name({'internal_client_conf_path': 'conf-path'}, + 'container-sync-ic') + _do_test_init_ic_log_name({'internal_client_conf_path': 'conf-path', + 'log_name': 'my-container-sync'}, + 'my-container-sync-ic') def test_run_forever(self): # This runs runs_forever with fakes to succeed for two loops, the first diff --git a/test/unit/obj/test_expirer.py b/test/unit/obj/test_expirer.py index cb9e274d9b..73e6d94b7f 100644 --- a/test/unit/obj/test_expirer.py +++ b/test/unit/obj/test_expirer.py @@ -168,6 +168,22 @@ class TestObjectExpirer(TestCase): ]) self.assertEqual(x.expiring_objects_account, '-expiring_objects') + def test_init_internal_client_log_name(self): + def _do_test_init_ic_log_name(conf, exp_internal_client_log_name): + with mock.patch( + 'swift.obj.expirer.InternalClient') \ + as mock_ic: + expirer.ObjectExpirer(conf) + mock_ic.assert_called_once_with( + '/etc/swift/object-expirer.conf', + 'Swift Object Expirer', 3, + global_conf={'log_name': exp_internal_client_log_name}, + use_replication_network=True) + + _do_test_init_ic_log_name({}, 'object-expirer-ic') + _do_test_init_ic_log_name({'log_name': 'my-object-expirer'}, + 'my-object-expirer-ic') + def test_get_process_values_from_kwargs(self): x = expirer.ObjectExpirer({}) vals = { diff --git a/test/unit/proxy/test_server.py b/test/unit/proxy/test_server.py index d87086a619..96e7e6e798 100644 --- a/test/unit/proxy/test_server.py +++ b/test/unit/proxy/test_server.py @@ -2110,6 +2110,55 @@ class TestProxyServerConfigLoading(unittest.TestCase): app = self._write_conf_and_load_app(conf_sections) self._check_policy_options(app, exp_options, {}) + def test_log_name(self): + # defaults... + conf_sections = """ + [DEFAULT] + log_statsd_host = example.com + swift_dir = %s + + [pipeline:main] + pipeline = proxy-server + + [app:proxy-server] + use = egg:swift#proxy + """ % self.tempdir + conf_path = self._write_conf(dedent(conf_sections)) + + with mock.patch('swift.common.utils.StatsdClient') as mock_statsd: + app = loadapp(conf_path, allow_modify_pipeline=False) + # logger name is hard-wired 'proxy-server' + self.assertEqual('proxy-server', app.logger.name) + self.assertEqual('swift', app.logger.server) + mock_statsd.assert_called_once_with( + 'example.com', 8125, '', 'swift', 1.0, 1.0, + logger=app.logger.logger) + + conf_sections = """ + [DEFAULT] + log_name = test-name + log_statsd_host = example.com + swift_dir = %s + + [pipeline:main] + pipeline = proxy-server + + [app:proxy-server] + use = egg:swift#proxy + """ % self.tempdir + conf_path = self._write_conf(dedent(conf_sections)) + + with mock.patch('swift.common.utils.StatsdClient') as mock_statsd: + app = loadapp(conf_path, allow_modify_pipeline=False) + # logger name is hard-wired 'proxy-server' + self.assertEqual('proxy-server', app.logger.name) + # server is defined by log_name option + self.assertEqual('test-name', app.logger.server) + # statsd prefix is defined by log_name option + mock_statsd.assert_called_once_with( + 'example.com', 8125, '', 'test-name', 1.0, 1.0, + logger=app.logger.logger) + class TestProxyServerConfigStringLoading(TestProxyServerConfigLoading): # The proxy may be loaded from a conf string rather than a conf file, for