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

Timeout coefficients #4070

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion modules/prebidServerBidAdapter/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ let _s2sConfig;
/**
* @typedef {Object} S2SDefaultConfig
* @property {boolean} enabled
* @property {number} timeout
* @property {number} timeout @deprecated This property will be removed in 3.0. Please refer <TBD github pr> for more info
* @property {number} maxBids
* @property {string} adapter
* @property {AdapterOptions} adapterOptions
Expand Down
10 changes: 9 additions & 1 deletion src/auction.js
Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,15 @@ export function auctionCallbacks(auctionDone, auctionInstance) {
}

export function doCallbacksIfTimedout(auctionInstance, bidResponse) {
if (bidResponse.timeToRespond > auctionInstance.getTimeout() + config.getConfig('timeoutBuffer')) {
// TODO: In 3.0 use default values defined in config module. <TBD github issue>
let buffer = config.getConfig('timeoutBuffer');
if (config.getConfig('timeoutBufferCoefficient')) {
let newBuffer = utils.evaluateTimeout(auctionInstance.getTimeout(), config.getConfig('timeoutBufferCoefficient'));
if (buffer) {
buffer = newBuffer
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would it be possible to somehow cache/store these derived timeout values inside the auction instance - so that this logic only performed once (per auction) and then we just reference the derived value each time we execute this function?

We call this doCallbackIfTimedout function each time we add a bid response to the auction instance, so it's going to be potentially ran many times. As most of the values we're using here aren't going to change (once the auction is running), this type of change may save some time.

if (bidResponse.timeToRespond > auctionInstance.getTimeout() + buffer) {
auctionInstance.executeCallback(true);
}
}
Expand Down
1 change: 1 addition & 0 deletions src/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ const DEFAULT_ENABLE_SEND_ALL_BIDS = true;
const DEFAULT_DISABLE_AJAX_TIMEOUT = false;
const DEFAULT_BID_CACHE = false;

// @deprecated This property will be removed in 3.0. Please refer <TBD github pr> for more info
const DEFAULT_TIMEOUTBUFFER = 400;

export const RANDOM = 'random';
Expand Down
10 changes: 10 additions & 0 deletions src/prebid.js
Original file line number Diff line number Diff line change
Expand Up @@ -411,6 +411,16 @@ $$PREBID_GLOBAL$$.removeAdUnit = function (adUnitCode) {
$$PREBID_GLOBAL$$.requestBids = hook('async', function ({ bidsBackHandler, timeout, adUnits, adUnitCodes, labels, auctionId } = {}) {
events.emit(REQUEST_BIDS);
const cbTimeout = timeout || config.getConfig('bidderTimeout');
const s2sConfig = config.getConfig('s2sConfig');

if (s2sConfig && s2sConfig.timeoutCoefficient) {
let newTimeout = utils.evaluateTimeout(cbTimeout, s2sConfig.timeoutCoefficient);
if (newTimeout) {
s2sConfig.timeout = newTimeout;
config.setConfig({s2sConfig: s2sConfig});
}
}

adUnits = adUnits || $$PREBID_GLOBAL$$.adUnits;

utils.logInfo('Invoking $$PREBID_GLOBAL$$.requestBids', arguments);
Expand Down
16 changes: 16 additions & 0 deletions src/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -1334,3 +1334,19 @@ export function compareOn(property) {
return 0;
}
}

/**
* This function calculates new timeout value
* @param {number} auctionTimeout auction timeout
* @param {float} coefficient floating point coefficient
* @returns {(number|undefined)} new timeout value
*/
export function evaluateTimeout(auctionTimeout, coefficient) {
let newTimeout = Math.floor(auctionTimeout * parseFloat(coefficient));
// Both timeoutBuffer and s2s timeout cannot be greater than auction timeout
if (newTimeout > auctionTimeout) {
internal.logInfo(`Ignoring evaluated timeout ${newTimeout} as it is greater than ${auctionTimeout}. Prebid will use default settings`);
newTimeout = undefined;
}
return newTimeout;
}
24 changes: 24 additions & 0 deletions test/spec/utils_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -1009,4 +1009,28 @@ describe('Utils', function () {
});
});
});

describe('evaluateTimeout', function() {
let logInfoStub;
before(function() {
logInfoStub = sinon.stub(utils.internal, 'logInfo');
});

after(function() {
logInfoStub.restore();
})

it('returns original timeout when auction timeout is higher', function() {
let auctionTimeout = 1000;
let coefficient = 2.00;
expect(utils.evaluateTimeout(auctionTimeout, coefficient)).to.equal(undefined);
expect(logInfoStub.calledOnce).to.be.true;
});

it('returns modified timeout when auction timeout is lower', function() {
let auctionTimeout = 1000;
let coefficient = 0.20;
expect(utils.evaluateTimeout(auctionTimeout, coefficient)).to.equal(200);
});
});
});