Change in vdsm[ovirt-3.3]: utils: Add unhandled exceptions logging decorator

nsoffer at redhat.com nsoffer at redhat.com
Mon Dec 9 17:19:21 UTC 2013


Hello Sergey Gotliv, Dan Kenigsberg,

I'd like you to do a code review.  Please visit

    http://gerrit.ovirt.org/22212

to review the following change.

Change subject: utils: Add unhandled exceptions logging decorator
......................................................................

utils: Add unhandled exceptions logging decorator

We have at least two places where exceptions in thread main function are
not handled, making debugging such threads impossible. This patch adds a
new decorator that make it easy to log tracebacks for unhandled
exceptions.

The recommended use case is to decorate a thread main function. This
ensure that we get a traceback for any error in the thread, including
errors in error handling code. This should be used on all in threads in
the system.

Note that the decorator only adds logging - it does not change the
program flow.  The unhandled exception is raise again so you can safely
use it on script main function, without changing the exit code. Do not
try to use this for ignoring exceptions.

See utilsTests for example usage.

Change-Id: I39265bac4f26fefa365bad080dafb6106145f846
Relates-To: https://bugzilla.redhat.com/1034741
Signed-off-by: Nir Soffer <nsoffer at redhat.com>
Reviewed-on: http://gerrit.ovirt.org/21778
Reviewed-by: Sergey Gotliv <sgotliv at redhat.com>
Reviewed-by: Dan Kenigsberg <danken at redhat.com>
---
M lib/vdsm/utils.py
M tests/utilsTests.py
2 files changed, 82 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/12/22212/1

diff --git a/lib/vdsm/utils.py b/lib/vdsm/utils.py
index d3e0c2b..ef0f512 100644
--- a/lib/vdsm/utils.py
+++ b/lib/vdsm/utils.py
@@ -599,6 +599,28 @@
     return int(stat.split(') ')[-1].split()[16])
 
 
+def traceback(on="", msg="Unhandled exception"):
+    """
+    Log a traceback for unhandled execptions.
+
+    :param on: Use specific logger name instead of root logger
+    :type on: str
+    :param msg: Use specified message for the exception
+    :type msg: str
+    """
+    def decorator(f):
+        @functools.wraps(f)
+        def wrapper(*a, **kw):
+            try:
+                return f(*a, **kw)
+            except Exception:
+                log = logging.getLogger(on)
+                log.exception(msg)
+                raise  # Do not swallow
+        return wrapper
+    return decorator
+
+
 def tobool(s):
     try:
         if s is None:
diff --git a/tests/utilsTests.py b/tests/utilsTests.py
index a6a222d..5f7c82d 100644
--- a/tests/utilsTests.py
+++ b/tests/utilsTests.py
@@ -17,7 +17,9 @@
 #
 # Refer to the README and COPYING files for full details of the license
 #
+import contextlib
 import errno
+import logging
 
 from testrunner import VdsmTestCase as TestCaseBase
 from vdsm import utils
@@ -80,3 +82,61 @@
 class GeneralUtilsTests(TestCaseBase):
     def testPanic(self):
         self.assertRaises(AssertionError, utils.panic, "panic test")
+
+
+ at contextlib.contextmanager
+def loghandler(handler, logger=""):
+    log = logging.getLogger(logger)
+    log.addHandler(handler)
+    try:
+        yield {}
+    finally:
+        log.removeHandler(handler)
+
+
+class TracebackTests(TestCaseBase):
+
+    def __init__(self, *a, **kw):
+        self.record = None
+        super(TestCaseBase, self).__init__(*a, **kw)
+
+    def testDefaults(self):
+        @utils.traceback()
+        def fail():
+            raise Exception
+        with loghandler(self):
+            self.assertRaises(Exception, fail)
+        self.assertEquals(self.record.name, "root")
+        self.assertTrue(self.record.exc_text is not None)
+
+    def testOn(self):
+        logger = "test"
+
+        @utils.traceback(on=logger)
+        def fail():
+            raise Exception
+        with loghandler(self, logger=logger):
+            self.assertRaises(Exception, fail)
+        self.assertEquals(self.record.name, logger)
+
+    def testMsg(self):
+        @utils.traceback(msg="WAT")
+        def fail():
+            raise Exception
+        with loghandler(self):
+            self.assertRaises(Exception, fail)
+        self.assertEquals(self.record.message, "WAT")
+
+    # Logging handler interface
+
+    level = logging.DEBUG
+
+    def acquire(self):
+        pass
+
+    def release(self):
+        pass
+
+    def handle(self, record):
+        assert self.record is None
+        self.record = record


-- 
To view, visit http://gerrit.ovirt.org/22212
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I39265bac4f26fefa365bad080dafb6106145f846
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.3
Gerrit-Owner: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Sergey Gotliv <sgotliv at redhat.com>


More information about the vdsm-patches mailing list