Interface changes and anonymity fixes

src/Lifecycle.ts

@@ -48,7 +48,7 @@ import { Jitsi } from "./widgets/Jitsi";
 import { SSO_HOMESERVER_URL_KEY, SSO_ID_SERVER_URL_KEY, SSO_IDP_ID_KEY } from "./BasePlatform";
 import ThreepidInviteStore from "./stores/ThreepidInviteStore";
 import CountlyAnalytics from "./CountlyAnalytics";
-import { Anonymity, getAnalytics, getPlatformProperties } from "./PosthogAnalytics";
+import { getAnalytics } from "./PosthogAnalytics";
 import CallHandler from './CallHandler';
 import LifecycleCustomisations from "./customisations/Lifecycle";
 import ErrorDialog from "./components/views/dialogs/ErrorDialog";
@@ -574,13 +574,7 @@ async function doSetLoggedIn(
         await abortLogin();
     }
 
-    if (SettingsStore.getValue("analyticsOptIn")) {
+    getAnalytics().updateAnonymityFromSettings(credentials.userId);
-        const analytics = getAnalytics();
-        analytics.setAnonymity(Anonymity.Pseudonymous);
-        await analytics.identifyUser(credentials.userId);
-    } else {
-        getAnalytics().setAnonymity(Anonymity.Anonymous);
-    }
 
     Analytics.setLoggedIn(credentials.guest, credentials.homeserverUrl);
 

src/PosthogAnalytics.ts

@@ -85,14 +85,10 @@ export async function getRedactedCurrentLocation(origin: string, hash: string, p
 export class PosthogAnalytics {
     private anonymity = Anonymity.Anonymous;
     private posthog?: PostHog = null;
-
+    // set true during the constructor if posthog config is present, otherwise false
-    // set true during init() if posthog config is present
     private enabled = false;
-
-    // set to true after init() has been called
-    private initialised = false;
-
     private static _instance = null;
+    private platformSuperProperties = {};
 
     public static instance(): PosthogAnalytics {
         if (!this._instance) {
@@ -103,10 +99,6 @@ export class PosthogAnalytics {
 
     constructor(posthog: PostHog) {
         this.posthog = posthog;
-    }
-
-    public init(anonymity: Anonymity) {
-        this.anonymity = anonymity;
         const posthogConfig = SdkConfig.get()["posthog"];
         if (posthogConfig) {
             this.posthog.init(posthogConfig.projectApiKey, {
@@ -123,7 +115,6 @@ export class PosthogAnalytics {
                 sanitize_properties: this.sanitizeProperties.bind(this),
                 respect_dnt: true,
             });
-            this.initialised = true;
             this.enabled = true;
         } else {
             this.enabled = false;
@@ -159,19 +150,39 @@ export class PosthogAnalytics {
         return properties;
     }
 
-    public async identifyUser(userId: string) {
+    private static getAnonymityFromSettings(): Anonymity {
-        if (this.anonymity == Anonymity.Anonymous) return;
+        // determine the current anonymity level based on curernt user settings
-        this.posthog.identify(await hashHex(userId));
+
+        // "Send anonymous usage data which helps us improve Element. This will use a cookie."
+        const analyticsOptIn = SettingsStore.getValue("analyticsOptIn");
+
+        // "Send pseudonymous usage data which helps us improve Element. This will use a cookie."
+        //
+        // Currently, this is only a labs flag, for testing purposes.
+        const pseudonumousOptIn = SettingsStore.getValue("feature_pseudonymousAnalyticsOptIn");
+
+        let anonymity;
+        if (pseudonumousOptIn) {
+            anonymity = Anonymity.Pseudonymous;
+        } else if (analyticsOptIn) {
+            anonymity = Anonymity.Anonymous;
+        } else {
+            anonymity = Anonymity.Disabled;
+        }
+
+        return anonymity;
     }
 
-    public registerSuperProperties(properties) {
+    public async identifyUser(userId: string) {
-        if (this.enabled) {
+        if (this.anonymity == Anonymity.Pseudonymous) {
-            this.posthog.register(properties);
+            this.posthog.identify(await hashHex(userId));
         }
     }
 
-    public isInitialised() {
+    private registerSuperProperties(properties) {
-        return this.initialised;
+        if (this.enabled) {
+            this.posthog.register(properties);
+        }
     }
 
     public isEnabled() {
@@ -179,6 +190,13 @@ export class PosthogAnalytics {
     }
 
     public setAnonymity(anonymity: Anonymity) {
+        if (this.enabled && (anonymity == Anonymity.Disabled || anonymity == Anonymity.Anonymous)) {
+            // when transitioning to Disabled or Anonymous ensure we clear out any prior state
+            // set in posthog e.g. distinct ID
+            this.posthog.reset();
+            // Restore any previously set platform super properties
+            this.registerSuperProperties(this.platformSuperProperties);
+        }
         this.anonymity = anonymity;
     }
 
@@ -194,9 +212,6 @@ export class PosthogAnalytics {
     }
 
     private async capture(eventName: string, properties: posthog.Properties) {
-        if (!this.initialised) {
-            throw Error("Tried to track event before PoshogAnalytics.init has completed");
-        }
         if (!this.enabled) {
             return;
         }
@@ -239,45 +254,36 @@ export class PosthogAnalytics {
             durationMs,
         });
     }
-}
 
-export async function getPlatformProperties() {
+    public async updatePlatformSuperProperties() {
-    const platform = PlatformPeg.get();
+        this.platformSuperProperties = await PosthogAnalytics.getPlatformProperties();
-    let appVersion;
+        this.registerSuperProperties(this.platformSuperProperties);
-    try {
-        appVersion = await platform.getAppVersion();
-    } catch (e) {
-        // this happens if no version is set i.e. in dev
-        appVersion = "unknown";
     }
 
-    return {
+    private static async getPlatformProperties() {
-        appVersion,
+        const platform = PlatformPeg.get();
-        appPlatform: platform.getHumanReadableName(),
+        let appVersion;
-    };
+        try {
+            appVersion = await platform.getAppVersion();
+        } catch (e) {
+            // this happens if no version is set i.e. in dev
+            appVersion = "unknown";
+        }
+
+        return {
+            appVersion,
+            appPlatform: platform.getHumanReadableName(),
+        };
+    }
+
+    public async updateAnonymityFromSettings(userId?: string) {
+        this.setAnonymity(PosthogAnalytics.getAnonymityFromSettings());
+        if (userId && this.getAnonymity() == Anonymity.Pseudonymous) {
+            await this.identifyUser(userId);
+        }
+    }
 }
 
 export function getAnalytics(): PosthogAnalytics {
     return PosthogAnalytics.instance();
 }
-
-export function getAnonymityFromSettings(): Anonymity {
-    // determine the current anonymity level based on curernt user settings
-
-    // "Send anonymous usage data which helps us improve Element. This will use a cookie."
-    const analyticsOptIn = SettingsStore.getValue("analyticsOptIn");
-
-    // "Send pseudonymous usage data which helps us improve Element. This will use a cookie."
-    const pseudonumousOptIn = SettingsStore.getValue("pseudonymousAnalyticsOptIn");
-
-    let anonymity;
-    if (pseudonumousOptIn) {
-        anonymity = Anonymity.Pseudonymous;
-    } else if (analyticsOptIn) {
-        anonymity = Anonymity.Anonymous;
-    } else {
-        anonymity = Anonymity.Disabled;
-    }
-
-    return anonymity;
-}

src/components/structures/MatrixChat.tsx

@@ -107,7 +107,7 @@ import UIStore, { UI_EVENTS } from "../../stores/UIStore";
 import SoftLogout from './auth/SoftLogout';
 import { makeRoomPermalink } from "../../utils/permalinks/Permalinks";
 import { copyPlaintext } from "../../utils/strings";
-import { Anonymity, getAnalytics, getAnonymityFromSettings, getPlatformProperties } from '../../PosthogAnalytics';
+import { getAnalytics } from '../../PosthogAnalytics';
 
 /** constants for MatrixChat.state.view */
 export enum Views {
@@ -390,10 +390,8 @@ export default class MatrixChat extends React.PureComponent<IProps, IState> {
         }
 
         const analytics = getAnalytics();
-        analytics.init(getAnonymityFromSettings());
+        analytics.updateAnonymityFromSettings();
-        // note this requires a network request in the browser, so some events can potentially
+        analytics.updatePlatformSuperProperties();
-        // before before registerSuperProperties has been called
-        getPlatformProperties().then((properties) => analytics.registerSuperProperties(properties));
 
         CountlyAnalytics.instance.enable(/* anonymous = */ true);
     }
@@ -831,11 +829,6 @@ export default class MatrixChat extends React.PureComponent<IProps, IState> {
                 if (CountlyAnalytics.instance.canEnable()) {
                     CountlyAnalytics.instance.enable(/* anonymous = */ false);
                 }
-                getAnalytics().setAnonymity(Anonymity.Pseudonymous);
-                // TODO: this is an async call and we're not waiting for it to complete -
-                // so potentially an event could be fired prior to it completing and would be
-                // missing the user identification.
-                getAnalytics().identifyUser(MatrixClientPeg.get().getUserId());
                 break;
             case 'reject_cookies':
                 SettingsStore.setValue("analyticsOptIn", null, SettingLevel.DEVICE, false);

src/components/views/settings/tabs/user/SecurityUserSettingsTab.js

@@ -36,7 +36,7 @@ import { UIFeature } from "../../../../../settings/UIFeature";
 import { isE2eAdvancedPanelPossible } from "../../E2eAdvancedPanel";
 import CountlyAnalytics from "../../../../../CountlyAnalytics";
 import { replaceableComponent } from "../../../../../utils/replaceableComponent";
-import { Anonymity, getAnalytics } from "../../../../../PosthogAnalytics";
+import { getAnalytics } from "../../../../../PosthogAnalytics";
 
 export class IgnoredUser extends React.Component {
     static propTypes = {
@@ -107,7 +107,7 @@ export default class SecurityUserSettingsTab extends React.Component {
     _updateAnalytics = (checked) => {
         checked ? Analytics.enable() : Analytics.disable();
         CountlyAnalytics.instance.enable(/* anonymous = */ !checked);
-        getAnalytics().setAnonymity(checked ? Anonymity.Pseudonymous : Anonymity.Anonymous);
+        getAnalytics().updateAnonymityFromSettings(MatrixClientPeg.get().getUserId());
     };
 
     _onExportE2eKeysClicked = () => {

src/settings/Settings.tsx

@@ -41,6 +41,7 @@ import { Layout } from "./Layout";
 import ReducedMotionController from './controllers/ReducedMotionController';
 import IncompatibleController from "./controllers/IncompatibleController";
 import SdkConfig from "../SdkConfig";
+import PseudonymousAnalyticsController from './controllers/PseudonymousAnalyticsController';
 
 // These are just a bunch of helper arrays to avoid copy/pasting a bunch of times
 const LEVELS_ROOM_SETTINGS = [
@@ -297,6 +298,13 @@ export const SETTINGS: {[setting: string]: ISetting} = {
         supportedLevels: LEVELS_FEATURE,
         default: false,
     },
+    "feature_pseudonymousAnalyticsOptIn": {
+        isFeature: true,
+        supportedLevels: LEVELS_FEATURE,
+        displayName: _td('Send pseudonymous analytics data'),
+        default: false,
+        controller: new PseudonymousAnalyticsController(),
+    },
     "advancedRoomListLogging": {
         // TODO: Remove flag before launch: https://github.com/vector-im/element-web/issues/14231
         displayName: _td("Enable advanced debugging for the room list"),

src/settings/controllers/PseudonymousAnalyticsController.ts

@@ -0,0 +1,26 @@
+/*
+Copyright 2021 The Matrix.org Foundation C.I.C.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+import SettingController from "./SettingController";
+import { SettingLevel } from "../SettingLevel";
+import { getAnalytics } from "../../PosthogAnalytics";
+import { MatrixClientPeg } from "../../MatrixClientPeg";
+
+export default class PseudonymousAnalyticsController extends SettingController {
+    public onChange(level: SettingLevel, roomId: string, newValue: any) {
+        getAnalytics().updateAnonymityFromSettings(MatrixClientPeg.get().getUserId());
+    }
+}

test/PosthogAnalytics-test.ts

@@ -7,11 +7,15 @@ class FakePosthog {
     public capture;
     public init;
     public identify;
+    public reset;
+    public register;
 
     constructor() {
         this.capture = jest.fn();
         this.init = jest.fn();
         this.identify = jest.fn();
+        this.reset = jest.fn();
+        this.register = jest.fn();
     }
 }
 
@@ -37,12 +41,11 @@ export interface ITestRoomEvent extends IRoomEvent {
 }
 
 describe("PosthogAnalytics", () => {
-    let analytics: PosthogAnalytics;
     let fakePosthog: FakePosthog;
 
     beforeEach(() => {
         fakePosthog = new FakePosthog();
-        analytics = new PosthogAnalytics(fakePosthog);
+
         window.crypto = {
             subtle: crypto.webcrypto.subtle,
         };
@@ -53,26 +56,28 @@ describe("PosthogAnalytics", () => {
     });
 
     describe("Initialisation", () => {
-        it("Should not initialise if config is not set", async () => {
+        it("Should not be enabled without config being set", () => {
             jest.spyOn(SdkConfig, "get").mockReturnValue({});
-            analytics.init(Anonymity.Pseudonymous);
+            const analytics = new PosthogAnalytics(fakePosthog);
             expect(analytics.isEnabled()).toBe(false);
         });
 
-        it("Should initialise if config is set", async () => {
+        it("Should be enabled if config is set", () => {
             jest.spyOn(SdkConfig, "get").mockReturnValue({
                 posthog: {
                     projectApiKey: "foo",
                     apiHost: "bar",
                 },
             });
-            analytics.init(Anonymity.Pseudonymous);
+            const analytics = new PosthogAnalytics(fakePosthog);
-            expect(analytics.isInitialised()).toBe(true);
+            analytics.setAnonymity(Anonymity.Pseudonymous);
             expect(analytics.isEnabled()).toBe(true);
         });
     });
 
     describe("Tracking", () => {
+        let analytics: PosthogAnalytics;
+
         beforeEach(() => {
             jest.spyOn(SdkConfig, "get").mockReturnValue({
                 posthog: {
@@ -80,10 +85,12 @@ describe("PosthogAnalytics", () => {
                     apiHost: "bar",
                 },
             });
+
+            analytics = new PosthogAnalytics(fakePosthog);
         });
 
         it("Should pass trackAnonymousEvent() to posthog", async () => {
-            analytics.init(Anonymity.Pseudonymous);
+            analytics.setAnonymity(Anonymity.Pseudonymous);
             await analytics.trackAnonymousEvent<ITestEvent>("jest_test_event", {
                 foo: "bar",
             });
@@ -92,7 +99,7 @@ describe("PosthogAnalytics", () => {
         });
 
         it("Should pass trackRoomEvent to posthog", async () => {
-            analytics.init(Anonymity.Pseudonymous);
+            analytics.setAnonymity(Anonymity.Pseudonymous);
             const roomId = "42";
             await analytics.trackRoomEvent<IRoomEvent>("jest_test_event", roomId, {
                 foo: "bar",
@@ -104,7 +111,7 @@ describe("PosthogAnalytics", () => {
         });
 
         it("Should pass trackPseudonymousEvent() to posthog", async () => {
-            analytics.init(Anonymity.Pseudonymous);
+            analytics.setAnonymity(Anonymity.Pseudonymous);
             await analytics.trackPseudonymousEvent<ITestEvent>("jest_test_pseudo_event", {
                 foo: "bar",
             });
@@ -112,17 +119,8 @@ describe("PosthogAnalytics", () => {
             expect(fakePosthog.capture.mock.calls[0][1]["foo"]).toEqual("bar");
         });
 
-        it("Should blow up if not inititalised prior to tracking", async () => {
-            const fn = () => {
-                return analytics.trackAnonymousEvent<ITestEvent>("jest_test_event", {
-                   foo: "bar",
-                });
-            };
-            await expect(fn()).rejects.toThrow();
-        });
-
         it("Should not track pseudonymous messages if anonymous", async () => {
-            analytics.init(Anonymity.Anonymous);
+            analytics.setAnonymity(Anonymity.Anonymous);
             await analytics.trackPseudonymousEvent<ITestEvent>("jest_test_event", {
                 foo: "bar",
             });
@@ -130,7 +128,7 @@ describe("PosthogAnalytics", () => {
         });
 
         it("Should not track any events if disabled", async () => {
-            analytics.init(Anonymity.Disabled);
+            analytics.setAnonymity(Anonymity.Disabled);
             await analytics.trackPseudonymousEvent<ITestEvent>("jest_test_event", {
                 foo: "bar",
             });
@@ -181,14 +179,14 @@ bd75b3e080945674c0351f75e0db33d1e90986fa07b318ea7edf776f5eef38d4`);
         });
 
         it("Should identify the user to posthog if pseudonymous", async () => {
-            analytics.init(Anonymity.Pseudonymous);
+            analytics.setAnonymity(Anonymity.Pseudonymous);
             await analytics.identifyUser("foo");
             expect(fakePosthog.identify.mock.calls[0][0])
                 .toBe("2c26b46b68ffc68ff99b453c1d30413413422d706483bfa0f98a5e886266e7ae");
         });
 
         it("Should not identify the user to posthog if anonymous", async () => {
-            analytics.init(Anonymity.Anonymous);
+            analytics.setAnonymity(Anonymity.Anonymous);
             await analytics.identifyUser("foo");
             expect(fakePosthog.identify.mock.calls.length).toBe(0);
         });