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

Prebid Server Adapter local storage issue affecting /cookie_sync #1582

Closed
bretg opened this issue Sep 13, 2017 · 2 comments
Closed

Prebid Server Adapter local storage issue affecting /cookie_sync #1582

bretg opened this issue Sep 13, 2017 · 2 comments
Labels

Comments

@bretg
Copy link
Collaborator

bretg commented Sep 13, 2017

Type of issue

Bug

Description

Under certain conditions, the Prebid Server Adapter code is improperly skipping the send of /cookie_sync requests. The /cookie_sync request does appear to have a solid impact on cookie match rate based on a limited data set.

The main impact here is that the current design will affect users who hit web sites using different clusters of Prebid Servers.

The code in question is in prebidServerBidAdapter.js:

    if (_cookiesQueued || syncedList.length === bidderCodes.length) {
      return;
    }

So /cookie_sync isn't sent in two scenarios:

  1. if there's already been a /cookie_sync request sent
  2. if the number of adapters in localStorage (pbjsSyncs) matches the number of adapters in the AdUnit

I ran into this when I had 2 bidders in localStorage (rubicon and appnexus), and 2 bidders in an AdUnit -- rubicon and a different bidder (not appnexus). Didn't see cookie_sync go out, despite there not being a uids cookie. The code is taking a shortcut here by comparing list length rather than a deep comparison. Also, it doesn't seem right that it's comparing the the pbjsSyncs list with the bids in the AdUnits -- should be comparing the bidders configured for S2S.

But more importantly, the design doesn't currently consider that a single domain might hit two clusters of Prebid Servers (e.g. A/B mode), or change from one cluster to another. In that scenario, the page will suppress /cookie_sync, which has a noticeably positive effect on user ID match rates.

Steps to reproduce

  1. Set localStorage (pbjsSyncs) to contain two entries. e.g. ["rubicon", "appnexus"]
  2. Set up an AdUnit with 2 bidders. e.g. "appnexus" and "pubmatic"
  3. Set up the S2S config with 1 bidder. e.g. "appnexus"
  4. remove the uids cookie

Expected results

The /cookie_sync request should go out because the uids cookie is empty.

Actual results

/cookie_sync won't go out because there are two entries in pbjsSyncs localStorage and and two bidders in the AdUnit

@mkendall07
Copy link
Member

thanks for the bug report. We can store sync flag in the localstorage namespace of the endpoint for the problem of multiple instances of prebid server if that makes sense. Although I'd think the cookies should probably be shared (technically not feasible with cookie but might be with localstorage). I'd propose fixing in 2 steps, the first fix being to make sure everything is synced and then the 2nd being handling multiple instance of PBS.

mkendall07 added a commit that referenced this issue Sep 18, 2017
matthewlane pushed a commit that referenced this issue Sep 19, 2017
* fix for #1582

* fixed typo and add unit tests
jbAdyoulike pushed a commit to jbAdyoulike/Prebid.js that referenced this issue Sep 21, 2017
* fix for prebid#1582

* fixed typo and add unit tests
@stale
Copy link

stale bot commented Mar 6, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Mar 6, 2018
dluxemburg pushed a commit to Genius/Prebid.js that referenced this issue Jul 17, 2018
* fix for prebid#1582

* fixed typo and add unit tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants