From f24a904dd303bc2f897950ad3ce27607f6f9a4db Mon Sep 17 00:00:00 2001 From: Pavel Boldin Date: Sat, 24 Jan 2015 01:39:51 +0200 Subject: [PATCH] Add a Python context that logs exceptions The Python context named `ExceptionLogger` can be used to route exceptions to a specified logging service. Change-Id: I85d74339ac8857e7d577302d29c5951b76a1d127 --- rally/benchmark/context/flavors.py | 8 ++----- rally/benchmark/context/network.py | 8 +++---- rally/benchmark/context/roles.py | 7 ++---- rally/benchmark/context/secgroup.py | 7 ++---- rally/common/log.py | 35 +++++++++++++++++++++++++++++ tests/unit/test_log.py | 24 ++++++++++++++++++++ 6 files changed, 69 insertions(+), 20 deletions(-) diff --git a/rally/benchmark/context/flavors.py b/rally/benchmark/context/flavors.py index 1b85b2f7b8..4085c8b80f 100755 --- a/rally/benchmark/context/flavors.py +++ b/rally/benchmark/context/flavors.py @@ -98,14 +98,10 @@ class FlavorsGenerator(base.Context): """Delete created flavors.""" clients = osclients.Clients(self.context["admin"]["endpoint"]) for flavor in self.context["flavors"].values(): - try: + with logging.ExceptionLogger( + LOG, _("Can't delete flavor %s") % flavor["id"]): rutils.retry(3, clients.nova().flavors.delete, flavor["id"]) LOG.debug("Flavor is deleted %s" % flavor["id"]) - except Exception as e: - LOG.error( - "Can't delete flavor %s: %s" % (flavor["id"], e.message)) - if logging.is_debug(): - LOG.exception(e) class FlavorConfig(dict): diff --git a/rally/benchmark/context/network.py b/rally/benchmark/context/network.py index 5cabab3233..b525389f6f 100644 --- a/rally/benchmark/context/network.py +++ b/rally/benchmark/context/network.py @@ -71,8 +71,8 @@ class Network(base.Context): def cleanup(self): for tenant_id, tenant_ctx in six.iteritems(self.context["tenants"]): for network in tenant_ctx.get("networks", []): - try: + with logging.ExceptionLogger( + LOG, + _("Failed to delete network for tenant %s") + % tenant_id): self.net_wrapper.delete_network(network) - except Exception as e: - LOG.error("Failed to delete network for tenant %s\n" - " reason: %s" % (tenant_id, e)) diff --git a/rally/benchmark/context/roles.py b/rally/benchmark/context/roles.py index 4d0b231688..3ed256be14 100644 --- a/rally/benchmark/context/roles.py +++ b/rally/benchmark/context/roles.py @@ -74,13 +74,10 @@ class RoleGenerator(base.Context): client = osclients.Clients(admin_endpoint).keystone() for user in self.context["users"]: - try: + with logging.ExceptionLogger( + LOG, _("Failed to remove role: %s") % role["id"]): client.roles.remove_user_role( user["id"], role["id"], tenant=user["tenant_id"]) - except Exception as ex: - LOG.warning("Failed to remove role: %(role_id)s. " - "Exception: %(ex)s" % - {"role_id": role["id"], "ex": ex}) @rutils.log_task_wrapper(LOG.info, _("Enter context: `roles`")) def setup(self): diff --git a/rally/benchmark/context/secgroup.py b/rally/benchmark/context/secgroup.py index 5a7dbc99d6..9e7829b1a4 100644 --- a/rally/benchmark/context/secgroup.py +++ b/rally/benchmark/context/secgroup.py @@ -108,9 +108,6 @@ class AllowSSH(base.Context): @utils.log_task_wrapper(LOG.info, _("Exit context: `allow_ssh`")) def cleanup(self): for secgroup in self.secgroup: - try: + with logging.ExceptionLogger( + LOG, _("Unable to delete secgroup: %s.") % secgroup.id): secgroup.delete() - except Exception as ex: - LOG.warning("Unable to delete secgroup: %(group_id)s. " - "Exception: %(ex)s" % - {"group_id": secgroup.id, "ex": ex}) diff --git a/rally/common/log.py b/rally/common/log.py index 2d8c5b4d39..e4ecfc85f3 100644 --- a/rally/common/log.py +++ b/rally/common/log.py @@ -67,5 +67,40 @@ class RallyContextAdapter(oslogging.ContextAdapter): self.log(logging.RDEBUG, msg, *args, **kwargs) +class ExceptionLogger(object): + """Context that intercepts and logs exceptions. + + Usage:: + LOG = logging.getLogger(__name__) + ... + + def foobar(): + with ExceptionLogger(LOG, "foobar warning") as e: + return house_of_raising_exception() + + if e.exception: + raise e.exception # remove if not required + """ + + def __init__(self, logger, warn=None): + self.logger = logger + self.warn = warn + self.exception = None + + def __enter__(self): + return self + + def __exit__(self, type_, value, traceback): + if value: + self.exception = value + + if self.warn: + self.logger.warning(self.warn) + self.logger.debug(value) + if is_debug(): + self.logger.exception(value) + return True + + def is_debug(): return CONF.debug or CONF.rally_debug diff --git a/tests/unit/test_log.py b/tests/unit/test_log.py index 80cc26750e..30af0834eb 100644 --- a/tests/unit/test_log.py +++ b/tests/unit/test_log.py @@ -76,3 +76,27 @@ class LogRallyContaxtAdapter(test.TestCase): radapter.log.assert_called_once_with(mock_logging.RDEBUG, fake_msg) + + +class ExceptionLoggerTestCase(test.TestCase): + + @mock.patch("rally.common.log.is_debug") + def test_context(self, mock_is_debug): + # Prepare + mock_is_debug.return_value = True + + logger = mock.MagicMock() + exception = Exception() + + # Run + with log.ExceptionLogger(logger, "foo") as e: + raise exception + + # Assertions + logger.warning.assert_called_once_with("foo") + + logger.exception.assert_called_once_with(exception) + + logger.debug.assert_called_once_with(exception) + + self.assertEqual(e.exception, exception)