From 9ecf71bb514e42072c23ea0e1f256ab43b665d07 Mon Sep 17 00:00:00 2001 From: Shane Tomlinson Date: Wed, 10 Sep 2014 13:06:59 +0100 Subject: [PATCH] feat(metrics): Log `entrypoint` to the metrics. * Add the FxDesktopRelier that imports `entrypoint` and `context` from the URL. * Relier extended to import `service`, `redirectTo`, `client_id`, and `state` * Initialize either the FxDesktopRelier or normal Relier on startup. * Instead of Metrics importing values directly from the URL, they are fetched from the Relier and passed in on creation. fixes #1568 --- app/scripts/lib/app-start.js | 37 +++++++++++++++--- app/scripts/lib/metrics.js | 9 +++-- app/scripts/models/reliers/fx-desktop.js | 38 +++++++++++++++++++ app/scripts/models/reliers/relier.js | 14 +++++-- app/tests/lib/helpers.js | 14 ++++++- app/tests/spec/lib/metrics.js | 7 +++- app/tests/spec/models/reliers/fx-desktop.js | 42 +++++++++++++++++++++ app/tests/spec/models/reliers/relier.js | 13 +++++-- app/tests/test_start.js | 3 +- server/lib/metrics-collector-stderr.js | 1 + tests/server/metrics-collector-stderr.js | 2 + 11 files changed, 159 insertions(+), 21 deletions(-) create mode 100644 app/scripts/models/reliers/fx-desktop.js create mode 100644 app/tests/spec/models/reliers/fx-desktop.js diff --git a/app/scripts/lib/app-start.js b/app/scripts/lib/app-start.js index f2d19407e..64946cbd5 100644 --- a/app/scripts/lib/app-start.js +++ b/app/scripts/lib/app-start.js @@ -32,7 +32,9 @@ define([ 'lib/fxa-client', 'lib/assertion', 'lib/profile', - 'models/reliers/relier' + 'lib/constants', + 'models/reliers/relier', + 'models/reliers/fx-desktop' ], function ( _, @@ -49,7 +51,9 @@ function ( FxaClient, Assertion, Profile, - Relier + Constants, + Relier, + FxDesktopRelier ) { function isMetricsCollectionEnabled (sampleRate) { @@ -91,10 +95,13 @@ function ( .then(_.bind(this.useConfig, this)) // both the metrics and router depend on the language // fetched from config. - .then(_.bind(this.initializeMetrics, this)) .then(_.bind(this.initializeRelier, this)) + // fxaClient depends on the relier. .then(_.bind(this.initializeFxaClient, this)) + // profileClient dependsd on fxaClient. .then(_.bind(this.initializeProfileClient, this)) + // metrics depends on the relier. + .then(_.bind(this.initializeMetrics, this)) // router depends on all of the above .then(_.bind(this.initializeRouter, this)); }, @@ -112,14 +119,32 @@ function ( initializeMetrics: function () { this._metrics = createMetrics(this._config.metricsSampleRate, { - lang: this._config.language + lang: this._config.language, + service: this._relier.get('service'), + context: this._relier.get('context'), + entrypoint: this._relier.get('entrypoint') }); this._metrics.init(); }, initializeRelier: function () { - this._relier = new Relier(); - return this._relier.fetch(); + if (! this._relier) { + if (this._isFxDesktop()) { + this._relier = new FxDesktopRelier({ + window: this._window + }); + } else { + this._relier = new Relier({ + window: this._window + }); + } + + return this._relier.fetch(); + } + }, + + _isFxDesktop: function () { + return this._searchParam('context') === Constants.FX_DESKTOP_CONTEXT; }, initializeFxaClient: function () { diff --git a/app/scripts/lib/metrics.js b/app/scripts/lib/metrics.js index 4b0180a03..4c0212b83 100644 --- a/app/scripts/lib/metrics.js +++ b/app/scripts/lib/metrics.js @@ -37,6 +37,7 @@ define([ 'timers', 'events', 'context', + 'entrypoint', 'service', 'lang', 'marketingLink', @@ -65,10 +66,9 @@ define([ this._window = options.window || window; this._lang = options.lang || 'unknown'; - - var searchParams = this._window.location.search; - this._context = Url.searchParam('context', searchParams) || 'web'; - this._service = Url.searchParam('service', searchParams) || 'none'; + this._context = options.context || 'web'; + this._entrypoint = options.entrypoint || 'none'; + this._service = options.service || 'none'; this._inactivityFlushMs = options.inactivityFlushMs || TEN_MINS_MS; } @@ -136,6 +136,7 @@ define([ context: this._context, service: this._service, lang: this._lang, + entrypoint: this._entrypoint, marketingType: this._marketingType || 'none', marketingLink: this._marketingLink || 'none', marketingClicked: this._marketingClicked || false diff --git a/app/scripts/models/reliers/fx-desktop.js b/app/scripts/models/reliers/fx-desktop.js new file mode 100644 index 000000000..a6181735c --- /dev/null +++ b/app/scripts/models/reliers/fx-desktop.js @@ -0,0 +1,38 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +/** + * The FxDesktop for Sync relier. In addition to the fields available on + * `Relier`, provides the following: + * + * - context + * - entrypoint + */ + +'use strict'; + +define([ + 'backbone', + 'underscore', + 'models/reliers/relier' +], function (Backbone, _, Relier) { + + var FxDesktopRelier = Relier.extend({ + defaults: _.extend({ + context: null, + entrypoint: null + }, Relier.prototype.defaults), + + fetch: function () { + var self = this; + return Relier.prototype.fetch.call(self) + .then(function () { + self.importSearchParam('context'); + self.importSearchParam('entrypoint'); + }); + } + }); + + return FxDesktopRelier; +}); diff --git a/app/scripts/models/reliers/relier.js b/app/scripts/models/reliers/relier.js index d9dc26610..ed79524b2 100644 --- a/app/scripts/models/reliers/relier.js +++ b/app/scripts/models/reliers/relier.js @@ -39,15 +39,23 @@ define([ * }); * } */ - fetch: function (options) { + fetch: function () { var self = this; return p() .then(function () { - self._setFromSearchParam('preVerifyToken'); + self.importSearchParam('preVerifyToken'); }); }, - _setFromSearchParam: function (paramName, modelName) { + /** + * Set a value based on a value in window.location.search. Only updates + * model if parameter exists in window.location.search. + * + * @param {String} paramName - name of the search parameter + * @param {String} [modelName] - name to set in model. If not specified, + * use the same value as `paramName` + */ + importSearchParam: function (paramName, modelName) { modelName = modelName || paramName; var value = Url.searchParam(paramName, this._window.location.search); diff --git a/app/tests/lib/helpers.js b/app/tests/lib/helpers.js index 2ceaf82bb..dcbc5d01a 100644 --- a/app/tests/lib/helpers.js +++ b/app/tests/lib/helpers.js @@ -109,6 +109,17 @@ define([ return isEventLogged(metrics, eventName); } + function toSearchString(obj) { + var searchString = '?'; + var pairs = []; + + for (var key in obj) { + pairs.push(key + '=' + encodeURIComponent(obj[key])); + } + + return searchString + pairs.join('&'); + } + return { requiresFocus: requiresFocus, addFxaClientSpy: addFxaClientSpy, @@ -119,6 +130,7 @@ define([ emailToUser: emailToUser, isEventLogged: isEventLogged, isErrorLogged: isErrorLogged, - isScreenLogged: isScreenLogged + isScreenLogged: isScreenLogged, + toSearchString: toSearchString }; }); diff --git a/app/tests/spec/lib/metrics.js b/app/tests/spec/lib/metrics.js index c0684fc0c..3d9982db2 100644 --- a/app/tests/spec/lib/metrics.js +++ b/app/tests/spec/lib/metrics.js @@ -24,10 +24,12 @@ function (chai, $, Metrics, AuthErrors, WindowMock, TestHelpers) { beforeEach(function () { windowMock = new WindowMock(); - windowMock.location.search = '?service=sync&context=fxa_desktop_v1'; metrics = new Metrics({ window: windowMock, - lang: 'db_LB' + lang: 'db_LB', + service: 'sync', + context: 'fxa_desktop_v1', + entrypoint: 'menupanel' }); metrics.init(); }); @@ -57,6 +59,7 @@ function (chai, $, Metrics, AuthErrors, WindowMock, TestHelpers) { assert.isTrue(filteredData.hasOwnProperty('context')); assert.isTrue(filteredData.hasOwnProperty('service')); assert.isTrue(filteredData.hasOwnProperty('lang')); + assert.isTrue(filteredData.hasOwnProperty('entrypoint')); assert.equal(filteredData.screen.width, window.screen.width); assert.equal(filteredData.screen.height, window.screen.height); }); diff --git a/app/tests/spec/models/reliers/fx-desktop.js b/app/tests/spec/models/reliers/fx-desktop.js new file mode 100644 index 000000000..08ca60b78 --- /dev/null +++ b/app/tests/spec/models/reliers/fx-desktop.js @@ -0,0 +1,42 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +'use strict'; + +define([ + 'chai', + 'models/reliers/fx-desktop', + '../../../mocks/window', + '../../../lib/helpers' +], function (chai, Relier, WindowMock, TestHelpers) { + var assert = chai.assert; + + describe('reliers/reliers/fx-desktop', function () { + describe('fetch', function () { + var relier, windowMock; + + beforeEach(function () { + windowMock = new WindowMock(); + + relier = new Relier({ + window: windowMock + }); + }); + + it('populates context & entrypoint from the search parameters', function () { + windowMock.location.search = TestHelpers.toSearchString({ + context: 'fx_desktop_v1', + entrypoint: 'menupanel' + }); + + return relier.fetch() + .then(function () { + assert.equal(relier.get('context'), 'fx_desktop_v1'); + assert.equal(relier.get('entrypoint'), 'menupanel'); + }); + }); + }); + }); +}); + diff --git a/app/tests/spec/models/reliers/relier.js b/app/tests/spec/models/reliers/relier.js index b6ef03102..2b1a2596a 100644 --- a/app/tests/spec/models/reliers/relier.js +++ b/app/tests/spec/models/reliers/relier.js @@ -7,8 +7,9 @@ define([ 'chai', 'models/reliers/relier', - '../../../mocks/window' -], function (chai, Relier, WindowMock) { + '../../../mocks/window', + '../../../lib/helpers' +], function (chai, Relier, WindowMock, TestHelpers) { var assert = chai.assert; describe('reliers/reliers/relier', function () { @@ -23,12 +24,16 @@ define([ }); }); - it('populates the preVerifyToken from the search parameters', function () { - windowMock.location.search = '?preVerifyToken=abigtoken'; + it('populates expected fields from the search parameters, unexpected search parameters are ignored', function () { + windowMock.location.search = TestHelpers.toSearchString({ + preVerifyToken: 'abigtoken', + ignored: 'ignored' + }); return relier.fetch() .then(function () { assert.equal(relier.get('preVerifyToken'), 'abigtoken'); + assert.isUndefined(relier.get('ignored')); }); }); }); diff --git a/app/tests/test_start.js b/app/tests/test_start.js index a8439b444..beca1b425 100644 --- a/app/tests/test_start.js +++ b/app/tests/test_start.js @@ -68,7 +68,8 @@ function (Translator, Session, FxaClientWrapper) { '../tests/spec/views/mixins/floating-placeholder-mixin', '../tests/spec/views/mixins/timer-mixin', '../tests/spec/views/mixins/service-mixin', - '../tests/spec/models/reliers/relier' + '../tests/spec/models/reliers/relier', + '../tests/spec/models/reliers/fx-desktop' ]; /*global mocha */ diff --git a/server/lib/metrics-collector-stderr.js b/server/lib/metrics-collector-stderr.js index baf7394a1..fe159737c 100644 --- a/server/lib/metrics-collector-stderr.js +++ b/server/lib/metrics-collector-stderr.js @@ -100,6 +100,7 @@ function toLoggableEvent(event) { 'agent', 'duration', 'context', + 'entrypoint', 'service', 'marketingLink', 'marketingType', diff --git a/tests/server/metrics-collector-stderr.js b/tests/server/metrics-collector-stderr.js index 02086b6bb..c463fcdf0 100644 --- a/tests/server/metrics-collector-stderr.js +++ b/tests/server/metrics-collector-stderr.js @@ -65,6 +65,7 @@ define([ assert.equal(loggedMetrics['screen.width'], 1680); assert.equal(loggedMetrics['screen.height'], 1050); + assert.equal(loggedMetrics.entrypoint, 'menupanel'); }); metricsCollector.write({ @@ -87,6 +88,7 @@ define([ lang: 'db_LB', service: 'sync', context: 'fx_desktop_v1', + entrypoint: 'menupanel', marketingType: 'survey', marketingLink: 'http://mzl.la/1oV7jUy', marketingClicked: false,