1482227 - Add GV API so apps can set Socorro product name and ID in GV crash reports

Status: RESOLVED FIXED

(GeckoView :: General, enhancement, P1)

Description

[Chris Peterson [:cpeterson]]

We would like a new Socorro product called "Focus" (bug 1481696). Focus or GV will need to set the "ProductName" field to "Focus" in the crash report.

The crash report ProductName is set from MOZ_APP_BASENAME, which is currently a compile-time constant. GV will need to make this dynamic if we want different GV-based apps (e.g. Focus, FxR, and Fenix) to have their own ProductNames.

https://searchfox.org/mozilla-central/rev/f0c15db995198a1013e1c5f5b5bea54ef83f1049/mobile/android/geckoview/src/main/java/org/mozilla/gecko/CrashHandler.java#472

https://searchfox.org/mozilla-central/rev/f0c15db995198a1013e1c5f5b5bea54ef83f1049/mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoAppShell.java#135

https://searchfox.org/mozilla-central/search?q=MOZ_APP_BASENAME%3D

TBD: Why do we have nearly identical getCrashExtras() methods in CrashHandler.java and GeckoAppShell.java? Are these for GV and Fennec?

Comment 1

[Chris Peterson [:cpeterson]]

willkg says new apps should also set ProductID (MOZ_APP_ID) so Socorro won't commingle FennecAndroid and GV crash reports.

TBD: what should GV do if an app crashes but has not (yet) set the product name and ID?

Comment 2

[Will Kahn-Greene [:willkg] ET needinfo? me]

I think GeckoView should have its own product name and product id that products override to use their product name and product id. We really don't want to use Fennec's values--that way lies madness.

The ProductName and ProductID are annotations set by the crash reporter:

https://hg.mozilla.org/mozilla-central/file/42e2eeaca65d/toolkit/crashreporter/CrashAnnotations.yaml#l547

Maybe there are other annotations that GeckoView and products want to set that are helpful for later crash analysis?

Comment 3

[Chris Peterson [:cpeterson]]

(In reply to Will Kahn-Greene [:willkg] ET needinfo? me from comment #2)
> I think GeckoView should have its own product name and product id that
> products override to use their product name and product id. We really don't
> want to use Fennec's values--that way lies madness.

So GeckoView's product name and id would just be the default fallback if the GeckoView-powered app did not override the product name and id?

Comment 4

[Will Kahn-Greene [:willkg] ET needinfo? me]

Yes--that's what I was thinking. I don't know enough about how GeckoView is built to know whether that's doable, but maybe it's possible given that it's currently defaulting to Fennec ProductName and ProductID?

Comment 5

[Chris Peterson [:cpeterson]]

(In reply to Will Kahn-Greene [:willkg] ET needinfo? me from comment #4)
> Yes--that's what I was thinking. I don't know enough about how GeckoView is
> built to know whether that's doable, but maybe it's possible given that it's
> currently defaulting to Fennec ProductName and ProductID?

Yes. That would solve the problem (like comment 0) where GeckoView crashes before the app sets a new product name.

Comment 6

[Chris Peterson [:cpeterson]]

To the GeckoView engineer who will fix this bug:

Will has created a "Focus" product name in the Socorro staging server. Once GeckoView and Focus can set the new "Focus" product name, we need to send some test crash reports to the staging server to confirm that the new Focus product name is processed correctly. See bug 1481696 comment 12 for instructions and then needinfo :willkg with the test results in that bug.

Comment 7

[Will Kahn-Greene [:willkg] ET needinfo? me]

(In reply to Chris Peterson [:cpeterson] from comment #5)
> (In reply to Will Kahn-Greene [:willkg] ET needinfo? me from comment #4)
> > Yes--that's what I was thinking. I don't know enough about how GeckoView is
> > built to know whether that's doable, but maybe it's possible given that it's
> > currently defaulting to Fennec ProductName and ProductID?
> 
> Yes. That would solve the problem (like comment 0) where GeckoView crashes
> before the app sets a new product name.

Awesome! If that works out, let me know what the default ProductName will be and I can add support for that to Socorro.

Comment 8

[Chris Peterson [:cpeterson]]

(In reply to Will Kahn-Greene [:willkg] ET needinfo? me from comment #7)
> Awesome! If that works out, let me know what the default ProductName will be
> and I can add support for that to Socorro.

I assume the default ProductName will be "GeckoView". I expect us to receive very few real crash reports with product name "GeckoView". Properly written apps should override GeckoView's default product name early during app startup.

Comment 9

[Will Kahn-Greene [:willkg] ET needinfo? me]

I wrote up bug #1483719 to add "GeckoView" as a supported product. I think I'm going to wait until that's implemented just in case it turns out it's hard/impossible and there are better options.

Comment 10

[James Willcox (:snorp) ([e-mail]) (he/him)]

Attachment: Bug 1482227 - Allow apps to specify product name in CrashReporter r=jchen

Comment 11

[Chris Peterson [:cpeterson]]

(In reply to James Willcox (:snorp) ([e-mail]) from comment #10)
> Created attachment 9003904 [details]
> Bug 1482227 - Allow apps to specify product name in CrashReporter r=jchen

willkg says we should also set the app ProductID (MOZ_APP_ID) in addition to the Product Name.

Looks like MOZ_APP_ID is a UUID:

https://searchfox.org/mozilla-central/search?q=MOZ_APP_ID&case=true

I assume the Socorro team will need to configure the backend to recognize the new Product IDs for GeckoView, Focus, FxR, and Fenix.

Comment 12

[James Willcox (:snorp) ([e-mail]) (he/him)]

(In reply to Chris Peterson [:cpeterson] from comment #11)
> (In reply to James Willcox (:snorp) ([e-mail]) from comment #10)
> > Created attachment 9003904 [details]
> > Bug 1482227 - Allow apps to specify product name in CrashReporter r=jchen
> 
> willkg says we should also set the app ProductID (MOZ_APP_ID) in addition to
> the Product Name.
> 
> Looks like MOZ_APP_ID is a UUID:
> 
> https://searchfox.org/mozilla-central/search?q=MOZ_APP_ID&case=true
> 
> I assume the Socorro team will need to configure the backend to recognize
> the new Product IDs for GeckoView, Focus, FxR, and Fenix.

Oh, hmm. Can we send *just* the ID then, and Socorro can figure out the name itself?

Comment 13

[Will Kahn-Greene [:willkg] ET needinfo? me]

The Socorro collector only looks at the ProductName--not the ProductID. However, there's a processor rule that triggers off of the Fennec ProductID and stomps on the ProductName with "FennecAndroid".

So Socorro needs a unique ProductName and any ProductID that isn't the Fennec one.

Does that work?

Comment 14

[James Willcox (:snorp) ([e-mail]) (he/him)]

Ah, ok. Yeah we can report something else for GV.

Comment 15

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 9003904 [details]
Bug 1482227 - Allow apps to specify product name in CrashReporter r=jchen

Jim Chen [:jchen] [:darchons] has approved the revision.

Comment 16

[Chris Peterson [:cpeterson]]

status-firefox62=wontfix because we won't have Breakpad crash reporting enabled in 62.

Comment 17

[Daosheng Mu[:daoshengmu]]

is there any update so far? It seems like it is ready to land. FxR is waiting for it to set ProductName for the crash reports.

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2522ddc2f7a2
Allow apps to specify product name in CrashReporter r=jchen

Comment 19

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/2522ddc2f7a2

Comment 20

[Daosheng Mu[:daoshengmu]]

But getting a build error after rebase to m-c

:app:compileOfficialWithoutGeckoBinariesNoMinApiPhotonDebugJavaWithJavac/Projects/mozilla/gecko-dev/mobile/android/base/java/org/mozilla/gecko/CrashHandlerService.java:16: error: cannot find symbol
 1:26.83         intent.setClass(this, CrashReporterActivity.class);

Comment 21

[Chris Peterson [:cpeterson]]

@ James, should we uplift to GV 63 Beta?

@ Daosheng, do you still have the CrashHandlerService.java build errors?

Comment 22

[Daosheng Mu[:daoshengmu]]

@Chris, no more build error. Snorp has fixed it at an another bug.

Comment 23

[James Willcox (:snorp) ([e-mail]) (he/him)]

This relies on all the other crash handling changes, which is a bit much to uplift to 63 IMO.