Closed Bug 1757229 Opened 11 months ago Closed 11 months ago

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

Categories

(Toolkit :: Application Update, defect, P1)

defect

Tracking

()

RESOLVED FIXED
100 Branch
Tracking Status
firefox100 --- fixed

People

(Reporter: nalexander, Assigned: nalexander)

References

(Blocks 1 open bug)

Details

(Whiteboard: [fidedi-ope])

Attachments

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

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
Status: NEW → ASSIGNED
Blocks: 1759901

Jumping on this to fix regression in Bug 1759901.

Assignee: nrishel → nalexander
Pushed by nalexander@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9231db3feeaf
Ensure `Telemetry::CanRecordExtended()` is `false` in background tasks. r=janerik
Status: ASSIGNED → RESOLVED
Closed: 11 months ago
Resolution: --- → FIXED
Target Milestone: --- → 100 Branch
You need to log in before you can comment on or make changes to this bug.