1442068 - Add telemetry probe to log what action we're trying to perform when a spinner shows up

Status: RESOLVED FIXED

(Firefox :: Tabbed Browser, enhancement)

Description

[Alex Thayer [:alexical] (she/her)]

We have several sources of information that are applicable to tab spinners, and none of them seem to line up as well as I'd like. I have a suspicion that a large number of spinners come from onEndSwapDocShells[1], which we fire on both windows when dragging a tab off window. I pretty reliably see a spinner on the original window when dragging a tab off of it on my fairly fast dev machine, because the spinner will display instantly, rather than after the usual 400ms delay.

In any case, it would be ideal to know what originated the tab change such that it caused a spinner - the best way I can think to do this is to just log what most recently set loadTimer to null, and then to log that when we decide to show a spinner, but I'm open to other suggestions.

[1]: https://searchfox.org/mozilla-central/rev/769222fadff46164f8cc0dc7a0bae5a60dc2f335/browser/base/content/tabbrowser.js#4812

Comment 1

[Alex Thayer [:alexical] (she/her)]

Additionally, we will reliably show a spinner if for some reason the switcher hasn't been destroyed yet after restoring a window from being minimized. Not altogether sure how common this or potentially similar scenarios could be, but all the more reason to have telemetry help inform us.

Comment 2

[Alex Thayer [:alexical] (she/her)]

Attachment: Bug 1442068 - Add diagnostic probe for tab switch spinners

We currently have a few probes which measure how long tab switch
spinners are displayed, but we have little information into their
source. In diagnosing and attempting to lower the number of
spinners that users see, we could use more information about
their source to help prioritize and narrow in on regions of code.

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

Comment 3

[Alex Thayer [:alexical] (she/her)]

Attachment: Bug 1442068 - Record last probable cause of tab switch spinner

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

Comment 4

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8955337 [details]
Bug 1442068 - Record last probable cause of tab switch spinner

https://reviewboard.mozilla.org/r/224502/#review230768

Yeah, this should be very helpful. Thanks!

Comment 5

[Chenxia Liu [:liuche]]

Hi Doug, this looks like Category 1 which is fine to collect, but can you fill out this data review request form and paste it into a comment?

https://github.com/mozilla/data-review/blob/master/request.md

(We're working on making this form simpler for probes like these, so it'll be less work to fill out!)

Comment 6

[Alex Thayer [:alexical] (she/her)]

# Request for data collection review form

1) What questions will you answer with this data?
  - What is the distribution of causes of tab switch spinners? With this information we can deduce i.e., 80% of spinners are shown when closing a tab.

2) Why does Mozilla need to answer these questions?  Are there benefits for users? Do we need this information to address product or business requirements? Some example responses:

  - Since different causes follow different code paths, if we notice one cause is heavily dominating the spinner numbers, it might suggest that something is wrong with that code path. The primary goal with this probe is diagnosis.

3) What alternative methods did you consider to answer these questions? Why were they not sufficient?

  - Poking around with a local build and reading the code to reason about potential bugs. This hasn't worked because there's too much to read and reason about to make meaningful progress without significant time investment.

4) Can current instrumentation answer these questions?

  - No.

5) List all proposed measurements and indicate the category of data collection for each measurement, using the Firefox [data c](https://wiki.mozilla.org/Firefox/Data_Collection)[ollection ](https://wiki.mozilla.org/Firefox/Data_Collection)[categories](https://wiki.mozilla.org/Firefox/Data_Collection) on the found on the Mozilla wiki.   

**Note that the data steward reviewing your request will characterize your data collection based on the highest (and most sensitive) category.**

<table>
  <tr>
    <td>Measurement Description</td>
    <td>Data Collection Category</td>
    <td>Tracking Bug #</td>
  </tr>
  <tr>
    <td>Categorize tab switch spinners by cause</td>
    <td>Category 1</td>
    <td>1442068</td>
  </tr>
</table>


6) How long will this data be collected?  Choose one of the following:

* I want this data to be collected for 6 months initially (potentially renewable).

7) What populations will you measure?

* Which release channels? All.

* Which countries? All.

* Which locales? All.

8) If this data collection is default on, what is the opt-out mechanism for users?

* The normal about:preferences opt-out mechanism.

9) Please provide a general description of how you will analyze this data.

* Basic analysis on TMO measurement dashboard.

10) Where do you intend to share the results of your analysis?

* This bug or I will file a new bug if something looks actionable.

Comment 7

[Chenxia Liu [:liuche]]

Comment on attachment 8955336 [details]
Bug 1442068 - Add diagnostic probe for tab switch spinners

https://reviewboard.mozilla.org/r/224500/#review232120

Data-review only

> Is there or will there be documentation that describes the schema for the ultimate data set available publicly, complete and accurate?

Yes, Histograms.js

> Is there a control mechanism that allows the user to turn the data collection on and off? (Note, for data collection not needed for security purposes, Mozilla provides such a control mechanism) Provide details as to the control mechanism available.

Firefox data controls

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

6 months

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

Type 1

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

default on

> 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

> Is the data collection covered by the existing Firefox privacy notice?

Ys

> Does there need to be a check-in in the future to determine whether to renew the data? (Yes/No) (If yes, set a todo reminder or file a bug if appropriate)

Probe expires in 63

Comment 8

[Alex Thayer [:alexical] (she/her)]

Comment on attachment 8955336 [details]
Bug 1442068 - Add diagnostic probe for tab switch spinners

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

Comment 9

[Alex Thayer [:alexical] (she/her)]

Comment on attachment 8955337 [details]
Bug 1442068 - Record last probable cause of tab switch spinner

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

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e8f77174972d
Add diagnostic probe for tab switch spinners r=liuche
https://hg.mozilla.org/integration/autoland/rev/648b41cbb8d1
Record last probable cause of tab switch spinner r=mconley

Comment 11

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/e8f77174972d
https://hg.mozilla.org/mozilla-central/rev/648b41cbb8d1

Comment 12

[Alex Thayer [:alexical] (she/her)]

Quick update on what we're seeing here:

none:               60034
preActions:         33
postActions:        4
onLoadTimeout:      19518
onLayersReady:      2524
onSizeModeOrOcc:    12
onEndSwapDocShells: 4960

"none" is far and away the most common trigger, which I kind of expected. The only case where we should see "none" is on tab removal, which makes sense, because when we remove tabs, we don't wait 400ms before showing a spinner like we do for standard tab switches. When removing tabs, we show a spinner as soon as the animation finishes, which I believe is ~100ms.

"onLoadTimeout" is second, which seems reasonable. This is the typical case that we are expecting. A user clicks to switch to a different tab, and then 400ms passes before everything we need comes back, so we show a spinner.

"onLayersReady" is interesting. I would have thought this one was impossible, and this looks like a good candidate for me as a cause of perma spinners. Going to look into this to see if it's likely just noise or if it's worth diving into.

"onEndSwapDocShells" is expected. This is roughly how often people drag tabs off into their own windows, which will reliably show a spinner.