ci(NODE-6519,NODE-6702): fix flaky listIndexes APM test and rtt calculation test (#4464)

Author: baileympearson

test/integration/command-logging-and-monitoring/command_monitoring.test.ts

@@ -1,13 +1,19 @@
 import { expect } from 'chai';
 
-import { ReadPreference } from '../../mongodb';
+import { type MongoClient, ObjectId, ReadPreference } from '../../mongodb';
 import { filterForCommands, ignoreNsNotFound, setupDatabase } from '../shared';
 
 describe('Command Monitoring', function () {
+  let client: MongoClient;
+
   before(function () {
     return setupDatabase(this.configuration);
   });
 
+  afterEach(async function () {
+    await client?.close();
+  });
+
   it('should correctly receive the APM events for an insert', {
     metadata: { requires: { topology: ['single', 'replicaset', 'sharded'] } },
 
@@ -65,81 +71,75 @@ describe('Command Monitoring', function () {
     }
   });
 
-  it('should correctly receive the APM events for a listCollections command', {
-    metadata: { requires: { topology: ['replicaset'], mongodb: '>=3.0.0' } },
+  it('records APM events for a listIndexes command', async function () {
+    const started = [];
+    const succeeded = [];
+    client = this.configuration.newClient(
+      { writeConcern: { w: 'majority' } },
+      { maxPoolSize: 1, monitorCommands: true }
+    );
 
-    test: function () {
-      const started = [];
-      const succeeded = [];
-      const client = this.configuration.newClient(
-        { writeConcern: { w: 1 } },
-        { maxPoolSize: 1, monitorCommands: true }
-      );
+    const desiredEvents = ['listIndexes'];
+    client.on('commandStarted', filterForCommands(desiredEvents, started));
+    client.on('commandSucceeded', filterForCommands(desiredEvents, succeeded));
 
-      client.on('commandStarted', filterForCommands('listCollections', started));
-      client.on('commandSucceeded', filterForCommands('listCollections', succeeded));
+    const db = client.db(new ObjectId().toHexString());
 
-      const db = client.db(this.configuration.db);
+    const collection = db.collection('apm_test_list_collections');
+    const session = client.startSession({ causalConsistency: true });
 
-      return db
-        .collection('apm_test_list_collections')
-        .insertOne({ a: 1 }, this.configuration.writeConcernMax())
-        .then(r => {
-          expect(r).property('insertedId').to.exist;
-          return db.listCollections({}, { readPreference: ReadPreference.primary }).toArray();
-        })
-        .then(() => db.listCollections({}, { readPreference: ReadPreference.secondary }).toArray())
-        .then(() => {
-          expect(started).to.have.lengthOf(2);
-          expect(started[0]).property('address').to.not.equal(started[1].address);
+    const r = await collection.insertOne({ a: 1 }, { writeConcern: { w: 'majority' }, session });
+    expect(r).property('insertedId').to.exist;
 
-          return client.close();
-        });
-    }
-  });
+    expect(await collection.listIndexes({ session }).toArray()).to.have.lengthOf(1);
 
-  it('should correctly receive the APM events for a listIndexes command', {
-    metadata: { requires: { topology: ['replicaset'], mongodb: '>=3.0.0' } },
+    const [{ commandName }] = started;
+    expect(commandName).to.equal('listIndexes');
+  });
 
-    test: function () {
+  it(
+    'records APM events for reads on secondaries',
+    { requires: { topology: ['replicaset'] } },
+    async function () {
       const started = [];
       const succeeded = [];
-      const client = this.configuration.newClient(
-        { writeConcern: { w: 1 } },
+      client = this.configuration.newClient(
+        { writeConcern: { w: 'majority' } },
         { maxPoolSize: 1, monitorCommands: true }
       );
 
       const desiredEvents = ['listIndexes', 'find'];
       client.on('commandStarted', filterForCommands(desiredEvents, started));
       client.on('commandSucceeded', filterForCommands(desiredEvents, succeeded));
 
-      const db = client.db(this.configuration.db);
+      const db = client.db(new ObjectId().toHexString());
 
-      return db
-        .collection('apm_test_list_collections')
-        .insertOne({ a: 1 }, this.configuration.writeConcernMax())
-        .then(r => {
-          expect(r).property('insertedId').to.exist;
+      const collection = db.collection('apm_test_list_collections');
+      const session = client.startSession({ causalConsistency: true });
 
-          return db
-            .collection('apm_test_list_collections')
-            .listIndexes({ readPreference: ReadPreference.PRIMARY })
-            .toArray();
-        })
-        .then(() =>
-          db
-            .collection('apm_test_list_collections')
-            .listIndexes({ readPreference: ReadPreference.SECONDARY })
-            .toArray()
-        )
-        .then(() => {
-          expect(started).to.have.lengthOf(2);
-          expect(started[0]).property('address').to.not.equal(started[1].address);
+      const r = await collection.insertOne({ a: 1 }, { writeConcern: { w: 'majority' }, session });
+      expect(r).property('insertedId').to.exist;
 
-          return client.close();
+      await collection
+        .listIndexes({ readPreference: ReadPreference.PRIMARY, session })
+        .toArray()
+        .catch(e => {
+          throw new Error('primary listIndexes failed', { cause: e });
         });
+
+      await collection
+        .listIndexes({ readPreference: ReadPreference.SECONDARY, session })
+        .toArray()
+        .catch(() => {
+          // reading with secondary read preference means the data may or may not have been propagated to the seconary
+          // node yet.  for this test, we are asserting that we did correctly send commands to different nodes, so
+          // the actual outcome of this listIndexes doesn't matter.
+        });
+
+      const [{ address: primaryAddress }, { address: secondaryAddress }] = started;
+      expect(primaryAddress).not.to.equal(secondaryAddress);
     }
-  });
+  );
 
   it('should correctly receive the APM events for a find with getmore and killcursor', {
     metadata: { requires: { topology: ['single', 'replicaset'] } },

test/integration/server-discovery-and-monitoring/server_discover_and_monitoring.test.ts

@@ -54,7 +54,9 @@ describe('Monitoring rtt tests', function () {
             // @ts-expect-error accessing private method
             .stub(Connection.prototype, 'sendCommand')
             .callsFake(async function* (...args) {
-              await setTimeout(DELAY_MS);
+              // https://github.com/nodejs/node/issues/26578
+              // setTimeout can result in the timeout being called in < the provided interval
+              await setTimeout(DELAY_MS + 1);
               yield* stub.wrappedMethod.call(this, ...args);
             });
           await client.connect();