Skip to content

Commit

Permalink
holdout experiment for AdSense amp-auto-ads (#9261)
Browse files Browse the repository at this point in the history
  • Loading branch information
tlong2 authored and keithwrightbos committed May 11, 2017
1 parent 60631a2 commit 106667a
Show file tree
Hide file tree
Showing 14 changed files with 389 additions and 51 deletions.
16 changes: 0 additions & 16 deletions ads/google/a4a/test/test-traffic-experiments.js
Expand Up @@ -18,7 +18,6 @@ import {ampdocServiceFor} from '../../../../src/ampdoc';
import {installDocService} from '../../../../src/service/ampdoc-impl';
import {
addExperimentIdToElement,
mergeExperimentIds,
isInExperiment,
isExternallyTriggeredExperiment,
isInternallyTriggeredExperiment,
Expand Down Expand Up @@ -214,21 +213,6 @@ describe('all-traffic-experiments-tests', () => {
});
});

describe('#mergeExperimentIds', () => {
it('should merge a single id to itself', () => {
expect(mergeExperimentIds('12345')).to.equal('12345');
});
it('should merge a single ID to a list', () => {
expect(mergeExperimentIds('12345', '3,4,5,6')).to.equal('3,4,5,6,12345');
});
it('should discard invalid ID', () => {
expect(mergeExperimentIds('frob', '3,4,5,6')).to.equal('3,4,5,6');
});
it('should return empty string for invalid input', () => {
expect(mergeExperimentIds('frob')).to.equal('');
});
});

describe('#isInExperiment', () => {
it('should return false for empty element and any query', () => {
const element = document.createElement('div');
Expand Down
48 changes: 45 additions & 3 deletions ads/google/a4a/test/test-utils.js
Expand Up @@ -19,6 +19,7 @@ import {
extractAmpAnalyticsConfig,
extractGoogleAdCreativeAndSignature,
googleAdUrl,
mergeExperimentIds,
} from '../utils';
import {createElementWithAttributes} from '../../../../src/dom';
import {base64UrlDecodeToBytes} from '../../../../src/utils/base64';
Expand Down Expand Up @@ -270,11 +271,11 @@ describe('Google A4A utils', () => {
const impl = new MockA4AImpl(elem);
noopMethods(impl, doc, sandbox);
return fixture.addElement(elem).then(() => {
return googleAdUrl(impl, '', 0, [], []).then(url1 => {
return googleAdUrl(impl, '', 0, [], [], []).then(url1 => {
expect(url1).to.match(/ifi=1/);
return googleAdUrl(impl, '', 0, [], []).then(url2 => {
return googleAdUrl(impl, '', 0, [], [], []).then(url2 => {
expect(url2).to.match(/ifi=2/);
return googleAdUrl(impl, '', 0, [], []).then(url3 => {
return googleAdUrl(impl, '', 0, [], [], []).then(url3 => {
expect(url3).to.match(/ifi=3/);
});
});
Expand Down Expand Up @@ -346,5 +347,46 @@ describe('Google A4A utils', () => {
});
});

it('should include all experiment ids', function() {
// When ran locally, this test tends to exceed 2000ms timeout.
this.timeout(5000);
return createIframePromise().then(fixture => {
setupForAdTesting(fixture);
const doc = fixture.doc;
const elem = createElementWithAttributes(doc, 'amp-a4a', {
'type': 'adsense',
'width': '320',
'height': '50',
'data-experiment-id': '123,456',
});
const impl = new MockA4AImpl(elem);
noopMethods(impl, doc, sandbox);
return fixture.addElement(elem).then(() => {
return googleAdUrl(impl, '', 0, [], [], ['789', '098']).then(url1 => {
expect(url1).to.match(/eid=123%2C456%2C789%2C098/);
});
});
});
});
});

describe('#mergeExperimentIds', () => {
it('should merge a single id to itself', () => {
expect(mergeExperimentIds(['12345'])).to.equal('12345');
});
it('should merge a single ID to a list', () => {
expect(mergeExperimentIds(['12345'], '3,4,5,6'))
.to.equal('3,4,5,6,12345');
});
it('should merge multiple IDs into a list', () => {
expect(mergeExperimentIds(['12345','6789'], '3,4,5,6'))
.to.equal('3,4,5,6,12345,6789');
});
it('should discard invalid ID', () => {
expect(mergeExperimentIds(['frob'], '3,4,5,6')).to.equal('3,4,5,6');
});
it('should return empty string for invalid input', () => {
expect(mergeExperimentIds(['frob'])).to.equal('');
});
});
});
30 changes: 6 additions & 24 deletions ads/google/a4a/traffic-experiments.js
Expand Up @@ -22,7 +22,11 @@
* impacts on click-throughs.
*/

import {isGoogleAdsA4AValidEnvironment, EXPERIMENT_ATTRIBUTE} from './utils';
import {
isGoogleAdsA4AValidEnvironment,
mergeExperimentIds,
EXPERIMENT_ATTRIBUTE,
} from './utils';
import {
isExperimentOn,
forceExperimentBranch,
Expand Down Expand Up @@ -309,28 +313,6 @@ export function validateExperimentIds(idList) {
return idList.every(id => { return !isNaN(parseInt(id, 10)); });
}

/**
* Add a new experiment ID to a (possibly empty) existing set of experiment IDs.
* The {@code currentIdString} may be {@code null} or {@code ''}, but if it is
* populated, it must contain a comma-separated list of integer experiment IDs
* (per {@code parseExperimentIds()}). Returns the new set of IDs, encoded
* as a comma-separated list. Does not de-duplicate ID entries.
*
* @param {!string} newId ID to merge in. Must be a stringified integer
* (base 10).
* @param {?string} currentIdString If present, a string containing a
* comma-separated list of integer experiment IDs.
* @returns {string} New experiment list string, including newId iff it is
* a valid (integer) experiment ID.
* @see parseExperimentIds, validateExperimentIds
*/
export function mergeExperimentIds(newId, currentIdString) {
if (newId && !isNaN(parseInt(newId, 10))) {
return currentIdString ? (currentIdString + ',' + newId) : newId;
}
return currentIdString || '';
}

/**
* Adds a single experimentID to an element iff it's a valid experiment ID.
*
Expand All @@ -341,7 +323,7 @@ export function addExperimentIdToElement(experimentId, element) {
const currentEids = element.getAttribute(EXPERIMENT_ATTRIBUTE);
if (currentEids && validateExperimentIds(parseExperimentIds(currentEids))) {
element.setAttribute(EXPERIMENT_ATTRIBUTE,
mergeExperimentIds(experimentId, currentEids));
mergeExperimentIds([experimentId], currentEids));
} else {
element.setAttribute(EXPERIMENT_ATTRIBUTE, experimentId);
}
Expand Down
34 changes: 32 additions & 2 deletions ads/google/a4a/utils.js
Expand Up @@ -132,10 +132,14 @@ export function isReportingEnabled(ampElement) {
* Parameters that will be put at the end of the URL, where they may be
* elided for length reasons. Intended for parameters with potentially
* long values, like URLs.
* @param {!Array<string>=} opt_experimentIds Any experiments IDs (in addition
* to those specified on the ad element) that should be included in the
* request.
* @return {!Promise<string>}
*/
export function googleAdUrl(
a4a, baseUrl, startTime, queryParams, unboundedQueryParams) {
a4a, baseUrl, startTime, queryParams, unboundedQueryParams,
opt_experimentIds) {
// TODO: Maybe add checks in case these promises fail.
/** @const {!Promise<string>} */
const referrerPromise = viewerForDoc(a4a.getAmpDoc()).getReferrerUrl();
Expand Down Expand Up @@ -172,6 +176,10 @@ export function googleAdUrl(
if (isCanary(win)) {
queryParams.push({name: 'isc', value: '1'});
}
let eids = adElement.getAttribute('data-experiment-id');
if (opt_experimentIds) {
eids = mergeExperimentIds(opt_experimentIds, eids);
}
const allQueryParams = queryParams.concat(
[
{
Expand All @@ -187,7 +195,7 @@ export function googleAdUrl(
{name: 'output', value: 'html'},
{name: 'nhd', value: iframeDepth},
{name: 'iu', value: adElement.getAttribute('data-ad-slot')},
{name: 'eid', value: adElement.getAttribute('data-experiment-id')},
{name: 'eid', value: eids},
{name: 'biw', value: viewportRect.width},
{name: 'bih', value: viewportRect.height},
{name: 'adx', value: slotRect.left},
Expand Down Expand Up @@ -480,3 +488,25 @@ export function extractAmpAnalyticsConfig(
}
return null;
}

/**
* Add new experiment IDs to a (possibly empty) existing set of experiment IDs.
* The {@code currentIdString} may be {@code null} or {@code ''}, but if it is
* populated, it must contain a comma-separated list of integer experiment IDs
* (per {@code parseExperimentIds()}). Returns the new set of IDs, encoded
* as a comma-separated list. Does not de-duplicate ID entries.
*
* @param {!Array<string>} newIds IDs to merge in. Should contain stringified
* integer (base 10) experiment IDs.
* @param {?string} currentIdString If present, a string containing a
* comma-separated list of integer experiment IDs.
* @returns {string} New experiment list string, including newId iff it is
* a valid (integer) experiment ID.
* @see parseExperimentIds, validateExperimentIds
*/
export function mergeExperimentIds(newIds, currentIdString) {
const newIdString = newIds.filter(newId => Number(newId)).join(',');
currentIdString = currentIdString || '';
return currentIdString + (currentIdString && newIdString ? ',' : '')
+ newIdString;
}
62 changes: 62 additions & 0 deletions ads/google/adsense-amp-auto-ads.js
@@ -0,0 +1,62 @@
/**
* Copyright 2017 The AMP HTML Authors. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS-IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/


import {
randomlySelectUnsetExperiments,
getExperimentBranch,
} from '../../src/experiments';


/** @const {string} */
export const ADSENSE_AMP_AUTO_ADS_HOLDOUT_EXPERIMENT_NAME =
'amp-auto-ads-adsense-holdout';


/**
* @enum {string}
*/
export const AdSenseAmpAutoAdsHoldoutBranches = {
CONTROL: '3782001', // don't run amp-auto-ads
EXPERIMENT: '3782002', // do run amp-auto-ads
};


/** @const {!../../src/experiments.ExperimentInfo} */
const ADSENSE_AMP_AUTO_ADS_EXPERIMENT_INFO = {
isTrafficEligible: win => !!win.document.querySelector('AMP-AUTO-ADS'),
branches: [
AdSenseAmpAutoAdsHoldoutBranches.CONTROL,
AdSenseAmpAutoAdsHoldoutBranches.EXPERIMENT,
],
};


/**
* This has the side-effect of selecting the page into a branch of the
* experiment, which becomes sticky for the entire pageview.
*
* @param {!Window} win
* @return {?string}
*/
export function getAdSenseAmpAutoAdsExpBranch(win) {
const experiments = {};
experiments[ADSENSE_AMP_AUTO_ADS_HOLDOUT_EXPERIMENT_NAME] =
ADSENSE_AMP_AUTO_ADS_EXPERIMENT_INFO;
randomlySelectUnsetExperiments(win, experiments);
return getExperimentBranch(win, ADSENSE_AMP_AUTO_ADS_HOLDOUT_EXPERIMENT_NAME)
|| null;
};
83 changes: 83 additions & 0 deletions ads/google/test/test-adsense-amp-auto-ads.js
@@ -0,0 +1,83 @@
/**
* Copyright 2017 The AMP HTML Authors. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS-IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import {
ADSENSE_AMP_AUTO_ADS_HOLDOUT_EXPERIMENT_NAME,
AdSenseAmpAutoAdsHoldoutBranches,
getAdSenseAmpAutoAdsExpBranch,
} from '../adsense-amp-auto-ads';
import {
RANDOM_NUMBER_GENERATORS,
toggleExperiment,
} from '../../../src/experiments';

describes.realWin('adsense-amp-auto-ads', {}, env => {
let win;
let sandbox;
let accurateRandomStub;
let cachedAccuratePrng;

beforeEach(() => {
win = env.win;
sandbox = env.sandbox;

accurateRandomStub = sandbox.stub().returns(-1);
cachedAccuratePrng = RANDOM_NUMBER_GENERATORS.accuratePrng;
RANDOM_NUMBER_GENERATORS.accuratePrng = accurateRandomStub;
});

afterEach(() => {
sandbox.restore();
RANDOM_NUMBER_GENERATORS.accuratePrng = cachedAccuratePrng;
});

it('should pick the control branch when experiment on and amp-auto-ads ' +
'tag present.', () => {
const ampAutoAdsEl = win.document.createElement('amp-auto-ads');
win.document.body.appendChild(ampAutoAdsEl);
toggleExperiment(win, ADSENSE_AMP_AUTO_ADS_HOLDOUT_EXPERIMENT_NAME, true);
RANDOM_NUMBER_GENERATORS.accuratePrng.onFirstCall().returns(0.4);
expect(getAdSenseAmpAutoAdsExpBranch(win))
.to.equal(AdSenseAmpAutoAdsHoldoutBranches.CONTROL);
});

it('should pick the experiment branch when experiment on and amp-auto-ads ' +
'tag present.', () => {
const ampAutoAdsEl = win.document.createElement('amp-auto-ads');
win.document.body.appendChild(ampAutoAdsEl);
toggleExperiment(win, ADSENSE_AMP_AUTO_ADS_HOLDOUT_EXPERIMENT_NAME, true);
RANDOM_NUMBER_GENERATORS.accuratePrng.onFirstCall().returns(0.6);
expect(getAdSenseAmpAutoAdsExpBranch(win))
.to.equal(AdSenseAmpAutoAdsHoldoutBranches.EXPERIMENT);
});

it('should not pick a branch when experiment off.', () => {
const ampAutoAdsEl = win.document.createElement('amp-auto-ads');
win.document.body.appendChild(ampAutoAdsEl);
toggleExperiment(win, ADSENSE_AMP_AUTO_ADS_HOLDOUT_EXPERIMENT_NAME, false);
RANDOM_NUMBER_GENERATORS.accuratePrng.onFirstCall().returns(0.4);
expect(getAdSenseAmpAutoAdsExpBranch(win)).to.be.null;
});

it('should not pick a branch when experiment on but no and amp-auto-ads ' +
'tag present.', () => {
toggleExperiment(win, ADSENSE_AMP_AUTO_ADS_HOLDOUT_EXPERIMENT_NAME, true);
RANDOM_NUMBER_GENERATORS.accuratePrng.onFirstCall().returns(0.4);
expect(getAdSenseAmpAutoAdsExpBranch(win)).to.be.null;
});
});


1 change: 1 addition & 0 deletions build-system/dep-check-config.js
Expand Up @@ -100,6 +100,7 @@ exports.rules = [
'ads/**->src/types.js',
'ads/**->src/string.js',
'ads/**->src/style.js',
'ads/google/adsense-amp-auto-ads.js->src/experiments.js',
// ads/google/a4a doesn't contain 3P ad code and should probably move
// somewhere else at some point
'ads/google/a4a/**->src/ad-cid.js',
Expand Down
Expand Up @@ -48,6 +48,9 @@ import {
import {viewerForDoc} from '../../../src/services';
import {AdsenseSharedState} from './adsense-shared-state';
import {insertAnalyticsElement} from '../../../src/analytics';
import {
getAdSenseAmpAutoAdsExpBranch,
} from '../../../ads/google/adsense-amp-auto-ads';

/** @const {string} */
const ADSENSE_BASE_URL = 'https://googleads.g.doubleclick.net/pagead/ads';
Expand Down Expand Up @@ -183,8 +186,14 @@ export class AmpAdNetworkAdsenseImpl extends AmpA4A {
paramList.push({name: 'prev_fmts', value: sharedStateParams.prevFmts});
}

const experimentIds = [];
const ampAutoAdsBranch = getAdSenseAmpAutoAdsExpBranch(this.win);
if (ampAutoAdsBranch) {
experimentIds.push(ampAutoAdsBranch);
}

return googleAdUrl(
this, ADSENSE_BASE_URL, startTime, paramList, []);
this, ADSENSE_BASE_URL, startTime, paramList, [], experimentIds);
}

/** @override */
Expand Down

0 comments on commit 106667a

Please sign in to comment.