Closed Bug 1794617 Opened 2 years ago Closed 2 years ago

add telemetry with targeting for "has import of each of bookmarks, history and password ever happened in this profile"

Categories

(Firefox :: Messaging System, enhancement, P2)

enhancement

Tracking

()

VERIFIED FIXED
113 Branch
Iteration:
113.2 - Mar 27 - Apr 7
Tracking Status
firefox113 --- verified

People

(Reporter: dmosedale, Assigned: mconley)

References

(Blocks 3 open bugs)

Details

Attachments

(3 files)

We want to get something that's both more correct and easier to analyze. Bookmarks are the priority here, but the hope is that doing all three will be fairly straightforward.

Leif, you mentioned you had some relevant documentation here and could help define exactly what's likely to be most helpful to data science?

Flags: needinfo?(loines)

I've added the docs for "Import 2.0", which discusses this somewhat. I'll add some more details from the recent meeting as well.

Just to clarify, that this is the "pref telemetry"

Iteration: 107.2 - Oct 3 - Oct 14 → 108.1 - Oct 17 - Oct 28

After chatting with Leif and looking at https://mozilla-hub.atlassian.net/browse/FXE-84?focusedCommentId=600071 (telemetry point number 2), I think what has been discussed so far is three preferences that default to false (one each for bookmarks, history, and passwords). Then, when an import of that category is done, the pref will be set to true.

These could be used as a targeting criteria for an experiment (or showing a dialog) since we can use the value of prefs for that, though we'd want to think about frequency capping for the case where someone doesn't want to import anything, since such targetting would never become true for them.

See Also section of this bug links to a Import 2.0 spec, and the Telemetry section of that page (which appears to be in very early stages) proposes having three pieces of "total bookmarks/history/password items imported". We could easily do that here, and then we'd just look at whether one of these prefs is zero or non-zero instead of using a boolean pref, and I'm inclined to do it. That said, Gijs, do you have any thoughts on this, given that that Telemetry section doesn't seem to have gotten a lot of attention yet?

Leif proposed putting these in the Telemetry Environment. According to that page,

Changes to most of these data points are detected (where possible and sensible) and will lead to a session split in the “main” ping.

:loines, is that the behavior you would want with this telemetry?

Flags: needinfo?(gijskruitbosch+bugs)

TBH I'm sorta confused about what motivated this and what question you need answered from me. The JIRA ticket helped, but the comment could have done with more context...

The counts of bookmarks/history/bookmarks imported are already recorded, I believe, albeit in histograms because that telemetry is old. It's FX_MIGRATION_BOOKMARKS_QUANTITY and friends. Same thing for error counts. If data science is happy there's no data continuity problem there then I'm happy to review patches to change those to telemetry events or scalars if that's preferred, I guess?

(In reply to Dan Mosedale (:dmosedale, :dmose) from comment #3)

See Also section of this bug links to a Import 2.0 spec, and the Telemetry section of that page (which appears to be in very early stages) proposes having three pieces of "total bookmarks/history/password items imported". We could easily do that here, and then we'd just look at whether one of these prefs is zero or non-zero instead of using a boolean pref, and I'm inclined to do it. That said, Gijs, do you have any thoughts on this, given that that Telemetry section doesn't seem to have gotten a lot of attention yet?

This is also confusing - are we talking about prefs or telemetry? Why would we store counts of imported data in prefs?

Maybe this needinfo should have gone to Ania? I don't know what kind of my thoughts you're looking for. ;-)

Flags: needinfo?(gijskruitbosch+bugs) → needinfo?(dmosedale)

(In reply to :Gijs (he/him) from comment #4)

TBH I'm sorta confused about what motivated this and what question you need answered from me. The JIRA ticket helped, but the comment could have done with more context...

Apologies; I definitely could have done better there.

The counts of bookmarks/history/bookmarks imported are already recorded, I believe, albeit in histograms because that telemetry is old. It's FX_MIGRATION_BOOKMARKS_QUANTITY and friends. Same thing for error counts. If data science is happy there's no data continuity problem there then I'm happy to review patches to change those to telemetry events or scalars if that's preferred, I guess?

a) I'm guessing what we would want to do is implement new probes and convince ourselves that they're generating correct data on the release channel before removing the old ones.

See Also section of this bug links to a Import 2.0 spec, and the Telemetry section of that page (which appears to be in very early stages) proposes having three pieces of "total bookmarks/history/password items imported". We could easily do that here, and then we'd just look at whether one of these prefs is zero or non-zero instead of using a boolean pref, and I'm inclined to do it. That said, Gijs, do you have any thoughts on this, given that that Telemetry section doesn't seem to have gotten a lot of attention yet?

This is also confusing - are we talking about prefs or telemetry? Why would we store counts of imported data in prefs?

b) The idea is to collect this information in telemetry in a way that would be useful for various things, including sizing experiments or messaging rollouts, and also useful for targeting experiments or messaging system messages. Since it's easy to target experiments against the value of the pref, the idea is to write the data to a pref when it changes, and then use that pref as backing store to send the telemetry. For this particular case, since we don't have any specific plans to target against "number of items imported", one could imagine making the telemetry and targeting separate, but it would be more work and more complex, and we'll already be collecting both kinds of data in telemetry anyway, so it's not clear to me why we would want to do that.

Maybe this needinfo should have gone to Ania? I don't know what kind of my thoughts you're looking for. ;-)

I'll hold off on needinfo-ing you until :loines has verified that comment 3 and comment 5 (this comment) describes a system that does what DS wants, or, if not, how a system that DS wants would be different...

Flags: needinfo?(dmosedale)
Blocks: 1279589
Iteration: 108.1 - Oct 17 - Oct 28 → ---
Summary: add basic modern telemetry for bookmark, history and password import → add basic modern telemetry with targeting for bookmark, history and password import

:gijs made an excellent suggestion in Slack:

anyway, I think the new migration dialog (or if we're adding telemetry before the new dialog, that new telemetry) should probably have a single event that fires when the dialog is closed, that includes indications of how much data of what types was imported (bearbeitet)

Summary: add basic modern telemetry with targeting for bookmark, history and password import → add telemetry with targeting for "has import of each of bookmarks, history and password ever happened in this profile"

So from my understanding product wants to target clients w/ messages who have never imported their browser data. When clients DO import, we record histogram telemetry during the session that the import was completed, but this information is not persisted on the client in a... persistent way that could be used for targeting.

One way (perhaps not the only way) to make "clients who have ever imported" a targetable population would be to durably set a pref on the client when they have successfully imported data. We could then report this in telemetry for sizing purposes, and then directly target on it for messaging purposes.

Another option which I AM NOT SURE IS POSSIBLE would be to use the data we have already collected and maintain a server-side list of clients that have reported at some point that they have imported. We MAY be able to use this list for targeting. Whether or not this is possible, it will be a less accurate way to target than using the prefs. The prefs would be the ground-truth for that particular client. The server-side list would not, because some clients share the same client_id; so when we targeted based on that list, we would target potentially more clients than we would want.

There may be other options that I am not aware of.

Flags: needinfo?(loines)

Gijs pointed out today that preferences don't generally survive profile refreshes. He also pointed out that it is possible to something kinda gross to make a specific pref (or three, in this case) survive refreshes, with https://searchfox.org/mozilla-central/rev/3c194fa1d6f339036d2ec9516bd310c6ad612859/browser/components/migration/FirefoxProfileMigrator.sys.mjs#261-284 being an example. He did, however, point out that we should use IOUtils instead of OS.File and see if we can avoid additional file reads in our hook, if we choose to go this direction.

Together, Gijs and Leif concluded that it may be possible to avoid doing this if we're willing to accept some amount of error. Leif agreed to do a bit of research into how much error this would be, and how this would (at a high level) change what targeting we needed to use in experiments so that we can come to a decision about which way to move forward. Leif, if you have any particular recommendation once you've got this info, I'd interested in hearing about it too...

Giving Leif a needinfo to put this data here along with any thoughts in this bug...

Thank you!

Flags: needinfo?(loines)
Severity: N/A → S3
Type: defect → enhancement

So about 2% of new clients in the last 6 months report undergoing a profile reset. That is slightly higher than my gut was expecting. Assuming that importing data from other browsers and resetting your profile are more less independent (they may not be), and we elected NOT to persist the "ever imported" pref on a reset, that would mean potentially over-targeting ~10's of k per million new clients that we identify for messaging over the course of half a year.

This is a product decision, but it may be that we would prefer making the code a bit more complex in exchange for saving these clients from being redundantly messaged, under the assumption that the extra complexity is of the less risky variety (I can't evaluate that).

Flags: needinfo?(loines)

Venetia, what do you think?

Flags: needinfo?(vtay)
Iteration: --- → 109.2 - Nov 28 - Dec 9
Priority: P2 → P1

To clarify, if they did a profile reset; they will be back to zero bookmarks/history etc. If this is the case, I think it is OK to re-message them as it could be potentially useful?

Flags: needinfo?(vtay)

(In reply to Venetia from comment #11)

To clarify, if they did a profile reset; they will be back to zero bookmarks/history etc. If this is the case, I think it is OK to re-message them as it could be potentially useful?

I don't understand this comment. Firefox refresh/reset keeps bookmarks+history. Are you talking about something else?

Flags: needinfo?(vtay)

Yes - that was what I was looking for clarification on - what happens to bookmarks + history when Firefox refresh/reset; thank you

@Dan let's chat when you are back

Flags: needinfo?(vtay) → needinfo?(dmosedale)

Notes to self:

Depending on what we implement here, once the telemetry/pref thing ships, it will only be guaranteed correct for profiles that were created with that version or newer. We may also want to add, at that same time, info & targeting about what version a profile was created with (if that info isn't already in the profile). Then we could at least target that subset of users. But for quite a while, that will still be the minority of profiles.

There may be ways to avoid the above problem; I need to do some investigation of the stuff discussed in comment 4, comment 5, and comment 7, at including FX_MIGRATION_BOOKMARKS_QUANTITY.

Flags: needinfo?(dmosedale)
Priority: P1 → P2
Iteration: 109.2 - Nov 28 - Dec 9 → 110.1 - Dec 12 - Dec 23
Iteration: 110.1 - Dec 12 - Dec 23 → 110.2 - Dec 26 - Jan 6
Iteration: 110.2 - Dec 26 - Jan 6 → 110.3 - Jan 9 - Jan 13
Iteration: 110.3 - Jan 9 - Jan 13 → 111.1 - Jan 16 - Jan 27
Iteration: 111.1 - Jan 16 - Jan 27 → 111.2 - Jan 30 - Feb 10
Iteration: 111.2 - Jan 30 - Feb 10 → 112.1 - Feb 13 - Feb 24

Najla and Ed were wondering earlier today whether this telemetry would be able to answer the question "of the X% of users who had import checked in the about:welcome screen that started the migration, what percentage actually imported data".

This bug isn't yet so tightly scoped as for the answer to that to be obvious. One could imagine supporting that, perhaps in concert with (something like?) FX_MIGRATION_ENTRY_POINT.

That said, Venetia and I were talking today, and since we want to see this particular bug implemented sooner rather than later, the scope needs to be clarified, and we need to be careful how much we allow into it. I somewhat suspect we'll want to do it as a separate bug.

Noting this here so that we decide whether to do it here or spin it off as the scope gets clarified, and needinfo-ing myself to do that clarification.

Flags: needinfo?(dmosedale)

More context for the above use case from Najla:

fwiw i think that having this insight would be helpful because we're starting to think about how we can make about:welcome more contextual based on users' actions. for example https://mozilla-hub.atlassian.net/browse/FXE-58

(In reply to Dan Mosedale (:dmosedale, :dmose) from comment #15)

Najla and Ed were wondering earlier today whether this telemetry would be able to answer the question "of the X% of users who had import checked in the about:welcome screen that started the migration, what percentage actually imported data".

This bug isn't yet so tightly scoped as for the answer to that to be obvious. One could imagine supporting that, perhaps in concert with (something like?) FX_MIGRATION_ENTRY_POINT.

I mean, to be fair, you could do exactly that with the existing telemetry histograms right? I thought the "point" of this bug was to ensure we could target experiments rather than changing the existing telemetry?

Iteration: 112.1 - Feb 13 - Feb 24 → 112.2 - Feb 27 - Mar 10

:Gijs, you're correct, things have gotten somewhat conflated here. By adding extra entry points to the existing enumeration, and modifying some (at least) some messaging-system code and some about:welcome code, we could do this with the existing telemetry. I suspect this wants to be spun off to a separate bug; we'll be talking about this in a meeting tomorrow (Thursday, March 2nd) which I hope will help get this sorted out (by setting some clearer limits around the boundaries of what this bug is for).

Iteration: 112.2 - Feb 27 - Mar 10 → 113.1 - Mar 13 - Mar 24

What's the current state of this work?

See Also: → 1824034

I've spun off the about:welcome entry point work to bug 1824034 so that we can get this bug focused again, and move it forward.

I believe there were two points to this bug. One was:

  1. be able to target clients that have never imported (bookmarks|history|passwords) before
  2. be able to get telemetry that reflects clients in the field that can be targeted that way so that such experiments can be more easily sized

At a semi-recent meeting, it was clarified that we'd like to get 1 in the tree sooner rather than later, and we're happy to delay 2 if that makes it easier.

Venetia, can you confirm (or not!) that what I've written in this comment correctly represents product's desires?

Flags: needinfo?(dmosedale) → needinfo?(vtay)

Gijs, I think the above comments mostly clarify the state of this bug. I'm assuming you're asking because of the overlap between this and bug 1824010 (which appears to be aimed at 114)?

The trick around point 1, of course, is that it starts out not being very useful (because few profiles will have yet been created in a way that matches that targeting), and its utility grows over time as an increasing percentage of our profiles has been created by a new-enough version.

(Assuming Venetia confirms my previous comment), our goal here is to get point 1 landed sooner rather than later, regardless of what body of work it lands as a part of.

Assignee: dmosedale → nobody

(In reply to Dan Mosedale (:dmosedale, :dmose) from comment #21)

Gijs, I think the above comments mostly clarify the state of this bug. I'm assuming you're asking because of the overlap between this and bug 1824010 (which appears to be aimed at 114)?

Yes.

The trick around point 1, of course, is that it starts out not being very useful (because few profiles will have yet been created in a way that matches that targeting), and its utility grows over time as an increasing percentage of our profiles has been created by a new-enough version.

(Assuming Venetia confirms my previous comment), our goal here is to get point 1 landed sooner rather than later, regardless of what body of work it lands as a part of.

Makes sense, thanks.

Blocks: 1824010
See Also: → 1824010

I'm going to take a crack at this.

Assignee: nobody → mconley

These preferences are sent via Telemetry through TelemetryEnvironment.

For the FirefoxProfileMigrator, which is used for profile resets, we don't
set these prefs during the migration of those resources. Instead, during
OTHERDATA, we examine the prefs.js of the old profile for the prefs and
attempt to migrate them to a new prefs file in the new profile.

Attachment #9324779 - Flags: data-review?(jhirsch)

I will follow-up the most recent patch with another one that will add the Nimbus / ASRouter targeting.

Yup that would be great! Thank you!

Flags: needinfo?(vtay)
Iteration: 113.1 - Mar 13 - Mar 24 → 113.2 - Mar 27 - Apr 7

Comment on attachment 9324779 [details]
Data collection request.md

Data-review+, ship it!


  1. Is there or will there be documentation that describes the schema for the ultimate data set in a public, complete, and accurate way?

Yes, the usual firefox data docs at https://probes.telemetry.mozilla.org/.

  1. Is there a control mechanism that allows the user to turn the data collection on and off?

Yes, the usual desktop firefox telemetry opt-out controls in about:preferences.

  1. If the request is for permanent data collection, is there someone who will monitor the data over time?

Yes, mconley will monitor.

  1. Using the category system of data types on the Mozilla wiki, what collection type of data do the requested measurements fall under?

Category 2, interaction data.

  1. Is the data collection request for default-on or default-off?

Default on.

  1. Does the instrumentation include the addition of any new identifiers (whether anonymous or otherwise; e.g., username, random IDs, etc. See the appendix for more details)?

No.

  1. Is the data collection covered by the existing Firefox privacy notice?

Yes.

  1. Does the data collection use a third-party collection tool?

No.

Attachment #9324779 - Flags: data-review?(jhirsch) → data-review+
Pushed by mconley@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/1e4b6f3adc98 Record interaction preferences when bookmarks, history, or passwords are imported. data-review=jhirsch,r=Gijs
Blocks: 1825632
Pushed by mconley@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/67998f2130b9 Add ASRouter targeting for whether or not the user has ever migrated bookmarks, history, or passwords. r=dmose,omc-reviewers
Status: NEW → RESOLVED
Closed: 2 years ago
Keywords: leave-open
Resolution: --- → FIXED
Target Milestone: --- → 113 Branch

I‘ve verified this enhancement using the latest Firefox Nightly 113.0a1 (Build ID: 20230404134922) on Windows 10 x64, macOS 11.7.1, and Linux Mint 20 x64.

  • The “hasMigratedBookmarks”, “hasMigratedHistory”, and “hasMigratedPasswords” targeting exist in the ASRouter devtool.
    • They have the “false” value if no import has occurred and the “true” value when bookmarks, history, and passwords have been imported.
  • The “browser.migrate.interactions.bookmarks”, “browser.migrate.interactions.history” and “browser.migrate.interactions.passwords” pref exist in the “about:config” page.
    • They are set to “false” as default.
    • After importing history, bookmarks, and passwords, their value is set to “true”
  • Also, the value of the targeting attributes and prefs remains unchanged after resetting the profile.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: