1487578 - ship Telemetry Coverage SAO update

Status: RESOLVED FIXED

(Data Platform and Tools :: General, enhancement)

Description

[Robert Helmer [:rhelmer]]

Per https://blog.mozilla.org/data/2018/08/20/effectively-measuring-search-in-firefox/ we want to ship an extension (system add-on update aka SAO) which will activate for only 1% of users and send in a privacy-preserving bit of diagnostic info.

Comment 1

[Robert Helmer [:rhelmer]]

Attachment: generate-clientid.js

Instructions for QA:

Overall this is a pretty typical system add-on update. It is a legacy extension, and only runs for 1% of users, but you can make yourself be in that 1% using the attached script (which can be run in the Browser Console).

I've already run this script and you can use this Telemetry client ID:
53fcb8b2-781a-3949-8b80-59bf15732200

This can be set in your profile (when Firefox is not running) in:
prefs.js ("toolkit.telemetry.cachedClientID" pref)
datareporting/state.json

If you start Nightly and use about:debugging, then you can load the unsigned extension (I am going to attach the extension momentarily and request signing+staging so it will work on Release)

I was able to confirm this was working via the "network" tab in the Browser Toolbox, if the extension is working there should be a connection to https://telemetry-coverage.mozilla.org with a payload similar to:

{
   "appVersion": "63.0a1",
   "appUpdateChannel": "nightly",
   "osName": "Darwin",
   "osVersion": "17.7.0",
   "telemetryEnabled": true
}

There should not be any UUID or other identifying info sent along with this payload, and note that it goes to a different endpoint than regular Telemetry.

The only other thing to test here is that this extension has a special boolean opt-out pref: "toolkit.telemetry.coverage.opt-out". This pref does not exist by default and must be created, if set to true then the extension should not send a payload as above for users in the 1% sample (such as the Telemetry client ID above will be)

Comment 2

[Robert Helmer [:rhelmer]]

Attachment: System add-on update for telemetry coverage (v1.0), unsigned

Please sign this system add-on update, thanks!

Comment 3

[Robert Helmer [:rhelmer]]

(In reply to Robert Helmer [:rhelmer] from comment #2)
> Created attachment 9005525 [details]
> System add-on update for telemetry coverage (v1.0), unsigned
> 
> Please sign this system add-on update, thanks!

Rehan, I am out tomorrow - would you mind staging this on the test channel (for release) when the signed version is ready? Thanks!

Comment 4

[Rehan Dalal (Inactive)]

I'm on PTO tomorrow and out till Tuesday. Tagging mythmon in.

Comment 5

[:wezhou]

Attachment: signed.9005525.xpi

Signed file attached. Please test.

Comment 6

[Michael Cooper [:mythmon]]

Sorry, I'm not confident doing this change without talking too Rehan or Rob. There is too much implied indirection for me to pick this up without documentation.

Re-tagging Rob, since he should be back before Rehan.

Comment 7

[Rehan Dalal (Inactive)]

This is now on the test channel (release-sysaddon) and pending sign off on release.

It has also replaced the previous version that was there (from bug 1480194). Let me know if they needed to be bundled.

Comment 8

[Robert Helmer [:rhelmer]]

(In reply to Rehan Dalal [:rehan, :rdalal] from comment #7)
> This is now on the test channel (release-sysaddon) and pending sign off on
> release.

Thanks!

> It has also replaced the previous version that was there (from bug 1480194).
> Let me know if they needed to be bundled.

Yes this should replace the previous version, thanks for the confirmation.

Comment 9

[Hani Yacoub, Desktop QA]

I tried using the singed extension attached (signed.9005525.xpi) and load it this time in about:addons by following the same steps mentioned before, but I received this message  "Telemetry coverage could not be installed because it's not compatible with firefox 62 " and I'm not sure what I'm doing wrong.

I also tried to trigger it using the following command "Components.utils.import("resource://gre/modules/AddonManager.jsm"); AddonManagerPrivate.backgroundUpdateCheck();" but nothing happened. (Please note that I updated the update channel to "release-sysaddon" value)

I am not sure what I am doing wrong.

Comment 10

[Julien Cristau [:jcristau]]

(In reply to Hani Yacoub from comment #9)
> I tried using the singed extension attached (signed.9005525.xpi) and load it
> this time in about:addons by following the same steps mentioned before, but
> I received this message  "Telemetry coverage could not be installed because
> it's not compatible with firefox 62 " and I'm not sure what I'm doing wrong.
> 
> I also tried to trigger it using the following command
> "Components.utils.import("resource://gre/modules/AddonManager.jsm");
> AddonManagerPrivate.backgroundUpdateCheck();" but nothing happened. (Please
> note that I updated the update channel to "release-sysaddon" value)
> 
Right now as far as I can tell this is set up to be offered to 61.x, *not* 62.0.

On 62.0rc2, I set extensions.systemAddon.update.url to https://aus5.mozilla.org/update/3/SystemAddons/61.0/%BUILD_ID%/%BUILD_TARGET%/%LOCALE%/release-sysaddon/%OS_VERSION%/%DISTRIBUTION%/%DISTRIBUTION_VERSION%/update.xml (replacing %VERSION% with 61.0 to fake it, and %CHANNEL% with release-sysaddon), then ran AddonManagerPrivate.backgroundUpdateTimerHandler() in the browser console, and the addon was installed.

Comment 11

[Julien Cristau [:jcristau]]

(In reply to Julien Cristau [:jcristau] from comment #10)
> Right now as far as I can tell this is set up to be offered to 61.x, *not*
> 62.0.

BTW the addon manifest says <em:minVersion>61.*</em:minVersion>, I seem to remember that this means it will refuse to install on 61.0.x because "61.*" is greater than 61.0.x; should that be 61.0, or am I misremembering?

Comment 12

[Hani Yacoub, Desktop QA]

Attachment: params.png

Thank you Julien for the details. I followed the steps you mentioned above on Firefox 62.0rc2 and I set up the followings: 
- Took the Telemetry client ID: 53fcb8b2-781a-3949-8b80-59bf15732200
and set it the profile (when Firefox is not running) in: prefs.js ("toolkit.telemetry.cachedClientID" pref) and datareporting/state.json
- I set extensions.systemAddon.update.url to "https://aus5.mozilla.org/update/3/SystemAddons/61.0/%BUILD_ID%/%BUILD_TARGET%/%LOCALE%/release-sysaddon/%OS_VERSION%/%DISTRIBUTION%/%DISTRIBUTION_VERSION%/update.xml" and triggered the system addons

The request URL displayed in the console looks a bit different that the one mentioned in comment 1. (please see headers.png and params.png screenshots attached)

And I'm not sure it this is the expected behavior.
Should it be installed on other Firefox version besides 62?

Comment 13

[Hani Yacoub, Desktop QA]

Attachment: headers.png

Comment 14

[Julien Cristau [:jcristau]]

(In reply to Hani Yacoub from comment #13)
> Created attachment 9006229 [details]
> headers.png

That shows a connection to versioncheck-bg.addons.mozilla.org, not telemetry-coverage.m.o?

Comment 15

[Hani Yacoub, Desktop QA]

The URL also contains "[e-mail]">[e-mail]" that why I wasn't sure about the results.

Comment 16

[Robert Helmer [:rhelmer]]

Did Julien's comments answer your concern or is there anything you wanted me to look at still? Thanks!

Comment 17

[Robert Helmer [:rhelmer]]

(In reply to Julien Cristau [:jcristau] from comment #11)
> (In reply to Julien Cristau [:jcristau] from comment #10)
> > Right now as far as I can tell this is set up to be offered to 61.x, *not*
> > 62.0.
> 
> BTW the addon manifest says <em:minVersion>61.*</em:minVersion>, I seem to
> remember that this means it will refuse to install on 61.0.x because "61.*"
> is greater than 61.0.x; should that be 61.0, or am I misremembering?

Ack you are right, good catch (from the browser console on Release w/ extension logging on):

addons.xpi	WARN	System add-on [e-mail]">[e-mail] isn't compatible with the application.

I'll update the XPI momentarily.

Comment 18

[Robert Helmer [:rhelmer]]

Attachment: System add-on update for telemetry coverage (v2.0), unsigned

Please sign this updated version of the Telemetry Coverage system add-on update. Thanks!

Comment 19

[:wezhou]

Attachment: signed.9006397.xpi

Signed file attached. Please test.

Comment 20

[Robert Helmer [:rhelmer]]

(In reply to :wezhou from comment #19)
> Created attachment 9006637 [details]
> signed.9006397.xpi
> 
> Signed file attached. Please test.

Rehan, would you mind staging this on the release-sysaddon channel (v2.0)? Thanks!

Comment 21

[Rehan Dalal (Inactive)]

This is on the tests channel (release-sysaddon) and pending sign off on release.

Comment 22

[Julien Cristau [:jcristau]]

This is currently set up in balrog for 61.0(.x).  Is that what we want, or should it be offered to 62.0(.x) too?

Comment 23

[Robert Helmer [:rhelmer]]

(In reply to Julien Cristau [:jcristau] from comment #22)
> This is currently set up in balrog for 61.0(.x).  Is that what we want, or
> should it be offered to 62.0(.x) too?

We should do 62 at this point too, thanks!

Comment 24

[Julien Cristau [:jcristau]]

(In reply to Robert Helmer [:rhelmer] from comment #23)
> (In reply to Julien Cristau [:jcristau] from comment #22)
> > This is currently set up in balrog for 61.0(.x).  Is that what we want, or
> > should it be offered to 62.0(.x) too?
> 
> We should do 62 at this point too, thanks!

I don't seem to have permissions to do that in balrog.  Rehan?

Comment 25

[Rehan Dalal (Inactive)]

This is now on the test channel (release-sysaddon) for 61.* and 62.* and pending sign off for both versions on release.

Comment 26

[Erin Lancaster [:elan]]

Can we get RelMan sign off? Many thanks.

Comment 27

[Julien Cristau [:jcristau]]

Hani, what is the status of testing on 61 and 62?

It also looks like https://github.com/mozilla/one-off-system-add-ons/pull/123#discussion_r215205711 didn't get to a resolution?

Comment 28

[Robert Helmer [:rhelmer]]

(In reply to Julien Cristau [:jcristau] from comment #27)
> Hani, what is the status of testing on 61 and 62?
> 
> It also looks like
> https://github.com/mozilla/one-off-system-add-ons/pull/
> 123#discussion_r215205711 didn't get to a resolution?

Yes we should resolve this.

Georg, did you get a chance to see my responses to the GH PR ^?

It's quite easy to miss these sorts of responses in GH unfortunately :/

Comment 29

[Hani Yacoub, Desktop QA]

Attachment: 62.0.png

I am attaching 2 screenshots comparing Firefox 62.0 and 61.0.2 results.
I followed the steps mentioned in comment 12 but without changing  "extensions.systemAddon.update.url" pref.

In firefox 62.0: "Telemetry coverage" is displayed in Firefox features in "about:support", and in the Network tab it shows a connection to "telemetry-coverage.m.o".

But in Firefox 61.0.2:  "Telemetry coverage" isn't displayed in Firefox features, and in the Network tab it shows a connection different than the one displayed in Firefox 62.
And this is displayed in the console:
1536320159663	addons.xpi	WARN	System add-on [e-mail]">[e-mail] isn't compatible with the application.

Comment 30

[Hani Yacoub, Desktop QA]

Attachment: 61.0.2.png

Comment 31

[Julien Cristau [:jcristau]]

The new version says "<em:minVersion>61.0.*</em:minVersion>" which is still > 61.0.2.  Robert, I think this needs to be
"<em:minVersion>61.0</em:minVersion>".

Comment 32

[Georg Fritzsche [:gfritzsche]]

(In reply to Robert Helmer [:rhelmer] from comment #28)
> (In reply to Julien Cristau [:jcristau] from comment #27)
> > Hani, what is the status of testing on 61 and 62?
> > 
> > It also looks like
> > https://github.com/mozilla/one-off-system-add-ons/pull/
> > 123#discussion_r215205711 didn't get to a resolution?
> 
> Yes we should resolve this.
> 
> Georg, did you get a chance to see my responses to the GH PR ^?
> 
> It's quite easy to miss these sorts of responses in GH unfortunately :/

Indeed, thanks for flagging this.
I commented there, let me know if more is needed.

Comment 33

[Robert Helmer [:rhelmer]]

(In reply to Julien Cristau [:jcristau] from comment #31)
> The new version says "<em:minVersion>61.0.*</em:minVersion>" which is still
> > 61.0.2.  Robert, I think this needs to be
> "<em:minVersion>61.0</em:minVersion>".

Fixed in the github review (we ended up setting it pretty far back as we may want to ship to older clients too)

Comment 34

[Robert Helmer [:rhelmer]]

Attachment: System add-on update for telemetry coverage (v3.0), unsigned

Please sign this Telemetry Coverage system add-on update. Thanks!

Comment 35

[Robert Helmer [:rhelmer]]

Comment on attachment 9008108 [details]
System add-on update for telemetry coverage (v3.0), unsigned

Sorry going to re-attach a new version momentarily.

Comment 36

[Robert Helmer [:rhelmer]]

Attachment: System add-on update for telemetry coverage (v4.0), unsigned

Please sign this Telemetry Coverage SAO update. Thank you!

Comment 37

[:wezhou]

Attachment: signed.9008113.xpi

Signed file attached. Please test.

Comment 38

[Julien Cristau [:jcristau]]

Rehan can you set this up on release-sysaddon?

Comment 39

[Rehan Dalal (Inactive)]

This is on the test channel (release-sysaddon) for 61.* and 62.* and pending sign off on release.

Comment 40

[Robert Helmer [:rhelmer]]

We went through one round of testing on this and have made some minor changes in the meantime, should we do another? Can you arrange this or do I file a new PI request or? :) Thanks!

Comment 41

[Robert Helmer [:rhelmer]]

(In reply to Robert Helmer [:rhelmer] from comment #40)
> We went through one round of testing on this and have made some minor
> changes in the meantime, should we do another? Can you arrange this or do I
> file a new PI request or? :) Thanks!

Hani, could you please test this again? Latest version (4.0) is on the release-sysaddon channel and signed for Release, thanks!

Comment 42

[Hani Yacoub, Desktop QA]

I tested on Firefox 62.0 and Firefox 61.0.2 and changed the update channel to "release-sysaddon" and the results are: 
"Telemetry coverage" is displayed in Firefox features in "about:support", and in the Network tab it shows a connection to "https://telemetry-coverage.mozilla.org/submit/coverage/coverage/1".

I tested on Firefox 62.0 and Firefox 61.0.2 WITHOUT changing the update channel and the results are:
"Telemetry coverage" isn't displayed in Firefox features, and in the Network tab it shows a connection different than the one displayed in Firefox 62.


Is this expected?

Comment 43

[Julien Cristau [:jcristau]]

That sounds about right to me, though I don't know what you mean by "it shows a connection different than the one displayed in Firefox 62".

Did you also check the contents of the telemetry-coverage ping when data collection is turned on/off?

Comment 44

[Hani Yacoub, Desktop QA]

I meant that it doesn't display a connection to "https://telemetry-coverage.mozilla.org/submit/coverage/coverage/1".

I didn't check that, I wasn't even sure how to do that until I asked a colleague who have experience working with telemetry.

From "about:preferences" in "Firefox Data Collection and Use" section, I left the first box checked and then I verified in about:telemetry to check the contents but I didn't see any of the params mentioned in comment 1. (Please see screenshot Data On.png)

Then I tried on a new profile, I unchecked the first box from "Firefox Data Collection and Use" section, and the results were as before, "Telemetry coverage" is displayed in Firefox features in "about:support", and in the Network tab it shows a connection to "https://telemetry-coverage.mozilla.org/submit/coverage/coverage/1".
In about:telemetry I didn't see any of the params mentioned before. (Please see screenshot Data Off.png)

I hope that what I just did is the same thing you were referring to.
Regards.

Comment 45

[Julien Cristau [:jcristau]]

(In reply to Hani Yacoub from comment #44)
> I meant that it doesn't display a connection to
> "https://telemetry-coverage.mozilla.org/submit/coverage/coverage/1".
> 
Right, that connection is added by the telemetry-coverage addon, so it's not expected to occur without it.

> I didn't check that, I wasn't even sure how to do that until I asked a
> colleague who have experience working with telemetry.
> 
> From "about:preferences" in "Firefox Data Collection and Use" section, I
> left the first box checked and then I verified in about:telemetry to check
> the contents but I didn't see any of the params mentioned in comment 1.
> (Please see screenshot Data On.png)
> 
> Then I tried on a new profile, I unchecked the first box from "Firefox Data
> Collection and Use" section, and the results were as before, "Telemetry
> coverage" is displayed in Firefox features in "about:support", and in the
> Network tab it shows a connection to
> "https://telemetry-coverage.mozilla.org/submit/coverage/coverage/1".
> In about:telemetry I didn't see any of the params mentioned before. (Please
> see screenshot Data Off.png)
> 
OK, nevermind me, it looks like the contents of the ping is not visible in the browser console so it's not so easy to check.  I tested 61.0.2 with telemetry on and off, with/without the opt-out pref, with/without the doctored telemetry clientID, and am satisfied things work as expected.

Comment 46

[Julien Cristau [:jcristau]]

Actually, scratch that.  The browser toolbox shows the request body:

-----------------------------30901277611907454981161232464
Content-Disposition: form-data; name="telemetry_enabled"

[object Object]
-----------------------------30901277611907454981161232464--

Robert, it looks like this is missing a call to JSON.stringify when adding the payload to the formdata?

Comment 47

[Robert Helmer [:rhelmer]]

Attachment: System add-on update for telemetry coverage (v5.0), unsigned

Please sign this SAO update (5.0), thanks!

Comment 48

[:wezhou]

Attachment: signed.9008786.xpi

Signed file attached. Please test.

Comment 49

[Robert Helmer [:rhelmer]]

Comment on attachment 9008830 [details]
signed.9008786.xpi

Rehan, could you please put this one (v5) up on the release-sysaddon channel? Thanks!

Comment 50

[Rehan Dalal (Inactive)]

This is now on the test channel (release-sysaddon) for 61.* and 62.*

It is pending signoff for 61.* and 62.* on release.

Comment 51

[Stefan [:StefanG_QA]]

We've managed to conduct a test run on the release-sysaddon channel. 

We successfully installed the addon and verified a ping to  https://telemetry-coverage.mozilla.org with the following content:

-----------------------------2176630147023
Content-Disposition: form-data; name="telemetry_enabled"

{"appVersion":"62.0","appUpdateChannel":"release-sysaddon","osName":"WINNT","osVersion":"10.0","telemetryEnabled":1}
-----------------------------2176630147023--


Once the pref "toolkit.telemetry.coverage.opt-out" is created and set to true, no ping is sent to "toolkit.telemetry.coverage.opt-out".

Let me know if you have any questions.

Comment 52

[Wesley Dawson [:whd]]

(In reply to Stefan [:StefanG_QA] from comment #51)
> Let me know if you have any questions.

I don't see these pings hitting our downstream processing systems. I'm investigating why this is. A cursory inspection suggests that the pings are being sent with an incorrect URL path:

/submit/coverage/coverage/1

when there should be a UUID in there e.g. 

/submit/coverage/coverage/1/245be947-c848-4205-8deb-e7382f4890a8

Comment 53

[Robert Helmer [:rhelmer]]

(In reply to Wesley Dawson [:whd] from comment #52)
> (In reply to Stefan [:StefanG_QA] from comment #51)
> > Let me know if you have any questions.
> 
> I don't see these pings hitting our downstream processing systems. I'm
> investigating why this is. A cursory inspection suggests that the pings are
> being sent with an incorrect URL path:
> 
> /submit/coverage/coverage/1
> 
> when there should be a UUID in there e.g. 
> 
> /submit/coverage/coverage/1/245be947-c848-4205-8deb-e7382f4890a8

After discussion we decided not to do a UUID, as this is one-shot and no retry mechanism etc.

Mark, did I get that right^?

Comment 54

[Robert Helmer [:rhelmer]]

(To clarify we'd like to add this in the future but we wanted to simplify this to get it going a little sooner)

Comment 55

[Wesley Dawson [:whd]]

(In reply to Robert Helmer [:rhelmer] from comment #53)
> (In reply to Wesley Dawson [:whd] from comment #52)
> 
> After discussion we decided not to do a UUID, as this is one-shot and no
> retry mechanism etc.
> 
> Mark, did I get that right^?

There may have been a miscommunication because we definitely need a unique document ID in the URL path. Our ingestion systems consider a submission without one to be invalid.

Comment 56

[Mark Reid [:mreid]]

(In reply to Wesley Dawson [:whd] from comment #55)
> (In reply to Robert Helmer [:rhelmer] from comment #53)
> > (In reply to Wesley Dawson [:whd] from comment #52)
> > 
> > After discussion we decided not to do a UUID, as this is one-shot and no
> > retry mechanism etc.
> > 
> > Mark, did I get that right^?
> 
> There may have been a miscommunication because we definitely need a unique
> document ID in the URL path. Our ingestion systems consider a submission
> without one to be invalid.

Sorry, I thought the document ID was optional. Rob, can we still add that?

Comment 57

[Robert Helmer [:rhelmer]]

(In reply to Mark Reid [:mreid] from comment #56)
> (In reply to Wesley Dawson [:whd] from comment #55)
> > (In reply to Robert Helmer [:rhelmer] from comment #53)
> > > (In reply to Wesley Dawson [:whd] from comment #52)
> > > 
> > > After discussion we decided not to do a UUID, as this is one-shot and no
> > > retry mechanism etc.
> > > 
> > > Mark, did I get that right^?
> > 
> > There may have been a miscommunication because we definitely need a unique
> > document ID in the URL path. Our ingestion systems consider a submission
> > without one to be invalid.
> 
> Sorry, I thought the document ID was optional. Rob, can we still add that?

We can, will need to go through another signing+staging round and possible QA, I'll get that started.

Comment 58

[Robert Helmer [:rhelmer]]

Would you mind reviewing https://github.com/mozilla/one-off-system-add-ons/pull/123 and making sure the submission bits look ok?

Specifically this const at the top:

  const REPORTING_ENDPOINT = "https://telemetry-coverage.mozilla.org/submit/coverage/1";

Which gets a unique UUID for each submission that looks like:

  let endpoint = `${REPORTING_ENDPOINT}/${uuid}`;

And the payload which is:

  const payload = {
    "appVersion": Services.appinfo.version,
    "appUpdateChannel": UpdateUtils.getUpdateChannel(false),
    "osName": Services.appinfo.OS,
    "osVersion": Services.sysinfo.getProperty("version"),
    "telemetryEnabled": enabled | 0
  };


Thanks!

Comment 59

[Robert Helmer [:rhelmer]]

Attachment: System add-on update for telemetry coverage (v6.0), unsigned

Could you please sign this (v6) of the Telemetry Coverage SAO update? Thanks!

Comment 60

[Wesley Dawson [:whd]]

(In reply to Robert Helmer [:rhelmer] from comment #58)
> Would you mind reviewing
> https://github.com/mozilla/one-off-system-add-ons/pull/123 and making sure
> the submission bits look ok?
> 
> Specifically this const at the top:
> 
>   const REPORTING_ENDPOINT =
> "https://telemetry-coverage.mozilla.org/submit/coverage/1";
> 
> Which gets a unique UUID for each submission that looks like:
> 
>   let endpoint = `${REPORTING_ENDPOINT}/${uuid}`;
> 
> And the payload which is:
> 
>   const payload = {
>     "appVersion": Services.appinfo.version,
>     "appUpdateChannel": UpdateUtils.getUpdateChannel(false),
>     "osName": Services.appinfo.OS,
>     "osVersion": Services.sysinfo.getProperty("version"),
>     "telemetryEnabled": enabled | 0
>   };
> 
> 
> Thanks!

This looks good to me.

Comment 61

[Hani Yacoub, Desktop QA]

Attachment: headers.png

I tested with the unsigned version of "System add-on update for telemetry coverage (v6.0)" on the latest nightly and the results are: 

- I can confirm that the pings are being sent with an correct URL path including UUID. (ex: https://telemetry-coverage.mozilla.org/submit/coverage/1/e2bd8234-644d-4992-8689-db7c77e80a74) Please see screenshot (heards.png).

- The payload is:
-----------------------------25441615019225
Content-Disposition: form-data; name="telemetry_enabled"

{"appVersion":"64.0a1","appUpdateChannel":"nightly","osName":"WINNT","osVersion":"10.0","telemetryEnabled":1}
-----------------------------25441615019225--

Comment 62

[Robert Helmer [:rhelmer]]

(In reply to Hani Yacoub from comment #61)
> Created attachment 9009114 [details]
> headers.png
> 
> I tested with the unsigned version of "System add-on update for telemetry
> coverage (v6.0)" on the latest nightly and the results are: 
> 
> - I can confirm that the pings are being sent with an correct URL path
> including UUID. (ex:
> https://telemetry-coverage.mozilla.org/submit/coverage/1/e2bd8234-644d-4992-
> 8689-db7c77e80a74) Please see screenshot (heards.png).
> 
> - The payload is:
> -----------------------------25441615019225
> Content-Disposition: form-data; name="telemetry_enabled"
> 
> {"appVersion":"64.0a1","appUpdateChannel":"nightly","osName":"WINNT",
> "osVersion":"10.0","telemetryEnabled":1}
> -----------------------------25441615019225--

Wesley, could you please confirm if you saw this come into Telemetry? You should have seen one from me too from last night :)

Thanks!

Comment 63

[Wesley Dawson [:whd]]

(In reply to Robert Helmer [:rhelmer] from comment #62)
> (In reply to Hani Yacoub from comment #61)

> Wesley, could you please confirm if you saw this come into Telemetry? You
> should have seen one from me too from last night :)
> 
> Thanks!

I should have checked this a little more closely. I remember skimming the PR and saw some notes about the value of telemetryEnabled being boolean (https://github.com/mozilla/one-off-system-add-ons/pull/123/files/df0d5e73804e198254cc19ad8c3ec70ef594ffca) but the code actually does the opposite, and we've got it the other way around in our schemas https://github.com/mozilla-services/mozilla-pipeline-schemas/pull/198/files.

However, this is trivial to fix pipeline-side so in the interest of time we should probably update our schema. I'm discussing this with :mreid.

Comment 64

[Wesley Dawson [:whd]]

https://github.com/mozilla-services/mozilla-pipeline-schemas/pull/202/files has been deployed, so we should be good to go. :rhelmer can you send another ping to confirm?

Comment 65

[Robert Helmer [:rhelmer]]

(In reply to Wesley Dawson [:whd] from comment #64)
> https://github.com/mozilla-services/mozilla-pipeline-schemas/pull/202/files
> has been deployed, so we should be good to go. :rhelmer can you send another
> ping to confirm?

Done, posted to endpoint: https://telemetry-coverage.mozilla.org/submit/coverage/1/5be7750c-86e6-9f41-8dd5-561583f2d7c7

With payload:

{ appVersion: "64.0a1",
  appUpdateChannel: "nightly",
  osName: "Darwin",
  osVersion: "17.7.0",
  telemetryEnabled: 1
}

Comment 66

[Wesley Dawson [:whd]]

(In reply to Robert Helmer [:rhelmer] from comment #65)
> (In reply to Wesley Dawson [:whd] from comment #64)
> > https://github.com/mozilla-services/mozilla-pipeline-schemas/pull/202/files
> > has been deployed, so we should be good to go. :rhelmer can you send another
> > ping to confirm?
> 
> Done, posted to endpoint:
> https://telemetry-coverage.mozilla.org/submit/coverage/1/5be7750c-86e6-9f41-
> 8dd5-561583f2d7c7
> 
> With payload:
> 
> { appVersion: "64.0a1",
>   appUpdateChannel: "nightly",
>   osName: "Darwin",
>   osVersion: "17.7.0",
>   telemetryEnabled: 1
> }

It turns out I've missed another thing in comment #62. We have should have both a namespace and doctype of coverage per https://bugzilla.mozilla.org/show_bug.cgi?id=1480194#c20, so the reporting endpoint should be:

const REPORTING_ENDPOINT = "https://telemetry-coverage.mozilla.org/submit/coverage/coverage/1";

Comment 67

[Robert Helmer [:rhelmer]]

(In reply to Wesley Dawson [:whd] from comment #66)
> (In reply to Robert Helmer [:rhelmer] from comment #65)
> > (In reply to Wesley Dawson [:whd] from comment #64)
> > > https://github.com/mozilla-services/mozilla-pipeline-schemas/pull/202/files
> > > has been deployed, so we should be good to go. :rhelmer can you send another
> > > ping to confirm?
> > 
> > Done, posted to endpoint:
> > https://telemetry-coverage.mozilla.org/submit/coverage/1/5be7750c-86e6-9f41-
> > 8dd5-561583f2d7c7
> > 
> > With payload:
> > 
> > { appVersion: "64.0a1",
> >   appUpdateChannel: "nightly",
> >   osName: "Darwin",
> >   osVersion: "17.7.0",
> >   telemetryEnabled: 1
> > }
> 
> It turns out I've missed another thing in comment #62. We have should have
> both a namespace and doctype of coverage per
> https://bugzilla.mozilla.org/show_bug.cgi?id=1480194#c20, so the reporting
> endpoint should be:
> 
> const REPORTING_ENDPOINT =
> "https://telemetry-coverage.mozilla.org/submit/coverage/coverage/1";

OK just sent another test, same payload w/ this endpoint:

https://telemetry-coverage.mozilla.org/submit/coverage/coverage/1/dd43518f-9a50-684b-bc40-ab6ef878238e

Comment 68

[Robert Helmer [:rhelmer]]

Attachment: System add-on update for telemetry coverage (v7.0), unsigned

:whd and I confirmed that this works and makes it through the pipeline:

https://pipeline-cep.prod.mozaws.net/dashboard_output/analysis.moz_generic_coverage_coverage_1_pings.submissions.json

Please sign this SAO update, thanks!

Comment 69

[:wezhou]

Attachment: signed.9009228.xpi

Signed file attached. Please test.

Comment 70

[Robert Helmer [:rhelmer]]

(In reply to :wezhou from comment #69)
> Created attachment 9009328 [details]
> signed.9009228.xpi
> 
> Signed file attached. Please test.

Please stage this on the release-sysaddon channel - thanks!

Comment 71

[Rehan Dalal (Inactive)]

This is on the test channel (release-sysaddon) and pending sign off on release.

Comment 72

[Robert Helmer [:rhelmer]]

The only changes since the last round of QA have been to change the way we deliver the payload and the endpoint URL.

Are we ready to ship this? Thanks!

Comment 73

[Robert Helmer [:rhelmer]]

(In reply to Robert Helmer [:rhelmer] from comment #72)
> The only changes since the last round of QA have been to change the way we
> deliver the payload and the endpoint URL.
> 
> Are we ready to ship this? Thanks!

Here's the diff comparing v5 and v7 from http://archive.mozilla.org/pub/system-addons/telemetry-coverage-bug1487578/:

diff -u -r t5/bootstrap.js t7/bootstrap.js
--- t5/bootstrap.js	2018-09-13 09:04:17.000000000 -0700
+++ t7/bootstrap.js	2018-09-14 12:09:42.000000000 -0700
@@ -48,14 +48,17 @@
     "telemetryEnabled": enabled | 0
   };
 
-  let formData = new FormData();
-  formData.append("telemetry_enabled", JSON.stringify(payload));
+  let uuidGenerator = Cc["@mozilla.org/uuid-generator;1"].getService(Ci.nsIUUIDGenerator);
+  // generateUUID() gives a UUID surrounded by {...}, slice them off.
+  let uuid = uuidGenerator.generateUUID().toString().slice(1, -1);
 
-  debug(`posting to endpoint ${REPORTING_ENDPOINT} with payload:`, payload);
+  let endpoint = `${REPORTING_ENDPOINT}/${uuid}`;
 
-  await fetch(REPORTING_ENDPOINT, {
+  debug(`posting to endpoint ${endpoint} with payload:`, payload);
+
+  await fetch(endpoint, {
     method: "PUT",
-    body: formData
+    body: JSON.stringify(payload),
   });
 }
 
diff -u -r t5/install.rdf t7/install.rdf
--- t5/install.rdf	2018-09-13 09:04:24.000000000 -0700
+++ t7/install.rdf	2018-09-14 12:10:59.000000000 -0700
@@ -2,7 +2,7 @@
 <RDF xmlns="http://www.w3.org/1999/02/22-rdf-syntax-ns#" xmlns:em="http://www.mozilla.org/2004/em-rdf#">
   <Description about="urn:mozilla:install-manifest">
     <em:id>[e-mail]">[e-mail]</em:id>
-    <em:version>5.0</em:version>
+    <em:version>7.0</em:version>
     <em:type>2</em:type>
     <em:bootstrap>true</em:bootstrap>
     <em:unpack>false</em:unpack>

Comment 74

[Stefan [:StefanG_QA]]

We've managed to conduct another test run on the release-sysaddon channel using Fx62. 

Once the addon is installed, we verified a ping to https://telemetry-coverage.mozilla.org with the following content:

appUpdateChannel	release-sysaddon
appVersion	62.0
osName	WINNT
osVersion	6.1
telemetryEnabled	1


In addition if the pref "toolkit.telemetry.coverage.opt-out" is created and set to true, no ping is sent to "toolkit.telemetry.coverage.opt-out".

Comment 75

[Robert Helmer [:rhelmer]]

(In reply to Stefan [:StefanG_QA] from comment #74)
> We've managed to conduct another test run on the release-sysaddon channel
> using Fx62. 
> 
> Once the addon is installed, we verified a ping to
> https://telemetry-coverage.mozilla.org with the following content:
> 
> appUpdateChannel	release-sysaddon
> appVersion	62.0
> osName	WINNT
> osVersion	6.1
> telemetryEnabled	1
> 
> 
> In addition if the pref "toolkit.telemetry.coverage.opt-out" is created and
> set to true, no ping is sent to "toolkit.telemetry.coverage.opt-out".

Thanks! These pings made it through the Telemetry pipeline too:

https://pipeline-cep.prod.mozaws.net/dashboard_output/analysis.moz_generic_coverage_coverage_1_pings.submissions.json

Julien/Ryan, this is awaiting sign-off in Balrog.

Comment 76

[Julien Cristau [:jcristau]]

I checked that a ping from a client with telemetry disabled does show up with "telemetryEnabled: 0" at the other end (all other pings look like they had telemetry on).

Slight concern that osVersion at least on Linux gives more information than we really need, but I've now signed off on the change for 61.* and 62.*, so this is live.

Comment 77

[Robert Helmer [:rhelmer]]

(In reply to Julien Cristau [:jcristau] from comment #76)
> I checked that a ping from a client with telemetry disabled does show up
> with "telemetryEnabled: 0" at the other end (all other pings look like they
> had telemetry on).
> 
> Slight concern that osVersion at least on Linux gives more information than
> we really need, but I've now signed off on the change for 61.* and 62.*, so
> this is live.

Thanks! It does seem that Services.sysinfo.getProperty("version") using the kernel version isn't quite consistent with what is reported on Windows/macOS (major OS version only). I'll see if we have anything more generic, something like distro + version would probably be more useful for tracking down problems than the kernel version anyway (although from a quick look I'm not sure if Firefox gathers that info).

Comment 78

[Robert Helmer [:rhelmer]]

(In reply to Robert Helmer [:rhelmer] from comment #77)
> (In reply to Julien Cristau [:jcristau] from comment #76)
> > I checked that a ping from a client with telemetry disabled does show up
> > with "telemetryEnabled: 0" at the other end (all other pings look like they
> > had telemetry on).
> > 
> > Slight concern that osVersion at least on Linux gives more information than
> > we really need, but I've now signed off on the change for 61.* and 62.*, so
> > this is live.
> 
> Thanks! It does seem that Services.sysinfo.getProperty("version") using the
> kernel version isn't quite consistent with what is reported on Windows/macOS
> (major OS version only). I'll see if we have anything more generic,
> something like distro + version would probably be more useful for tracking
> down problems than the kernel version anyway (although from a quick look I'm
> not sure if Firefox gathers that info).

Actually I take that back - looks like this is reporting kernel version which is at least consistent, and on Windows/macOS lets us map that to an OS version at least.

Comment 79

[Robert Helmer [:rhelmer]]

I see data coming in that looks reasonable, I think we're done for now. Thanks everyone!