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.

Sign up for GitHub

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

Jump to bottom

Magnite Analytics Adapter : add seat non bid handling #9696

Merged
merged 8 commits into from
Apr 13, 2023
Merged

Magnite Analytics Adapter : add seat non bid handling #9696

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
ccab78d
Return all bids
spotxslagle Mar 21, 2023
481847d
Adjust findMatch function
spotxslagle Mar 21, 2023
a667e40
Return all buds unit testing
spotxslagle Mar 28, 2023
d4a2145
Responds to review comments
spotxslagle Mar 31, 2023
c90c7d1
Unit test adjustments
spotxslagle Mar 31, 2023
d5e116f
Remove extra line for lint
robertrmartinez Apr 3, 2023
3894aa2
minor changes
robertrmartinez Apr 3, 2023
0d0f275
doh
robertrmartinez Apr 3, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
96 changes: 87 additions & 9 deletions modules/magniteAnalyticsAdapter.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ const {
BIDDER_DONE,
BID_TIMEOUT,
BID_WON,
BILLABLE_EVENT
BILLABLE_EVENT,
SEAT_NON_BID
},
STATUS: {
GOOD,
Expand Down Expand Up @@ -437,12 +438,20 @@ const sizeToDimensions = size => {
};
}

const findMatchingAdUnitFromAuctions = (matchesFunction, returnFirstMatch) => {
const findMatchingAdUnitFromOrderedAuctions = (matchesFunction, returnFirstMatch) => {
return findAdUnitFromAuctions(matchesFunction, returnFirstMatch, cache.auctionOrder);
};

const findMatchingAdUnitFromAllAuctions = (matchesFunction, returnFirstMatch) => {
return findAdUnitFromAuctions(matchesFunction, returnFirstMatch, Object.keys(cache.auctions));
};

const findAdUnitFromAuctions = (matchesFunction, returnFirstMatch, auctions) => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With other proposed change I think all of this can be reverted

// finding matching adUnit / auction
let matches = {};

// loop through auctions in order and adunits
for (const auctionId of cache.auctionOrder) {
for (const auctionId of auctions) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With other proposed change I think all of this can be reverted

const auction = cache.auctions[auctionId].auction;
for (const transactionId in auction.adUnits) {
const adUnit = auction.adUnits[transactionId];
Expand All @@ -464,7 +473,7 @@ const findMatchingAdUnitFromAuctions = (matchesFunction, returnFirstMatch) => {
}
}
return matches;
}
};

const getRenderingIds = bidWonData => {
// if bid caching off -> return the bidWon auction id
Expand All @@ -481,7 +490,7 @@ const getRenderingIds = bidWonData => {
const gamHasRendered = deepAccess(cache, `auctions.${auction.auctionId}.gamRenders.${adUnit.transactionId}`);
return adUnit.adUnitCode === bidWonData.adUnitCode && gamHasRendered;
}
let { adUnit, auction } = findMatchingAdUnitFromAuctions(matchingFunction, false);
let { adUnit, auction } = findMatchingAdUnitFromOrderedAuctions(matchingFunction, false);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With other proposed change I think all of this can be reverted

// If no match was found, we will use the actual bid won auction id
return {
renderTransactionId: (adUnit && adUnit.transactionId) || bidWonData.transactionId,
Expand Down Expand Up @@ -554,7 +563,7 @@ const subscribeToGamSlots = () => {
const gamHasRendered = deepAccess(cache, `auctions.${auction.auctionId}.gamRenders.${adUnit.transactionId}`);
return matchesSlot && !gamHasRendered;
}
let { adUnit, auction } = findMatchingAdUnitFromAuctions(matchingFunction, true);
let { adUnit, auction } = findMatchingAdUnitFromOrderedAuctions(matchingFunction, true);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With other proposed change I think all of this can be reverted


const slotName = `${event.slot.getAdUnitPath()} - ${event.slot.getSlotElementId()}`;

Expand Down Expand Up @@ -821,10 +830,16 @@ magniteAdapter.track = ({ eventType, args }) => {
auctionEntry.floors.dealsEnforced = args.floorData.enforcements.floorDeals;
}

// Log error if no matching bid!
// no-bid from server. Document it!
if (!bid) {
logError(`${MODULE_NAME}: Could not find associated bid request for bid response with requestId: `, args.requestId);
break;
bid = adUnit.bids[generateUUID()] = {
bidder: 'mgnipbs',
source: 'server',
status: 'no-bid',
bidId: args.requestId,
clientLatencyMillis: args.timeToRespond || Date.now() - cache.auctions[args.auctionId].auction.auctionStart
};
return;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For this scenario, I think we want something a bit different.

This is a valid bid response, so in this scenario we actually want to create a new bid in our adUnit

And then just let the rest of the bid_response processing logic do what it normally does:

So something like this:

      // If we still do not have a matching bid object and this bidResponse has a seatBidId, lets create a default one
      if (!bid && args.seatBidId) {
        bid = adUnit.bids[args.seatBidId] = {
          bidder: args.bidderCode,
          bidId: args.seatBidId,
          status: 'no-bid',
          source: 'server',
          unknownBid: true // just so we can see how often this happens at log level
        }
      }

      // finally if still no bid just bail like before
      if (!bid) {
        logError(`${MODULE_NAME}: Could not find associated bid request for bid response with requestId: `, args.requestId);
        break;
      }

So we just have a new check before this !bid check to see if it falls into our unknown bid response from PBS scenario. And if it does, then we can just create a bid on our adUnit, set it to its defaults, and then let the regular bid_response processing go.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most of that sounds good to me, except letting regular bid response processing go. When that happens, we get this:
Screenshot 2023-03-30 at 4 14 16 PM
Some of that could be weirdness with how we mock the getStatusCode function, but do we really want any of the rest besides latency? And if we want to avoid copy pasta with latency, I could just put that code into a function that gets called twice.

}

// set bid status
Expand Down Expand Up @@ -852,6 +867,9 @@ magniteAdapter.track = ({ eventType, args }) => {
bid.pbsBidId = pbsBidId;
}
break;
case SEAT_NON_BID:
handleNonBidEvent(args);
break;
case BIDDER_DONE:
const serverError = deepAccess(args, 'serverErrors.0');
const serverResponseTimeMs = args.serverResponseTimeMs;
Expand Down Expand Up @@ -939,6 +957,66 @@ magniteAdapter.track = ({ eventType, args }) => {
}
};

const handleNonBidEvent = function(args) {
let {seatnonbid, auctionId} = args;
seatnonbid.forEach(seatnonbid => {
let {seat} = seatnonbid;
seatnonbid.nonbid.forEach(nonbid => {
let {status, impid} = nonbid;
let matchByImpid = matchAuctionBeforeAdUnit(auctionId, impid)
let {adUnit} = findMatchingAdUnitFromAllAuctions(matchByImpid, true);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not think it has to loop through all auctions.

Since we are given the aucitonId by the event, we can just grab the related cache auction and then find the right adUnit:
image

Perhaps it can be simplified to something like:

const handleNonBidEvent = function(args) {
  let { seatnonbid, auctionId } = args;
  // grab the auction
  const auction = deepAccess(cache, `auctions.${auctionId}.auction`);

  // grab the adUnits for this auction
  const adUnits = auction.adUnits;
  seatnonbid.forEach(seatnonbid => {
    let {seat} = seatnonbid;
    seatnonbid.nonbid.forEach(nonbid => {
      try {
        let { status, impid } = nonbid;

        // find matching transactionId for this nonbid and use it to grab the matching adUnit object
        const matchingTid = Object.keys(adUnits).find(tid => adUnits[tid].adUnitCode === impid);
        const adUnit = adUnits[matchingTid];

        let statusInfo = statusMap[status] || { status: 'no-bid' }
        adUnit.bids[generateUUID()] = {
          source: 'server',
          bidder: seat,
          isSeatNonBid: true,
          clientLatencyMillis: Date.now() - auction.auctionStart,
          ...statusInfo
        };
      } catch (error) {
        logWarn(`Unable to match nonbid to adUnit`);
      }
    });
  });
};

Then we can remove all the other changes to the looping functions findMatchingAdUnitFromAuctions etc

try {
let statusInfo = statusMap[status] || { status: 'no-bid' }
adUnit.bids[generateUUID()] = {
source: 'server',
bidder: seat,
isSeatNonBid: true,
clientLatencyMillis: Date.now() - cache.auctions[args.auctionId].auction.auctionStart,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can be simplified to

clientLatencyMillis: Date.now() - auction.auctionStart,

since auction is saved above

...statusInfo
};
} catch (error) {
logWarn(`Unable to match nonbid to adUnit`);
}
});
});
};

const matchAuctionBeforeAdUnit = function(auctionId, impid) {
return function (adUnit, auction) {
if (auctionId !== auction.auctionId) return false;
return adUnit.adUnitCode === impid;
}
};
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

think this can be removed when the refactor is done


const statusMap = {
0: {
status: 'no-bid'
},
100: {
status: 'error',
error: {
code: 'request-error',
description: 'general error'
}
},
101: {
status: 'error',
error: {
code: 'timeout-error',
description: 'prebid server timeout'
}
},
200: {
status: 'rejected'
},
202: {
status: 'rejected'
},
301: {
status: 'rejected-ipf'
}
};

adapterManager.registerAnalyticsAdapter({
adapter: magniteAdapter,
code: 'magnite',
Expand Down
127 changes: 126 additions & 1 deletion test/spec/modules/magniteAnalyticsAdapter_spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ const {
BIDDER_DONE,
BID_WON,
BID_TIMEOUT,
BILLABLE_EVENT
BILLABLE_EVENT,
SEAT_NON_BID
}
} = CONSTANTS;

Expand Down Expand Up @@ -160,6 +161,23 @@ const MOCK = {
'status': 'rendered',
getStatusCode: () => 1,
},
NO_BID: {
'requestId': 'fakeId',
'transactionId': '7b10a106-89ea-4e19-bc51-9b2e970fc42a',
'auctionId': '99785e47-a7c8-4c8a-ae05-ef1c717a4b4d',
'adUnitCode': 'box',
getStatusCode: () => 1,
},
SEAT_NON_BID: {
auctionId: '99785e47-a7c8-4c8a-ae05-ef1c717a4b4d',
seatnonbid: [{
seat: 'rubicon',
nonbid: [{
status: 1,
impid: 'box'
}]
}]
},
AUCTION_END: {
'auctionId': '99785e47-a7c8-4c8a-ae05-ef1c717a4b4d',
'auctionEnd': 1658868384019,
Expand Down Expand Up @@ -2039,4 +2057,111 @@ describe('magnite analytics adapter', function () {
}
})
});

describe('BID_RESPONSE events', () => {
beforeEach(() => {
magniteAdapter.enableAnalytics({
options: {
endpoint: '//localhost:9999/event',
accountId: 1001
}
});
config.setConfig({ rubicon: { updatePageView: true } });
});

it('should add a no-bid bid to the add unit if it recieves one from the server', () => {
const bidResponse = utils.deepClone(MOCK.NO_BID);
const auctionInit = utils.deepClone(MOCK.AUCTION_INIT);

delete auctionInit.adUnits[0].sizes;

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove one of these new lines to solve the linting issue!

bidResponse.requestId = 'fakeId';
events.emit(AUCTION_INIT, auctionInit);
events.emit(BID_REQUESTED, MOCK.BID_REQUESTED);
events.emit(BID_RESPONSE, bidResponse)
events.emit(BIDDER_DONE, MOCK.BIDDER_DONE);
events.emit(AUCTION_END, MOCK.AUCTION_END);
clock.tick(rubiConf.analyticsBatchTimeout + 1000);

let message = JSON.parse(server.requests[0].requestBody);
expect(utils.generateUUID.called).to.equal(true);

expect(message.auctions[0].adUnits[0].bids[1]).to.deep.equal(
{
bidder: 'mgnipbs',
source: 'server',
status: 'no-bid',
bidId: 'fakeId',
clientLatencyMillis: -139101369960
}
);
});
});

describe('SEAT_NON_BID events', () => {
let seatnonbid;

const runNonBidAuction = () => {
events.emit(AUCTION_INIT, MOCK.AUCTION_INIT);
events.emit(BID_REQUESTED, MOCK.BID_REQUESTED);
events.emit(SEAT_NON_BID, seatnonbid)
events.emit(BIDDER_DONE, MOCK.BIDDER_DONE);
events.emit(AUCTION_END, MOCK.AUCTION_END);
clock.tick(rubiConf.analyticsBatchTimeout + 1000);
};
const checkStatusAgainstCode = (status, code, error, index) => {
seatnonbid.seatnonbid[0].nonbid[0].status = code;
runNonBidAuction();
let message = JSON.parse(server.requests[index].requestBody);
let bid = message.auctions[0].adUnits[0].bids[1];

if (error) {
expect(bid.error).to.deep.equal(error);
} else {
expect(bid.error).to.equal(undefined);
}
expect(bid.source).to.equal('server');
expect(bid.status).to.equal(status);
expect(bid.isSeatNonBid).to.equal(true);
};
beforeEach(() => {
magniteAdapter.enableAnalytics({
options: {
endpoint: '//localhost:9999/event',
accountId: 1001
}
});
config.setConfig({ rubicon: { updatePageView: true } });
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is probably not needed but not a big deal

seatnonbid = utils.deepClone(MOCK.SEAT_NON_BID);
});

it('adds seatnonbid info to bids array', () => {
runNonBidAuction();
let message = JSON.parse(server.requests[0].requestBody);

expect(message.auctions[0].adUnits[0].bids[1]).to.deep.equal(
{
bidder: 'rubicon',
source: 'server',
status: 'no-bid',
isSeatNonBid: true,
clientLatencyMillis: -139101369960
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this one tests if the status is not in the status map, and then below test is all of the other ones cool.

);
});

it('adjusts the status according to the status map', () => {
const statuses = [
{code: 0, status: 'no-bid'},
{code: 100, status: 'error', error: {code: 'request-error', description: 'general error'}},
{code: 101, status: 'error', error: {code: 'timeout-error', description: 'prebid server timeout'}},
{code: 200, status: 'rejected'},
{code: 202, status: 'rejected'},
{code: 301, status: 'rejected-ipf'}
];
statuses.forEach((info, index) => {
checkStatusAgainstCode(info.status, info.code, info.error, index);
});
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 nice!

});
});
});