diff --git a/CHANGELOG.md b/CHANGELOG.md index 6c03cc9c..7118e610 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,12 @@ # Unreleased changes -[Full changelog](https://github.com/mozilla-rally/core-addon/compare/v1.4.1...master) +[Full changelog](https://github.com/mozilla-rally/core-addon/compare/v1.4.2...master) + +# v1.4.2 (2022-01-26) + +[Full changelog](https://github.com/mozilla-rally/core-addon/compare/v1.4.1...v1.4.2) + +* [#782](https://github.com/mozilla-rally/rally-core-addon/pull/782): Optional legacy telemetry and send schema namespace in glean # v1.4.1 (2022-01-11) diff --git a/core-addon/Core.js b/core-addon/Core.js index 0c2c719b..0b38eefa 100644 --- a/core-addon/Core.js +++ b/core-addon/Core.js @@ -458,12 +458,16 @@ export default class Core { await this._storage.appendActivatedStudy(studyAddonId); enrollmentMetrics.studyId.set(studyAddonId); + enrollmentMetrics.schemaNamespace.set(knownStudy.schemaNamespace); rallyPings.studyEnrollment.submit(); - // Finally send the ping. Important: remove this line once the migration + // Finally send the ping. Important: remove this block once the migration // to Glean.js is finally complete. - let rallyId = await this._storage.getRallyID(); - await this._dataCollection.sendEnrollmentPing(rallyId, knownStudy.schemaNamespace); + if ("useLegacyTelemetry" in knownStudy && knownStudy.useLegacyTelemetry === true) { + console.warn("Sending study enrollment ping using legacy telemetry"); + let rallyId = await this._storage.getRallyID(); + await this._dataCollection.sendEnrollmentPing(rallyId, knownStudy.schemaNamespace); + } } /** @@ -509,12 +513,16 @@ export default class Core { } unenrollmentMetrics.studyId.set(studyAddonId); + unenrollmentMetrics.schemaNamespace.set(knownStudy.schemaNamespace); rallyPings.studyUnenrollment.submit(); - // Important: remove these lines once the migration + // Important: remove these block once the migration // to Glean.js is finally complete. - let rallyId = await this._storage.getRallyID(); - await this._dataCollection.sendDeletionPing(rallyId, knownStudy.schemaNamespace); + if ("useLegacyTelemetry" in knownStudy && knownStudy.useLegacyTelemetry === true) { + console.warn("Sending study unenrollment ping using legacy telemetry"); + let rallyId = await this._storage.getRallyID(); + await this._dataCollection.sendDeletionPing(rallyId, knownStudy.schemaNamespace); + } } /** @@ -760,7 +768,7 @@ export default class Core { // Match the version number used by the core add-on, such as "1.3.7buildid20220107.052711". // this uses capture groups to isolate the part before the build ID, the semantic version number "1.3.7". - const versionRegex = /^([0-9]+)\.([0-9]+)\.([0-9]).*/; + const versionRegex = /^([0-9]+)\.([0-9]+)\.([0-9]+).*/; try { const [fullCore, majorCore, minorCore, patchCore] = coreVersion.match(versionRegex); diff --git a/docs/metrics.md b/docs/metrics.md index 6c7c894e..6b821586 100644 --- a/docs/metrics.md +++ b/docs/metrics.md @@ -108,6 +108,7 @@ In addition to those built-in metrics, the following metrics are added to the pi | Name | Type | Description | Data reviews | Extras | Expiration | [Data Sensitivity](https://wiki.mozilla.org/Firefox/Data_Collection) | | --- | --- | --- | --- | --- | --- | --- | +| enrollment.schema_namespace |[string](https://mozilla.github.io/glean/book/user/metrics/string.html) |The schema namespace for the study the user has joined. |[Review 1](TODO)||never | | | enrollment.study_id |[string](https://mozilla.github.io/glean/book/user/metrics/string.html) |The id of the study user has joined. |[Bug 1663857](https://bugzilla.mozilla.org/show_bug.cgi?id=1663857#c5)||never | | | rally.id |[uuid](https://mozilla.github.io/glean/book/user/metrics/uuid.html) |The id of the Rally client. |[mozilla-rally/rally-core-addon#505](https://github.com/mozilla-rally/rally-core-addon/pull/505#issuecomment-815826426)||never | | @@ -133,6 +134,7 @@ In addition to those built-in metrics, the following metrics are added to the pi | Name | Type | Description | Data reviews | Extras | Expiration | [Data Sensitivity](https://wiki.mozilla.org/Firefox/Data_Collection) | | --- | --- | --- | --- | --- | --- | --- | | rally.id |[uuid](https://mozilla.github.io/glean/book/user/metrics/uuid.html) |The id of the Rally client. |[mozilla-rally/rally-core-addon#505](https://github.com/mozilla-rally/rally-core-addon/pull/505#issuecomment-815826426)||never | | +| unenrollment.schema_namespace |[string](https://mozilla.github.io/glean/book/user/metrics/string.html) |The schema namespace for the study the user has left. |[Review 1](TODO)||never | | | unenrollment.study_id |[string](https://mozilla.github.io/glean/book/user/metrics/string.html) |The id of the study user has left. |[Bug 1646151](https://bugzilla.mozilla.org/show_bug.cgi?id=1646151#c32)||never | | ## uninstall-deletion diff --git a/manifest.json b/manifest.json index 3bc3e571..fdd318ca 100644 --- a/manifest.json +++ b/manifest.json @@ -3,7 +3,7 @@ "author": "Mozilla", "manifest_version": 2, "name": "Mozilla Rally", - "version": "1.4.1", + "version": "1.4.2", "homepage_url": "https://github.com/mozilla-rally/rally-core-addon", "icons": { "48": "public/img/rally-favicon.svg", diff --git a/metrics.yaml b/metrics.yaml index 04f91f0d..22d921ee 100644 --- a/metrics.yaml +++ b/metrics.yaml @@ -50,6 +50,21 @@ enrollment: - than@mozilla.com expires: never + schema_namespace: + type: string + lifetime: ping + send_in_pings: + - study-enrollment + description: | + The schema namespace for the study the user has joined. + bugs: + - https://github.com/mozilla-rally/rally-core-addon/issues/545 + data_reviews: + - https://bugzilla.mozilla.org/show_bug.cgi?id=1663857#c5 + notification_emails: + - than@mozilla.com + expires: never + unenrollment: study_id: type: string @@ -66,6 +81,21 @@ unenrollment: - than@mozilla.com expires: never + schema_namespace: + type: string + lifetime: ping + send_in_pings: + - study-unenrollment + description: | + The schema namespace for the study the user has left. + bugs: + - https://github.com/mozilla-rally/rally-core-addon/issues/545 + data_reviews: + - https://bugzilla.mozilla.org/show_bug.cgi?id=1646151#c32 + notification_emails: + - than@mozilla.com + expires: never + user: age: type: labeled_boolean diff --git a/package-lock.json b/package-lock.json index 4f444d7d..43728eed 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "rally_core", - "version": "1.4.1", + "version": "1.4.2", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "rally_core", - "version": "1.4.1", + "version": "1.4.2", "license": "MPL-2.0", "dependencies": { "sirv-cli": "^1.0.12" diff --git a/package.json b/package.json index d9ce0013..9b8f0752 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,10 @@ { "name": "rally_core", +<<<<<<< HEAD "version": "1.4.1", +======= + "version": "1.4.2", +>>>>>>> release-v1.4.2 "type": "module", "scripts": { "prebuild": "node scripts/setupTaskcluster.js", diff --git a/public/locally-available-studies.json b/public/locally-available-studies.json index dc87a1cd..58afe6d0 100644 --- a/public/locally-available-studies.json +++ b/public/locally-available-studies.json @@ -21,6 +21,7 @@ "dataCollectionDetails": ["The date and time"], "tags": ["misinformation", "social media", "demo"], "schemaNamespace": "rally-basic-study-template", - "minimumCoreVersion": "1.3.8" + "minimumCoreVersion": "1.3.8", + "useLegacyTelemetry": true } ] diff --git a/public/studies-schema.json b/public/studies-schema.json index 8fc72d2b..9d8b11f6 100644 --- a/public/studies-schema.json +++ b/public/studies-schema.json @@ -91,6 +91,9 @@ }, "minimumCoreVersion": { "type": "string" + }, + "useLegacyTelemetry": { + "type": "boolean" } } } diff --git a/tests/core-addon/unit/Core.test.js b/tests/core-addon/unit/Core.test.js index f4f1314f..2501d3b9 100644 --- a/tests/core-addon/unit/Core.test.js +++ b/tests/core-addon/unit/Core.test.js @@ -20,20 +20,30 @@ const OFFBOARD_URL = "https://production.rally.mozilla.org/offboard"; // A fake study id to use in the tests when looking for a // "known" study. -const FAKE_STUDY_ID = "test@ion-studies.com"; +const FAKE_STUDY_ID = "test@rally-studies.com"; +const FAKE_LEGACY_STUDY_ID = "test-legacy@rally-studies.com"; const FAKE_STUDY_NAMESPACE = "fake-study-installed-namespace" -const FAKE_STUDY_ID_NOT_INSTALLED = "test-not-installed@ion-studies.com"; +const FAKE_LEGACY_STUDY_NAMESPACE = "fake-legacy-study-installed-namespace" +const FAKE_STUDY_ID_NOT_INSTALLED = "test-not-installed@rally-studies.com"; const FAKE_STUDY_NOT_INSTALLED_NAMESPACE = "fake-study-not-installed-namespace" const FAKE_STUDY_LIST = [ { "addonId": FAKE_STUDY_ID, "schemaNamespace": FAKE_STUDY_NAMESPACE, - "minimumCoreVersion": "0.0.1" + "minimumCoreVersion": "0.0.1", + "useLegacyTelemetry": false + }, + { + "addonId": FAKE_LEGACY_STUDY_ID, + "schemaNamespace": FAKE_LEGACY_STUDY_NAMESPACE, + "minimumCoreVersion": "0.0.1", + "useLegacyTelemetry": true }, { "addonId": FAKE_STUDY_ID_NOT_INSTALLED, "schemaNamespace": FAKE_STUDY_NOT_INSTALLED_NAMESPACE, - "minimumCoreVersion": "0.0.1" + "minimumCoreVersion": "0.0.1", + "useLegacyTelemetry": true } ]; const FAKE_UUID = "c0ffeec0-ffee-c0ff-eec0-ffeec0ffeec0"; @@ -251,10 +261,11 @@ describe('Core', function () { await this.core._enrollStudy(FAKE_STUDY_ID); assert.equal(await enrollmentMetrics.studyId.testGetValue("study-enrollment"), FAKE_STUDY_ID); + assert.equal(await enrollmentMetrics.schemaNamespace.testGetValue("study-enrollment"), FAKE_STUDY_NAMESPACE); // We expect to store the fake ion ID. assert.ok( - this.core._dataCollection.sendEnrollmentPing.withArgs(FAKE_UUID, FAKE_STUDY_NAMESPACE).calledOnce + this.core._dataCollection.sendEnrollmentPing.withArgs(FAKE_UUID, FAKE_STUDY_NAMESPACE).notCalled ); enrollmentPingMock.verify(); }); @@ -341,7 +352,7 @@ describe('Core', function () { // We to submit a ping with the expected type. assert.ok( - this.core._dataCollection.sendDeletionPing.withArgs(FAKE_UUID, FAKE_STUDY_NAMESPACE).calledOnce + this.core._dataCollection.sendDeletionPing.withArgs(FAKE_UUID, FAKE_STUDY_NAMESPACE).notCalled ); // Make sure that we're generating an uninstall message for @@ -445,7 +456,7 @@ describe('Core', function () { {type: "telemetry-ping", data: {}}, {id: FAKE_STUDY_ID} ), - { message: "Core._handleExternalMessage - test@ion-studies.com not joined"} + { message: `Core._handleExternalMessage - ${FAKE_STUDY_ID} not joined`} ); }); }); @@ -486,8 +497,9 @@ describe('Core', function () { describe('_fetchAvailableStudies()', function () { it('returns a list of addons', async function () { let studies = await this.core._fetchAvailableStudies(); - assert.equal(studies.length, 2); + assert.equal(studies.length, 3); assert.ok(studies.filter(a => (a.addonId === FAKE_STUDY_ID))); + assert.ok(studies.filter(a => (a.addonId === FAKE_LEGACY_STUDY_ID))); assert.ok(studies.filter(a => (a.addonId === FAKE_STUDY_ID_NOT_INSTALLED))); }); @@ -507,7 +519,7 @@ describe('Core', function () { // Kick off an update task. let studies = await this.core._updateInstalledStudies(FAKE_STUDY_LIST); - assert.equal(studies.length, 2); + assert.equal(studies.length, 3); // Check that the FAKE_STUDY_ID is marked as installed (as per // our fake data, see the beginning of this file). assert.equal(studies @@ -537,7 +549,7 @@ describe('Core', function () { it('rejects on target study ids', async function () { assert.rejects( this.core._sendMessageToStudy(FAKE_STUDY_ID, "unknown-type-test", {}), - { message: "Core._sendMessageToStudy - unexpected message \"unknown-type-test\" to study \"test@ion-studies.com\""} + { message: `Core._sendMessageToStudy - unexpected message "unknown-type-test" to study "${FAKE_STUDY_ID}"`} ); }); @@ -610,7 +622,7 @@ describe('Core', function () { it('pauses studies properly', async function () { // Make sure the functions yield during tests! browser.storage.local.get - .callsArgWith(1, {activatedStudies: [FAKE_STUDY_ID]}) + .callsArgWith(1, {activatedStudies: [FAKE_STUDY_ID, FAKE_LEGACY_STUDY_ID]}) .resolves(); chrome.runtime.sendMessage.yields(); @@ -626,7 +638,38 @@ describe('Core', function () { {}, // This is the callback hidden away by webextension-polyfill. sinon.match.any - ).calledOnce + ).calledOnce, + chrome.runtime.sendMessage.withArgs( + FAKE_LEGACY_STUDY_ID, + sinon.match({type: "pause", data: {}}), + // We're not providing any option. + {}, + // This is the callback hidden away by webextension-polyfill. + sinon.match.any + ).notCalled + ); + + const fakeLegacyStudy = FAKE_STUDY_LIST[1]; + fakeLegacyStudy.studyPaused = true; + await this.core._sendRunState(FAKE_STUDY_LIST, [FAKE_LEGACY_STUDY_ID]); + + assert.ok( + chrome.runtime.sendMessage.withArgs( + FAKE_LEGACY_STUDY_ID, + sinon.match({type: "pause", data: {}}), + // We're not providing any option. + {}, + // This is the callback hidden away by webextension-polyfill. + sinon.match.any + ).calledOnce, + chrome.runtime.sendMessage.withArgs( + FAKE_STUDY_ID, + sinon.match({type: "pause", data: {}}), + // We're not providing any option. + {}, + // This is the callback hidden away by webextension-polyfill. + sinon.match.any + ).notCalled ); }); @@ -637,9 +680,9 @@ describe('Core', function () { .resolves(); chrome.runtime.sendMessage.yields(); - const fakeStudy = FAKE_STUDY_LIST[0]; - fakeStudy.studyPaused = true; - await this.core._sendRunState(FAKE_STUDY_LIST, [FAKE_STUDY_ID]); + const fakeLegacyStudy = FAKE_STUDY_LIST[1]; + fakeLegacyStudy.studyPaused = true; + await this.core._sendRunState(FAKE_STUDY_LIST, [FAKE_LEGACY_STUDY_ID]); sinon.spy(this.core._dataCollection, "sendPing"); @@ -671,7 +714,7 @@ describe('Core', function () { {type: "telemetry-ping", data: SENT_PING}, {id: FAKE_STUDY_ID} ), - { message: "Core._handleExternalMessage - test@ion-studies.com is paused and may not send data"} + { message: `Core._handleExternalMessage - ${FAKE_STUDY_ID} is paused and may not send data`} ); assert.ok( @@ -712,6 +755,8 @@ describe('Core', function () { await this.core._unenrollStudy(FAKE_STUDY_ID); assert.equal(await unenrollmentMetrics.studyId.testGetValue("study-unenrollment"), FAKE_STUDY_ID); + assert.equal(await unenrollmentMetrics.schemaNamespace.testGetValue("study-unenrollment"), FAKE_STUDY_NAMESPACE); + unenrollmentPingMock.verify(); }); @@ -739,7 +784,7 @@ describe('Core', function () { // Attempt to unenroll from the study. assert.rejects( this.core._unenrollStudy(FAKE_STUDY_ID), - { message: "Core._unenrollStudy - Unenrolling study which has ended, not sending deletion pings for test@ion-studies.com"} + { message: `Core._unenrollStudy - Unenrolling study which has ended, not sending deletion pings for ${FAKE_STUDY_ID}`} ); // No unenrollment pings are sent. diff --git a/tests/core-addon/unit/DataCollection.test.js b/tests/core-addon/unit/DataCollection.test.js index 203a389e..1e9bc89d 100644 --- a/tests/core-addon/unit/DataCollection.test.js +++ b/tests/core-addon/unit/DataCollection.test.js @@ -10,7 +10,7 @@ import DataCollection from '../../../core-addon/DataCollection.js'; // A fake study id to use in the tests when looking for a // "known" study. -const FAKE_STUDY_ID = "test@ion-studies.com"; +const FAKE_STUDY_ID = "test@rally-studies.com"; describe('DataCollection', function () { beforeEach(function () {