1480826 - Reenable telemetry.os_shutting_down probe

Status: VERIFIED FIXED

(Toolkit :: Telemetry, defect)

Description

[Adam Gashlin (he/him) [:agashlin] (ex-moco)]

For an upcoming study on release we want to be able to test the new Register Application Restart feature (bug 603903). To get good measurements, and to be able to know who to survey, we need a way of knowing what users could possibly see the feature in action. There was a telemetry.os_shutting_down probe available (added in bug 1363345) that recorded whether Firefox was still running when the OS was shutting down, this is the population we want for the test. It was removed in 57 as it was expiring in 58, in bug 1389992.

The plan is to enable this ASAP on Nightly, check that we get telemetry, and uplift to 61.0.2 release on Monday 2018-08-06.

The code change will be minor as the mechanism to prevent sending pings on OS shutdown is still in place, all that is needed is a line to set the scalar.

Comment 1

[Adam Gashlin (he/him) [:agashlin] (ex-moco)]

Attachment: Bug 1480826: Reenable telemetry.os_shutting_down probe

This records whether the shutdown ping was generated during an OS shutdown.

Comment 2

[Phabricator Automation]

Comment on attachment 8997519 [details]
Bug 1480826: Reenable telemetry.os_shutting_down probe

Chris H-C :chutten has approved the revision.

https://phabricator.services.mozilla.com/D2734

Comment 3

[Adam Gashlin (he/him) [:agashlin] (ex-moco)]

Attachment: Request for data collection review

Comment 4

[Chris H-C :chutten]

Comment on attachment 8997548 [details]
Request for data collection review

DATA COLLECTION REVIEW RESPONSE

    Is there or will there be documentation that describes the schema for the ultimate data set available publicly, complete and accurate?

Standard Telemetry mechanisms apply.

    Is there a control mechanism that allows the user to turn the data collection on and off? 

Standard Telemetry mechanisms apply.

    If the request is for permanent data collection, is there someone who will monitor the data over time?**

Not permanent.

    Using the category system of data types on the Mozilla wiki, what collection type of data do the requested measurements fall under? **

Category 2

    Is the data collection request for default-on or default-off?

Default-on

    Does the instrumentation include the addition of any new identifiers (whether anonymous or otherwise; e.g., username, random IDs, etc. See the appendix for more details)?

No.

    Is the data collection covered by the existing Firefox privacy notice? 

Yes.

    Does there need to be a check-in in the future to determine whether to renew the data?

Yes. :agashlin will be responsible for removing or renewing the probe when it is no longer useful.

---
Result: datareview+

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4c130e2f5246
Reenable telemetry.os_shutting_down probe r=chutten

Comment 6

[Adam Gashlin (he/him) [:agashlin] (ex-moco)]

For the record, try on release:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=90e6603ae25b7d78eaf92059c342522172e46293
and mozilla-central:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=0c18d6ef0a22ace475e0a662c182a091b92af6ec

Comment 7

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/4c130e2f5246

Comment 8

[Adam Gashlin (he/him) [:agashlin] (ex-moco)]

Attachment: Patch for uplift to 61

The Nightly patch seems to be returning data: https://mzl.la/2vHy35P
Note that the probe is simply not present if it would be false, and it would only show up here if the client updated, shutdown while running, and restarted, all in the 2 hours before the end of Friday 8/4, so I think that explains the low numbers.

This patch is only modified to apply cleanly to Scalars.yaml.

Approval Request Comment
[Feature/Bug causing the regression]: N/A
[User impact if declined]: The application restart feature, which protects the session in the case of automatic Windows restarts, will be delayed by at least one release
[Is this code covered by automated tests?]: Yes
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: None, but it is needed for measurement of 1480612
[Is the change risky?]: No
[Why is the change risky/not risky?]: Adding a telemetry probe, which was previously present, using the standard mechanisms. Actual in-product code change is a single line setting the scalar value
[String changes made/needed]: None

Comment 9

[Adam Gashlin (he/him) [:agashlin] (ex-moco)]

Comment on attachment 8997519 [details]
Bug 1480826: Reenable telemetry.os_shutting_down probe

Approval Request Comment

see comment 8

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 8997963 [details] [diff] [review]
Patch for uplift to 61

Telemetry probe needed in support of one of our 61.0.2 drivers. Approved for 61.0.2.

Comment 11

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8997519 [details]
Bug 1480826: Reenable telemetry.os_shutting_down probe

Let's uplift this for the beta 15 build, seems like it should be safe since we are just adding back a telemetry probe we had in product before.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/c66300bc349b

Comment 13

[Andreea Pavel [:apavel]]

Backed out for build bustages

Push that started the failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-beta&revision=c66300bc349b1ab7e2c13c0dc71d6d17ed5d4290

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=192318431&repo=mozilla-beta&lineNumber=3366

Backout: https://hg.mozilla.org/releases/mozilla-beta/rev/42fef3b61d6611f8c927b319bef97a7fa46f7010

Comment 14

[Adam Gashlin (he/him) [:agashlin] (ex-moco)]

Sorry about that, but I don't think there's anything wrong with the patch. It looks like when it was applied the two spaces in the first line of the Scalars.yaml got lost. In the patch:

https://d3kxowhw4s8amj.cloudfront.net/file/data/tt6crpqd5k4ftem5qvkb/PHID-FILE-lhdphxigzunu3v2p3aud/D2734.diff
+  os_shutting_down:
+    bug_numbers:
+      - 1480826
...

in the changeset

https://hg.mozilla.org/releases/mozilla-beta/diff/c66300bc349b/toolkit/components/telemetry/Scalars.yaml
+os_shutting_down:
+    bug_numbers:
+      - 1480826
...

Comment 15

[Andreea Pavel [:apavel]]

Grafted the patch, added the spaces to os_shutting_down

https://hg.mozilla.org/releases/mozilla-beta/rev/2545d974faf880a9c4d2c0cf5326c71e8f7127a2

Comment 16

[Andreea Pavel [:apavel]]

Indentation fix: https://hg.mozilla.org/releases/mozilla-beta/rev/7ce198bb7ce027d450af3f69a609896671adfab8

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-release/rev/e6b8ec3c45a4

Comment 18

[Cornel Ionce [:noni] [Hubs QA]]

This seems to be related to the Windows Restart Manager feature.

Abe, could you please take a look over it on 61.0.2?

Comment 19

[Adam Gashlin (he/him) [:agashlin] (ex-moco)]

You can test this by restarting Windows with Firefox running, then check about:telemetry for telemetry.os_shutting_down in the Scalars for the parent process (or just search for telemetry.os_shutting_down across all sections), it should be true. Also, this value should be absent for main pings with reason "shutdown" caused by manually exiting Firefox.

Comment 20

[Abe - QA (:Abe_LV)]

I tested this on Windows 10 x64 with release 61.0.2, beta 62.0b15 and latest nightly. When Windows is restarted with Firefox running, telemetry data shows telemetry.os_shutting_down;true. 

This flag is also not shown in the main ping (having a reason "shutdown") when only Firefox is restarted. 

It is verified as fixed.

One of the test Environments:
----------------------------
Version 	61.0.2
Build ID 	20180807170231
Update Channel 	release
User Agent 	Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:61.0) Gecko/20100101 Firefox/61.0

Comment 21

[Adam Gashlin (he/him) [:agashlin] (ex-moco)]

Attachment: Bug 1480826: Remove expiring telemetry.os_shutting_down probe