Closed
Bug 1166897
Opened 9 years ago
Closed 9 years ago
Make telemetry more B2G friendly
Categories
(Toolkit :: Telemetry, defect)
Toolkit
Telemetry
Tracking
()
RESOLVED
FIXED
mozilla41
People
(Reporter: fabrice, Assigned: fabrice)
References
Details
(Whiteboard: [uplift])
Attachments
(1 file)
4.48 KB,
patch
|
gfritzsche
:
review+
Sylvestre
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
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).
Assignee | ||
Comment 1•9 years ago
|
||
Attachment #8608304 -
Flags: review?(gfritzsche)
Comment 2•9 years ago
|
||
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?
Flags: needinfo?(fabrice)
Comment 3•9 years ago
|
||
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.
Attachment #8608304 -
Flags: review?(gfritzsche) → review+
Assignee | ||
Comment 4•9 years ago
|
||
(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.
Flags: needinfo?(fabrice)
Comment 6•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/062a3f6e3a37
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
Comment 7•9 years ago
|
||
(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?
Assignee | ||
Comment 8•9 years ago
|
||
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•9 years ago
|
||
(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•9 years ago
|
||
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.
Attachment #8608304 -
Flags: approval-mozilla-aurora?
Comment 11•9 years ago
|
||
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.
Attachment #8608304 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 12•9 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/be983703a966
status-firefox40:
--- → fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•