Closed Bug 1746983 Opened 1 year ago Closed 2 months ago

Re-enable pingsender2

Categories

(Toolkit :: Telemetry, task)

task

Tracking

()

RESOLVED FIXED
109 Branch
Tracking Status
firefox109 --- fixed

People

(Reporter: bholley, Assigned: nalexander)

References

(Depends on 2 open bugs)

Details

(Whiteboard: [fidedi-ope])

Attachments

(2 files, 1 obsolete file)

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.

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

Flags: needinfo?(nalexander)
Depends on: 1747917
Depends on: 1747918

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

Depends on: 1736910
Depends on: 1753718

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

Assignee: nobody → nalexander
Status: NEW → ASSIGNED
Flags: needinfo?(nalexander)
Whiteboard: [fidedi-ope]

Thanks Nick!

No longer depends on: 1736910
Depends on: 1722777

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

Depends on: 1765628

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.

Flags: needinfo?(nalexander)
Flags: needinfo?(bhearsum)
Pushed by nalexander@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e8751087ba26
Re-enable pingsender2. r=bhearsum,bholley
Status: ASSIGNED → RESOLVED
Closed: 9 months ago
Resolution: --- → FIXED
Target Milestone: --- → 102 Branch

Clearing NI since this has finally landed.

Flags: needinfo?(nalexander)
Flags: needinfo?(bhearsum)
Regressions: 1768961
Regressions: 1769013

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.

Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: 102 Branch → ---
Depends on: 1770473
Depends on: 1772864
Depends on: 1773535
Depends on: 1791675

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.

Attachment #9272236 - Attachment is obsolete: true

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.

Flags: needinfo?(krosylight)

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.

Flags: needinfo?(krosylight)

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

Flags: needinfo?(krosylight)

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.)

Flags: needinfo?(krosylight)
Pushed by nalexander@mozilla.com:
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
Status: REOPENED → RESOLVED
Closed: 9 months ago2 months ago
Resolution: --- → FIXED
Target Milestone: --- → 109 Branch
You need to log in before you can comment on or make changes to this bug.