Merge lp:~ev/oops-repository/whoopsie-daisy into lp:oops-repository

[Evan]

Apologies for the accidental deletion there.

This branch:
 - Brings oops-repository up to the latest pycassa API.
 - Replaces the insert_bson function with insert_dict, as we want to inspect the contents of the OOPS in the WSGI code to make a determination as to how we're going to process it, and it doesn't make sense to parse the BSON string twice.
 - Adds a bucket method on the oops module to store crashes that are of the same issue in a single collection.
 - Counts the additions to these buckets at a day interval.
 - Adds tests.

[j.c.sackett (jcsackett)]

This largely looks okay to me, but I'm holding off on voting b/c I'm not certain what has changed in oopses.py; I can see the LoC count is different, and I think everything looks alright, but could you post a diff here of just that file against the current trunk?

Or, if someone with greater knowledge of the oops-repository code thinks this is good, that person is welcome to just mark approved and run with it.

review: Needs Information

[Evan (ev)]

http://paste.ubuntu.com/886125/

[j.c.sackett (jcsackett)]

Thanks so much.

This looks good to me.

review: Approve

[lp:~ev/oops-repository/whoopsie-daisy]

27. By Evan on 2012-03-23

Handle unicode data in OOPSes.

28. By Evan on 2012-03-23

Missed that comment in DESIGN.txt about all the columns being strings. Simplify the previous commit by just setting a default_validation_class.

[Evan (ev)]

I've added a commit to handle unicode data in OOPSes by setting a default_validation_class of utf8. The previous behavior was causing exceptions as submissions were made to Ubuntu's crash database:
https://pastebin.canonical.com/62948/

I originally implemented this by encoding unicode dictionary keys and values as utf8 in oops-repository, but then I noticed the comment in DESIGN.txt about the columns all being strings. If that interpretation is incorrect, do feel free to take r27 instead of r28.

[Robert Collins (lifeless)]

This is landable - we need to get you direct push rights, for now, perhaps jc can land it for you.

[j.c.sackett (jcsackett)]

I'll assume that counts as a vote for "Approve" on your pending, Robert, and I'm happy to land it.

=== modified file 'oopsrepository/cassandra.py'
--- oopsrepository/cassandra.py	2011-02-26 19:47:58 +0000
+++ oopsrepository/cassandra.py	2012-03-23 23:37:20 +0000
@@ -6,7 +6,7 @@
 
 """Things to ease working with cassandra."""
 
-from pycassa.cassandra.Cassandra import InvalidRequestException
+from pycassa.cassandra.ttypes import InvalidRequestException
 
 def workaround_1779(callable, *args, **kwargs):
     """Workaround cassandra not being able to do concurrent schema edits.
 
=== added file 'oopsrepository/oopses.py'
--- oopsrepository/oopses.py	1970-01-01 00:00:00 +0000
+++ oopsrepository/oopses.py	2012-03-23 23:37:20 +0000
@@ -0,0 +1,116 @@
+# oops-repository is Copyright 2011 Canonical Ltd.
+#
+# Canonical Ltd ("Canonical") distributes the oops-repository source code under
+# the GNU Affero General Public License, version 3 ("AGPLv3"). See the file
+# LICENSE in the source tree for more information.
+
+"""basic operations on oopses in the db."""
+
+import json
+import bson
+import time
+import uuid
+
+import pycassa
+from pycassa.index import create_index_expression, LT, create_index_clause
+from pycassa.cassandra.ttypes import IndexExpression
+
+DAY = 60*60*24
+MONTH = DAY*30
+
+def prune(config):
+    """Remove OOPSES that are over 30 days old."""
+    pool = pycassa.pool.ConnectionPool(config['keyspace'], config['host'])
+    dayoops_cf = pycassa.ColumnFamily(pool, 'DayOOPS')
+    oops_cf = pycassa.ColumnFamily(pool, 'OOPS')
+    # Find days to prune
+    days = set()
+    prune_to = time.strftime('%Y%m%d', time.gmtime(time.time() - MONTH))
+    for key, _ in dayoops_cf.get_range():
+        if key < prune_to:
+            days.add(key)
+    if not days:
+        return
+    # collect all the oopses (buffers all in memory; may want to make
+    # incremental in future)
+    batch_size = 10000
+    oopses = []
+    for day in days:
+        columns_found = batch_size
+        start_col = ''
+        while columns_found==batch_size:
+            columns = dayoops_cf.get(day, column_start=start_col)
+            columns_found = len(columns)
+            for column, oopsid in columns.items():
+                start_col = column
+                oopses.append(oopsid)
+    # Remove the oopses
+    batch = oops_cf.batch()
+    map(batch.remove, oopses)
+    batch.send()
+    # Clean out the days aggregates
+    # Clean out the days index
+    batch = dayoops_cf.batch()
+    map(batch.remove, days)
+    batch.send()
+
+def insert(config, oopsid, oops_json, user_token=None):
+    """Insert an OOPS into the system.
+    
+    :return: The day which the oops was filed under.
+    """
+    # make sure the oops report is a json dict, and break out each key to a
+    # separate column. For now, rather than worrying about typed column values
+    # we just coerce them all to strings.
+    oops_dict = json.loads(oops_json)
+    assert isinstance(oops_dict, dict)
+    insert_dict = {}
+    for key, value in oops_dict.items():
+        insert_dict[key] = json.dumps(value)
+    return _insert(config, oopsid, insert_dict, user_token)
+
+def insert_dict(config, oopsid, oops_dict, user_token=None):
+    """Insert an OOPS into the system.
+    
+    :return: The day which the oops was filed under.
+    """
+    assert isinstance(oops_dict, dict)
+    return _insert(config, oopsid, oops_dict, user_token)
+
+def _insert(config, oopsid, insert_dict, user_token=None):
+    pool = pycassa.pool.ConnectionPool(config['keyspace'], config['host'])
+    dayoops_cf = pycassa.ColumnFamily(pool, 'DayOOPS')
+    oops_cf = pycassa.ColumnFamily(pool, 'OOPS')
+    day_key = time.strftime('%Y%m%d', time.gmtime())
+    now_uuid = uuid.uuid1()
+
+    oops_cf.insert(oopsid, insert_dict)
+    dayoops_cf.insert(day_key, {now_uuid:oopsid})
+    if user_token:
+        useroops_cf = pycassa.ColumnFamily(pool, 'UserOOPS')
+        useroops_cf.insert(user_token, {oopsid : ''})
+
+    return day_key
+
+def bucket(config, oopsid, bucketid):
+    """Adds an OOPS to a bucket, a collection of OOPSes that form a single
+    issue. If the bucket does not exist, it will be created.
+    
+    :return: The day which the bucket was filed under.
+    """
+    pool = pycassa.pool.ConnectionPool(config['keyspace'], config['host'])
+    bucket_cf = pycassa.ColumnFamily(pool, 'Buckets')
+    daybucket_cf = pycassa.ColumnFamily(pool, 'DayBuckets')
+    daybucketcount_cf = pycassa.ColumnFamily(pool, 'DayBucketsCount')
+    day_key = time.strftime('%Y%m%d', time.gmtime())
+
+    bucket_cf.insert(bucketid, {oopsid : ''})
+    daybucket_cf.insert((day_key, bucketid), {oopsid : ''})
+    # We have no way of knowing whether an increment has been performed if the
+    # write fails unexpectedly (CASSANDRA-2495). We will apply eventual
+    # consistency to this problem and tolerate slightly inaccurate counts for
+    # the span of a single day, cleaning up once this period has passed. This
+    # will be done by counting the number of columns in DayBuckets for the day
+    # and bucket ID.
+    daybucketcount_cf.add(day_key, bucketid)
+    return day_key
 
=== removed file 'oopsrepository/oopses.py'
--- oopsrepository/oopses.py	2012-02-21 10:56:50 +0000
+++ oopsrepository/oopses.py	1970-01-01 00:00:00 +0000
@@ -1,94 +0,0 @@
-# oops-repository is Copyright 2011 Canonical Ltd.
-#
-# Canonical Ltd ("Canonical") distributes the oops-repository source code under
-# the GNU Affero General Public License, version 3 ("AGPLv3"). See the file
-# LICENSE in the source tree for more information.
-
-"""basic operations on oopses in the db."""
-
-import json
-import bson
-import time
-import uuid
-
-import pycassa
-from pycassa.index import create_index_expression, LT, create_index_clause
-from pycassa.cassandra.ttypes import IndexExpression
-
-DAY = 60*60*24
-MONTH = DAY*30
-
-def prune(config):
-    """Remove OOPSES that are over 30 days old."""
-    pool = pycassa.connect(config['keyspace'], config['host'])
-    dayoops_cf = pycassa.ColumnFamily(pool, 'DayOOPS')
-    oops_cf = pycassa.ColumnFamily(pool, 'OOPS')
-    # Find days to prune
-    days = set()
-    prune_to = time.strftime('%Y%m%d', time.gmtime(time.time() - MONTH))
-    for key, _ in dayoops_cf.get_range(columns=()):
-        if key < prune_to:
-            days.add(key)
-    if not days:
-        return
-    # collect all the oopses (buffers all in memory; may want to make
-    # incremental in future)
-    batch_size = 10000
-    oopses = []
-    for day in days:
-        columns_found = batch_size
-        start_col = ''
-        while columns_found==batch_size:
-            columns = dayoops_cf.get(day, column_start=start_col)
-            columns_found = len(columns)
-            for column, oopsid in columns.items():
-                start_col = column
-                oopses.append(oopsid)
-    # Remove the oopses
-    batch = oops_cf.batch()
-    map(batch.remove, oopses)
-    batch.send()
-    # Clean out the days aggregates
-    # Clean out the days index
-    batch = dayoops_cf.batch()
-    map(batch.remove, days)
-    batch.send()
-
-def insert(config, oopsid, oops_json, user_token=None):
-    """Insert an OOPS into the system.
-    
-    :return: The day which the oops was filed under.
-    """
-    # make sure the oops report is a json dict, and break out each key to a
-    # separate column. For now, rather than worrying about typed column values
-    # we just coerce them all to strings.
-    oops_dict = json.loads(oops_json)
-    assert isinstance(oops_dict, dict)
-    insert_dict = {}
-    for key, value in oops_dict.items():
-        insert_dict[key] = json.dumps(value)
-    return _insert(config, oopsid, insert_dict, user_token)
-
-def insert_bson(config, oopsid, oops_bson, user_token=None):
-    """Insert an OOPS into the system.
-    
-    :return: The day which the oops was filed under.
-    """
-    oops_dict = bson.BSON(oops_bson)
-    insert_dict = oops_dict.to_dict()
-    return _insert(config, oopsid, insert_dict, user_token)
-
-def _insert(config, oopsid, insert_dict, user_token=None):
-    pool = pycassa.connect(config['keyspace'], config['host'])
-    dayoops_cf = pycassa.ColumnFamily(pool, 'DayOOPS')
-    oops_cf = pycassa.ColumnFamily(pool, 'OOPS')
-    day_key = time.strftime('%Y%m%d', time.gmtime())
-    now_uuid = uuid.uuid1()
-
-    oops_cf.insert(oopsid, insert_dict)
-    dayoops_cf.insert(day_key, {now_uuid:oopsid})
-    if user_token:
-        useroops_cf = pycassa.ColumnFamily(pool, 'UserOOPS')
-        useroops_cf.insert(user_token, {oopsid : ''})
-
-    return day_key
 
=== modified file 'oopsrepository/schema.py'
--- oopsrepository/schema.py	2011-04-03 11:14:12 +0000
+++ oopsrepository/schema.py	2012-03-23 23:37:20 +0000
@@ -6,6 +6,11 @@
 
 """The schema for oopsrepository."""
 
+from pycassa.types import (
+    CompositeType,
+    UTF8Type,
+    CounterColumnType,
+    )
 from pycassa.system_manager import (
     LONG_TYPE,
     SystemManager,
@@ -28,9 +33,20 @@
     mgr = SystemManager()
     try:
         workaround_1779(mgr.create_column_family, keyspace, 'OOPS',
-            comparator_type=UTF8_TYPE)
+            comparator_type=UTF8_TYPE, default_validation_class=UTF8_TYPE)
         workaround_1779(mgr.create_column_family, keyspace, 'DayOOPS',
             comparator_type=TIME_UUID_TYPE)
+        workaround_1779(mgr.create_column_family, keyspace, 'UserOOPS',
+            comparator_type=UTF8_TYPE)
+        workaround_1779(mgr.create_column_family, keyspace, 'Buckets',
+            comparator_type=UTF8_TYPE)
+        # TODO It might be more performant to use just the date for the key and
+        # a composite key of the bucket_id and the oops_id as the column name.
+        composite = CompositeType(UTF8Type(), UTF8Type())
+        workaround_1779(mgr.create_column_family, keyspace, 'DayBuckets',
+            comparator_type=UTF8_TYPE, key_validation_class=composite)
+        workaround_1779(mgr.create_column_family, keyspace, 'DayBucketsCount',
+            comparator_type=UTF8_TYPE, default_validation_class=CounterColumnType())
     finally:
         mgr.close()
 
 
=== modified file 'oopsrepository/testing/cassandra.py'
--- oopsrepository/testing/cassandra.py	2011-02-27 04:46:26 +0000
+++ oopsrepository/testing/cassandra.py	2012-03-23 23:37:20 +0000
@@ -28,7 +28,7 @@
         self.keyspace = os.path.basename(tempdir.path)
         self.mgr = SystemManager()
         workaround_1779(self.mgr.create_keyspace, self.keyspace,
-            replication_factor=1)
+            pycassa.SIMPLE_STRATEGY, {'replication_factor' : '1'})
         self.addCleanup(workaround_1779, self.mgr.drop_keyspace,
             self.keyspace)
 
 
=== modified file 'oopsrepository/testing/matchers.py'
--- oopsrepository/testing/matchers.py	2011-04-03 11:14:12 +0000
+++ oopsrepository/testing/matchers.py	2012-03-23 23:37:20 +0000
@@ -11,7 +11,7 @@
 import uuid
 
 import pycassa
-from pycassa.cassandra.Cassandra import NotFoundException
+from pycassa.cassandra.ttypes import NotFoundException
 from testtools.matchers import Matcher, Mismatch
 
 
@@ -22,12 +22,21 @@
     """
 
     def match(self, keyspace):
-        pool = pycassa.connect(keyspace)
+        pool = pycassa.ConnectionPool(keyspace, ['localhost:9160'])
         try:
             cf = pycassa.ColumnFamily(pool, 'OOPS')
             cf.insert('key',
                 {"date":json.dumps(time.time()), "URL":'a bit boring'})
             cf = pycassa.ColumnFamily(pool, 'DayOOPS')
             cf.insert('20100212', {uuid.uuid1(): 'key'})
+            cf = pycassa.ColumnFamily(pool, 'UserOOPS')
+            cf.insert('user-token', {'key':''})
+
+            cf = pycassa.ColumnFamily(pool, 'Buckets')
+            cf.insert('/bin/bash:11:x86_64:[vdso]+70c:...', {'key':''})
+            cf = pycassa.ColumnFamily(pool, 'DayBuckets')
+            cf.insert(('20100212', '/bin/bash:11:x86_64:[vdso]+70c:...'), {'key':''})
+            cf = pycassa.ColumnFamily(pool, 'DayBucketsCount')
+            cf.add('20100212', '/bin/bash:11:x86_64:[vdso]+70c:...', 13)
         except NotFoundException as e:
             return Mismatch(e.why)
 
=== modified file 'oopsrepository/tests/test_matchers.py'
--- oopsrepository/tests/test_matchers.py	2011-02-27 04:46:26 +0000
+++ oopsrepository/tests/test_matchers.py	2012-03-23 23:37:20 +0000
@@ -16,6 +16,6 @@
     def test_creates_columnfamily(self):
         keyspace = self.useFixture(TemporaryKeyspace()).keyspace
         self.assertNotEqual(None, HasOOPSSchema().match(keyspace))
-        config = dict(keyspace=keyspace)
+        config = dict(keyspace=keyspace, host=['localhost:9160'])
         schema.create(config)
         self.assertThat(keyspace, HasOOPSSchema())
 
=== modified file 'oopsrepository/tests/test_oopses.py'
--- oopsrepository/tests/test_oopses.py	2011-04-03 11:14:12 +0000
+++ oopsrepository/tests/test_oopses.py	2012-03-23 23:37:20 +0000
@@ -1,3 +1,4 @@
+# -*- coding: utf-8; -*-
 # oops-repository is Copyright 2011 Canonical Ltd.
 #
 # Canonical Ltd ("Canonical") distributes the oops-repository source code under
@@ -19,10 +20,10 @@
 
     def test_fresh_oops_kept(self):
         keyspace = self.useFixture(TemporaryOOPSDB()).keyspace
-        config = dict(keyspace=keyspace)
+        config = dict(keyspace=keyspace, host=['localhost:9160'])
         day_key = oopses.insert(config, 'key',
             json.dumps({"date":time.time(), "URL":'a bit boring'}))
-        pool = pycassa.connect(keyspace)
+        pool = pycassa.ConnectionPool(keyspace, ['localhost:9160'])
         dayoopses_cf = pycassa.ColumnFamily(pool, 'DayOOPS')
         oopses_cf = pycassa.ColumnFamily(pool, 'OOPS')
         oopses.prune(config)
@@ -32,8 +33,8 @@
 
     def test_old_oops_deleted(self):
         keyspace = self.useFixture(TemporaryOOPSDB()).keyspace
-        config = dict(keyspace=keyspace)
-        pool = pycassa.connect(keyspace)
+        config = dict(keyspace=keyspace, host=['localhost:9160'])
+        pool = pycassa.ConnectionPool(keyspace, ['localhost:9160'])
         dayoopses_cf = pycassa.ColumnFamily(pool, 'DayOOPS')
         datestamp = time.time() - oopses.MONTH - oopses.DAY
         day_key = time.strftime('%Y%m%d', time.gmtime(datestamp))
@@ -50,20 +51,68 @@
 
 class TestInsert(TestCase):
 
-    def test_insert_oops(self):
-        keyspace = self.useFixture(TemporaryOOPSDB()).keyspace
-        oopsid = 'booyah'
-        oops = json.dumps({'duration': 13000})
-        config = dict(keyspace=keyspace)
-        day_key = oopses.insert(config, oopsid, oops)
-        pool = pycassa.connect(keyspace)
+    def _test_insert_check(self, keyspace, oopsid, day_key, value=None):
+        pool = pycassa.ConnectionPool(keyspace, host=['localhost:9160'])
         oopses_cf = pycassa.ColumnFamily(pool, 'OOPS')
+        if value is None:
+            value = '13000'
         # The oops is retrievable
         columns = oopses_cf.get(oopsid)
-        self.assertEqual('13000', columns['duration'])
+        self.assertEqual(value, columns['duration'])
         # The oops has been indexed by day
-        pool = pycassa.connect(keyspace)
+        pool = pycassa.ConnectionPool(keyspace, host=['localhost:9160'])
         dayoops_cf = pycassa.ColumnFamily(pool, 'DayOOPS')
         oops_refs = dayoops_cf.get(day_key)
         self.assertEqual([oopsid], oops_refs.values())
         ## TODO - the aggregates for the OOPS have been updated.
+
+    def test_insert_oops(self):
+        keyspace = self.useFixture(TemporaryOOPSDB()).keyspace
+        oopsid = 'booyah'
+        oops = json.dumps({'duration': 13000})
+        config = dict(keyspace=keyspace, host=['localhost:9160'])
+        day_key = oopses.insert(config, oopsid, oops)
+        self._test_insert_check(keyspace, oopsid, day_key)
+
+    def test_insert_oops_dict(self):
+        keyspace = self.useFixture(TemporaryOOPSDB()).keyspace
+        oopsid = 'booyah'
+        oops = {'duration': '13000'}
+        config = dict(keyspace=keyspace, host=['localhost:9160'])
+        day_key = oopses.insert_dict(config, oopsid, oops)
+        self._test_insert_check(keyspace, oopsid, day_key)
+
+    def test_insert_unicode(self):
+        keyspace = self.useFixture(TemporaryOOPSDB()).keyspace
+        oopsid = 'booyah'
+        oops = {'duration': u'♥'}
+        config = dict(keyspace=keyspace, host=['localhost:9160'])
+        day_key = oopses.insert_dict(config, oopsid, oops)
+        self._test_insert_check(keyspace, oopsid, day_key, value=u'♥')
+
+class TestBucket(TestCase):
+
+    def test_insert_bucket(self):
+        keyspace = self.useFixture(TemporaryOOPSDB()).keyspace
+        config = dict(keyspace=keyspace, host=['localhost:9160'])
+        oopsid = 'booyah'
+        oops = json.dumps({'duration': 13000})
+        oopses.insert(config, oopsid, oops)
+        day_key = oopses.bucket(config, oopsid, 'bucket-key')
+
+        pool = pycassa.ConnectionPool(keyspace, ['localhost:9160'])
+        bucket_cf = pycassa.ColumnFamily(pool, 'Buckets')
+        daybucketcount_cf = pycassa.ColumnFamily(pool, 'DayBucketsCount')
+
+        oops_refs = bucket_cf.get('bucket-key')
+        self.assertEqual([oopsid], oops_refs.keys())
+        self.assertEqual(
+            daybucketcount_cf.get(day_key, ['bucket-key']).values(), [1])
+
+        oopsid = 'foobar'
+        oops = json.dumps({'wibbles': 12})
+        oopses.insert(config, oopsid, oops)
+        day_key = oopses.bucket(config, oopsid, 'bucket-key')
+        self.assertEqual(
+            daybucketcount_cf.get(day_key, ['bucket-key']).values(), [2])
+
 
=== modified file 'oopsrepository/tests/test_schema.py'
--- oopsrepository/tests/test_schema.py	2011-02-27 04:46:26 +0000
+++ oopsrepository/tests/test_schema.py	2012-03-23 23:37:20 +0000
@@ -16,6 +16,6 @@
 
     def test_creates_columnfamily(self):
         keyspace = self.useFixture(TemporaryKeyspace()).keyspace
-        config = dict(keyspace=keyspace)
+        config = dict(keyspace=keyspace, host=['localhost:9160'])
         schema.create(config)
         self.assertThat(keyspace, HasOOPSSchema())