From 153a2ef045c515aa4a0d3a069d6886f0274a0ac3 Mon Sep 17 00:00:00 2001 From: Nan Jiang Date: Wed, 19 Jul 2017 13:25:46 -0400 Subject: [PATCH 1/3] feat(mc): Add telemetry for Pocket --- docs/v2-system-addon/data_dictionary.md | 38 +++++++++++++ docs/v2-system-addon/data_events.md | 43 +++++++++++++++ system-addon/common/Actions.jsm | 21 +++++++- system-addon/lib/TelemetryFeed.jsm | 10 ++++ system-addon/test/schemas/pings.js | 15 ++++++ system-addon/test/unit/common/Actions.test.js | 17 +++++- .../test/unit/lib/TelemetryFeed.test.js | 53 +++++++++++++++++-- 7 files changed, 190 insertions(+), 7 deletions(-) diff --git a/docs/v2-system-addon/data_dictionary.md b/docs/v2-system-addon/data_dictionary.md index bd4379fde5..1bc73708d4 100644 --- a/docs/v2-system-addon/data_dictionary.md +++ b/docs/v2-system-addon/data_dictionary.md @@ -5,6 +5,7 @@ The Activity Stream system add-on sends various types of pings to the backend (H - an `event` ping that records specific data about individual user interactions while interacting with Activity Stream - a `performance` ping that records specific performance related events - an `undesired` ping that records data about bad app states and missing data +- an `impresion_stats` ping that records data about Pocket impressions and user interactions Schema definitions/validations that can be used for tests can be found in `system-addon/test/schemas/pings.js`. @@ -95,6 +96,39 @@ Schema definitions/validations that can be used for tests can be found in `syste "date": "2016-03-07" } ``` +# Example Activity Stream `impression_stats` Logs + +```js +{ + "action": "activity_stream_impression_stats", + "client_id": "26288a14-5cc4-d14f-ae0a-bb01ef45be9c", + "session_id": "005deed0-e3e4-4c02-a041-17405fd703f6", + "addon_version": "1.0.12", + "locale": "en-US", + "source": "pocket", + "page": "about:newtab", + "tiles": [{"id": 10000}, {"id": 10001}, {"id": 10002}] +} +``` + +```js +{ + "action": "activity_stream_impression_stats", + "client_id": "26288a14-5cc4-d14f-ae0a-bb01ef45be9c", + "session_id": "005deed0-e3e4-4c02-a041-17405fd703f6", + "addon_version": "1.0.12", + "locale": "en-US", + "source": "pocket", + "page": "about:newtab", + + // "pos" is the 0-based index to record the tile's position in the Pocket section. + "tiles": [{"id": 10000, "pos": 0}], + + // A 0-based index to record which tile in the "tiles" list that the user just interacted with. + "click|block|pocket": 0 +} +``` + | KEY | DESCRIPTION |   | |-----|-------------|:-----:| @@ -133,6 +167,10 @@ and losing focus. | :one: | `topsites_tippytop` | [Optional] The size of the Topsites set with TippyTop metadata. | :one: | `user_prefs` | [optional] The encoded integer of user's preferences. | :one: & :four: | `visibility_event_rcvd_ts` | [Optional][Server Counter][Server Alert for too many omissions] DOMHighResTimeStamp of when the page itself receives an event that document.visibilityState == visible. | :one: +| `tiles` | [Required] A list of tile objects for the Pocket articles. Each tile object mush have a ID, and optionally a "pos" property to indicate the tile position | :one: +| `click` | [Optional] An integer to record the 0-based index when user clicks on a Pocket tile. | :one: +| `block` | [Optional] An integer to record the 0-based index when user blocks a Pocket tile. | :one: +| `pocket` | [Optional] An integer to record the 0-based index when user saves a Pocket tile to Pocket. | :one: **Where:** diff --git a/docs/v2-system-addon/data_events.md b/docs/v2-system-addon/data_events.md index 21357f5f46..5d8054661b 100644 --- a/docs/v2-system-addon/data_events.md +++ b/docs/v2-system-addon/data_events.md @@ -279,3 +279,46 @@ perf: { "topsites_first_painted_ts": 5, } ``` + +## Pocket pings + +When Pocket is enabled in Activity Stream, the browser will send following `activity_stream_impression_stats` to our metrics servers. + +### Impression stats + +This reports all the Pocket recommended articles (a list of `id`s) when the user opens a newtab. + +```js +{ + "action": "activity_stream_impression_stats", + "client_id": "26288a14-5cc4-d14f-ae0a-bb01ef45be9c", + "session_id": "005deed0-e3e4-4c02-a041-17405fd703f6", + "addon_version": "1.0.12", + "locale": "en-US", + "source": "pocket", + "page": "about:newtab", + "tiles": [{"id": 10000}, {"id": 10001}, {"id": 10002}] +} +``` + +### Click/block/save_to_pocket ping + +This reports the user's interaction with those Pocket tiles. + +```js +{ + "action": "activity_stream_impression_stats", + "client_id": "26288a14-5cc4-d14f-ae0a-bb01ef45be9c", + "session_id": "005deed0-e3e4-4c02-a041-17405fd703f6", + "addon_version": "1.0.12", + "locale": "en-US", + "source": "pocket", + "page": "about:newtab", + + // "pos" is the 0-based index to record the tile's position in the Pocket section. + "tiles": [{"id": 10000, "pos": 0}], + + // A 0-based index to record which tile in the "tiles" list that the user just interacted with. + "click|block|pocket": 0 +} +``` diff --git a/system-addon/common/Actions.jsm b/system-addon/common/Actions.jsm index a0bb9702e8..ebbd53bb95 100644 --- a/system-addon/common/Actions.jsm +++ b/system-addon/common/Actions.jsm @@ -62,6 +62,7 @@ for (const type of [ "SNIPPETS_DATA", "SNIPPETS_RESET", "SYSTEM_TICK", + "TELEMETRY_IMPRESSION_STATS", "TELEMETRY_PERFORMANCE_EVENT", "TELEMETRY_UNDESIRED_EVENT", "TELEMETRY_USER_EVENT", @@ -157,7 +158,7 @@ function UserEvent(data) { * UndesiredEvent - A telemetry ping indicating an undesired state. * * @param {object} data Fields to include in the ping (value, etc.) - * @param {int} importContext (For testing) Override the import context for testing. + * @param {int} importContext (For testing) Override the import context for testing. * @return {object} An action. For UI code, a SendToMain action. */ function UndesiredEvent(data, importContext = globalImportContext) { @@ -172,7 +173,7 @@ function UndesiredEvent(data, importContext = globalImportContext) { * PerfEvent - A telemetry ping indicating a performance-related event. * * @param {object} data Fields to include in the ping (value, etc.) - * @param {int} importContext (For testing) Override the import context for testing. + * @param {int} importContext (For testing) Override the import context for testing. * @return {object} An action. For UI code, a SendToMain action. */ function PerfEvent(data, importContext = globalImportContext) { @@ -183,6 +184,21 @@ function PerfEvent(data, importContext = globalImportContext) { return importContext === UI_CODE ? SendToMain(action) : action; } +/** + * ImpressionStats - A telemetry ping indicating an impression stats. + * + * @param {object} data Fields to include in the ping + * @param {int} importContext (For testing) Override the import context for testing. + * #return {object} An action. For UI code, a SendToMain action. + */ +function ImpressionStats(data, importContext = globalImportContext) { + const action = { + type: actionTypes.TELEMETRY_IMPRESSION_STATS, + data + }; + return importContext === UI_CODE ? SendToMain(action) : action; +} + function SetPref(name, value, importContext = globalImportContext) { const action = {type: actionTypes.SET_PREF, data: {name, value}}; return importContext === UI_CODE ? SendToMain(action) : action; @@ -195,6 +211,7 @@ this.actionCreators = { UserEvent, UndesiredEvent, PerfEvent, + ImpressionStats, SendToContent, SendToMain, SetPref diff --git a/system-addon/lib/TelemetryFeed.jsm b/system-addon/lib/TelemetryFeed.jsm index f8c0906fe2..a3722d2479 100644 --- a/system-addon/lib/TelemetryFeed.jsm +++ b/system-addon/lib/TelemetryFeed.jsm @@ -151,6 +151,14 @@ this.TelemetryFeed = class TelemetryFeed { return ping; } + async createImpressionStats(action) { + return Object.assign( + await this.createPing(au.getPortIdOfSender(action)), + action.data, + {action: "activity_stream_impression_stats"} + ); + } + async createUserEvent(action) { return Object.assign( await this.createPing(au.getPortIdOfSender(action)), @@ -207,6 +215,8 @@ this.TelemetryFeed = class TelemetryFeed { break; case at.SAVE_SESSION_PERF_DATA: this.saveSessionPerfData(au.getPortIdOfSender(action), action.data); + case at.TELEMETRY_IMPRESSION_STATS: + this.sendEvent(this.createImpressionStats(action)); break; case at.TELEMETRY_UNDESIRED_EVENT: this.sendEvent(this.createUndesiredEvent(action)); diff --git a/system-addon/test/schemas/pings.js b/system-addon/test/schemas/pings.js index 88abd25467..2c0f855c3e 100644 --- a/system-addon/test/schemas/pings.js +++ b/system-addon/test/schemas/pings.js @@ -60,6 +60,20 @@ const UndesiredPing = Joi.object().keys(Object.assign({}, baseKeys, { value: Joi.number().required() })); +const TileSchema = Joi.object().keys({ + id: Joi.number().integer().required(), + pos: Joi.number().integer() +}); + +const ImpressionStatsPing = Joi.object().keys(Object.assign({}, baseKeys, { + source: Joi.string().required(), + action: Joi.valid("activity_stream_impression_stats").required(), + tiles: Joi.array().items(TileSchema).required(), + click: Joi.number().integer(), + block: Joi.number().integer(), + pocket: Joi.number().integer() +})); + const PerfPing = Joi.object().keys(Object.assign({}, baseKeys, { source: Joi.string(), event: Joi.string().required(), @@ -147,6 +161,7 @@ module.exports = { UndesiredPing, UserEventPing, UserEventAction, + ImpressionStatsPing, PerfPing, SessionPing, chaiAssertions diff --git a/system-addon/test/unit/common/Actions.test.js b/system-addon/test/unit/common/Actions.test.js index 49756cd520..288e7e5a7a 100644 --- a/system-addon/test/unit/common/Actions.test.js +++ b/system-addon/test/unit/common/Actions.test.js @@ -129,7 +129,7 @@ describe("ActionCreators", () => { it("should wrap with SendToMain if in UI code", () => { assert.isTrue(au.isSendToMain(ac.UndesiredEvent({action: "foo"})), "isSendToMain"); }); - it("should not wrap with SendToMain if in UI code", () => { + it("should not wrap with SendToMain if not in UI code", () => { const action = ac.UndesiredEvent({action: "foo"}, BACKGROUND_PROCESS); assert.isFalse(au.isSendToMain(action), "isSendToMain"); }); @@ -142,11 +142,24 @@ describe("ActionCreators", () => { it("should wrap with SendToMain if in UI code", () => { assert.isTrue(au.isSendToMain(ac.PerfEvent({action: "foo"})), "isSendToMain"); }); - it("should not wrap with SendToMain if in UI code", () => { + it("should not wrap with SendToMain if not in UI code", () => { const action = ac.PerfEvent({action: "foo"}, BACKGROUND_PROCESS); assert.isFalse(au.isSendToMain(action), "isSendToMain"); }); }); + describe("ImpressionStats", () => { + it("should include the right data", () => { + const data = {action: "foo"}; + assert.equal(ac.ImpressionStats(data).data, data); + }); + it("should wrap with SendToMain if in UI code", () => { + assert.isTrue(au.isSendToMain(ac.ImpressionStats({action: "foo"})), "isSendToMain"); + }); + it("should not wrap with SendToMain if not in UI code", () => { + const action = ac.ImpressionStats({action: "foo"}, BACKGROUND_PROCESS); + assert.isFalse(au.isSendToMain(action), "isSendToMain"); + }); + }); }); describe("ActionUtils", () => { diff --git a/system-addon/test/unit/lib/TelemetryFeed.test.js b/system-addon/test/unit/lib/TelemetryFeed.test.js index a0de0478d7..6a075be6cf 100644 --- a/system-addon/test/unit/lib/TelemetryFeed.test.js +++ b/system-addon/test/unit/lib/TelemetryFeed.test.js @@ -7,6 +7,7 @@ const { BasePing, UndesiredPing, UserEventPing, + ImpressionStatsPing, PerfPing, SessionPing } = require("test/schemas/pings"); @@ -259,6 +260,44 @@ describe("TelemetryFeed", () => { }); }); }); + describe("#createImpressionStats", () => { + it("should create a valid impression stats ping", async () => { + const tiles = [{id: 10001}, {id: 10002}, {id: 10003}]; + const action = ac.ImpressionStats({source: "POCKET", tiles}); + const ping = await instance.createImpressionStats(action); + + assert.validate(ping, ImpressionStatsPing); + assert.propertyVal(ping, "source", "POCKET"); + assert.propertyVal(ping, "tiles", tiles); + }); + it("should create a valid click ping", async () => { + const tiles = [{id: 10001, pos: 2}]; + const action = ac.ImpressionStats({source: "POCKET", tiles, click: 0}); + const ping = await instance.createImpressionStats(action); + + assert.validate(ping, ImpressionStatsPing); + assert.propertyVal(ping, "click", 0); + assert.propertyVal(ping, "tiles", tiles); + }); + it("should create a valid block ping", async () => { + const tiles = [{id: 10001, pos: 2}]; + const action = ac.ImpressionStats({source: "POCKET", tiles, block: 0}); + const ping = await instance.createImpressionStats(action); + + assert.validate(ping, ImpressionStatsPing); + assert.propertyVal(ping, "block", 0); + assert.propertyVal(ping, "tiles", tiles); + }); + it("should create a valid pocket ping", async () => { + const tiles = [{id: 10001, pos: 2}]; + const action = ac.ImpressionStats({source: "POCKET", tiles, pocket: 0}); + const ping = await instance.createImpressionStats(action); + + assert.validate(ping, ImpressionStatsPing); + assert.propertyVal(ping, "pocket", 0); + assert.propertyVal(ping, "tiles", tiles); + }); + }); describe("#sendEvent", () => { it("should call telemetrySender", async () => { sandbox.stub(instance.telemetrySender, "sendPing"); @@ -365,7 +404,7 @@ describe("TelemetryFeed", () => { assert.calledWith(stub, "port123", data); }); - it("should send an event on an TELEMETRY_UNDESIRED_EVENT action", () => { + it("should send an event on a TELEMETRY_UNDESIRED_EVENT action", () => { const sendEvent = sandbox.stub(instance, "sendEvent"); const eventCreator = sandbox.stub(instance, "createUndesiredEvent"); const action = {type: at.TELEMETRY_UNDESIRED_EVENT}; @@ -373,7 +412,7 @@ describe("TelemetryFeed", () => { assert.calledWith(eventCreator, action); assert.calledWith(sendEvent, eventCreator.returnValue); }); - it("should send an event on an TELEMETRY_USER_EVENT action", () => { + it("should send an event on a TELEMETRY_USER_EVENT action", () => { const sendEvent = sandbox.stub(instance, "sendEvent"); const eventCreator = sandbox.stub(instance, "createUserEvent"); const action = {type: at.TELEMETRY_USER_EVENT}; @@ -381,7 +420,7 @@ describe("TelemetryFeed", () => { assert.calledWith(eventCreator, action); assert.calledWith(sendEvent, eventCreator.returnValue); }); - it("should send an event on an TELEMETRY_PERFORMANCE_EVENT action", () => { + it("should send an event on a TELEMETRY_PERFORMANCE_EVENT action", () => { const sendEvent = sandbox.stub(instance, "sendEvent"); const eventCreator = sandbox.stub(instance, "createPerformanceEvent"); const action = {type: at.TELEMETRY_PERFORMANCE_EVENT}; @@ -389,5 +428,13 @@ describe("TelemetryFeed", () => { assert.calledWith(eventCreator, action); assert.calledWith(sendEvent, eventCreator.returnValue); }); + it("should send an event on a TELEMETRY_IMPRESSION_STATS action", () => { + const sendEvent = sandbox.stub(instance, "sendEvent"); + const eventCreator = sandbox.stub(instance, "createImpressionStats"); + const action = {type: at.TELEMETRY_IMPRESSION_STATS}; + instance.onAction(action); + assert.calledWith(eventCreator, action); + assert.calledWith(sendEvent, eventCreator.returnValue); + }); }); }); From e81b38eca241c198a6d38590f81bd930d501a623 Mon Sep 17 00:00:00 2001 From: Nan Jiang Date: Wed, 19 Jul 2017 15:52:22 -0400 Subject: [PATCH 2/3] review fixes r=csadilek --- docs/v2-system-addon/data_dictionary.md | 2 +- docs/v2-system-addon/data_events.md | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/v2-system-addon/data_dictionary.md b/docs/v2-system-addon/data_dictionary.md index 1bc73708d4..b2248443a4 100644 --- a/docs/v2-system-addon/data_dictionary.md +++ b/docs/v2-system-addon/data_dictionary.md @@ -5,7 +5,7 @@ The Activity Stream system add-on sends various types of pings to the backend (H - an `event` ping that records specific data about individual user interactions while interacting with Activity Stream - a `performance` ping that records specific performance related events - an `undesired` ping that records data about bad app states and missing data -- an `impresion_stats` ping that records data about Pocket impressions and user interactions +- an `impression_stats` ping that records data about Pocket impressions and user interactions Schema definitions/validations that can be used for tests can be found in `system-addon/test/schemas/pings.js`. diff --git a/docs/v2-system-addon/data_events.md b/docs/v2-system-addon/data_events.md index 5d8054661b..d945be9489 100644 --- a/docs/v2-system-addon/data_events.md +++ b/docs/v2-system-addon/data_events.md @@ -280,9 +280,9 @@ perf: { } ``` -## Pocket pings +## Top Story pings -When Pocket is enabled in Activity Stream, the browser will send following `activity_stream_impression_stats` to our metrics servers. +When Top Story (currently powered by Pocket) is enabled in Activity Stream, the browser will send following `activity_stream_impression_stats` to our metrics servers. ### Impression stats From 8fa19c14110be37050f1a3e948985ce888c7e895 Mon Sep 17 00:00:00 2001 From: Nan Jiang Date: Fri, 4 Aug 2017 14:17:22 -0400 Subject: [PATCH 3/3] Add support to empty out user specific IDs for top stories telemetry --- docs/v2-system-addon/data_dictionary.md | 4 ++-- docs/v2-system-addon/data_events.md | 6 ++++-- system-addon/lib/TelemetryFeed.jsm | 20 ++++++++++++++++++- .../test/unit/lib/TelemetryFeed.test.js | 13 ++++++++++++ 4 files changed, 38 insertions(+), 5 deletions(-) diff --git a/docs/v2-system-addon/data_dictionary.md b/docs/v2-system-addon/data_dictionary.md index b2248443a4..c613f7a281 100644 --- a/docs/v2-system-addon/data_dictionary.md +++ b/docs/v2-system-addon/data_dictionary.md @@ -114,8 +114,8 @@ Schema definitions/validations that can be used for tests can be found in `syste ```js { "action": "activity_stream_impression_stats", - "client_id": "26288a14-5cc4-d14f-ae0a-bb01ef45be9c", - "session_id": "005deed0-e3e4-4c02-a041-17405fd703f6", + "client_id": "n/a", + "session_id": "n/a", "addon_version": "1.0.12", "locale": "en-US", "source": "pocket", diff --git a/docs/v2-system-addon/data_events.md b/docs/v2-system-addon/data_events.md index d945be9489..6a371316d3 100644 --- a/docs/v2-system-addon/data_events.md +++ b/docs/v2-system-addon/data_events.md @@ -308,8 +308,10 @@ This reports the user's interaction with those Pocket tiles. ```js { "action": "activity_stream_impression_stats", - "client_id": "26288a14-5cc4-d14f-ae0a-bb01ef45be9c", - "session_id": "005deed0-e3e4-4c02-a041-17405fd703f6", + + // both "client_id" and "session_id" are set to "n/a" in this ping. + "client_id": "n/a", + "session_id": "n/a", "addon_version": "1.0.12", "locale": "en-US", "source": "pocket", diff --git a/system-addon/lib/TelemetryFeed.jsm b/system-addon/lib/TelemetryFeed.jsm index a3722d2479..02d5babc00 100644 --- a/system-addon/lib/TelemetryFeed.jsm +++ b/system-addon/lib/TelemetryFeed.jsm @@ -151,12 +151,29 @@ this.TelemetryFeed = class TelemetryFeed { return ping; } + /** + * createImpressionStats - Create a ping for an impression stats + * + * @param {ob} action The object with data to be included in the ping. + * For some user interactions, a boolean "incognito" + * field of the "data" object could be used to empty + * all the user specific IDs with "n/a" in the ping. + * @return {obj} A telemetry ping + */ async createImpressionStats(action) { - return Object.assign( + let ping = Object.assign( await this.createPing(au.getPortIdOfSender(action)), action.data, {action: "activity_stream_impression_stats"} ); + + if (ping.incognito) { + ping.client_id = "n/a"; + ping.session_id = "n/a"; + delete ping.incognito; + } + + return ping; } async createUserEvent(action) { @@ -215,6 +232,7 @@ this.TelemetryFeed = class TelemetryFeed { break; case at.SAVE_SESSION_PERF_DATA: this.saveSessionPerfData(au.getPortIdOfSender(action), action.data); + break; case at.TELEMETRY_IMPRESSION_STATS: this.sendEvent(this.createImpressionStats(action)); break; diff --git a/system-addon/test/unit/lib/TelemetryFeed.test.js b/system-addon/test/unit/lib/TelemetryFeed.test.js index 6a075be6cf..da18b39435 100644 --- a/system-addon/test/unit/lib/TelemetryFeed.test.js +++ b/system-addon/test/unit/lib/TelemetryFeed.test.js @@ -270,6 +270,19 @@ describe("TelemetryFeed", () => { assert.propertyVal(ping, "source", "POCKET"); assert.propertyVal(ping, "tiles", tiles); }); + it("should empty all the user specific IDs in the ping", async () => { + const tiles = [{id: 10001, pos: 2}]; + const incognito = true; + const action = ac.ImpressionStats({source: "POCKET", incognito, tiles, click: 0}); + const ping = await instance.createImpressionStats(action); + + assert.validate(ping, ImpressionStatsPing); + assert.propertyVal(ping, "click", 0); + assert.propertyVal(ping, "tiles", tiles); + assert.propertyVal(ping, "client_id", "n/a"); + assert.propertyVal(ping, "session_id", "n/a"); + assert.isUndefined(ping.incognito); + }); it("should create a valid click ping", async () => { const tiles = [{id: 10001, pos: 2}]; const action = ac.ImpressionStats({source: "POCKET", tiles, click: 0});