Last Comment Bug 763991 - fennec build configs should define MOZ_TELEMETRY_REPORTING
: fennec build configs should define MOZ_TELEMETRY_REPORTING
Status: RESOLVED FIXED
:
Product: Firefox for Android
Classification: Client Software
Component: General (show other bugs)
: unspecified
: All All
: -- normal (vote)
: Firefox 16
Assigned To: Matt Brubeck (:mbrubeck)
:
:
Mentors:
Depends on: 722240 762620
Blocks:
  Show dependency treegraph
 
Reported: 2012-06-12 09:16 PDT by Nathan Froyd [:froydnj]
Modified: 2012-07-14 14:31 PDT (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
wontfix
+
fixed


Attachments
patch to add MOZ_TELEMETRY_REPORTING (10.25 KB, patch)
2012-06-12 09:51 PDT, Nathan Froyd [:froydnj]
blassey.bugs: review+
akeybl: approval‑mozilla‑aurora+
akeybl: approval‑mozilla‑beta-
Details | Diff | Splinter Review
Disable telemetry prompt in non-telemetry builds (3.82 KB, patch)
2012-06-12 15:10 PDT, Matt Brubeck (:mbrubeck)
mark.finkle: review+
Details | Diff | Splinter Review

Description Nathan Froyd [:froydnj] 2012-06-12 09:16:16 PDT
bug 722240 disabled telemetry for non-official builds based on the presence of MOZ_TELEMETRY_REPORTING.  Unfortunately, the Android builds don't define MOZ_TELEMETRY_REPORTING, so bug 722240 completely disabled telemetry for them.

We should add the necessary logic in the mozconfigs.  Probably shouldn't uplift this until at least bug 762620 is resolved.
Comment 1 Nathan Froyd [:froydnj] 2012-06-12 09:51:44 PDT
Created attachment 632299 [details] [diff] [review]
patch to add MOZ_TELEMETRY_REPORTING

Wasn't sure about the android-{armv6,x86} builds, decided to add the necessary stanzas there too.

Wasn't sure about the l10n-* configs, decided to follow existing practice and not add them.
Comment 2 Matt Brubeck (:mbrubeck) 2012-06-12 15:10:30 PDT
Created attachment 632434 [details] [diff] [review]
Disable telemetry prompt in non-telemetry builds

In addition, we should disable the telemetry UI in builds without telemetry.
Comment 3 Mark Finkle (:mfinkle) (use needinfo?) 2012-06-13 06:06:40 PDT
Comment on attachment 632434 [details] [diff] [review]
Disable telemetry prompt in non-telemetry builds

>diff --git a/mobile/android/chrome/content/browser.js b/mobile/android/chrome/content/browser.js

>+#ifdef MOZ_TELEMETRY_REPORTING
>     Telemetry.init();
>+#endif

You missed a Telemetry.unint();

r+ with that change
Comment 4 Nathan Froyd [:froydnj] 2012-06-13 06:13:58 PDT
(In reply to Matt Brubeck (:mbrubeck) from comment #2)
> Disable telemetry prompt in non-telemetry builds

Matt, do you want to land this patch prior to mine, or does it not really matter?
Comment 5 Matt Brubeck (:mbrubeck) 2012-06-13 08:31:54 PDT
(In reply to Mark Finkle (:mfinkle) from comment #3)
> You missed a Telemetry.unint();

Fixed.

(In reply to Nathan Froyd (:froydnj) from comment #4)
> Matt, do you want to land this patch prior to mine, or does it not really
> matter?

Landed both patches together:
https://hg.mozilla.org/integration/mozilla-inbound/rev/112ab5ba3ac4
https://hg.mozilla.org/integration/mozilla-inbound/rev/7b447b08eb73

Do we also want these on Aurora and Beta, or should we wait until bug 762620 is resolved?
Comment 7 Gian-Carlo Pascutto [:gcp] 2012-07-09 10:44:55 PDT
Comment on attachment 632299 [details] [diff] [review]
patch to add MOZ_TELEMETRY_REPORTING

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 762590, Bug 764144 and Bug 762620
User impact if declined: No Telemetry on Fennec
Testing completed (on m-c, etc.): Has been in m-c for a while.
Risk to taking this patch (and alternatives if risky): Now that bug 762590 and 762620 are fixed, I'd say very low risk of accidentally using up battery/user data quota.
Comment 8 Alex Keybl [:akeybl] 2012-07-09 14:09:43 PDT
Comment on attachment 632299 [details] [diff] [review]
patch to add MOZ_TELEMETRY_REPORTING

[Triage Comment]
I wish this had been nominated for uplift sooner - we can't risk any regressions to power consumption in 14.0.1 and I'm not comfortable with going out having only had the device coverage testing we've gotten on Nightlies so far. Let's get this into Aurora 15, however. If anybody disagrees, please email/ping me directly as we're hours away from going to build.
Comment 9 Brad Lassey [:blassey] (use needinfo?) 2012-07-10 04:38:01 PDT
Comment on attachment 632299 [details] [diff] [review]
patch to add MOZ_TELEMETRY_REPORTING

Alex, the reason we scrambled to get idle-daily fixed was to turn on telemetry for 14.01, renominating.
Comment 10 Alex Keybl [:akeybl] 2012-07-10 10:09:28 PDT
Comment on attachment 632299 [details] [diff] [review]
patch to add MOZ_TELEMETRY_REPORTING

[Triage comment]
Blassey and I spoke further about this. Based upon the risk in bug 762620, we actually backed that out. Given that, no need to land this on beta.
Comment 11 Matt Brubeck (:mbrubeck) 2012-07-14 14:31:09 PDT
Pushed the patch to enable telemetry at build time:
https://hg.mozilla.org/releases/mozilla-aurora/rev/34379dbcf2cc

Note: I did not uplift the second patch, to disable the Telemetry UI in unofficial builds, because it is not approved and does not apply cleanly to Aurora.  Since that patch only affects unofficial builds, it's not a priority for Aurora uplift.

Note You need to log in before you can comment on or make changes to this bug.