From 60c715d5dfdfd67d40b39899499683abcf49e3f1 Mon Sep 17 00:00:00 2001 From: Timo <16718859+toger5@users.noreply.github.com> Date: Wed, 7 Jun 2023 15:36:40 +0200 Subject: [PATCH] Extend stats summary with call device and user count based on room state (#3424) * send expected peer connections to posthog. (based on roomState event) * add tests * change GroupCallStats initialized * prettier * more test and catch for promise * seperate the participant logic in a summary extend function Signed-off-by: Timo K * remove unused Signed-off-by: Timo K * rename summaryStatsReportGatherer to "Reporter" for the summary stats there is only one instance because there is only one summary. Since we dont have a list of gatherers it this class only reports. Hence we rename it to be a reporter. Signed-off-by: Timo K * review Signed-off-by: Timo K * Update src/webrtc/stats/groupCallStats.ts Co-authored-by: Robin * revert rename Signed-off-by: Timo K * Update all non-major dependencies (#3433) * Update all non-major dependencies * Remove name wrap-ansi-cjs * Remove name string-width-cjs --------- Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: Michael Telatynski <7t3chguy@gmail.com> * Update definitelyTyped (#3430) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: Michael Telatynski <7t3chguy@gmail.com> * Export FALLBACK_ICE_SERVER (#3429) * Add an integration test for verification (#3436) * Move existing crypto integ tests into a subdirectory * Factor out some common bits from `crypto.spec.ts` * Integration test for device verification * Ignore generated file in prettier * Always show a summary after Jest tests (#3440) ... because it is otherwise impossible to see what failed. * Use correct /v3 prefix for /refresh (#3016) * Add tests to ensure /v3/refresh is called + automatic /v1 retry * Request /refresh with v3 prefix, and quietly fall back to v1 * Add tests checking re-raising errors * Update spec/unit/login.spec.ts * Update comment Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --------- Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> * Update Mutual Rooms (MSC2666) support (#3381) * update mutual rooms support * clarify docs and switch eslint comment with todo * please the holy linter * change query variable names around * add mock tests and fix issue * ye holy linter * GHA: build and cypress-test a copy of element-web after each push (#3412) * Build a copy of element-web after each push * Run cypress after each build of element-web * Fix downstream-artifacts build (#3443) * Fix downstream-artifacts build * Update cypress.yml * Fix edge cases around 2nd order relations and threads (#3437) * Fix tests oversimplifying threads fixtures * Check for unsigned thread_id in MatrixEvent::threadRootId * Fix threads order being racy * Make Sonar happier * Iterate * Make sliding sync linearize processing of sync requests (#3442) * Make sliding sync linearize processing of sync requests * Iterate * Iterate * Iterate * Iterate * Disable downstream artifacts build for develop branch (#3444) * Export thread-related types from SDK (#3447) * Export thread-related types from SDK * address PR feedback * Integration test for QR code verification (#3439) * Integration test for QR code verification Followup to https://github.com/matrix-org/matrix-js-sdk/pull/3436: another integration test, this time using the QR code flow * Use Object.defineProperty, and restore afterwards Apparently global.crypto exists in some environments * apply ts-ignore * remove stray comment * Update spec/integ/crypto/verification.spec.ts * Add `getShowSasCallbacks`, `getShowQrCodeCallbacks` to VerifierBase (#3422) * Add `getShowSasCallbacks`, `getShowQrCodeCallbacks` to VerifierBase ... to avoid some type-casting * Integration test for QR code verification Followup to https://github.com/matrix-org/matrix-js-sdk/pull/3436: another integration test, this time using the QR code flow * Rename method ... it turns out not to be used quite as I thought. * tests for new methods * Use Object.defineProperty, and restore afterwards Apparently global.crypto exists in some environments * apply ts-ignore * More test coverage * fix bad merge * Fix changelog_head.py script to be Python 3 compatible * Prepare changelog for v25.2.0-rc.1 * v25.2.0-rc.1 * Fix tsconfig-build.json * Prepare changelog for v25.2.0-rc.2 * v25.2.0-rc.2 * Fix docs deployment * Prepare changelog for v25.2.0-rc.3 * v25.2.0-rc.3 * Prepare changelog for v25.2.0-rc.4 * v25.2.0-rc.4 * [Backport staging] Attempt a potential workaround for stuck notifs (#3387) Co-authored-by: Andy Balaam * Prepare changelog for v25.2.0-rc.5 * v25.2.0-rc.5 * [Backport staging] Fix mark as unread button (#3401) Co-authored-by: Michael Weimann * Prepare changelog for v26.0.0-rc.1 * v26.0.0-rc.1 * Prepare changelog for v26.0.0 * v26.0.0 * Resetting package fields for development * use cli.canSupport to determine intentional mentions support (#3445) * use cli.canSupport to determine intentional mentions support * more specific comment * Update src/client.ts Co-authored-by: Michael Telatynski <7t3chguy@gmail.com> --------- Co-authored-by: Michael Telatynski <7t3chguy@gmail.com> * git fixup Signed-off-by: Timo K * import updates Signed-off-by: Timo K * dont revert enricos change Signed-off-by: Timo K * temp rename for lowercase * lowercase --------- Signed-off-by: Timo K Co-authored-by: Robin Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: Michael Telatynski <7t3chguy@gmail.com> Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Co-authored-by: David Lee Co-authored-by: Jonathan de Jong Co-authored-by: Stanislav Demydiuk Co-authored-by: ElementRobot Co-authored-by: Andy Balaam Co-authored-by: Michael Weimann Co-authored-by: Kerry --- spec/test-utils/webrtc.ts | 75 ++++++++++++ .../stats/summaryStatsReportGatherer.spec.ts | 113 +++++++++++++++++- src/webrtc/groupCall.ts | 10 ++ src/webrtc/stats/groupCallStats.ts | 7 +- src/webrtc/stats/statsReport.ts | 5 + .../stats/summaryStatsReportGatherer.ts | 29 ++++- 6 files changed, 236 insertions(+), 3 deletions(-) diff --git a/spec/test-utils/webrtc.ts b/spec/test-utils/webrtc.ts index 2037767cc94..d4d50b458d7 100644 --- a/spec/test-utils/webrtc.ts +++ b/spec/test-utils/webrtc.ts @@ -745,3 +745,78 @@ export const REMOTE_SFU_DESCRIPTION = "a=sctp-port:5000\n" + "a=ice-ufrag:obZwzAcRtxwuozPZ\n" + "a=ice-pwd:TWXNaPeyKTTvRLyIQhWHfHlZHJjtcoKs"; + +export const groupCallParticipantsFourOtherDevices = new Map([ + [ + new RoomMember("roomId0", "user1"), + new Map([ + [ + "deviceId0", + { + sessionId: "0", + screensharing: false, + }, + ], + [ + "deviceId1", + { + sessionId: "1", + screensharing: false, + }, + ], + [ + "deviceId2", + { + sessionId: "2", + screensharing: false, + }, + ], + ]), + ], + [ + new RoomMember("roomId0", "user2"), + new Map([ + [ + "deviceId3", + { + sessionId: "0", + screensharing: false, + }, + ], + [ + "deviceId4", + { + sessionId: "1", + screensharing: false, + }, + ], + ]), + ], +]); + +export const groupCallParticipantsOneOtherDevice = new Map([ + [ + new RoomMember("roomId1", "thisMember"), + new Map([ + [ + "deviceId0", + { + sessionId: "0", + screensharing: false, + }, + ], + ]), + ], + [ + new RoomMember("roomId1", "opponentMember"), + new Map([ + [ + "deviceId1", + { + sessionId: "1", + screensharing: false, + }, + ], + ]), + ], +]); diff --git a/spec/unit/webrtc/stats/summaryStatsReportGatherer.spec.ts b/spec/unit/webrtc/stats/summaryStatsReportGatherer.spec.ts index eefff0d9b49..aca482e219c 100644 --- a/spec/unit/webrtc/stats/summaryStatsReportGatherer.spec.ts +++ b/spec/unit/webrtc/stats/summaryStatsReportGatherer.spec.ts @@ -15,6 +15,7 @@ limitations under the License. */ import { SummaryStatsReportGatherer } from "../../../../src/webrtc/stats/summaryStatsReportGatherer"; import { StatsReportEmitter } from "../../../../src/webrtc/stats/statsReportEmitter"; +import { groupCallParticipantsFourOtherDevices } from "../../../test-utils/webrtc"; describe("SummaryStatsReportGatherer", () => { let reporter: SummaryStatsReportGatherer; @@ -584,8 +585,118 @@ describe("SummaryStatsReportGatherer", () => { percentageReceivedVideoMedia: 1, maxJitter: 2, maxPacketLoss: 40, - peerConnections: 3, + peerConnections: 4, + percentageConcealedAudio: 0, + }); + }); + it("should report missing peer connections", async () => { + const summary = [ + { + isFirstCollection: true, + receivedMedia: 1, + receivedAudioMedia: 1, + receivedVideoMedia: 1, + audioTrackSummary: { + count: 1, + muted: 0, + maxJitter: 20, + maxPacketLoss: 5, + concealedAudio: 0, + totalAudio: 0, + }, + videoTrackSummary: { + count: 1, + muted: 0, + maxJitter: 0, + maxPacketLoss: 0, + concealedAudio: 0, + totalAudio: 0, + }, + }, + { + isFirstCollection: false, + receivedMedia: 1, + receivedAudioMedia: 1, + receivedVideoMedia: 1, + audioTrackSummary: { + count: 1, + muted: 0, + maxJitter: 2, + maxPacketLoss: 5, + concealedAudio: 0, + totalAudio: 0, + }, + videoTrackSummary: { + count: 1, + muted: 0, + maxJitter: 0, + maxPacketLoss: 40, + concealedAudio: 0, + totalAudio: 0, + }, + }, + ]; + reporter.build(summary); + expect(emitter.emitSummaryStatsReport).toHaveBeenCalledWith({ + percentageReceivedMedia: 1, + percentageReceivedAudioMedia: 1, + percentageReceivedVideoMedia: 1, + maxJitter: 2, + maxPacketLoss: 40, + peerConnections: 2, + percentageConcealedAudio: 0, + }); + }); + }); + describe("extend Summary Stats Report", () => { + it("should extend the report with the appropriate data based on a user map", async () => { + const summary = { + percentageReceivedMedia: 1, + percentageReceivedAudioMedia: 1, + percentageReceivedVideoMedia: 1, + maxJitter: 2, + maxPacketLoss: 40, + peerConnections: 4, + percentageConcealedAudio: 0, + }; + SummaryStatsReportGatherer.extendSummaryReport(summary, groupCallParticipantsFourOtherDevices); + expect(summary).toStrictEqual({ + percentageReceivedMedia: 1, + percentageReceivedAudioMedia: 1, + percentageReceivedVideoMedia: 1, + maxJitter: 2, + maxPacketLoss: 40, + peerConnections: 4, + percentageConcealedAudio: 0, + opponentUsersInCall: 1, + opponentDevicesInCall: 4, + diffDevicesToPeerConnections: 0, + ratioPeerConnectionToDevices: 1, + }); + }); + it("should extend the report data based on a user map", async () => { + const summary = { + percentageReceivedMedia: 1, + percentageReceivedAudioMedia: 1, + percentageReceivedVideoMedia: 1, + maxJitter: 2, + maxPacketLoss: 40, + peerConnections: 4, + percentageConcealedAudio: 0, + }; + SummaryStatsReportGatherer.extendSummaryReport(summary, new Map()); + expect(summary).toStrictEqual({ + percentageReceivedMedia: 1, + percentageReceivedAudioMedia: 1, + percentageReceivedVideoMedia: 1, + maxJitter: 2, + maxPacketLoss: 40, + peerConnections: 4, percentageConcealedAudio: 0, + opponentUsersInCall: 0, + opponentDevicesInCall: 0, + diffDevicesToPeerConnections: -4, + ratioPeerConnectionToDevices: 0, }); }); }); diff --git a/src/webrtc/groupCall.ts b/src/webrtc/groupCall.ts index 6d98586c92b..d6f7b4f1ecf 100644 --- a/src/webrtc/groupCall.ts +++ b/src/webrtc/groupCall.ts @@ -26,6 +26,7 @@ import { IScreensharingOpts } from "./mediaHandler"; import { mapsEqual } from "../utils"; import { GroupCallStats } from "./stats/groupCallStats"; import { ByteSentStatsReport, ConnectionStatsReport, StatsReport, SummaryStatsReport } from "./stats/statsReport"; +import { SummaryStatsReportGatherer } from "./stats/summaryStatsReportGatherer"; export enum GroupCallIntent { Ring = "m.ring", @@ -98,6 +99,9 @@ export enum GroupCallStatsReportEvent { SummaryStats = "GroupCall.summary_stats", } +/** + * The final report-events that get consumed by client. + */ export type GroupCallStatsReportEventHandlerMap = { [GroupCallStatsReportEvent.ConnectionStats]: (report: GroupCallStatsReport) => void; [GroupCallStatsReportEvent.ByteSentStats]: (report: GroupCallStatsReport) => void; @@ -269,14 +273,18 @@ export class GroupCall extends TypedEventEmitter< } private onConnectionStats = (report: ConnectionStatsReport): void => { + // Final emit of the summary event, to be consumed by the client this.emit(GroupCallStatsReportEvent.ConnectionStats, { report }); }; private onByteSentStats = (report: ByteSentStatsReport): void => { + // Final emit of the summary event, to be consumed by the client this.emit(GroupCallStatsReportEvent.ByteSentStats, { report }); }; private onSummaryStats = (report: SummaryStatsReport): void => { + SummaryStatsReportGatherer.extendSummaryReport(report, this.participants); + // Final emit of the summary event, to be consumed by the client this.emit(GroupCallStatsReportEvent.SummaryStats, { report }); }; @@ -1595,6 +1603,8 @@ export class GroupCall extends TypedEventEmitter< }); if (this.state === GroupCallState.Entered) this.placeOutgoingCalls(); + + // Update the participants stored in the stats object }; private onStateChanged = (newState: GroupCallState, oldState: GroupCallState): void => { diff --git a/src/webrtc/stats/groupCallStats.ts b/src/webrtc/stats/groupCallStats.ts index 40ee9797a44..de660f9909f 100644 --- a/src/webrtc/stats/groupCallStats.ts +++ b/src/webrtc/stats/groupCallStats.ts @@ -17,6 +17,7 @@ import { CallStatsReportGatherer } from "./callStatsReportGatherer"; import { StatsReportEmitter } from "./statsReportEmitter"; import { CallStatsReportSummary } from "./callStatsReportSummary"; import { SummaryStatsReportGatherer } from "./summaryStatsReportGatherer"; +import { logger } from "../../logger"; export class GroupCallStats { private timer: undefined | ReturnType; @@ -75,7 +76,11 @@ export class GroupCallStats { summary.push(c.processStats(this.groupCallId, this.userId)); }); - Promise.all(summary).then((s: Awaited[]) => this.summaryStatsReportGatherer.build(s)); + Promise.all(summary) + .then((s: Awaited[]) => this.summaryStatsReportGatherer.build(s)) + .catch((err) => { + logger.error("Could not build summary stats report", err); + }); } public setInterval(interval: number): void { diff --git a/src/webrtc/stats/statsReport.ts b/src/webrtc/stats/statsReport.ts index 750c26051bb..737e8db11f5 100644 --- a/src/webrtc/stats/statsReport.ts +++ b/src/webrtc/stats/statsReport.ts @@ -83,4 +83,9 @@ export interface SummaryStatsReport { maxPacketLoss: number; percentageConcealedAudio: number; peerConnections: number; + opponentUsersInCall?: number; + opponentDevicesInCall?: number; + diffDevicesToPeerConnections?: number; + ratioPeerConnectionToDevices?: number; + // Todo: Decide if we want an index (or a timestamp) of this report in relation to the group call, to help differenciate when issues occur and ignore/track initial connection delays. } diff --git a/src/webrtc/stats/summaryStatsReportGatherer.ts b/src/webrtc/stats/summaryStatsReportGatherer.ts index 87601dcac7a..b0227670d98 100644 --- a/src/webrtc/stats/summaryStatsReportGatherer.ts +++ b/src/webrtc/stats/summaryStatsReportGatherer.ts @@ -13,6 +13,8 @@ limitations under the License. import { StatsReportEmitter } from "./statsReportEmitter"; import { CallStatsReportSummary } from "./callStatsReportSummary"; import { SummaryStatsReport } from "./statsReport"; +import { ParticipantState } from "../groupCall"; +import { RoomMember } from "../../matrix"; interface CallStatsReportSummaryCounter { receivedAudio: number; @@ -31,9 +33,12 @@ export class SummaryStatsReportGatherer { // webrtcStats as basement all the calculation are 0. We don't want track the 0 stats. const summary = allSummary.filter((s) => !s.isFirstCollection); const summaryTotalCount = summary.length; + // For counting the peer connections we also want to consider the ignored summaries + const peerConnectionsCount = allSummary.length; if (summaryTotalCount === 0) { return; } + const summaryCounter: CallStatsReportSummaryCounter = { receivedAudio: 0, receivedVideo: 0, @@ -65,11 +70,33 @@ export class SummaryStatsReportGatherer { ? (summaryCounter.concealedAudio / summaryCounter.totalAudio).toFixed(decimalPlaces) : 0, ), - peerConnections: summaryTotalCount, + peerConnections: peerConnectionsCount, } as SummaryStatsReport; this.emitter.emitSummaryStatsReport(report); } + public static extendSummaryReport( + report: SummaryStatsReport, + callParticipants: Map>, + ): void { + // Calculate the actual number of devices based on the participants state event + // (this is used, to compare the expected participant count from the room state with the acutal peer connections) + // const devices = callParticipants.() + const devices: [string, ParticipantState][] = []; + const users: [RoomMember, Map][] = []; + for (const userEntry of callParticipants) { + users.push(userEntry); + for (const device of userEntry[1]) { + devices.push(device); + } + } + report.opponentDevicesInCall = Math.max(0, devices.length - 1); + report.opponentUsersInCall = Math.max(0, users.length - 1); + report.diffDevicesToPeerConnections = Math.max(0, devices.length - 1) - report.peerConnections; + report.ratioPeerConnectionToDevices = + Math.max(0, devices.length - 1) == 0 ? 0 : report.peerConnections / (devices.length - 1); + } + private countTrackListReceivedMedia(counter: CallStatsReportSummaryCounter, stats: CallStatsReportSummary): void { let hasReceivedAudio = false; let hasReceivedVideo = false;