1615986 - [Thunderbird Telemetry] collect number of address books + types + number of contacts

Status: RESOLVED FIXED

(Thunderbird :: Address Book, task)

Description

[Magnus Melin [:mkmelin] (away until Aug 10)]

It would be useful to understand the design boundaries for address books. How many address books do people have, of what type, and how many contacts do they have [not for ldap].

Comment 1

[Ping Chen [:rnons] (away)]

Attachment: 1615986.patch

Collect address book count and contact count, keyed by address book type.

The address book type can be 0, 2 or 101, do we want to convert it to string?

Comment 2

[Ping Chen [:rnons] (away)]

Do we want to collect the count

Comment 3

[Magnus Melin [:mkmelin] (away until Aug 10)]

Comment on attachment 9154107 [details] [diff] [review]
1615986.patch

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

We probably need to hook this up somewhere else. Many people might not ever open the address book tab, but still use their address books.

::: mail/components/addrbook/content/abTrees.js
@@ +174,5 @@
> + */
> +function reportAddressBook(directories) {
> +  let report = {};
> +  for (let dir of directories) {
> +    let type = dir.dirType;

Maybe it would be better to add a more selfexplanatory key. Like using the uri scheme, e.g. jscarddav, jsaddrbook, moz-abmdbdirectory, ldap, ldaps

Comment 4

[Ping Chen [:rnons] (away)]

Attachment: 1615986.patch

• Moved the probe to msgMail3PaneWindow.js, so that it will run on TB starts.
• Used the URI scheme as address book type.

Comment 5

[Ping Chen [:rnons] (away)]

Attachment: 1615986.patch

Remove a commented line.

Comment 6

[Magnus Melin [:mkmelin] (away until Aug 10)]

Comment on attachment 9154840 [details] [diff] [review]
1615986.patch

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

LGTM

::: mail/base/content/msgMail3PaneWindow.js
@@ +805,5 @@
> +function reportAddressBookTypes() {
> +  let report = {};
> +  for (let dir of MailServices.ab.directories) {
> +    let index = dir.URI.indexOf(":");
> +    let type = dir.URI.slice(0, index);

maybe easier to just do

let type = dir.URI.split(":")[0];

::: mail/components/telemetry/Scalars.yaml
@@ +150,5 @@
> +tb.addressbook:
> +  addressbook_count:
> +    bug_numbers:
> +      - 1615986
> +    description: How many addressbooks were set up, keyed by addressbook type.

Let's clarify that the type is the directory URI scheme.

Comment 7

[Ping Chen [:rnons] (away)]

Attachment: 1615986.patch

• Changed to use split(":")[0]
• Updated scalar probe description

Comment 8

[Magnus Melin [:mkmelin] (away until Aug 10)]

Somehow the attached patch is mangled and can't be applied. Please try exporting and uploading it again.

Comment 9

[Ping Chen [:rnons] (away)]

Attachment: 1615986.patch

Rebased, should work now.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/813ca5b455a8
Collect statistics of addressbooks and contacts. r=mkmelin

Comment 11

[Magnus Melin [:mkmelin] (away until Aug 10)]

Sorry had to back this out due to test failures - backout: https://hg.mozilla.org/comm-central/rev/d4c0fe8b3368616586331c15efca65594e206c5d

E.g. https://treeherder.mozilla.org/#/jobs?repo=comm-central&selectedTaskRun=LL_HQ90mTn6cxnTTiYEe-w-0

Comment 12

[Ping Chen [:rnons] (away)]

Attachment: 1615986.patch

Sorry, forgot to push to Try. Tests should pass now https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=0073a1be8fda7c6b6913806f3d6a52f84dc1&selectedTaskRun=afp2hTK0QjGvAtu9OO9qjA-0

Comment 13

[Magnus Melin [:mkmelin] (away until Aug 10)]

Comment on attachment 9155835 [details] [diff] [review]
1615986.patch

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

::: mail/test/browser/addrbook/browser_addressBook.js
@@ +401,5 @@
> +  delete_address_book(addrBook1);
> +  delete_address_book(addrBook2);
> +  delete_address_book(addrBook3);
> +  delete_address_book(addrBook4);
> +  delete_address_book(ldapBook);

I think these belong in the registerCleanupFunction instead.

r=mkmelin with that fixed

Comment 14

[Ping Chen [:rnons] (away)]

Attachment: 1615986.patch

Changed to use registerCleanupFunction.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/f5437b1e2765
Collect statistics of addressbooks and contacts. r=mkmelin DONTBUILD

Comment 16

[Magnus Melin [:mkmelin] (away until Aug 10)]

Comment on attachment 9155882 [details] [diff] [review]
1615986.patch

Telemetry addition - not risky.

Comment 17

[Wayne Mery (:wsmwk)]

Comment on attachment 9155882 [details] [diff] [review]
1615986.patch

Approved for beta

Comment 18

[Geoff Lankow (:darktrojan)]

Thunderbird 78.0b2:
https://hg.mozilla.org/releases/comm-beta/rev/c19b6d0fdf3a