perf(NODE-6452): Optimize CommandStartedEvent and CommandSucceededEvent constructors (#4371)

Author: W-A-James

src/cmap/command_monitoring_events.ts

@@ -6,7 +6,7 @@ import {
   LEGACY_HELLO_COMMAND,
   LEGACY_HELLO_COMMAND_CAMEL_CASE
 } from '../constants';
-import { calculateDurationInMs, deepCopy } from '../utils';
+import { calculateDurationInMs } from '../utils';
 import {
   DocumentSequence,
   OpMsgRequest,
@@ -125,7 +125,7 @@ export class CommandSucceededEvent {
     this.requestId = command.requestId;
     this.commandName = commandName;
     this.duration = calculateDurationInMs(started);
-    this.reply = maybeRedact(commandName, cmd, extractReply(command, reply));
+    this.reply = maybeRedact(commandName, cmd, extractReply(reply));
     this.serverConnectionId = serverConnectionId;
   }
 
@@ -214,7 +214,6 @@ const HELLO_COMMANDS = new Set(['hello', LEGACY_HELLO_COMMAND, LEGACY_HELLO_COMM
 
 // helper methods
 const extractCommandName = (commandDoc: Document) => Object.keys(commandDoc)[0];
-const namespace = (command: OpQueryRequest) => command.ns;
 const collectionName = (command: OpQueryRequest) => command.ns.split('.')[1];
 const maybeRedact = (commandName: string, commandDoc: Document, result: Error | Document) =>
   SENSITIVE_COMMANDS.has(commandName) ||
@@ -242,19 +241,10 @@ const LEGACY_FIND_OPTIONS_MAP = {
   returnFieldSelector: 'projection'
 } as const;
 
-const OP_QUERY_KEYS = [
-  'tailable',
-  'oplogReplay',
-  'noCursorTimeout',
-  'awaitData',
-  'partial',
-  'exhaust'
-] as const;
-
 /** Extract the actual command from the query, possibly up-converting if it's a legacy format */
 function extractCommand(command: WriteProtocolMessageType): Document {
   if (command instanceof OpMsgRequest) {
-    const cmd = deepCopy(command.command);
+    const cmd = { ...command.command };
     // For OP_MSG with payload type 1 we need to pull the documents
     // array out of the document sequence for monitoring.
     if (cmd.ops instanceof DocumentSequence) {
@@ -276,72 +266,37 @@ function extractCommand(command: WriteProtocolMessageType): Document {
       result = { find: collectionName(command) };
       Object.keys(LEGACY_FIND_QUERY_MAP).forEach(key => {
         if (command.query[key] != null) {
-          result[LEGACY_FIND_QUERY_MAP[key]] = deepCopy(command.query[key]);
+          result[LEGACY_FIND_QUERY_MAP[key]] = { ...command.query[key] };
         }
       });
     }
 
     Object.keys(LEGACY_FIND_OPTIONS_MAP).forEach(key => {
       const legacyKey = key as keyof typeof LEGACY_FIND_OPTIONS_MAP;
       if (command[legacyKey] != null) {
-        result[LEGACY_FIND_OPTIONS_MAP[legacyKey]] = deepCopy(command[legacyKey]);
-      }
-    });
-
-    OP_QUERY_KEYS.forEach(key => {
-      if (command[key]) {
-        result[key] = command[key];
+        result[LEGACY_FIND_OPTIONS_MAP[legacyKey]] = command[legacyKey];
       }
     });
 
-    if (command.pre32Limit != null) {
-      result.limit = command.pre32Limit;
-    }
-
-    if (command.query.$explain) {
-      return { explain: result };
-    }
     return result;
   }
 
-  const clonedQuery: Record<string, unknown> = {};
-  const clonedCommand: Record<string, unknown> = {};
+  let clonedQuery: Record<string, unknown> = {};
+  const clonedCommand: Record<string, unknown> = { ...command };
   if (command.query) {
-    for (const k in command.query) {
-      clonedQuery[k] = deepCopy(command.query[k]);
-    }
+    clonedQuery = { ...command.query };
     clonedCommand.query = clonedQuery;
   }
 
-  for (const k in command) {
-    if (k === 'query') continue;
-    clonedCommand[k] = deepCopy((command as unknown as Record<string, unknown>)[k]);
-  }
   return command.query ? clonedQuery : clonedCommand;
 }
 
-function extractReply(command: WriteProtocolMessageType, reply?: Document) {
+function extractReply(reply?: Document) {
   if (!reply) {
     return reply;
   }
 
-  if (command instanceof OpMsgRequest) {
-    return deepCopy(reply.result ? reply.result : reply);
-  }
-
-  // is this a legacy find command?
-  if (command.query && command.query.$query != null) {
-    return {
-      ok: 1,
-      cursor: {
-        id: deepCopy(reply.cursorId),
-        ns: namespace(command),
-        firstBatch: deepCopy(reply.documents)
-      }
-    };
-  }
-
-  return deepCopy(reply.result ? reply.result : reply);
+  return reply.result ? reply.result : reply;
 }
 
 function extractConnectionDetails(connection: Connection) {

src/utils.ts

@@ -620,43 +620,6 @@ export function isRecord(
   return true;
 }
 
-/**
- * Make a deep copy of an object
- *
- * NOTE: This is not meant to be the perfect implementation of a deep copy,
- * but instead something that is good enough for the purposes of
- * command monitoring.
- */
-export function deepCopy<T>(value: T): T {
-  if (value == null) {
-    return value;
-  } else if (Array.isArray(value)) {
-    return value.map(item => deepCopy(item)) as unknown as T;
-  } else if (isRecord(value)) {
-    const res = {} as any;
-    for (const key in value) {
-      res[key] = deepCopy(value[key]);
-    }
-    return res;
-  }
-
-  const ctor = (value as any).constructor;
-  if (ctor) {
-    switch (ctor.name.toLowerCase()) {
-      case 'date':
-        return new ctor(Number(value));
-      case 'map':
-        return new Map(value as any) as unknown as T;
-      case 'set':
-        return new Set(value as any) as unknown as T;
-      case 'buffer':
-        return Buffer.from(value as unknown as Buffer) as unknown as T;
-    }
-  }
-
-  return value;
-}
-
 type ListNode<T> = {
   value: T;
   next: ListNode<T> | HeadNode<T>;

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

@@ -603,49 +603,4 @@ describe('Command Monitoring', function () {
         });
     }
   });
-
-  describe('Internal state references', function () {
-    let client;
-
-    beforeEach(function () {
-      client = this.configuration.newClient(
-        { writeConcern: { w: 1 } },
-        { maxPoolSize: 1, monitorCommands: true }
-      );
-    });
-
-    afterEach(function (done) {
-      client.close(done);
-    });
-
-    // NODE-1502
-    it('should not allow mutation of internal state from commands returned by event monitoring', function () {
-      const started = [];
-      const succeeded = [];
-      client.on('commandStarted', filterForCommands('insert', started));
-      client.on('commandSucceeded', filterForCommands('insert', succeeded));
-      const documentToInsert = { a: { b: 1 } };
-      const db = client.db(this.configuration.db);
-      return db
-        .collection('apm_test')
-        .insertOne(documentToInsert)
-        .then(r => {
-          expect(r).to.have.property('insertedId').that.is.an('object');
-          expect(started).to.have.lengthOf(1);
-          // Check if contents of returned document are equal to document inserted (by value)
-          expect(documentToInsert).to.deep.equal(started[0].command.documents[0]);
-          // Check if the returned document is a clone of the original. This confirms that the
-          // reference is not the same.
-          expect(documentToInsert !== started[0].command.documents[0]).to.equal(true);
-          expect(documentToInsert.a !== started[0].command.documents[0].a).to.equal(true);
-
-          started[0].command.documents[0].a.b = 2;
-          expect(documentToInsert.a.b).to.equal(1);
-
-          expect(started[0].commandName).to.equal('insert');
-          expect(started[0].command.insert).to.equal('apm_test');
-          expect(succeeded).to.have.lengthOf(1);
-        });
-    });
-  });
 });