Skip to content

Commit

Permalink
fix(plugin-meetings): send metrics on gathered ice candidates count (#…
Browse files Browse the repository at this point in the history
…3796)

Co-authored-by: kwasniow <[email protected]>
  • Loading branch information
k-wasniowski and kwasniow authored Aug 27, 2024
1 parent 876dc44 commit da7fd81
Show file tree
Hide file tree
Showing 6 changed files with 63 additions and 17 deletions.
2 changes: 1 addition & 1 deletion packages/@webex/media-helpers/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
"deploy:npm": "yarn npm publish"
},
"dependencies": {
"@webex/internal-media-core": "2.7.4",
"@webex/internal-media-core": "2.8.0",
"@webex/ts-events": "^1.1.0",
"@webex/web-media-effects": "2.18.0"
},
Expand Down
2 changes: 1 addition & 1 deletion packages/@webex/plugin-meetings/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@
},
"dependencies": {
"@webex/common": "workspace:*",
"@webex/internal-media-core": "2.7.4",
"@webex/internal-media-core": "2.8.0",
"@webex/internal-plugin-conversation": "workspace:*",
"@webex/internal-plugin-device": "workspace:*",
"@webex/internal-plugin-llm": "workspace:*",
Expand Down
19 changes: 19 additions & 0 deletions packages/@webex/plugin-meetings/src/meeting/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -694,6 +694,7 @@ export default class Meeting extends StatelessWebexPlugin {
private joinWithMediaRetryInfo?: {isRetry: boolean; prevJoinResponse?: any};
private connectionStateHandler?: ConnectionStateHandler;
private iceCandidateErrors: Map<string, number>;
private iceCandidatesCount: number;

/**
* @param {Object} attrs
Expand Down Expand Up @@ -1508,6 +1509,15 @@ export default class Meeting extends StatelessWebexPlugin {
* @memberof Meeting
*/
this.iceCandidateErrors = new Map();

/**
* Gathered ICE Candidates count
* @instance
* @type {number}
* @private
* @memberof Meeting
*/
this.iceCandidatesCount = 0;
}

/**
Expand Down Expand Up @@ -6119,6 +6129,13 @@ export default class Meeting extends StatelessWebexPlugin {

this.iceCandidateErrors.set(error, count + 1);
});

this.iceCandidatesCount = 0;
this.mediaProperties.webrtcMediaConnection.on(Event.ICE_CANDIDATE, (event) => {
if (event.candidate) {
this.iceCandidatesCount += 1;
}
});
};

/**
Expand Down Expand Up @@ -6990,6 +7007,7 @@ export default class Meeting extends StatelessWebexPlugin {
retriedWithTurnServer: this.addMediaData.retriedWithTurnServer,
isJoinWithMediaRetry: this.joinWithMediaRetryInfo.isRetry,
...reachabilityStats,
iceCandidatesCount: this.iceCandidatesCount,
});
// @ts-ignore
this.webex.internal.newMetrics.submitClientEvent({
Expand Down Expand Up @@ -7045,6 +7063,7 @@ export default class Meeting extends StatelessWebexPlugin {
'unknown',
...reachabilityMetrics,
...iceCandidateErrors,
iceCandidatesCount: this.iceCandidatesCount,
});

await this.cleanUpOnAddMediaFailure();
Expand Down
31 changes: 29 additions & 2 deletions packages/@webex/plugin-meetings/test/unit/spec/meeting/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2022,6 +2022,7 @@ describe('plugin-meetings', () => {
someReachabilityMetric2: 'some value2',
selectedCandidatePairChanges: 2,
numTransports: 1,
iceCandidatesCount: 0,
}
);
});
Expand Down Expand Up @@ -2129,6 +2130,7 @@ describe('plugin-meetings', () => {
someReachabilityMetric2: 'some value2',
selectedCandidatePairChanges: 2,
numTransports: 1,
iceCandidatesCount: 0,
}
);
});
Expand Down Expand Up @@ -2670,6 +2672,7 @@ describe('plugin-meetings', () => {
iceConnectionState: 'unknown',
selectedCandidatePairChanges: 2,
numTransports: 1,
iceCandidatesCount: 0,
},
]);

Expand Down Expand Up @@ -2858,6 +2861,7 @@ describe('plugin-meetings', () => {
isMultistream: false,
retriedWithTurnServer: true,
isJoinWithMediaRetry: false,
iceCandidatesCount: 0,
},
]);
meeting.roap.doTurnDiscovery;
Expand Down Expand Up @@ -2986,6 +2990,8 @@ describe('plugin-meetings', () => {
someReachabilityMetric2: 'some value2',
}),
};
meeting.iceCandidatesCount = 3;

await meeting.addMedia({
mediaSettings: {},
});
Expand All @@ -3005,6 +3011,7 @@ describe('plugin-meetings', () => {
isJoinWithMediaRetry: false,
someReachabilityMetric1: 'some value1',
someReachabilityMetric2: 'some value2',
iceCandidatesCount: 3,
}
);

Expand Down Expand Up @@ -3067,6 +3074,7 @@ describe('plugin-meetings', () => {
iceConnectionState: 'unknown',
selectedCandidatePairChanges: 2,
numTransports: 1,
iceCandidatesCount: 0,
}
);

Expand Down Expand Up @@ -3126,7 +3134,8 @@ describe('plugin-meetings', () => {
selectedCandidatePairChanges: 2,
numTransports: 1,
'701_error': 2,
'701_turn_host_lookup_received_error': 1
'701_turn_host_lookup_received_error': 1,
iceCandidatesCount: 0,
}
);

Expand Down Expand Up @@ -7503,6 +7512,7 @@ describe('plugin-meetings', () => {
assert.isFunction(eventListeners[Event.REMOTE_TRACK_ADDED]);
assert.isFunction(eventListeners[Event.PEER_CONNECTION_STATE_CHANGED]);
assert.isFunction(eventListeners[Event.ICE_CONNECTION_STATE_CHANGED]);
assert.isFunction(eventListeners[Event.ICE_CANDIDATE]);
assert.isFunction(eventListeners[Event.ICE_CANDIDATE_ERROR]);
});

Expand Down Expand Up @@ -7539,10 +7549,27 @@ describe('plugin-meetings', () => {
});
});

describe('should react on a ICE_CANDIDATE_ERROR event', () => {
describe('should react on a ICE_CANDIDATE event', () => {
beforeEach(() => {
meeting.setupMediaConnectionListeners();
});

it('should collect ice candidates', () => {
eventListeners[Event.ICE_CANDIDATE]({candidate: 'candidate'});

assert.equal(meeting.iceCandidatesCount, 1);
});

it('should not collect null ice candidates', () => {
eventListeners[Event.ICE_CANDIDATE]({candidate: null});

assert.equal(meeting.iceCandidatesCount, 0);
});
});

describe('should react on a ICE_CANDIDATE_ERROR event', () => {
beforeEach(() => {
meeting.setupMediaConnectionListeners();
});

it('should not collect skipped ice candidates error', () => {
Expand Down
2 changes: 1 addition & 1 deletion packages/calling/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
},
"dependencies": {
"@types/platform": "1.3.4",
"@webex/internal-media-core": "2.7.4",
"@webex/internal-media-core": "2.8.0",
"@webex/media-helpers": "workspace:*",
"async-mutex": "0.4.0",
"buffer": "6.0.3",
Expand Down
24 changes: 12 additions & 12 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -7400,7 +7400,7 @@ __metadata:
"@typescript-eslint/eslint-plugin": 5.38.1
"@typescript-eslint/parser": 5.38.1
"@web/dev-server": 0.4.5
"@webex/internal-media-core": 2.7.4
"@webex/internal-media-core": 2.8.0
"@webex/media-helpers": "workspace:*"
async-mutex: 0.4.0
buffer: 6.0.3
Expand Down Expand Up @@ -7691,19 +7691,19 @@ __metadata:
languageName: unknown
linkType: soft

"@webex/internal-media-core@npm:2.7.4":
version: 2.7.4
resolution: "@webex/internal-media-core@npm:2.7.4"
"@webex/internal-media-core@npm:2.8.0":
version: 2.8.0
resolution: "@webex/internal-media-core@npm:2.8.0"
dependencies:
"@babel/runtime": ^7.18.9
"@webex/ts-sdp": 1.7.0
"@webex/web-client-media-engine": 3.22.4
"@webex/web-client-media-engine": 3.23.1
events: ^3.3.0
typed-emitter: ^2.1.0
uuid: ^8.3.2
webrtc-adapter: ^8.1.2
xstate: ^4.30.6
checksum: 0610035ae8dfe3031a0d9004daa0b5f0d155743aa2eaff17c532786de544b77bb64c77a3b65964bf8ff24a1a308525c3256f22814aec73e177ccaad5bec221c9
checksum: a27ddbd484404dc99b627879df10ff97212fddb3bf94829aa41408d719436b33d6bce824b2a33a30a5dd83eabc9c3d70cbb4fe086fc00d5c1aca5da918503e3d
languageName: node
linkType: hard

Expand Down Expand Up @@ -8471,7 +8471,7 @@ __metadata:
"@babel/preset-typescript": 7.22.11
"@webex/babel-config-legacy": "workspace:*"
"@webex/eslint-config-legacy": "workspace:*"
"@webex/internal-media-core": 2.7.4
"@webex/internal-media-core": 2.8.0
"@webex/jest-config-legacy": "workspace:*"
"@webex/legacy-tools": "workspace:*"
"@webex/test-helper-chai": "workspace:*"
Expand Down Expand Up @@ -8707,7 +8707,7 @@ __metadata:
"@webex/babel-config-legacy": "workspace:*"
"@webex/common": "workspace:*"
"@webex/eslint-config-legacy": "workspace:*"
"@webex/internal-media-core": 2.7.4
"@webex/internal-media-core": 2.8.0
"@webex/internal-plugin-conversation": "workspace:*"
"@webex/internal-plugin-device": "workspace:*"
"@webex/internal-plugin-llm": "workspace:*"
Expand Down Expand Up @@ -9419,9 +9419,9 @@ __metadata:
languageName: node
linkType: hard

"@webex/web-client-media-engine@npm:3.22.4":
version: 3.22.4
resolution: "@webex/web-client-media-engine@npm:3.22.4"
"@webex/web-client-media-engine@npm:3.23.1":
version: 3.23.1
resolution: "@webex/web-client-media-engine@npm:3.23.1"
dependencies:
"@webex/json-multistream": 2.1.3
"@webex/rtcstats": ^1.3.2
Expand All @@ -9434,7 +9434,7 @@ __metadata:
js-logger: ^1.6.1
typed-emitter: ^2.1.0
uuid: ^8.3.2
checksum: 13f24b0411cceaaef046887f6fee8e57be8e12ed07b5382a9a51eb07f56f1ec4eb02bdbb2ed567925690618862a02e831c6775019f9abe559bf7f29c1d3b5fe9
checksum: e1a502725ac0ad588891e4ec0e73ffa5242d44fc612161a4c09d9f5f3a31734f22be28c8bf41838d3ae779e5f5689933f0b8deeb325c755499d3648d6e8035a5
languageName: node
linkType: hard

Expand Down

0 comments on commit da7fd81

Please sign in to comment.