1429510 - Make last shutdown time a scalar

Status: RESOLVED FIXED

(Toolkit :: Telemetry, enhancement, P1)

Description

[Georg Fritzsche [:gfritzsche]]

We have `shutdownDuration` in simpleMeasurements, but scalars have better tooling.
Duplicate the measurements into scalars.

Comment 1

[Georg Fritzsche [:gfritzsche]]

Mike, would you happen to know who cares about shutdown times these days?
I'm looking for some person or team to put in as an owner / alert recipient.

Comment 2

[Georg Fritzsche [:gfritzsche]]

Attachment: Make last shutdown time a scalar

Comment 3

[Chris H-C :chutten]

Comment on attachment 8941534 [details] [diff] [review]
Make last shutdown time a scalar

Review of attachment 8941534 [details] [diff] [review]:
-----------------------------------------------------------------

One bikeshed request.

::: toolkit/components/telemetry/Scalars.yaml
@@ +1329,5 @@
>      release_channel_collection: opt-out
>      record_in_processes:
>        - main
>  
> +timings:

What kind of timings? Maybe app.timings? fx.timings? (bikeshed, bikeshed, ...)

Comment 4

[Mike Conley (:mconley) (:⚙️)]

Hey florian, does your interest in startup come with an interest in shutdown?

Comment 5

[Florian Quèze [:florian]]

(In reply to Mike Conley (:mconley) (:⚙️) - Backlogged on reviews and needinfos from comment #4)
> Hey florian, does your interest in startup come with an interest in shutdown?

Sure (although it's currently more a passive interest than an active one, as I expect to still be busy with startup for at least Q1, and profiling shutdown is still pretty difficult).

Comment 6

[Georg Fritzsche [:gfritzsche]]

Cheers, i'll put you there for now, we can always change it in the future as needed.

Comment 7

[Georg Fritzsche [:gfritzsche]]

Attachment: Make last shutdown time a scalar

Comment 8

[Georg Fritzsche [:gfritzsche]]

Comment on attachment 8941818 [details] [diff] [review]
Make last shutdown time a scalar

Requesting data review.

This is a duplication of an existing measurement, simpleMeasurements.shutdownDuration [1] in the main ping.
This is duplicated as part of our effort to move more data points into standard datatypes, for better tooling support.

As the existing measurement, this is opt-out and collected from the full Firefox population.
This is a core metric that tracks how long Firefox shutdown takes.

1: https://firefox-source-docs.mozilla.org/toolkit/components/telemetry/telemetry/data/main-ping.html#shutdownduration

Comment 9

[Chenxia Liu [:liuche]]

Comment on attachment 8941818 [details] [diff] [review]
Make last shutdown time a scalar

Review of attachment 8941818 [details] [diff] [review]:
-----------------------------------------------------------------

Data-review only.

This is a duplicate of the existing simpleMeasurements probe `shutdownDuration`. I'm having trouble finding the original expiration 

> Is there or will there be documentation that describes the schema for the ultimate data set available publicly, complete and accurate? (see here, here, and here for examples). Refer to the appendix for "documentation" if more detail about documentation standards is needed.

Yes, Scalars.yaml and firefox-source-docs

> Is there a control mechanism that allows the user to turn the data collection on and off? (Note, for data collection not needed for security purposes, Mozilla provides such a control mechanism) Provide details as to the control mechanism available.

Yes, telemetry toggle

> If the request is for permanent data collection, is there someone who will monitor the data over time?**
[e-mail]

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

> 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/No) (If yes, set a todo reminder or file a bug if appropriate)**
This is part of mainping and is used to measure Firefox performance and health - does not need a checkin to be renewed.

Comment 10

[Chenxia Liu [:liuche]]

Georg, will you still need the shutdownDuration in simple measurements? If not, can you file a bug to remove it now that it is duplicated in scalars?

Comment 11

[Georg Fritzsche [:gfritzsche]]

Thanks Chenxia.

(In reply to Chenxia Liu [:liuche] - not actively working on Fennec from comment #9)
> This is a duplicate of the existing simpleMeasurements probe
> `shutdownDuration`. I'm having trouble finding the original expiration 

Effectively this has an expiry of "never".
It is part of the custom simpleMeasurements JSON blob in the main ping, so it doesn't have an explicitly defined expiry.

> Georg, will you still need the shutdownDuration in simple measurements? If
> not, can you file a bug to remove it now that it is duplicated in scalars?

We are tracking the various "old" measurements in a bigger sheet:
https://docs.google.com/spreadsheets/d/1y5gbtHibpEcZMObKDt1AXSXM4wLtw4vwXkN10OKV7H0/

Given the amount of measurements we want to cleanup/migrate (all of simpleMeasurements, others), it will be more efficient to track it there for now and do removal of many in one bug.
The actual removal is still blocked on migrating existing ETL jobs and giving removal notice to consumers.
We will need to track this better as a project, i think we will get there in Q2 or Q3.

Comment 12

[Georg Fritzsche [:gfritzsche]]

Alessio, given what you pointed out in the main ping review doc, would this actually record proper data on opt-out Telemetry?

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/809a14e1c12b
Make last shutdown time a scalar. r=chutten,liuche

Comment 14

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/db7225f74e1f
Backed out 1 changesets for build bustage on CLOSED TREE

Comment 15

[Tiberius Oros[:tiberius_oros]]

Backed out 1 changesets (bug 1429510)for build bustage on CLOSED TREE. Aryx tried to fix bitrotting.

https://hg.mozilla.org/integration/mozilla-inbound/rev/db7225f74e1f33a86616795aa0bdd110a8de9f8b

https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=5d64b00f5795af39f9cd9bbdcda47c89d3d80b0f

Comment 16

[Alessio Placitelli [:Dexter]]

(In reply to Georg Fritzsche [:gfritzsche] from comment #12)
> Alessio, given what you pointed out in the main ping review doc, would this
> actually record proper data on opt-out Telemetry?

This would only record data if |canRecordExtended| is true, so on pre-release channels only.

This is because:

- asyncFetchTelemetryData [1] gets called on Telemetry delayed init;
- but bails out in its implementation if |CanRecordExtended| [2] is false;
- if that happens, the nsFetchTelemetryData runnable doesn't get dispatched [3], the data doesn't get loaded and the scalar doesn't get set.


[1] - https://searchfox.org/mozilla-central/rev/137f1b2f434346a0c3756ebfcbdbee4069e15dc8/toolkit/components/telemetry/TelemetrySession.jsm#1512
[2] - https://searchfox.org/mozilla-central/rev/137f1b2f434346a0c3756ebfcbdbee4069e15dc8/toolkit/components/telemetry/Telemetry.cpp#427-431,471
[3] - https://searchfox.org/mozilla-central/rev/137f1b2f434346a0c3756ebfcbdbee4069e15dc8/toolkit/components/telemetry/Telemetry.cpp#470-476

Comment 17

[Georg Fritzsche [:gfritzsche]]

Attachment: Make last shutdown time a scalar

Ok, thanks. Making this properly opt-out could have potential startup impact, so i am just marking the probe opt-in to make the semantics clear.

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6f2cf6d0e15a
Make last shutdown time a scalar. r=chutten

Comment 19

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/6f2cf6d0e15a