Add collaborative session statistics #103

acicovic · 2025-11-05T19:04:46Z

Summary

This PR implements telemetry infrastructure to gather and log statistics for multi-user real-time collaborative editing sessions.

Key Changes

New Telemetry Infrastructure:

Implements TelemetryLogger base class providing a standard interface for logging telemetry data to multiple destinations (Logger, Pendo)
Implements modular telemetry system under src/telemetry/ for extensibility

Session Statistics Tracking:

Implements SessionStats class to manage multi-user session data using Yjs shared state
Implements session-specific logger (SessionStatsTelemetryLogger) that gathers multi-user session statistics
Sessions are only tracked when 2+ users are present, and logged when 1 user remains

Session Activity Management:

Implements SessionActivityManager to coordinate session lifecycle events
Monitors user joins/leaves and transitions between single/multi-user states
Detects document changes and marks locally active users
Implements UserInactivityTimer with 30-minute inactivity timeout (no document changes) to detect idle sessions and trigger session logging
Uses debouncing and throttling to avoid redundant updates

Statistics Collected:

Post ID
Count of maximum users during the session
Count of active users (performed edits) vs. inactive users (view-only)
Session duration and activity timestamps

Integration:

Integrates SessionActivityManager initialization into AwarenessManager lifecycle
Statistics stored in Yjs document under session-stats origin map
Provides utilities for checking user connections and exporting session stats

Technical Details

Data Integrity and Safeguards:

Transaction-based updates to prevent race conditions
Graceful degradation when telemetry data cannot be prepared
Prevents duplicate initialization and handles reconnection edge cases
Prevents race conditions when logging stats, with checks on user activity and session state
Uses grace periods and validation to avoid duplicate or incomplete logs during quick user connect/disconnect cycles
Uses leader election pattern to prevent duplicate logging from multiple clients
Elects a fallback leader when the current leader disconnects

Testing

Basic functionality tested across the latest version of Brave, Vivaldi, Safari and Firefox.
To test this, simulate a multi-user session locally. You will see output in the console when 2+ users join (session initialization) and when only 1 user remains in the session (final session stats).

Summary of all tested scenarios:

Single user sessions (no logging occurs)
Leader logs when non-leaders leave
Non-leader takes over and logs when leader leaves
Inactivity timeout triggers logging with correct stats
Activity resumption starts new session
Multiple inactivity/activity cycles log correctly
Multiple tabs per user handled correctly
Computer sleep/wake cycles
Short disconnects and reconnections
Browser back/forward navigation

Future Work

Pendo integration for telemetry logging will be implemented in a follow-up PR. This will involve creating a WordPress REST API endpoint that receives the telemetry data and forwards it to the VIP Telemetry class.

ingeniumed · 2025-11-06T04:14:52Z

src/telemetry/session-stats/SessionActivityController.ts

+			}
+		}
+
+		if ( ! awareness?.doc ) {


Should this be checked right away instead?

ingeniumed · 2025-11-06T04:19:21Z

src/telemetry/session-stats/SessionActivityController.ts

+ * activity, responds to user and document changes, handles inactivity, logs
+ * session data.
+ */
+export class SessionActivityController {


Nitpick: Feel like calling it a manager is better imo rather than a controller. A controller to me implies rest endpoints that'll be actively called.

Unless there's work planned in a follow up PR to add that in? I assumed the follow up work was to add it on the PHP side.

ingeniumed · 2025-11-06T04:29:01Z

src/telemetry/session-stats/SessionActivityController.ts

+	public static initialize( awareness: Awareness ): void {
+		if ( SessionActivityController.__instance ) {
+			if ( SessionActivityController.__instance.awareness?.doc !== awareness.doc ) {
+				SessionActivityController.destroySingletonInstance();


What would an example be of this being called? Awareness Manager is initialized once per client, and then errors subsequently.

This feels like a singleton anti-pattern, if we are re-creating the entire singleton instance when the awareness doc changes? Might be a naming thing, but I wouldn't expect a singleton to be destroyed like this if we are using this pattern unless absolutely necessary?

ingeniumed · 2025-11-06T04:33:42Z

src/telemetry/session-stats/SessionActivityManager.ts

+			() => this.handleInactivityTimeout()
+		);
+
+		this.subscribeToUserChanges();


Looking at this, I wonder if the class could be re-done such that:

Instantiate it within the constructor of the awareness manager, so their lifecycles match. This means a single clientID at a time would have this class instantiated.

Within the existing subscriptions, call in the methods on this class that require those subscriptions. That way, there's one single grouped awareness subscription for the awareness changes to be transmitted to. That was a performance optimization that had been mentioned on some forum posts (don't have the link on hand but I remember running into that as an improvement as more clients join).

ingeniumed · 2025-11-06T04:41:56Z

src/telemetry/session-stats/SessionActivityController.ts

+
+				try {
+					const { getActiveUsers } = select( awarenessStore );
+					const userId = getActiveUsers().get( clientId )?.userInfo?.id ?? null;


You can also look for the isMe property, and that'll do the trick.

I've added an isMe validation check to the user lookup. I kept the direct clientID map access for O(1) performance (this runs on every local document change), but now we validate with isMe before proceeding.

ingeniumed · 2025-11-06T04:44:22Z

src/telemetry/session-stats/SessionStats.ts

+	 * @param logger The Logger instance to use for logging
+	 */
+	constructor( private awareness: Awareness, private logger: Logger ) {
+		this.sessionMap = this.awareness.doc.getMap( SESSION_STATS_ORIGIN );


Would check awareness?.doc before accessing the getMap on it out of safety 🤔

awareness.doc is technically guaranteed by the Awareness type definition (it's a required constructor parameter and non-optional property). Although I'm unsure where this could go wrong, I've added a defensive check.

From what I can see, we generally use awareness.doc across the codebase with the only exception being convertRelativePositionToAbsolutePosition() which can be called externally.

ingeniumed · 2025-11-06T04:46:43Z

src/telemetry/session-stats/SessionStats.ts

+	/**
+	 * Returns the current post ID.
+	 */
+	private getCurrentPostId(): PostId {


Why do we wrap an existing selector offered by Gutenberg?

This wrapper is used in exportStats(), allowing partial success. If getCurrentPostId() fails, we still log the session with postId: null rather than losing all telemetry data. Without it, a selector error would trigger the outer catch and discard the entire export.

ingeniumed · 2025-11-06T04:50:20Z

src/telemetry/session-stats/SessionStats.ts

+ * Manages multi-user session statistics using Yjs Awareness for shared state.
+ */
+export class SessionStats {
+	private sessionMap: Y.Map< unknown >;


This is different than how we are using other maps that we are setting on the doc.

Usually, what we do this within a function rather than at the class level:

Use observe or observeDeep to monitor for changes, if that's relevant

Use transact to make changes to a map on the doc itself

Store the doc in memory like in Manager, and access what's needed on that

Could this way have any issues related to state?

I don't think we could have an issue here. doc.getMap() is idempotent and returns a live reference, not a snapshot. Storing it should be safe and avoids repeated lookups. To put this in more words:

doc.getMap('key') returns the same instance every time

The map reference remains valid for the lifetime of the doc

Changes to the map are reflected immediately in the stored reference

The map is the actual shared state object, not a snapshot

Let me know if you think I could be missing something.

ingeniumed · 2025-11-06T04:55:11Z

src/telemetry/session-stats/utils.ts

+ *
+ * @returns Array of unique connected user IDs
+ */
+export function getConnectedUserIds(): number[] {


Would it be worth adding methods that you think are useful, on the awareness store itself so they could be re-used elsewhere?

In the context of this PR, I've tried to touch "outside files" the least amount possible. If any of these methods become useful for other parts of the code, we could definitely move them where it makes more sense. I don't know if this holds true currently.

ingeniumed · 2025-11-06T04:56:23Z

src/awareness-manager.ts

 		}

 		AwarenessManager.__instance = new AwarenessManager( awareness, await getCurrentUserInfo() );
+		SessionActivityController.initialize( awareness );


This feels like an odd place to initialize it imo. I feel like it should be in the awareness manager constructor in some way? This is assuming the other comment I made about changing how SessionActivityController is initialized.

acicovic · 2025-11-06T18:54:31Z

@ingeniumed, thank you for your review.

I've addressed some of your feedback in 3505352 and left comments in the remaining items. Let's keep the discussion going where needed, and let me know if the changes made look good to you or if you have more feedback.

Thanks!

chriszarate · 2025-11-06T19:57:28Z

@acicovic This PR is pretty big!

Statistics stored in Yjs document under session-stats origin map

Could you explain why storing the stats in a Yjs doc is useful / necessary? If it's to aggregate stats among peers, could we instead get these answers using aggregation functions when querying the "snapshot-style" events?

acicovic · 2025-11-07T08:29:18Z

@acicovic This PR is pretty big!

Yeah, I'm sorry about this.

Could you explain why storing the stats in a Yjs doc is useful / necessary?

If you have any particular concern about this implementation, let me know so I can look into it.

The Yjs doc enables us to log one accurate, fully-aggregated event per session.

Shared session state: All clients need to track the same user list (allUserIds, activeUserIds) and timing data (sessionTimeStart, sessionTimeLastActivity) as users join/leave and make edits. Without this shared state, each client would only see its own partial view.
Leader election: The sessionInitializerClientId stored in the Yjs doc enables leader election so exactly one client logs the final aggregated stats when the session ends. Otherwise, multiple clients would each log conflicting "session ended" events.
Consistent inactivity detection: All clients check the shared sessionTimeLastActivity to detect the 30-minute timeout. Without shared state, clients would have different timeout values and log prematurely.
Edge case handling: Clients joining mid-session need to see who was there before them. The shared Yjs doc makes this possible.

Potential issues with snapshot events + aggregation:

No way to coordinate which client should log the final event
Race conditions from multiple clients sending conflicting session-end events
Missing data (users who left before you joined)
Reconciliation logic required

…ion expiration handling

acicovic · 2025-12-01T06:38:57Z

Hi, I've pushed some additional commits to this PR. I'm not planning any additional changes, so this is ready for review.

chriszarate · 2025-12-01T15:11:01Z

src/websocket-client.ts

+				// Wait for initial sync before initializing awareness.
+				await new Promise< void >( resolve => {
+					provider.once( 'sync', ( isSynced: boolean ) => {
+						if ( isSynced ) {
+							resolve();
+						}
+					} );
+				} );


Why do we need to await this sync event firing?

This feels like a potential bug. What if the sync event doesn't fire?

chriszarate · 2025-12-01T15:30:11Z

Potential issues with snapshot events + aggregation:

No way to coordinate which client should log the final event

Race conditions from multiple clients sending conflicting session-end events

Missing data (users who left before you joined)

Reconciliation logic required

I don't know what we are trying to measure, but I can't help thinking that this client-side approach is not a good conceptual match for tracking session length. Wouldn't it be much simpler to dispatch these events from the websocket server, which can easily track connected users and "inactivity" based on received messages?

I am very wary of storing statistics in the Yjs doc, since this can only grow and become more complex. In addition, the dependence between the maps risks an infinite loop.

chriszarate · 2025-12-01T23:35:58Z

src/telemetry/session-stats/SessionActivityManager.ts

+		}
+
+		if ( this.sessionStatsObserver ) {
+			const sessionStatsMap = this.awareness.doc.getMap( SESSION_STATS_ORIGIN );


This isn't an origin

chriszarate · 2025-12-01T23:36:25Z

src/telemetry/session-stats/SessionActivityManager.ts

+		// Maps that should not contain any content changes.
+		const excludedMaps = new Set( [


This feels likely to lead to future bugs

acicovic · 2025-12-02T18:51:32Z

Wouldn't it be much simpler to dispatch these events from the websocket server, which can easily track connected users and "inactivity" based on received messages?

I agree that this sounds like a simpler approach. The advantage of the approach proposed in this PR is that since we're hooking into the RTC session itself, it gives us the ability to measure much more, which is something that might come up.

I am very wary of storing statistics in the Yjs doc, since this can only grow and become more complex. In addition, the dependence between the maps risks an infinite loop.

I'll be looking to understand the infinite loop possibility. About the first part, are there any viable alternatives you could suggest? For example, could we have another document? I don't see this working well with REST API endpoints writing the stats to the WP database. Also the stats data we're storing is both very small and simple, though I'm not familiar with the Yjs implications.

chriszarate · 2025-12-02T19:39:34Z

it gives us the ability to measure much more, which is something that might come up

I've mentioned the value of working backwards from the questions we need to answer, so that we can define what needs to be measured. Potentially over-engineering to maximize flexibility can lead to unwanted complexity.

About the first part, are there any viable alternatives you could suggest?

I would suggest that telemetry events be shipped by the websocket server, which has access to the clients and document, and controls the lifecycle of the session. I haven't verified this but it should also have access to the user info via the awareness document.

I don't see this working well with REST API endpoints writing the stats to the WP database.

Not sure I understand this.

acicovic · 2025-12-02T19:43:34Z

I don't see this working well with REST API endpoints writing the stats to the WP database.
Not sure I understand this.

I was alluding to writing the stats into the database instead of the Yjs doc since you raised that concern.

Add Collaborative Session Statistics

448bfca

acicovic self-assigned this Nov 5, 2025

acicovic marked this pull request as ready for review November 5, 2025 19:06

acicovic requested a review from a team as a code owner November 5, 2025 19:06

acicovic requested review from chriszarate and removed request for a team November 5, 2025 19:06

ingeniumed reviewed Nov 6, 2025

View reviewed changes

Address review feedback

3505352

acicovic added 13 commits November 18, 2025 09:28

Merge branch 'trunk' into add/colaborative-session-statistics

6631054

getTelemetryData(): Remove invalid session filtering

afb67d2

Session stats: Simplify exported stats

f68f2cd

Session stats: Calculate session duration based on last activity

3f9bf84

Session stats: Improve content change detection

44a12ba

Session stats: Rename usersMax to usersTotal

dca4153

Remove rapid re-initialization check

271879e

Fix multiple clients initializing sessions

936810d

Session stats: Remove flaky race condition detection and enhance sess…

25e1ab5

…ion expiration handling

Mark a user as active when they reactivate a session

e536954

Session stats: Add some informational logger messages

3569cdf

Fix linting issues

c5bda2b

Merge branch 'trunk' into add/colaborative-session-statistics

b716748

acicovic mentioned this pull request Nov 28, 2025

Add collaborative session statistics unit tests #114

Open

chriszarate reviewed Dec 1, 2025

View reviewed changes

		// Maps that should not contain any content changes.
		const excludedMaps = new Set( [

Add collaborative session statistics #103

Are you sure you want to change the base?

Add collaborative session statistics #103

Uh oh!

Conversation

acicovic commented Nov 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Key Changes

Technical Details

Testing

Future Work

Uh oh!

ingeniumed Nov 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

acicovic commented Nov 6, 2025

Uh oh!

chriszarate commented Nov 6, 2025

Uh oh!

acicovic commented Nov 7, 2025

Uh oh!

acicovic commented Dec 1, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chriszarate commented Dec 1, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

acicovic commented Dec 2, 2025

Uh oh!

chriszarate commented Dec 2, 2025

Uh oh!

acicovic commented Dec 2, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

acicovic commented Nov 5, 2025 •

edited

Loading

ingeniumed Nov 6, 2025 •

edited

Loading