Since migration from other browsers is hard to test in an automated fashion while keeping up with other browser's updates, it would be useful to report success and failure rates for each migrator to know when there are problems that need to be investigated. Use Case 1: * A new version of Chrome is released which changes the data format of bookmarks causing our import to fail. We will see an increase in Chrome migration failures then investigate and fix the migrator.

Well, at the time auto-migration runs (no profiles at all), isn't Telemetry guaranteed to be disabled? Two options, if I'm right: 1. "Record" the errors somewhere MigrationUtils.jsm (having a module actually helps here!) and build the Record it for real only once the user enables Telemetry (it should all be done during the same Fx run, obviously). 2. In the case of an error during auto-migration, ask the user, within the migration wizard, to help us. That's pretty bad UE, in my opinion. It would be nice if telemetry-provided a canRecord=not-yet state, in which you can records are not rejected completely, but kept around until the user decides if he or she wants to send this data.

telemetry sends data at the second idle, but you can register values at any time

ehr, except when telemetry is disabled, that is indeed the case on the first migration. So that's indeed an issue.

A possible solution would be to indeed record that data in the module and start listening for gather-telemetry to register it. Though I wonder if sending telemetry data collected before telemetry is enabled is considered a privacy hit.

cc-ing sid for the privacy implications.

I really don't like the idea of submitting data to us that was collected before the user granted us permission. That might surprise our users. Telemetry is going "on by default" for nightly and aurora (bug 699806). Can we collect this info from those channels instead of buffering stuff and waiting for an opt-in?

(In reply to Sid Stamm [:geekboy] from comment #6) > Telemetry is going "on by default" for nightly and aurora (bug 699806). Can > we collect this info from those channels instead of buffering stuff and > waiting for an opt-in? I don't think it's worth it. It's very unlikely that Nightly or Aurora users would ever use first-time migration.

You're being too nice to me Mano... you should just say "Sid, that's dumb, those users already use Firefox." :) I'm still not comfortable collecting data generated before we ask if we can collect data. Is there a way to create an automated test for this instead?

(In reply to Sid Stamm [:geekboy] from comment #8) > Is there a way to create an automated test for this instead? The problem is that it's changes outside our code, in other browsers that needs to be tested. It would require us to either 1) Have all the browsers we migrate from up to date on all the build machines or 2) Someone would have to migrate the source profiles with each browser release and update the test suite. With the telemetry approach we are avoiding the maintenance burden.

So, WONTFIX? Surely this does not block 718608. About the "QA option", Marco (in bug 718608 comment 34): "I briefly discusses with Juan to setup a special QA session for migrators, I just have to collect all the bugs with their ETA and send them a mail to plan it, so we may get some better coverage."

A few QA passes in the short term won't let us know when a migrator breaks in the long term. If the migration litmus tests are being run for each source browser on Aurora then that will catch many problems but it's still not as good as getting data from the wild with different configurations (OS, source browser version, source profile data, etc.). I still think this is a good idea but if the privacy concern is a blocker then we can WONTFIX.

We can reevaluate this if/when the migration ui changes.

Please add privacy-review-needed flag if this gets reopened.

We may be able to use opt-out probes for this since it could be a key factor in retaining users. Probe proposal: * MIGRATION_ENTRY_POINT – enumerated with buckets for: 0) Other/catch-all if not specified 1) firstrun 2) Refresh Firefox 3) Places window. Note: We could merge this with MIGRATION_SOURCE_BROWSER by making the entry point a key for the browser histogram but then there wouldn't be a single dashboard view to understand the popularity of different sources. * MIGRATION_SOURCE_BROWSER – enumerated histogram 0 = Other/uncategorized (e.g. a new migrator that didn't get added to this probe. This should be rare but I'd rather not drop the data on the ground) 1 = Don't import anything 2 = firefox (for Firefox Refresh) 3 = ie 4 = chrome 5 = safari 6 = 360se * MIGRATION_ERRORS – enumerated and keyed by browser ID. The key is the migrator internal name[1] e.g. "chrome". Each key has buckets for each data type using the existing internal number[2] representing each data type (excluding nsIBrowserProfileMigrator.ALL). The bucket gets incremented if a error occurred during migration. We should define what an error is for each data type. KEY BUCKET "ie" 0 = settings 1 = cookies 2 = history 3 = form data … "chrome" 0 = settings 1 = cookies 2 = history 3 = form data … … … * MIGRATION_USAGE – same as MIGRATION_ERRORS but buckets get incremented if we attempt migration for a data type (regardless of whether it was successful or not). This is only useful for the entry point where the user manually opens the migrator in an existing profile as the other entry points don't offer a choice of data types. Given how low the usage of that entry point is, I'm not sure it's worth recording this. * MIGRATION_HOMEPAGE_IMPORTED – boolean histogram indicating whether the user chose to import their homepage from the source browser. This is only offered in release builds during firstrun. See https://wiki.mozilla.org/QA/Firefox_migrators#Differences_between_official_and_unofficial_builds [1] https://mxr.mozilla.org/mozilla-central/source/browser/components/migration/MigrationUtils.jsm?rev=eeeaa094d312#44 [2] https://mxr.mozilla.org/mozilla-central/source/browser/components/migration/nsIBrowserProfileMigrator.idl#14

[Tracking Requested - why for this release]: This is on our list for 41 but isn't done yet. Adding the tracking flag so we can see when it lands and make a call.

Tracked though we do not have much runway left to uplift this in Beta41. If this doesn't land in time, it will need to be deferred to 42.

Too late to take a fix in 41. Let's target 42 instead.

Comment on attachment 8659900 [details] [diff] [review] WIP Review of attachment 8659900 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/components/migration/content/migration.js @@ +481,5 @@ > Cc["@mozilla.org/consoleservice;1"] > .getService(Ci.nsIConsoleService) > .logStringMessage("some " + type + " did not successfully migrate."); > + Services.telemetry.getKeyedHistogramById("FX_MIGRATION_ERRORS") > + .add(type); Drive-by which you may already know: You're missing the migrator ID as the key. Also I think you should assign parseInt(aData) to a temp. variable and record Math.log2(temp) instead of using the type string. ::: toolkit/components/telemetry/Histograms.json @@ +4113,5 @@ > + "expires_in_version": "never", > + "kind": "enumerated", > + "n_values": "5", > + "releaseChannelCollection": "opt-out", > + "description": "Where the migration wizard was entered from. 0=Other/catch-all, 1=first-run, 2=refresh-firefox, 3=Places window, 4=Password manager" Where/how do you handle the --migration command line argument? The reason I suggested a catch-all is because I was thinking this would be accumulated in a single place (showMigrationWizard) but maybe it doesn't make sense to change the arguments just for the sake of telemetry. If we go with this approach, please add a comment on showMigrationWizard to remind consumers to record the telemetry.

Gijs, would you be able to take over this bug? I can go over the patch with you if you'd like.

I can take this. I'll ping you with questions if I have them.

(In reply to Matthew N. [:MattN] (behind on mail) from comment #19) > Comment on attachment 8659900 [details] [diff] [review] > WIP > > Review of attachment 8659900 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: browser/components/migration/content/migration.js > @@ +481,5 @@ > > Cc["@mozilla.org/consoleservice;1"] > > .getService(Ci.nsIConsoleService) > > .logStringMessage("some " + type + " did not successfully migrate."); > > + Services.telemetry.getKeyedHistogramById("FX_MIGRATION_ERRORS") > > + .add(type); > > Drive-by which you may already know: You're missing the migrator ID as the > key. Also I think you should assign parseInt(aData) to a temp. variable and > record Math.log2(temp) instead of using the type string. Can you elaborate on this feedback? Why log2 ? Likewise, the patch is using string keys for the browsers, which is different from comment #14. Is there a compelling reason not to use human-readable strings (which also ensures that new things work semi-automatically, instead of having to assign them a number (or worse, shifting the numbers if they get inserted in the middle of the list of browsers) ? Or am I misunderstanding how you intended this to work? > ::: toolkit/components/telemetry/Histograms.json > @@ +4113,5 @@ > > + "expires_in_version": "never", > > + "kind": "enumerated", > > + "n_values": "5", > > + "releaseChannelCollection": "opt-out", > > + "description": "Where the migration wizard was entered from. 0=Other/catch-all, 1=first-run, 2=refresh-firefox, 3=Places window, 4=Password manager" > > Where/how do you handle the --migration command line argument? > > The reason I suggested a catch-all is because I was thinking this would be > accumulated in a single place (showMigrationWizard) but maybe it doesn't > make sense to change the arguments just for the sake of telemetry. If we go > with this approach, please add a comment on showMigrationWizard to remind > consumers to record the telemetry. Personally, I think it'd be sensible to update the API in order to guarantee accurate sources. I'll look into adapting the patch to use that approach. I'm a bit concerned about the current code in nsAppRunner.cpp and whether that gets stored in the post-migration profile in the case of Firefox reset, but I've not looked into the details yet. If you have pointers regarding this that would be helpful.

(In reply to :Gijs Kruitbosch from comment #22) > (In reply to Matthew N. [:MattN] (behind on mail) from comment #19) > > Comment on attachment 8659900 [details] [diff] [review] > > WIP > > > > Review of attachment 8659900 [details] [diff] [review]: > > ----------------------------------------------------------------- > > > > ::: browser/components/migration/content/migration.js > > @@ +481,5 @@ > > > Cc["@mozilla.org/consoleservice;1"] > > > .getService(Ci.nsIConsoleService) > > > .logStringMessage("some " + type + " did not successfully migrate."); > > > + Services.telemetry.getKeyedHistogramById("FX_MIGRATION_ERRORS") > > > + .add(type); > > > > Drive-by which you may already know: You're missing the migrator ID as the > > key. Also I think you should assign parseInt(aData) to a temp. variable and > > record Math.log2(temp) instead of using the type string. > > Can you elaborate on this feedback? Why log2 ? Take a look at the data type consts in nsIBrowserProfileMigrator[1]. They are 2^n for n=1…7 and so I'm saying to use log2 so we can record this in a linear fashion (probably with an enumerated histogram) instead of exponential which would be weird to consume for exclusive options that don't have exponential meaning to humans. [1] https://mxr.mozilla.org/mozilla-central/source/browser/components/migration/nsIBrowserProfileMigrator.idl#17 > Likewise, the patch is using string keys for the browsers, which is > different from comment #14. Is there a compelling reason not to use > human-readable strings (which also ensures that new things work > semi-automatically, instead of having to assign them a number (or worse, > shifting the numbers if they get inserted in the middle of the list of > browsers) ? Or am I misunderstanding how you intended this to work? I did suggest using the ID of the migrator (which is a string) as the key for MIGRATION_ERRORS and MIGRATION_USAGE in comment 14. For MIGRATION_SOURCE_BROWSER, we want to know which one was ultimately chosen when the migration actually happens so an enumerated histogram makes sense to me and you can't give string arguments for the value and have it map to a specific bucket and I thought that one bucket per browser was nicer for analysis than a separate keyed histogram per browser. > > ::: toolkit/components/telemetry/Histograms.json > > @@ +4113,5 @@ > > > + "expires_in_version": "never", > > > + "kind": "enumerated", > > > + "n_values": "5", > > > + "releaseChannelCollection": "opt-out", > > > + "description": "Where the migration wizard was entered from. 0=Other/catch-all, 1=first-run, 2=refresh-firefox, 3=Places window, 4=Password manager" > > > > Where/how do you handle the --migration command line argument? > > > > The reason I suggested a catch-all is because I was thinking this would be > > accumulated in a single place (showMigrationWizard) but maybe it doesn't > > make sense to change the arguments just for the sake of telemetry. If we go > > with this approach, please add a comment on showMigrationWizard to remind > > consumers to record the telemetry. > > Personally, I think it'd be sensible to update the API in order to guarantee > accurate sources. I'll look into adapting the patch to use that approach. > > I'm a bit concerned about the current code in nsAppRunner.cpp and whether > that gets stored in the post-migration profile in the case of Firefox reset, > but I've not looked into the details yet. If you have pointers regarding > this that would be helpful. The nsAppRunner.cpp changes are in the startup code path so I'm not worried about it being in the old profile. It's possible that telemetry isn't ready for data at that point but that would be easy to verify in about:telemetry and I suspect it will fine since we record other telemetry during startup (though maybe things like "simple measures" have special handling).

Bug 731025 - Add telemetry for migrator usage and errors, r?MattN

Comment on attachment 8670748 [details] MozReview Request: Bug 731025 - Add telemetry for migrator usage and errors, r?MattN https://reviewboard.mozilla.org/r/21445/#review19335 There are still some things I didn't look closely at but it seems like perhaps there is some leftover or WIP code in this patch and I have a some proposed changes to make things more accurate so I'd like to take another look. ::: browser/components/migration/MigrationUtils.jsm:550 (Diff revision 1) > * @param [optioanl] aParams > * arguments for the migration wizard, in the form of an nsIArray. > * This is passed as-is for the params argument of > * nsIWindowWatcher.openWindow. This is no longer expecting an nsIArray and isn't passed as-is. Could you also fix the typo with "optioanl"? Even better if you add JSDoc data types. ::: browser/components/migration/MigrationUtils.jsm:571 (Diff revision 1) > + let params; > + if (Array.isArray(aParams)) { > + params = Cc["@mozilla.org/array;1"].createInstance(Ci.nsIMutableArray); > + for (let item of aParams) { > + let comtaminatedVal; > + if (typeof item == "boolean") { > + comtaminatedVal = Cc["@mozilla.org/supports-PRBool;1"]. > + createInstance(Ci.nsISupportsPRBool); Sorry, why are we comtaminating the arguments? Was this so that the callers passing the source didn't need to use PRUint32? If so, that's fine. ::: browser/components/migration/MigrationUtils.jsm:665 (Diff revision 1) > - let params = Cc["@mozilla.org/array;1"].createInstance(Ci.nsIMutableArray); > - let keyCSTR = Cc["@mozilla.org/supports-cstring;1"]. > - createInstance(Ci.nsISupportsCString); > - keyCSTR.data = migratorKey; > + let params = [ > + this.MIGRATION_SOURCE_FIRSTRUN, migratorKey, migrator, > + aProfileStartup, skipSourcePage > + ]; IMO, one per line would be easier to read. Partially due to the lack of JSM syntax highlighting on mozreview (bug 1157502), I thought there were only 2 params at first glance. ::: browser/components/migration/MigrationUtils.jsm:666 (Diff revision 1) > - let keyCSTR = Cc["@mozilla.org/supports-cstring;1"]. > + this.MIGRATION_SOURCE_FIRSTRUN, migratorKey, migrator, Sometimes this should be MIGRATION_SOURCE_FXREFRESH IIRC. This code doesn't make that explicit but I believe the only time aMigratorKey is set is for MIGRATION_SOURCE_FXREFRESH, otherwise it's MIGRATION_SOURCE_FIRSTRUN. ::: browser/components/migration/MigrationUtils.jsm:681 (Diff revision 1) > + MIGRATION_SOURCE_UNKNOWN: 0, > + MIGRATION_SOURCE_FIRSTRUN: 1, > + MIGRATION_SOURCE_FXREFRESH: 2, > + MIGRATION_SOURCE_PLACES: 3, > + MIGRATION_SOURCE_PASSWORDS: 4, > + > + _sourceNameToIdMapping: { > + "nothing": 1, > + "firefox": 2, I think these MIGRATION_SOURCE_\* should be renamed to refer to something like entry points instead of "source" which we use to indentify the source application below. ::: browser/components/migration/content/migration.js:127 (Diff revision 1) > + Services.telemetry.getHistogramById("FX_MIGRATION_SOURCE_BROWSER") > + .add(MigrationUtils.getSourceIdForTelemetry(newSource)); This should only be recorded when an actual migration occurs. At this point the user can still go back and forward to change their mind and I don't think that's as useful as knowing what they ultimately choose. Then we can also get rid of the duplication of settings this for a Refresh. ::: browser/components/places/content/places.js:366 (Diff revision 1) > + MigrationUtils.showMigrationWizard(window, [3]); MigrationUtils.MIGRATION_SOURCE_PLACES? ::: toolkit/components/passwordmgr/content/passwordManager.js:454 (Diff revision 1) > + MigrationUtils.showMigrationWizard(window, [4]); MigrationUtils.MIGRATION_SOURCE_PASSWORDS? ::: toolkit/xre/nsAppRunner.cpp:2401 (Diff revision 1) > + mozilla::Telemetry::Accumulate(mozilla::Telemetry::FX_MIGRATION_ENTRY_POINT, 2); Maybe add a comment with "MIGRATION_SOURCE_FXREFRESH" but I actually think this should be handled in startupMigration ::: toolkit/xre/nsAppRunner.cpp:2402 (Diff revision 1) > + mozilla::Telemetry::Accumulate(mozilla::Telemetry::FX_MIGRATION_SOURCE_BROWSER, NS_LITERAL_CSTRING("firefox"), 1); Why not do this in the actual wizard code?

https://reviewboard.mozilla.org/r/21445/#review19337 ::: browser/components/migration/content/migration.js:127 (Diff revision 1) > + Services.telemetry.getHistogramById("FX_MIGRATION_SOURCE_BROWSER") > + .add(MigrationUtils.getSourceIdForTelemetry(newSource)); No they can't, this calls .cancel() straight after the telemetry is added, closing the dialog. Maybe I should hardcode "nothing" as the param here, instead of referring to newSource? Would that be clearer?

https://reviewboard.mozilla.org/r/21445/#review19337 > No they can't, this calls .cancel() straight after the telemetry is added, closing the dialog. Maybe I should hardcode "nothing" as the param here, instead of referring to newSource? Would that be clearer? Ah, sorry, you're right. I think your suggestion would be clearer though this is also fine as-is.

Comment on attachment 8670748 [details] MozReview Request: Bug 731025 - Add telemetry for migrator usage and errors, r?MattN Bug 731025 - Add telemetry for migrator usage and errors, r?MattN

FWIW, I did not retest extensively, but I believe this addresses your review comments, and that the check for FXREFRESH is correct (if a little overzealous, perhaps; see also https://dxr.mozilla.org/mozilla-central/rev/1f4cf75c894862cf3634d6014d8de9c807a054a7/toolkit/xre/nsAppRunner.cpp#4142 )

Comment on attachment 8670748 [details] MozReview Request: Bug 731025 - Add telemetry for migrator usage and errors, r?MattN https://reviewboard.mozilla.org/r/21445/#review19421 ::: browser/components/migration/content/migration.js:503 (Diff revision 2) > Cc["@mozilla.org/consoleservice;1"] > .getService(Ci.nsIConsoleService) > .logStringMessage("some " + type + " did not successfully migrate."); > + Services.telemetry.getKeyedHistogramById("FX_MIGRATION_ERRORS") > + .add(this._source, Math.log2(numericType)); It seems like you may have done this already: I think it would be useful to audit the migrators to make sure this counts all "useful" errors and isn't counting expected errors. ::: toolkit/components/telemetry/Histograms.json:4184 (Diff revision 2) > + "releaseChannelCollection": "opt-out", Please request privacy review for opt-out.

(In reply to Matthew N. [:MattN] (PTO Oct. 8-9) from comment #30) > ::: toolkit/components/telemetry/Histograms.json:4184 > (Diff revision 2) > > + "releaseChannelCollection": "opt-out", > > Please request privacy review for opt-out. Benjamin, do I need to file a separate bug for this? How does this work for telemetry changes?

You don't need a separate bug. See the red note at the top of https://developer.mozilla.org/en-US/docs/Mozilla/Performance/Adding_a_new_Telemetry_probe

Not tracking this bug. We won't block the release for a telemetry issue (and we didn't for 41).

Note for the future: all telemetry probes need to be reviewed, not just opt-out ones. Standard questions: * What are the questions that these probes are designed to address? Is this the minimum amount of data necessary to answer those questions? * Who is responsible for monitoring the data? How often will they do it? Do the necessary monitoring dashboards already exist or who is going to create them? * What's the user value? I think I can already see the user value in making sure our migrators keep working, but I want to make sure the questions and monitoring plan actually provide that user value.

(In reply to Benjamin Smedberg [:bsmedberg] from comment #34) > Note for the future: all telemetry probes need to be reviewed, not just > opt-out ones. > > Standard questions: > * What are the questions that these probes are designed to address? Is this > the minimum amount of data necessary to answer those questions? - How many people use the migrator (how often), both on startup and via other entries -- for the other entries, what data are people more/less interested in migrating (is it even worth offering options) - How successful is migration in the wild (how many users encounter errors) -- how do those numbers change when looking at specific browsers; - how does migrator usage (from which browsers?) correlate with retention etc. ? I believe that the 5 probes described in comment 14 cover pretty much exactly those questions. I don't know that I could think of a usable different way of gathering this data. > * Who is responsible for monitoring the data? How often will they do it? Do > the necessary monitoring dashboards already exist or who is going to create > them? AIUI the monitoring for the error rate and so on will be automatic with histogram alerts. After the first release including this telemetry I expect we will investigate the results immediately to focus potential efforts to improve error rates and/or reporting per browser (if they lead us to suspect we're dropping errors on the floor (still)). Regarding the migrator usage, I expect the owners of the given entry points will want to investigate how much value the added UI is bringing people, as well as whether we might want other importers (in the Library, perhaps we could import from online services, or for passwords, perhaps from LastPass or other non-browser password stores). Generally, I expect the people responsible for investigating/monitoring this will be product/UX for the different entrypoints, and Firefox UX for the general migration/retention correlation. > * What's the user value? I think I can already see the user value in making > sure our migrators keep working, but I want to make sure the questions and > monitoring plan actually provide that user value. Basically, the primary aim is ensuring our migrators work in the wild. There are known cases where specific applications (e.g. Skype, see bug 1194692 comment 15 and onwards) break our migration code in unexpected ways. There are likely a lot more that are currently escaping our notice. Furthermore, we're doing increasingly volatile stuff to migrate e.g. passwords (on win8+ for IE, and for Edge, we use undocumented Windows vault APIs) and because it is not possible for us to test these exhaustively, the error reporting through telemetry will help us ensure that they continue working in the face of changing applications and operating systems. This has obvious user value in that working migrators > broken migrators. :-) Secondary aims would include having data to guide decisions about where to focus improving the migrators (they are not complete, see https://wiki.mozilla.org/QA/Firefox_migrators ), as well as whether additional entrypoints or a more streamlined UX are possible/desirable to improve retention if there is indeed correlation with migrator usage (see also what Safari has been doing recently on OS X). This is mostly focused on our retention / market share, but has user value in that a better migration experience (without Apple's Dark Pattern to steal default browser status unexpectedly) is likely to make the user experience better for people that start using (or perhaps go back to using) Firefox. Benjamin, does this adequately answer your questions? :-)

> - How many people use the migrator (how often), both on startup and via > other entries Note: the telemetry dashboards can tell you how many browser sessions have use the migrator, but it will not tell you how many users have used the migrator. I don't think that actually matters in this case, but you should be aware of the difference. > - how does migrator usage (from which browsers?) correlate with retention > etc. ? No existing dashboard can provide this information. If this is important, who will be developing this analysis? > > * Who is responsible for monitoring the data? How often will they do it? Do > > the necessary monitoring dashboards already exist or who is going to create > > them? > > AIUI the monitoring for the error rate and so on will be automatic with > histogram alerts. There is no email address associated with the histograms. Who should be receiving and acting on any alerts? They should be added to the "alert_emails" in the histogram. Also, the existing alerting mechanism is design for performance-type data and may not trigger on tiny changes to error rates such as those being measured here. You should talk to Katie's team to test whether the current alerts actually work before relying on them. The denominator to measure error rates should really be the number of attempted migrations, not the total number of sessions. > After the first release including this telemetry I expect > we will investigate the results immediately to focus potential efforts to > improve error rates and/or reporting per browser (if they lead us to suspect > we're dropping errors on the floor (still)). File a followup bug on this? > or other non-browser password stores). Generally, I expect the people > responsible for investigating/monitoring this will be product/UX for the > different entrypoints, and Firefox UX for the general migration/retention > correlation. This needs specific names. Who in particular will be monitoring this, and have they agreed to monitor this regularly (every 6 weeks) forever?

(In reply to Benjamin Smedberg [:bsmedberg] from comment #36) > > - How many people use the migrator (how often), both on startup and via > > other entries > > Note: the telemetry dashboards can tell you how many browser sessions have > use the migrator, but it will not tell you how many users have used the > migrator. I don't think that actually matters in this case, but you should > be aware of the difference. Fair enough. I agree that it's unlikely to be too important. > > - how does migrator usage (from which browsers?) correlate with retention > > etc. ? > > No existing dashboard can provide this information. If this is important, > who will be developing this analysis? I don't think it's amazingly important, but it would be useful. Is there literally nothing in telemetry that tells us anything about browser session length or frequency? Dolske, can you clarify the importance here? > > > * Who is responsible for monitoring the data? How often will they do it? Do > > > the necessary monitoring dashboards already exist or who is going to create > > > them? > > > > AIUI the monitoring for the error rate and so on will be automatic with > > histogram alerts. > > There is no email address associated with the histograms. Who should be > receiving and acting on any alerts? They should be added to the > "alert_emails" in the histogram. > > Also, the existing alerting mechanism is design for performance-type data > and may not trigger on tiny changes to error rates such as those being > measured here. You should talk to Katie's team to test whether the current > alerts actually work before relying on them. The denominator to measure > error rates should really be the number of attempted migrations, not the > total number of sessions. I talked to gfritzsche. From what you and he said, it seems like the alerts are unlikely to help us at the moment, so I will commit to monitoring the data manually (or handing that off to someone else) for now. I've filed bug 1214159 to cover this aspect. > > After the first release including this telemetry I expect > > we will investigate the results immediately to focus potential efforts to > > improve error rates and/or reporting per browser (if they lead us to suspect > > we're dropping errors on the floor (still)). > > File a followup bug on this? Filed bug 1214154. > > or other non-browser password stores). Generally, I expect the people > > responsible for investigating/monitoring this will be product/UX for the > > different entrypoints, and Firefox UX for the general migration/retention > > correlation. > > This needs specific names. Who in particular will be monitoring this, and > have they agreed to monitor this regularly (every 6 weeks) forever? I'm not sure why this would need to happen every 6 weeks. The error monitoring will be continuous. I don't know that assessing the importance of improving the UX here will happen as often (seems unlikely), but that doesn't let us remove the probes... Perhaps it makes more sense to ignore this aspect as far as the privacy review goes? Sorry if I muddied the waters by bringing it up.

(In reply to :Gijs Kruitbosch from comment #37) > > > - how does migrator usage (from which browsers?) correlate with retention > > > etc. ? > > > > No existing dashboard can provide this information. If this is important, > > who will be developing this analysis? > > I don't think it's amazingly important, but it would be useful. Is there > literally nothing in telemetry that tells us anything about browser session > length or frequency? > > Dolske, can you clarify the importance here? I don't think this needs any kind of special dashboard or alerts. We're trying to understand basic impact of migrator usage as a first step, and manual examination of the data by Bryan Clark and/or I is fine. No need to overengineer this or make it burdensome.

Vladan, can you help get the privacy/data collection side of this unstuck? Is comment #38 enough and can I just go ahead and land this?

Sorry for the delay. Gijs and I talked, and we're going to land this now to expire in 49 (August of next year). Once Bryan gets back from PTO, if he wants to commit to monitoring this permanently we can make that change later. f+ based on that.

Comment on attachment 8670748 [details] MozReview Request: Bug 731025 - Add telemetry for migrator usage and errors, r?MattN It makes me sad but realistically, I don't think this is upliftable into RC. Approval Request Comment [Feature/regressing bug #]: n/a [User impact if declined]: We have no idea if migrators are working or not working [Describe test coverage new/current, TreeHerder]: not really applicable, these are telemetry probes [Risks and why]: medium-low. The telemetry itself is risk-free, but to add it we tweaked some callpaths for creating the migration dialog which in principle carries risk. I checked for add-on callers and I don't *think* anyone is impacted, but it is in principle possible that the change will catch people out. [String/UUID change made/needed]: nope.

Comment on attachment 8670748 [details] MozReview Request: Bug 731025 - Add telemetry for migrator usage and errors, r?MattN Approved for uplift to aurora. Landed ok on m-c, and this gives us some time to catch issues while 43 is still in aurora next week.

Looks like in the past few days, all the collected FX_MIGRATION_ENTRY_POINT is MIGRATION_ENTRYPOINT_UNKNOWN(0).

(In reply to Hector Zhao [:hectorz] from comment #46) > Looks like in the past few days, all the collected FX_MIGRATION_ENTRY_POINT > is MIGRATION_ENTRYPOINT_UNKNOWN(0). I'll look into this.

I started a dashboard at https://sql.telemetry.mozilla.org/dashboard/fx_migration Feel free to add more charts