Closed Bug 1338812 Opened 3 years ago Closed 3 years ago

Call TelemetryStopwatch only once per item (history / bookmarks / logins) rather than for each resource

Categories

(Firefox :: Migration, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 54
Tracking Status
firefox52 --- fixed
firefox-esr52 --- fixed
firefox53 --- fixed
firefox54 --- fixed

People

(Reporter: dao, Assigned: dao)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

Gijs says it shouldn't be a problem for the data we're looking at, but it's probably still a good idea to clean this up.

<dao> Gijs: what kind of things are migrationType and itemResources in https://dxr.mozilla.org/mozilla-central/source/browser/components/migration/MigrationUtils.jsm#297 ?
<Gijs> dao: itemResources are objects that have a 'migrate()' function that gets called to migrate that particular resource
<Gijs> dao: migrationType is the type of resource migration (bookmarks, passwords, etc.)
<Gijs> dao: so for instance, for edge we import 2 resources of type bookmarks
<Gijs> (the normal bookmarks and the reading list)
<dao> and we use a separate TelemetryStopwatch for both?
<Gijs> dao: there's also an "other data" type that I think we use several of when doing a Firefox reset/refresh
<Gijs> dao: looks like it, yes.
<dao> Gijs: is that intentional? seems weird to me
<Gijs> dao: I don't think it was considered when implementing the stopwatches. Regardless, I don't think it makes a big difference here...
<Gijs> dao: there's only 1 item of each resource type for Chrome, so...
<Gijs> ditto for IE, though we have 2 separate ways of importing passwords (depending on the Windows/IE version), I'm fairly sure we only ever use one at a time.
Assignee: nobody → dao+bmo
Status: NEW → ASSIGNED
Comment on attachment 8836401 [details]
Bug 1338812 - Call TelemetryStopwatch only once per item (history / bookmarks / logins) rather than for each resource.

https://reviewboard.mozilla.org/r/111826/#review113098
Attachment #8836401 - Flags: review?(gijskruitbosch+bugs) → review+
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/80d9bac9ec22
Call TelemetryStopwatch only once per item (history / bookmarks / logins) rather than for each resource. r=Gijs
Comment on attachment 8836401 [details]
Bug 1338812 - Call TelemetryStopwatch only once per item (history / bookmarks / logins) rather than for each resource.

Approval Request Comment
[Feature/Bug causing the regression]: bug 1289436

[User impact if declined]: the data collected for FX_MIGRATION_BOOKMARKS_IMPORT_MS, FX_MIGRATION_HISTORY_IMPORT_MS and FX_MIGRATION_LOGINS_IMPORT_MS could be wrong / misleading in some cases

[Is this code covered by automated tests?]: no

[Has the fix been verified in Nightly?]: no

[Needs manual test from QE? If yes, steps to reproduce]: no

[List of other uplifts needed for the feature/fix]: /

[Is the change risky?]: not particularly risky

[Why is the change risky/not risky?]: it's fairly trivial

[String changes made/needed]: /
Attachment #8836401 - Flags: approval-mozilla-beta?
Attachment #8836401 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/80d9bac9ec22
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 54
Comment on attachment 8836401 [details]
Bug 1338812 - Call TelemetryStopwatch only once per item (history / bookmarks / logins) rather than for each resource.

fix some telemetry probes, aurora53+, beta52+
Attachment #8836401 - Flags: approval-mozilla-beta?
Attachment #8836401 - Flags: approval-mozilla-beta+
Attachment #8836401 - Flags: approval-mozilla-aurora?
Attachment #8836401 - Flags: approval-mozilla-aurora+
has problems to apply to beta:

merging browser/components/migration/MigrationUtils.jsm
warning: conflicts while merging browser/components/migration/MigrationUtils.jsm! (edit, then use 'hg resolve --mark')
abort: unresolved conflicts, can't continue
(use 'hg resolve' and 'hg graft --continue')
Flags: needinfo?(dao+bmo)
Flags: needinfo?(dao+bmo)
You need to log in before you can comment on or make changes to this bug.