don't report crashes for programs that don't match the file on disk (like for kernel crashes)

[Steve Langasek]

[Impact]
Reporting of crashes against currently-installed versions of packages, which this is not the version of package on disk, results in garbage data in the errors.ubuntu.com graphs that make it impossible to know with certainty that a given version of a package fixed a given crash. This severely hinders our reliance on errors.ubuntu.com for SRU validation.

[TEST CASE]

1) install lptools
2) lp-grab-attachments 2039920 (should crash with a keyerror)
3) close the apport dialog regarding the application crashing (via the X in the corner)
4) sudo vi $(which lp-grab-attachments)
5) modify copyright year or some comment
6) ubuntu-bug /var/crash/_usr_bin_lp-grab-attachments.*crash
7) Check the send error repot check box and click continue
8) ls -l /var/crash (observe _usr_bin_lp-grab-attachments .upload and .uploaded files)
9) remove _usr_bin_lp-grab-attachments files in /var/crash
10) reinstall lptools
11) install package from -proposed, follow test case and there should *not* be a .upload or .uploaded file as seen in step 8

[Regression Potential]
Worst-case scenario is that some crashes that should be reported fail to be reported as a result of a bug in this change.

Every crash report that gets fixed in a package upload winds up with some fuzziness in the bucket showing a small number of crash reports in the very version that fixes it. E.g.:
   https://errors.ubuntu.com/bucket/?id=%2Fusr%2Fbin%2Fubuntuone-installer%3AGError%3Afinished%3Afunction
   (bug #853060)

This makes it impossible to tell with certainty by looking at errors.ubuntu.com if a crash is really fixed, or if it's just been made less frequent. We need to have good data here to be able to use errors.u.c for decision making around SRU publication.

The suspicion is that the rule preventing launchpad bugs from being filed when the running process doesn't match the file on disk is not being applied correctly to whoopsie. It definitely should be.

(It's possible that this is therefore a duplicate of bug #1020994. I'm not sure, I haven't looked at the code.)

I would also argue that, if this happens in the "system service" case, we should not show the user any crash dialog for it either, so long as we don't have support for leading them to a targeted update to fix the crash. We get no useful crash information from the user in this case, and the risk of confusion from the dialog is great when it's a system service crash. As a matter of policy, front-end applications need to be resilient in the face of failures of back-end daemons.

ProblemType: BugDistroRelease: Ubuntu 12.10
Package: apport 2.4-0ubuntu8
ProcVersionSignature: Ubuntu 3.5.0-10.10-generic 3.5.1
Uname: Linux 3.5.0-10-generic x86_64
ApportVersion: 2.4-0ubuntu8
Architecture: amd64
Date: Mon Aug 20 12:34:10 2012InstallationMedia: Ubuntu 10.04.1 LTS "Lucid Lynx" - Release amd64 (20100816.1)
PackageArchitecture: allSourcePackage: apport
UpgradeStatus: Upgraded to quantal on 2012-06-11 (70 days ago)
modified.conffile..etc.apport.crashdb.conf: [modified]
mtime.conffile..etc.apport.crashdb.conf: 2012-05-04T17:01:02

[Steve Langasek (vorlon)]

Since apport's handling of this is mainlined in /usr/share/apport/apport, it's probably not an issue for the kernel crash handler. It may be an issue specific to the python crash handler.

[Steve Langasek (vorlon)]

Confirmed that /usr/share/pyshared/apport_python_hook.py has no check for whether the process matches the package on disk.

[Steve Langasek (vorlon)]

Brian, could you please have a look at this?

[Brian Murray (brian-murray)]

This applies to every type of crash:

        # check if binary changed since the crash happened
        if 'ExecutablePath' in self.report and 'ExecutableTimestamp' in self.report:
            orig_time = int(self.report['ExecutableTimestamp'])
            del self.report['ExecutableTimestamp']
            cur_time = int(os.stat(self.report['ExecutablePath']).st_mtime)

            if orig_time != cur_time:
                self.report['UnreportableReason'] = (
                    _('The problem happened with the program %s which changed '
                      'since the crash occurred.') % self.report['ExecutablePath'])
                return

However, because collect_info just returns here there will be a complete .crash file with 'UnreportableReason' in it which apport will then create a .upload file for, because the check_unreportable check is later. Whoopsie seems to deliberately ignore the UnreportableReason field and not upload that to daisy at all. This makes sense as you might want to be able to find out what kind of things are unreportable, but there currently is no way to filter these out.

[Evan (ev)]

As discussed with Brian in email, whoopsie does indeed purposefully ignore UnreportableReason reports, as they're often things we really do want to report (out of date packages, etc). However, in this case we don't want to continue to confuse developers with inaccurate version information on the individual problem pages.

It might still be worth counting these in the future, just to see the depth of the problem, but lets address that later.

[Martin Pitt (pitti)]

In the original code from Evan this seems to have been done on purpose:

            apport.fileutils.mark_report_upload(report_file)
            # We check for duplicates and unreportable crashes here, rather
            # than before we show the dialog, as we want to submit these to the
            # crash database, but not Launchpad.
            if self.crashdb.accepts(self.report):

But as Evan, Brian, and Steve all agree that this should not be done any more, I'll change the behaviour (FWIW, I fully agree to this -- there is a plethora of reasons for unreportable crashes, like full disks, RAM errors, modified packages, etc. -- none of which we want on Launchpad, and presuambly none of them belong into the error tracker either).

[Martin Pitt (pitti)]

Ah, the one notable exception for UnreportableReason that should go to whoopsie seems to be the "You have some obsolete package versions installed." case. So I think we should treat that specially and allow that, but ignore all others.

[Brian Murray (brian-murray)]

I think only the specific situation where the binary has changed should not be sent to the crash database. Other unreportable reasons like full disks may lead us to discover that a lot of people have full seperate boot partitions and that we should be helping them by offering to remove kernels from old releases. We won't know without having the data. Additionally, not reporting these won't effect the user experience because they will still receive a dialog saying that the error was unreportable.

[Evan (ev)]

I agree with Brian on this. We do not at present know how much of a problem full disks really is.

Martin, what are your thoughts on encoding something akin to UnreportableReason in a machine parseable, non-translated form? We could single these out on the server side and start monitoring the prevalence.

[Martin Pitt (pitti)]

OK, thanks for your feedback. So let's just take out that one particular error then.

> what are your thoughts on encoding something akin to UnreportableReason in a machine parseable, non-translated form

I'm not too keen on it, to be honest. First, that's not really (only) under apport's control, package hooks can invent arbitrary conditions and reasons. Second, the _point_ of "unreportable reason" is to, well, _not_ get reported, as the error happened because of conditions that are not the fault of the distro. I still take this stance, but if you are really keen on getting all of those to errors.u.c., then my suggestion is that for _this_ particular error (executable changed on disk since crash happened), apport adds a new tag ("binary-updated-after-crash") or field to the report which whoopsie can check for and ignore?

[Brian Murray (brian-murray)]

Given that whoopsie doesn't currently send the UnreportableReason field and that the field contains translated data I don't think we should currently be sending bugs with an UnreportableReason to the crash database. I've proposed a branch for merging that does the unreportable reason check earlier than marking the report for upload.

[Martin Pitt (pitti)]

Brian, what changed your mind in comment 12 as compared to comment 9? FWIW, I'm fine with Brian's merge proposal as it honors UnreportableReason as what I intended it to be, but I'd like to get Evan's consent here as well before I merge it. Has this been discussed on IRC since the last comments?

Thanks!

[Evan (ev)]

I've followed up to this in the merge proposal:
https://code.launchpad.net/~brian-murray/apport/no-unreportable/+merge/126082

[Steve Langasek (vorlon)]

Hello Steve, or anyone else affected,

Accepted apport into quantal-proposed. The package will build now and be available at http://launchpad.net/ubuntu/+source/apport/2.6.1-0ubuntu4 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please change the bug tag from verification-needed to verification-done. If it does not, change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

[C de-Avillez (hggdh2)]

verified that the crash report is not sent over if there are changes in the installed package. Note that no warning that this report is unreportable is now given.

Tagging verification-done.

[Brian Murray (brian-murray)]

The unreportable warning does not happen as that appears when post-processing the crash report before sending it to Launchpad. Since 12.10 is a stable release now the crash reports no longer goto Launchpad. I've removed that bit from the test case.

[Brian Murray (brian-murray)]

This was fixed in quantal in this upload:

apport (2.6.1-0ubuntu6) quantal-proposed; urgency=low

  * Do not refer to the current release in the InstallationDate field
    (LP: #1070400). We have the installed release version in the
    InstallationMedia field.

 -- Evan Dandrea <email address hidden> Wed, 24 Oct 2012 14:31:10 +0100

apport (2.6.1-0ubuntu5) quantal-proposed; urgency=low

  * Include the date that Ubuntu was installed with each report
    (LP: #1070400).

 -- Evan Dandrea <email address hidden> Wed, 24 Oct 2012 11:29:18 +0100

apport (2.6.1-0ubuntu4) quantal-proposed; urgency=low

  * apport/ui.py: create a MarkForUpload field and set that to false for
    binaries that changed since the crash happened thereby preventing uploads
    to the crash database (LP: #1039220)

 -- Brian Murray <email address hidden> Tue, 16 Oct 2012 13:31:24 -0700

[Launchpad Janitor (janitor)]

This bug was fixed in the package apport - 2.6.1-0ubuntu6

---------------
apport (2.6.1-0ubuntu6) quantal-proposed; urgency=low

  * Do not refer to the current release in the InstallationDate field
    (LP: #1070400). We have the installed release version in the
    InstallationMedia field.

apport (2.6.1-0ubuntu5) quantal-proposed; urgency=low

  * Include the date that Ubuntu was installed with each report
    (LP: #1070400).
 -- Evan Dandrea <email address hidden> Wed, 24 Oct 2012 14:31:10 +0100

[Adam Conrad (adconrad)]

Hello Steve, or anyone else affected,

Accepted apport into precise-proposed. The package will build now and be available at http://launchpad.net/ubuntu/+source/apport/2.0.1-0ubuntu15 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please change the bug tag from verification-needed to verification-done. If it does not, change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

[Brian Murray (brian-murray)]

I tested apport version 2.0.1-0ubuntu15 from precise-proposed on a Xubuntu system and did not see a .upload or .uploaded file in /var/crash for a crash about a package which I'd modified. Marking as verification-done.

[Scott Kitterman (kitterman)]

The verification of this Stable Release Update has completed successfully and the package has now been released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regresssions.

[Launchpad Janitor (janitor)]

This bug was fixed in the package apport - 2.0.1-0ubuntu15

---------------
apport (2.0.1-0ubuntu15) precise-proposed; urgency=low

  * apport/ui.py: create a MarkForUpload field and set that to false for
    binaries that changed since the crash happened thereby preventing uploads
    to the crash database (LP: #1039220)
 -- Brian Murray <email address hidden> Fri, 26 Oct 2012 15:17:13 -0700