1232050 - Collect telemetry for number of Sync devices connected.

Status: RESOLVED FIXED

(Firefox :: Sync, defect, P1)

Description

[Mark Hammond [:markh] [:mhammond]]

Attachment: 0001-Bug-XXXXXXX-add-telemetry-for-number-of-devices-atta.patch

Chris More has been using FHR data to track how the number of devices attached to a sync account affects user retention. We need to start tracking this data using telemetry so the same analysis can be done going forward.

I'm attaching a patch that adds to a keyed histogram. The key is either "desktop" or "mobile" and the number is the number of devices of that type associated with the sync account. Like the FHR data, this count includes the "current" device.

Chris, can you please verify that this probe sounds like it will fit your requirements for retention analysis? In particular, I'd like to make sure that the breakdown by device type is suitable and allows the aggregation you need.

Benjamin, if I'm reading the docs correctly, an "enumerated" probe is the correct choice here ("If you need a linear histogram with buckets < 0, 1, 2 ... N >, then you should declare an enumerated histogram.") n_values=10 which sounds fine for these requirements.

Comment 1

[Chris More [:cmore]]

This looks right. I first want to see if they have sync enabled and if they do, how many desktop and mobile devices are connected to the account from Telemetry.

In FHR v2, it was denoted at:

org.mozilla.sync.devices": {
          "_v": 1,
          "desktop": 1,
          "mobile": 1
        },

Comment 2

[Chris More [:cmore]]

I want to ensure we get this on the train to get it lined up for Firefox 44 (1/26/2016) since :trink was building a new cohort rollup based off of v4 in bug 1226379.

Comment 3

[Benjamin Smedberg]

Comment on attachment 8697705 [details] [diff] [review]
0001-Bug-XXXXXXX-add-telemetry-for-number-of-devices-atta.patch

In terms of data processing, it's much more efficient to just have two separate histograms WEAVE_DEVICE_COUNT_DESKTOP and WEAVE_DEVICE_COUNT_MOBILE. So I'd recommend that unless there's some reason you want the separate key, you do that.

I think it's important to document that this histogram is recorded each time the client syncs in the description field. Also please document whether the count includes or excludes the "current" device. So e.g. if you only use sync with one device, will this record 0 or 1? Is this recorded only on successful syncs, or all syncs?

Is there a concise statement of the user value we're providing by collecting this data? From our direct conversation ISTR the following things mentioned:

* The new-user churn analysis cohorts by sync usage so that we can measure whether sync is providing user value. If so, we should have a bug to make the churn analysis public.

* Were you going to use this as a denominator for error counts so that we could chart the number of syncs that were failing? If so is that WEAVE_ENGINE_APPLY_NEW_FAILURES? Or some other error metric? 

* If there are other user benefits for this data collection, please document them and who is responsible for analyzing/maintaining the reports.

Please add "bug_numbers" to this histogram to reference this bug.

Comment 4

[Edwin Wong [:edwong]]

setting P1 and we want to uplift into Fx44

Comment 5

[Mark Hammond [:markh] [:mhammond]]

Attachment: 0001-Bug-1232050-add-telemetry-for-number-of-devices-atta.patch

(In reply to Benjamin Smedberg  [:bsmedberg] from comment #3)
> In terms of data processing, it's much more efficient to just have two
> separate histograms WEAVE_DEVICE_COUNT_DESKTOP and
> WEAVE_DEVICE_COUNT_MOBILE. So I'd recommend that unless there's some reason
> you want the separate key, you do that.

You mentioned that you were helping Chris with his retention analysis using telemetry instead of FHR, so I've made this change under the assumption you know that this will work for his use-case.

> I think it's important to document that this histogram is recorded each time
> the client syncs in the description field. Also please document whether the
> count includes or excludes the "current" device. So e.g. if you only use
> sync with one device, will this record 0 or 1? Is this recorded only on
> successful syncs, or all syncs?

Done.

> Is there a concise statement of the user value we're providing by collecting
> this data? From our direct conversation ISTR the following things mentioned:
> 
> * The new-user churn analysis cohorts by sync usage so that we can measure
> whether sync is providing user value. If so, we should have a bug to make
> the churn analysis public.

Chris is updating his existing analysis, so I assume this comment is for him?

> * Were you going to use this as a denominator for error counts so that we
> could chart the number of syncs that were failing? If so is that
> WEAVE_ENGINE_APPLY_NEW_FAILURES? Or some other error metric? 

This is unrelated to other Sync telemetry probes.

> * If there are other user benefits for this data collection, please document
> them and who is responsible for analyzing/maintaining the reports.

That's Chris, and as mentioned above, I believe you said you were assisting him with this. I've no insight into this, but was just asked to make the data available. So again, I assume this is a request for Chris?

> Please add "bug_numbers" to this histogram to reference this bug.

Done.

Comment 6

[Georg Fritzsche [:gfritzsche]]

Comment on attachment 8698821 [details] [diff] [review]
0001-Bug-1232050-add-telemetry-for-number-of-devices-atta.patch

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

Can we add test coverage for this to test_syncengine_sync.js or a similar test?

::: services/sync/modules/engines/clients.js
@@ +148,5 @@
>      }
>      SyncEngine.prototype._syncStartup.call(this);
>    },
>  
> +  _syncFinish() {

|_syncFinish: function() { ...| ?

Comment 7

[Mark Hammond [:markh] [:mhammond]]

(In reply to Georg Fritzsche [:gfritzsche] [away until Jan 3] from comment #6)
> > +  _syncFinish() {
> 
> |_syncFinish: function() { ...| ?

FWIW, the front-end is being fairly aggressive with changing to the more modern object definitions when touching them, so unless there is a strong objection I'd prefer to keep doing that here.

Comment 8

[Mark Reid [:mreid]]

Should the WEAVE_CONFIGURED histogram definition be updated to include "releaseChannelCollection": "opt-out" too?

Comment 9

[Mark Hammond [:markh] [:mhammond]]

Attachment: 0001-Bug-1232050-add-telemetry-for-number-of-devices-atta.patch

(In reply to Mark Reid [:mreid] from comment #8)
> Should the WEAVE_CONFIGURED histogram definition be updated to include
> "releaseChannelCollection": "opt-out" too?

Yes, I think it should, thanks. This patch includes that.

Comment 10

[Benjamin Smedberg]

Comment on attachment 8704886 [details] [diff] [review]
0001-Bug-1232050-add-telemetry-for-number-of-devices-atta.patch

From a separate email thread:
* sync team is responsible for the correctness of this data
* At the present time, the churn report correlating churn against new user retention is the only artifact driven by this data, but the team has a plan to add other quality metrics and reporting over the next quarter. Chris More and Chris Karlof are both responsible for monitoring the churn report.
* The immediate user benefit is measuring and optimizing the sync experience and activation flow, and that is going to drive product improvements for the next year at least.

Please document that WEAVE_CONFIGURED is only recorded once per session (near startup). This is not the ideal configuration since this metric will not appear in any but the first subsession; please file a followup to record this either in the environment or some more persistent flag space.

data-steward approval=me

Comment 11

[Mark Hammond [:markh] [:mhammond]]

Attachment: 0001-Bug-1232050-add-telemetry-for-number-of-devices-atta.patch

Thanks Benjamin.

This patch adds the requested comment re FXA_CONFIGURED. Kit, are you able to review this fairly soon?

Comment 12

[Chris Karlof [:ckarlof]]

Thanks Benjamin!

Comment 13

[Lina Butler [:lina]]

Comment on attachment 8706187 [details] [diff] [review]
0001-Bug-1232050-add-telemetry-for-number-of-devices-atta.patch

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

Looks good!

Comment 14

[Mark Hammond [:markh] [:mhammond]]

(In reply to Benjamin Smedberg  [:bsmedberg] from comment #10)
> Please document that WEAVE_CONFIGURED is only recorded once per session
> (near startup). This is not the ideal configuration since this metric will
> not appear in any but the first subsession; please file a followup to record
> this either in the environment or some more persistent flag space.

I opened bug 1238810
> 
> data-steward approval=me

Comment 15

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/81427927d338

Comment 16

[Mark Hammond [:markh] [:mhammond]]

Comment on attachment 8706187 [details] [diff] [review]
0001-Bug-1232050-add-telemetry-for-number-of-devices-atta.patch

Approval Request Comment
[Feature/regressing bug #]: Unified telemetry
[User impact if declined]: We are unable to measure Sync retention stats and how Sync changes general Firefox retention metrics
[Describe test coverage new/current, TreeHerder]: Existing tests pass
[Risks and why]: Low risk, limited to Sync
[String/UUID change made/needed]: None

Comment 17

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/81427927d338

Comment 18

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8706187 [details] [diff] [review]
0001-Bug-1232050-add-telemetry-for-number-of-devices-atta.patch

Sure, taking it in 45.

Comment 19

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/01bba60dd271

Comment 20

[Mayur Patil]

[bugday-20160323]

Status: RESOLVED,FIXED -> UNVERIFIED

Comments:
STR: Not clear.
Developer specific testing

Component: 
Name			Firefox
Version			46.0b9
Build ID		20160322075646
Update Channel	        beta
User Agent		Mozilla/5.0 (Windows NT 6.1; WOW64; rv:46.0) Gecko/20100101 Firefox/46.0
OS			Windows 7 SP1 x86_64

Expected Results: 
Developer specific testing

Actual Results: 
As expected