Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

splitTestGroup ads are shown when AdvertiserSplitTestStudy is not defined in seed #15164

Closed
btlechowski opened this issue Apr 7, 2021 · 4 comments · Fixed by brave/brave-core#8697

Comments

@btlechowski
Copy link

btlechowski commented Apr 7, 2021

Follow up to #14613

This could skew the a/b testing results. For example when seed becomes available, but some split testing ads have already been shown.

The splitTestGroup ads should only be shown when the Group is defined in AdvertiserSplitTestStudy in the seed variation.

Steps to Reproduce

  1. Follow Notification ad - AdvertiserSplitTestStudy is not defined in seed from https://github.com/brave/internal/issues/762

Actual result:

splitTestGroup ad is shown.

[22735:22735:0407/025832.503329:VERBOSE1:ad_notification_serving.cc(98)] Ad notification delivered:
  uuid: f59741bf-7883-45f3-a43a-7b6301ac2ad6
  creativeInstanceId: 54592ab6-5a8e-42e6-8e11-9e9c04e9eecb
  creativeSetId: c7e7a381-4c74-4dd9-9b6d-4edf9330454b
  campaignId: 0d01425b-c970-4a25-b2d0-ccf748cfaf6e
  segment: untargeted
  title: Ad - GroupB
  body: Ad - GroupB
  targetUrl: https://youtube.com

Expected result:

splitTestGroup ads are filtered out

Reproduces how often:

Easily reproduced

Brave version (brave://version info)

Brave 1.23.63 Chromium: 89.0.4389.114 (Official Build) beta (64-bit)
Revision 1ea76e193b4fadb723bfea2a19a66c93a1bc0ca6-refs/branch-heads/4389@{#1616}
OS Ubuntu 18.04 LTS

cc @moritzhaller @tmancey @jsecretan @brave/legacy_qa

@moritzhaller
Copy link

This is intended behaviour as we were optimising for pacing hence allowing the ad if a client didn't sign up. @jsecretan @tmancey what do you think?

@tmancey
Copy link
Contributor

tmancey commented Apr 30, 2021

@moritzhaller Agreed.

@jsecretan
Copy link

Ok I actually think the behavior you were mentioning is the right one @btlechowski, if a client doesn't have the group defined, it shouldn't participate in the campaign. This should be rare in practice, so I'll put a low priority on this one.

@btlechowski
Copy link
Author

Verification passed on

Brave 1.26.59 Chromium: 91.0.4472.101 (Official Build) beta (64-bit)
Revision af52a90bf87030dd1523486a1cd3ae25c5d76c9b-refs/branch-heads/4472@{#1462}
OS Linux

Verified test plan from the description
Verified ads are not shown.

[7236:7236:0614/162814.829271:VERBOSE1:ad_notification_serving.cc(174)] Serve ad for segments:
[7236:7236:0614/162814.829432:VERBOSE1:ad_notification_serving.cc(176)]   untargeted
[7236:7236:0614/162814.830894:VERBOSE2:exclusion_rule_util.h(26)] creativeSetId 5e96fecc-2926-4c72-bb76-4e745bf6b539 excluded as not associated with advertiser split test group
[7236:7236:0614/162814.831602:VERBOSE2:exclusion_rule_util.h(26)] creativeSetId c7e7a381-4c74-4dd9-9b6d-4edf9330454b excluded as not associated with advertiser split test group
[7236:7236:0614/162814.831707:VERBOSE1:ad_notification_serving.cc(190)] No eligible ads found for segments
[7236:7236:0614/162814.831811:VERBOSE1:ad_notification_serving.cc(206)] Serve ad for parent segments:
[7236:7236:0614/162814.831902:VERBOSE1:ad_notification_serving.cc(208)]   untargeted
[7236:7236:0614/162814.833244:VERBOSE2:exclusion_rule_util.h(26)] creativeSetId 5e96fecc-2926-4c72-bb76-4e745bf6b539 excluded as not associated with advertiser split test group
[7236:7236:0614/162814.833937:VERBOSE2:exclusion_rule_util.h(26)] creativeSetId c7e7a381-4c74-4dd9-9b6d-4edf9330454b excluded as not associated with advertiser split test group
[7236:7236:0614/162814.834054:VERBOSE1:ad_notification_serving.cc(222)] No eligible ads found for parent segments
[7236:7236:0614/162814.834193:VERBOSE1:ad_notification_serving.cc(235)] Serve untargeted ad
[7236:7236:0614/162814.835350:VERBOSE2:exclusion_rule_util.h(26)] creativeSetId 5e96fecc-2926-4c72-bb76-4e745bf6b539 excluded as not associated with advertiser split test group
[7236:7236:0614/162814.846894:VERBOSE2:exclusion_rule_util.h(26)] creativeSetId c7e7a381-4c74-4dd9-9b6d-4edf9330454b excluded as not associated with advertiser split test group
[7236:7236:0614/162814.847062:VERBOSE1:ad_notification_serving.cc(251)] No eligible ads found for untargeted segment
[7236:7236:0614/162814.847222:VERBOSE1:ad_notification_serving.cc(252)] Ad notification not served: No eligible ads found
[7236:7236:0614/162814.847324:VERBOSE1:ad_notification_serving.cc(91)] Ad notification not delivered

Verified splitTestGroup ads are shown when group is defined in the seed:
image

@tmancey tmancey added this to Ads Jun 10, 2024
@tmancey tmancey moved this to Done in Ads Jun 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
4 participants