Skip to content

Commit

Permalink
fix(meetings): added reachability trigger to metrics (webex#3850)
Browse files Browse the repository at this point in the history
  • Loading branch information
marcin-bazyl authored Sep 24, 2024
1 parent 0af7097 commit b93ddef
Show file tree
Hide file tree
Showing 5 changed files with 94 additions and 22 deletions.
7 changes: 4 additions & 3 deletions packages/@webex/plugin-meetings/src/meetings/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -765,7 +765,7 @@ export default class Meetings extends WebexPlugin {
return Promise.all([
this.fetchUserPreferredWebexSite(),
this.getGeoHint(),
this.startReachability().catch((error) => {
this.startReachability('registration').catch((error) => {
LoggerProxy.logger.error(`Meetings:index#register --> GDM error, ${error.message}`);
}),
// @ts-ignore
Expand Down Expand Up @@ -967,12 +967,13 @@ export default class Meetings extends WebexPlugin {

/**
* initializes and starts gathering reachability for Meetings
* @param {string} trigger - explains the reason for starting reachability
* @returns {Promise}
* @public
* @memberof Meetings
*/
startReachability() {
return this.getReachability().gatherReachability();
startReachability(trigger = 'client') {
return this.getReachability().gatherReachability(trigger);
}

/**
Expand Down
8 changes: 7 additions & 1 deletion packages/@webex/plugin-meetings/src/reachability/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,8 @@ export default class Reachability extends EventsScope {
expectedResultsCount = {videoMesh: {udp: 0}, public: {udp: 0, tcp: 0, xtls: 0}};
resultsCount = {videoMesh: {udp: 0}, public: {udp: 0, tcp: 0, xtls: 0}};

protected lastTrigger?: string;

/**
* Creates an instance of Reachability.
* @param {object} webex
Expand Down Expand Up @@ -142,13 +144,16 @@ export default class Reachability extends EventsScope {

/**
* Gets a list of media clusters from the backend and performs reachability checks on all the clusters
* @param {string} trigger - explains the reason for starting reachability
* @returns {Promise<ReachabilityResults>} reachability results
* @public
* @memberof Reachability
*/
public async gatherReachability(): Promise<ReachabilityResults> {
public async gatherReachability(trigger: string): Promise<ReachabilityResults> {
// Fetch clusters and measure latency
try {
this.lastTrigger = trigger;

// kick off ip version detection. For now we don't await it, as we're doing it
// to gather the timings and send them with our reachability metrics
// @ts-ignore
Expand Down Expand Up @@ -552,6 +557,7 @@ export default class Reachability extends EventsScope {
// @ts-ignore
totalTime: this.webex.internal.device.ipNetworkDetector.totalTime,
},
trigger: this.lastTrigger,
};
Metrics.sendBehavioralMetric(
BEHAVIORAL_METRICS.REACHABILITY_COMPLETED,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,7 @@ export default class ReconnectionManager {
}

try {
await this.webex.meetings.startReachability();
await this.webex.meetings.startReachability('reconnection');
} catch (err) {
LoggerProxy.logger.info(
'ReconnectionManager:index#reconnect --> Reachability failed, continuing with reconnection attempt, err: ',
Expand Down
33 changes: 30 additions & 3 deletions packages/@webex/plugin-meetings/test/unit/spec/meetings/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ describe('plugin-meetings', () => {
let locusInfo;
let services;
let catalog;
let startReachabilityStub;

describe('meetings index', () => {
beforeEach(() => {
Expand Down Expand Up @@ -129,9 +130,7 @@ describe('plugin-meetings', () => {
logger,
});

Object.assign(webex.meetings, {
startReachability: sinon.stub().returns(Promise.resolve()),
});
startReachabilityStub = sinon.stub(webex.meetings, 'startReachability').resolves();

Object.assign(webex.internal, {
llm: {on: sinon.stub()},
Expand Down Expand Up @@ -197,6 +196,34 @@ describe('plugin-meetings', () => {
assert.calledOnce(MeetingsUtil.checkH264Support);
});

describe('#startReachability', () => {
let gatherReachabilitySpy;
let fakeResult = {id: 'fake-result'};

beforeEach(() => {
startReachabilityStub.restore();
gatherReachabilitySpy = sinon
.stub(webex.meetings.getReachability(), 'gatherReachability')
.resolves(fakeResult);
});

it('should gather reachability with default trigger value', async () => {
const result = await webex.meetings.startReachability();

assert.calledOnceWithExactly(gatherReachabilitySpy, 'client');
assert.equal(result, fakeResult);
});

it('should gather reachability and pass custom trigger value', async () => {
const trigger = 'custom-trigger';

const result = await webex.meetings.startReachability(trigger);

assert.calledOnceWithExactly(gatherReachabilitySpy, trigger);
assert.equal(result, fakeResult);
});
});

describe('#_toggleUnifiedMeetings', () => {
it('should have toggleUnifiedMeetings', () => {
assert.equal(typeof webex.meetings._toggleUnifiedMeetings, 'function');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import sinon from 'sinon';
import EventEmitter from 'events';
import testUtils from '../../../utils/testUtils';
import Reachability, {
ReachabilityResults,
ReachabilityResultsForBackend,
} from '@webex/plugin-meetings/src/reachability/';
import {ClusterNode} from '../../../../src/reachability/request';
Expand Down Expand Up @@ -507,6 +506,11 @@ describe('gatherReachability', () => {
mockClusterReachabilityInstances[id] = mockInstance;
return mockInstance;
});

webex.config.meetings.experimental = {
enableTcpReachability: false,
enableTlsReachability: false,
};
});

afterEach(() => {
Expand Down Expand Up @@ -1044,13 +1048,14 @@ describe('gatherReachability', () => {
enableTlsReachability: true,
};

// the metrics related to ipver are not tested in these tests and are all the same, so setting them up here
// the metrics related to ipver and trigger are not tested in these tests and are all the same, so setting them up here
const expectedMetricsFull = {
...expectedMetrics,
ipver_firstIpV4: -1,
ipver_firstIpV6: -1,
ipver_firstMdns: -1,
ipver_totalTime: -1,
trigger: 'test',
};

const receivedEvents = {
Expand Down Expand Up @@ -1082,7 +1087,7 @@ describe('gatherReachability', () => {

reachability.reachabilityRequest.getClusters = sinon.stub().returns(mockGetClustersResult);

const resultPromise = reachability.gatherReachability();
const resultPromise = reachability.gatherReachability('test');

await testUtils.flushPromises();

Expand Down Expand Up @@ -1142,6 +1147,38 @@ describe('gatherReachability', () => {
})
);

it('sends the trigger parameter in the metrics', async () => {
const reachability = new TestReachability(webex);

const mockGetClustersResult = {
clusters: {
clusterA: {
udp: ['udp-url'],
tcp: [],
xtls: [],
isVideoMesh: false,
},
},
joinCookie: {id: 'id'},
};

reachability.reachabilityRequest.getClusters = sinon.stub().returns(mockGetClustersResult);

const resultPromise = reachability.gatherReachability('some trigger');

// let it time out
await testUtils.flushPromises();
clock.tick(15000);
await resultPromise;

// check the metric contains the right trigger value
assert.calledWith(
Metrics.sendBehavioralMetric,
'js_sdk_reachability_completed',
sinon.match({trigger: 'some trigger'})
);
});

it(`starts ip network version detection and includes the results in the metrics`, async () => {
webex.config.meetings.experimental = {
enableTcpReachability: true,
Expand Down Expand Up @@ -1180,7 +1217,7 @@ describe('gatherReachability', () => {
joinCookie: {id: 'id'},
});

const resultPromise = reachability.gatherReachability();
const resultPromise = reachability.gatherReachability('test');

await testUtils.flushPromises();

Expand Down Expand Up @@ -1217,6 +1254,7 @@ describe('gatherReachability', () => {
ipver_firstIpV6: webex.internal.device.ipNetworkDetector.firstIpV6,
ipver_firstMdns: webex.internal.device.ipNetworkDetector.firstMdns,
ipver_totalTime: webex.internal.device.ipNetworkDetector.totalTime,
trigger: 'test',
});
});

Expand Down Expand Up @@ -1248,7 +1286,7 @@ describe('gatherReachability', () => {

reachability.reachabilityRequest.getClusters = sinon.stub().returns(mockGetClustersResult);

const resultPromise = reachability.gatherReachability();
const resultPromise = reachability.gatherReachability('test');

await testUtils.flushPromises();

Expand Down Expand Up @@ -1341,7 +1379,7 @@ describe('gatherReachability', () => {

reachability.reachabilityRequest.getClusters = sinon.stub().returns(mockGetClustersResult);

const resultPromise = reachability.gatherReachability();
const resultPromise = reachability.gatherReachability('test');

await testUtils.flushPromises();

Expand Down Expand Up @@ -1382,7 +1420,7 @@ describe('gatherReachability', () => {

reachability.reachabilityRequest.getClusters = sinon.stub().throws();

const result = await reachability.gatherReachability();
const result = await reachability.gatherReachability('test');

assert.empty(result);

Expand All @@ -1400,7 +1438,7 @@ describe('gatherReachability', () => {
reachability.reachabilityRequest.getClusters = sinon.stub().returns(getClustersResult);
(reachability as any).performReachabilityChecks = sinon.stub().throws();

const result = await reachability.gatherReachability();
const result = await reachability.gatherReachability('test');

assert.empty(result);

Expand Down Expand Up @@ -1435,7 +1473,7 @@ describe('gatherReachability', () => {

reachability.reachabilityRequest.getClusters = sinon.stub().returns(getClustersResult);

const promise = reachability.gatherReachability();
const promise = reachability.gatherReachability('test');

await simulateTimeout();
await promise;
Expand Down Expand Up @@ -1481,7 +1519,7 @@ describe('gatherReachability', () => {

reachability.reachabilityRequest.getClusters = sinon.stub().returns(getClustersResult);

const promise = reachability.gatherReachability();
const promise = reachability.gatherReachability('test');
await simulateTimeout();
await promise;

Expand Down Expand Up @@ -1515,7 +1553,7 @@ describe('gatherReachability', () => {

reachability.reachabilityRequest.getClusters = sinon.stub().returns(getClustersResult);

const promise = reachability.gatherReachability();
const promise = reachability.gatherReachability('test');

await simulateTimeout();
await promise;
Expand Down Expand Up @@ -1550,7 +1588,7 @@ describe('gatherReachability', () => {

reachability.reachabilityRequest.getClusters = sinon.stub().returns(getClustersResult);

const promise = reachability.gatherReachability();
const promise = reachability.gatherReachability('test');

await simulateTimeout();
await promise;
Expand Down Expand Up @@ -1595,7 +1633,7 @@ describe('gatherReachability', () => {
return getClustersResult;
});

const promise = reachability.gatherReachability();
const promise = reachability.gatherReachability('test');

await simulateTimeout();
await promise;
Expand All @@ -1616,7 +1654,7 @@ describe('gatherReachability', () => {
throw new Error('fake error');
});

const promise = reachability.gatherReachability();
const promise = reachability.gatherReachability('test');

await simulateTimeout();

Expand Down

0 comments on commit b93ddef

Please sign in to comment.