From a43f5507a3a4dd767d5cb71a138f42d89e0e82ff Mon Sep 17 00:00:00 2001 From: James Salter Date: Tue, 14 Sep 2021 14:57:26 +0100 Subject: [PATCH 1/5] Use a UUID instead of hashed user ID for tracking Generate a UUID and save it to account data for cross device tracking. --- src/Lifecycle.ts | 5 +++-- src/PosthogAnalytics.ts | 26 ++++++++++++++++++++++++-- 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/src/Lifecycle.ts b/src/Lifecycle.ts index e48fd52cb1..5f5aeb389f 100644 --- a/src/Lifecycle.ts +++ b/src/Lifecycle.ts @@ -574,11 +574,12 @@ async function doSetLoggedIn( await abortLogin(); } - PosthogAnalytics.instance.updateAnonymityFromSettings(credentials.userId); - Analytics.setLoggedIn(credentials.guest, credentials.homeserverUrl); MatrixClientPeg.replaceUsingCreds(credentials); + + PosthogAnalytics.instance.updateAnonymityFromSettings(credentials.userId); + const client = MatrixClientPeg.get(); if (credentials.freshLogin && SettingsStore.getValue("feature_dehydration")) { diff --git a/src/PosthogAnalytics.ts b/src/PosthogAnalytics.ts index 860a155aff..f01246ec00 100644 --- a/src/PosthogAnalytics.ts +++ b/src/PosthogAnalytics.ts @@ -18,6 +18,7 @@ import posthog, { PostHog } from 'posthog-js'; import PlatformPeg from './PlatformPeg'; import SdkConfig from './SdkConfig'; import SettingsStore from './settings/SettingsStore'; +import { MatrixClientPeg } from "./MatrixClientPeg"; /* Posthog analytics tracking. * @@ -274,9 +275,30 @@ export class PosthogAnalytics { this.anonymity = anonymity; } - public async identifyUser(userId: string): Promise { + private static getUUIDv4(): string { + // https://stackoverflow.com/questions/105034/how-to-create-a-guid-uuid + return ([1e7]+-1e3+-4e3+-8e3+-1e11).replace(/[018]/g, c => + (c ^ crypto.getRandomValues(new Uint8Array(1))[0] & 15 >> c / 4).toString(16), + ); + } + + public async identifyUser(): Promise { if (this.anonymity == Anonymity.Pseudonymous) { - this.posthog.identify(await hashHex(userId)); + // Check the user's account_data for an analytics ID to use. Storing the ID in account_data allows + // different devices to send the same ID. + const client = MatrixClientPeg.get(); + const accountData = await client.getAccountDataFromServer("im.vector.web.analytics_id"); + let analyticsID = accountData?.id; + if (!analyticsID) { + // Couldn't retrieve an analytics ID from user settings, so create one and set it on the server. + // Note there's a race condition here - if two devices do these steps at the same time, last write + // wins, and the first writer will send tracking with an ID that doesn't match the one on the server + // until the next time account data is refreshed and this function is called (most likely on next + // page load). This will happen pretty infrequently, so we can tolerate the possibility. + analyticsID = PosthogAnalytics.getUUIDv4(); + await client.setAccountData("im.vector.web.analytics_id", { id: analyticsID }); + } + this.posthog.identify(analyticsID); } } From 7344a177e3fff83e0f49cb6772925cd5d9b316c1 Mon Sep 17 00:00:00 2001 From: James Salter Date: Tue, 14 Sep 2021 15:53:33 +0100 Subject: [PATCH 2/5] Fix tests, swallow errors --- src/PosthogAnalytics.ts | 34 ++++++++++++++++++++-------------- test/PosthogAnalytics-test.ts | 22 ++++++++++++++++++---- 2 files changed, 38 insertions(+), 18 deletions(-) diff --git a/src/PosthogAnalytics.ts b/src/PosthogAnalytics.ts index f01246ec00..e68cae5390 100644 --- a/src/PosthogAnalytics.ts +++ b/src/PosthogAnalytics.ts @@ -19,6 +19,7 @@ import PlatformPeg from './PlatformPeg'; import SdkConfig from './SdkConfig'; import SettingsStore from './settings/SettingsStore'; import { MatrixClientPeg } from "./MatrixClientPeg"; +import { MatrixClient } from "../../matrix-js-sdk"; /* Posthog analytics tracking. * @@ -282,23 +283,28 @@ export class PosthogAnalytics { ); } - public async identifyUser(): Promise { + public async identifyUser(client: MatrixClientPeg, analyticsIdGenerator: () => string): Promise { if (this.anonymity == Anonymity.Pseudonymous) { // Check the user's account_data for an analytics ID to use. Storing the ID in account_data allows // different devices to send the same ID. - const client = MatrixClientPeg.get(); - const accountData = await client.getAccountDataFromServer("im.vector.web.analytics_id"); - let analyticsID = accountData?.id; - if (!analyticsID) { - // Couldn't retrieve an analytics ID from user settings, so create one and set it on the server. - // Note there's a race condition here - if two devices do these steps at the same time, last write - // wins, and the first writer will send tracking with an ID that doesn't match the one on the server - // until the next time account data is refreshed and this function is called (most likely on next - // page load). This will happen pretty infrequently, so we can tolerate the possibility. - analyticsID = PosthogAnalytics.getUUIDv4(); - await client.setAccountData("im.vector.web.analytics_id", { id: analyticsID }); + try { + const accountData = await client.getAccountDataFromServer("im.vector.web.analytics_id"); + let analyticsID = accountData?.id; + if (!analyticsID) { + // Couldn't retrieve an analytics ID from user settings, so create one and set it on the server. + // Note there's a race condition here - if two devices do these steps at the same time, last write + // wins, and the first writer will send tracking with an ID that doesn't match the one on the server + // until the next time account data is refreshed and this function is called (most likely on next + // page load). This will happen pretty infrequently, so we can tolerate the possibility. + analyticsID = analyticsIdGenerator(); + await client.setAccountData("im.vector.web.analytics_id", { id: analyticsID }); + } + this.posthog.identify(analyticsID); + } catch (e) { + // The above could fail due to network requests, but not essential to starting the application, + // so swallow it. + console.log("Unable to identify user for tracking" + e.toString()); } - this.posthog.identify(analyticsID); } } @@ -371,7 +377,7 @@ export class PosthogAnalytics { // Identify the user (via hashed user ID) to posthog if anonymity is pseudonmyous this.setAnonymity(PosthogAnalytics.getAnonymityFromSettings()); if (userId && this.getAnonymity() == Anonymity.Pseudonymous) { - await this.identifyUser(userId); + await this.identifyUser(MatrixClientPeg.get(), PosthogAnalytics.getUUIDv4); } } } diff --git a/test/PosthogAnalytics-test.ts b/test/PosthogAnalytics-test.ts index 6cb1743051..10dec617fc 100644 --- a/test/PosthogAnalytics-test.ts +++ b/test/PosthogAnalytics-test.ts @@ -24,6 +24,7 @@ import { } from '../src/PosthogAnalytics'; import SdkConfig from '../src/SdkConfig'; +import { MatrixClientPeg } from "../src/MatrixClientPeg"; class FakePosthog { public capture; @@ -218,15 +219,28 @@ bd75b3e080945674c0351f75e0db33d1e90986fa07b318ea7edf776f5eef38d4`); it("Should identify the user to posthog if pseudonymous", async () => { analytics.setAnonymity(Anonymity.Pseudonymous); - await analytics.identifyUser("foo"); - expect(fakePosthog.identify.mock.calls[0][0]) - .toBe("2c26b46b68ffc68ff99b453c1d30413413422d706483bfa0f98a5e886266e7ae"); + class FakeClient { + getAccountDataFromServer = jest.fn().mockResolvedValue(null); + setAccountData = jest.fn().mockResolvedValue({}); + } + await analytics.identifyUser(new FakeClient(), () => "analytics_id" ); + expect(fakePosthog.identify.mock.calls[0][0]).toBe("analytics_id"); }); it("Should not identify the user to posthog if anonymous", async () => { analytics.setAnonymity(Anonymity.Anonymous); - await analytics.identifyUser("foo"); + await analytics.identifyUser(null); expect(fakePosthog.identify.mock.calls.length).toBe(0); }); + + it("Should identify using the server's analytics id if present", async () => { + analytics.setAnonymity(Anonymity.Pseudonymous); + class FakeClient { + getAccountDataFromServer = jest.fn().mockResolvedValue({ id: "existing_analytics_id" }); + setAccountData = jest.fn().mockResolvedValue({}); + } + await analytics.identifyUser(new FakeClient(), () => "new_analytics_id" ); + expect(fakePosthog.identify.mock.calls[0][0]).toBe("existing_analytics_id"); + }); }); }); From 6c1dea09e8b9b6d0370090a06408a34f34771281 Mon Sep 17 00:00:00 2001 From: James Salter Date: Tue, 14 Sep 2021 17:46:56 +0100 Subject: [PATCH 3/5] lint --- src/PosthogAnalytics.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/PosthogAnalytics.ts b/src/PosthogAnalytics.ts index e68cae5390..d026db1303 100644 --- a/src/PosthogAnalytics.ts +++ b/src/PosthogAnalytics.ts @@ -18,8 +18,7 @@ import posthog, { PostHog } from 'posthog-js'; import PlatformPeg from './PlatformPeg'; import SdkConfig from './SdkConfig'; import SettingsStore from './settings/SettingsStore'; -import { MatrixClientPeg } from "./MatrixClientPeg"; -import { MatrixClient } from "../../matrix-js-sdk"; +import { IMatrixClientPeg, MatrixClientPeg } from "./MatrixClientPeg"; /* Posthog analytics tracking. * @@ -278,12 +277,12 @@ export class PosthogAnalytics { private static getUUIDv4(): string { // https://stackoverflow.com/questions/105034/how-to-create-a-guid-uuid - return ([1e7]+-1e3+-4e3+-8e3+-1e11).replace(/[018]/g, c => + return ("10000000-1000-4000-8000-100000000000").replace(/[018]/g, c => (c ^ crypto.getRandomValues(new Uint8Array(1))[0] & 15 >> c / 4).toString(16), ); } - public async identifyUser(client: MatrixClientPeg, analyticsIdGenerator: () => string): Promise { + public async identifyUser(client: IMatrixClientPeg, analyticsIdGenerator: () => string): Promise { if (this.anonymity == Anonymity.Pseudonymous) { // Check the user's account_data for an analytics ID to use. Storing the ID in account_data allows // different devices to send the same ID. From c2192a78bce9ae9f328087bb566c98839643e533 Mon Sep 17 00:00:00 2001 From: James Salter Date: Tue, 14 Sep 2021 18:16:48 +0100 Subject: [PATCH 4/5] More lint --- src/PosthogAnalytics.ts | 14 ++++++-------- test/PosthogAnalytics-test.ts | 1 - 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/src/PosthogAnalytics.ts b/src/PosthogAnalytics.ts index d026db1303..422fcc475f 100644 --- a/src/PosthogAnalytics.ts +++ b/src/PosthogAnalytics.ts @@ -18,7 +18,8 @@ import posthog, { PostHog } from 'posthog-js'; import PlatformPeg from './PlatformPeg'; import SdkConfig from './SdkConfig'; import SettingsStore from './settings/SettingsStore'; -import { IMatrixClientPeg, MatrixClientPeg } from "./MatrixClientPeg"; +import { MatrixClientPeg } from "./MatrixClientPeg"; +import { MatrixClient } from "../../matrix-js-sdk"; /* Posthog analytics tracking. * @@ -275,14 +276,11 @@ export class PosthogAnalytics { this.anonymity = anonymity; } - private static getUUIDv4(): string { - // https://stackoverflow.com/questions/105034/how-to-create-a-guid-uuid - return ("10000000-1000-4000-8000-100000000000").replace(/[018]/g, c => - (c ^ crypto.getRandomValues(new Uint8Array(1))[0] & 15 >> c / 4).toString(16), - ); + private static getRandomAnalyticsId(): string { + return [...crypto.getRandomValues(new Uint8Array(16))].map((c) => c.toString(16)).join(''); } - public async identifyUser(client: IMatrixClientPeg, analyticsIdGenerator: () => string): Promise { + public async identifyUser(client: MatrixClient, analyticsIdGenerator: () => string): Promise { if (this.anonymity == Anonymity.Pseudonymous) { // Check the user's account_data for an analytics ID to use. Storing the ID in account_data allows // different devices to send the same ID. @@ -376,7 +374,7 @@ export class PosthogAnalytics { // Identify the user (via hashed user ID) to posthog if anonymity is pseudonmyous this.setAnonymity(PosthogAnalytics.getAnonymityFromSettings()); if (userId && this.getAnonymity() == Anonymity.Pseudonymous) { - await this.identifyUser(MatrixClientPeg.get(), PosthogAnalytics.getUUIDv4); + await this.identifyUser(MatrixClientPeg.get(), PosthogAnalytics.getRandomAnalyticsId); } } } diff --git a/test/PosthogAnalytics-test.ts b/test/PosthogAnalytics-test.ts index 10dec617fc..2832fbe92e 100644 --- a/test/PosthogAnalytics-test.ts +++ b/test/PosthogAnalytics-test.ts @@ -24,7 +24,6 @@ import { } from '../src/PosthogAnalytics'; import SdkConfig from '../src/SdkConfig'; -import { MatrixClientPeg } from "../src/MatrixClientPeg"; class FakePosthog { public capture; From 48fbbf2f444f9bd0069e47896945ed9182d93768 Mon Sep 17 00:00:00 2001 From: James Salter Date: Wed, 15 Sep 2021 09:48:48 +0100 Subject: [PATCH 5/5] Fix import, convert event type to constant --- src/PosthogAnalytics.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/PosthogAnalytics.ts b/src/PosthogAnalytics.ts index 422fcc475f..ca0d321e7c 100644 --- a/src/PosthogAnalytics.ts +++ b/src/PosthogAnalytics.ts @@ -19,7 +19,7 @@ import PlatformPeg from './PlatformPeg'; import SdkConfig from './SdkConfig'; import SettingsStore from './settings/SettingsStore'; import { MatrixClientPeg } from "./MatrixClientPeg"; -import { MatrixClient } from "../../matrix-js-sdk"; +import { MatrixClient } from "matrix-js-sdk/src/client"; /* Posthog analytics tracking. * @@ -143,6 +143,7 @@ export class PosthogAnalytics { private enabled = false; private static _instance = null; private platformSuperProperties = {}; + private static ANALYTICS_ID_EVENT_TYPE = "im.vector.web.analytics_id"; public static get instance(): PosthogAnalytics { if (!this._instance) { @@ -285,7 +286,7 @@ export class PosthogAnalytics { // Check the user's account_data for an analytics ID to use. Storing the ID in account_data allows // different devices to send the same ID. try { - const accountData = await client.getAccountDataFromServer("im.vector.web.analytics_id"); + const accountData = await client.getAccountDataFromServer(PosthogAnalytics.ANALYTICS_ID_EVENT_TYPE); let analyticsID = accountData?.id; if (!analyticsID) { // Couldn't retrieve an analytics ID from user settings, so create one and set it on the server.