1019298 - Use Telemetry to count which directory links were shown in which tile position

Status: VERIFIED FIXED

(Firefox :: New Tab Page, defect)

Description

[Ed Lee :Mardak]

While bug 975235 is being set up to send impression data to the server in bug 995262, we can start getting per-tile impression data through telemetry (note: not click data).

There's a caveat that we'll only record links data for the first 9 links shown in the first 9 positions. So if we end up with different links data down the line, this data will need some extra care when analyzing as we won't be able to differentiate which list the link came from.

Comment 1

[Ed Lee :Mardak]

Attachment: v1 screenshot

I happened to have wired (#2/sponsored), wikipedia (#5/organic), bbc (#8/sponsored) shown when I opened the new tab page.

Comment 2

[Ed Lee :Mardak]

Attachment: v1

Comment 3

[Drew Willcoxon :adw]

Comment on attachment 8432919 [details] [diff] [review]
v1

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

::: browser/base/content/newtab/page.js
@@ +211,5 @@
>        if (site) {
>          site.captureIfMissing();
> +
> +        // Record which tile index a directory link was shown
> +        let {directoryIndex, type} = site.link;

This won't emit any kind of annoying warning about directoryIndex not being defined for links that don't define it, right?  I'm thinking especially of warnings dumped to the terminal in debug builds.  It's fine for not all links to define it, but if there is such a warning, please write this in a way that doesn't emit it.

Comment 4

[Ed Lee :Mardak]

(In reply to Drew Willcoxon :adw from comment #3)
> > +        let {directoryIndex, type} = site.link;
> This won't emit any kind of annoying warning about directoryIndex not being
> defined for links that don't define it, right?
I'm assuming you're talking about these types of strict warnings?

JavaScript strict warning: chrome://browser/content/tabbrowser.xml, line 1461: reference to undefined property params.referrerURI


I tried doing both ".. = site.link.directoryIndex" and "{directoryIndex} = site.link" and neither result in the warnings... ?

Comment 5

[Drew Willcoxon :adw]

Yeah, good then.  I truly wasn't sure if there would be, so I wanted to make sure there isn't. :-)

Comment 6

[Georg Fritzsche [:gfritzsche]]

Comment on attachment 8432919 [details] [diff] [review]
v1

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

The histogram usage seems a little awkward here - maybe you could ask in #perf if there is a better option?
Otherwise this looks good to me.

Comment 7

[Ed Lee :Mardak]

The feedback from #perf (irving) was perhaps we should use "simple measurement" and store a JSON string with the appropriate data we want as the telemetry histograms probably won't show the results in a way we want anyway, so we'll need some special processing anyway.

Also, with an average of 4 newtabs per session and that this would only record an impression if Directory links were shown, it seems to be okay to not aggregate on the client and append additional pings. This would be more in line with bug 975235.

Comment 8

[Ed Lee :Mardak]

To be clear, we would want to start using "simple measurement" to add in the extra data that we are discussing in bug 975235 (e.g., list id, scores, grid size).

Comment 9

[Ed Lee :Mardak]

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

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/f3bc97e83968

Comment 11

[Petruta Horea [:phorea], Desktop QA]

Attachment: TelemetryResults (2).png

In order to verify this, I opened newtab page and removed some sponsored tiles. I reloaded the page for 3 times. The ones left were: 1. facebook, 2. mozilla, 3. Wikipedia, 4.bbc, 5.Webmaker.
The only data I have in about:telemetry is:

NEWTAB_PAGE_DIRECTORY_AFFILIATE_SHOWN
3 samples, average = 0, sum = 0
3
0 1
NEWTAB_PAGE_DIRECTORY_ORGANIC_SHOWN
3 samples, average = 0, sum = 0
3
0 1
NEWTAB_PAGE_DIRECTORY_SPONSORED_SHOWN
3 samples, average = 0, sum = 0
3
0 1
NEWTAB_PAGE_SHOWN
3 samples, average = 1, sum = 3
  3
0 1

Mardak, could you please let me know how this data can be interpreted? Why only facebook appears as affiliate, organic and sponsored and what happened with the other tiles? Thank you!

Comment 12

[Ed Lee :Mardak]

A couple things look wrong. All 3 of those directory_*_shown probes should have been non-0 -- this functionality was added and tested in bug 972936. The new probes added in this bug for directory_link*_shown are not being reported at all ?

Comment 13

[Petruta Horea [:phorea], Desktop QA]

(In reply to Ed Lee :Mardak from comment #12)
> A couple things look wrong. All 3 of those directory_*_shown probes should
> have been non-0 -- this functionality was added and tested in bug 972936.
> The new probes added in this bug for directory_link*_shown are not being
> reported at all ?
It seems that this kind of data is produced when restarting the browser having about:telemetry and about:newtab pages in view. Opening (or refreshing) the existing new tab page shows only the 3 directory_*_shown with 0 probes. Opening a new tab page displays the correct data. (I restarted the browser to obtain new data each time)

I managed to verify the directory_link*_shown data using latest Aurora 32.0a2 20140615004003 under Win 7 64-bit, Ubuntu 32-bit and Mac OSX 10.9.3: the data indicates correctly the tiles position: at startup, after removing tiles from several positions, after changing  the number of rows and columns in about:config.

The only thing I found so far is that directory_link*_shown is displayed even when not all the tiles are in view - the browser is in window mode and only a part of the tiles can fit the screen.

Should I file a new bug for this? Are there any other scenarios I should test? Thank you!

Comment 14

[[SV Manager] Florin Mezei, QA (:FlorinMezei)]

It seems no additional information was received for a few weeks now. Marco, how should we proceed with this? Should we make the call here?

Comment 15

[Marco Mucci [:MarcoM]]

(In reply to Florin Mezei, QA (:FlorinMezei) from comment #14)
> It seems no additional information was received for a few weeks now. Marco,
> how should we proceed with this? Should we make the call here?

Hi Florin, let's verify for now and Petruta can file a new bug as mentioned in comment #13.  Thanks.

Comment 16

[Ed Lee :Mardak]

Yes, please file a new bug to track the visibility or not of a tile. Thanks

Comment 17

[Petruta Horea [:phorea], Desktop QA]

Marking as verified as per comment 13. Logged bug 1034502 for the remaining issue.