1757229 - Ensure `Telemetry::CanRecordExtended()` is `false` in background tasks

Status: RESOLVED FIXED

(Toolkit :: Application Update, defect, P1)

Description

[Nick Alexander :nalexander [he/him]]

While investigating Bug 1722777, I noticed late writes to ShutdownDuration.json. Those writes race with deleting the temporary profile that background tasks try to delete as they shutdown. The writing mechanism is already guarded by Telemetry::CanRecordExtended(). Since background tasks will never submit regular Firefox Telemetry (tasks may use Glean, like the background update task), we should turn off this Telemetry recording. Maybe we should turn off extended and base?

chutten: can you see any likely issue with this? Can you propose a way to do this early on? Background tasks don't initialize Telemetry (there's no backgroundtasks entry for https://searchfox.org/mozilla-central/rev/9d66919569655a8fae9b431550241c058fa85b8a/toolkit/components/telemetry/components.conf#8-13). I think we can probably invoke nsITelemetry::SetCanRecordBase more as less as soon as we have XPCOM and know that we're doing background task work. But that might not be soon enough for all consumers, so perhaps we should make the CanRecordBase logic recognize if we're in background task mode?

Comment 1

[Chris H-C :chutten]

Hrm. That's a thinker.

Turning off CanRecordedExtended shouldn't be necessary if the channel is anything other than nightly, beta, or aurora (or the other pieces of technicality)... oh right, we don't run TelemetryController. So it gets the default value from CreateInstance which is almost always true.

In the case of the background task runtime JSMs you could "just" Services.telemetry.canRecordBase = Services.telemetry.canRecordExtended = false;. But if we want to make this a little cleverer, then somewhere around here is where we'll want to figure out if we're a background task, and if so, set useTelemetry to false.

Ramifications-wise, this operating mode should work just peachily (it's the mode the fuzzing builds work in, so there's coverage of a sort). There's a risk something else might turn these back on for you, but it's not likely (the only references I see about these are in Telemetry JSMs which we're explicitly not loading). I think you should be in the clear.

Comment 2

[Nick Alexander :nalexander [he/him]]

nrishel will pick this up shortly.

Comment 3

[Nick Alexander :nalexander [he/him]]

Attachment: Bug 1757229 - Ensure `Telemetry::CanRecordExtended()` is `false` in background tasks. r?janerik

Comment 4

[Nick Alexander :nalexander [he/him]]

Jumping on this to fix regression in Bug 1759901.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9231db3feeaf
Ensure `Telemetry::CanRecordExtended()` is `false` in background tasks. r=janerik

Comment 6

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/9231db3feeaf