Skip to content

Commit

Permalink
fix(NODE-3513): default command monitoring to off (#2926)
Browse files Browse the repository at this point in the history
* fix(NODE-3513): default command monitoring to off

* fix: tests that expect command monitoring to be on by default

* test: setting monitorCommands to true
  • Loading branch information
nbbeeken authored and ljhaywar committed Nov 9, 2021
1 parent d2eb75b commit 5163b4b
Show file tree
Hide file tree
Showing 7 changed files with 113 additions and 40 deletions.
2 changes: 1 addition & 1 deletion src/connection_string.ts
Original file line number Diff line number Diff line change
Expand Up @@ -724,7 +724,7 @@ export const OPTIONS = {
type: 'uint'
},
monitorCommands: {
default: true,
default: false,
type: 'boolean'
},
name: {
Expand Down
2 changes: 1 addition & 1 deletion test/functional/causal_consistency.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ describe('Causal Consistency', function () {

beforeEach(function () {
test.commands = { started: [], succeeded: [] };
test.client = this.configuration.newClient({ w: 1 }, { maxPoolSize: 1 });
test.client = this.configuration.newClient({ w: 1 }, { maxPoolSize: 1, monitorCommands: true });
test.client.on('commandStarted', event => {
if (ignoredCommands.indexOf(event.commandName) === -1) test.commands.started.push(event);
});
Expand Down
5 changes: 4 additions & 1 deletion test/functional/cursor.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3564,7 +3564,10 @@ describe('Cursor', function () {
test: function (done) {
var started = [];
const configuration = this.configuration;
const client = configuration.newClient(configuration.writeConcernMax(), { maxPoolSize: 1 });
const client = configuration.newClient(configuration.writeConcernMax(), {
maxPoolSize: 1,
monitorCommands: true
});
client.on('commandStarted', function (event) {
if (event.commandName === 'count') started.push(event);
});
Expand Down
15 changes: 12 additions & 3 deletions test/functional/find_and_modify.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,11 @@ describe('Find and Modify', function () {
var succeeded = [];

var configuration = this.configuration;
var client = configuration.newClient(configuration.writeConcernMax(), { maxPoolSize: 1 });
var client = configuration.newClient(configuration.writeConcernMax(), {
maxPoolSize: 1,
monitorCommands: true
});

client.on('commandStarted', function (event) {
if (event.commandName === 'findAndModify') started.push(event);
});
Expand Down Expand Up @@ -85,7 +89,11 @@ describe('Find and Modify', function () {
var succeeded = [];

var configuration = this.configuration;
var client = configuration.newClient(configuration.writeConcernMax(), { maxPoolSize: 1 });
var client = configuration.newClient(configuration.writeConcernMax(), {
maxPoolSize: 1,
monitorCommands: true
});

client.on('commandStarted', function (event) {
if (event.commandName === 'findAndModify') started.push(event);
});
Expand Down Expand Up @@ -146,7 +154,8 @@ describe('Find and Modify', function () {
url = url.indexOf('?') !== -1 ? f('%s&%s', url, 'fsync=true') : f('%s?%s', url, 'fsync=true');

// Establish connection to db
const client = configuration.newClient(url, { sslValidate: false });
const client = configuration.newClient(url, { sslValidate: false, monitorCommands: true });

client.on('commandStarted', function (event) {
if (event.commandName === 'findAndModify') started.push(event);
});
Expand Down
5 changes: 4 additions & 1 deletion test/functional/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -947,7 +947,10 @@ describe('Indexes', function () {
var configuration = this.configuration;
var started = [];
var succeeded = [];
var client = configuration.newClient(configuration.writeConcernMax(), { maxPoolSize: 1 });
var client = configuration.newClient(configuration.writeConcernMax(), {
maxPoolSize: 1,
monitorCommands: true
});

client.on('commandStarted', function (event) {
if (event.commandName === 'createIndexes') started.push(event);
Expand Down
40 changes: 7 additions & 33 deletions test/functional/insert.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2377,7 +2377,10 @@ describe('Insert', function () {
var succeeded = [];

var configuration = this.configuration;
var client = configuration.newClient(configuration.writeConcernMax(), { maxPoolSize: 1 });
var client = configuration.newClient(configuration.writeConcernMax(), {
maxPoolSize: 1,
monitorCommands: true
});
client.on('commandStarted', function (event) {
if (event.commandName === 'insert') started.push(event);
});
Expand Down Expand Up @@ -2409,39 +2412,10 @@ describe('Insert', function () {
var succeeded = [];

var configuration = this.configuration;
var client = configuration.newClient(configuration.writeConcernMax(), { maxPoolSize: 1 });
client.on('commandStarted', function (event) {
if (event.commandName === 'insert') started.push(event);
});

client.on('commandSucceeded', function (event) {
if (event.commandName === 'insert') succeeded.push(event);
});

client.connect(function (err, client) {
var db = client.db(configuration.db);
expect(err).to.not.exist;

db.collection('apm_test')
.insertMany([{ a: 1 }], { forceServerObjectId: true })
.then(function () {
expect(started[0].command.documents[0]._id).to.not.exist;

client.close(done);
});
var client = configuration.newClient(configuration.writeConcernMax(), {
maxPoolSize: 1,
monitorCommands: true
});
}
});

it('Correctly allow forceServerObjectId for insertMany', {
metadata: { requires: { topology: ['single'] } },

test: function (done) {
var started = [];
var succeeded = [];

var configuration = this.configuration;
var client = configuration.newClient(configuration.writeConcernMax(), { maxPoolSize: 1 });
client.on('commandStarted', function (event) {
if (event.commandName === 'insert') started.push(event);
});
Expand Down
84 changes: 84 additions & 0 deletions test/unit/mongo_client_options.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
const os = require('os');
const fs = require('fs');
const { expect } = require('chai');
const { getSymbolFrom } = require('../tools/utils');
const { parseOptions } = require('../../src/connection_string');
const { ReadConcern } = require('../../src/read_concern');
const { WriteConcern } = require('../../src/write_concern');
Expand Down Expand Up @@ -450,4 +451,87 @@ describe('MongoOptions', function () {
);
});
});

describe('default options', () => {
const doNotCheckEq = Symbol('do not check equality');
const KNOWN_DEFAULTS = [
['connecttimeoutms', 30000],
['directconnection', false],
['forceserverobjectid', false],
['heartbeatfrequencyms', 10000],
['keepalive', true],
['keepaliveinitialdelay', 120000],
['localthresholdms', 15],
['logger', doNotCheckEq],
['maxidletimems', 0],
['maxpoolsize', 100],
['minpoolsize', 0],
['minheartbeatfrequencyms', 500],
['monitorcommands', false], // NODE-3513
['nodelay', true],
['raw', false],
['retryreads', true],
['retrywrites', true],
['serverselectiontimeoutms', 30000],
['sockettimeoutms', 0],
['waitqueuetimeoutms', 0],
['zlibcompressionlevel', 0],

// map to objects that are not worth checking deep equality
['compressors', doNotCheckEq],
['readpreference', doNotCheckEq],
['pkfactory', doNotCheckEq]
];

const findMatchingKey = (o, searchKey) => {
return Object.keys(o).filter(key => {
return key.toLowerCase() === searchKey;
})[0];
};

it(`should define known defaults in client.options`, () => {
const client = new MongoClient('mongodb://localhost');
const clientOptions = client.options;

for (const [optionName, value] of KNOWN_DEFAULTS) {
const camelCaseName = findMatchingKey(clientOptions, optionName);
expect(camelCaseName, `did not find a camelcase match for ${optionName}`).to.be.a('string');

expect(clientOptions).to.have.property(camelCaseName);

if (value !== doNotCheckEq) {
expect(clientOptions).to.have.property(camelCaseName).that.deep.equals(value);
}
}
});

it('set monitorCommands to false (NODE-3513)', function () {
const client = new MongoClient('mongodb://localhost');
const clientOptions = client.options;

expect(clientOptions).to.have.property('monitorCommands', false);
expect(client.s.options).to.have.property('monitorCommands', false);
expect(client).to.have.property('monitorCommands', false);
const optionsSym = getSymbolFrom(client, 'options');
expect(client[optionsSym]).to.have.property('monitorCommands', false);
});

it('respects monitorCommands option passed in', function () {
const clientViaOpt = new MongoClient('mongodb://localhost', { monitorCommands: true });
const clientViaUri = new MongoClient('mongodb://localhost?monitorCommands=true');

const testTable = [
[clientViaOpt, clientViaOpt.options],
[clientViaUri, clientViaUri.options]
];

for (const [client, clientOptions] of testTable) {
expect(clientOptions).to.have.property('monitorCommands', true);
expect(client.s.options).to.have.property('monitorCommands', true);
expect(client).to.have.property('monitorCommands', true);
const optionsSym = getSymbolFrom(client, 'options');
expect(client[optionsSym]).to.have.property('monitorCommands', true);
}
});
});
});

0 comments on commit 5163b4b

Please sign in to comment.