From ae8f7873449bf42ef8592b9c3000662041872645 Mon Sep 17 00:00:00 2001 From: Valere Date: Wed, 8 Nov 2023 11:27:47 +0100 Subject: [PATCH 1/3] Report crypto sdk in posthog --- src/PosthogAnalytics.ts | 41 +++++++++++++- test/PosthogAnalytics-test.ts | 101 ++++++++++++++++++++++++++++++++++ 2 files changed, 141 insertions(+), 1 deletion(-) diff --git a/src/PosthogAnalytics.ts b/src/PosthogAnalytics.ts index 93b84ff3059..70ebc4132a7 100644 --- a/src/PosthogAnalytics.ts +++ b/src/PosthogAnalytics.ts @@ -136,6 +136,11 @@ export class PosthogAnalytics { private authenticationType: Signup["authenticationType"] = "Other"; private watchSettingRef?: string; + // Temporary flag until we can switch to the Rust SDK without restarting the app. + // Currently, you have to set up the flag then logout and login again to switch. + // On login the matrixClient is passed to the analytics object, and we can check the crypto backend version. + private hasLoggedInWithRustCrypto = false; + public static get instance(): PosthogAnalytics { if (!this._instance) { this._instance = new PosthogAnalytics(posthog); @@ -169,7 +174,9 @@ export class PosthogAnalytics { dis.register(this.onAction); SettingsStore.monitorSetting("layout", null); SettingsStore.monitorSetting("useCompactLayout", null); + SettingsStore.monitorSetting("feature_rust_crypto", null); this.onLayoutUpdated(); + this.updateCryptoSuperProperty(); } private onLayoutUpdated = (): void => { @@ -198,6 +205,9 @@ export class PosthogAnalytics { if (["layout", "useCompactLayout"].includes(settingsPayload.settingName)) { this.onLayoutUpdated(); } + if (["feature_rust_crypto"].includes(settingsPayload.settingName)) { + this.updateCryptoSuperProperty(); + } }; // we persist the last `$screen_name` and send it for all events until it is replaced @@ -251,6 +261,14 @@ export class PosthogAnalytics { }; } + private getCryptoSDKPropertyValue(): "Rust" | "Legacy" { + // Until we have migration code to switch crypto sdk, we need to track which sdk is being used. + if (SettingsStore.getValue("feature_rust_crypto") && this.hasLoggedInWithRustCrypto) { + return "Rust"; + } + return "Legacy"; + } + // eslint-disable-nextline no-unused-vars private capture(eventName: string, properties: Properties, options?: CaptureOptions): void { if (!this.enabled) { @@ -278,6 +296,8 @@ export class PosthogAnalytics { this.registerSuperProperties(this.platformSuperProperties); } this.anonymity = anonymity; + // update anyhow, no-op if not enabled or Disabled. + this.updateCryptoSuperProperty(); } private static getRandomAnalyticsId(): string { @@ -367,7 +387,25 @@ export class PosthogAnalytics { this.registerSuperProperties(this.platformSuperProperties); } + private updateCryptoSuperProperty(): void { + if (!this.enabled || this.anonymity === Anonymity.Disabled) return; + // Update super property for cryptoSDK in posthog. + // This property will be subsequently passed in every event. + const value = this.getCryptoSDKPropertyValue(); + this.registerSuperProperties({ cryptoSDK: value }); + } + public async updateAnonymityFromSettings(client: MatrixClient, pseudonymousOptIn: boolean): Promise { + // Temporary until we have migration code to switch crypto sdk. + if (client.getCrypto()) { + const cryptoVersion = client.getCrypto()!.getVersion(); + // version for rust is something like "Rust SDK 0.6.0 (9c6b550), Vodozemac 0.5.0" + // for legacy it will be 'Olm x.x.x" + if (cryptoVersion.indexOf("Rust SDK") != -1) { + this.hasLoggedInWithRustCrypto = true; + } + } + // Update this.anonymity based on the user's analytics opt-in settings const anonymity = pseudonymousOptIn ? Anonymity.Pseudonymous : Anonymity.Disabled; this.setAnonymity(anonymity); @@ -379,7 +417,8 @@ export class PosthogAnalytics { } if (anonymity !== Anonymity.Disabled) { - await PosthogAnalytics.instance.updatePlatformSuperProperties(); + await this.updatePlatformSuperProperties(); + this.updateCryptoSuperProperty(); } } diff --git a/test/PosthogAnalytics-test.ts b/test/PosthogAnalytics-test.ts index 1ba7c01d533..aa6d4bfe07e 100644 --- a/test/PosthogAnalytics-test.ts +++ b/test/PosthogAnalytics-test.ts @@ -25,6 +25,7 @@ import { Layout } from "../src/settings/enums/Layout"; import defaultDispatcher from "../src/dispatcher/dispatcher"; import { Action } from "../src/dispatcher/actions"; import { SettingLevel } from "../src/settings/SettingLevel"; +import { CryptoApi, MatrixClient } from "../../matrix-js-sdk"; const getFakePosthog = (): PostHog => ({ @@ -37,6 +38,7 @@ const getFakePosthog = (): PostHog => persistence: { get_user_state: jest.fn(), }, + identifyUser: jest.fn(), } as unknown as PostHog); interface ITestEvent extends IPosthogEvent { @@ -274,4 +276,103 @@ describe("PosthogAnalytics", () => { }); }); }); + + describe("CryptoSdk", () => { + let analytics: PosthogAnalytics; + const getFakeClient = (): MatrixClient => + ({ + getCrypto: jest.fn(), + setAccountData: jest.fn(), + // just fake return an `im.vector.analytics` content + getAccountDataFromServer: jest.fn().mockReturnValue({ + id: "0000000", + pseudonymousAnalyticsOptIn: true, + }), + } as unknown as MatrixClient); + + beforeEach(async () => { + SdkConfig.put({ + brand: "Testing", + posthog: { + project_api_key: "foo", + api_host: "bar", + }, + }); + + analytics = new PosthogAnalytics(fakePosthog); + }); + + async function simulateLoginWorkaroundUntilMigration(pseudonymous = true) { + // To simulate a switch we call updateAnonymityFromSettings. + // As per documentation this function is called On login. + const mockClient = getFakeClient(); + mocked(mockClient.getCrypto).mockReturnValue({ + getVersion: () => "Rust SDK 0.6.0 (9c6b550), Vodozemac 0.5.0", + } as unknown as CryptoApi); + await analytics.updateAnonymityFromSettings(mockClient, pseudonymous); + } + + it("should send rust cryptoSDK superProperty correctly", async () => { + analytics.setAnonymity(Anonymity.Pseudonymous); + + await SettingsStore.setValue("feature_rust_crypto", null, SettingLevel.DEVICE, true); + defaultDispatcher.dispatch( + { + action: Action.SettingUpdated, + settingName: "feature_rust_crypto", + }, + true, + ); + + // Has for now you need to logout and login again to make the switch, until then it sbould stay as Legacy + expect(mocked(fakePosthog).register.mock.lastCall![0]["cryptoSDK"]).toStrictEqual("Legacy"); + // Simulate a logout / login with a rust crypto backend + await simulateLoginWorkaroundUntilMigration(); + + // Super Properties are properties associated with events that are set once and then sent with every capture call. + // They are set using posthog.register + expect(mocked(fakePosthog).register.mock.lastCall![0]["cryptoSDK"]).toStrictEqual("Rust"); + }); + + it("should send Legacy cryptoSDK superProperty correctly", async () => { + analytics.setAnonymity(Anonymity.Pseudonymous); + + await SettingsStore.setValue("feature_rust_crypto", null, SettingLevel.DEVICE, false); + defaultDispatcher.dispatch( + { + action: Action.SettingUpdated, + settingName: "feature_rust_crypto", + }, + true, + ); + // Super Properties are properties associated with events that are set once and then sent with every capture call. + // They are set using posthog.register + expect(mocked(fakePosthog).register.mock.lastCall![0]["cryptoSDK"]).toStrictEqual("Legacy"); + }); + + it("should send cryptoSDK superProperty when enabling analytics", async () => { + analytics.setAnonymity(Anonymity.Disabled); + + await SettingsStore.setValue("feature_rust_crypto", null, SettingLevel.DEVICE, true); + + defaultDispatcher.dispatch( + { + action: Action.SettingUpdated, + settingName: "feature_rust_crypto", + }, + true, + ); + + await simulateLoginWorkaroundUntilMigration(false); + + // This initial call is due to the call to register platformSuperProperties + expect(mocked(fakePosthog).register.mock.lastCall![0]).toStrictEqual({}); + + // switching to pseudonymous should ensure that the cryptoSDK superProperty is set correctly + analytics.setAnonymity(Anonymity.Pseudonymous); + // Super Properties are properties associated with events that are set once and then sent with every capture call. + // They are set using posthog.register + expect(mocked(fakePosthog).register.mock.lastCall![0]["cryptoSDK"]).toStrictEqual("Rust"); + }); + }); }); From 3ccc67e315f9d68d76337fe2bd023c65f772c3e1 Mon Sep 17 00:00:00 2001 From: Valere Date: Thu, 9 Nov 2023 12:24:54 +0100 Subject: [PATCH 2/3] fix import --- test/PosthogAnalytics-test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/PosthogAnalytics-test.ts b/test/PosthogAnalytics-test.ts index aa6d4bfe07e..a1416cb9597 100644 --- a/test/PosthogAnalytics-test.ts +++ b/test/PosthogAnalytics-test.ts @@ -16,6 +16,7 @@ limitations under the License. import { mocked } from "jest-mock"; import { PostHog } from "posthog-js"; +import { CryptoApi, MatrixClient } from "matrix-js-sdk/src/matrix"; import { Anonymity, getRedactedCurrentLocation, IPosthogEvent, PosthogAnalytics } from "../src/PosthogAnalytics"; import SdkConfig from "../src/SdkConfig"; @@ -25,7 +26,6 @@ import { Layout } from "../src/settings/enums/Layout"; import defaultDispatcher from "../src/dispatcher/dispatcher"; import { Action } from "../src/dispatcher/actions"; import { SettingLevel } from "../src/settings/SettingLevel"; -import { CryptoApi, MatrixClient } from "../../matrix-js-sdk"; const getFakePosthog = (): PostHog => ({ From 219c6c4fa4bc5473293dbfc4a32d86ff8a46c08b Mon Sep 17 00:00:00 2001 From: Valere Date: Thu, 9 Nov 2023 17:23:06 +0100 Subject: [PATCH 3/3] do not use the setting as not live --- src/PosthogAnalytics.ts | 29 +++++++-------------- test/PosthogAnalytics-test.ts | 47 ++++++++--------------------------- 2 files changed, 20 insertions(+), 56 deletions(-) diff --git a/src/PosthogAnalytics.ts b/src/PosthogAnalytics.ts index 70ebc4132a7..20c4ff2f332 100644 --- a/src/PosthogAnalytics.ts +++ b/src/PosthogAnalytics.ts @@ -136,10 +136,8 @@ export class PosthogAnalytics { private authenticationType: Signup["authenticationType"] = "Other"; private watchSettingRef?: string; - // Temporary flag until we can switch to the Rust SDK without restarting the app. - // Currently, you have to set up the flag then logout and login again to switch. - // On login the matrixClient is passed to the analytics object, and we can check the crypto backend version. - private hasLoggedInWithRustCrypto = false; + // Will be set when the matrixClient is passed to the analytics object (e.g. on login). + private currentCryptoBackend?: "Rust" | "Legacy" = undefined; public static get instance(): PosthogAnalytics { if (!this._instance) { @@ -174,7 +172,6 @@ export class PosthogAnalytics { dis.register(this.onAction); SettingsStore.monitorSetting("layout", null); SettingsStore.monitorSetting("useCompactLayout", null); - SettingsStore.monitorSetting("feature_rust_crypto", null); this.onLayoutUpdated(); this.updateCryptoSuperProperty(); } @@ -205,9 +202,6 @@ export class PosthogAnalytics { if (["layout", "useCompactLayout"].includes(settingsPayload.settingName)) { this.onLayoutUpdated(); } - if (["feature_rust_crypto"].includes(settingsPayload.settingName)) { - this.updateCryptoSuperProperty(); - } }; // we persist the last `$screen_name` and send it for all events until it is replaced @@ -261,14 +255,6 @@ export class PosthogAnalytics { }; } - private getCryptoSDKPropertyValue(): "Rust" | "Legacy" { - // Until we have migration code to switch crypto sdk, we need to track which sdk is being used. - if (SettingsStore.getValue("feature_rust_crypto") && this.hasLoggedInWithRustCrypto) { - return "Rust"; - } - return "Legacy"; - } - // eslint-disable-nextline no-unused-vars private capture(eventName: string, properties: Properties, options?: CaptureOptions): void { if (!this.enabled) { @@ -391,8 +377,9 @@ export class PosthogAnalytics { if (!this.enabled || this.anonymity === Anonymity.Disabled) return; // Update super property for cryptoSDK in posthog. // This property will be subsequently passed in every event. - const value = this.getCryptoSDKPropertyValue(); - this.registerSuperProperties({ cryptoSDK: value }); + if (this.currentCryptoBackend) { + this.registerSuperProperties({ cryptoSDK: this.currentCryptoBackend }); + } } public async updateAnonymityFromSettings(client: MatrixClient, pseudonymousOptIn: boolean): Promise { @@ -401,8 +388,10 @@ export class PosthogAnalytics { const cryptoVersion = client.getCrypto()!.getVersion(); // version for rust is something like "Rust SDK 0.6.0 (9c6b550), Vodozemac 0.5.0" // for legacy it will be 'Olm x.x.x" - if (cryptoVersion.indexOf("Rust SDK") != -1) { - this.hasLoggedInWithRustCrypto = true; + if (cryptoVersion.includes("Rust SDK")) { + this.currentCryptoBackend = "Rust"; + } else { + this.currentCryptoBackend = "Legacy"; } } diff --git a/test/PosthogAnalytics-test.ts b/test/PosthogAnalytics-test.ts index a1416cb9597..d73ee1ec669 100644 --- a/test/PosthogAnalytics-test.ts +++ b/test/PosthogAnalytics-test.ts @@ -302,12 +302,16 @@ describe("PosthogAnalytics", () => { analytics = new PosthogAnalytics(fakePosthog); }); - async function simulateLoginWorkaroundUntilMigration(pseudonymous = true) { + // `updateAnonymityFromSettings` is called On page load / login / account data change. + // We manually call it so we can test the behaviour. + async function simulateLogin(rustBackend: boolean, pseudonymous = true) { // To simulate a switch we call updateAnonymityFromSettings. // As per documentation this function is called On login. const mockClient = getFakeClient(); mocked(mockClient.getCrypto).mockReturnValue({ - getVersion: () => "Rust SDK 0.6.0 (9c6b550), Vodozemac 0.5.0", + getVersion: () => { + return rustBackend ? "Rust SDK 0.6.0 (9c6b550), Vodozemac 0.5.0" : "Olm 3.2.0"; + }, } as unknown as CryptoApi); await analytics.updateAnonymityFromSettings(mockClient, pseudonymous); } @@ -315,36 +319,16 @@ describe("PosthogAnalytics", () => { it("should send rust cryptoSDK superProperty correctly", async () => { analytics.setAnonymity(Anonymity.Pseudonymous); - await SettingsStore.setValue("feature_rust_crypto", null, SettingLevel.DEVICE, true); - defaultDispatcher.dispatch( - { - action: Action.SettingUpdated, - settingName: "feature_rust_crypto", - }, - true, - ); + await simulateLogin(false); - // Has for now you need to logout and login again to make the switch, until then it sbould stay as Legacy expect(mocked(fakePosthog).register.mock.lastCall![0]["cryptoSDK"]).toStrictEqual("Legacy"); - // Simulate a logout / login with a rust crypto backend - await simulateLoginWorkaroundUntilMigration(); - - // Super Properties are properties associated with events that are set once and then sent with every capture call. - // They are set using posthog.register - expect(mocked(fakePosthog).register.mock.lastCall![0]["cryptoSDK"]).toStrictEqual("Rust"); }); it("should send Legacy cryptoSDK superProperty correctly", async () => { analytics.setAnonymity(Anonymity.Pseudonymous); - await SettingsStore.setValue("feature_rust_crypto", null, SettingLevel.DEVICE, false); - defaultDispatcher.dispatch( - { - action: Action.SettingUpdated, - settingName: "feature_rust_crypto", - }, - true, - ); + await simulateLogin(false); + // Super Properties are properties associated with events that are set once and then sent with every capture call. // They are set using posthog.register expect(mocked(fakePosthog).register.mock.lastCall![0]["cryptoSDK"]).toStrictEqual("Legacy"); @@ -353,19 +337,10 @@ describe("PosthogAnalytics", () => { it("should send cryptoSDK superProperty when enabling analytics", async () => { analytics.setAnonymity(Anonymity.Disabled); - await SettingsStore.setValue("feature_rust_crypto", null, SettingLevel.DEVICE, true); - - defaultDispatcher.dispatch( - { - action: Action.SettingUpdated, - settingName: "feature_rust_crypto", - }, - true, - ); - - await simulateLoginWorkaroundUntilMigration(false); + await simulateLogin(true, false); // This initial call is due to the call to register platformSuperProperties + // The important thing is that the cryptoSDK superProperty is not set. expect(mocked(fakePosthog).register.mock.lastCall![0]).toStrictEqual({}); // switching to pseudonymous should ensure that the cryptoSDK superProperty is set correctly