Closed Bug 1161656 Opened 5 years ago Closed 5 years ago

Report total views preceding a click

Categories

(Firefox :: New Tab Page, defect)

defect
Not set
Points:
8

Tracking

()

RESOLVED FIXED
Firefox 42
Iteration:
42.1 - Jul 13
Tracking Status
firefox40 --- affected
firefox42 --- fixed

People

(Reporter: mzhilyaev, Assigned: mzhilyaev)

References

(Blocks 1 open bug)

Details

(Whiteboard: .002)

Attachments

(1 file, 6 obsolete files)

We want to compute optimal frequency cap for a tile.
For that we may attach number of total views preceding a click to a click ping.
This will be trivial to do when Bug 1159571 lands, since all views are collected in a frequency cap object maintained by DirectoryLinksProvider
This should probably include the different types of clicks including block.
Moving this to Fx40.
Blocks: 1145418
Iteration: 40.3 - 11 May → ---
Iteration: --- → 41.3 - Jun 29
Whiteboard: .? → .002
Assignee: nobody → mzhilyaev
report pastImpressions for suggested tiles on any action but "view"
Attachment #8621130 - Flags: review?(msamuel)
Comment on attachment 8621130 [details] [diff] [review]
v1.  first cut at pastImpressions report

>+        pastImpressions = (this._frequencyCaps[url]) ? this._frequencyCaps[url].totalViews : undefined;
It could be useful to report both the current daily view and total views. Probably check with jterry on the usefulness?
(In reply to Ed Lee :Mardak from comment #4)

> >+        pastImpressions = (this._frequencyCaps[url]) ? this._frequencyCaps[url].totalViews : undefined;
> It could be useful to report both the current daily view and total views.
> Probably check with jterry on the usefulness?

I really do not see how this could help us. If we want to answer "how many days pass before click", or "does click mostly happen when user sees it first", we should keep days-after-first-impression-counter and report that. But then we would have to have a day-cap attached to a suggested tile much like we have frequency cap
The user doesn't have to exceed the daily cap each day, so we can't calculate how many days anyway.

Sending the daily cap would allow us to know if the user clicked on it on the first showing of a new day. E.g., the user saw it the previous few days various times with a total count of 8 then clicked the very first time the suggested tile was shown on the new day with a daily count of 1.

This could be because the user wanted to click on a suggestion but couldn't because it was frequency capped out on the previous day.
jterry, if we only sent one frequency cap value, should it be how many times it has shown in the current day or how many times it has been shown across all days?
Flags: needinfo?(jterry)
Just to clarify my thinking on it:  there is absolutely no problem sending both.
But more to Ed's point above - looking at daily past_impressions what can we tell?  We do not know the past daily impressions, hence it will be hard to make statistical inference on how reducing/increasing daily cap will affect the campaign.  

With total past impressions we will empirically know that a given campaign does not generate a click after 5 "views", hence we just set its frequency cap to 5 and we are good. Doing same inference from reports of last day campaign impressions... i am not clear how (but it may be possible)

We could collect daily impressions per each day of campaign and report an array instead of a single number.  From this we can compute probability distributions of a click (or a pin/block) for prior impressions, or days, or daily impressions, or their combination.

Should we report that instead?
I'm not talking about the previous daily amounts. I'm talking about the current day's count so far.
Comment on attachment 8621130 [details] [diff] [review]
v1.  first cut at pastImpressions report

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

Cool looks good so far. In terms of whether we also send daily views, I think as Ed was saying, it does help us make a guess if someone was perhaps waiting for another impression the next day so they clicked it immediately. In which case, we *might* need to increase the daily view cap. I think to Max's point though, perhaps the user did not *ever* reach the daily frequency cap even though they clicked the ad on the first view of a day. So I'm also not sure how collecting the daily views for only the current day would give us enough information for a daily view cap change.

::: browser/modules/test/xpcshell/test_DirectoryLinksProvider.js
@@ +1976,5 @@
> +    DirectoryLinksProvider.reportSitesAction(sites, action, index);
> +    return deferred.promise;
> +  }
> +
> +  // Start with a view ping virst

virst -> first

@@ +2002,5 @@
> +
> +  expectedImpressions = DirectoryLinksProvider._frequencyCaps[testUrl].totalViews;
> +  do_check_eq(expectedImpressions, 3);
> +
> +  // now report pin, unpin and click

What about 'block'?
Attachment #8621130 - Flags: review?(msamuel)
It's also interesting to note that if we are to send the full ad-view-history (views for days) for clicks and blocks only, the user is not tractable since this information is only submitted ones and the ad is never shown again to a user.
Marina makes some good points here as well. But, ideally we would be able to see impressions prior to clicks (pins/blocks or any other engagement as well) on a daily tile-by-tile basis, as well as a category-level basis. It would also be useful/interesting to see impressions prior to clicks for several days leading up to the click. Perhaps days prior to click is a separate metric? Something like average impressions per day, per Tile (or Category) and then avg impressions/days per click (or any other engagement).

Regarding why we'd need daily views for the current day, it will likely be the case, if we have numerous content partners, that we may need to make some tweaks to test out different pieces of content. Really we just need the flexibility as we learn how people use/engage with our product.
Flags: needinfo?(jterry)
corrected reviewer comments and added daily views to past_impressions report
Attachment #8621130 - Attachment is obsolete: true
Attachment #8622577 - Flags: review?(adw)
Attachment #8622577 - Flags: review?(adw) → review+
Comment on attachment 8622577 [details] [diff] [review]
v2.  added daily views to past_impressions

>+++ b/browser/modules/DirectoryLinksProvider.jsm
>           // Make the payload in a way so keys can be excluded when stringified
>           let id = link.directoryId;
>           tiles.push({
>             id: id || site.enhancedId,
>             pin: site.isPinned() ? 1 : undefined,
>             pos: pos != tilesIndex ? pos : undefined,
>+            past_impressions: pos == triggeringSiteIndex ? pastImpressions : undefined,
You need to update the .rst and request f?bsmedberg
Added "past_impressions" example and explanation to ping object description
Attachment #8622577 - Attachment is obsolete: true
Attachment #8623957 - Flags: feedback?(benjamin)
Comment on attachment 8623957 [details] [diff] [review]
v3. Updated DirectoryLinksProvider.rst

>+++ b/browser/docs/DirectoryLinksProvider.rst
>@@ -241,16 +241,17 @@ followed by a history tile and a directory tile. The first tile is being
>               "id": 702,
>               "pin": 1,
>+              "past_impressions": {"total": 5, "daily": 1},
nit: move this above pin
> - ``url`` - empty string if it's an enhanced tile; not present otherwise
>+- ``past_impressions`` - total and current day impressions (or new tab "views")
nit: keep this sorted alphabetically, so move this up

>+++ b/browser/modules/DirectoryLinksProvider.jsm
>-  _setFrequencyCapClick: function DirectoryLinksProvider_reportFrequencyCapClick(url) {
>+  _setFrequencyCapClick: function DirectoryLinksProvider_setFrequencyCapClick(url) {
nit: if you're just fixing up the name, might as well just use the new syntax
_setFrequencyCapClick(url) {
Attached patch v4. fixed reviewer comments (obsolete) — Splinter Review
waiting on feedback from bsmedberg on past_impressions reporting
Attachment #8623957 - Attachment is obsolete: true
Attachment #8623957 - Flags: feedback?(benjamin)
Attachment #8624828 - Flags: feedback?(benjamin)
Blocks: 1172713
No longer blocks: 1145418
oyiptong, reminder to look into server-side ingest/onyx bugs.
Flags: needinfo?(oyiptong)
Comment on attachment 8624828 [details] [diff] [review]
v4. fixed reviewer comments

Past impressions over what time period? Is there a rolling window or forever? Forever sounds like a problem in general.

"total" and "daily" aren't documented. Is daily just "today" (and if so is that since local midnight or past 24 hours)? Or is that a "total divided by # of days this tile has been seen"?
Attachment #8624828 - Flags: feedback?(benjamin) → feedback-
Flags: needinfo?(oyiptong)
- supplied description of total and daily counters
- regarding the total counter: it's a total counter that accumulates up to frequency cap impressions for the live of campaign. The default total frequency cap is 10 impressions, however the default can be changed by the server. 

This information is likely to be reported only once when a user clicks, blocks or pins a suggested tile, which makes me unclear on the privacy concerned reporting total number of impressions for a tile may cause. Please advice
Attachment #8624828 - Attachment is obsolete: true
Attachment #8627329 - Flags: feedback?(benjamin)
Comment on attachment 8627329 [details] [diff] [review]
v5. Clarified total and daily counters usage

The things I still don't understand, even after reading the docs:

Sequence I:
* user runs the browser, sees tile X once
* next day, user runs the browser, sees tile X once
* later, runs the browser again, sees tile X once more, pins it, then clicks it

Past impressions will be submitted twice, once for the pin and once for the click.
For the pin, will the data be {"total": 3, "daily": 2}? If so, what is the cutoff for daily? Local midnight?
Does the total immediately reset? If so the data would be:
{"total": 0, "daily": 2}
Or perhaps {"total": 0, "daily": 0}
Or if no reset {"total": 3, "daily": 2}

If it doesn't reset, aren't you risking counting the impressions multiple times? Isn't the purpose of this data to get accurate impression counts?

Sequence II (continuation):
Now the user has a pinned suggested tile. Every time the users clicks the tile (forever? or until the current campaign ends?), we will report the past_impressions data. If that data doesn't reset, the "total" will keep growing. It is highly likely that we could use the growth of that number to track a user over time.

Hence my confusion over the lifetime of how we're sending the data and the resetting-or-not behavior.
Attachment #8627329 - Flags: feedback?(benjamin)
Depends on: 1178586
(In reply to Benjamin Smedberg  [:bsmedberg] from comment #21)

Apologies for the poor clarification. I think you are correct: pinned suggested tile will send "past-impressions" many times over, hence a potential for user tracking. But let me detail how impression counting works. For the purpose of frequency capping (bug 1159571), each suggested tile has an object that accumulates tile's "total" and "daily" impressions. When a tile is displayed (viewed) both the "total" and the "daily" counters are incremented.  However, the object also contains a day-stamp of last view; if this day-stamp is different from the day of the immediate "view" action, the "daily" counter is reset (but "total" counter keeps increasing).  

Now consider your sequence:

> Sequence I:
> * user runs the browser, sees tile X once
  {total: 1, daily: 1}
> * next day, user runs the browser, sees tile X once
  {total: 2, daily: 1}  ### note that daily was reset and then incremented
> * later, runs the browser again, sees tile X once more
  {total: 3, daily: 2}

Now, a user pins the tile, what happens?
The fix to bug 1145428 turns pinned suggested tile into a regular "history" tile.
The "views" of a "history" tile are not tracked.
After a pin, the "past impressions" are the same:
  {total: 3, daily: 2}

Now a user clicks the tile, but again, it's just a "history" tile, and "past impressions" are same:
  {total: 3, daily: 2}

No matter how many times a user views/clicks the pinned tile, 
the "past impressions" remain what they were when the tile was first pinned.
  {total: 3, daily: 2}

> Hence my confusion over the lifetime of how we're sending the data and the resetting-or-not behavior.

Just a note on suggested tile lifetime - the tile is shown until:
 - it's being clicked or blocked
 - it's pinned (note bug 1178586 - which i am fixing now)
 - it exceeds its frequency cap: 
   exceeding "total" will remove tile forever
   exceeding "daily" will stop tile for just one day
 - it's removed from "directory source file"
 - it passes the campaign end time

In all of these cases "past-impressions" is reported either once, or same "past-impressions" will be reported on repeated clicks from a pinned "used-to-be-suggested" tile.  Which I think is track-able, and I agree that should be avoided. One possible work around is to report "past-impressions" on the pin and never again for that tile; this way "past-impressions" are reported once no matter what click action is.  Please, suggest other solutions or verify that the proposed workaround is acceptable.
Flags: needinfo?(benjamin)
ok, I think we're on the same page: for each suggested tile, we will only report past_impressions once, because:

* on click, we submit and then stop showing a suggested tile
* on pin, we submit, the tile converts to a history tile and past_impressions is no longer relevant
* on block, we submit and stop showing the tile
Flags: needinfo?(benjamin)
Iteration: 41.3 - Jun 29 → 42.1 - Jul 13
Attachment #8627329 - Attachment is obsolete: true
Attachment #8630677 - Flags: review?(adw)
Attachment #8630677 - Flags: feedback?(benjamin)
Attachment #8630677 - Flags: feedback?(benjamin) → feedback+
Comment on attachment 8630677 [details] [diff] [review]
v6. clarification of docs and rebasing on 1178586 fix

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

::: browser/docs/DirectoryLinksProvider.rst
@@ +265,5 @@
>  - ``id`` - id that was provided as part of the downloaded link object (for all
>    types of links: directory, suggested, enhanced); not present if the tile was
>    created from user behavior, e.g., visiting pages
> +- ``past_impressions`` - number of impressions (new tab "views") a suggested was
> +  shown tile before it was clicked, pinned or blocked. Where the "total" counter

This sentence is mangled.

@@ +267,5 @@
>    created from user behavior, e.g., visiting pages
> +- ``past_impressions`` - number of impressions (new tab "views") a suggested was
> +  shown tile before it was clicked, pinned or blocked. Where the "total" counter
> +  is the overall number of impressions accumulated prior to a click action, and
> +  "daily" counter is the number impressions occurred on the day of the click action.

Should say what "day" means: a calendar day or a 24-hour period before the action.
Attachment #8630677 - Flags: review?(adw) → review+
Attachment #8630677 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/dc2cd47542da
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 42
You need to log in before you can comment on or make changes to this bug.