fix(meetings): added reachability trigger to metrics (#3850)

Author: marcin-bazyl

packages/@webex/plugin-meetings/src/meetings/index.ts

@@ -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
@@ -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);
   }
 
   /**

packages/@webex/plugin-meetings/src/reachability/index.ts

@@ -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
@@ -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
@@ -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,

packages/@webex/plugin-meetings/src/reconnection-manager/index.ts

@@ -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: ',

packages/@webex/plugin-meetings/test/unit/spec/meetings/index.js

@@ -79,6 +79,7 @@ describe('plugin-meetings', () => {
   let locusInfo;
   let services;
   let catalog;
+  let startReachabilityStub;
 
   describe('meetings index', () => {
     beforeEach(() => {
@@ -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()},
@@ -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');

packages/@webex/plugin-meetings/test/unit/spec/reachability/index.ts

@@ -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';
@@ -507,6 +506,11 @@ describe('gatherReachability', () => {
         mockClusterReachabilityInstances[id] = mockInstance;
         return mockInstance;
       });
+
+    webex.config.meetings.experimental = {
+      enableTcpReachability: false,
+      enableTlsReachability: false,
+    };
   });
 
   afterEach(() => {
@@ -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 = {
@@ -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();
 
@@ -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,
@@ -1180,7 +1217,7 @@ describe('gatherReachability', () => {
       joinCookie: {id: 'id'},
     });
 
-    const resultPromise = reachability.gatherReachability();
+    const resultPromise = reachability.gatherReachability('test');
 
     await testUtils.flushPromises();
 
@@ -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',
     });
   });
 
@@ -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();
 
@@ -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();
 
@@ -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);
 
@@ -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);
 
@@ -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;
@@ -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;
 
@@ -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;
@@ -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;
@@ -1595,7 +1633,7 @@ describe('gatherReachability', () => {
       return getClustersResult;
     });
 
-    const promise = reachability.gatherReachability();
+    const promise = reachability.gatherReachability('test');
 
     await simulateTimeout();
     await promise;
@@ -1616,7 +1654,7 @@ describe('gatherReachability', () => {
       throw new Error('fake error');
     });
 
-    const promise = reachability.gatherReachability();
+    const promise = reachability.gatherReachability('test');
 
     await simulateTimeout();