From 0dcaa90650f8096a727fe80d861b194350f2a6d2 Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 5 Apr 2023 13:06:55 +0100 Subject: [PATCH 1/3] Fix exception when loading PostHog PostHog was expecting the matrix client object to be initialised at the point it ran its setup, which wasn't the case. Check to see if it's there on login and add an onLoginStatusChanged hook that to re-check. Also make a few methods private that didn't need to be public. Also fix a few instances where the OpenTelemetry group call tried to report metrics using a tracer which didn't exist anymore, if the user disabled analytics and then joined the same call again. --- src/ClientContext.tsx | 2 ++ src/analytics/PosthogAnalytics.ts | 17 +++++++++++++---- src/otel/OTelGroupCallMembership.ts | 10 +++++++++- 3 files changed, 24 insertions(+), 5 deletions(-) diff --git a/src/ClientContext.tsx b/src/ClientContext.tsx index 46dfe5a5..6a7d1753 100644 --- a/src/ClientContext.tsx +++ b/src/ClientContext.tsx @@ -342,6 +342,8 @@ export const ClientProvider: FC = ({ children }) => { useEffect(() => { window.matrixclient = client; window.isPasswordlessUser = isPasswordlessUser; + + PosthogAnalytics.instance.onLoginStatusChanged(); }, [client, isPasswordlessUser]); if (error) { diff --git a/src/analytics/PosthogAnalytics.ts b/src/analytics/PosthogAnalytics.ts index e2e8fdae..0f8ba38d 100644 --- a/src/analytics/PosthogAnalytics.ts +++ b/src/analytics/PosthogAnalytics.ts @@ -227,7 +227,7 @@ export class PosthogAnalytics { .join(""); } - public async identifyUser(analyticsIdGenerator: () => string) { + private async identifyUser(analyticsIdGenerator: () => string) { if (this.anonymity == Anonymity.Pseudonymous && this.enabled) { // 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. @@ -319,7 +319,12 @@ export class PosthogAnalytics { this.setAnonymity(Anonymity.Disabled); } - public updateSuperProperties() { + public onLoginStatusChanged(): void { + const optInAnalytics = getSetting("opt-in-analytics", false); + this.updateAnonymityAndIdentifyUser(optInAnalytics); + } + + private updateSuperProperties() { // Update super properties in posthog with our platform (app version, platform). // These properties will be subsequently passed in every event. // @@ -339,7 +344,7 @@ export class PosthogAnalytics { return this.eventSignup.getSignupEndTime() > new Date(0); } - public async updateAnonymityAndIdentifyUser( + private async updateAnonymityAndIdentifyUser( pseudonymousOptIn: boolean ): Promise { // Update this.anonymity based on the user's analytics opt-in settings @@ -348,6 +353,10 @@ export class PosthogAnalytics { : Anonymity.Disabled; this.setAnonymity(anonymity); + // We may not yet have a Matrix client at this point, if not, bail. This should get + // triggered again by onLoginStatusChanged once we do have a client. + if (!window.matrixclient) return; + if (anonymity === Anonymity.Pseudonymous) { this.setRegistrationType( window.matrixclient.isGuest() || window.isPasswordlessUser @@ -385,7 +394,7 @@ export class PosthogAnalytics { this.capture(eventName, properties, options); } - public startListeningToSettingsChanges(): void { + private startListeningToSettingsChanges(): void { // Listen to account data changes from sync so we can observe changes to relevant flags and update. // This is called - // * On page load, when the account data is first received by sync diff --git a/src/otel/OTelGroupCallMembership.ts b/src/otel/OTelGroupCallMembership.ts index 8aa24c74..14c0f98d 100644 --- a/src/otel/OTelGroupCallMembership.ts +++ b/src/otel/OTelGroupCallMembership.ts @@ -124,6 +124,8 @@ export class OTelGroupCallMembership { } public onJoinCall() { + if (!ElementCallOpenTelemetry.instance) return; + // Create the main span that tracks the time we intend to be in the call this.callMembershipSpan = ElementCallOpenTelemetry.instance.tracer.startSpan( @@ -174,7 +176,7 @@ export class OTelGroupCallMembership { for (const [userId, userCalls] of calls.entries()) { for (const [deviceId, call] of userCalls.entries()) { if (!this.callsByCallId.has(call.callId)) { - const span = ElementCallOpenTelemetry.instance.tracer.startSpan( + const span = ElementCallOpenTelemetry.instance?.tracer.startSpan( `matrix.call`, undefined, this.groupCallContext @@ -321,6 +323,8 @@ export class OTelGroupCallMembership { public onConnectionStatsReport( statsReport: GroupCallStatsReport ) { + if (!ElementCallOpenTelemetry.instance) return; + const type = OTelStatsReportType.ConnectionReport; const data = ObjectFlattener.flattenConnectionStatsReportObject(statsReport); @@ -330,6 +334,8 @@ export class OTelGroupCallMembership { public onByteSentStatsReport( statsReport: GroupCallStatsReport ) { + if (!ElementCallOpenTelemetry.instance) return; + const type = OTelStatsReportType.ByteSentReport; const data = ObjectFlattener.flattenByteSentStatsReportObject(statsReport); this.buildStatsEventSpan({ type, data }); @@ -338,6 +344,8 @@ export class OTelGroupCallMembership { public onSummaryStatsReport( statsReport: GroupCallStatsReport ) { + if (!ElementCallOpenTelemetry.instance) return; + const type = OTelStatsReportType.SummaryReport; const data = ObjectFlattener.flattenSummaryStatsReportObject(statsReport); this.buildStatsEventSpan({ type, data }); From 5e4aa53997b0edc9034c331fdf81b0463eee1410 Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 5 Apr 2023 15:00:14 +0100 Subject: [PATCH 2/3] Don't call posthog before its initialised --- src/ClientContext.tsx | 3 ++- src/analytics/PosthogAnalytics.ts | 4 ++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/ClientContext.tsx b/src/ClientContext.tsx index 6a7d1753..08593a4b 100644 --- a/src/ClientContext.tsx +++ b/src/ClientContext.tsx @@ -343,7 +343,8 @@ export const ClientProvider: FC = ({ children }) => { window.matrixclient = client; window.isPasswordlessUser = isPasswordlessUser; - PosthogAnalytics.instance.onLoginStatusChanged(); + if (PosthogAnalytics.hasInstance()) + PosthogAnalytics.instance.onLoginStatusChanged(); }, [client, isPasswordlessUser]); if (error) { diff --git a/src/analytics/PosthogAnalytics.ts b/src/analytics/PosthogAnalytics.ts index 0f8ba38d..ed8ada35 100644 --- a/src/analytics/PosthogAnalytics.ts +++ b/src/analytics/PosthogAnalytics.ts @@ -102,6 +102,10 @@ export class PosthogAnalytics { private platformSuperProperties = {}; private registrationType: RegistrationType = RegistrationType.Guest; + public static hasInstance(): boolean { + return Boolean(this.internalInstance); + } + public static get instance(): PosthogAnalytics { if (!this.internalInstance) { this.internalInstance = new PosthogAnalytics(posthog); From fec299ab20ae381ecba05da0e7aa9dfb402910bb Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 5 Apr 2023 15:11:51 +0100 Subject: [PATCH 3/3] Skip whole block if no otel instance --- src/otel/OTelGroupCallMembership.ts | 32 +++++++++++++++-------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/src/otel/OTelGroupCallMembership.ts b/src/otel/OTelGroupCallMembership.ts index 14c0f98d..c8fce617 100644 --- a/src/otel/OTelGroupCallMembership.ts +++ b/src/otel/OTelGroupCallMembership.ts @@ -176,21 +176,23 @@ export class OTelGroupCallMembership { for (const [userId, userCalls] of calls.entries()) { for (const [deviceId, call] of userCalls.entries()) { if (!this.callsByCallId.has(call.callId)) { - const span = ElementCallOpenTelemetry.instance?.tracer.startSpan( - `matrix.call`, - undefined, - this.groupCallContext - ); - // XXX: anonymity - span.setAttribute("matrix.call.target.userId", userId); - span.setAttribute("matrix.call.target.deviceId", deviceId); - const displayName = - this.groupCall.room.getMember(userId)?.name ?? "unknown"; - span.setAttribute("matrix.call.target.displayName", displayName); - this.callsByCallId.set( - call.callId, - new OTelCall(userId, deviceId, call, span) - ); + if (ElementCallOpenTelemetry.instance) { + const span = ElementCallOpenTelemetry.instance.tracer.startSpan( + `matrix.call`, + undefined, + this.groupCallContext + ); + // XXX: anonymity + span.setAttribute("matrix.call.target.userId", userId); + span.setAttribute("matrix.call.target.deviceId", deviceId); + const displayName = + this.groupCall.room.getMember(userId)?.name ?? "unknown"; + span.setAttribute("matrix.call.target.displayName", displayName); + this.callsByCallId.set( + call.callId, + new OTelCall(userId, deviceId, call, span) + ); + } } } }