Add telemetry probe for number of active tabs

VERIFIED FIXED in Firefox 30

Status

defect
P2
normal
VERIFIED FIXED
6 years ago
5 years ago

People

(Reporter: TimAbraldes, Assigned: emtwo)

Tracking

unspecified
Firefox 30
x86
Windows 8.1
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: p=2 s=it-30c-29a-28b.1 r=ff30)

Attachments

(1 attachment, 1 obsolete attachment)

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
QA Contact: jbecerra
Priority: -- → P2
Priority: P2 → P4

Updated

6 years ago
Whiteboard: feature=story c=data_submission u=metro_firefox_user p=8 → feature=story c=data_submission u=metro_firefox_user p=2

Updated

6 years ago
Priority: P4 → P3
Depends on: 872206
No longer depends on: 872206
Blocks: metrobacklog
No longer blocks: metrov1backlog, 850347
Whiteboard: feature=story c=data_submission u=metro_firefox_user p=2 → feature=story c=data_submission u=metro_firefox_user p=0
Priority: P3 → --
Summary: Story - Add telemetry probe for number of active tabs → Add telemetry probe for number of active tabs
Whiteboard: feature=story c=data_submission u=metro_firefox_user p=0 → [story]
Blocks: 956798
Whiteboard: [story] → [feature] p=0
Whiteboard: [feature] p=0 → [feature] p=2
Priority: -- → P1
Component: Metro Operations → General
Product: Tracking → Firefox for Metro
QA Contact: jbecerra
Target Milestone: --- → Firefox 30
Version: --- → unspecified
(Assignee)

Updated

5 years ago
Assignee: nobody → msamuel
Status: NEW → ASSIGNED
Priority: P1 → P2
QA Contact: jbecerra
Whiteboard: [feature] p=2 → [feature] p=2 s=it-30c-29a-28b.1
(Assignee)

Updated

5 years ago
Attachment #8372603 - Flags: review?(sfoster)
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.
Attachment #8372603 - Flags: review?(sfoster) → review-
Whiteboard: [feature] p=2 s=it-30c-29a-28b.1 → p=2 s=it-30c-29a-28b.1 r=ff30
Target Milestone: Firefox 30 → ---
(Assignee)

Comment 3

5 years ago
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
Attachment #8372603 - Attachment is obsolete: true
Attachment #8373435 - Flags: review?(sfoster)
(Assignee)

Comment 4

5 years ago
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 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
Attachment #8373435 - Flags: review?(sfoster) → review+
(Assignee)

Comment 6

5 years ago
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.
(Assignee)

Updated

5 years ago
Whiteboard: p=2 s=it-30c-29a-28b.1 r=ff30 → [fixed-in-fx-team] p=2 s=it-30c-29a-28b.1 r=ff30
https://hg.mozilla.org/mozilla-central/rev/9865c9b5c369
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team] p=2 s=it-30c-29a-28b.1 r=ff30 → p=2 s=it-30c-29a-28b.1 r=ff30
Target Milestone: --- → Firefox 30
Could anyone please give guidance in order for the QA to verify this?
Flags: needinfo?(msamuel)
(Assignee)

Comment 9

5 years ago
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)
Flags: needinfo?(msamuel)
(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)
Status: RESOLVED → VERIFIED
OS: Windows 8 Metro → Windows 8.1
You need to log in before you can comment on or make changes to this bug.