1853004 - Investigate why ads_hidden is sometimes unequal to ads_loaded

Status: VERIFIED FIXED

(Firefox :: Search, defect, P2)

Description

[James Teow [:jteow]]

Blocked ads should have the same number of loaded ads as ad blockers hide them from users.

Data Science let us know that in a sample of data, they found 2.5% of the time, ads_hidden (when it had a non-zero number) had a number lower than ads_loaded for the corresponding component.

In that sample, it only happened with ad_link and ad_sidebar.

We should investigate 1) which search provider this is happening with, and 2) why this is happening.

Comment 1

[James Teow [:jteow]]

Attachment: Bug 1853004 - Add ad_image_row to ad components - r?standard8!

Comment 2

[James Teow [:jteow]]

One reason for the discrepancy is because we're not properly categorizing one of the ads in one of the providers, so it falls back to the default ad type, ad_link which will use the anchor itself to see if it is hidden. In this case, the anchor is indeed hidden even if the ad is actually shown to the user.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/dc0996a4dcf5
Add ad_image_row to ad components - r=Standard8

Comment 4

[James Teow [:jteow]]

Cristian, I've added a changes to Remote Settings Stage - Preview to search-telemetry-v2. Could you validate that the proposed change is working properly?

The steps to reproduce are hard because this type of ad won't always show up if you do a search so I've had to be persistent (e.g. search a term that has a high likelihood of showing ads over and over again) and/or clear cookies.

Here is what works for me:

1. On Google, search "shop shoes" or "shop candles" in US/Canada/UK (to be clear, I've only tested these locales, it probably shows up in all locales).
2. Look at the ad at the top: Keep submitting that search (or clear cookies if you stop seeing ads) until you see an ad that has a fixed number of images across the top. It looks like a carousel because products will be inside boxes, but isn't a carousel because it won't have arrows that scroll to show more images to load.
3. In the logs, one of the entries should be: "Counting ad:" ({type:"ad_image_row", adsLoaded:n, adsVisible:n, adsHidden:0}) where n is the number of images.

Comment 5

[Sandor Molnar[:smolnar]]

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

Comment 6

[James Teow [:jteow]]

There's an issue with syncing with records on Remote Setting Stage that isn't because of this patch which might affect QA, but the fix should be in Nightly shortly.

Comment 7

[James Teow [:jteow]]

We're moving the Remote Settings change to Bug 1856647.

Search telemetry v2 on production was not changed in this bug.

Comment 8

[Cristian Baica [:cbaica], Release Desktop QA]

Marking the issue as verified fixed using RS stage-preview and Fx 120.0a1.
After a number of searches and cookie clears I've managed to trigger "ad_image_row" ad type. They're registered in glean as well as browser console whenever triggered.

Comment 9

[James Teow [:jteow]]

Thanks for verifying it Cristian.

I've reverted the change on Stage because of the miscount you found. It doesn't make sense to commit it when we know its flawed.

Comment 10

[James Teow [:jteow]]

Comment on attachment 9353463 [details]
Bug 1853004 - Add ad_image_row to ad components - r?standard8!

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: A Remote Settings change to search-telemetry-v2.json is causing issues with test_search_telemetry_config_validation.js on ESR because when the test checks search-telemetry-v2.json, it will find a value inside "type" that is missing from the list of allowable strings.
• User impact if declined: None, but the unit test test_search_telemetry_config_validation.js will continue to fail.
• Fix Landed on Version:
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): It only changes documentation and a JSON file used in test_search_telemetry_config_validation.js.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9353463 [details]
Bug 1853004 - Add ad_image_row to ad components - r?standard8!

Approved for 115.5esr

Comment 12

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr115/rev/9800392efc7f

Comment 13

[Donal Meehan [:dmeehan]]

Comment on attachment 9353463 [details]
Bug 1853004 - Add ad_image_row to ad components - r?standard8!

Approved for 119.0.1

Comment 14

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-release/rev/ab5b08a89882

Comment 15

[Cristian Baica [:cbaica], Release Desktop QA]

Issue is verified fixed using the latest Fx 115.5ESR and 119.0.1 builds on Windows 10 and Ubuntu 20. ad_image_row is correctly registered in SERP telemetry logs.