From c2324c9449241d89ea3f1b2d0a66ff4868625a0f Mon Sep 17 00:00:00 2001 From: Warren James Date: Fri, 14 Apr 2023 16:19:00 -0400 Subject: [PATCH 1/8] feat(NODE-4814): Add severity loggers and unskip tests --- src/mongo_logger.ts | 20 ++++++++++++++++++-- test/unit/mongo_logger.test.ts | 3 +-- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/src/mongo_logger.ts b/src/mongo_logger.ts index 40102ed550..4ed4bc49e8 100644 --- a/src/mongo_logger.ts +++ b/src/mongo_logger.ts @@ -430,14 +430,30 @@ export class MongoLogger { maxDocumentLength: number; logDestination: MongoDBLogWritable | Writable; + emergency = this.log.bind(this, 'emergency'); + /** @experimental */ + alert = this.log.bind(this, 'alert'); + /** @experimental */ + critical = this.log.bind(this, 'critical'); + /** @experimental */ + error = this.log.bind(this, 'error'); + /** @experimental */ + warning = this.log.bind(this, 'warn'); + /** @experimental */ + notice = this.log.bind(this, 'notice'); + /** @experimental */ + info = this.log.bind(this, 'info'); + /** @experimental */ + debug = this.log.bind(this, 'debug'); + /** @experimental */ + trace = this.log.bind(this, 'trace'); + constructor(options: MongoLoggerOptions) { this.componentSeverities = options.componentSeverities; this.maxDocumentLength = options.maxDocumentLength; this.logDestination = options.logDestination; } - emergency = this.log.bind(this, 'emergency'); - private log( severity: SeverityLevel, component: MongoLoggableComponent, diff --git a/test/unit/mongo_logger.test.ts b/test/unit/mongo_logger.test.ts index 9f9ca46826..be69360a83 100644 --- a/test/unit/mongo_logger.test.ts +++ b/test/unit/mongo_logger.test.ts @@ -648,8 +648,7 @@ describe('class MongoLogger', function () { expect(stream.buffer).to.have.lengthOf(0); }); - // TODO(NODE-4814): Unskip this test - context.skip('when the log severity is greater than what was configured', function () { + context('when the log severity is greater than what was configured', function () { it('does not write to logDestination', function () { const stream = new BufferingStream(); const logger = new MongoLogger({ From f745a8c852fd18d29920d83fc06e8c27d2738aab Mon Sep 17 00:00:00 2001 From: Warren James Date: Mon, 17 Apr 2023 11:25:02 -0400 Subject: [PATCH 2/8] refactor(NODE-4814): remove unneeded loggers --- src/mongo_logger.ts | 14 +------ test/unit/mongo_logger.test.ts | 77 +++++++++++++++------------------- 2 files changed, 34 insertions(+), 57 deletions(-) diff --git a/src/mongo_logger.ts b/src/mongo_logger.ts index 4ed4bc49e8..a5f35ca3e3 100644 --- a/src/mongo_logger.ts +++ b/src/mongo_logger.ts @@ -430,22 +430,10 @@ export class MongoLogger { maxDocumentLength: number; logDestination: MongoDBLogWritable | Writable; - emergency = this.log.bind(this, 'emergency'); - /** @experimental */ - alert = this.log.bind(this, 'alert'); - /** @experimental */ - critical = this.log.bind(this, 'critical'); - /** @experimental */ error = this.log.bind(this, 'error'); - /** @experimental */ - warning = this.log.bind(this, 'warn'); - /** @experimental */ - notice = this.log.bind(this, 'notice'); - /** @experimental */ + warn = this.log.bind(this, 'warn'); info = this.log.bind(this, 'info'); - /** @experimental */ debug = this.log.bind(this, 'debug'); - /** @experimental */ trace = this.log.bind(this, 'trace'); constructor(options: MongoLoggerOptions) { diff --git a/test/unit/mongo_logger.test.ts b/test/unit/mongo_logger.test.ts index be69360a83..439609cb47 100644 --- a/test/unit/mongo_logger.test.ts +++ b/test/unit/mongo_logger.test.ts @@ -23,7 +23,6 @@ import { MongoDBLogWritable, MongoLogger, MongoLoggerOptions, - SEVERITY_LEVEL_MAP, SeverityLevel } from '../mongodb'; @@ -79,11 +78,11 @@ describe('class MongoLogger', function () { } } as { buffer: any[]; write: (log: Log) => void }; const logger = new MongoLogger({ - componentSeverities: { command: 'emergency' } as any, + componentSeverities: { command: 'error' } as any, logDestination } as any); - logger.emergency('command', 'Hello world!'); + logger.error('command', 'Hello world!'); expect(logDestination.buffer).to.have.lengthOf(1); }); }); @@ -99,11 +98,11 @@ describe('class MongoLogger', function () { }); const logger = new MongoLogger({ - componentSeverities: { command: 'emergency' } as any, + componentSeverities: { command: 'error' } as any, logDestination } as any); - logger.emergency('command', 'Hello world!'); + logger.error('command', 'Hello world!'); expect(buffer).to.have.lengthOf(1); }); }); @@ -119,8 +118,8 @@ describe('class MongoLogger', function () { ]); function* makeValidOptions(): Generator<[string, string]> { - const validOptions = Object.values(SeverityLevel).filter( - option => option !== SeverityLevel.OFF + const validOptions = Object.values(SeverityLevel).filter(option => + ['error', 'warn', 'info', 'debug', 'trace'].includes(option) ); for (const option of validOptions) { yield [option, option]; @@ -427,11 +426,11 @@ describe('class MongoLogger', function () { const options = MongoLogger.resolveOptions( { MONGODB_LOG_PATH: unsetEnvironmentOption, - MONGODB_LOG_COMMAND: 'emergency' + MONGODB_LOG_COMMAND: 'error' }, { mongodbLogPath: unsetOption as any } ); - const log: Log = { t: new Date(), c: 'command', s: 'emergency' }; + const log: Log = { t: new Date(), c: 'command', s: 'error' }; options.logDestination.write(log); expect(stderrStub.write).to.have.been.calledOnceWith( @@ -449,11 +448,11 @@ describe('class MongoLogger', function () { const options = MongoLogger.resolveOptions( { MONGODB_LOG_PATH: unsetEnvironmentOption, - MONGODB_LOG_COMMAND: 'emergency' + MONGODB_LOG_COMMAND: 'error' }, { mongodbLogPath: invalidOption as any } ); - const log: Log = { t: new Date(), c: 'command', s: 'emergency' }; + const log: Log = { t: new Date(), c: 'command', s: 'error' }; options.logDestination.write(log); expect(stderrStub.write).to.have.been.calledOnceWith( @@ -471,12 +470,12 @@ describe('class MongoLogger', function () { const options = MongoLogger.resolveOptions( { MONGODB_LOG_PATH: unsetEnvironmentOption, - MONGODB_LOG_COMMAND: 'emergency' + MONGODB_LOG_COMMAND: 'error' }, { mongodbLogPath: validOption as any } ); - const log: Log = { t: new Date(), c: 'command', s: 'emergency' }; + const log: Log = { t: new Date(), c: 'command', s: 'error' }; options.logDestination.write(log); const correctDestination = validOptions.get(validOption); expect(correctDestination?.write).to.have.been.calledOnce; @@ -496,11 +495,11 @@ describe('class MongoLogger', function () { const options = MongoLogger.resolveOptions( { MONGODB_LOG_PATH: invalidEnvironmentOption, - MONGODB_LOG_COMMAND: 'emergency' + MONGODB_LOG_COMMAND: 'error' }, { mongodbLogPath: unsetClientOption as any } ); - const log: Log = { t: new Date(), c: 'command', s: 'emergency' }; + const log: Log = { t: new Date(), c: 'command', s: 'error' }; options.logDestination.write(log); expect(stderrStub.write).to.have.been.calledOnceWith( @@ -520,11 +519,11 @@ describe('class MongoLogger', function () { const options = MongoLogger.resolveOptions( { MONGODB_LOG_PATH: invalidEnvironmentOption, - MONGODB_LOG_COMMAND: 'emergency' + MONGODB_LOG_COMMAND: 'error' }, { mongodbLogPath: invalidOption as any } ); - const log: Log = { t: new Date(), c: 'command', s: 'emergency' }; + const log: Log = { t: new Date(), c: 'command', s: 'error' }; options.logDestination.write(log); expect(stderrStub.write).to.have.been.calledOnceWith( @@ -543,12 +542,12 @@ describe('class MongoLogger', function () { const options = MongoLogger.resolveOptions( { MONGODB_LOG_PATH: invalidEnvironmentOption, - MONGODB_LOG_COMMAND: 'emergency' + MONGODB_LOG_COMMAND: 'error' }, { mongodbLogPath: validOption as any } ); const correctDestination = validOptions.get(validOption); - const log: Log = { t: new Date(), c: 'command', s: 'emergency' }; + const log: Log = { t: new Date(), c: 'command', s: 'error' }; options.logDestination.write(log); expect(correctDestination?.write).to.have.been.calledOnce; }); @@ -566,12 +565,12 @@ describe('class MongoLogger', function () { const options = MongoLogger.resolveOptions( { MONGODB_LOG_PATH: validEnvironmentOption, - MONGODB_LOG_COMMAND: 'emergency' + MONGODB_LOG_COMMAND: 'error' }, { mongodbLogPath: unsetOption as any } ); const correctDestination = validOptions.get(validEnvironmentOption); - options.logDestination.write({ t: new Date(), c: 'command', s: 'emergency' }); + options.logDestination.write({ t: new Date(), c: 'command', s: 'error' }); expect(correctDestination?.write).to.have.been.calledOnce; }); @@ -588,13 +587,13 @@ describe('class MongoLogger', function () { const options = MongoLogger.resolveOptions( { MONGODB_LOG_PATH: validEnvironmentOption, - MONGODB_LOG_COMMAND: 'emergency' + MONGODB_LOG_COMMAND: 'error' }, { mongodbLogPath: invalidValue as any } ); const correctDestination = validOptions.get(validEnvironmentOption); - const log: Log = { t: new Date(), c: 'command', s: 'emergency' }; + const log: Log = { t: new Date(), c: 'command', s: 'error' }; options.logDestination.write(log); expect(correctDestination?.write).to.have.been.calledOnce; @@ -615,12 +614,12 @@ describe('class MongoLogger', function () { const options = MongoLogger.resolveOptions( { MONGODB_LOG_PATH: validEnvironmentOption, - MONGODB_LOG_COMMAND: 'emergency' + MONGODB_LOG_COMMAND: 'error' }, { mongodbLogPath: validValue as any } ); const correctDestination = validOptions.get(validValue); - options.logDestination.write({ t: new Date(), c: 'command', s: 'emergency' }); + options.logDestination.write({ t: new Date(), c: 'command', s: 'error' }); expect(correctDestination?.write).to.have.been.calledOnce; }); } @@ -632,8 +631,10 @@ describe('class MongoLogger', function () { describe('severity helpers', function () { // TODO(NODE-4814): Ensure we test on all valid severity levels - const severities = Object.values(SeverityLevel).filter(severity => severity === 'emergency'); - for (const severityLevel of severities) { + const severities = Object.values(SeverityLevel).filter(severity => + ['error', 'warn', 'info', 'debug', 'trace'].includes(severity) + ); + for (const [index, severityLevel] of severities.entries()) { describe(`${severityLevel}()`, function () { it('does not log when logging for the component is disabled', () => { const stream = new BufferingStream(); @@ -658,13 +659,8 @@ describe('class MongoLogger', function () { logDestination: stream } as any); - const TRACE = 8; - for ( - let l = SEVERITY_LEVEL_MAP.getNumericSeverityLevel(severityLevel) + 1; - l <= TRACE; - l++ - ) { - const severity = SEVERITY_LEVEL_MAP.getSeverityLevelName(l); + for (let i = index + 1; i < severities.length; i++) { + const severity = severities[i]; logger[severity as SeverityLevel]('command', 'Hello'); } @@ -682,20 +678,13 @@ describe('class MongoLogger', function () { logDestination: stream } as any); - const EMERGENCY = 0; // Calls all severity logging methods with a level less than or equal to what severityLevel - for ( - let l = SEVERITY_LEVEL_MAP.getNumericSeverityLevel(severityLevel); - l >= EMERGENCY; - l-- - ) { - const severity = SEVERITY_LEVEL_MAP.getSeverityLevelName(l); + for (let i = index; i >= 0; i--) { + const severity = severities[i]; logger[severity as SeverityLevel]('command', 'Hello'); } - expect(stream.buffer).to.have.lengthOf( - SEVERITY_LEVEL_MAP.getNumericSeverityLevel(severityLevel) + 1 - ); + expect(stream.buffer).to.have.lengthOf(index + 1); }); }); From 33c5a0436a2cd3d296d465486c3344c7f052f70a Mon Sep 17 00:00:00 2001 From: Warren James Date: Mon, 17 Apr 2023 11:26:58 -0400 Subject: [PATCH 3/8] test(NODE-4814): remove todos --- test/unit/mongo_logger.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/test/unit/mongo_logger.test.ts b/test/unit/mongo_logger.test.ts index 439609cb47..9bd12fe2af 100644 --- a/test/unit/mongo_logger.test.ts +++ b/test/unit/mongo_logger.test.ts @@ -630,7 +630,6 @@ describe('class MongoLogger', function () { }); describe('severity helpers', function () { - // TODO(NODE-4814): Ensure we test on all valid severity levels const severities = Object.values(SeverityLevel).filter(severity => ['error', 'warn', 'info', 'debug', 'trace'].includes(severity) ); From 4bcb802b8e691bc7eebdf0ba355daba2ce0320ee Mon Sep 17 00:00:00 2001 From: Warren James Date: Mon, 17 Apr 2023 15:19:11 -0400 Subject: [PATCH 4/8] test(NODE-4814): type narrowing --- test/unit/mongo_logger.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/unit/mongo_logger.test.ts b/test/unit/mongo_logger.test.ts index 9bd12fe2af..b30dd8f3ef 100644 --- a/test/unit/mongo_logger.test.ts +++ b/test/unit/mongo_logger.test.ts @@ -630,7 +630,7 @@ describe('class MongoLogger', function () { }); describe('severity helpers', function () { - const severities = Object.values(SeverityLevel).filter(severity => + const severities: SeverityLevel[] = Object.values(SeverityLevel).filter(severity => ['error', 'warn', 'info', 'debug', 'trace'].includes(severity) ); for (const [index, severityLevel] of severities.entries()) { @@ -660,7 +660,7 @@ describe('class MongoLogger', function () { for (let i = index + 1; i < severities.length; i++) { const severity = severities[i]; - logger[severity as SeverityLevel]('command', 'Hello'); + logger[severity]('command', 'Hello'); } expect(stream.buffer).to.have.lengthOf(0); @@ -680,7 +680,7 @@ describe('class MongoLogger', function () { // Calls all severity logging methods with a level less than or equal to what severityLevel for (let i = index; i >= 0; i--) { const severity = severities[i]; - logger[severity as SeverityLevel]('command', 'Hello'); + logger[severity]('command', 'Hello'); } expect(stream.buffer).to.have.lengthOf(index + 1); From 2ca6b3940962e989942a7ee8afa8112620c0bac8 Mon Sep 17 00:00:00 2001 From: Warren James Date: Tue, 18 Apr 2023 14:47:46 -0400 Subject: [PATCH 5/8] docs(NODE-4814): add tsdoc comments --- src/mongo_logger.ts | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/mongo_logger.ts b/src/mongo_logger.ts index a5f35ca3e3..d838de8f18 100644 --- a/src/mongo_logger.ts +++ b/src/mongo_logger.ts @@ -430,10 +430,30 @@ export class MongoLogger { maxDocumentLength: number; logDestination: MongoDBLogWritable | Writable; + /** + * This method should be used when logging errors that do not have a public driver API for + * reporting errors. + **/ error = this.log.bind(this, 'error'); + /** + * This method should be used to log situations where undesirable application behaviour might + * occur. E.g.: encountering an unrecognized option in a connection string + **/ warn = this.log.bind(this, 'warn'); + /** + * This method should be used to report high-level information about normal driver behaviour. + * E.g.: the creation of a `MongoClient` + **/ info = this.log.bind(this, 'info'); + /** + * This method should be used to report information that would be helpful when debugging an + * application. E.g.: a command starting, succeeding or failing + * */ debug = this.log.bind(this, 'debug'); + /** + * This method should be used to report fine-grained details related to logic flow. E.g.: + * entering and exiting a function body + * */ trace = this.log.bind(this, 'trace'); constructor(options: MongoLoggerOptions) { From 8f6f8c8951ef24c8e480626daa202c523d6a2c74 Mon Sep 17 00:00:00 2001 From: Warren James Date: Tue, 18 Apr 2023 15:40:39 -0400 Subject: [PATCH 6/8] docs(NODE-4814): Update doc comments --- src/mongo_logger.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/mongo_logger.ts b/src/mongo_logger.ts index d838de8f18..c257f67769 100644 --- a/src/mongo_logger.ts +++ b/src/mongo_logger.ts @@ -437,21 +437,21 @@ export class MongoLogger { error = this.log.bind(this, 'error'); /** * This method should be used to log situations where undesirable application behaviour might - * occur. E.g.: encountering an unrecognized option in a connection string + * occur. For example, failing to end sessions on `MongoClient.close` **/ warn = this.log.bind(this, 'warn'); /** * This method should be used to report high-level information about normal driver behaviour. - * E.g.: the creation of a `MongoClient` + * For example, the creation of a `MongoClient` **/ info = this.log.bind(this, 'info'); /** * This method should be used to report information that would be helpful when debugging an - * application. E.g.: a command starting, succeeding or failing + * application. For example, a command starting, succeeding or failing * */ debug = this.log.bind(this, 'debug'); /** - * This method should be used to report fine-grained details related to logic flow. E.g.: + * This method should be used to report fine-grained details related to logic flow. For example, * entering and exiting a function body * */ trace = this.log.bind(this, 'trace'); From e3c363faee588d5e6189c1a5ae25a29bae15d1d3 Mon Sep 17 00:00:00 2001 From: Warren James Date: Tue, 18 Apr 2023 15:42:50 -0400 Subject: [PATCH 7/8] docs(NODE-4814): Update doc comments --- src/mongo_logger.ts | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/mongo_logger.ts b/src/mongo_logger.ts index c257f67769..fa27cf47ca 100644 --- a/src/mongo_logger.ts +++ b/src/mongo_logger.ts @@ -433,27 +433,27 @@ export class MongoLogger { /** * This method should be used when logging errors that do not have a public driver API for * reporting errors. - **/ + */ error = this.log.bind(this, 'error'); /** * This method should be used to log situations where undesirable application behaviour might - * occur. For example, failing to end sessions on `MongoClient.close` - **/ + * occur. For example, failing to end sessions on `MongoClient.close`. + */ warn = this.log.bind(this, 'warn'); /** * This method should be used to report high-level information about normal driver behaviour. - * For example, the creation of a `MongoClient` - **/ + * For example, the creation of a `MongoClient`. + */ info = this.log.bind(this, 'info'); /** * This method should be used to report information that would be helpful when debugging an - * application. For example, a command starting, succeeding or failing - * */ + * application. For example, a command starting, succeeding or failing. + */ debug = this.log.bind(this, 'debug'); /** * This method should be used to report fine-grained details related to logic flow. For example, - * entering and exiting a function body - * */ + * entering and exiting a function body. + */ trace = this.log.bind(this, 'trace'); constructor(options: MongoLoggerOptions) { From 170d70aa3e205fd745f7b00307061a4897431e11 Mon Sep 17 00:00:00 2001 From: Warren James Date: Wed, 19 Apr 2023 11:19:40 -0400 Subject: [PATCH 8/8] style(NODE-4814): eslint --- test/unit/mongo_logger.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/test/unit/mongo_logger.test.ts b/test/unit/mongo_logger.test.ts index 172f984b1b..cab88c7949 100644 --- a/test/unit/mongo_logger.test.ts +++ b/test/unit/mongo_logger.test.ts @@ -24,7 +24,6 @@ import { MongoDBLogWritable, MongoLogger, MongoLoggerOptions, - SEVERITY_LEVEL_MAP, SeverityLevel, stringifyWithMaxLen } from '../mongodb';