Make last shutdown time a scalar

RESOLVED FIXED in Firefox 59

Status

()

enhancement
P1
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: gfritzsche, Assigned: gfritzsche)

Tracking

(Blocks 1 bug)

Trunk
mozilla59
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox59 fixed)

Details

Attachments

(1 attachment, 2 obsolete attachments)

We have `shutdownDuration` in simpleMeasurements, but scalars have better tooling.
Duplicate the measurements into scalars.
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.
Flags: needinfo?(mconley)
Attachment #8941534 - Flags: review?(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, ...)
Attachment #8941534 - Flags: review?(chutten) → review+
Hey florian, does your interest in startup come with an interest in shutdown?
Flags: needinfo?(mconley) → needinfo?(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).
Flags: needinfo?(florian)
Cheers, i'll put you there for now, we can always change it in the future as needed.
Attachment #8941534 - Attachment is obsolete: true
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
Attachment #8941818 - Flags: review?(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?**
florian@mozilla.com

> 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.
Attachment #8941818 - Flags: review?(liuche) → review+
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?
Flags: needinfo?(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.
Flags: needinfo?(gfritzsche)
Keywords: checkin-needed
Alessio, given what you pointed out in the main ping review doc, would this actually record proper data on opt-out Telemetry?
Flags: needinfo?(alessio.placitelli)
Pushed by archaeopteryx@coole-files.de:
https://hg.mozilla.org/integration/mozilla-inbound/rev/809a14e1c12b
Make last shutdown time a scalar. r=chutten,liuche
Keywords: checkin-needed
Backout by toros@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/db7225f74e1f
Backed out 1 changesets for build bustage on CLOSED TREE
(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
Flags: needinfo?(alessio.placitelli)
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.
Attachment #8941818 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/6f2cf6d0e15a
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
You need to log in before you can comment on or make changes to this bug.