1289436 - Add telemetry for the time it takes to import history, bookmarks and logins

Status: RESOLVED FIXED

(Firefox :: Migration, defect, P3)

Comment 1

[:Gijs (out for now; he/him)]

Attachment: Bug 1289436 - add telemetry for the length of time we take to import data,

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

Comment 2

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8774811 [details]
Bug 1289436 - add telemetry for the length of time we take to import data,

https://reviewboard.mozilla.org/r/67222/#review64154

::: browser/components/migration/MigrationUtils.jsm:239
(Diff revision 1)
> +    let maybeStartTelemetry = (resourceType, resource) => {
> +      let histogram = getHistogramForResourceType(resourceType);
> +      if (histogram) {
> +        TelemetryStopwatch.startKeyed(histogram, this.getKey(), resource);
> +      }
> +    };
> +    let maybeStopTelemetry = (resourceType, resource) => {
> +      let histogram = getHistogramForResourceType(resourceType);
> +      if (histogram) {
> +        TelemetryStopwatch.finishKeyed(histogram, this.getKey(), resource);

These function names almost sound like they are starting and stopping the complete Telemetry system.

Can you rename these functions to
maybeStartTelemetryStopwatch and maybeStopTelemetryStopwatch? Or maybe you can think of a better name?

Comment 3

[:Gijs (out for now; he/him)]

Comment on attachment 8774811 [details]
Bug 1289436 - add telemetry for the length of time we take to import data,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/67222/diff/1-2/

Comment 4

[:Gijs (out for now; he/him)]

Comment on attachment 8774811 [details]
Bug 1289436 - add telemetry for the length of time we take to import data,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/67222/diff/2-3/

Comment 5

[Benjamin Smedberg]

Comment on attachment 8774811 [details]
Bug 1289436 - add telemetry for the length of time we take to import data,

https://reviewboard.mozilla.org/r/67222/#review66136

data-review=me (I did not review the code)

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3b0f290f1326
add telemetry for the length of time we take to import data, r=jaws,bsmedberg

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/3b0f290f1326

Comment 8

[:Gijs (out for now; he/him)]

Comment on attachment 8774811 [details]
Bug 1289436 - add telemetry for the length of time we take to import data,

Approval Request Comment
[Feature/regressing bug #]: both user profile data migration generally and automatic migration specifically (funnelcakes from bug 1295873) 
[User impact if declined]: we won't know how long the migration takes.
Initial data from nightly suggests the range could be quite wide, but we all know that nightly is pretty different from "normal" release users. Especially with automatic migration, which happens while we start the browser but doesn't currently have explicit feedback ("we finished importing your data" type UI), it would be good to know how long it typically takes to import this data, also because we're investigating using about:newtab tiles to reflect this data. If the data is not there (or if we'd have to wait a long time for the data to be there) that obviously doesn't work. So it's quite important to know how long this import process takes "in the wild", so we can evaluate if specific importers need performance improvements, or if it's completely unrealistic to want it done "quickly".

[Describe test coverage new/current, TreeHerder]: this is purely telemetry data, so there's no explicit test coverage.
[Risks and why]: medium-ish risk. The telemetry works on nightly, so it's not completely broken or anything. We're not aware of any regressions, but manual QE would be useful as this patch isn't as old as the other migration work.
[String/UUID change made/needed]: no.

Comment 9

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8774811 [details]
Bug 1289436 - add telemetry for the length of time we take to import data,

Telemetry for support for funnelcake onboarding. 
Gijs, any specific manual QE that will be useful here?

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/c2aa582a5980

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/ea34f05424a8

Comment 12

[:Gijs (out for now; he/him)]

(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #9)
> Comment on attachment 8774811 [details]
> Bug 1289436 - add telemetry for the length of time we take to import data,
> 
> Telemetry for support for funnelcake onboarding. 
> Gijs, any specific manual QE that will be useful here?

well, for the entirety of this set of uplifts, useful smoketesting would be:

- check that a fresh install on a machine with no profiles starts up and shows migration as before
- check that the migrations from chrome and IE on Windows run normally (NB: of course it's possible this testing turns up unrelated issues with those migrations - it sometimes has in the past...). To aid testing, you can also test the startup paths on non-vanilla machines by explicitly passing the -migration flag on the commandline.

This particular cset only adds telemetry, so there's not much besides waiting for the beta/aurora builds to be released to users and then keeping an eye on telemetry in case something odd turns up. I guess we could manually verify with about:telemetry and a manual import (which can also be done via the bookmarks manager (library) and the passwords manager in about:preferences).