Closed Bug 1261181 Opened 4 years ago Closed 4 years ago

Various Sync xpcshell tests are going to permafail when Gecko is bumped to version 49

Categories

(Firefox :: Sync, defect)

defect
Not set
critical

Tracking

()

RESOLVED FIXED
Tracking Status
firefox48 --- fixed

People

(Reporter: RyanVM, Assigned: markh)

Details

Attachments

(1 file)

Merge day is about two weeks away, so this is pretty critical.

https://treeherder.mozilla.org/logviewer.html#?job_id=18720292&repo=try
Flags: needinfo?(markh)
As seems customary these days, most of these errors are due to telemetry probes expiring.
Flags: firefox-backlog+
Lots of probes are set to expire in 48, so this removes them completely (ie, from Histograms.json, from the code and from the tests).

Ryan, can you please verify this solves the test failures? The all still pass locally for me, but I'm not sure how to push a try run that simulates a world post-merge.
Assignee: nobody → markh
Status: NEW → ASSIGNED
Flags: needinfo?(markh) → needinfo?(ryanvm)
Attachment #8737044 - Flags: review?(kcambridge)
https://hg.mozilla.org/try/rev/75ce8e268e62 is all you need to apply locally.
Flags: needinfo?(ryanvm)
Comment on attachment 8737044 [details] [diff] [review]
0001-Bug-1261181-remove-telemetry-probes-set-to-expire-in.patch

Review of attachment 8737044 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good! <3 deleting superfluous code.

::: services/sync/modules/engines.js
@@ +1228,5 @@
>      Observers.notify("weave:engine:sync:applied", count, this.name);
>    },
>  
>    _noteApplyFailure: function () {
> +    // here would be a good place to record telemetry...

The probes we had weren't actionable, I assume.
Attachment #8737044 - Flags: review?(kcambridge) → review+
(In reply to Kit Cambridge [:kitcambridge] from comment #5)
> The probes we had weren't actionable, I assume.

Probably closer to saying "they weren't being actioned" (but yeah, last I looked we actioned all we could).

Note that even without this change, the probes were being ignored starting 49, so there's no value in keeping them without bumping the expires version. 48 will be in release until August this year, so the data is still available, and I figured we should just re-add new probes as we find them necessary.
Cool, sounds like a plan!
Comment on attachment 8737044 [details] [diff] [review]
0001-Bug-1261181-remove-telemetry-probes-set-to-expire-in.patch

Looks good on Try, thanks!
Attachment #8737044 - Flags: feedback+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/eaab4a335361
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Component: Firefox Sync: Backend → Sync
Product: Cloud Services → Firefox
You need to log in before you can comment on or make changes to this bug.