1963812 - Migrate Legacy Telemetry Custom Ping "sync" to Glean

Status: RESOLVED FIXED

(Firefox :: Sync, task, P2)

Description

[Chris H-C :chutten]

The Legacy Telemetry "sync" ping is implemented in services/sync/modules/telemetry.sys.mjs. Its purpose is to report, about twice a day, information on any syncs that might have happened over that period.

We are receiving 20M-25M "sync" pings a day, a rate that is very stable over the year to-date. Users of this ping in re:dash over the past four months include :loines, :mhammond, and :skhamis. According to datahub's lineage explorer the "sync" ping only feeds into the telemetry.sync dataset. It isn't used in Looker or as an input into other derived tables.

This bug is about building a Glean equivalent to the "sync" ping so that analyses may be moved over to use the Glean-sent data later this year.

Comment 1

[Chris H-C :chutten]

Hey, :markh, I hear through the grapevine that maybe the sync backend is scheduled for replacement in the near future? That could mean it would be wasted work if I migrated the legacy ping, and we wouldn't want that.

Comment 2

[Mark Hammond [:markh] [:mhammond]]

(In reply to Chris H-C :chutten from comment #1)

Hey, :markh, I hear through the grapevine that maybe the sync backend is scheduled for replacement in the near future?

Nope, for better or worse that's an unfounded rumour.

Moving the sync ping to glean sounds fine. What would be fantastic though would be if the new ping can align towards the glean pings already defined on mobile (eg, there we were asked to not have single ping but instead have each engine have its own ping etc) which will make any sharing of the sync code we do in the future much easier to manage and rationalize about. This would likely screw up some of our dashboards, but I think that would be fine as it would mean we could just re-do them by largely copying the dashboards we have for mobile.

cc @skhamis

Comment 3

[Chris H-C :chutten]

Alas, I don't have the bandwidth to make those changes in this bug, but I'd be really happy to help you move to such an approach in the future. (Due to the volume of the work, we need to focus on making it possible to turn off the legacy collection on time (whenever that turns out to be in the months-to-year scale) which means preferring direct translation)

For now I'll proceed with reinstrumenting the "sync" ping as a Glean ping (to be clear: leaving the Legacy ping in place as well) mimicking very closely the schedule and the structure of the existing one. Any obvious and small wins (breaking up compound fields so you don't have to write parsers in SQL, firming up semantics, removing anything that is clearly unnecessary, updating descriptions and documentation, etc.) I'll make as I find them and take as feedback on the code review.

I mean, unless I'm overestimating how much of a change you're asking for. If it's straightforward and roughly the same amount of work, I can give it a go? Or support someone on your side who can get it done? So long as we can land it before, say, mid-June.

Comment 4

[Mark Hammond [:markh] [:mhammond]]

I doubt our team can get this kind of change done in 2 weeks. A direct translation seems perfectly fine in the first instance.

Comment 5

[Chris H-C :chutten]

Attachment: Bug 1963812 - Migrate the "sync" ping to Glean r?markh,skhamis

Glean conveniently supports send_in_pings, meaning we can plumb the external
events and histograms into the Glean "sync" ping without using observer/notify.

Normally, I'd place the Glean data checks alongside the Telemetry ones in the
instrumentation tests, but test_telemetry.js proved tricky.
Thus we now have test_glean.js which includes all the same data checks,
but pointed at Glean.
Plus a new testcase for testing migrations.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/b5d2ff22684a
https://hg.mozilla.org/integration/autoland/rev/71a7cf336537
Migrate the "sync" ping to Glean r=markh,credential-management-reviewers,sync-reviewers,dimi

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/4af280a39685
https://hg.mozilla.org/integration/autoland/rev/546d484fe538
Revert "Bug 1963812 - Migrate the "sync" ping to Glean r=markh,credential-management-reviewers,sync-reviewers,dimi" for causing build bustages @rust.mk.

Comment 8

[Chris H-C :chutten]

Oh. Okay? Rust is interpreting the backtick blocks in the metrics descriptions as tests to run. Welp.

Comment 9

[agoloman]

https://hg.mozilla.org/mozilla-central/rev/71a7cf336537

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/6e9ef9cfb2b4
https://hg.mozilla.org/integration/autoland/rev/8708d3935a69
Migrate the "sync" ping to Glean r=markh,credential-management-reviewers,sync-reviewers,dimi

Comment 11

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/8708d3935a69