Last Comment Bug 722240 - Telemetry shouldn't be accepted for non-official builds
: Telemetry shouldn't be accepted for non-official builds
Status: RESOLVED FIXED
:
Product: Toolkit
Classification: Components
Component: Telemetry (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla13
Assigned To: Nathan Froyd [:froydnj]
:
: Georg Fritzsche [:gfritzsche]
Mentors:
Depends on: 826998
Blocks: 763991
  Show dependency treegraph
 
Reported: 2012-01-30 01:00 PST by Gian-Carlo Pascutto [:gcp]
Modified: 2013-01-05 02:34 PST (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (1.94 KB, patch)
2012-02-02 03:04 PST, Nathan Froyd [:froydnj]
taras.mozilla: review+
Details | Diff | Splinter Review
patch (1.96 KB, patch)
2012-02-02 05:58 PST, Nathan Froyd [:froydnj]
nfroyd: review+
Details | Diff | Splinter Review
patch (2.43 KB, patch)
2012-02-02 06:02 PST, Nathan Froyd [:froydnj]
nfroyd: review+
Details | Diff | Splinter Review
patch (2.43 KB, patch)
2012-02-09 08:49 PST, Nathan Froyd [:froydnj]
nfroyd: review+
Details | Diff | Splinter Review

Description Gian-Carlo Pascutto [:gcp] 2012-01-30 01:00:16 PST
It looks like we currently accept Telemetry submissions from non-official builds, i.e. development builds make by developers. This is bad for at least 2 reasons:

1) Those builds will be atypical and might have severe bugs or different behavior. They will clutter up real user data, or present outliers/extreme cases that don't really exist.

2) Temporary telemetry or telemetry with typos is showing up. i.e. "1", "7", "URLCLARSIFIER" (yeah, the latter one is my fault!)
Comment 1 Pedro Alves 2012-02-01 03:19:57 PST
We have similar problems on the rest of the data, and it's very hard to "validate" a build. The only thing I can think of is a list of platformbuildid's that we'd check against.

Does anything like that exists?
Comment 2 Gian-Carlo Pascutto [:gcp] 2012-02-01 03:24:17 PST
Not sure. Maybe taras can clarify if anything extra needs to be sent along. There's always the possibility of not sending Telemetry data unless we're being built with MOZ_OFFICIAL=1, I think?
Comment 3 (dormant account) 2012-02-01 03:56:26 PST
good point. we currently don't prompt for telemetry on development builds, but if you enable the pref, it'll get sent. We should probably not set the server pref for unofficial builds.
Comment 4 Daniel Einspanjer [:dre] [:deinspanjer] 2012-02-01 04:07:55 PST
Is it not possible that developers would want to look at telemetry data for these unofficial builds? What about things like partner builds?

What if we accept all the data, but we partition it on our side by adding a build-type field and have a checkbox on the dashboards that says "include unofficial builds"?
Comment 5 Gian-Carlo Pascutto [:gcp] 2012-02-01 04:18:28 PST
>Is it not possible that developers would want to look at telemetry data for these 
>unofficial builds? 

For their local builds, they can already do so without actually submitting it (about:telemetry). 

Looking at aggregate date for developer builds: can't see much use for that honestly.

>What about things like partner builds?

If this includes things like Iceweasel then I'd really like to filter them out, yes.

>What if we accept all the data, but we partition it on our side by adding a build-type 
>field and have a checkbox on the dashboards that says "include unofficial builds"?

That's fine too, if you're able to tell what an official build is. Comment 1 suggests that this is lacking?
Comment 6 Daniel Einspanjer [:dre] [:deinspanjer] 2012-02-01 05:17:48 PST
(In reply to Gian-Carlo Pascutto (:gcp) from comment #5)
> >Is it not possible that developers would want to look at telemetry data for these 
> >unofficial builds? 
> 
> For their local builds, they can already do so without actually submitting
> it (about:telemetry). 
> 
> Looking at aggregate date for developer builds: can't see much use for that
> honestly.

I was thinking about the case where a developer might be working on code related to a particular histogram, and would want to be able to see a trend or comparison of builds to determine if there was an improvement or regression.  If this is not a likely use case then I agree we should just change the submit URL for custom builds.  Actually, if there was a developer who wanted to keep the data, they could always spin up a local instance of the collection server for themselves and point the build at that.

> >What about things like partner builds?
> 
> If this includes things like Iceweasel then I'd really like to filter them
> out, yes.

Not so much IceWeasel as things like the Yahoo or Yandex or EUBallot builds that are our core code but customized by partners for their own distribution.
> 
> >What if we accept all the data, but we partition it on our side by adding a build-type 
> >field and have a checkbox on the dashboards that says "include unofficial builds"?
> 
> That's fine too, if you're able to tell what an official build is. Comment 1
> suggests that this is lacking?

If we could use a combination of existing fields such as applicationABI, channel, distribution, then we might be able to figure it out. Otherwise, we would need to build a list of allowed build ids and remove or flag submissions from builds that don't match.
Comment 7 Nathan Froyd [:froydnj] 2012-02-01 09:36:23 PST
This is a small patch to Telemetry to consult MOZ_OFFICIAL; I'll work on that.
Comment 8 Nathan Froyd [:froydnj] 2012-02-02 03:04:31 PST
Created attachment 593781 [details] [diff] [review]
patch

Here's a simple patch: add isOfficial boolean to the interface and make sure that we only send "idle-daily" reasons for idle pings, "test-ping" for everything else.
Comment 9 (dormant account) 2012-02-02 03:25:27 PST
Comment on attachment 593781 [details] [diff] [review]
patch

use MOZ_TELEMETRY_REPORTING

use CanSend.
Comment 10 Nathan Froyd [:froydnj] 2012-02-02 05:58:56 PST
Created attachment 593805 [details] [diff] [review]
patch

Renamed the variable and made it dependent on official-ness and telemetry reporting.

Note that we now test MOZILLA_OFFICIAL, not MOZ_OFFICIAL_BRANDING.  The former is what gets used for all builds (Nightly, Aurora, Beta, Release), whereas the latter only gets used for release builds.
Comment 11 Nathan Froyd [:froydnj] 2012-02-02 06:02:12 PST
Created attachment 593806 [details] [diff] [review]
patch

Er.  It works better if you rebase before attaching patches.
Comment 12 Nathan Froyd [:froydnj] 2012-02-09 08:49:54 PST
Created attachment 595767 [details] [diff] [review]
patch

Rebased to deal with fuzz.  Carrying over r+.
Comment 13 Gian-Carlo Pascutto [:gcp] 2012-02-09 11:26:59 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/1c1ad3dcd782
Comment 14 Ed Morley [:emorley] 2012-02-10 07:47:36 PST
https://hg.mozilla.org/mozilla-central/rev/1c1ad3dcd782

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