1675310 - telemetry::RecordEvent cannot be called during startup off the main thread

Status: RESOLVED FIXED

(Toolkit :: Telemetry, defect, P1)

Description

[Simon Giesecke [:sg] [he/him]]

telemetry::RecordEvent cannot be safely called during early startup off the main thread, because then it might call Preferences::GetBool off the main thread to initialize its local statics.

We hit this when trying to add telemetry recording during early startup in Bug 1665088.

Comment 1

[Chris H-C :chutten]

Do you mean this one? If so, we can probably get rid of it. It was an off switch to move back to sending events in the "main" ping in case the "event" ping didn't work out.

Comment 2

[Simon Giesecke [:sg] [he/him]]

Yes, that one and https://searchfox.org/mozilla-central/rev/c938c7416c633639a5c8ce4412be586eefb48005/toolkit/components/telemetry/core/TelemetryEvent.cpp#483 a few lines below.

Comment 3

[Chris H-C :chutten]

The default value has proven acceptable these past years, so we can probably de-pref it too.

Comment 4

[Chris H-C :chutten]

Attachment: Bug 1675310 - Remove enabled and limit prefs for the event ping r?janerik!

Recording events involved asking whether they should go in the event ping or
not, and also whether we hit the limit for how many are allowed in a ping.
These values are stored in prefs which aren't terribly accessible off the main
thread, even if the event recording is.

Both of these prefs have been set in stone in their default values for over two
years now, so we can safely remove them and their scaffolding. Huzzah!

Comment 5

[Simon Giesecke [:sg] [he/him]]

Sounds good. Thanks for taking care of this so quickly!

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d1443dd277ee
Remove enabled and limit prefs for the event ping r=janerik

Comment 7

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/d1443dd277ee

Comment 8

[Simon Giesecke [:sg] [he/him]]

Unfortunately, I just noticed there's another preference: Preferences::GetUint("toolkit.telemetry.maxEventSummaryKeys", 500); Can this be eliminated as well?

Comment 9

[Chris H-C :chutten]

It sure can. Let's give it its own bug (bug 1676404) so I don't confuse the tools by adding new patches to this one.