Skip to content
This repository has been archived by the owner on Jan 17, 2023. It is now read-only.

Commit

Permalink
Fix #2077, design and implement an A/B testing system
Browse files Browse the repository at this point in the history
People are put into tests by the server at login time
Tests may stick to the shots created when the test is in place, then viewers will be associated with that test
Tests each map to a GA field (cdX for some value of X)
  • Loading branch information
ianb committed Jan 13, 2017
1 parent 168d197 commit 86c8663
Show file tree
Hide file tree
Showing 11 changed files with 168 additions and 34 deletions.
4 changes: 4 additions & 0 deletions addon/lib/req.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,10 @@ exports.sendEvent = function (action, label, options) {
options = options || {};
options.applicationName = "firefox";
options.applicationVersion = self.version;
let abTests = user.getAbTests();
for (let testName in abTests) {
options[abTests[testName].gaField] = abTests[testName].value;
}
let showOptions = Object.keys(options).length > 2;
console.info(`sendEvent ${event}/${action}/${label || 'none'} ${showOptions ? JSON.stringify(options) : ""}`);
exports.request(`${main.getBackend()}/event`, {
Expand Down
12 changes: 11 additions & 1 deletion addon/lib/shooter.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ const { Class } = require('sdk/core/heritage');
const { watchPromise, watchFunction, watchWorker } = require("./errors");
const clipboard = require("sdk/clipboard");
const { AbstractShot } = require("./shared/shot");
const { getDeviceIdInfo } = require("./user");
const { getDeviceIdInfo, getAbTests } = require("./user");
const { URL } = require("sdk/url");
const notifications = require("sdk/notifications");
const { randomString } = require("./randomstring");
Expand Down Expand Up @@ -107,6 +107,16 @@ const ShotContext = Class({
url: this.tabUrl,
deviceId: deviceIdInfo.deviceId
});
let shotAbTests = {};
let userAbTests = getAbTests();
for (let testName in userAbTests) {
if (userAbTests[testName].shotField) {
shotAbTests[testName] = userAbTests[testName].value;
}
}
if (Object.keys(shotAbTests).length) {
this.shot.abTests = shotAbTests;
}
this._deregisters = [];
this._workerActive = false;
this.watchTab("pageshow", function (tab) {
Expand Down
28 changes: 22 additions & 6 deletions addon/lib/user.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ const { hasCookieForBackend } = require("./get-cookies");

let initialized = false;
let sentryPublicDSN = "https://d58fc53f92cd47bba7266ad6444514d8@sentry.prod.mozaws.net/74";
let abTests = {};

function getDeviceIdInfo() {
let info = prefs.deviceIdInfo || "null";
Expand All @@ -34,6 +35,21 @@ exports.getSentryPublicDSN = function() {
return sentryPublicDSN;
};

function setVariablesFromServer(responseJson) {
// FIXME: should send Raven/Sentry messages in case of any error
try {
sentryPublicDSN = responseJson.sentryPublicDSN;
console.info("got sentry DSN response from server");
} catch (e) {
console.error("Error looking for the sentry DSN", e);
}
try {
abTests = responseJson.abTests;
} catch (e) {
console.error("Error looking for the A/B tests", e);
}
}

exports.deleteEverything = function () {
let backend = require("./main").getBackend();
setDeviceIdInfo(null);
Expand Down Expand Up @@ -111,12 +127,7 @@ exports.initialize = function (backend, reason) {
}
initialized = true;
console.info("logged in with cookie:", !!response.headers["Set-Cookie"]);
try {
sentryPublicDSN = response.json.sentryPublicDSN;
console.info("got sentry DSN response from server");
} catch (e) {
console.error("Error looking for the sentry DSN", e);
}
setVariablesFromServer(response.json);
// The only other thing we do is preload the cookies
resolve();
})
Expand All @@ -136,6 +147,7 @@ function saveLogin(backend, info) {
if (response.status == 200) {
console.info("Registered login with cookie:", !!response.headers["Set-Cookie"]);
initialized = true;
setVariablesFromServer(response.json);
resolve();
} else {
console.error("Error registering:", response.status, response.statusText, response.text);
Expand Down Expand Up @@ -294,3 +306,7 @@ exports.OAuthHandler = class OAuthHandler {
});
}
};

exports.getAbTests = function () {
return abTests;
};
45 changes: 39 additions & 6 deletions server/src/ab-tests.js
Original file line number Diff line number Diff line change
@@ -1,37 +1,54 @@
// Note: these get turned into Test objects:
let allTests = {};

/* Example of how this could be set (until we have real tests to serve as docs): */
/*
let allTests = {
autoOpenSharePanel: {
description: "Open the share panel immediately after shot creation",
// Any actions the user does will have this GA field set:
gaField: "cd3",
// If the user creates a shot, and then someone else VIEWS that shot, then
// this field will be set in the viewers events:
shotField: "cd4",
version: 2,
// If you make updates (like add an option) and increment this, then users
// who were previously in the control group may get put into a new group:
version: 1,
// Keep the user in control if they aren't in control in any of these tests:
exclude: ["highlightButtonOnInstall", "myShotsDisplay"],
// These are the actual allowed A/B options (control is never specified):
options: [
{name: "autoopen", probability: 0.9}
// The name of the option, and its probabilty (e.g., 10% chance of getting
// into this group)
{name: "autoopen", probability: 0.1}
]
},
highlightButtonOnInstall: {
description: "Highlight the Page Shot button when Page Shot is installed",
gaField: "cd5",
version: 2,
version: 1,
exclude: ["autoOpenSharePanel", "myShotsDisplay"],
options: [
{name: "uitour", probability: 0.9}
{name: "uitour", probability: 0.1}
]
},
myShotsDisplay: {
description: "Show My Shots button/CTA differently",
gaField: "cd6",
version: 2,
version: 1,
exclude: ["highlightButtonOnInstall", "autoOpenSharePanel"],
options: [
// Note no one will end up in control in this example:
{name: "intropopup", probability: 0.9},
{name: "blink", probability: 0.1}
]
}
};
*/

let deprecatedTests = ["exampleTest"];
// Any test names listed here will get removed from the A/B tests. Tests should
// be moved here once we are uninterested in any future data from the test:
let deprecatedTests = [];

class Test {
constructor(options) {
Expand Down Expand Up @@ -161,3 +178,19 @@ function getRandom() {
}
return Math.random();
}

/** Given a test name and test value that were set when a Shot was created (not
set on the user), return the GA field that should be set for someone viewing
the shot (or null if nothing should be set) */
exports.shotGaFieldForValue = function (testName, testValue) {
if (deprecatedTests.includes(testName)) {
// Silently ignore deprecated tests
return null;
}
let test = allTests[testName];
if (! test) {
console.error("Test name", testName, "is not known");
return null;
}
return test.shotField || null;
};
10 changes: 9 additions & 1 deletion server/src/ga-activation.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ const gaJs = `
${stubGaJs}
return;
}
window.abTests = __ABTESTS__;
window.GoogleAnalyticsObject = "ga";
window.ga = window.ga || function () {
(window.ga.q = window.ga.q || []).push(arguments);
Expand Down Expand Up @@ -127,14 +128,20 @@ window.sendEvent = function (action, label, options) {
options = label;
label = undefined;
}
options = options || {};
if (window.abTests) {
for (var testName in window.abTests) {
options[window.abTests[testName].gaField] = window.abTests[testName].value;
}
}
console.debug("sendEvent", event + "/" + action + (label ? "/" + label : "") || "none", options || "no-options");
ga("send", "event", event, action, label, options);
};
`;

const idRegex = /^[a-zA-Z0-9_.,-]+$/;

exports.makeGaActivationString = function (gaId, userId, hashLocation) {
exports.makeGaActivationString = function (gaId, userId, abTests, hashLocation) {
if (gaId === "") {
// Don't enable ga if no id was provided
return stubGaJs;
Expand All @@ -151,5 +158,6 @@ exports.makeGaActivationString = function (gaId, userId, hashLocation) {
}
let script = gaJs.replace(/__GA_ID__/g, gaId).replace(/__USER_ID__/g, userId);
script = script.replace(/__HASH_LOCATION__/g, hashLocation ? "true" : "false");
script = script.replace(/__ABTESTS__/g, JSON.stringify(abTests));
return script;
};
16 changes: 15 additions & 1 deletion server/src/pages/shot/controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
const sendEvent = require("../../browser-send-event.js");
const page = require("./page").page;
const { AbstractShot } = require("../../../shared/shot");
const { shotGaFieldForValue } = require("../../ab-tests.js");

// This represents the model we are rendering:
let model;
Expand All @@ -17,7 +18,20 @@ exports.launch = function (data) {
model.shot.expireTime = new Date(model.expireTime);
model.shot.deleted = model.deleted;
model.controller = exports;

if (model.shot.abTests) {
for (let testName in model.shot.abTests) {
let testValue = model.shot.abTests[testName];
if (testValue) {
let shotFieldName = shotGaFieldForValue(testName, testValue);
if (shotFieldName) {
window.abTests[testName + "_shot"] = {
gaField: shotFieldName,
value: testValue
};
}
}
}
}
if (firstSet) {
document.addEventListener("helper-ready", function onHelperReady(e) {
document.removeEventListener("helper-ready", onHelperReady, false);
Expand Down
1 change: 0 additions & 1 deletion server/src/pages/shotindex/view.js
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,6 @@ class Body extends React.Component {
onClickDelete(shot, event) {
event.stopPropagation();
event.preventDefault();
console.log("deleting", shot.title, event);
sendEvent("start-delete", "my-shots", {useBeacon: true});
if (window.confirm(`Delete ${shot.title}?`)) {
sendEvent("delete", "my-shots-popup-confirm", {useBeacon: true});
Expand Down
6 changes: 4 additions & 2 deletions server/src/reactrender.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,15 @@ exports.render = function (req, res, page) {
authenticated: !!req.deviceId,
sentryPublicDSN: req.config.sentryPublicDSN,
backend: req.backend,
gitRevision: getGitRevision()
gitRevision: getGitRevision(),
abTests: req.abTests
}, jsonModel);
serverModel = Object.assign({
authenticated: !!req.deviceId,
sentryPublicDSN: req.config.sentryPublicDSN,
staticLink: req.staticLink,
staticLinkWithHost: req.staticLinkWithHost
staticLinkWithHost: req.staticLinkWithHost,
abTests: req.abTests
}, serverModel);
if (req.query.data == "json") {
if (req.query.pretty !== undefined) {
Expand Down
34 changes: 26 additions & 8 deletions server/src/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ const { captureRavenException, sendRavenMessage,
addRavenRequestHandler, addRavenErrorHandler } = require("./ravenclient");
const { errorResponse, simpleResponse, jsResponse } = require("./responses");
const selfPackage = require("./package.json");
const { b64EncodeJson, b64DecodeJson } = require("./b64");

const PROXY_HEADER_WHITELIST = {
"content-type": true,
Expand Down Expand Up @@ -224,6 +225,13 @@ app.use(function (req, res, next) {
req.userAnalytics = req.userAnalytics.debug();
}
}
let abTests = cookies.get("abtests", {signed: true});
if (abTests) {
abTests = b64DecodeJson(abTests);
} else {
abTests = {};
}
req.abTests = abTests;
req.backend = `${req.protocol}://${req.headers.host}`;
req.config = config;
next();
Expand Down Expand Up @@ -264,7 +272,7 @@ function sendGaActivation(req, res, hashPage) {
promise = Promise.resolve("");
}
promise.then((userUuid) => {
let script = gaActivation.makeGaActivationString(config.gaId, userUuid, hashPage);
let script = gaActivation.makeGaActivationString(config.gaId, userUuid, req.abTests, hashPage);
jsResponse(res, script);
}).catch((e) => {
errorResponse(res, "Error creating user UUID:", e);
Expand Down Expand Up @@ -467,11 +475,17 @@ app.post("/api/register", function (req, res) {
secret: vars.secret,
nickname: vars.nickname || null,
avatarurl: vars.avatarurl || null
}, canUpdate).then(function (ok) {
if (ok) {
}, canUpdate).then(function (userAbTests) {
if (userAbTests) {
let cookies = new Cookies(req, res, {keys: dbschema.getKeygrip()});
cookies.set("user", vars.deviceId, {signed: true});
simpleResponse(res, "Created", 200);
cookies.set("abtests", b64EncodeJson(userAbTests), {signed: true});
let responseJson = {
ok: "User created",
sentryPublicDSN: config.sentryPublicDSN,
abTests: userAbTests
};
simpleResponse(res, JSON.stringify(responseJson), 200);
} else {
sendRavenMessage(req, "Attempted to register existing user", {
extra: {
Expand Down Expand Up @@ -521,11 +535,13 @@ app.post("/api/login", function (req, res) {
throw e;
}
}
checkLogin(vars.deviceId, vars.secret, deviceInfo.addonVersion).then((ok) => {
if (ok) {
checkLogin(vars.deviceId, vars.secret, deviceInfo.addonVersion).then((userAbTests) => {
if (userAbTests) {
let cookies = new Cookies(req, res, {keys: dbschema.getKeygrip()});
cookies.set("user", vars.deviceId, {signed: true});
simpleResponse(res, JSON.stringify({"ok": "User logged in", "sentryPublicDSN": config.sentryPublicDSN}), 200);
cookies.set("abtests", b64EncodeJson(userAbTests), {signed: true});
let responseJson = {"ok": "User logged in", "sentryPublicDSN": config.sentryPublicDSN, abTests: userAbTests};
simpleResponse(res, JSON.stringify(responseJson), 200);
if (config.gaId) {
let userAnalytics = ua(config.gaId, vars.deviceId, {strictCidFormat: false});
userAnalytics.event({
Expand All @@ -535,7 +551,7 @@ app.post("/api/login", function (req, res) {
ni: true
}).send();
}
} else if (ok === null) {
} else if (! userAbTests) {
simpleResponse(res, '{"error": "No such user"}', 404);
} else {
sendRavenMessage(req, "Invalid login");
Expand All @@ -554,6 +570,8 @@ app.post("/api/unload", function (req, res) {
// This erases the session cookie:
cookies.set("user");
cookies.set("user.sig");
cookies.set("abtests");
cookies.set("abtests.sig");
simpleResponse(res, "Noted", 200);
});

Expand Down
Loading

0 comments on commit 86c8663

Please sign in to comment.