855930 - Add telemetry probe for number of active tabs

Status: VERIFIED FIXED

(Firefox for Metro Graveyard :: General, defect, P2)

Description

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

See bug 828562 comment 1.

We would like to keep track of the number of tabs that users generally have open.  I imagine that we'll want to track the
  1) Average number of active tabs in a session (we could take a sample each time a tab is opened or closed)
  2) Max number of simultaneously active tabs

Comment 1

[Marina Samuel [:emtwo]]

Attachment: v1: Add telemetry for max and avg number of tabs in a session.

Comment 2

[Sam Foster [:sfoster] (he/him)]

Comment on attachment 8372603 [details] [diff] [review]
v1: Add telemetry for max and avg number of tabs in a session.

Review of attachment 8372603 [details] [diff] [review]:
-----------------------------------------------------------------

The TelemetryPing is triggered by idle-daily. At that point it will go through each of the registered simple measures functions (added via addSimpleMeasureFunction) and call them in turn, stashing whatever data they can return at that time. The only way to get max tabs over the full lifetime of a session as I understand it would be to save it in the session store so it can be retrieved *next* time the ping comes around. Do we need that max tabs high-water mark? Or is it enough to simply return window.Browser.tabs.length as a kind of random sampling of tab count for that user at that (arbitrary) time. Or just watch TabOpen and conditionally update _maxTabsOpen.

::: browser/metro/components/SessionStore.js
@@ +258,5 @@
>          // Freeze the data at what we've got (ignoring closing windows)
>          this._loadState = STATE_QUITTING;
>          break;
>        case "quit-application":
> +        // Send tab telemetry on close

The telemetry ping is a "dont call us, we'll call you" thing. We should call addSimpleMeasureFunction during init, so that it is registered already if/when the ping comes around.

Comment 3

[Marina Samuel [:emtwo]]

Attachment: v2: Add telemetry for max and avg number of tabs in a session.

Ah, I misunderstood when telemetry is being sent. I'd still like to keep both a current and a max tab count though because I think they will provide us with different information

Comment 4

[Marina Samuel [:emtwo]]

Also, I'm ok with the random sampling of the current tab count because I think it can be used to estimate the avg number of tabs a user has open.

Comment 5

[Sam Foster [:sfoster] (he/him)]

Comment on attachment 8373435 [details] [diff] [review]
v2: Add telemetry for max and avg number of tabs in a session.

Review of attachment 8373435 [details] [diff] [review]:
-----------------------------------------------------------------

Looks great.

::: browser/metro/components/SessionStore.js
@@ +196,5 @@
>        aFile.remove(false);
>      })
>    },
>  
> +  getTabStats: function() {

should we make this "private" as _getTabStats? No strong opinion, but public methods form a contract that we need to keep to and this seems mostly an internal method

Comment 6

[Marina Samuel [:emtwo]]

https://hg.mozilla.org/integration/fx-team/rev/9865c9b5c369

Addressed the comment about _getTabStats. Also, some tests were failing so I made two other small changes:

* "TabOpen" was in the switch statement twice so the second case never got executed. Pulled out the code into an "updateTabTelemetryVars()" function to be called twice.
* When mochitests ran altogether, _maxTabsOpen was affected by previous tests that ran and had incorrect values. Added code to reset this value just for the purpose of tests.

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/9865c9b5c369

Comment 8

[Manuela Muntean [Away]]

Could anyone please give guidance in order for the QA to verify this?

Comment 9

[Marina Samuel [:emtwo]]

Open about:telemetry in Metro and under Simple Measurements -> UITelemetry, there should be an attribute called "metro-tabs" with a currTabCount and a maxTabCount. Ensure the values for these are correct(e.g. your current number of open tabs is equal to currTabCount)

Comment 10

[Manuela Muntean [Away]]

(In reply to Marina Samuel [:emtwo] from comment #9)
> Open about:telemetry in Metro and under Simple Measurements -> UITelemetry,
> there should be an attribute called "metro-tabs" with a currTabCount and a
> maxTabCount. Ensure the values for these are correct(e.g. your current
> number of open tabs is equal to currTabCount)

Verified as fixed with latest Nightly on Win 8 64-bit, for iteration IT-30C-29A-28B.1 (it #24)