Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Jan 17, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 10 additions & 55 deletions src/cmap/command_monitoring_events.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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) ||
Expand Down Expand Up @@ -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) {
Expand All @@ -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) {
Expand Down
37 changes: 0 additions & 37 deletions src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});
});
});
Loading