From e5351f214629c209fce3e5981367a173ca2fdde8 Mon Sep 17 00:00:00 2001 From: Aditi Khare Date: Thu, 1 Feb 2024 11:41:20 -0500 Subject: [PATCH 01/11] forming willLog --- src/mongo_logger.ts | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/mongo_logger.ts b/src/mongo_logger.ts index 57864c4c4ef..d9195c58da2 100644 --- a/src/mongo_logger.ts +++ b/src/mongo_logger.ts @@ -728,6 +728,7 @@ export class MongoLogger { logDestination: MongoDBLogWritable; logDestinationIsStdErr: boolean; pendingLog: PromiseLike | unknown = null; + willLog: any; //Record>; /** * This method should be used when logging errors that do not have a public driver API for @@ -760,10 +761,20 @@ export class MongoLogger { this.maxDocumentLength = options.maxDocumentLength; this.logDestination = options.logDestination; this.logDestinationIsStdErr = options.logDestinationIsStdErr; + this.willLog = this.createWillLog(); } - willLog(severity: SeverityLevel, component: MongoLoggableComponent): boolean { - return compareSeverity(severity, this.componentSeverities[component]) <= 0; + createWillLog() { + const to_return = {}; + console.log(Object.values(SEVERITY_LEVEL_MAP)) + for (const component of Object.values(MongoLoggableComponent)) { + const componentSeverityNum = SEVERITY_LEVEL_MAP.getNumericSeverityLevel(this.componentSeverities[component]); + for (const severityLevel of Object.entries(SEVERITY_LEVEL_MAP)) { + + } + + } + return to_return; } turnOffSeverities() { From f9103a331fc024a8da692217d32202d279611bc3 Mon Sep 17 00:00:00 2001 From: Aditi Khare Date: Fri, 2 Feb 2024 12:52:18 -0500 Subject: [PATCH 02/11] willLog precomputed --- src/mongo_logger.ts | 30 ++++++++------ test/unit/cmap/connection_pool.test.js | 6 ++- test/unit/mongo_logger.test.ts | 56 ++++++++++++++++++++++++++ 3 files changed, 77 insertions(+), 15 deletions(-) diff --git a/src/mongo_logger.ts b/src/mongo_logger.ts index d9195c58da2..cd86a81fc99 100644 --- a/src/mongo_logger.ts +++ b/src/mongo_logger.ts @@ -728,7 +728,10 @@ export class MongoLogger { logDestination: MongoDBLogWritable; logDestinationIsStdErr: boolean; pendingLog: PromiseLike | unknown = null; - willLog: any; //Record>; + willLog: Record> = {} as Record< + MongoLoggableComponent, + Record + >; /** * This method should be used when logging errors that do not have a public driver API for @@ -761,25 +764,26 @@ export class MongoLogger { this.maxDocumentLength = options.maxDocumentLength; this.logDestination = options.logDestination; this.logDestinationIsStdErr = options.logDestinationIsStdErr; - this.willLog = this.createWillLog(); + this.createWillLog(); } createWillLog() { - const to_return = {}; - console.log(Object.values(SEVERITY_LEVEL_MAP)) for (const component of Object.values(MongoLoggableComponent)) { - const componentSeverityNum = SEVERITY_LEVEL_MAP.getNumericSeverityLevel(this.componentSeverities[component]); - for (const severityLevel of Object.entries(SEVERITY_LEVEL_MAP)) { - - } - + this.willLog[component] = Object.fromEntries( + Object.entries(SeverityLevel).map(([_key, value]) => [ + value as SeverityLevel, + compareSeverity(value, this.componentSeverities[component]) <= 0 + ]) + ) as Record; } - return to_return; } turnOffSeverities() { - for (const key of Object.values(MongoLoggableComponent)) { - this.componentSeverities[key as MongoLoggableComponent] = SeverityLevel.OFF; + for (const component of Object.values(MongoLoggableComponent)) { + this.componentSeverities[component] = SeverityLevel.OFF; + for (const severityLevel of Object.values(SeverityLevel)) { + this.willLog[component][severityLevel] = false; + } } } @@ -813,7 +817,7 @@ export class MongoLogger { component: MongoLoggableComponent, message: Loggable | string ): void { - if (!this.willLog(severity, component)) return; + if (!this.willLog[component][severity]) return; let logMessage: Log = { t: new Date(), c: component, s: severity }; if (typeof message === 'string') { diff --git a/test/unit/cmap/connection_pool.test.js b/test/unit/cmap/connection_pool.test.js index cdbf00bf67f..ce79870572b 100644 --- a/test/unit/cmap/connection_pool.test.js +++ b/test/unit/cmap/connection_pool.test.js @@ -7,7 +7,7 @@ const sinon = require('sinon'); const { expect } = require('chai'); const { setImmediate } = require('timers'); const { promisify } = require('util'); -const { ns, isHello } = require('../../mongodb'); +const { ns, isHello, MongoLoggableComponent } = require('../../mongodb'); const { LEGACY_HELLO_COMMAND } = require('../../mongodb'); const { createTimerSandbox } = require('../timer_sandbox'); const { topologyWithPlaceholderClient } = require('../../tools/utils'); @@ -19,7 +19,9 @@ describe('Connection Pool', function () { client: { mongoLogger: { debug: () => null, - willLog: () => null + willLog: Object.fromEntries( + Object.values(MongoLoggableComponent).map(([value]) => [value, {}]) + ) } } } diff --git a/test/unit/mongo_logger.test.ts b/test/unit/mongo_logger.test.ts index 0d60cf4d076..ef802e4123b 100644 --- a/test/unit/mongo_logger.test.ts +++ b/test/unit/mongo_logger.test.ts @@ -23,6 +23,7 @@ import { DEFAULT_MAX_DOCUMENT_LENGTH, type Log, type MongoDBLogWritable, + MongoLoggableComponent, MongoLogger, type MongoLoggerOptions, parseSeverityFromString, @@ -114,6 +115,61 @@ describe('class MongoLogger', async function () { expect(buffer).to.have.lengthOf(1); }); }); + + describe('willLog', function () { + const expectedWillLog = {}; + const errorWillLogRow = { + off: true, + emergency: true, + alert: true, + critical: true, + error: true, + warn: false, + notice: false, + info: false, + debug: false, + trace: false + }; + const debugWillLogRow = { + off: true, + emergency: true, + alert: true, + critical: true, + error: true, + warn: true, + notice: true, + info: true, + debug: true, + trace: false + }; + + for (const component of Object.values(MongoLoggableComponent)) { + context(`when component is ${component}`, function () { + it(`willLog[${component}] object should only return true for values <= its componentSeverity `, function () { + // generate component severities, set default severity to 'error' and relevant component severity to 'debug' + const componentSeverities = Object.fromEntries( + Object.entries(MongoLoggableComponent).map(([_key, value]) => + value === component ? [value, 'debug'] : [value, 'error'] + ) + ); + + // create expected willLog + for (const component2 of Object.values(MongoLoggableComponent)) { + expectedWillLog[component2] = + component2 === component ? debugWillLogRow : errorWillLogRow; + } + + const logger = new MongoLogger({ + componentSeverities: componentSeverities, + logDestination: createStdioLogger(process.stderr), + logDestinationIsStdErr: true + } as any); + + expect(logger.willLog).to.deep.equal(expectedWillLog); + }); + }); + } + }); }); describe('static #resolveOptions()', function () { From 42926403dcf0e793bf15e8c330d8891aa4397b86 Mon Sep 17 00:00:00 2001 From: Aditi Khare Date: Fri, 2 Feb 2024 15:35:50 -0500 Subject: [PATCH 03/11] unit test fix --- src/cmap/connection.ts | 2 +- test/unit/cmap/connection_pool.test.js | 9 +++++++-- test/unit/mongo_client.test.js | 5 ++++- 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/cmap/connection.ts b/src/cmap/connection.ts index e036457a586..bea7716dfb9 100644 --- a/src/cmap/connection.ts +++ b/src/cmap/connection.ts @@ -280,7 +280,7 @@ export class Connection extends TypedEventEmitter { (this.monitorCommands || (this.established && !this.authContext?.reauthenticating && - this.mongoLogger?.willLog(SeverityLevel.DEBUG, MongoLoggableComponent.COMMAND))) ?? + this.mongoLogger?.willLog[MongoLoggableComponent.COMMAND][SeverityLevel.DEBUG])) ?? false ); } diff --git a/test/unit/cmap/connection_pool.test.js b/test/unit/cmap/connection_pool.test.js index ce79870572b..234897633f2 100644 --- a/test/unit/cmap/connection_pool.test.js +++ b/test/unit/cmap/connection_pool.test.js @@ -7,7 +7,7 @@ const sinon = require('sinon'); const { expect } = require('chai'); const { setImmediate } = require('timers'); const { promisify } = require('util'); -const { ns, isHello, MongoLoggableComponent } = require('../../mongodb'); +const { ns, isHello, MongoLoggableComponent, SeverityLevel } = require('../../mongodb'); const { LEGACY_HELLO_COMMAND } = require('../../mongodb'); const { createTimerSandbox } = require('../timer_sandbox'); const { topologyWithPlaceholderClient } = require('../../tools/utils'); @@ -20,7 +20,12 @@ describe('Connection Pool', function () { mongoLogger: { debug: () => null, willLog: Object.fromEntries( - Object.values(MongoLoggableComponent).map(([value]) => [value, {}]) + Object.values(MongoLoggableComponent).map(component => [ + component, + Object.fromEntries( + Object.values(SeverityLevel).map(severityLevel => [severityLevel, false]) + ) + ]) ) } } diff --git a/test/unit/mongo_client.test.js b/test/unit/mongo_client.test.js index d325b073568..21d55093264 100644 --- a/test/unit/mongo_client.test.js +++ b/test/unit/mongo_client.test.js @@ -800,7 +800,10 @@ describe('MongoOptions', function () { context('loggingOptions', function () { const expectedLoggingObject = { maxDocumentLength: 20, - logDestination: new Writable() + logDestination: new Writable(), + componentSeverities: Object.fromEntries( + Object.values(MongoLoggableComponent).map(([value]) => [value, SeverityLevel.OFF]) + ) }; before(() => { From 3937c2e88423760ebef3d74cf6de2f4854419213 Mon Sep 17 00:00:00 2001 From: Aditi Khare Date: Fri, 2 Feb 2024 17:52:22 -0500 Subject: [PATCH 04/11] conditional logger instantiation --- src/cmap/connection_pool.ts | 2 +- src/mongo_client.ts | 18 +++++++--- src/sdam/monitor.ts | 2 +- src/sdam/topology.ts | 18 +++++----- .../node-specific/feature_flags.test.ts | 34 ++++++------------- test/unit/mongo_client.test.js | 8 ++--- 6 files changed, 38 insertions(+), 44 deletions(-) diff --git a/src/cmap/connection_pool.ts b/src/cmap/connection_pool.ts index 6071794f602..4fe5249738f 100644 --- a/src/cmap/connection_pool.ts +++ b/src/cmap/connection_pool.ts @@ -251,7 +251,7 @@ export class ConnectionPool extends TypedEventEmitter { this[kMetrics] = new ConnectionPoolMetrics(); this[kProcessingWaitQueue] = false; - this.mongoLogger = this[kServer].topology.client.mongoLogger; + this.mongoLogger = this[kServer].topology.client?.mongoLogger; this.component = 'connection'; process.nextTick(() => { diff --git a/src/mongo_client.ts b/src/mongo_client.ts index a8be513d176..15beb16ae87 100644 --- a/src/mongo_client.ts +++ b/src/mongo_client.ts @@ -25,7 +25,8 @@ import { type LogComponentSeveritiesClientOptions, type MongoDBLogWritable, MongoLogger, - type MongoLoggerOptions + type MongoLoggerOptions, + SeverityLevel } from './mongo_logger'; import { TypedEventEmitter } from './mongo_types'; import { executeOperation } from './operations/execute_operation'; @@ -345,7 +346,7 @@ export class MongoClient extends TypedEventEmitter { /** @internal */ topology?: Topology; /** @internal */ - override readonly mongoLogger: MongoLogger; + override readonly mongoLogger: MongoLogger | undefined; /** @internal */ private connectionLock?: Promise; @@ -359,7 +360,14 @@ export class MongoClient extends TypedEventEmitter { super(); this[kOptions] = parseOptions(url, this, options); - this.mongoLogger = new MongoLogger(this[kOptions].mongoLoggerOptions); + + const shouldSetLogger = + Object.values(this[kOptions].mongoLoggerOptions.componentSeverities).filter( + value => value !== SeverityLevel.OFF + ).length !== 0; + this.mongoLogger = shouldSetLogger + ? new MongoLogger(this[kOptions].mongoLoggerOptions) + : undefined; // eslint-disable-next-line @typescript-eslint/no-this-alias const client = this; @@ -405,9 +413,9 @@ export class MongoClient extends TypedEventEmitter { const srvHostIsCosmosDB = isHostMatch(COSMOS_DB_CHECK, this[kOptions].srvHost); if (documentDBHostnames.length !== 0 || srvHostIsDocumentDB) { - this.mongoLogger.info('client', DOCUMENT_DB_MSG); + this.mongoLogger?.info('client', DOCUMENT_DB_MSG); } else if (cosmosDBHostnames.length !== 0 || srvHostIsCosmosDB) { - this.mongoLogger.info('client', COSMOS_DB_MSG); + this.mongoLogger?.info('client', COSMOS_DB_MSG); } } diff --git a/src/sdam/monitor.ts b/src/sdam/monitor.ts index a85403c7461..6accf8cd3c1 100644 --- a/src/sdam/monitor.ts +++ b/src/sdam/monitor.ts @@ -120,7 +120,7 @@ export class Monitor extends TypedEventEmitter { serverMonitoringMode: options.serverMonitoringMode }); this.isRunningInFaasEnv = getFAASEnv() != null; - this.mongoLogger = this[kServer].topology.client.mongoLogger; + this.mongoLogger = this[kServer].topology.client?.mongoLogger; const cancellationToken = this[kCancellationToken]; // TODO: refactor this to pull it directly from the pool, requires new ConnectionPool integration diff --git a/src/sdam/topology.ts b/src/sdam/topology.ts index e5c318cb7b4..2526aad535b 100644 --- a/src/sdam/topology.ts +++ b/src/sdam/topology.ts @@ -102,7 +102,7 @@ export type ServerSelectionCallback = Callback; export interface ServerSelectionRequest { serverSelector: ServerSelector; topologyDescription: TopologyDescription; - mongoLogger: MongoLogger; + mongoLogger: MongoLogger | undefined; transaction?: Transaction; startTime: number; callback: ServerSelectionCallback; @@ -568,7 +568,7 @@ export class Topology extends TypedEventEmitter { } options = { serverSelectionTimeoutMS: this.s.serverSelectionTimeoutMS, ...options }; - this.client.mongoLogger.debug( + this.client.mongoLogger?.debug( MongoLoggableComponent.SERVER_SELECTION, new ServerSelectionStartedEvent(selector, this.description, options.operationName) ); @@ -578,7 +578,7 @@ export class Topology extends TypedEventEmitter { const transaction = session && session.transaction; if (isSharded && transaction && transaction.server) { - this.client.mongoLogger.debug( + this.client.mongoLogger?.debug( MongoLoggableComponent.SERVER_SELECTION, new ServerSelectionSucceededEvent( selector, @@ -611,7 +611,7 @@ export class Topology extends TypedEventEmitter { `Server selection timed out after ${options.serverSelectionTimeoutMS} ms`, this.description ); - this.client.mongoLogger.debug( + this.client.mongoLogger?.debug( MongoLoggableComponent.SERVER_SELECTION, new ServerSelectionFailedEvent( selector, @@ -896,7 +896,7 @@ function drainWaitQueue(queue: List, err?: MongoDriverEr if (!waitQueueMember[kCancelled]) { if (err) { - waitQueueMember.mongoLogger.debug( + waitQueueMember.mongoLogger?.debug( MongoLoggableComponent.SERVER_SELECTION, new ServerSelectionFailedEvent( waitQueueMember.serverSelector, @@ -943,7 +943,7 @@ function processWaitQueue(topology: Topology) { : serverDescriptions; } catch (e) { waitQueueMember.timeoutController.clear(); - topology.client.mongoLogger.debug( + topology.client.mongoLogger?.debug( MongoLoggableComponent.SERVER_SELECTION, new ServerSelectionFailedEvent( waitQueueMember.serverSelector, @@ -959,7 +959,7 @@ function processWaitQueue(topology: Topology) { let selectedServer: Server | undefined; if (selectedDescriptions.length === 0) { if (!waitQueueMember.waitingLogged) { - topology.client.mongoLogger.info( + topology.client.mongoLogger?.info( MongoLoggableComponent.SERVER_SELECTION, new WaitingForSuitableServerEvent( waitQueueMember.serverSelector, @@ -992,7 +992,7 @@ function processWaitQueue(topology: Topology) { 'server selection returned a server description but the server was not found in the topology', topology.description ); - topology.client.mongoLogger.debug( + topology.client.mongoLogger?.debug( MongoLoggableComponent.SERVER_SELECTION, new ServerSelectionFailedEvent( waitQueueMember.serverSelector, @@ -1011,7 +1011,7 @@ function processWaitQueue(topology: Topology) { waitQueueMember.timeoutController.clear(); - topology.client.mongoLogger.debug( + topology.client.mongoLogger?.debug( MongoLoggableComponent.SERVER_SELECTION, new ServerSelectionSucceededEvent( waitQueueMember.serverSelector, diff --git a/test/integration/node-specific/feature_flags.test.ts b/test/integration/node-specific/feature_flags.test.ts index cafdd5cc665..74b0fb969f3 100644 --- a/test/integration/node-specific/feature_flags.test.ts +++ b/test/integration/node-specific/feature_flags.test.ts @@ -45,6 +45,7 @@ describe('Feature Flags', () => { } }); + // TODO(NODE-5672): Release Standardized Logger describe('@@mdb.enableMongoLogger', () => { let cachedEnv; const loggerFeatureFlag = Symbol.for('@@mdb.enableMongoLogger'); @@ -74,7 +75,7 @@ describe('Feature Flags', () => { const client = new MongoClient('mongodb://localhost:27017', { [loggerFeatureFlag]: true }); - expect(client.mongoLogger.componentSeverities).to.have.property( + expect(client.mongoLogger?.componentSeverities).to.have.property( 'command', SeverityLevel.EMERGENCY ); @@ -86,16 +87,11 @@ describe('Feature Flags', () => { process.env = {}; }); - it('does not enable logging for any component', () => { + it('does not create logger', () => { const client = new MongoClient('mongodb://localhost:27017', { [loggerFeatureFlag]: true }); - for (const component of components) { - expect(client.mongoLogger.componentSeverities).to.have.property( - component, - SeverityLevel.OFF - ); - } + expect(client.mongoLogger).to.not.exist; }); }); }); @@ -107,16 +103,11 @@ describe('Feature Flags', () => { process.env['MONGODB_LOG_COMMAND'] = SeverityLevel.EMERGENCY; }); - it('does not enable logging', () => { + it('does not instantiate logger', () => { const client = new MongoClient('mongodb://localhost:27017', { [loggerFeatureFlag]: featureFlagValue }); - for (const component of components) { - expect(client.mongoLogger.componentSeverities).to.have.property( - component, - SeverityLevel.OFF - ); - } + expect(client.mongoLogger).to.not.exist; }); }); @@ -125,16 +116,11 @@ describe('Feature Flags', () => { process.env = {}; }); - it('does not enable logging', () => { + it('does not instantiate logger', () => { const client = new MongoClient('mongodb://localhost:27017', { [loggerFeatureFlag]: featureFlagValue }); - for (const component of components) { - expect(client.mongoLogger.componentSeverities).to.have.property( - component, - SeverityLevel.OFF - ); - } + expect(client.mongoLogger).to.not.exist; }); }); }); @@ -162,7 +148,7 @@ describe('Feature Flags', () => { [Symbol.for('@@mdb.internalLoggerConfig')]: undefined }); - expect(client.mongoLogger.componentSeverities).to.have.property( + expect(client.mongoLogger?.componentSeverities).to.have.property( 'command', SeverityLevel.EMERGENCY ); @@ -178,7 +164,7 @@ describe('Feature Flags', () => { } }); - expect(client.mongoLogger.componentSeverities).to.have.property( + expect(client.mongoLogger?.componentSeverities).to.have.property( 'command', SeverityLevel.ALERT ); diff --git a/test/unit/mongo_client.test.js b/test/unit/mongo_client.test.js index 21d55093264..c9c3c9923fc 100644 --- a/test/unit/mongo_client.test.js +++ b/test/unit/mongo_client.test.js @@ -1078,9 +1078,9 @@ describe('MongoOptions', function () { ).to.not.throw(MongoAPIError); const client = new MongoClient('mongodb://a/', { [loggerFeatureFlag]: true, - mongodbLogComponentSeverities: {} + mongodbLogComponentSeverities: { client: 'error' } // dummy so logger doesn't turn on }); - expect(client.mongoLogger.componentSeverities.default).to.equal('off'); + expect(client.mongoLogger?.componentSeverities.command).to.equal('off'); }); }); context('when valid client option is provided', function () { @@ -1094,9 +1094,9 @@ describe('MongoOptions', function () { ).to.not.throw(MongoAPIError); const client = new MongoClient('mongodb://a/', { [loggerFeatureFlag]: true, - mongodbLogComponentSeverities: { default: 'emergency' } + mongodbLogComponentSeverities: { command: 'emergency' } }); - expect(client.mongoLogger.componentSeverities.default).to.equal('emergency'); + expect(client.mongoLogger?.componentSeverities.command).to.equal('emergency'); }); }); }); From 0e3e4b25db14fb27b39135f8c77c1c0aa64bf393 Mon Sep 17 00:00:00 2001 From: Aditi Khare Date: Mon, 5 Feb 2024 10:46:31 -0500 Subject: [PATCH 05/11] lint fix and PR requested changes 0 --- src/mongo_logger.ts | 12 ++++++------ test/integration/node-specific/feature_flags.test.ts | 9 +-------- 2 files changed, 7 insertions(+), 14 deletions(-) diff --git a/src/mongo_logger.ts b/src/mongo_logger.ts index cd86a81fc99..aac39745d5a 100644 --- a/src/mongo_logger.ts +++ b/src/mongo_logger.ts @@ -769,12 +769,12 @@ export class MongoLogger { createWillLog() { for (const component of Object.values(MongoLoggableComponent)) { - this.willLog[component] = Object.fromEntries( - Object.entries(SeverityLevel).map(([_key, value]) => [ - value as SeverityLevel, - compareSeverity(value, this.componentSeverities[component]) <= 0 - ]) - ) as Record; + for (const severityLevel of Object.values(SeverityLevel)) { + this.willLog[component] = { + ...this.willLog[component], + [severityLevel]: compareSeverity(severityLevel, this.componentSeverities[component]) <= 0 + }; + } } } diff --git a/test/integration/node-specific/feature_flags.test.ts b/test/integration/node-specific/feature_flags.test.ts index 74b0fb969f3..6bfeef0f8e3 100644 --- a/test/integration/node-specific/feature_flags.test.ts +++ b/test/integration/node-specific/feature_flags.test.ts @@ -1,6 +1,6 @@ import { expect } from 'chai'; -import { MongoClient, type MongoLoggableComponent, SeverityLevel } from '../../mongodb'; +import { MongoClient, SeverityLevel } from '../../mongodb'; describe('Feature Flags', () => { describe('@@mdb.skipPingOnConnect', () => { @@ -49,13 +49,6 @@ describe('Feature Flags', () => { describe('@@mdb.enableMongoLogger', () => { let cachedEnv; const loggerFeatureFlag = Symbol.for('@@mdb.enableMongoLogger'); - const components: Array = [ - 'default', - 'topology', - 'serverSelection', - 'connection', - 'command' - ]; before(() => { cachedEnv = process.env; From ef0909447f6f3442cf2d9b9042db9d538b0d176c Mon Sep 17 00:00:00 2001 From: Aditi Khare Date: Mon, 5 Feb 2024 16:42:22 -0500 Subject: [PATCH 06/11] Casting fix --- src/mongo_logger.ts | 18 ++++++++---------- .../retryable_reads.spec.test.js | 2 +- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/src/mongo_logger.ts b/src/mongo_logger.ts index aac39745d5a..8d91152c872 100644 --- a/src/mongo_logger.ts +++ b/src/mongo_logger.ts @@ -728,10 +728,7 @@ export class MongoLogger { logDestination: MongoDBLogWritable; logDestinationIsStdErr: boolean; pendingLog: PromiseLike | unknown = null; - willLog: Record> = {} as Record< - MongoLoggableComponent, - Record - >; + willLog: Record>; /** * This method should be used when logging errors that do not have a public driver API for @@ -764,18 +761,19 @@ export class MongoLogger { this.maxDocumentLength = options.maxDocumentLength; this.logDestination = options.logDestination; this.logDestinationIsStdErr = options.logDestinationIsStdErr; - this.createWillLog(); + this.willLog = this.createWillLog(); } - createWillLog() { + createWillLog(): Record> { + const willLog = Object(); for (const component of Object.values(MongoLoggableComponent)) { + willLog[component] = {}; for (const severityLevel of Object.values(SeverityLevel)) { - this.willLog[component] = { - ...this.willLog[component], - [severityLevel]: compareSeverity(severityLevel, this.componentSeverities[component]) <= 0 - }; + willLog[component][severityLevel] = + compareSeverity(severityLevel, this.componentSeverities[component]) <= 0; } } + return willLog; } turnOffSeverities() { diff --git a/test/integration/retryable-reads/retryable_reads.spec.test.js b/test/integration/retryable-reads/retryable_reads.spec.test.js index 9dbe1166b17..53eb768e8b9 100644 --- a/test/integration/retryable-reads/retryable_reads.spec.test.js +++ b/test/integration/retryable-reads/retryable_reads.spec.test.js @@ -3,7 +3,7 @@ const { TestRunnerContext, generateTopologyTests } = require('../../tools/spec-r const { loadSpecTests } = require('../../spec'); const { runUnifiedSuite } = require('../../tools/unified-spec-runner/runner'); -describe('Retryable Reads (legacy)', function () { +describe.only('Retryable Reads (legacy)', function () { const testContext = new TestRunnerContext(); const testSuites = loadSpecTests(path.join('retryable-reads', 'legacy')); From f5cf09f212eb56608a4f1284c7d93e101a032f8b Mon Sep 17 00:00:00 2001 From: Aditi Khare Date: Mon, 5 Feb 2024 16:51:45 -0500 Subject: [PATCH 07/11] Remove stray only --- test/integration/retryable-reads/retryable_reads.spec.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/retryable-reads/retryable_reads.spec.test.js b/test/integration/retryable-reads/retryable_reads.spec.test.js index 53eb768e8b9..9dbe1166b17 100644 --- a/test/integration/retryable-reads/retryable_reads.spec.test.js +++ b/test/integration/retryable-reads/retryable_reads.spec.test.js @@ -3,7 +3,7 @@ const { TestRunnerContext, generateTopologyTests } = require('../../tools/spec-r const { loadSpecTests } = require('../../spec'); const { runUnifiedSuite } = require('../../tools/unified-spec-runner/runner'); -describe.only('Retryable Reads (legacy)', function () { +describe('Retryable Reads (legacy)', function () { const testContext = new TestRunnerContext(); const testSuites = loadSpecTests(path.join('retryable-reads', 'legacy')); From 2211ccf7c77d4bb8ccd4c16b05f17a45b34b81d8 Mon Sep 17 00:00:00 2001 From: Aditi Khare Date: Wed, 7 Feb 2024 14:53:35 -0500 Subject: [PATCH 08/11] partial PR requested changes: --- src/mongo_client.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/mongo_client.ts b/src/mongo_client.ts index 15beb16ae87..476a912aece 100644 --- a/src/mongo_client.ts +++ b/src/mongo_client.ts @@ -361,10 +361,9 @@ export class MongoClient extends TypedEventEmitter { this[kOptions] = parseOptions(url, this, options); - const shouldSetLogger = - Object.values(this[kOptions].mongoLoggerOptions.componentSeverities).filter( - value => value !== SeverityLevel.OFF - ).length !== 0; + const shouldSetLogger = Object.values( + this[kOptions].mongoLoggerOptions.componentSeverities + ).some(value => value !== SeverityLevel.OFF); this.mongoLogger = shouldSetLogger ? new MongoLogger(this[kOptions].mongoLoggerOptions) : undefined; From d1cd3d8e88567a9d303f6a4ebefa19b40dac91aa Mon Sep 17 00:00:00 2001 From: Aditi Khare Date: Wed, 7 Feb 2024 17:49:56 -0500 Subject: [PATCH 09/11] PR requested changes --- src/cmap/connection.ts | 2 +- src/mongo_logger.ts | 22 +++-- test/unit/cmap/connection_pool.test.js | 9 +- test/unit/mongo_logger.test.ts | 121 ++++++++++++++----------- 4 files changed, 81 insertions(+), 73 deletions(-) diff --git a/src/cmap/connection.ts b/src/cmap/connection.ts index bea7716dfb9..3406fed594b 100644 --- a/src/cmap/connection.ts +++ b/src/cmap/connection.ts @@ -280,7 +280,7 @@ export class Connection extends TypedEventEmitter { (this.monitorCommands || (this.established && !this.authContext?.reauthenticating && - this.mongoLogger?.willLog[MongoLoggableComponent.COMMAND][SeverityLevel.DEBUG])) ?? + this.mongoLogger?.willLog(MongoLoggableComponent.COMMAND, SeverityLevel.DEBUG))) ?? false ); } diff --git a/src/mongo_logger.ts b/src/mongo_logger.ts index 8d91152c872..a6548acaaf9 100644 --- a/src/mongo_logger.ts +++ b/src/mongo_logger.ts @@ -728,7 +728,7 @@ export class MongoLogger { logDestination: MongoDBLogWritable; logDestinationIsStdErr: boolean; pendingLog: PromiseLike | unknown = null; - willLog: Record>; + private severities: Record>; /** * This method should be used when logging errors that do not have a public driver API for @@ -761,26 +761,26 @@ export class MongoLogger { this.maxDocumentLength = options.maxDocumentLength; this.logDestination = options.logDestination; this.logDestinationIsStdErr = options.logDestinationIsStdErr; - this.willLog = this.createWillLog(); + this.severities = this.createLoggingSeverities(); } - createWillLog(): Record> { - const willLog = Object(); + createLoggingSeverities(): Record> { + const severities = Object(); for (const component of Object.values(MongoLoggableComponent)) { - willLog[component] = {}; + severities[component] = {}; for (const severityLevel of Object.values(SeverityLevel)) { - willLog[component][severityLevel] = + severities[component][severityLevel] = compareSeverity(severityLevel, this.componentSeverities[component]) <= 0; } } - return willLog; + return severities; } turnOffSeverities() { for (const component of Object.values(MongoLoggableComponent)) { this.componentSeverities[component] = SeverityLevel.OFF; for (const severityLevel of Object.values(SeverityLevel)) { - this.willLog[component][severityLevel] = false; + this.severities[component][severityLevel] = false; } } } @@ -810,12 +810,16 @@ export class MongoLogger { this.pendingLog = null; } + willLog(component: MongoLoggableComponent, severity: SeverityLevel): boolean { + return this.severities[component][severity]; + } + private log( severity: SeverityLevel, component: MongoLoggableComponent, message: Loggable | string ): void { - if (!this.willLog[component][severity]) return; + if (!this.willLog(component, severity)) return; let logMessage: Log = { t: new Date(), c: component, s: severity }; if (typeof message === 'string') { diff --git a/test/unit/cmap/connection_pool.test.js b/test/unit/cmap/connection_pool.test.js index 234897633f2..03348d65ff0 100644 --- a/test/unit/cmap/connection_pool.test.js +++ b/test/unit/cmap/connection_pool.test.js @@ -19,14 +19,7 @@ describe('Connection Pool', function () { client: { mongoLogger: { debug: () => null, - willLog: Object.fromEntries( - Object.values(MongoLoggableComponent).map(component => [ - component, - Object.fromEntries( - Object.values(SeverityLevel).map(severityLevel => [severityLevel, false]) - ) - ]) - ) + willLog: () => null } } } diff --git a/test/unit/mongo_logger.test.ts b/test/unit/mongo_logger.test.ts index ef802e4123b..988591733dd 100644 --- a/test/unit/mongo_logger.test.ts +++ b/test/unit/mongo_logger.test.ts @@ -115,61 +115,6 @@ describe('class MongoLogger', async function () { expect(buffer).to.have.lengthOf(1); }); }); - - describe('willLog', function () { - const expectedWillLog = {}; - const errorWillLogRow = { - off: true, - emergency: true, - alert: true, - critical: true, - error: true, - warn: false, - notice: false, - info: false, - debug: false, - trace: false - }; - const debugWillLogRow = { - off: true, - emergency: true, - alert: true, - critical: true, - error: true, - warn: true, - notice: true, - info: true, - debug: true, - trace: false - }; - - for (const component of Object.values(MongoLoggableComponent)) { - context(`when component is ${component}`, function () { - it(`willLog[${component}] object should only return true for values <= its componentSeverity `, function () { - // generate component severities, set default severity to 'error' and relevant component severity to 'debug' - const componentSeverities = Object.fromEntries( - Object.entries(MongoLoggableComponent).map(([_key, value]) => - value === component ? [value, 'debug'] : [value, 'error'] - ) - ); - - // create expected willLog - for (const component2 of Object.values(MongoLoggableComponent)) { - expectedWillLog[component2] = - component2 === component ? debugWillLogRow : errorWillLogRow; - } - - const logger = new MongoLogger({ - componentSeverities: componentSeverities, - logDestination: createStdioLogger(process.stderr), - logDestinationIsStdErr: true - } as any); - - expect(logger.willLog).to.deep.equal(expectedWillLog); - }); - }); - } - }); }); describe('static #resolveOptions()', function () { @@ -1604,4 +1549,70 @@ describe('class MongoLogger', async function () { }); }); }); + + describe('willLog', function () { + const expectedWillLogResults = {}; + const errorWillLogRow = { + off: true, + emergency: true, + alert: true, + critical: true, + error: true, + warn: false, + notice: false, + info: false, + debug: false, + trace: false + }; + const debugWillLogRow = { + off: true, + emergency: true, + alert: true, + critical: true, + error: true, + warn: true, + notice: true, + info: true, + debug: true, + trace: false + }; + + let logger: MongoLogger; + + for (const component of Object.values(MongoLoggableComponent)) { + context(`when component is ${component}`, function () { + beforeEach(function () { + // create expectedWillLogResults + for (const component2 of Object.values(MongoLoggableComponent)) { + expectedWillLogResults[component2] = + component2 === component ? debugWillLogRow : errorWillLogRow; + } + + // generate component severities, set default severity to 'error' and relevant component severity to 'debug' + const componentSeverities = Object.fromEntries( + Object.entries(MongoLoggableComponent).map(([_key, value]) => + value === component ? [value, 'debug'] : [value, 'error'] + ) + ); + + // create logger with relevant component severities + logger = new MongoLogger({ + componentSeverities: componentSeverities, + logDestination: createStdioLogger(process.stderr), + logDestinationIsStdErr: true + } as any); + }); + + it(`willLog method should only return true for values <= its componentSeverity`, function () { + for (const component3 of Object.values(MongoLoggableComponent)) { + for (const severityLevel of Object.values(SeverityLevel)) { + expect(logger.willLog(component3, severityLevel)).to.equal( + expectedWillLogResults[component3][severityLevel] + ); + } + } + }); + }); + } + }); }); From 36a3de86f095e00cd07853e3deb8ee65ef0ecccf Mon Sep 17 00:00:00 2001 From: aditi-khare-mongoDB <106987683+aditi-khare-mongoDB@users.noreply.github.com> Date: Thu, 8 Feb 2024 13:52:48 -0500 Subject: [PATCH 10/11] Durran's Test Refactor Suggestion Co-authored-by: Durran Jordan --- test/unit/mongo_logger.test.ts | 118 +++++++++++++++++---------------- 1 file changed, 60 insertions(+), 58 deletions(-) diff --git a/test/unit/mongo_logger.test.ts b/test/unit/mongo_logger.test.ts index 988591733dd..0bc080ae278 100644 --- a/test/unit/mongo_logger.test.ts +++ b/test/unit/mongo_logger.test.ts @@ -1550,69 +1550,71 @@ describe('class MongoLogger', async function () { }); }); - describe('willLog', function () { - const expectedWillLogResults = {}; - const errorWillLogRow = { - off: true, - emergency: true, - alert: true, - critical: true, - error: true, - warn: false, - notice: false, - info: false, - debug: false, - trace: false - }; - const debugWillLogRow = { - off: true, - emergency: true, - alert: true, - critical: true, - error: true, - warn: true, - notice: true, - info: true, - debug: true, - trace: false - }; - - let logger: MongoLogger; - - for (const component of Object.values(MongoLoggableComponent)) { - context(`when component is ${component}`, function () { - beforeEach(function () { - // create expectedWillLogResults - for (const component2 of Object.values(MongoLoggableComponent)) { - expectedWillLogResults[component2] = - component2 === component ? debugWillLogRow : errorWillLogRow; - } + describe('#willLog', function () { + const severityLevels = Object.values(SeverityLevel); + for (const severityLevel of severityLevels) { + context(`when the severity level is ${severityLevel}`, function () { + let logger: MongoLogger; + let componentSeverities; + const components = Object.values(MongoLoggableComponent); + + for (const component of components) { + context(`when ${component} severity level <= ${severityLevel}`, function () { + beforeEach(function () { + const index = severityLevels.indexOf(severityLevel); + componentSeverities = components.reduce((severities, value) => { + component === value + ? (severities[component] = severityLevel) + : (severities[value] = + severityLevels[index + 1] === 'off' + ? severityLevel + : severityLevels[index + 1]); + return severities; + }, {}); + logger = new MongoLogger({ + componentSeverities: componentSeverities, + logDestination: createStdioLogger(process.stderr), + logDestinationIsStdErr: true + } as any); + }); - // generate component severities, set default severity to 'error' and relevant component severity to 'debug' - const componentSeverities = Object.fromEntries( - Object.entries(MongoLoggableComponent).map(([_key, value]) => - value === component ? [value, 'debug'] : [value, 'error'] - ) - ); + if (severityLevel === 'off') { + it('returns false always for off', function () { + expect(logger.willLog(component, severityLevel)).to.be.false; + }); + } else { + it('returns true', function () { + expect(logger.willLog(component, severityLevel)).to.be.true; + }); + } + }); - // create logger with relevant component severities - logger = new MongoLogger({ - componentSeverities: componentSeverities, - logDestination: createStdioLogger(process.stderr), - logDestinationIsStdErr: true - } as any); - }); + context(`when ${component} severity level > ${severityLevel}`, function () { + if (severityLevel !== 'emergency') { + beforeEach(function () { + const index = severityLevels.indexOf(severityLevel); + componentSeverities = components.reduce((severities, value) => { + component === value + ? (severities[component] = severityLevels[index - 1]) + : (severities[value] = severityLevel); + return severities; + }, {}); + logger = new MongoLogger({ + componentSeverities: componentSeverities, + logDestination: createStdioLogger(process.stderr), + logDestinationIsStdErr: true + } as any); + }); - it(`willLog method should only return true for values <= its componentSeverity`, function () { - for (const component3 of Object.values(MongoLoggableComponent)) { - for (const severityLevel of Object.values(SeverityLevel)) { - expect(logger.willLog(component3, severityLevel)).to.equal( - expectedWillLogResults[component3][severityLevel] - ); + it('returns false', function () { + expect(logger.willLog(component, severityLevel)).to.be.false; + }); } - } - }); + }); + } }); } }); }); + +}); From c4a52f4064a5bc07c261d3a79b8321d1a022e8f3 Mon Sep 17 00:00:00 2001 From: Aditi Khare Date: Thu, 8 Feb 2024 14:57:16 -0500 Subject: [PATCH 11/11] PR requested changes --- src/mongo_logger.ts | 1 + test/unit/cmap/connection_pool.test.js | 2 +- test/unit/mongo_logger.test.ts | 2 -- 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/mongo_logger.ts b/src/mongo_logger.ts index a6548acaaf9..27fcbf8d308 100644 --- a/src/mongo_logger.ts +++ b/src/mongo_logger.ts @@ -811,6 +811,7 @@ export class MongoLogger { } willLog(component: MongoLoggableComponent, severity: SeverityLevel): boolean { + if (severity === SeverityLevel.OFF) return false; return this.severities[component][severity]; } diff --git a/test/unit/cmap/connection_pool.test.js b/test/unit/cmap/connection_pool.test.js index 03348d65ff0..cdbf00bf67f 100644 --- a/test/unit/cmap/connection_pool.test.js +++ b/test/unit/cmap/connection_pool.test.js @@ -7,7 +7,7 @@ const sinon = require('sinon'); const { expect } = require('chai'); const { setImmediate } = require('timers'); const { promisify } = require('util'); -const { ns, isHello, MongoLoggableComponent, SeverityLevel } = require('../../mongodb'); +const { ns, isHello } = require('../../mongodb'); const { LEGACY_HELLO_COMMAND } = require('../../mongodb'); const { createTimerSandbox } = require('../timer_sandbox'); const { topologyWithPlaceholderClient } = require('../../tools/utils'); diff --git a/test/unit/mongo_logger.test.ts b/test/unit/mongo_logger.test.ts index 0bc080ae278..2e9cecf66d6 100644 --- a/test/unit/mongo_logger.test.ts +++ b/test/unit/mongo_logger.test.ts @@ -1616,5 +1616,3 @@ describe('class MongoLogger', async function () { } }); }); - -});