1303067 - Enable unsubmitted crash notification for more users

Status: RESOLVED FIXED

(Toolkit :: Crash Reporting, defect)

Description

[Mike Conley (:mconley) (:⚙️)]

So we landed bug 1287178, which adds a "Send Always" button to the unsubmitted crash report notification. Then we landed bug 1301722, which will suppress the notification for 30 days if it's not interacted with over several days.

Are we at a point where we're comfortable turning this notification on for more users? I know that blassey is interested in getting this up to _at least_ Beta. Do we want this to go out to Release too?

The notification is activated by the browser.crashReports.unsubmittedCheck.enabled pref, set here: http://searchfox.org/mozilla-central/rev/515bca4fca5e2e8233ed30e6b6d48fcab8497dbf/browser/app/profile/firefox.js#1515 (currently only enabled for Nightly and Aurora).

Comment 1

[Mike Conley (:mconley) (:⚙️)]

The questions are:

1) Are we at a point where we're happy to expose this notification bar to more users in order to get those crash reports?
2) Which channels should get the notification?

Comment 2

[Mike Conley (:mconley) (:⚙️)]

Hey blassey,

I understand we want this on at least Beta due to some sandboxing stuff that's going to ride the trains. Is there a bug for that work this should be attached to?

Comment 3

[Brad Lassey [:blassey] (use needinfo?)]

redirecting to jimm. I thought we wanted this in 51, not beta

Comment 4

[Mike Conley (:mconley) (:⚙️)]

Sorry, to clarify what I said in comment 2:

Currently, this notification is held back to just Nightly and Dev Edition users. I understand that we want this on when 51 reaches Beta due to some sandboxing stuff that's going to ride the trains. Is that true? Is the idea also for the notification to reach release channel users when 51 goes out on release?

Comment 5

[Brad Lassey [:blassey] (use needinfo?)]

Yes, I think if the UX has been sorted out we want this to ride the trains so we can start collecting data about hidden crashes on all branches.

Comment 6

[Jim Mathies [:jimm]]

Sandboxing isn't blocked on or in need of this reporting.

I think product has to sign off on this UI rolling out to beta/release. Personally I'm not a fan of having this out on release since it tends to show up a lot. Beta sounds like a good idea though.

Comment 7

[Stephen Horlander [:shorlander]]

(In reply to Mike Conley (:mconley) - (needinfo me!) from comment #1)
> The questions are:
> 
> 1) Are we at a point where we're happy to expose this notification bar to
> more users in order to get those crash reports?
> 2) Which channels should get the notification?

With the improvements allowing people to always send and the auto-dismiss I think this will be fine for beta. 

This is kind of a band-aid fix for us to get these crash reports. I don't see this as something we would show release users.

Comment 8

[Brad Lassey [:blassey] (use needinfo?)]

(In reply to Jim Mathies [:jimm] from comment #6)
> Sandboxing isn't blocked on or in need of this reporting.
> 
> I think product has to sign off on this UI rolling out to beta/release.
> Personally I'm not a fan of having this out on release since it tends to
> show up a lot. Beta sounds like a good idea though.

The point of Mike's work is to make it only show once assuming users will then click always or never submit.

(In reply to Stephen Horlander [:shorlander] from comment #7)
> (In reply to Mike Conley (:mconley) - (needinfo me!) from comment #1)
> > The questions are:
> > 
> > 1) Are we at a point where we're happy to expose this notification bar to
> > more users in order to get those crash reports?
> > 2) Which channels should get the notification?
> 
> With the improvements allowing people to always send and the auto-dismiss I
> think this will be fine for beta. 
> 
> This is kind of a band-aid fix for us to get these crash reports. I don't
> see this as something we would show release users.

band-aid implies that we think this will be fixed in some other, better way in the future, but I'm unaware of what that other work is. Can you link me to a bug?

Comment 9

[Stephen Horlander [:shorlander]]

(In reply to Brad Lassey [:blassey] (use needinfo?) from comment #8)
> band-aid implies that we think this will be fixed in some other, better way
> in the future, but I'm unaware of what that other work is. Can you link me
> to a bug?

I think we could (probably should) rethink how the crash experience works. It feels more like an experience of accumulated parts at this point rather than a planned experience. 

A result of that could be a better way to present these choices that we would feel comfortable shipping to everyone. It's also possible that we would still want to segment what UI to show users based on release vs. pre-release channels.

There is no existing initiative to do that work that I am aware of though.

As it stands though I don't think we should be showing this notification bar to general release users because: 1) it highlights Firefox crashiness as a problem when it's mostly an invisible problem to users; 2) we already have an abundance of notification bars that we inundate people with; 3) I am comfortable showing this to pre-release users because the idea of "testing" is (in theory) built in, not so much with release users

Comment 10

[Mike Conley (:mconley) (:⚙️)]

(In reply to Stephen Horlander [:shorlander] from comment #9)
> As it stands though I don't think we should be showing this notification bar
> to general release users because: 1) it highlights Firefox crashiness as a
> problem when it's mostly an invisible problem to users; 2) we already have
> an abundance of notification bars that we inundate people with; 3) I am
> comfortable showing this to pre-release users because the idea of "testing"
> is (in theory) built in, not so much with release users

For clarity, is our beta population included when you say "pre-release" users? In other words (sorry if I'm making you repeat yourself, I'm just trying to be absolutely sure) - can we enable this notification for users on Nightly, Dev Edition and Beta? Or must it stay only on Nightly and Dev Edition?

Comment 11

[Stephen Horlander [:shorlander]]

(In reply to Mike Conley (:mconley) - (high latency on reviews and needinfos) from comment #10)
> (In reply to Stephen Horlander [:shorlander] from comment #9)
> > As it stands though I don't think we should be showing this notification bar
> > to general release users because: 1) it highlights Firefox crashiness as a
> > problem when it's mostly an invisible problem to users; 2) we already have
> > an abundance of notification bars that we inundate people with; 3) I am
> > comfortable showing this to pre-release users because the idea of "testing"
> > is (in theory) built in, not so much with release users
> 
> For clarity, is our beta population included when you say "pre-release"
> users? In other words (sorry if I'm making you repeat yourself, I'm just
> trying to be absolutely sure) - can we enable this notification for users on
> Nightly, Dev Edition and Beta? Or must it stay only on Nightly and Dev
> Edition?

Yes, by "pre-release" I was referring to Beta, Nightly and Dev Edition.

Comment 12

[Mike Conley (:mconley) (:⚙️)]

Attachment: Bug 1303067 - Enable unsubmitted crash notification for more users.

Review commit: https://reviewboard.mozilla.org/r/88372/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/88372/

Comment 13

[Mike Conley (:mconley) (:⚙️)]

Alright. Looking at firefox.js, I don't see us turning a lot of things off just for release. We have EARLY_BETA_OR_EARLIER, which turns on a feature for early betas only, and shuts it off for later betas. I understand that's to make the later betas that we test to be as close to what users receive as possible.

So the patch above uses that same convention and will turn the unsubmitted crash report notification for early betas or earlier.

Comment 14

[:Felipe Gomes (needinfo for replies!)]

Comment on attachment 8804372 [details]
Bug 1303067 - Enable unsubmitted crash notification for more users.

https://reviewboard.mozilla.org/r/88372/#review87452

Comment 15

[Mike Conley (:mconley) (:⚙️)]

Thanks felipe.

ni?'ing myself to request uplift to Aurora.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/07ae41e73a6f
Enable unsubmitted crash notification for more users. r=Felipe

Comment 17

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/07ae41e73a6f

Comment 18

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8804372 [details]
Bug 1303067 - Enable unsubmitted crash notification for more users.

Approval Request Comment
[Feature/regressing bug #]:

Unsubmitted crash report notification.

[User impact if declined]:

Users that run early betas will not have the opportunity to submit backlogged crash reports to Mozilla.

[Describe test coverage new/current, TreeHerder]:

This feature has been enabled on Nightly and Aurora for a while, but we want to turn it on for early betas for the next uplift. UX is fine with that, now that we automatically hide the notification if the user doesn't interact with it after a number of sessions.

[Risks and why]: 

Low risk. Just a pref flip that we can flip back if needs be.

[String/UUID change made/needed]:

None.

Comment 19

[Gerry Chang [:gchang]]

Comment on attachment 8804372 [details]
Bug 1303067 - Enable unsubmitted crash notification for more users.

Enable unsubmitted crash notification for early beta. Take it in 51 aurora.

Comment 20

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

I'm hitting a conflict uplifting this to aurora, could we get a rebased patch?

Comment 21

[Mike Conley (:mconley) (:⚙️)]

Attachment: Rebased for aurora

MozReview-Commit-ID: F1B0QA8a8wX

Comment 22

[Mike Conley (:mconley) (:⚙️)]

(In reply to Wes Kocher (:KWierso) from comment #20)
> I'm hitting a conflict uplifting this to aurora, could we get a rebased
> patch?

Thanks for letting me know. Does this one work better?

Comment 23

[Mike Conley (:mconley) (:⚙️)]

Where "this one" is attachment 8806456 [details] [diff] [review]

Comment 24

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/b128f5deb5e4

Comment 25

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Appears so, thanks.

Comment 26

[Marco Castelluccio [:marco]]

This change (together with bug 1282776) will make some dashboards and automated notification systems a bit unhappy twice for each cycle (at the beginning of the cycle we'll always see spikes, at the middle of the cycle we'll always see volume reductions).

Comment 27

[[:philipp]]

how is EARLY_BETA_OR_EARLIER defined? (it looks like the mechanism is still active in 51.0b7)

Comment 28

[Mike Conley (:mconley) (:⚙️)]

(In reply to [:philipp] from comment #27)
> how is EARLY_BETA_OR_EARLIER defined? (it looks like the mechanism is still
> active in 51.0b7)

Normally EARLY_BETA_OR_EARLIER is shut off around b5, but 51 has an extended beta cycle for a variety of reasons. The flag was recently unset in a commit landed earlier today (https://hg.mozilla.org/releases/mozilla-beta/rev/d946c3ef0da36339cf3f3988547f4c6aebea37ae), so the next beta (b8) should not have the mechanism enabled.

Comment 29

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Hi Mike, Brad, enabling infobar crash submissions during early Beta cycle is making it difficult for relman to compare crash rates during early Beta cycle with past Beta cycles. Is this a necessary change? I was under the impression that we would leave this on during Nightly and Aurora cycle and not in Beta cycle. Is there such a significant benefit here? Relman team would recommend disabling this for the entire Beta cycle if possible.

Comment 30

[Brad Lassey [:blassey] (use needinfo?)]

As I said at the channel meeting, there are classes of crashes that will go unreported without the info bar. I don't know why we would purposefully not collect that data.

Comment 31

[Marco Castelluccio [:marco]]

Why is it only enabled for half the beta cycle? Enabling it for the entire beta cycle would solve the issue of crash rates changing suddenly at the middle and at the beginning of the cycle.

Comment 32

[Mike Conley (:mconley) (:⚙️)]

(In reply to Marco Castelluccio [:marco] from comment #31)
> Why is it only enabled for half the beta cycle? Enabling it for the entire
> beta cycle would solve the issue of crash rates changing suddenly at the
> middle and at the beginning of the cycle.

If I recall correctly, it's because the later betas are supposed to be as close as possible to what we finally ship as release.