Closed Bug 1757229 Opened 2 years ago Closed 2 years ago

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


(Toolkit :: Application Update, defect, P1)




100 Branch
Tracking Status
firefox100 --- fixed


(Reporter: nalexander, Assigned: nalexander)



(Whiteboard: [fidedi-ope])


(1 file)

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 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?

Flags: needinfo?(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.

Flags: needinfo?(chutten)

nrishel will pick this up shortly.

Assignee: nobody → nrishel
Blocks: 1759901

Jumping on this to fix regression in Bug 1759901.

Assignee: nrishel → nalexander
Pushed by
Ensure `Telemetry::CanRecordExtended()` is `false` in background tasks. r=janerik
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 100 Branch
You need to log in before you can comment on or make changes to this bug.