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

RESOLVED FIXED in Firefox 49

Status

()

defect
P3
normal
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: Gijs, Assigned: Gijs)

Tracking

Trunk
Firefox 51
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox49 fixed, firefox50 fixed, firefox51 fixed)

Details

(Whiteboard: [migration-needs-uplift])

Attachments

(1 attachment)

No description provided.
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?
Attachment #8774811 - Flags: review?(jaws) → review+
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 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 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)
Attachment #8774811 - Flags: review?(benjamin) → review+
Pushed by gijskruitbosch@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3b0f290f1326
add telemetry for the length of time we take to import data, r=jaws,bsmedberg
Whiteboard: [migration-needs-uplift]
https://hg.mozilla.org/mozilla-central/rev/3b0f290f1326
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 51
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.
Attachment #8774811 - Flags: approval-mozilla-beta?
Attachment #8774811 - Flags: approval-mozilla-aurora?
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?
Attachment #8774811 - Flags: approval-mozilla-beta?
Attachment #8774811 - Flags: approval-mozilla-beta+
Attachment #8774811 - Flags: approval-mozilla-aurora?
Attachment #8774811 - Flags: approval-mozilla-aurora+
(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).
Depends on: 1338812
You need to log in before you can comment on or make changes to this bug.