1746983 - Re-enable pingsender2

Status: RESOLVED FIXED

(Toolkit :: Telemetry, task)

Description

[Bobby Holley (:bholley)]

We landed a Gecko-based replacement for pingsender in bug 1734262, and then turned it off due to updater breakage. We believe this breakage to be understood and fixed by bug 1737117, but Nick wanted to do some thinking/testing before we try flipping it on again.

Once we're ready, we should also verify that the issues in bug 1736796, bug 1736189, and bug 1736524 don't resurface. It seems like at least some of those may have been related to another issue, so we'll have to see. But I'll wait for the green light from Nick to dig into that.

Comment 1

[Bobby Holley (:bholley)]

Nick has next steps on letting us know when we're ready to proceed. No hurry on this one.

Comment 2

[Bobby Holley (:bholley)]

I think it would also be prudent to fix bug 1736910 before shipping this.

Comment 3

[Nick Alexander :nalexander [he/him]]

We're (finally) ready to move on this. I'll plan to land it in the Fx101 cycle.

Comment 4

[Bobby Holley (:bholley)]

Thanks Nick!

Comment 5

[Nick Alexander :nalexander [he/him]]

Attachment: Bug 1746983 - Re-enable pingsender2. r?bhearsum

Many prerequisite tickets have landed, so let's try this again.

Comment 6

[BugBot [:suhaib / :marco/ :calixte]]

There's a r+ patch which didn't land and no activity in this bug for 2 weeks.
:nalexander, could you have a look please?
If you still have some work to do, you can add an action "Plan Changes" in Phabricator.
For more information, please visit auto_nag documentation.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e8751087ba26
Re-enable pingsender2. r=bhearsum,bholley

Comment 8

[bszekely]

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

Comment 9

[Nick Alexander :nalexander [he/him]]

Clearing NI since this has finally landed.

Comment 10

[Nick Alexander :nalexander [he/him]]

Something in pingsender2 is not encoding control characters correcting, leading to spiking error rates due to malformed JSON and dropped Telemetry. Note that this remains Nightly only, so the bad-data window is limited to the last few days and on Nightly only. We'll back this out while I investigate.

Comment 11

[Nick Alexander :nalexander [he/him]]

Relevant query with error rates: https://sql.telemetry.mozilla.org/queries/85929/source.

Comment 12

[Cosmin Sabou [:CosminS]]

Backed out from mozilla-central at Nick's request: https://hg.mozilla.org/mozilla-central/rev/fb6a807eb4ab550f942a672447d560eec950f526

Comment 13

[Nick Alexander :nalexander [he/him]]

Attachment: Bug 1746983 - Pre: New ping server for every test. r?chutten

Re-using a ping server can cause problems when running multiple test
functions in a single test class. This should be safe since a
specific test cannot depend on a specific ping server being maintained
across test invocations.

While here, be careful to start the ping server before launching
Firefox: Firefox might send very early pings that, were we to race
startups, could be dropped.

Comment 14

[Nick Alexander :nalexander [he/him]]

Attachment: Bug 1746983 - Add Nimbus feature (and Gecko preference) enabling pingsender background task. r?chutten

Depends on D163130

Comment 15

[Nick Alexander :nalexander [he/him]]

Try build is percolating at:

https://treeherder.mozilla.org/jobs?repo=try&revision=fba73e190983534f2975038fe1aad886b032c1b8

Comment 16

[Nick Alexander :nalexander [he/him]]

I just remembered there's a helper for running detached correctly, namely: https://searchfox.org/mozilla-central/source/toolkit/components/backgroundtasks/BackgroundTasksRunner.cpp. Kagami, is there access to that API from JS? I don't see it.

Comment 17

[Kagami Rosylight [:saschanaz] (they/them)]

It's in https://phabricator.services.mozilla.com/D160674 which is not landed yet. I can land it if you need it now. Just let me know.

Comment 18

[Nick Alexander :nalexander [he/him]]

(In reply to Kagami :saschanaz from comment #17)

It's in https://phabricator.services.mozilla.com/D160674 which is not landed yet. I can land it if you need it now. Just let me know.

Please do. Thanks!

Comment 19

[Kagami Rosylight [:saschanaz] (they/them)]

Landing queued. BTW, if your intention is to use it on every platform then you should make sure it's only used at shutdown, otherwise it may cause zombie processes on Unix. (Just filed bug 1802559 for this.)

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6b640c6eaab6
Pre: New ping server for every test. r=chutten
https://hg.mozilla.org/integration/autoland/rev/6d023758cb32
Add Nimbus feature (and Gecko preference) enabling pingsender background task. r=chutten

Comment 21

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/6b640c6eaab6
https://hg.mozilla.org/mozilla-central/rev/6d023758cb32