Closed Bug 1853004 Opened 10 months ago Closed 10 months ago

Investigate why ads_hidden is sometimes unequal to ads_loaded

Categories

(Firefox :: Search, defect, P2)

defect

Tracking

()

VERIFIED FIXED
120 Branch
Tracking Status
firefox-esr115 --- verified
firefox119 --- verified
firefox120 --- verified

People

(Reporter: jteow, Assigned: jteow)

References

Details

(Whiteboard: [sng])

Attachments

(1 file)

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.

Assignee: nobody → jteow
Status: NEW → ASSIGNED

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.

Depends on: 1854958
Pushed by jteow@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/dc0996a4dcf5
Add ad_image_row to ad components - r=Standard8

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.
Flags: needinfo?(cbaica)
Status: ASSIGNED → RESOLVED
Closed: 10 months ago
Resolution: --- → FIXED
Target Milestone: --- → 120 Branch

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.

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

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

Flags: needinfo?(cbaica)

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.

Status: RESOLVED → VERIFIED

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 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.
Attachment #9353463 - Flags: approval-mozilla-esr115?
Attachment #9353463 - Flags: approval-mozilla-release?

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

Approved for 115.5esr

Attachment #9353463 - Flags: approval-mozilla-esr115? → approval-mozilla-esr115+

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

Approved for 119.0.1

Attachment #9353463 - Flags: approval-mozilla-release? → approval-mozilla-release+

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.

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: