1166897 - Make telemetry more B2G friendly

Status: RESOLVED FIXED

(Toolkit :: Telemetry, defect)

Description

[[:fabrice] Fabrice Desré]

On device, we get errors from the telemetry code because it's
1) not doing the right thing to run when the jsloader.reuseGlobal pref is true.
2) tries to use the AddonManager.jsm module but we don't have it in device builds.
3) relies on components that are not implemented on b2g (nsIShellService in this case).

Comment 1

[[:fabrice] Fabrice Desré]

Attachment: telemetry-environment.patch

Comment 2

[Georg Fritzsche [:gfritzsche]]

Does it make sense to use the rest of TelemetryEnvironment on B2G? Do you want to use it?
Just checking because talking with Tamara the plan did not seem finalized yet.

More of a side-note:
I'd love to go for less pre-processor usage in the Telemetry modules (i.e. use JS constants instead), would that be bad for B2G resource constraints?

Comment 3

[Georg Fritzsche [:gfritzsche]]

Comment on attachment 8608304 [details] [diff] [review]
telemetry-environment.patch

Review of attachment 8608304 [details] [diff] [review]:
-----------------------------------------------------------------

This looks fine with the one note below addressed.

::: toolkit/components/telemetry/TelemetryEnvironment.jsm
@@ +204,3 @@
>    return new Promise((resolve) =>
>                       AddonManager.getAddonsByTypes(aTypes, (addons) => resolve(addons)));
> +#endif

We shouldn't call that function at all on Gonk, so we don't need this change.

Comment 4

[[:fabrice] Fabrice Desré]

(In reply to Georg Fritzsche [:gfritzsche] from comment #2)
> Does it make sense to use the rest of TelemetryEnvironment on B2G? Do you
> want to use it?
> Just checking because talking with Tamara the plan did not seem finalized
> yet.

I don't know what the plans are, and I'm not even sure who's making the plan.

> More of a side-note:
> I'd love to go for less pre-processor usage in the Telemetry modules (i.e.
> use JS constants instead), would that be bad for B2G resource constraints?

I don't think that would matter too much. Do we have constants to identify products and platforms independently? b2g desktop still support add-ons for instance.

Comment 5

[Pulsebot]

https://hg.mozilla.org/integration/b2g-inbound/rev/062a3f6e3a37

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/062a3f6e3a37

Comment 7

[Georg Fritzsche [:gfritzsche]]

(In reply to Fabrice Desré [:fabrice] from comment #4)
> (In reply to Georg Fritzsche [:gfritzsche] from comment #2)
> > Does it make sense to use the rest of TelemetryEnvironment on B2G? Do you
> > want to use it?
> > Just checking because talking with Tamara the plan did not seem finalized
> > yet.
> 
> I don't know what the plans are, and I'm not even sure who's making the plan.

Ah, ok - thills & rnicoletti are currently picking up Telemetry on B2G.
> 
> > More of a side-note:
> > I'd love to go for less pre-processor usage in the Telemetry modules (i.e.
> > use JS constants instead), would that be bad for B2G resource constraints?
> 
> I don't think that would matter too much. Do we have constants to identify
> products and platforms independently? b2g desktop still support add-ons for
> instance.

Interesting, i didn't know that. Is B2G desktop a separate product that we care about (i.e. should separately support here) or is it just for testing/developing?

Comment 8

[[:fabrice] Fabrice Desré]

B2G desktop is supported as the "simulator addon" for webide. I'm not 100% sure they are still using add-ons or not. We also have a couple of add-ons used with b2g desktop for gaia developers. So short term we likely want to keep that working, but as final products, nothing based on the b2g runtime should need traditional add-on support.

Comment 9

[Georg Fritzsche [:gfritzsche]]

(In reply to Fabrice Desré [:fabrice] from comment #4)
> (In reply to Georg Fritzsche [:gfritzsche] from comment #2)
> > Does it make sense to use the rest of TelemetryEnvironment on B2G? Do you
> > want to use it?
> > Just checking because talking with Tamara the plan did not seem finalized
> > yet.
> 
> I don't know what the plans are, and I'm not even sure who's making the plan.
> 
> > More of a side-note:
> > I'd love to go for less pre-processor usage in the Telemetry modules (i.e.
> > use JS constants instead), would that be bad for B2G resource constraints?
> 
> I don't think that would matter too much. Do we have constants to identify
> products and platforms independently? b2g desktop still support add-ons for
> instance.

I found we have AppConstants.jsm now. We could extend that where it is still lacking any constants.

Comment 10

[Georg Fritzsche [:gfritzsche]]

Comment on attachment 8608304 [details] [diff] [review]
telemetry-environment.patch

Approval Request Comment
[Feature/regressing bug #]:
Unified Telemetry, https://wiki.mozilla.org/Unified_Telemetry
This is part of the first (main) batch of uplifts to 40 to enable shipping on that train, see bug 1120356, comment 2.
[User impact if declined]:
Data & measurement insight projects delayed or blocked with direct impact on projects depending on this.
[Describe test coverage new/current, TreeHerder]:
We have good automation coverage of the feature.
We also had manual tests of the main tasks as well as confirmation of correct behavior on Nightly for the patches here.
[Risks and why]:
Low-risk - these patches are rather isolated to Telemetry and have been on Nightly for a while with no bad reports.
We intend to track on-going data quality and other issues during the 40 aurora & beta and flip the new behavior off when it doesn't meet the requirements.
[String/UUID change made/needed]:
The only string changes were to the about:telemetry page.
We decided that we can live with missing translations on that page for a cycle as that page is not exactly user-facing.

Comment 11

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8608304 [details] [diff] [review]
telemetry-environment.patch

Unified telemetry is an important new feature. It is blocking some other projects. Taking it.

Comment 12

[Georg Fritzsche [:gfritzsche]]

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