Merge into trunk : refactor-submission : Code : Daisy

Status:	Merged
Approved by:	Brian Murray on 2013-04-15
Approved revision:	309
Merged at revision:	312
Proposed branch:	lp://staging/~ev/daisy/refactor-submission
Merge into:	lp://staging/daisy
Diff against target:	379 lines (+186/-86) 5 files modified daisy/retracer.py (+1/-2) daisy/schema.py (+6/-1) daisy/submit.py (+99/-73) daisy/utils.py (+13/-10) test/test_submit.py (+67/-0)
To merge this branch:	bzr merge lp://staging/~ev/daisy/refactor-submission
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Daisy Pluckers		2013-04-12	Pending
Review via email: mp+158566@code.staging.launchpad.net

Description of the change

This branch refactors the crash submission process. This in turn fixes the submission of kernel OOPS reports since we now first rely on apport to generate a crash signature from the data we have, then fall through to trying to find a matching signature in the database for binary crashes. The old behaviour was to attempt to isolate each type of problem we supported, and it was prone to breakage (as was the case with kernel OOPS).

The branch also adds a new Column Family, CouldNotBucket. This CF will contain the OOPS IDs (as actual uuid()s) for the day period that were not bucketed because no crash signature could be produced and they did not appear to be a binary crash.

I've also moved the creation and modification of the crash signature into the utils module, ensuring that the signature always gets truncated to the same length and always encoded as a utf-8 string.

Revision history for this message

Brian Murray (brian-murray) wrote on 2013-04-15:

#

Download full text (12.3 KiB)

This looks good aside of a couple of minor nits which follow in line.

On Fri, Apr 12, 2013 at 11:28:29AM -0000, Evan Dandrea wrote:
> Evan Dandrea has proposed merging lp:~ev/daisy/refactor-submission into lp:daisy.
>
> Requested reviews:
> Daisy Pluckers (daisy-pluckers)
>
> For more details, see:
> https://code.launchpad.net/~ev/daisy/refactor-submission/+merge/158566
>
> This branch refactors the crash submission process. This in turn fixes the submission of kernel OOPS reports since we now first rely on apport to generate a crash signature from the data we have, then fall through to trying to find a matching signature in the database for binary crashes. The old behaviour was to attempt to isolate each type of problem we supported, and it was prone to breakage (as was the case with kernel OOPS).
>
> The branch also adds a new Column Family, CouldNotBucket. This CF will contain the OOPS IDs (as actual uuid()s) for the day period that were not bucketed because no crash signature could be produced and they did not appear to be a binary crash.
>
> I've also moved the creation and modification of the crash signature into the utils module, ensuring that the signature always gets truncated to the same length and always encoded as a utf-8 string.
> --
> https://code.launchpad.net/~ev/daisy/refactor-submission/+merge/158566
> You are subscribed to branch lp:daisy.

> === modified file 'daisy/retracer.py'
> --- daisy/retracer.py 2013-04-10 13:32:23 +0000
> +++ daisy/retracer.py 2013-04-12 11:27:29 +0000
> @@ -603,10 +603,9 @@
> report.load(fp)
> stacktrace_addr_sig = report['StacktraceAddressSignature']
>
> - crash_signature = report.crash_signature()
> + crash_signature = utils.generate_crash_signature(report)
> if crash_signature:
> has_signature = True
> - crash_signature = crash_signature.encode('utf-8')
>
> if has_signature:
> try:
>
> === modified file 'daisy/schema.py'
> --- daisy/schema.py 2013-03-28 00:44:05 +0000
> +++ daisy/schema.py 2013-04-12 11:27:29 +0000
> @@ -21,6 +21,8 @@
> UTF8_TYPE,
> LONG_TYPE,
> ASCII_TYPE,
> + INT_TYPE,
> + TIME_UUID_TYPE,
> )
>
> from daisy import config
> @@ -69,8 +71,11 @@
> comparator_type=ASCII_TYPE)
> if 'BugToCrashSignatures' not in cfs:
> workaround_1779(mgr.create_column_family, keyspace, 'BugToCrashSignatures',
> - key_validation_class=INTEGER_TYPE,
> + key_validation_class=INT_TYPE,
> comparator_type=UTF8_TYPE)
> + if 'CouldNotBucket' not in cfs:
> + workaround_1779(mgr.create_column_family, keyspace, 'CouldNotBucket',
> + comparator_type=TIME_UUID_TYPE)
> finally:
> mgr.close()
>
>
> === modified file 'daisy/submit.py'
> --- daisy/submit.py 2013-04-06 22:25:02 +0000
> +++ daisy/submit.py 2013-04-12 11:27:29 +0000
> @@ -40,14 +40,27 @@
> def update_release_pkg_counter(counters_fam, release, src_package, date):
> counters_fam.insert('%s:%s' % (release, src_package), ...

This looks good aside of a couple of minor nits which follow in line.

On Fri, Apr 12, 2013 at 11:28:29AM -0000, Evan Dandrea wrote:
> Evan Dandrea has proposed merging lp:~ev/daisy/refactor-submission into lp:daisy.
> 
> Requested reviews:
>   Daisy Pluckers (daisy-pluckers)
> 
> For more details, see:
> https://code.launchpad.net/~ev/daisy/refactor-submission/+merge/158566
> 
> This branch refactors the crash submission process. This in turn fixes the submission of kernel OOPS reports since we now first rely on apport to generate a crash signature from the data we have, then fall through to trying to find a matching signature in the database for binary crashes. The old behaviour was to attempt to isolate each type of problem we supported, and it was prone to breakage (as was the case with kernel OOPS).
> 
> The branch also adds a new Column Family, CouldNotBucket. This CF will contain the OOPS IDs (as actual uuid()s) for the day period that were not bucketed because no crash signature could be produced and they did not appear to be a binary crash.
> 
> I've also moved the creation and modification of the crash signature into the utils module, ensuring that the signature always gets truncated to the same length and always encoded as a utf-8 string.
> -- 
> https://code.launchpad.net/~ev/daisy/refactor-submission/+merge/158566
> You are subscribed to branch lp:daisy.

> === modified file 'daisy/retracer.py'
> --- daisy/retracer.py	2013-04-10 13:32:23 +0000
> +++ daisy/retracer.py	2013-04-12 11:27:29 +0000
> @@ -603,10 +603,9 @@
>                      report.load(fp)
>                  stacktrace_addr_sig = report['StacktraceAddressSignature']
>  
> -                crash_signature = report.crash_signature()
> +                crash_signature = utils.generate_crash_signature(report)
>                  if crash_signature:
>                      has_signature = True
> -                    crash_signature = crash_signature.encode('utf-8')
>  
>              if has_signature:
>                  try:
> 
> === modified file 'daisy/schema.py'
> --- daisy/schema.py	2013-03-28 00:44:05 +0000
> +++ daisy/schema.py	2013-04-12 11:27:29 +0000
> @@ -21,6 +21,8 @@
>      UTF8_TYPE,
>      LONG_TYPE,
>      ASCII_TYPE,
> +    INT_TYPE,
> +    TIME_UUID_TYPE,
>      )
>  
>  from daisy import config
> @@ -69,8 +71,11 @@
>                  comparator_type=ASCII_TYPE)
>          if 'BugToCrashSignatures' not in cfs:
>              workaround_1779(mgr.create_column_family, keyspace, 'BugToCrashSignatures',
> -                key_validation_class=INTEGER_TYPE,
> +                key_validation_class=INT_TYPE,
>                  comparator_type=UTF8_TYPE)
> +        if 'CouldNotBucket' not in cfs:
> +            workaround_1779(mgr.create_column_family, keyspace, 'CouldNotBucket',
> +                comparator_type=TIME_UUID_TYPE)
>      finally:
>          mgr.close()
>  
> 
> === modified file 'daisy/submit.py'
> --- daisy/submit.py	2013-04-06 22:25:02 +0000
> +++ daisy/submit.py	2013-04-12 11:27:29 +0000
> @@ -40,14 +40,27 @@
>  def update_release_pkg_counter(counters_fam, release, src_package, date):
>      counters_fam.insert('%s:%s' % (release, src_package), {date: 1})
>  
> +def create_report_from_bson(data):
> +    report = apport.Report()
> +    for key in data:
> +        report[key.encode('UTF-8')] = data[key].encode('UTF-8')
> +    return report
> +
> +def try_to_repair_sas(data):
> +    '''Try to repair the StacktraceAddressSignature, if this is a binary
> +    crash.'''

I believe the above should be indented by three spaces.

> +
> +    if 'StacktraceTop' in data and 'Signal' in data:
> +        if not 'StacktraceAddressSignature' in data:
> +            report = create_report_from_bson(data)
> +            sas = report.crash_signature_addresses()
> +            if sas:
> +                data['StacktraceAddressSignature'] = sas
>  
>  def submit(_pool, environ, system_token):
> -    indexes_fam = pycassa.ColumnFamily(_pool, 'Indexes')
> -    awaiting_retrace_fam = pycassa.ColumnFamily(_pool, 'AwaitingRetrace')
>      counters_fam = pycassa.ColumnFamily(_pool, 'Counters',
>                                          retry_counter_mutations=True)
>  
> -    oops_id = str(uuid.uuid1())
>      try:
>          data = environ['wsgi.input'].read()
>      except IOError as e:
> @@ -61,16 +74,22 @@
>      except bson.errors.InvalidBSON:
>          return (False, 'Invalid BSON.')
>  
> +    # Keep a reference to the decoded report data. If we crash, we'll
> +    # potentially attach it to the OOPS report.
> +    environ['wsgi.input.decoded'] = data
> +
> +    oops_id = str(uuid.uuid1())
>      day_key = time.strftime('%Y%m%d', time.gmtime())
> +
>      if 'KernelCrash' in data or 'VmCore' in data:
>          # We do not process these yet, but we keep track of how many reports
>          # we're receiving to determine when it's worth solving.
>          counters_fam.add('KernelCrash', day_key)
>          return (False, 'Kernel crashes are not handled yet.')
>  
> -    # Keep a reference to the decoded report data. If we crash, we'll
> -    # potentially attach it to the OOPS report.
> -    environ['wsgi.input.decoded'] = data
> +    # Write the SHA512 hash of the system UUID in with the report.
> +    if system_token:
> +        data['SystemIdentifier'] = system_token
>  
>      release = data.get('DistroRelease', '')
>      package = data.get('Package', '')
> @@ -79,79 +98,86 @@
>      third_party = False
>      if '[origin:' in package:
>          third_party = True
> +
>      package, version = utils.split_package_and_version(package)
>      src_package, src_version = utils.split_package_and_version(src_package)
>      fields = utils.get_fields_for_bucket_counters(problem_type, release, package, version)
> -    if system_token:
> -        data['SystemIdentifier'] = system_token
> +
> +    if not third_party and problem_type == 'Crash':
> +        update_release_pkg_counter(counters_fam, release, src_package, day_key)
> +
> +    try_to_repair_sas(data)
>      oopses.insert_dict(oops_config, oops_id, data, system_token, fields)
>  
> -    if 'DuplicateSignature' in data:
> -        utils.bucket(oops_config, oops_id, data['DuplicateSignature'], data)
> -        if not third_party and problem_type == 'Crash':
> -            update_release_pkg_counter(counters_fam, release, src_package, day_key)
> +    success, output = bucket(_pool, oops_config, oops_id, data)
> +    return (success, output)
> +
> +def bucket(_pool, oops_config, oops_id, data):
> +    '''Bucket oops_id.
> +       If the report was malformed, return (False, failure_msg)
> +       If a core file is to be requested, return (True, 'CORE')
> +       If no further action is needed, return (True, '')
> +    '''
> +
> +    indexes_fam = pycassa.ColumnFamily(_pool, 'Indexes')
> +    day_key = time.strftime('%Y%m%d', time.gmtime())

day_key is already defined in submit so perhaps you could pass it to
bucket().

> +
> +    # General bucketing
> +    report = create_report_from_bson(data)
> +    crash_signature = utils.generate_crash_signature(report)
> +    if crash_signature:
> +        utils.bucket(oops_config, oops_id, crash_signature, data)
>          return (True, '')
> -    elif 'InterpreterPath' in data and not 'StacktraceAddressSignature' in data:
> -        # Python crashes can be immediately bucketed.
> -        report = apport.Report()
> -        # TODO just pull in all keys
> -        for key in ('ExecutablePath', 'Traceback', 'ProblemType'):
> -            try:
> -                report[key.encode('UTF-8')] = data[key].encode('UTF-8')
> -            except KeyError:
> -                return (False, 'Missing keys in interpreted report.')
> -        # FIXME always try to generate a crash signature, *then* check for a
> -        # crash needing retracing (see tools/rebuild_bucketversions.py)
> -        crash_signature = report.crash_signature()
> -        if crash_signature:
> -            utils.bucket(oops_config, oops_id, crash_signature, data)
> -            if not third_party and problem_type == 'Crash':
> -                update_release_pkg_counter(counters_fam, release, src_package, day_key)
> -            return (True, '')
> -        else:
> -            return (False, 'Could not generate crash signature for interpreted report.')
> -
> -    addr_sig = data.get('StacktraceAddressSignature', None)
> -    if not addr_sig:
> -        counters_fam.add('MissingSAS', day_key)
> -        # We received BSON data with unexpected keys.
> -        return (False, 'No StacktraceAddressSignature found in report.')
>  
>      # Binary
> -    output = ''
> -    crash_sig = None
> -    try:
> -        crash_sig = indexes_fam.get(
> -            'crash_signature_for_stacktrace_address_signature', [addr_sig])
> -        crash_sig = crash_sig.values()[0]
> -    except (NotFoundException, KeyError):
> -        pass
> -    if crash_sig:
> -        # We have already retraced for this address signature, so this crash
> -        # can be immediately bucketed.
> -        utils.bucket(oops_config, oops_id, crash_sig, data)
> -    else:
> -        # Are we already waiting for this stacktrace address signature to be
> -        # retraced?
> -        waiting = True
> +    if 'StacktraceTop' in data and 'Signal' in data:
> +        output = ''
> +        addr_sig = data.get('StacktraceAddressSignature', None)
> +        if not addr_sig:
> +            counters_fam = pycassa.ColumnFamily(_pool, 'Counters',
> +                                                retry_counter_mutations=True)
> +            counters_fam.add('MissingSAS', day_key)
> +            # We received BSON data with unexpected keys.
> +            return (False, 'No StacktraceAddressSignature found in report.')
> +
> +        crash_sig = None
>          try:
> -            indexes_fam.get('retracing', [addr_sig])
> -        except NotFoundException:
> -            waiting = False
> -
> -        if not waiting and utils.retraceable_release(release):
> -            # We do not have a core file in the queue, so ask for one. Do
> -            # not assume we're going to get one, so also add this ID the
> -            # the AwaitingRetrace CF queue as well.
> -
> -            # We don't ask derivatives for core dumps. We could technically
> -            # check to make sure the Packages and Dependencies fields do not
> -            # have '[origin:' lines; however, apport-retrace looks for
> -            # configuration data in a directory named by the DistroRelease, so
> -            # these would always fail regardless.
> -            output = '%s CORE' % oops_id
> -
> -        awaiting_retrace_fam.insert(addr_sig, {oops_id : ''})
> -    if not third_party and problem_type == 'Crash':
> -        update_release_pkg_counter(counters_fam, release, src_package, day_key)
> -    return (True, output)
> +            crash_sig = indexes_fam.get(
> +                'crash_signature_for_stacktrace_address_signature', [addr_sig])
> +            crash_sig = crash_sig.values()[0]
> +        except (NotFoundException, KeyError):
> +            pass
> +        if crash_sig:
> +            # We have already retraced for this address signature, so this crash
> +            # can be immediately bucketed.
> +            utils.bucket(oops_config, oops_id, crash_sig, data)
> +        else:
> +            # Are we already waiting for this stacktrace address signature to be
> +            # retraced?
> +            waiting = True
> +            try:
> +                indexes_fam.get('retracing', [addr_sig])
> +            except NotFoundException:
> +                waiting = False
> +
> +            release = data.get('DistroRelease', '')
> +            if not waiting and utils.retraceable_release(release):
> +                # We do not have a core file in the queue, so ask for one. Do
> +                # not assume we're going to get one, so also add this ID the
> +                # the AwaitingRetrace CF queue as well.
> +
> +                # We don't ask derivatives for core dumps. We could technically
> +                # check to make sure the Packages and Dependencies fields do not
> +                # have '[origin:' lines; however, apport-retrace looks for
> +                # configuration data in a directory named by the DistroRelease, so
> +                # these would always fail regardless.
> +                output = '%s CORE' % oops_id

This does not match the documentation for output:

+ If a core file is to be requested, return (True, 'CORE')

--
Brian Murray

Revision history for this message

Evan (ev) wrote on 2013-04-16:

#

Fixes were made as r313.

Daisy

Merge lp://staging/~ev/daisy/refactor-submission into lp://staging/daisy

Commit message

Description of the change

Preview Diff

Subscribers