This bug is for stopping UITelemetry from being reported, and updating the necessary documentation and tests. Stopping reporting may just be removing this line[1] from TelemetrySession, though maybe we should also ensure that UITelemetry.enabled always returns false while we're at it. There are some rather extensive tests to be disabled, and some documentation to be removed.

[1]: https://searchfox.org/mozilla-central/rev/bffd3e0225b65943364be721881470590b9377c1/toolkit/components/telemetry/TelemetrySession.jsm#766 (( note the interesting interplay with Android over here, which may complicate matters: https://searchfox.org/mozilla-central/rev/bffd3e0225b65943364be721881470590b9377c1/toolkit/components/telemetry/TelemetrySession.jsm#1309 ))

It's not entirely clear to me from this bug: Should UITelemetry only be disabled for Desktop and still be active for Android? Or should it be disabled completely?

No one spoke up when I said we were removing it, so stop reporting all of it for all platforms.

Comment on attachment 8960924 [details] [diff] [review] Stop reporting legacy Telemetry component `UITelemetry` Review of attachment 8960924 [details] [diff] [review]: ----------------------------------------------------------------- I thought we'd decided to stop reporting this for all platforms, not just Desktop?

As you mentioned in comment 3, `UITelemetry` is still used for Android as `UIMeasurements` here: https://searchfox.org/mozilla-central/rev/bffd3e0225b65943364be721881470590b9377c1/toolkit/components/telemetry/TelemetrySession.jsm#1309 My patch removes the `UITelemetry` payload and marks `UITelemetry` disabled on Desktop. On Android it is kept enabled (based on the preference), so `UIMeasurements` get reported. If I misunderstood the intention here I can disable that as well. There are still lots of uses of `UITelemetry.*` APIs in the Android code base.

Fair enough, we can keep those for now.

Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/a8c4f87b7c3d Stop reporting legacy Telemetry component `UITelemetry`. r=chutten

Backed out changeset a8c4f87b7c3d (bug 1443605) for browser-chrome failures at browser/components/uitour/test/browser_UITour_registerPageID.js on a CLOSED TREE Failure push: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=a8c4f87b7c3d4e049deb5ba637abd9b80dafc995 Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=169554102&repo=mozilla-inbound&lineNumber=5039 Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/9185d9c69f0ed9ccc1a1d5eacf5beb8df38af060

The UITour still uses UITelemetry, e.g. here: https://searchfox.org/mozilla-central/rev/c217fbde244344fedfd07b57a740c694a456dbca/browser/components/uitour/UITour.jsm#288 3 tests in [1] are now failing, that relied on UITelemetry being enabled. [1] https://searchfox.org/mozilla-central/rev/c217fbde244344fedfd07b57a740c694a456dbca/browser/components/uitour/test/browser_UITour_registerPageID.js Can we remove or disable these tests now? The UITour should switch over to Event Telemetry. :gijs, does this fall under your responsibility or can you name someone who is responsible for that part?

(In reply to Chris H-C :chutten from comment #3) > No one spoke up when I said we were removing it, so stop reporting all of it > for all platforms. I considered speaking up, because my understanding has always been that we would have loved to have the UI telemetry data but there was literally no way of accessing it easily. I got sidetracked and forgot about responding, and I also wasn't sure whether people like :bwinton and other folks who know about UI Telemetry (like other folks on the UI/UX team) had been consulted directly - I kind of assumed they would have been, esp. given they might not read fx-dev? So I'm a bit surprised to hear about this, and in this way... The other part of not commenting was that the implication from the post seemed to be "we will take care of replacing it with something better", and that doesn't now seem to be happening in here - we're just ripping stuff out with no replacement. (In reply to Jan-Erik Rediger [:janerik] from comment #10) > The UITour still uses UITelemetry, e.g. here: > https://searchfox.org/mozilla-central/rev/ > c217fbde244344fedfd07b57a740c694a456dbca/browser/components/uitour/UITour. > jsm#288 > > 3 tests in [1] are now failing, that relied on UITelemetry being enabled. > [1] > https://searchfox.org/mozilla-central/rev/ > c217fbde244344fedfd07b57a740c694a456dbca/browser/components/uitour/test/ > browser_UITour_registerPageID.js > > Can we remove or disable these tests now? > > The UITour should switch over to Event Telemetry. You can pretty much never just remove tests that highlight broken behaviour without fixing the broken behaviour. If everyone is on one page about all of this being removed and not replaced, then you'd need to remove the test *and* the code in UITour that relies on UITelemetry. Note that if the UI Tour usage is through web-exposed APIs, implying that mozilla.org pages care about those APIs and resulting UI Telemetry, you will need to talk to the mozilla.org people about that if you haven't already (it's not clear from this bug). If we're replacing it, then you will need to write the replacement code and update the tests so they test the replacement "Event Telemetry" (whatever that is) instead. > :gijs, does this fall under your responsibility or can you name someone who > is responsible for that part? Just to be 100% clear, to me this comment sounds like you're asking: "Hi, I'm removing the thing from a module you care about, if you want something that works, feel free to do the work yourself, please can I have your permission to remove this thing" ? I hope it's obvious that that doesn't come across very well. Now, about this patch... doesn't it basically just make BrowserUITelemetry and friends all dead code? There's not even a pref to turn it back on, but the patch keeps shipping it to our users, having all the code do a bunch of work for no purpose? Because that also doesn't seem great - if you're ripping it out, please rip it out thoroughly (and by implication, please get reviews from people who know that code).

I'm sorry for the miscommunication. I did hope to elicit exactly this sort of "I'm not sure about this" feedback with the announcements, and I failed to make that clear. Thank you for bringing it up now. It was my understanding that the UI Tour was only collecting UI Telemetry for historical reasons and the data was and is not being used... that the code was "alive" but only inasmuch as it was sending us data which sat unused. :janerik was working under my plan to do the easy first bits of stopping us from sending the data. The heavy lifting for removing the support structure was to follow in bug 1443609 (which I now see I forgot to mark as depending on this, which makes it impossible to find. Let me fix that...). This two-step approach was designed to ensure our pipeline handled the deprecation properly (it shouldn't, but fortune favours the prudent), and to give extra time for people to reach out to us if this would cause them any problems. (As well as giving our new hire, :janerik, some simpler tasks to help ramp him up in his role) We (the Firefox Telemetry Team) are planning to do the work of cleanly extricating UITelemetry from its call sites. We aren't sneakily trying to get you to do the work for us (either that or you caught us, and I'm "cleverly" disguising that fact). Do you know of anyone consuming the UITour UITelemetry data? In preparation (sadly mostly done in google docs) for removing these legacy Telemetry pieces we did ask around, but we may have missed someone. Or several someones. Initially we were focusing mostly on projects that were generating and analysing their own events (Activity Stream and other Ping Centre users were the big ones from September), so it's possible someone struggling along with UITelemetry might have been missed during the migration phases last year. (( Sadly, I wasn't involved in most of the Event Telemetry stuff (I was still on :ddurst's team at the time), so please bear with me as I dig through old half-remembered things and hand-wave until Georg and Alessio return :S ))

(In reply to Chris H-C :chutten from comment #12) > I'm sorry for the miscommunication. I did hope to elicit exactly this sort > of "I'm not sure about this" feedback with the announcements, and I failed > to make that clear. Thank you for bringing it up now. > > It was my understanding that the UI Tour was only collecting UI Telemetry > for historical reasons and the data was and is not being used... that the > code was "alive" but only inasmuch as it was sending us data which sat > unused. OK. Did you talk to cmore and/or agibson and/or MattN about the UI Tour / page id stuff? I have no idea if that is as unused as the code looks. Looks like we updated it when we did initial Hello/Loop work, which is semi-recent (ie only about a year or two ago). But not sure if that means it's still in active use or not. Did you talk to bwinton about the browser ui telemetry stuff? From a quick look, it seems there used to be a dashboard for the latter at https://useradvocacy.mozilla.org/dashboards/telemetry-ui which has gone AWOL. I don't know if there's a replacement or not. At least for that bit I'm 99% sure that UI/UX and browser frontend people would like to have the relevant data, and if this is an opportunity to shift it to a collection method where we *can* reasonably see the data, that would be great, and kinda better than just removing all of it. FWIW, having looked at this again, I think it would have been good to send the note about this stuff going away to fx-dev. More of the relevant browser folks would have seen it there, compared to data-dev (which is the only place I'm seeing in my email, but it's possible I'm missing an older post or something?) > :janerik was working under my plan to do the easy first bits of stopping us > from sending the data. The heavy lifting for removing the support structure > was to follow in bug 1443609 (which I now see I forgot to mark as depending > on this, which makes it impossible to find. Let me fix that...). This > two-step approach was designed to ensure our pipeline handled the > deprecation properly (it shouldn't, but fortune favours the prudent), I think there's a negation too many / too little here? Oh well. > and to > give extra time for people to reach out to us if this would cause them any > problems. > > (As well as giving our new hire, :janerik, some simpler tasks to help ramp > him up in his role) This makes sense, and I'm sorry for throwing a bunch of stop energy in the way here. :-( > We (the Firefox Telemetry Team) are planning to do the work of cleanly > extricating UITelemetry from its call sites. We aren't sneakily trying to > get you to do the work for us (either that or you caught us, and I'm > "cleverly" disguising that fact). Sure - when you say "extricating" does that mean "extricating and then replacing with something that works", or just "removing" ? :-) > Do you know of anyone consuming the UITour UITelemetry data? In preparation > (sadly mostly done in google docs) for removing these legacy Telemetry > pieces we did ask around, but we may have missed someone. Or several > someones. Yeah, see above - not sure if you already asked those people or not. > Initially we were focusing mostly on projects that were generating > and analysing their own events (Activity Stream and other Ping Centre users > were the big ones from September), so it's possible someone struggling along > with UITelemetry might have been missed during the migration phases last > year. > > (( Sadly, I wasn't involved in most of the Event Telemetry stuff (I was > still on :ddurst's team at the time), so please bear with me as I dig > through old half-remembered things and hand-wave until Georg and Alessio > return :S )) Are there meta bugs I could look at, or was the migration stuff all in google docs (and/or maybe github for AS) as well?

(In reply to :Gijs from comment #13) > (In reply to Chris H-C :chutten from comment #12) > > I'm sorry for the miscommunication. I did hope to elicit exactly this sort > > of "I'm not sure about this" feedback with the announcements, and I failed > > to make that clear. Thank you for bringing it up now. > > > > It was my understanding that the UI Tour was only collecting UI Telemetry > > for historical reasons and the data was and is not being used... that the > > code was "alive" but only inasmuch as it was sending us data which sat > > unused. > > OK. Did you talk to cmore and/or agibson and/or MattN about the UI Tour / > page id stuff? I have no idea if that is as unused as the code looks. Looks > like we updated it when we did initial Hello/Loop work, which is semi-recent > (ie only about a year or two ago). But not sure if that means it's still in > active use or not. I'll add that there *is* a UITour API here so whether it's "in use" kind of depends on whether mozilla.org pages call those APIs, and I don't know of a good way to figure that out quickly.

(In reply to :Gijs (out until 26th) from comment #13) > (In reply to Chris H-C :chutten from comment #12) > Did you talk to bwinton about the browser ui telemetry stuff? Yep! In fact, I suggested he loop you in, Gijs, cause I thought you would have a lot of useful information on who was using it. :) > From a quick > look, it seems there used to be a dashboard for the latter at > https://useradvocacy.mozilla.org/dashboards/telemetry-ui which has gone > AWOL. I don't know if there's a replacement or not. There isn't. There were a few attempts at getting something up, but nothing ever came of it. > At least for that bit > I'm 99% sure that UI/UX and browser frontend people would like to have the > relevant data, and if this is an opportunity to shift it to a collection > method where we *can* reasonably see the data, that would be great, and > kinda better than just removing all of it. Yeah, this is one of the reasons I'm excited about removing UITelemetry (as long as we file bugs to get the relevant probes added to EventTelemetry)! > FWIW, having looked at this again, I think it would have been good to send > the note about this stuff going away to fx-dev. More of the relevant browser > folks would have seen it there, compared to data-dev (which is the only > place I'm seeing in my email, but it's possible I'm missing an older post or > something?) I don't think it's too late to send a note to fx-dev, so that more browser peeps can see it… (In reply to :Gijs (out until 26th) from comment #14) > I'll add that there *is* a UITour API here so whether it's "in use" kind of > depends on whether mozilla.org pages call those APIs, and I don't know of a > good way to figure that out quickly. I _believe_ the firstrun page uses a UITour API to redirect to `about:home` at the end of the animation, for what that's worth.

(In reply to :Gijs (out until 26th) from comment #13) > (In reply to Chris H-C :chutten from comment #12) > > I'm sorry for the miscommunication. I did hope to elicit exactly this sort > > of "I'm not sure about this" feedback with the announcements, and I failed > > to make that clear. Thank you for bringing it up now. > > > > It was my understanding that the UI Tour was only collecting UI Telemetry > > for historical reasons and the data was and is not being used... that the > > code was "alive" but only inasmuch as it was sending us data which sat > > unused. > > OK. Did you talk to cmore and/or agibson and/or MattN about the UI Tour / > page id stuff? I have no idea if that is as unused as the code looks. Looks > like we updated it when we did initial Hello/Loop work, which is semi-recent > (ie only about a year or two ago). But not sure if that means it's still in > active use or not. It looks as though highlights were updated when Photon moved things around as well. But this is less about whether UITour is being used and updated and more about whether the UITelemetry it submits is being used. So let me strike up a conversation with cmore, agibson, and MattN (shall I assume you'd like a +Cc on the thread?) to see what's what. And then I'll publish the result here on the bug so others can find it in the future. > FWIW, having looked at this again, I think it would have been good to send > the note about this stuff going away to fx-dev. (In reply to :bwinton from comment #15) > I don't think it's too late to send a note to fx-dev, so that more browser peeps can see it… Fine! Fine! You win! :) I'll pop it up to fx-dev today for visibility. To document the communication methods we've done so far: thuelbert brought it up at the cross-functional, and then emails went out to data-platform and data-dev. That's where it stopped because the impact of these particular legacy bits was thought to be limited to just the data collection peeps. > This makes sense, and I'm sorry for throwing a bunch of stop energy in the > way here. :-( I have yet to find a Mozillian that isn't susceptible to letting enthusiasm get the better of them. Pumping the brakes is incredibly important to make sure we take the time to do things right. > > We (the Firefox Telemetry Team) are planning to do the work of cleanly > > extricating UITelemetry from its call sites. We aren't sneakily trying to > > get you to do the work for us (either that or you caught us, and I'm > > "cleverly" disguising that fact). > > Sure - when you say "extricating" does that mean "extricating and then > replacing with something that works", or just "removing" ? :-) Ah, you caught me :) We were only planning on doing the removing. The idea was: no one's using the data now, so let's not replace it with new data that will continue to not be used. Also, we don't know UITour well enough to develop an event taxonomy or to know what's important to instrument. If the UITelemetry data -is- being used, that changes the equation. > > Initially we were focusing mostly on projects that were generating > > and analysing their own events (Activity Stream and other Ping Centre users > > were the big ones from September), so it's possible someone struggling along > > with UITelemetry might have been missed during the migration phases last > > year. > > > > (( Sadly, I wasn't involved in most of the Event Telemetry stuff (I was > > still on :ddurst's team at the time), so please bear with me as I dig > > through old half-remembered things and hand-wave until Georg and Alessio > > return :S )) > > Are there meta bugs I could look at, or was the migration stuff all in > google docs (and/or maybe github for AS) as well? *digs through some email* I found a planning doc from September: https://docs.google.com/document/d/1PAcl_ak0bljHPayG7fJ2YQ4DaryDxMg-nFcYmt-gkrI/edit#heading=h.he0a1id61ks I found an AS bug for instrumenting using events as recent as two months ago with bug 1429497 ...and this is where I wave my hands to buy time until next week when Georg and Alessio return *waves his hands* --- tl;dr - I will start an email thread with cmore, agibson, and MattN (with Gijs on Cc) about whether UITelemetry is being used in UITour. I will also echo the "we're removing legacy Telemetry cruft" email to fx-dev for visibility. Until those threads resolve, we'll leave UITelemetry alone.

Comment on attachment 8960924 [details] [diff] [review] Stop reporting legacy Telemetry component `UITelemetry` Review of attachment 8960924 [details] [diff] [review]: ----------------------------------------------------------------- Clearing review so we don't check this in.

Feedback from :agibson is that bedrock (mozilla.org) uses pageid in exactly one place, and on a page that's on the way out. So not only does it not use data from UITelemetry, it almost doesn't record any either. Feedback from :MattN is that TreatmentTag fills any analysis void, and that pageid wasn't being used lately. Feedback from :cmore is that everything's fine so long as we don't accidentally interfere with UITour's ability to tell a whitelisted website (like firstrun) about a core feature (like sync) being used or not. So we're clear on that. The only remaining question was what do we do about the removal of a built-in capability to measure users' use of the browser UI. The answer to that appears to be :phlsa's work on Project Savant which will instrument the Firefox UI with Event Telemetry. With UX driving the requirements, involvement from :gfritzsche for the client side, and support from :sunahsuh on the pipeline side it seems like it should succeed where UITelemetry failed. Especially on tooling. With that we're good to continue work on this. Passing back to :janerik.

I decided to not set `UITelemetry.enabled` to false generally as it just passes through whether Telemetry in general is enabled. This makes the previously failing test work again, so I can then remove the code and test in the followup bug.

Comment on attachment 8964510 [details] [diff] [review] Stop reporting legacy Telemetry component `UITelemetry` Review of attachment 8964510 [details] [diff] [review]: ----------------------------------------------------------------- As mentioned earlier UITelemetry is not a required field in the schema so we don't need to change anything pipeline-side, right? If so, this seems good to go.

I double checked in [1] and it turns out `UITelemetry` is not even in there, so it's definitely optional and no pipeline changes required. [1]: https://github.com/mozilla-services/mozilla-pipeline-schemas/blob/55d3ff1f474bde69c824e2657f2b293f04ecc116/schemas/telemetry/main/main.4.schema.json

Pushed by rgurzau@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/a3ac84a960c7 Stop reporting legacy Telemetry component `UITelemetry` r=chutten