Merge lp:~tcole/wsgi-oops/configurable-result-tracking into lp:wsgi-oops

[Tim Cole]

Add hooks for request completion tracking.

This lets client code configure the mechanism used for tracking completion of requests for oops accounting. This will be needed in order for wsgi-oops to be compatible with the IBodyProducer wsgi hack.

[Martin Albisetti (beuno)]

review: Approve

[Stuart Colville (muffinresearch)]

Looks good, tests pass.

review: Approve

=== modified file 'canonical/oops/serializer.py'
--- canonical/oops/serializer.py	2010-07-06 15:11:35 +0000
+++ canonical/oops/serializer.py	2010-12-21 03:16:10 +0000
@@ -55,6 +55,8 @@
 import weakref
 import zipfile
 
+from canonical.oops.utils import resolve_name
+
 from pprint import pformat
 try:
     from canonical.versioninfo import version_info
@@ -70,10 +72,7 @@
 
 def create(serializer_factory_name, *args, **kw):
     """Creates a serializer instance using the named factory."""
-    factory_path = serializer_factory_name.split(".")
-    module_name = ".".join(factory_path[:-1])
-    module = __import__(module_name)
-    factory = reduce(lambda m, n: getattr(m, n), factory_path[1:], module)
+    factory = resolve_name(serializer_factory_name)
     return factory(*args, **kw)
 
 

=== modified file 'canonical/oops/tests/test_wsgi.py'
--- canonical/oops/tests/test_wsgi.py	2010-07-06 15:45:29 +0000
+++ canonical/oops/tests/test_wsgi.py	2010-12-21 03:16:10 +0000
@@ -20,6 +20,8 @@
 from canonical.oops.tests import testcase
 from canonical.oops import wsgi
 from paste.fixture import TestApp
+from canonical.oops.wsgi import track_iterator_completion
+
 
 class TestSimpleWSGIOops(testcase.WSGIOopsTestCase):
     """ simple test of wsgi oops just test for all ok and error """
@@ -41,6 +43,14 @@
         self.assertFalse(self.oops.wanted)
 
 
+def track_completion(body, on_completion):
+    """Prove that we've been called by adding to the body."""
+    yield "OK"
+    iterator = track_iterator_completion(body, on_completion)
+    for chunk in iterator:
+        yield chunk
+
+
 class TestResultIteration(testcase.WSGIOopsTestCase):
     """Test iteration on results."""
 
@@ -48,7 +58,7 @@
         """Test that we start without waiting for body consumption."""
 
         def dummy_app(environ, start_response):
-            """Produce a response with infinite garbage."""
+            """Produce a brief response."""
             start_response("200 OK", {}, exc_info=None)
             return ["foobar"]
 
@@ -62,6 +72,25 @@
         app({}, dummy_start_response)
         self.assertTrue(started[0])
 
+    def test_result_tracking(self):
+        """Test that our own result tracker gets called."""
+
+        def dummy_app(environ, start_response):
+            """Produce a simple response."""
+            start_response("200 OK", [('Content-Type', 'text/plain')],
+                           exc_info=None)
+            return ["foobar"]
+
+        def dummy_start_response(status, headers, exc_info=None):
+            """Do nothing."""
+
+        name = "canonical.oops.tests.test_wsgi.track_completion"
+        app = wsgi.OopsWare(dummy_app, self.tmp_dir,
+                            completion_tracker_name=name)
+        body = app({}, dummy_start_response)
+        result = list(body)
+        self.assertEqual(result, ["OK", "foobar"])
+
     def _generate_garbage(self):
         """Generate an infinite sequence of garbage until closed."""
         while True:

=== added file 'canonical/oops/utils.py'
--- canonical/oops/utils.py	1970-01-01 00:00:00 +0000
+++ canonical/oops/utils.py	2010-12-21 03:16:10 +0000
@@ -0,0 +1,25 @@
+# Copyright 2008, 2009 Canonical Ltd.
+#
+# This file is part of wsgi-oops.
+#
+# wsgi-oops is free software: you can redistribute it and/or modify
+# it under the terms of the GNU Lesser General Public License version 3
+# as published by the Free Software Foundation.
+#
+# wsgi-oops is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public License
+# along with wsgi-oops.  If not, see <http://www.gnu.org/licenses/>.
+
+"""Miscellaneous utilities."""
+
+def resolve_name(name):
+    """Resolve a name in the python module hierarchy."""
+    path = name.split(".")
+    module_name = ".".join(path[:-1])
+    module = __import__(module_name)
+    value = reduce(getattr, path[1:], module)
+    return value

=== modified file 'canonical/oops/wsgi.py'
--- canonical/oops/wsgi.py	2010-07-06 15:30:55 +0000
+++ canonical/oops/wsgi.py	2010-12-21 03:16:10 +0000
@@ -25,7 +25,7 @@
 """
 
 __metaclass__ = type
-__all__ = ["OopsWare"]
+__all__ = ["OopsWare", "track_iterator_completion"]
 
 import sys
 import time
@@ -36,9 +36,33 @@
 from canonical.oops import serializer
 from canonical.oops.oops        import (
     OOPS, OOPSLog, OOPSMetaData, OOPSLoggingHandler )
+from canonical.oops.utils import resolve_name
 from canonical.oops.stormtracer import OOPSStorm, OOPSStormTracer
 
 DEFAULT_SERIALIZER_FACTORY = 'canonical.oops.serializer.OOPSBz2Serializer'
+DEFAULT_COMPLETION_TRACKER = 'canonical.oops.wsgi.track_iterator_completion'
+
+def track_iterator_completion(body, on_completion):
+    """Wrap the given body iterator in one that tracks completion."""
+
+    def iterate():
+        """Track iterator completion."""
+        try:
+            yield None # throwaway value for priming
+            for chunk in body:
+                yield chunk
+        except GeneratorExit:
+            close = getattr(body, 'close', lambda: None)
+            close()
+            on_completion(None, None, None)
+        except:
+            on_completion(*sys.exc_info())
+        else:
+            on_completion(None, None, None)
+
+    iterator = iterate()
+    iterator.next() # prime generator
+    return iterator
 
 
 class OopsWare(object):
@@ -48,6 +72,7 @@
     """
     def __init__(self, app, oops_dir,
                  serializer_factory_name=DEFAULT_SERIALIZER_FACTORY,
+                 completion_tracker_name=DEFAULT_COMPLETION_TRACKER,
                  debug=False, key="appserver"):
         """
         Initialise the middleware for oops.
@@ -66,14 +91,9 @@
         fh = debug and sys.stderr or None
         self.serial = serializer.create(serializer_factory_name,
                                         key, oops_dir, fh=fh)
+        self.track_completion = resolve_name(completion_tracker_name)
 
     def __call__(self, environ, start_response):
-        """Call do_request and pump once to start request processing."""
-        gen = self.do_request(environ, start_response)
-        gen.next() # first value yielded is meant to be discarded
-        return gen
-
-    def do_request(self, environ, start_response):
         """
         Capture exceptions during WSGI application execution and dump
         oopses as needed.
@@ -123,37 +143,31 @@
         setattr(trap_response, 'oops', oops)
 
         metadata["request-start"] = time.time()
-        try:
-            body = self.app(environ, trap_response)
-            yield "" # thrown away by __call__
-            for chunk in body:
-                yield chunk
-        except GeneratorExit:
-            close = getattr(body, 'close', None)
-            if close:
-                close()
-        except:
-            oops.keep()
-            exc_info = sys.exc_info()
-            try:
-                message = "Unhandled WSGI application exception (OOPS %s)" % \
-                          oops.id
+
+        def on_completion(ex_type, ex_value, ex_tb):
+            """Handle request completion."""
+            if ex_type is not None:
+                oops.keep()
+                message = "Unhandled WSGI application exception " \
+                          "(OOPS %s)" % (oops.id,)
+                exc_info = (ex_type, ex_value, ex_tb)
                 root_logger.error(message, exc_info=exc_info)
-                raise exc_info[0], exc_info[1], exc_info[2]
-            finally:
-                exc_info = None
-        finally:
+
             metadata["request-end"] = time.time()
             if environ.has_key('dump-oops') and not oops.wanted:
-                oops.keep()
-                root_logger.error("Forced dump of OOPS %s" % oops.id)
-            if oops.wanted:
                 try:
                     self.serial.dump(oops)
                 except:
                     exc_info = sys.exc_info()
-                    try:
-                        message = "Error when dumping OOPS %s" % oops.id
-                        root_logger.error(message, exc_info=exc_info)
-                    finally:
-                        exc_info = None
+                try:
+                    message = "Error when dumping OOPS %s" % oops.id
+                    root_logger.error(message, exc_info=exc_info)
+                finally:
+                    exc_info = None
+
+        try:
+            body = self.app(environ, trap_response)
+        except Exception:
+            on_completion(*sys.exc_info())
+        else:
+            return self.track_completion(body, on_completion)