1608011 - Sync telemetry isn't reporting the syncNodeType

Status: VERIFIED FIXED

(Firefox :: Sync, defect, P1)

Description

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

Because I neglected to extract it from the tokenserver response :(

[Tracking Requested - why for this release]:

We really want telemetry which indicates what if a user is on the "new" sync storage node.

Comment 1

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

Attachment: Bug 1608011 - ensure sync telemetry reports the storage node type. r?tcsc

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fe11ab9196ba
ensure sync telemetry reports the storage node type. r=tcsc

Comment 3

[Rachel Tublitz [:rachel]]

Comment on attachment 9119643 [details]
Bug 1608011 - ensure sync telemetry reports the storage node type. r?tcsc

Beta/Release Uplift Approval Request

• User impact if declined: We're starting the rollout of Durable Sync on Jan 13th; this is one piece that will help us track sync usage. Without it, we have less visibility into sync usage by node type (new storage backend vs old), so run a higher risk of having problems go unnoticed.
• Is this code covered by automated tests?: Unknown
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): not risky; relates to adding another "allowed" value for sync ping.
• String changes made/needed:

Comment 4

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9119643 [details]
Bug 1608011 - ensure sync telemetry reports the storage node type. r?tcsc

Looks pretty low-risk. Approved for 73.0b3.

Comment 5

[Cristian Brindusan [:cbrindusan]]

Backed out changeset fe11ab9196ba (bug 1608011) for xpcshell failures at test_tokenserverclient.js.
https://hg.mozilla.org/integration/autoland/rev/3ad59cfd0746a2ee76d5ebddbfeb4c3de0ef797c

Push with failures:
https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=pending%2Crunning%2Csuccess%2Ctestfailed%2Cbusted%2Cexception&revision=fe11ab9196ba2473c9a123c4624a5373a605f9f7&selectedJob=284273453

Failure log:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=284273453&repo=autoland&lineNumber=2756

Comment 6

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/releases/mozilla-beta/rev/9f11ce98a971

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/783350b56e3e
ensure sync telemetry reports the storage node type. r=tcsc

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e166c43d60e3
Fix ESlint. r=lint-fix CLOSED TREE

Comment 9

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/releases/mozilla-beta/rev/70d807a5a61d

Comment 10

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/783350b56e3e
https://hg.mozilla.org/mozilla-central/rev/e166c43d60e3

Comment 11

[Vasilica Tamas, QA [:vtamas]]

Manual QA attention is confusing here, considering that in Comment 3 is specified that there is no need for manual test from QE and therefore no steps to reproduce are listed, but in Comment 4 the qe-verify flag is marked with “+”.

Could someone clarify this?

Comment 12

[Ryan VanderMeulen [:RyanVM]]

I thought it would be a good idea to verify that the Telemetry pings are containing the expected information now.

Comment 13

[Rachel Tublitz [:rachel]]

Good call, :RyanVM. I think we've got what we need now, but let me ni? Leif here just to confirm that.

Comment 14

[Leif Oines [:loines]]

I can confirm we are getting data in this field now, see this query

Comment 15

[Ryan VanderMeulen [:RyanVM]]

I followed up with Leif and confirmed that the pings are coming from all the expected channels at this point. I think that's enough to call this verified :)