Closed Bug 1629116 Opened 3 months ago Closed 1 month ago

Identify any additions to sync telemetry we want for rust-webext-storage (and future rust sync engines)

Categories

(Firefox :: Sync, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 79
Tracking Status
firefox78 --- fixed
firefox79 --- fixed

People

(Reporter: tcsc, Assigned: tcsc)

References

(Depends on 1 open bug)

Details

(Whiteboard: SACI)

Attachments

(2 files)

At a minimum this is probably:

  • A flag that the user has the rust sync engine enabled.
  • Very likely some new telemetry error types.
  • Some way of determining the version of the rust components.
  • Any telemetry needed for us to gain confidence in sync/merge algorithms. Probably conflict counts and such.

These are mostly small, and can either land together or in their own bug.

This is just the sync ping changes.

Depends on: 1634482
Depends on: 1634483

Add an overrideTelemetryName to bridged extension storage.

Assignee: nobody → tchiovoloni
Status: NEW → ASSIGNED

The patch that's up is simpler than the listed description as:

  • New telemetry error types are covered by Bug 1634483
  • Rust component version can be determined by firefox version / buildid
  • Existing sync telemetry seems sufficient for merge info (already have some info in the ping around this).

This leaves the flag, but we can just do it via the overrideTelemetryName much more safely and with less churn.

Pushed by tchiovoloni@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/89b2ab5018a8
Add an overrideTelemetryName to bridged extension storage. r=markh
Status: ASSIGNED → RESOLVED
Closed: 1 month ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 79

Comment on attachment 9154407 [details]
Bug 1629116 - Add an overrideTelemetryName to bridged extension storage. r?markh

Beta/Release Uplift Approval Request

  • User impact if declined: Errors in the new extension-storage sync would be masked due an inability to distinguish between problems in the (old) extension-storage engine and the (new) rust-based extension storage engine it replaces.
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: No
  • If yes, steps to reproduce: N/A
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): It's a very small change to change the name of an engine in sync telemetry. The change is entirely limited to sync telemetry
  • String changes made/needed: N/A
Attachment #9154407 - Flags: approval-mozilla-beta?

Comment on attachment 9154407 [details]
Bug 1629116 - Add an overrideTelemetryName to bridged extension storage. r?markh

approved for 78.0b5

Attachment #9154407 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Flags: needinfo?(tchiovoloni)

oh no! looking.

I have a fix for this but am unsure what the process is for getting it up. Do I submit to phab again? attach a patch?

Flags: needinfo?(tchiovoloni) → needinfo?(jcristau)

Submit a new phab revision, please. And request beta approval if it touches more than tests. Thank you in advance.

Flags: needinfo?(jcristau) → needinfo?(tchiovoloni)
Pushed by tchiovoloni@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ae2a0ff04460
Ensure the sync engine we want telemetry from is actually registered in tests. r=lina

Okay, that fixes it. It's only tests.

Flags: needinfo?(tchiovoloni)

Should be good now, meant to ni.

Flags: needinfo?(aryx.bugmail)

Thanks!

Flags: needinfo?(aryx.bugmail)
You need to log in before you can comment on or make changes to this bug.