722240 - Telemetry shouldn't be accepted for non-official builds

Status: RESOLVED FIXED

(Toolkit :: Telemetry, defect)

Description

[Gian-Carlo Pascutto [:gcp]]

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]

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]]

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)]

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]]

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]]

>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]]

(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]]

This is a small patch to Telemetry to consult MOZ_OFFICIAL; I'll work on that.

Comment 8

[Nathan Froyd [:froydnj]]

Attachment: 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)]

Comment on attachment 593781 [details] [diff] [review]
patch

use MOZ_TELEMETRY_REPORTING

use CanSend.

Comment 10

[Nathan Froyd [:froydnj]]

Attachment: 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]]

Attachment: patch

Er.  It works better if you rebase before attaching patches.

Comment 12

[Nathan Froyd [:froydnj]]

Attachment: patch

Rebased to deal with fuzz.  Carrying over r+.

Comment 13

[Gian-Carlo Pascutto [:gcp]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1c1ad3dcd782

Comment 14

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/1c1ad3dcd782