-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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 Bid Adapter: normalize pbs adapter auction ids #6836
Prebid Server Bid Adapter: normalize pbs adapter auction ids #6836
Conversation
…ebid.js into master-rubicon-clean
# Conflicts: # modules/advangelistsBidAdapter.js # test/spec/modules/advangelistsBidAdapter_spec.js
…ter-remote-updated
@idettman pulling in upstream should get rid of the current circle ci errors |
id: s2sBidRequest.tid, | ||
source: {tid: s2sBidRequest.tid}, | ||
id: firstBidRequest.auctionId, | ||
source: {tid: firstBidRequest.auctionId}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is possible that publishers can pass in their own auctionId
's into requestBids
And in this case, we would want to be sure source.tid is always a UUID.
So after speaking with @bretg I think we should just generate a random UUID for source.tid here every time.
utils.generateUUID()
@bretg Can you comment and confirm?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch on the pub-specified auctionId. source.tid needs to be the same even if there are multiple PBS vendors. i.e. generate a new UUID for source.tid before we start looping through the vendors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi there, can we move forward on this PR? I can help if needed.
cc @idettman
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@osazos - your help would be appreciated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @bretg, I finally got time to work on this one.
Regarding your last comment I would like to be sure we want this behavior because it is a breaking change. Actually a large part of the process is based on the fact the tid
is unique by Vendor. See there:
Prebid.js/src/adapterManager.js
Lines 346 to 370 in 07ab54d
var uniqueServerBidRequests = []; | |
serverBidRequests.forEach(serverBidRequest => { | |
var index = -1; | |
for (var i = 0; i < uniqueServerBidRequests.length; ++i) { | |
if (serverBidRequest.tid === uniqueServerBidRequests[i].tid) { | |
index = i; | |
break; | |
} | |
} | |
if (index <= -1) { | |
uniqueServerBidRequests.push(serverBidRequest); | |
} | |
}); | |
let counter = 0 | |
_s2sConfigs.forEach((s2sConfig) => { | |
if (s2sConfig && uniqueServerBidRequests[counter] && includes(s2sConfig.bidders, uniqueServerBidRequests[counter].bidderCode)) { | |
// s2s should get the same client side timeout as other client side requests. | |
const s2sAjax = ajaxBuilder(requestBidsTimeout, requestCallbacks ? { | |
request: requestCallbacks.request.bind(null, 's2s'), | |
done: requestCallbacks.done | |
} : undefined); | |
let adaptersServerSide = s2sConfig.bidders; | |
const s2sAdapter = _bidderRegistry[s2sConfig.adapter]; | |
let tid = uniqueServerBidRequests[counter].tid; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for looking at this @osazos. It would take me some time to sort out what the heck this thing is doing with "uniqueServerBidRequests", but I can say that source.tid needs to be the same for multiple PBS calls in the same auction.
So maybe we just leave "uniqueServerBidRequests" there and just generate a brand new thing to use as source.tid for the auction.
Addressed in new PR #7585 |
Type of change
Description of change
Normalize PBJS and PBS auction IDs #6563
Update for Issue: #6563