1305878 - Add sync client counts and profile info to UITour getConfiguration

Status: RESOLVED WONTFIX

(Firefox :: Tours, defect)

Description

[Osmose [:osmose, :mkelly]]

cbeard has asked mgrimes to put out a promotion for Firefox for Android to our release users by the end of the year via SHIELD. The original plan was to send the message out to a large percentage of our users, but to avoid bugging tons of users who may not care about the message, mgrimes wants to target a smaller portion of users based on:

- The number of mobile/desktop devices they have set up with sync (services.sync.clients.devices.desktop, services.sync.clients.devices.mobile, and services.sync.numClients)
- The date their profile was created (environment.profile.creationDate in telemetry)
- The number of "activeTicks" we've tracked; roughly, this translates to the amount of time the user has been actively using Firefox (simpleMeasurements.activeTicks in the main telemetry payload)

All of these pieces of data are either in existing prefs, or in the main telemetry packet. In order to allow filtering on them by SHIELD (before the system add-on lands, which won't be in time for the promo), we'd like to make this data accessible via UITour. Then, the existing SHIELD service/code can access the data and filter on it.

Comment 1

[Matt Grimes [:Matt_G]]

I spoke to rweiss this afternoon. She also mentioned the list of add-ons + state. For example, for users that have an adblocker we could inform them that could have that same ad-blocked experience on mobile as well. Installing add-ons is also likely an indicator that the user is invested in our products and may be interested in hearing about more of our offerings.

Comment 2

[Osmose [:osmose, :mkelly]]

Attachment: Bug 1305878 - Add sync client counts to UITour.

Amends UITour's getConfiguration function to include "desktopDevices",
"mobileDevices", and "totalDevices" counts (pulled from the Sync
preferences services.sync.clients.devices.desktop,
services.sync.clients.devices.mobile, and services.sync.numClients
respectively) if they are available.

Also adds a new type of configuration called "profileInfo" that
includes "activeTicks" and "creationDate" properties describing the
active time a user has spent with the client and the date the current
profile was created. Both of these values are taken from the "main"
Telemetry ping (mainPing.payload.simpleMeasurements.activeTicks and
mainPing.environment.profile.creationDate respectively).

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

Comment 3

[Osmose [:osmose, :mkelly]]

Comment on attachment 8795559 [details]
Bug 1305878 - Add sync client counts to UITour.

I can't figure out how to get ReviewBoard to do a feedback request, so for now I'm using Bugzilla flags.

This is a WIP patch that adds the data I described in Comment #1 to UITour.

Asking for feedback from:
- MattN: Both general feedback on the patch / request, and specifically, I'm stuck figuring out how to get Telemetry working properly for my tests for the profileInfo config.

- rweiss: Privacy feedback / review.

Comment 4

[Osmose [:osmose, :mkelly]]

(In reply to Matt Grimes [:Matt_G] from comment #1)
> I spoke to rweiss this afternoon. She also mentioned the list of add-ons +
> state. For example, for users that have an adblocker we could inform them
> that could have that same ad-blocked experience on mobile as well.
> Installing add-ons is also likely an indicator that the user is invested in
> our products and may be interested in hearing about more of our offerings.

I'm down with this, but maybe for a followup patch? Working through the stuff we have in this patch will probably take some time, as will requesting uplift, and I'd be more comfortable getting through that process with this patch and then doing it again for subsequent data we want than with batching it all together.

Oh yeah, I forgot to mention that we hope to uplift this to Beta after merging to get it to release before the end of the year. I checked with Ritu to see if it was worth trying to get this patch uplifted and she thought we had a good justification (although that is not actual approval, so we'll see!).

Comment 5

[Osmose [:osmose, :mkelly]]

Comment on attachment 8795559 [details]
Bug 1305878 - Add sync client counts to UITour.

Gijs has graciously offered to take a look at this patch. :D

Comment 6

[:Gijs (he/him)]

Comment on attachment 8795559 [details]
Bug 1305878 - Add sync client counts to UITour.

https://reviewboard.mozilla.org/r/81572/#review81472

::: browser/components/uitour/UITour.jsm:1867
(Diff revision 1)
> +        TelemetryArchive.promiseArchivedPingList().then(pings => {
> +          let mainPing = pings.find(ping => ping.type === "main");
> +          return TelemetryArchive.promiseArchivedPingById(mainPing.id);
> +        }).then(mainPing => {

Please add a comment indicating that promiseArchivedPingList doesn't actually return 'real' ping data, and don't call the promise return value `pings`, but maybe something like `pingDescriptions` or whatever.

Note also that the documentation here:

https://dxr.mozilla.org/mozilla-central/source/toolkit/components/telemetry/TelemetryArchive.jsm#29-40

suggests that you should wait for `promiseInitialized` (presumably the one on TelemetryController).

::: browser/components/uitour/UITour.jsm:1872
(Diff revision 1)
> +        TelemetryArchive.promiseArchivedPingList().then(pings => {
> +          let mainPing = pings.find(ping => ping.type === "main");
> +          return TelemetryArchive.promiseArchivedPingById(mainPing.id);
> +        }).then(mainPing => {
> -        this.sendPageCallback(aMessageManager, aCallbackID, {
> +          this.sendPageCallback(aMessageManager, aCallbackID, {
> -          setup: Services.prefs.prefHasUserValue("services.sync.username"),
> +            activeTicks: mainPing.payload.simpleMeasurements.activeTicks,

Are we convinced `activeTicks` is useful? I've heard rumours that it's not particularly trustworthy. (No, I don't have an awful lot more detail right now - I can ask, if necessary...)

::: browser/components/uitour/UITour.jsm:1875
(Diff revision 1)
> +        }).catch(error => {
> +          this.sendPageCallback(aMessageManager, aCallbackID, {});
>          });

The order here is odd. This will call sendPageCallback a second time if the first call to sendPageCallback in the `.then` callback fails.

Instead, IMO it would make more sense to use `.then` with two callbacks, which means the error handler will only be called when the original set of promises is rejected (and not if `sendPageCallback` fails, which is not a case we cover anywhere else).

I'd also expect that an error here is unusual and should be reported. You can use `Cu.reportError()` for this.

::: browser/components/uitour/UITour.jsm:1884
(Diff revision 1)
> +          data.desktopDevices = Services.prefs.getIntPref("services.sync.clients.devices.desktop");
> +          data.mobileDevices = Services.prefs.getIntPref("services.sync.clients.devices.mobile");
> +          data.totalDevices = Services.prefs.getIntPref("services.sync.numClients");
> +        } catch (ex) {}
> +        this.sendPageCallback(aMessageManager, aCallbackID, data);

For profiles that only have desktop clients we'll now miss total clients.

I would suggest using Preferences.jsm, and then using:

data.desktopDevices = Preferences.get("services.sync.clients.devices.desktop", 0);

etc.

instead of the try...catch.

::: browser/components/uitour/test/browser_UITour_profileInfo.js:12
(Diff revision 1)
> +Cu.import("resource://gre/modules/TelemetryController.jsm", this);
> +Cu.import("resource://gre/modules/TelemetryStorage.jsm", this);
> +
> +add_task(setup_UITourTest);
> +
> +// TODO: How to set up Telemetry for these tests?

Seems like this should be gone.

::: browser/components/uitour/test/browser_UITour_profileInfo.js:14
(Diff revision 1)
> +add_UITour_task(function* test_checkProfileInfo_missingPing() {
> +  let result = yield getConfigurationPromise("profileInfo");
> +  is(Object.keys(result).length, 0, "profileInfo should be empty by default");
> +});

Can we actually depend on this being the case (ie there being 0 pings in telemetry etc.) ?

::: browser/components/uitour/test/browser_UITour_profileInfo.js:20
(Diff revision 1)
> +  yield TelemetryController.submitExternalPing("main", {
> +    payload: 'does not match expected payload'
> +  });

Does this (and the analogous call later) actually submit anything? Comments here would be helpful.

::: browser/components/uitour/test/browser_UITour_profileInfo.js:23
(Diff revision 1)
> +  let result = yield getConfigurationPromise("profileInfo");
> +  is(Object.keys(result).length, 0, "profileInfo should be empty");

Is this true even if length was previously not 0?

On the whole, it feels like the last task here is the only one that really would be reliable. For the first ones, you'd need a way to clear out the telemetry storage. If it's not obvious how to do that, I would hope that :Dexter or :gfritzsche can help.

::: browser/components/uitour/test/browser_UITour_sync.js:34
(Diff revision 1)
> +add_UITour_task(function* test_checkSyncCounts_missing() {
> +  Services.prefs.clearUserPref("services.sync.clients.devices.desktop");
> +  Services.prefs.clearUserPref("services.sync.clients.devices.mobile");
> +  Services.prefs.clearUserPref("services.sync.numClients");

This should be unified with the previous task (and the one before that) and should test that if only desktop and the number of clients is set, the data for those clients is still produced, rather than getting rid of all of it.

Comment 7

[:Gijs (he/him)]

Comment on attachment 8795559 [details]
Bug 1305878 - Add sync client counts to UITour.

For the actual review, it might be a good idea to split up the two patches, as the sync and telemetry changes are, AFAICT, orthogonal to each other.

Comment 8

[Osmose [:osmose, :mkelly]]

Comment on attachment 8795559 [details]
Bug 1305878 - Add sync client counts to UITour.

https://reviewboard.mozilla.org/r/81572/#review81472

> Are we convinced `activeTicks` is useful? I've heard rumours that it's not particularly trustworthy. (No, I don't have an awful lot more detail right now - I can ask, if necessary...)

The request for that value came from Strategy & Insights, so they at least think it's trustworthy, but that's not really a strong assertion. Who would be a good person to ask? (The also use it for their telemetry analysis so it'd be good for them to know this outside of the context of this bug anyway.)

> Seems like this should be gone.

I was actually hoping for advice on this, as I couldn't find an example that I could get working to control the telemetry pings within these tests.

> Is this true even if length was previously not 0?
> 
> On the whole, it feels like the last task here is the only one that really would be reliable. For the first ones, you'd need a way to clear out the telemetry storage. If it's not obvious how to do that, I would hope that :Dexter or :gfritzsche can help.

Yeah, this is incomplete mainly due to the lack of Telemetry clearing/mocking for this test. I'll ask those two for advice on how to do this.

Comment 9

[:Gijs (he/him)]

(In reply to Michael Kelly [:mkelly,:Osmose] from comment #8)
> Comment on attachment 8795559 [details]
> Bug 1305878 - Add sync client counts and profile info to UITour.
> 
> https://reviewboard.mozilla.org/r/81572/#review81472
> 
> > Are we convinced `activeTicks` is useful? I've heard rumours that it's not particularly trustworthy. (No, I don't have an awful lot more detail right now - I can ask, if necessary...)
> 
> The request for that value came from Strategy & Insights, so they at least
> think it's trustworthy, but that's not really a strong assertion. Who would
> be a good person to ask? (The also use it for their telemetry analysis so
> it'd be good for them to know this outside of the context of this bug
> anyway.)

Dolske, am I right in thinking it was you who told me activeTicks wasn't necessarily a reliable indicator of user activity / session length? If so, can you remember where that came from?

Comment 10

[Justin Dolske [:Dolske]]

This is the discussion I saw in #telemetry a few weeks ago, bsmedberg may have more to say.

<digitarald> mreid bsmedberg chutten: is there any documentation on why we choose usage hours over active ticks?
<bsmedberg> there's documentation on why active ticks was broken
<bsmedberg> spread across a bunch of bugs
<bsmedberg> (e.g. video-watching, or basic page-reading)
<chutten> One active tick means one five-second period in which the user, via the OS, gave us a "user interaction"-type event (mouse, keyboard, touch). If you are actively reading a webpage, you're probably not accumulating activeTicks. If you are actively watching a video, you're probably not accumulating activeTicks


But separate from that, I'm wary at the prospect of using UITour to expose telemetry data. That leads to some awkward questions about if it's respecting opt-in/opt-out settings, and if it needs data privacy reviews and such.

Generally we've limited the info exposed by UITour to the minimum required for specific tour functionality. For example, instead providing the exact time the profile was created (to the millisecond?), expose a .olderThan6Months boolean or .ageInMonths integer. Or even do all the data-peeking in the client, and expose a userMightWantMobile boolean. Can these kinds of minimizations be done here?

Comment 11

[Osmose [:osmose, :mkelly]]

(In reply to Justin Dolske [:Dolske] from comment #10)
> <digitarald> mreid bsmedberg chutten: is there any documentation on why we
> choose usage hours over active ticks?

Is usage hours stored in a place that we could fetch it instead of ticks? I imagine that would be more useful for prompt targeting than ticks if we can.

> But separate from that, I'm wary at the prospect of using UITour to expose
> telemetry data. That leads to some awkward questions about if it's
> respecting opt-in/opt-out settings, and if it needs data privacy reviews and
> such.

I'm assuming this particular patch will need a privacy review (Matt_G has already talked informally to rweiss about it and I flagged her on the patch for feedback and will flag for review once it's finished) either way. In any case, if there's a better way to pull this data than from telemetry (like, perhaps, wherever telemetry gets this data in the first place) that helps respect opt-in/opt-out policies, I'm happy to grab them from there. Although intuitively I'd expect pulling the data from telemetry to fail if the user has opted-out (however that works) since the pings presumably wouldn't have been sent at all. But that could easily be wrong.
 
> Generally we've limited the info exposed by UITour to the minimum required
> for specific tour functionality. For example, instead providing the exact
> time the profile was created (to the millisecond?), expose a
> .olderThan6Months boolean or .ageInMonths integer. Or even do all the
> data-peeking in the client, and expose a userMightWantMobile boolean. Can
> these kinds of minimizations be done here?

Matt_G can probably provide suggestions for alternatives like that that will work for S&I, if they're an issue. I was waiting for the privacy review before asking for those alternatives.

Comment 12

[Osmose [:osmose, :mkelly]]

Attachment: Review fixes.

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

Comment 13

[Osmose [:osmose, :mkelly]]

Comment on attachment 8795559 [details]
Bug 1305878 - Add sync client counts to UITour.

https://reviewboard.mozilla.org/r/81572/#review81472

> Please add a comment indicating that promiseArchivedPingList doesn't actually return 'real' ping data, and don't call the promise return value `pings`, but maybe something like `pingDescriptions` or whatever.
> 
> Note also that the documentation here:
> 
> https://dxr.mozilla.org/mozilla-central/source/toolkit/components/telemetry/TelemetryArchive.jsm#29-40
> 
> suggests that you should wait for `promiseInitialized` (presumably the one on TelemetryController).

AFAICT TelemetryController.promiseInitialized doesn't resolve during test runs, and I can't figure out why. Dexter suggested I used TelemetryController.getCurrentPingData, which doesn't seem to need promiseInitialized (I see other pieces of code using it without waiting), so it seemed like a moot point.

Comment 14

[Osmose [:osmose, :mkelly]]

Comment on attachment 8795559 [details]
Bug 1305878 - Add sync client counts to UITour.

Gijs: Sorry if I screwed up adding a commit to the review, I'm struggling with the interface.

I added a commit with fixes from feedback from the review itself and from Dexter on how to fetch data from Telemetry; he suggested I used TelemetryController.getCurrentPingData, which always returns the main ping, which invalidated some of the fedback from the review.

He also pointed me to https://dxr.mozilla.org/mozilla-central/source/browser/base/content/test/general/browser_aboutHealthReport.js as an example of how to set Telemetry up with data before a test, but I'm still unable to get Telemetry to have the proper data during my tests. 

gfritzsche: Could you take a look at this patch and give feedback on how I should pull the data from telemetry and how to test the code pulling that data?

Comment 15

[Osmose [:osmose, :mkelly]]

Attachment: Bug 1305878 - Pull activeTicks and profile creation from the source.

Instead of pulling activeTicks and the profile creation date from the
Telemetry ping, instead we pull them from the places they're tracked,
namely ProfileAge.jsm and SessionRecorder.jsm.

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

Comment 16

[Osmose [:osmose, :mkelly]]

Comment on attachment 8795559 [details]
Bug 1305878 - Add sync client counts to UITour.

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

Comment 17

[Osmose [:osmose, :mkelly]]

Attachment: Bug 1305878 - Add profileInfo to UITour.getConfiguration.

Add a new type of configuration called "profileInfo" that includes
"activeTicks" and "created" properties describing the active time a
user has spent with the client and the date the current profile was
created. These values are provided by ProfileAge.jsm and
SessionRecorder.jsm (via TelemetryController) respectively.

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

Comment 18

[Osmose [:osmose, :mkelly]]

Comment on attachment 8795559 [details]
Bug 1305878 - Add sync client counts to UITour.

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

Comment 19

[Osmose [:osmose, :mkelly]]

Okay. Sorry for the bugspam, I'm riding the mozreview learning curve currently.

Gijs: I split this up as you recommended, and based on feedback I got from others, was able to change the patches to pull the values they need directly instead of going through the telemetry ping. I also set up some mocking so that I could test my code without having to make big modifications to the modules they depend on, which let me finish up the tests. I'm not sure if this is the _best_ way to do things, but it's the best way I can come up with right now. :P

rweiss: I added you as a reviewer since the patch is now complete IMO, and ready for formal privacy approval of exposing the data mentioned in Comment 1.

Let me know if I've messed anything up. :D

Comment 20

[:Gijs (he/him)]

Comment on attachment 8795559 [details]
Bug 1305878 - Add sync client counts to UITour.

https://reviewboard.mozilla.org/r/81572/#review83116

LGTM.

Comment 21

[:Gijs (he/him)]

Comment on attachment 8799086 [details]
Bug 1305878 - Add profileInfo to UITour.getConfiguration.

https://reviewboard.mozilla.org/r/84390/#review83118

It looks like you're adding a separate test, but didn't "hg add" the new test file, so it's not in the changeset and I can't review it.

Comment 22

[Osmose [:osmose, :mkelly]]

Comment on attachment 8799086 [details]
Bug 1305878 - Add profileInfo to UITour.getConfiguration.

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

Comment 23

[Osmose [:osmose, :mkelly]]

Comment on attachment 8799086 [details]
Bug 1305878 - Add profileInfo to UITour.getConfiguration.

https://reviewboard.mozilla.org/r/84390/#review83118

Oops. Updated, sorry about that!

Comment 24

[:Gijs (he/him)]

Comment on attachment 8799086 [details]
Bug 1305878 - Add profileInfo to UITour.getConfiguration.

https://reviewboard.mozilla.org/r/84390/#review83184

This looks sane, thanks!

::: browser/components/uitour/UITour.jsm:1878
(Diff revision 2)
>          });
>          break;
> +      case "profileInfo":
> +        Task.spawn(function*() {
> +          let profileInfo = {
> +            activeTicks: null,

Wouldn't 0 be a more sensible value than null in case of an error? I don't really mind either way, I guess. Maybe check with the www folks who'll be consuming this?

Comment 25

[Osmose [:osmose, :mkelly]]

Comment on attachment 8799086 [details]
Bug 1305878 - Add profileInfo to UITour.getConfiguration.

https://reviewboard.mozilla.org/r/84390/#review83184

> Wouldn't 0 be a more sensible value than null in case of an error? I don't really mind either way, I guess. Maybe check with the www folks who'll be consuming this?

Funnily enough, _I_ am the www folk who will be consuming this. :P

I like null vs 0 because it lets us distinguish between not knowing the activeTicks, or pulling this data right at the start of a session when there haven't been any yet. And also because we can distinguish between the attribute being completely missing, although that's not really a possibility right now.

Comment 26

[[e-mail]]

Benjamin and I had some discussion about areas of concern related to data review and other implementation details related to this patch.  I'd like for him to present the concerning issues in this bug before data review can commence.  Unfortunately we are both out-of-the-office this week, so it makes it hard to coordinate responses.

Comment 27

[Matt Grimes [:Matt_G]]

Adding an additional data steward that may not be out this week.

Comment 28

[Benjamin Smedberg]

Comment on attachment 8795559 [details]
Bug 1305878 - Add sync client counts to UITour.

https://reviewboard.mozilla.org/r/81572/#review83772

::: browser/components/uitour/UITour.jsm:1869
(Diff revision 3)
>          });
>          break;
>        case "sync":
>          this.sendPageCallback(aMessageManager, aCallbackID, {
>            setup: Services.prefs.prefHasUserValue("services.sync.username"),
> +          desktopDevices: Preferences.get("services.sync.clients.devices.desktop", 0),

Because UITour is only basically secured (via HTTPS and not via strict code signing), we want to avoid leaking information which is more attackable. I think this information is ok.

I'm surprised that we don't have in-tree docs for the UITour API. Data review typically happens against doc changes. Can we get an in-tree .rst file that documents the API so that we don't have to do data audits against the code itself?

provisional data-review for this part=me, if you do a followup for docs.

Comment 29

[Benjamin Smedberg]

Comment on attachment 8799086 [details]
Bug 1305878 - Add profileInfo to UITour.getConfiguration.

https://reviewboard.mozilla.org/r/84390/#review83774

::: browser/components/uitour/UITour.jsm:1883
(Diff revision 2)
> +            activeTicks: null,
> +            created: null,
> +          };
> +
> +          try {
> +            profileInfo.activeTicks = yield this._getActiveTicks();

On the other hand, I'm not convinced that this change is acceptable.

Firstly, this appears to be handing out the activeTicks for the *current* session, which doesn't seem like what the spec in the bug calls for. Won't this API be called from somewhere near startup where activeTicks is always near-0?

Secondly, there is no documentation about the format of the profile-created date, but that is very identifying and can be correlated. I do not think we should include this information as-is. Instead we should use very rough ranges (e.g. "today", "less than one week", "less than two months", "less than one year", ">one year") or at a minimum round this to an interval like weeks.

Thirdly (least important), activeTicks is a crazy implementation-specific metric which doesn't have a unit. Why is this better than the session length which has good QA and we know exactly what it means?

data-review- on this part for at least improvements to the profile-created de-identification.

Comment 30

[[e-mail]]

I'll weigh in as well, even though bds has essentially performed the data review above.  I agree with the concerns about activeTicks (session length is a better measure, try using that instead).  Additionally, it's unlikely that you want that degree of precision with the profile creation date, and I agree with the suggested categorization of creation.

Comment 31

[Osmose [:osmose, :mkelly]]

I'll get to work on addressing the feedback for the second patch then.

Gijs: I don't have the rights to run try builds or ask for landing patches, would you be able to help land the first commit while I work on the replacement for the second one? They're standalone like you recommended and I'd like to at least get the first commit landed and uplifted if possible in case the second patch gets delayed.

Comment 32

[:Gijs (he/him)]

(In reply to Michael Kelly [:mkelly,:Osmose] from comment #31)
> I'll get to work on addressing the feedback for the second patch then.
> 
> Gijs: I don't have the rights to run try builds or ask for landing patches,
> would you be able to help land the first commit while I work on the
> replacement for the second one? They're standalone like you recommended and
> I'd like to at least get the first commit landed and uplifted if possible in
> case the second patch gets delayed.

If they need to be landed and uplifted separately, ideally we need a separate bug, because the tracking flags should reflect the fixed/affected status correctly, and if those are different for both patches we risk either confusion about what is available where, or things getting uplifted when they shouldn't be / neglected to be uplifted when they should be.

I'll do some bug cloning, I guess.

Comment 33

[Osmose [:osmose, :mkelly]]

I talked with mgrimes about a followup patch for profile age based on the privacy feedback, and he's interested in having it, but it's no longer necessary for the campaign mentioned in comment 1, and would probably need another bug anyway like what we did for the first patch.

I'll leave this open since it's marked as depending on bug 1310150 for documenting UITour, but in terms of required functionality, I'd call this resolved. Thanks all!

Comment 34

[:Gijs (he/him)]

(In reply to Michael Kelly [:mkelly,:Osmose] from comment #33)
> I talked with mgrimes about a followup patch for profile age based on the
> privacy feedback, and he's interested in having it, but it's no longer
> necessary for the campaign mentioned in comment 1, and would probably need
> another bug anyway like what we did for the first patch.

Well, we needed 2 bugs in total for the 2 things you were adding, so this could be it if you still wanted it...

> I'll leave this open since it's marked as depending on bug 1310150 for
> documenting UITour, but in terms of required functionality, I'd call this
> resolved. Thanks all!

It's not necessary to keep this bug open only because the deps are not resolved. It sounds like we don't need to pursue this for now, though, so marking this WONTFIX and we can reopen if/when there's enough of a need to warrant reopening the conversation with the data/privacy team to make this happen.