1333233 - Add telemetry for the time it takes to undo initial automatic migration / import of data from other browsers, as well as any errors in that process

Status: RESOLVED FIXED

(Firefox :: Migration, defect)

Description

[:Gijs (he/him)]

In bug 1331800 we noticed that sometimes 'undo' of the initial automatic import can take a substantial amount of time. To figure out how common that is and how highly we should prioritize making it faster, we would need some added telemetry.

Comment 1

[Justin Dolske [:Dolske]]

(And Gijs mentioned we already have FX_STARTUP_MIGRATION_AUTOMATED_IMPORT_UNDO, for monitoring success/failures during the undo process)

Comment 2

[:Gijs (he/him)]

(In reply to Justin Dolske [:Dolske] from comment #1)
> (And Gijs mentioned we already have
> FX_STARTUP_MIGRATION_AUTOMATED_IMPORT_UNDO, for monitoring success/failures
> during the undo process)

And as you then realized, now that we're try...catch-ing some of those, we won't necessarily know how frequent those are, so we should probably add telemetry for this as well.

Comment 3

[Justin Dolske [:Dolske]]

Yeah. :/

I feel like I just mentioned this somewhere, but for counting the errors this feels similar to a problem we ran into with FX_MIGRATION_ERRORS... Sometimes you want to know "how many users encountered any error during the process", and other times you want to know "how many items failed".  Sort of a difference between "how widespread" and "how severe". FX_MIGRATION_ERRORS only tells us the former.

So maybe we want a similar FX_MIGRATION_UNDO_ERRORS, as well as FX_MIGRATION_*_UNDO_NUMERRORS probes for bookmarks/history/logins?

Actually, now that I think about it, another problem with FX_MIGRATION_ERRORS was that you have to compare it with something else to get error percentages (since it doesn't record successes). So maybe we should just unpack it too, into FX_MIGRATION_*_UNDO_HADERROR booleans?

Comment 4

[:Gijs (he/him)]

Attachment: Bug 1333233 - part 1: add telemetry for error counts from undo operations,

Review commit: https://reviewboard.mozilla.org/r/107496/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/107496/

Comment 5

[:Gijs (he/him)]

Attachment: Bug 1333233 - part 2: add telemetry for the time it takes to do individual undo operations,

Review commit: https://reviewboard.mozilla.org/r/107498/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/107498/

Comment 6

[:Gijs (he/him)]

(In reply to Justin Dolske [:Dolske] from comment #3)
> Yeah. :/
> 
> I feel like I just mentioned this somewhere, but for counting the errors
> this feels similar to a problem we ran into with FX_MIGRATION_ERRORS...
> Sometimes you want to know "how many users encountered any error during the
> process", and other times you want to know "how many items failed".  Sort of
> a difference between "how widespread" and "how severe". FX_MIGRATION_ERRORS
> only tells us the former.
> 
> So maybe we want a similar FX_MIGRATION_UNDO_ERRORS, as well as
> FX_MIGRATION_*_UNDO_NUMERRORS probes for bookmarks/history/logins?
> 
> Actually, now that I think about it, another problem with
> FX_MIGRATION_ERRORS was that you have to compare it with something else to
> get error percentages (since it doesn't record successes). So maybe we
> should just unpack it too, into FX_MIGRATION_*_UNDO_HADERROR booleans?

I ended up writing 1 histogram per resource type, and using the number of errors, which means '0' will also show up, which should make it possible to easily see how frequent any / some number of errors is (unlike for FX_MIGRATION_ERRORS, which I think uses a count histogram that only counts 1 for every time a migration produces any error(s), and keys a single histogram on the resource type instead of having 1 histogram per resource type keyed on the imported browser).

Comment 7

[Benjamin Smedberg]

Comment on attachment 8830777 [details]
Bug 1333233 - part 2: add telemetry for the time it takes to do individual undo operations,

https://reviewboard.mozilla.org/r/107498/#review108950

data-r=me (review Histograms.json only)

Comment 8

[Benjamin Smedberg]

Comment on attachment 8830776 [details]
Bug 1333233 - part 1: add telemetry for error counts from undo operations,

https://reviewboard.mozilla.org/r/107496/#review108952

data-r=me I suspect that in the future scalars will be cheaper/better for this, but given the newness and uplift to 52 the histograms are fine (I reviewed histograms.json only)

Comment 9

[Justin Dolske [:Dolske]]

Comment on attachment 8830776 [details]
Bug 1333233 - part 1: add telemetry for error counts from undo operations,

https://reviewboard.mozilla.org/r/107496/#review109500

Comment 10

[Justin Dolske [:Dolske]]

Comment on attachment 8830777 [details]
Bug 1333233 - part 2: add telemetry for the time it takes to do individual undo operations,

https://reviewboard.mozilla.org/r/107498/#review109502

Comment 11

[Justin Dolske [:Dolske]]

(In reply to :Gijs from comment #6)

> I ended up writing 1 histogram per resource type, and using the number of
> errors, which means '0' will also show up, which should make it possible to
> easily see how frequent any / some number of errors is

I suppose a small downside of this is that the histogram chart on TMO isn't going to be useful (because the giant bar for "0" will dwarf everything else", but the table view should still be sufficient.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/72a2e6016719
part 1: add telemetry for error counts from undo operations, r=bsmedberg,Dolske
https://hg.mozilla.org/integration/autoland/rev/1f8f9b5e74b0
part 2: add telemetry for the time it takes to do individual undo operations, r=bsmedberg,Dolske

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/72a2e6016719
https://hg.mozilla.org/mozilla-central/rev/1f8f9b5e74b0

Comment 14

[:Gijs (he/him)]

Comment on attachment 8830776 [details]
Bug 1333233 - part 1: add telemetry for error counts from undo operations,

Approval Request Comment
[Feature/Bug causing the regression]: automigration that we use in funnelcakes
[User impact if declined]: we won't have good data about how fast/slow the undo process is, or how often there are issues with it, in the funnelcakes
[Is this code covered by automated tests?]: yes!
[Has the fix been verified in Nightly?]: automatically, yes, not manually
[Needs manual test from QE? If yes, steps to reproduce]: don't think it needs it given the automatic tests
[List of other uplifts needed for the feature/fix]: n/a
[Is the change risky?]: no
[Why is the change risky/not risky?]: because this code is turned off for release (so this only affects the funnelcake) and there are automated tests for it
[String changes made/needed]: nope

Comment 15

[Julien Cristau [:jcristau]]

Just to make sure, the uplift request is for both patches, right?

Comment 16

[:Gijs (he/him)]

(In reply to Julien Cristau [:jcristau] from comment #15)
> Just to make sure, the uplift request is for both patches, right?

Yes!

Comment 17

[:Gijs (he/him)]

(In reply to :Gijs from comment #16)
> (In reply to Julien Cristau [:jcristau] from comment #15)
> > Just to make sure, the uplift request is for both patches, right?
> 
> Yes!

Bouncing ni back. :-)

Comment 18

[Julien Cristau [:jcristau]]

Comment on attachment 8830776 [details]
Bug 1333233 - part 1: add telemetry for error counts from undo operations,

telemetry probe to count import undo errors, aurora53+, beta52+

Comment 19

[Julien Cristau [:jcristau]]

Comment on attachment 8830777 [details]
Bug 1333233 - part 2: add telemetry for the time it takes to do individual undo operations,

telemetry probes for migration undo timing, aurora53+ and beta52+

Thanks Gijs!

Comment 20

[Carsten Book [:Tomcat]]

remote:   https://hg.mozilla.org/releases/mozilla-aurora/rev/5eb8b2de07550343d9222d9a52c199231fe150cc
remote:   https://hg.mozilla.org/releases/mozilla-aurora/rev/9df215ec5759683c835b3f25f30493eb669ca627

Comment 21

[Carsten Book [:Tomcat]]

remote:   https://hg.mozilla.org/releases/mozilla-beta/rev/7496413fecb8d6c5349db408c8689294e163877d
remote:   https://hg.mozilla.org/releases/mozilla-beta/rev/44777332a82750e2440b88f61a8db6b463377665

Comment 22

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Modified from Uplift Dashboard.

Comment 23

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8830776 [details]
Bug 1333233 - part 1: add telemetry for error counts from undo operations,

Adds more telemetry data for automigration scenarios. Aurora53+