Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extend stats summary with call device and user count based on room state #3424

Merged
merged 56 commits into from
Jun 7, 2023

Conversation

toger5
Copy link
Contributor

@toger5 toger5 commented May 30, 2023

Send the expected peer connections based on roomState event in the SummaryReport.

Checklist

  • Tests written for new code (and old code if feasible)
  • Linter and other CI checks pass
  • Sign-off given on the changes (see CONTRIBUTING.md)

Signed-off-by: Timo Kandra toger5@hotmail.de


Here's what your changelog entry will look like:

✨ Features

  • Extend stats summary with call device and user count based on room state (#3424). Contributed by @toger5.

roomStateExpectedPeerConnections: number;
missingPeerConnections: number;
percentageEstablishedPeerConnections: 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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the names are interpretations of the actual statistics. I would name the values according to their real meaning and do the interpretation in Posthog. And I would also add the joined users in the room. Because it is unusual for a user to take part in a conference with several devices. If it happens too often, we can use this to identify a caching issues.

usersInRoom: number;
devicesInRoom: number;
diffDevicesToPeerConnections: number;
ratioPeerConnectionToDevices : number;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To make the devices more comparable with the peerConnections i only count the devices without the "local" device. I changed it to oppDevicesInCall and oppUsersInCall.

src/webrtc/groupCall.ts Outdated Show resolved Hide resolved
@t3chguy
Copy link
Member

t3chguy commented Jun 1, 2023

Having posthog in the js-sdk changelog doesn't make much sense given that js-sdk has no concept of posthog, could you reword?

@toger5 toger5 changed the title Send expected peer connections to posthog Extend stats summary with expected peer connections Jun 1, 2023
@toger5 toger5 changed the title Extend stats summary with expected peer connections Extend stats summary with call device and user count based on room state Jun 1, 2023
Signed-off-by: Timo K <toger5@hotmail.de>
Signed-off-by: Timo K <toger5@hotmail.de>
Signed-off-by: Timo K <toger5@hotmail.de>
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 <toger5@hotmail.de>
src/webrtc/stats/groupCallStats.ts Outdated Show resolved Hide resolved
src/webrtc/stats/statsReport.ts Outdated Show resolved Hide resolved
src/webrtc/stats/summaryStatsReporter.ts Outdated Show resolved Hide resolved
RiotRobot and others added 12 commits June 7, 2023 11:51
Co-authored-by: Michael Weimann <michaelw@matrix.org>
* 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>
Signed-off-by: Timo K <toger5@hotmail.de>
Signed-off-by: Timo K <toger5@hotmail.de>
@toger5 toger5 requested a review from a team as a code owner June 7, 2023 10:12
@toger5 toger5 requested a review from justjanne June 7, 2023 10:12
@toger5 toger5 requested review from robintown and removed request for justjanne June 7, 2023 10:22
@robintown
Copy link
Member

I don't think a review from the app team should be necessary here, it looks like something went wrong when you merged develop that has made GitHub think you've touched package.json

@toger5 toger5 added this pull request to the merge queue Jun 7, 2023
@robintown robintown removed the request for review from a team June 7, 2023 13:44
Merged via the queue into develop with commit 60c715d Jun 7, 2023
@toger5 toger5 deleted the toger5/splitbrainIndicatorRoomState branch June 7, 2023 13:49
su-ex added a commit to SchildiChat/matrix-js-sdk that referenced this pull request Feb 22, 2024
* Introduce a new `Crypto.Verifier` interface, and deprecate direct access to `VerificationBase`, `SAS` and `ReciprocateQRCode` ([\matrix-org#3414](matrix-org#3414)).
* Add `rust-crypto#isCrossSigningReady` implementation ([\matrix-org#3462](matrix-org#3462)). Contributed by @florianduros.
* OIDC: Validate `m.authentication` configuration ([\matrix-org#3419](matrix-org#3419)). Contributed by @kerryarchibald.
* ElementR: Add `CryptoApi.getCrossSigningStatus` ([\matrix-org#3452](matrix-org#3452)). Contributed by @florianduros.
* Extend stats summary with call device and user count based on room state ([\matrix-org#3424](matrix-org#3424)). Contributed by @toger5.
* Update MSC3912 implementation to use `with_rel_type` instead of `with_relations` ([\matrix-org#3420](matrix-org#3420)).
* Export thread-related types from SDK ([\matrix-org#3447](matrix-org#3447)). Contributed by @stas-demydiuk.
* Use correct /v3 prefix for /refresh ([\matrix-org#3016](matrix-org#3016)). Contributed by @davidisaaclee.
* Fix thread list being ordered based on all updates ([\matrix-org#3458](matrix-org#3458)). Fixes element-hq/element-web#25522.
* Fix: handle `baseUrl` with trailing slash in `fetch.getUrl` ([\matrix-org#3455](matrix-org#3455)). Fixes element-hq/element-web#25526. Contributed by @kerryarchibald.
* use cli.canSupport to determine intentional mentions support ([\matrix-org#3445](matrix-org#3445)). Fixes element-hq/element-web#25497. Contributed by @kerryarchibald.
* Make sliding sync linearize processing of sync requests ([\matrix-org#3442](matrix-org#3442)).
* Fix edge cases around 2nd order relations and threads ([\matrix-org#3437](matrix-org#3437)).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants