1483331 - Send crash pings for GeckoView

Status: RESOLVED DUPLICATE of bug 1522461

(Toolkit :: Crash Reporting, enhancement, P2)

Description

[Chris Peterson [:cpeterson]]

Gabriele implemented Fennec crash pings in bug 1307153. Much of that code can probably be used in GeckoView.

Gabriele says:

There's a few bits that need to be working together so that GeckoView can send full crash pings. First of all you need the code that gathers the crash data required to populate the ping. This currently lives in the CrashReporterActivity in Fennec:

https://searchfox.org/mozilla-central/rev/aff5d4ad5d7fb2919d267cbc23b1d87ae3cf0110/mobile/android/base/java/org/mozilla/gecko/CrashReporterActivity.java#157-196

This requires three other chunks of code:

* the minidump-analyzer, which is a standalone tool for gathering stack traces from a minidump
* the TelemetryCrashPingBuilder which is the code responsible for assembling the actual ping
* the TelemetryDispatcher which has been modified to deal with crash pings

From what I can tell some of this code has been copied into GeckoView's CrashReporterService:

https://searchfox.org/mozilla-central/rev/aff5d4ad5d7fb2919d267cbc23b1d87ae3cf0110/mobile/android/geckoview/src/main/java/org/mozilla/gecko/CrashReporterService.java#173-206

However it's missing the part where we actually send the ping. I also had the vague recollection that crash ping support had been added to the mozilla-mobile/android-components repository but it doesn't seem to be the case either.

Comment 1

[Gabriele Svelto [:gsvelto] (PTO)]

Since I've got some spare cycles I figured I could implement this however the notes in comment 0 are now outdated. It seems that GeckoView delegates crash handling to the implementer of the application so I was wondering where do we want to put the crash ping machinery. Do we add it to the ExampleCrashHandler so that it can be used as a starting point by others? Or do we want it separate from the crash handler? Also I'm a bit confused about how GeckoViewTelemetryController works, it doesn't seem to have a way of submitting external pings like the regular TelemetryController (which is required for crash pings). I'm I missing something?

Comment 2

[Chris Peterson [:cpeterson]]

Eugen, you volunteered to work on this bug before I saw Gabriele's comment here. Would you like to guide Gabriele through this bug (so you can then focus on the new content blocking API)?

Comment 3

[Eugen Sawin [:esawin]]

GV does not send out telemetry, it collects the data persistently and exposes it in the mobile-ping format through RuntimeTelemetry.java (which uses GeckoViewTelemetryController under the hood).

It is the app's responsibility to send the ping (e.g., using the Android component).
That allows us to cater GV to privacy-focused apps and opens up the option of bundling the mobile-ping with app-specific telemetry for increased efficiency.

I don't know if that model is compatible with the crash ping or if we have different plans for it, snorp?

Comment 4

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

(In reply to Eugen Sawin [:esawin] from comment #3)
> GV does not send out telemetry, it collects the data persistently and
> exposes it in the mobile-ping format through RuntimeTelemetry.java (which
> uses GeckoViewTelemetryController under the hood).
> 
> It is the app's responsibility to send the ping (e.g., using the Android
> component).
> That allows us to cater GV to privacy-focused apps and opens up the option
> of bundling the mobile-ping with app-specific telemetry for increased
> efficiency.
> 
> I don't know if that model is compatible with the crash ping or if we have
> different plans for it, snorp?

The crash ping is a separate thing. It's basically a small crash report with no PII that (AFAIK?) we always send unconditionally. The reason this would be good for GV is that it would allow us to assess the health of GV in the field even if those apps are not sending their crash reports to us (which is entirely optional).

Gabriele added the support for Fennec, so maybe there's something more he'd like to add (or correct)?

Comment 5

[Gabriele Svelto [:gsvelto] (PTO)]

OK, I didn't know we wanted to delegate Telemetry to the application itself, do we have a pre-made telemetry component that the applications could use? In that case it would be best to add the crash ping construction there. Once the ping is built it can be sent just like other ones so there's nothing special there.

I believe it would be possibly to send crash pings unconditionally from GeckoView but it would require us to build some GeckoView-specific telemetry machinery. It doesn't need to be something complex, just something super-simple like the pingsender we use on desktop that only tries to send the ping once and that's it. However I don't know if that would be desirable for developers using GeckoView, we should at least give them the possibility of opting out of it.

Comment 6

[Eugen Sawin [:esawin]]

(In reply to Gabriele Svelto [:gsvelto] from comment #5)
> OK, I didn't know we wanted to delegate Telemetry to the application itself,
> do we have a pre-made telemetry component that the applications could use?
> In that case it would be best to add the crash ping construction there. Once
> the ping is built it can be sent just like other ones so there's nothing
> special there.

I think Focus is using this Android component: https://github.com/mozilla-mobile/android-components/tree/master/components/service/telemetry

> I believe it would be possibly to send crash pings unconditionally from
> GeckoView but it would require us to build some GeckoView-specific telemetry
> machinery. It doesn't need to be something complex, just something
> super-simple like the pingsender we use on desktop that only tries to send
> the ping once and that's it. However I don't know if that would be desirable
> for developers using GeckoView, we should at least give them the possibility
> of opting out of it.

Seems like that would be the preferred way for the crash ping.
We can add a setting to disable it in GeckoRuntimeSettings.

Comment 7

[Gabriele Svelto [:gsvelto] (PTO)]

(In reply to Eugen Sawin [:esawin] from comment #6)
> I think Focus is using this Android component:
> https://github.com/mozilla-mobile/android-components/tree/master/components/
> service/telemetry

That seems to be missing the crash ping builder, maybe we should start by adding it there.

> > I believe it would be possibly to send crash pings unconditionally from
> > GeckoView but it would require us to build some GeckoView-specific telemetry
> > machinery. It doesn't need to be something complex, just something
> > super-simple like the pingsender we use on desktop that only tries to send
> > the ping once and that's it. However I don't know if that would be desirable
> > for developers using GeckoView, we should at least give them the possibility
> > of opting out of it.
> 
> Seems like that would be the preferred way for the crash ping.
> We can add a setting to disable it in GeckoRuntimeSettings.

OK, I'll investigate if it's possible to add it this way without lumping too much code into GeckoView.

Comment 8

[Gabriele Svelto [:gsvelto] (PTO)]

While thinking about the best way to implement this I realized something important: the crash ping assembly relies among other things on a lot of information that has to be specifically gathered from Gecko (for example the whitelisted crash annotations in Fennec are generated by a script). Delegating the assembly outside of Gecko is almost meaningless. So to reiterate the conclusion I came up with above it's best to assemble the ping in Gecko, provide a simple way to transfer it but leave the application the ability to opt out.

Comment 9

[Chris Peterson [:cpeterson]]

James says that GeckoView-powered apps will be responsible for sending their own crash pings and that this GeckoView bug is redundant with the work to use Glean to send crash pings in Fenix: bug 1522461 and https://github.com/mozilla-mobile/android-components/issues/1820