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

PBjs Core : add ability to inject tracking in video #10191

Merged
merged 22 commits into from
Feb 7, 2024
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
e407ca2
add vast impression tracking
matthieularere-msq Jul 7, 2023
14d1d3b
support additional context macro
matthieularere-msq Jul 7, 2023
643c74a
fix spaces and singlequotes
matthieularere-msq Jul 7, 2023
e70f141
remove 2494945CONTEXT2494945 macro
matthieularere-msq Jul 10, 2023
2019c74
remove CONTEXT macro
matthieularere-msq Jul 10, 2023
53f94b3
do not update vastImpUrl anymore
matthieularere-msq Jul 11, 2023
e182c18
add impression trackers in video cache
matthieularere-msq Jul 11, 2023
2587ba6
insert ony unique trackers
matthieularere-msq Jul 11, 2023
4f8086f
rename registerVastTrackers
matthieularere-msq Jul 11, 2023
94bbe1f
rename arrayVastTrackers
matthieularere-msq Jul 11, 2023
1934a1b
trackers object change
matthieularere-msq Jul 12, 2023
4a1eeaf
check modules are allowed to add trackers based on isActivityAllowed
matthieularere-msq Jul 12, 2023
1138443
rename validVastTracker and add line breaks
matthieularere-msq Jul 21, 2023
a7d72f5
removes duplicates verification in isValidVastTracker
matthieularere-msq Jul 21, 2023
2c18305
changes in wrapURI + typo fix
matthieularere-msq Jul 21, 2023
ecd61fe
requested changes
matthieularere-msq Aug 31, 2023
641c597
update function trackersToMap
matthieularere-msq Aug 31, 2023
cb593d5
using Set in trackers map
matthieularere-msq Aug 31, 2023
b74c5c6
changes suggested by dgirardi
matthieularere-msq Jan 19, 2024
8083d25
changes suggested by dgirardi
matthieularere-msq Jan 19, 2024
98f5d80
Update test/spec/video_spec.js
matthieularere-msq Jan 31, 2024
84d039c
add spaces
matthieularere-msq Jan 31, 2024
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
6 changes: 5 additions & 1 deletion src/auction.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ import {config} from './config.js';
import {userSync} from './userSync.js';
import {hook} from './hook.js';
import {find, includes} from './polyfill.js';
import {OUTSTREAM} from './video.js';
import {OUTSTREAM, getVastTrackers, insertVastTrackers} from './video.js';
import {VIDEO} from './mediaTypes.js';
import {auctionManager} from './auctionManager.js';
import {bidderSettings} from './bidderSettings.js';
Expand Down Expand Up @@ -582,7 +582,11 @@ function tryAddVideoBid(auctionInstance, bidResponse, afterBidAdded, {index = au
}), 'video');
const context = videoMediaType && deepAccess(videoMediaType, 'context');
const useCacheKey = videoMediaType && deepAccess(videoMediaType, 'useCacheKey');
const vastTrackers = getVastTrackers(bidResponse);

if (vastTrackers) {
bidResponse.vastXml = insertVastTrackers(vastTrackers, bidResponse.vastXml);
}
if (config.getConfig('cache.url') && (useCacheKey || context !== OUTSTREAM)) {
if (!bidResponse.videoCacheKey || config.getConfig('cache.ignoreBidderCacheKey')) {
addBid = false;
Expand Down
68 changes: 68 additions & 0 deletions src/video.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ import { config } from '../src/config.js';
import {includes} from './polyfill.js';
import { hook } from './hook.js';
import {auctionManager} from './auctionManager.js';
import {isActivityAllowed} from './activities/rules.js';
import {activityParams} from './activities/activityParams.js';
import {ACTIVITY_REPORT_ANALYTICS} from './activities/activities.js';

const VIDEO_MEDIA_TYPE = 'video';
export const OUTSTREAM = 'outstream';
Expand All @@ -21,6 +24,8 @@ export const videoBidder = bid => includes(adapterManager.videoAdapters, bid.bid
export const hasNonVideoBidder = adUnit =>
adUnit.bids.filter(bid => !videoBidder(bid)).length;

let vastTrackers = [];

/**
* @typedef {object} VideoBid
* @property {string} adId id of the bid
Expand Down Expand Up @@ -64,3 +69,66 @@ export const checkVideoBidSetup = hook('sync', function(bid, adUnit, videoMediaT

return true;
}, 'checkVideoBidSetup');

export function registerVastTrackers(moduleType, moduleName, trackerFn) {
if (typeof trackerFn === 'function') {
vastTrackers.push({'moduleType': moduleType, 'moduleName': moduleName, 'trackerFn': trackerFn});
}
}

export function insertVastTrackers(trackers, vastXml) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

this logic already exists in the Video Module Library. Why not reuse existing code ?
See getVastXmlWithTracking https://github.com/prebid/Prebid.js/blob/master/libraries/video/shared/vastXmlEditor.js#L8

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wasn't aware of this library. I would say not reuse this existing code avoids to run querySelectorAll and loop on it for each tracker to insert.

const doc = new DOMParser().parseFromString(vastXml, 'text/xml');
karimMourra marked this conversation as resolved.
Show resolved Hide resolved
const wrappers = doc.querySelectorAll('VAST Ad Wrapper, VAST Ad InLine');
try {
if (wrappers.length) {
wrappers.forEach(wrapper => {
if (trackers.get('impressions')) {
trackers.get('impressions').forEach(trackingUrl => {
const impression = doc.createElement('Impression');
impression.appendChild(doc.createCDATASection(trackingUrl));
wrapper.appendChild(impression)
});
}
});
vastXml = new XMLSerializer().serializeToString(doc);
karimMourra marked this conversation as resolved.
Show resolved Hide resolved
}
} catch (error) {
logError('an error happened trying to insert trackers in vastXml');
}
return vastXml;
}

export function getVastTrackers(bid) {
let trackers = [];
vastTrackers.filter(
({moduleType, moduleName, trackerFn}) => isActivityAllowed(ACTIVITY_REPORT_ANALYTICS, activityParams(moduleType, moduleName))
).forEach(({trackerFn}) => {
let trackersToAdd = trackerFn(bid);
trackersToAdd.forEach(trackerToAdd => {
if (isValidVastTracker(trackers, trackerToAdd)) { trackers.push(trackerToAdd); }
});
});
const trackersMap = trackersToMap(trackers);
return (trackersMap.size ? trackersMap : null);
};

function isValidVastTracker(trackers, trackerToAdd) {
return trackerToAdd.hasOwnProperty('event') && trackerToAdd.hasOwnProperty('url');
}

function trackersToMap(trackers) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

this could be simplified as something like

function trackersToMap(trackers) {
  return trackers.reduce((map, {url, event}) => {
    !map.has(event)  && map.set(event, new Set())
    map.get(event).add(url);
    return map;
  }, new Map())
}

which would also get rid of isTrackingDuplicate. if you combine it with also removing addImpUrlToTrackers (see below), you don't need to convert map values into arrays.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

return trackers.reduce((map, {url, event}) => {
!map.has(event) && map.set(event, new Set())
map.get(event).add(url);
return map;
}, new Map())
}

export function addImpUrlToTrackers(bid, trackersMap) {
Copy link
Collaborator

@dgirardi dgirardi Aug 9, 2023

Choose a reason for hiding this comment

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

this could be replaced by a single line in getVastTrackers instead:

// ... just before calling trackersToMap
  bid.vastImpUrl && trackers.push({event: 'impression', url: bid.vastImpUrl});
  const trackersMap = trackersToMap(trackers);
// ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Adding bid.vastImpUrl is not mandatory that's the reason why I've made is through a dedicated function which is call only when required. The current use case is when the video needs to be cached and there is no bid.vastXml object.

if (bid.vastImpUrl) {
if (!trackersMap) { trackersMap = new Map(); }
if (!trackersMap.get('impressions')) { trackersMap.set('impressions', new Set()); }
trackersMap.get('impressions').add(bid.vastImpUrl);
}
return trackersMap;
}
14 changes: 10 additions & 4 deletions src/videoCache.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import {ajaxBuilder} from './ajax.js';
import {config} from './config.js';
import {auctionManager} from './auctionManager.js';
import {getVastTrackers, addImpUrlToTrackers} from './video.js';

/**
* Might be useful to be configurable in the future
Expand Down Expand Up @@ -42,17 +43,22 @@ const ttlBufferInSeconds = 15;
* @param {string} impUrl An impression tracker URL for the delivery of the video ad
* @return A VAST URL which loads XML from the given URI.
*/
function wrapURI(uri, impUrl) {
function wrapURI(uri, vastTrackers) {
// Technically, this is vulnerable to cross-script injection by sketchy vastUrl bids.
// We could make sure it's a valid URI... but since we're loading VAST XML from the
// URL they provide anyway, that's probably not a big deal.
let vastImp = (impUrl) ? `<![CDATA[${impUrl}]]>` : ``;
let impressions = '';
if (vastTrackers && vastTrackers.get('impressions')) {
vastTrackers.get('impressions').forEach(trackingImp => {
impressions += `<Impression><![CDATA[${trackingImp}]]></Impression>`;
});
}
return `<VAST version="3.0">
<Ad>
<Wrapper>
<AdSystem>prebid.org wrapper</AdSystem>
<VASTAdTagURI><![CDATA[${uri}]]></VASTAdTagURI>
<Impression>${vastImp}</Impression>
${impressions}
<Creatives></Creatives>
</Wrapper>
</Ad>
Expand All @@ -67,7 +73,7 @@ function wrapURI(uri, impUrl) {
* @param index
*/
function toStorageRequest(bid, {index = auctionManager.index} = {}) {
const vastValue = bid.vastXml ? bid.vastXml : wrapURI(bid.vastUrl, bid.vastImpUrl);
const vastValue = bid.vastXml ? bid.vastXml : wrapURI(bid.vastUrl, addImpUrlToTrackers(bid, getVastTrackers(bid)));
const auction = index.getAuction(bid);
const ttlWithBuffer = Number(bid.ttl) + ttlBufferInSeconds;
let payload = {
Expand Down
37 changes: 36 additions & 1 deletion test/spec/videoCache_spec.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
import chai from 'chai';
import { registerVastTrackers } from 'src/video.js';
import { getCacheUrl, store } from 'src/videoCache.js';
import { config } from 'src/config.js';
import { server } from 'test/mocks/xhr.js';
import {auctionManager} from '../../src/auctionManager.js';
import {AuctionIndex} from '../../src/auctionIndex.js';
import { batchingCache } from '../../src/auction.js';
import { MODULE_TYPE_ANALYTICS } from 'src/activities/modules.js';

const should = chai.should();

Expand Down Expand Up @@ -127,7 +129,7 @@ describe('The video cache', function () {
<Wrapper>
<AdSystem>prebid.org wrapper</AdSystem>
<VASTAdTagURI><![CDATA[my-mock-url.com]]></VASTAdTagURI>
<Impression></Impression>

<Creatives></Creatives>
</Wrapper>
</Ad>
Expand All @@ -149,6 +151,39 @@ describe('The video cache', function () {
assertRequestMade({ vastUrl: 'my-mock-url.com', vastImpUrl: 'imptracker.com', ttl: 25 }, expectedValue)
});

it('should include additional impressions trackers on top of vastImpUrl when they exist', function() {
registerVastTrackers(MODULE_TYPE_ANALYTICS, 'test', function(bidResponse) {
return [
{'event': 'impressions', 'url': `https://vasttracking.mydomain.com/vast?cpm=${bidResponse.cpm}`}
];
});
const expectedValue = `<VAST version="3.0">
<Ad>
<Wrapper>
<AdSystem>prebid.org wrapper</AdSystem>
<VASTAdTagURI><![CDATA[my-mock-url.com]]></VASTAdTagURI>
<Impression><![CDATA[https://vasttracking.mydomain.com/vast?cpm=1.2]]></Impression><Impression><![CDATA[imptracker.com]]></Impression>
<Creatives></Creatives>
</Wrapper>
</Ad>
</VAST>`;
assertRequestMade({ vastUrl: 'my-mock-url.com', vastImpUrl: 'imptracker.com', ttl: 25, cpm: 1.2 }, expectedValue)
});

it('should include only additional impressions trackers when they exist', function() {
const expectedValue = `<VAST version="3.0">
<Ad>
<Wrapper>
<AdSystem>prebid.org wrapper</AdSystem>
<VASTAdTagURI><![CDATA[my-mock-url.com]]></VASTAdTagURI>
<Impression><![CDATA[https://vasttracking.mydomain.com/vast?cpm=1.2]]></Impression>
<Creatives></Creatives>
</Wrapper>
</Ad>
</VAST>`;
assertRequestMade({ vastUrl: 'my-mock-url.com', ttl: 25, cpm: 1.2 }, expectedValue)
});

it('should make the expected request when store() is called on an ad with vastXml', function () {
const vastXml = '<VAST version="3.0"></VAST>';
assertRequestMade({ vastXml: vastXml, ttl: 25 }, vastXml);
Expand Down
32 changes: 31 additions & 1 deletion test/spec/video_spec.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { isValidVideoBid } from 'src/video.js';
import { isValidVideoBid, registerVastTrackers, insertVastTrackers, getVastTrackers, addImpUrlToTrackers } from 'src/video.js';
import {hook} from '../../src/hook.js';
import {stubAuctionIndex} from '../helpers/indexStub.js';
import { MODULE_TYPE_ANALYTICS } from 'src/activities/modules.js';
matthieularere-msq marked this conversation as resolved.
Show resolved Hide resolved

describe('video.js', function () {
before(() => {
Expand Down Expand Up @@ -100,4 +101,33 @@ describe('video.js', function () {
const valid = isValidVideoBid(bid, {index: stubAuctionIndex({adUnits})});
expect(valid).to.equal(false);
});

it('insert into tracker list', function() {
let trackers = getVastTrackers({'cpm': 1.0});
if (!trackers || !trackers.get('impressions')) {
registerVastTrackers(MODULE_TYPE_ANALYTICS, 'test', function(bidResponse) {
return [
{'event': 'impressions', 'url': `https://vasttracking.mydomain.com/vast?cpm=${bidResponse.cpm}`}
];
});
}
trackers = getVastTrackers({'cpm': 1.0});
expect(trackers).to.be.a('map');
expect(trackers.get('impressions')).to.exists;
expect(trackers.get('impressions').has('https://vasttracking.mydomain.com/vast?cpm=1')).to.be.true;
});

it('insert trackers in vastXml', function() {
const trackers = getVastTrackers({'cpm': 1.0});
let vastXml = '<VAST><Ad><Wrapper></Wrapper></Ad></VAST>';
vastXml = insertVastTrackers(trackers, vastXml);
expect(vastXml).to.equal('<VAST><Ad><Wrapper><Impression><![CDATA[https://vasttracking.mydomain.com/vast?cpm=1]]></Impression></Wrapper></Ad></VAST>');
});

it('test addImpUrlToTrackers', function() {
const trackers = addImpUrlToTrackers({'vastImpUrl': 'imptracker.com'}, getVastTrackers({'cpm': 1.0}));
expect(trackers).to.be.a('map');
expect(trackers.get('impressions')).to.exists;
expect(trackers.get('impressions').has('imptracker.com')).to.be.true;
});
});