From 9eb25b138a3969fd5d52fbe70724cec6af3affc5 Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Sun, 20 Nov 2022 22:29:45 -0500 Subject: [PATCH 01/50] feat: finish Logger env options parsing --- src/connection_string.ts | 47 ++++++++++++++++++++ src/logging_api.ts | 95 ++++++++++++++++++++++++++++++++++++++++ src/mongo_client.ts | 8 +++- src/sandbox.ts | 54 +++++++++++++++++++++++ 4 files changed, 203 insertions(+), 1 deletion(-) create mode 100644 src/logging_api.ts create mode 100644 src/sandbox.ts diff --git a/src/connection_string.ts b/src/connection_string.ts index 30d23a6df9..ab99e521bf 100644 --- a/src/connection_string.ts +++ b/src/connection_string.ts @@ -1,6 +1,7 @@ import * as dns from 'dns'; import * as fs from 'fs'; import ConnectionString from 'mongodb-connection-string-url'; +import type { WritableStream } from 'stream/web'; import { URLSearchParams } from 'url'; import type { Document } from './bson'; @@ -15,6 +16,11 @@ import { MongoParseError } from './error'; import { Logger, LoggerLevel } from './logger'; +import { + Logger as LoggingApi, + LoggerOptions as LoggingApiOptions, + SeverityLevel +} from './logging_api'; import { DriverInfo, MongoClient, @@ -507,6 +513,36 @@ export function parseOptions( ); } + mongoOptions.loggingApiOptions = { + MONGODB_LOG_COMMAND: getSeverityForComponent(process.env.MONGODB_LOG_COMMAND ?? ''), + MONGODB_LOG_TOPOLOGY: getSeverityForComponent(process.env.MONGODB_LOG_TOPOLOGY ?? ''), + MONGODB_LOG_SERVER_SELECTION: getSeverityForComponent( + process.env.MONGODB_LOG_SERVER_SELECTION ?? '' + ), + MONGODB_LOG_CONNECTION: getSeverityForComponent(process.env.MONGODB_LOG_CONNECTION ?? ''), + MONGODB_LOG_ALL: getSeverityForComponent(process.env.MONGODB_LOG_ALL ?? ''), + MONGODB_LOG_MAX_DOCUMENT_LENGTH: + typeof process.env.MONGODB_LOG_MAX_DOCUMENT_LENGTH === 'string' + ? Number.parseInt(process.env.MONGODB_LOG_MAX_DOCUMENT_LENGTH) + : 1000, + MONGODB_LOG_PATH: + typeof process.env.MONGODB_LOG_PATH === 'string' + ? process.env.MONGODB_LOG_PATH + : mongoOptions.mongodbLogPath ?? 'stderr' + }; + + const loggingComponents = [ + mongoOptions.loggingApiOptions.MONGODB_LOG_COMMAND, + mongoOptions.loggingApiOptions.MONGODB_LOG_TOPOLOGY, + mongoOptions.loggingApiOptions.MONGODB_LOG_SERVER_SELECTION, + mongoOptions.loggingApiOptions.MONGODB_LOG_CONNECTION, + mongoOptions.loggingApiOptions.MONGODB_LOG_ALL + ]; + + if (loggingComponents.some(element => typeof element === 'string')) { + mongoOptions.loggingApi = new LoggingApi(mongoOptions.loggingApiOptions); + } + return mongoOptions; } @@ -592,6 +628,11 @@ function setOption( } } +function getSeverityForComponent(rawComponent: string): SeverityLevel | undefined { + const validSeverities = Object.values(SeverityLevel); + return validSeverities.find(severity => severity === rawComponent); +} + interface OptionDescriptor { target?: string; type?: 'boolean' | 'int' | 'uint' | 'record' | 'string' | 'any'; @@ -866,6 +907,12 @@ export const OPTIONS = { return new Logger('MongoClient', { loggerLevel: value as LoggerLevel }); } }, + loggingApi: { + target: 'loggingApi' + }, + loggingApiOptions: { + target: 'loggingApi' + }, maxConnecting: { default: 2, transform({ name, values: [value] }): number { diff --git a/src/logging_api.ts b/src/logging_api.ts new file mode 100644 index 0000000000..70fbbc712c --- /dev/null +++ b/src/logging_api.ts @@ -0,0 +1,95 @@ +import * as fs from 'fs'; +import type { WritableStream } from 'stream/web'; + +/** @public */ +export const SeverityLevel = Object.freeze({ + EMERGENCY: 'emergency', + ALERT: 'alert', + CRITICAL: 'critical', + ERROR: 'error', + WARNING: 'warn', + NOTICE: 'notice', + INFORMATIONAL: 'info', + DEBUG: 'debug', + TRACE: 'trace', + OFF: 'off' +} as const); + +/** @public */ +export type SeverityLevel = typeof SeverityLevel[keyof typeof SeverityLevel]; + +/** @internal */ +const LoggableComponent = Object.freeze({ + COMMAND: 'command', + TOPOLOGY: 'topology', + SERVER_SELECTION: 'serverSelection', + CONNECTION: 'connection' +}); + +/** @internal */ +type LoggableComponent = typeof LoggableComponent[keyof typeof LoggableComponent]; + +/** @public */ +export interface LoggerOptions { + MONGODB_LOG_COMMAND?: SeverityLevel; + MONGODB_LOG_TOPOLOGY?: SeverityLevel; + MONGODB_LOG_SERVER_SELECTION?: SeverityLevel; + MONGODB_LOG_CONNECTION?: SeverityLevel; + MONGODB_LOG_ALL?: SeverityLevel; + MONGODB_LOG_MAX_DOCUMENT_LENGTH?: number; + + /** + * TODO(andymina): make this a WritableStream only when used within the class. + * the i can always call .getWriter().write(); + */ + MONGODB_LOG_PATH?: string | WritableStream; +} + +/* eslint-disable @typescript-eslint/no-unused-vars */ +/* eslint-disable @typescript-eslint/no-empty-function */ +/** @public */ +export class Logger { + /** @internal */ + componentSeverities: Map; + options: LoggerOptions; + + constructor(options: LoggerOptions) { + // validate log path + if (typeof options.MONGODB_LOG_PATH === 'string' && options.MONGODB_LOG_PATH !== 'stderr' && options.MONGODB_LOG_PATH !== 'stdout') { + fs.createWriteStream(options.MONGODB_LOG_PATH, { flags: 'a+' }); + } + + } + + emergency(component: any, message: any): void {} + + alert(component: any, message: any): void {} + + critical(component: any, message: any): void {} + + error(component: any, message: any): void {} + + warn(component: any, message: any): void {} + + notice(component: any, message: any): void {} + + info(component: any, message: any): void {} + + debug(component: any, message: any): void {} + + trace(component: any, message: any): void {} + + #validateOptions(): void { + if + } +} + +/** + MONGODB_LOG_COMMAND + MONGODB_LOG_TOPOLOGY + MONGODB_LOG_SERVER_SELECTION + MONGODB_LOG_CONNECTION + MONGODB_LOG_ALL + MONGODB_LOG_MAX_DOCUMENT_LENGTH + MONGODB_LOG_PATH + */ diff --git a/src/mongo_client.ts b/src/mongo_client.ts index e52d8b22f0..119f9a71c7 100644 --- a/src/mongo_client.ts +++ b/src/mongo_client.ts @@ -15,7 +15,12 @@ import { Db, DbOptions } from './db'; import type { AutoEncrypter, AutoEncryptionOptions } from './deps'; import type { Encrypter } from './encrypter'; import { MongoInvalidArgumentError } from './error'; -import type { Logger, LoggerLevel } from './logger'; +import type { Logger, LoggerLevel, LoggerOptions } from './logger'; +import type { + Logger as LoggingApi, + LoggerOptions as LoggingApiOptions, + SeverityLevel as LoggingApiSeverity +} from './logging_api'; import { TypedEventEmitter } from './mongo_types'; import type { ReadConcern, ReadConcernLevel, ReadConcernLike } from './read_concern'; import { ReadPreference, ReadPreferenceMode } from './read_preference'; @@ -772,6 +777,7 @@ export interface MongoOptions proxyPort?: number; proxyUsername?: string; proxyPassword?: string; + loggerOptions?: LoggingApiOptions; /** @internal */ connectionType?: typeof Connection; diff --git a/src/sandbox.ts b/src/sandbox.ts new file mode 100644 index 0000000000..011a8c774b --- /dev/null +++ b/src/sandbox.ts @@ -0,0 +1,54 @@ +/** @public */ +export const SeverityLevel = Object.freeze({ + EMERGENCY: 'emergency', + ALERT: 'alert', + CRITICAL: 'critical', + ERROR: 'error', + WARNING: 'warn', + NOTICE: 'notice', + INFORMATIONAL: 'info', + DEBUG: 'debug', + TRACE: 'trace', + OFF: 'off' +} as const); + +/** @public */ +export type SeverityLevel = typeof SeverityLevel[keyof typeof SeverityLevel]; + +/** @public */ +export interface LoggerOptions { + [key: string]: SeverityLevel | string | number | undefined; + MONGODB_LOG_COMMAND?: SeverityLevel; + MONGODB_LOG_TOPOLOGY?: SeverityLevel; + MONGODB_LOG_SERVER_SELECTION?: SeverityLevel; + MONGODB_LOG_CONNECTION?: SeverityLevel; + MONGODB_LOG_ALL?: SeverityLevel; + MONGODB_LOG_MAX_DOCUMENT_LENGTH?: number; + MONGODB_LOG_PATH?: string; +} + +// function extractLoggerEnvOptions(): LoggerOptions { +// const loggerOptions: LoggerOptions = {}; + +// // set comp severities +// const validSeverities = Object.values(SeverityLevel); +// const loggingComponents = [ +// 'MONGODB_LOG_COMMAND', +// 'MONGODB_LOG_TOPOLOGY', +// 'MONGODB_LOG_SERVER_SELECTION', +// 'MONGODB_LOG_CONNECTION', +// 'MONGODB_LOG_ALL' +// ]; + +// for (const component of loggingComponents) { +// const severity = (process.env[component] ?? '').toLowerCase(); +// if (validSeverities.includes(severity as SeverityLevel)) +// loggerOptions[component] = severity as SeverityLevel; +// } + +// return loggerOptions; +// } + +const t = SeverityLevel.CRITICAL; + +console.log(typeof t); From ff792fd2326c3e797c58e4bda3b20cdde57cfcbb Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Mon, 21 Nov 2022 22:33:59 -0500 Subject: [PATCH 02/50] feat: implement new Logger constructor --- src/connection_string.ts | 12 +++---- src/logging_api.ts | 71 ++++++++++++++++++++++++++-------------- 2 files changed, 52 insertions(+), 31 deletions(-) diff --git a/src/connection_string.ts b/src/connection_string.ts index ab99e521bf..34dc0d8c86 100644 --- a/src/connection_string.ts +++ b/src/connection_string.ts @@ -514,13 +514,13 @@ export function parseOptions( } mongoOptions.loggingApiOptions = { - MONGODB_LOG_COMMAND: getSeverityForComponent(process.env.MONGODB_LOG_COMMAND ?? ''), - MONGODB_LOG_TOPOLOGY: getSeverityForComponent(process.env.MONGODB_LOG_TOPOLOGY ?? ''), + MONGODB_LOG_COMMAND: getSeverityForComponent(process.env.MONGODB_LOG_COMMAND ?? 'off'), + MONGODB_LOG_TOPOLOGY: getSeverityForComponent(process.env.MONGODB_LOG_TOPOLOGY ?? 'off'), MONGODB_LOG_SERVER_SELECTION: getSeverityForComponent( - process.env.MONGODB_LOG_SERVER_SELECTION ?? '' + process.env.MONGODB_LOG_SERVER_SELECTION ?? 'off' ), - MONGODB_LOG_CONNECTION: getSeverityForComponent(process.env.MONGODB_LOG_CONNECTION ?? ''), - MONGODB_LOG_ALL: getSeverityForComponent(process.env.MONGODB_LOG_ALL ?? ''), + MONGODB_LOG_CONNECTION: getSeverityForComponent(process.env.MONGODB_LOG_CONNECTION ?? 'off'), + MONGODB_LOG_ALL: getSeverityForComponent(process.env.MONGODB_LOG_ALL ?? 'off'), MONGODB_LOG_MAX_DOCUMENT_LENGTH: typeof process.env.MONGODB_LOG_MAX_DOCUMENT_LENGTH === 'string' ? Number.parseInt(process.env.MONGODB_LOG_MAX_DOCUMENT_LENGTH) @@ -539,7 +539,7 @@ export function parseOptions( mongoOptions.loggingApiOptions.MONGODB_LOG_ALL ]; - if (loggingComponents.some(element => typeof element === 'string')) { + if (loggingComponents.some(severity => severity !== SeverityLevel.OFF)) { mongoOptions.loggingApi = new LoggingApi(mongoOptions.loggingApiOptions); } diff --git a/src/logging_api.ts b/src/logging_api.ts index 70fbbc712c..b82ec40e4a 100644 --- a/src/logging_api.ts +++ b/src/logging_api.ts @@ -1,5 +1,7 @@ import * as fs from 'fs'; -import type { WritableStream } from 'stream/web'; +import type { Writable } from 'stream'; + +import { MongoInvalidArgumentError } from './error'; /** @public */ export const SeverityLevel = Object.freeze({ @@ -24,43 +26,66 @@ const LoggableComponent = Object.freeze({ TOPOLOGY: 'topology', SERVER_SELECTION: 'serverSelection', CONNECTION: 'connection' -}); +} as const); /** @internal */ type LoggableComponent = typeof LoggableComponent[keyof typeof LoggableComponent]; /** @public */ export interface LoggerOptions { - MONGODB_LOG_COMMAND?: SeverityLevel; - MONGODB_LOG_TOPOLOGY?: SeverityLevel; - MONGODB_LOG_SERVER_SELECTION?: SeverityLevel; - MONGODB_LOG_CONNECTION?: SeverityLevel; - MONGODB_LOG_ALL?: SeverityLevel; - MONGODB_LOG_MAX_DOCUMENT_LENGTH?: number; - - /** - * TODO(andymina): make this a WritableStream only when used within the class. - * the i can always call .getWriter().write(); - */ - MONGODB_LOG_PATH?: string | WritableStream; + MONGODB_LOG_COMMAND: SeverityLevel; + MONGODB_LOG_TOPOLOGY: SeverityLevel; + MONGODB_LOG_SERVER_SELECTION: SeverityLevel; + MONGODB_LOG_CONNECTION: SeverityLevel; + MONGODB_LOG_ALL: SeverityLevel; + MONGODB_LOG_MAX_DOCUMENT_LENGTH: number; + MONGODB_LOG_PATH: string | Writable; } -/* eslint-disable @typescript-eslint/no-unused-vars */ -/* eslint-disable @typescript-eslint/no-empty-function */ /** @public */ export class Logger { /** @internal */ - componentSeverities: Map; - options: LoggerOptions; + componentSeverities: Map = new Map(); + maxDocumentLength: number; + logDestination: Writable; constructor(options: LoggerOptions) { // validate log path - if (typeof options.MONGODB_LOG_PATH === 'string' && options.MONGODB_LOG_PATH !== 'stderr' && options.MONGODB_LOG_PATH !== 'stdout') { - fs.createWriteStream(options.MONGODB_LOG_PATH, { flags: 'a+' }); + if (typeof options.MONGODB_LOG_PATH === 'string') { + this.logDestination = + options.MONGODB_LOG_PATH === 'stderr' || options.MONGODB_LOG_PATH === 'stdout' + ? process[options.MONGODB_LOG_PATH] + : fs.createWriteStream(options.MONGODB_LOG_PATH, { flags: 'a+' }); + } else { + this.logDestination = options.MONGODB_LOG_PATH; } - + + // fill component severities + this.componentSeverities.set( + LoggableComponent.COMMAND, + options.MONGODB_LOG_COMMAND ?? options.MONGODB_LOG_ALL + ); + this.componentSeverities.set( + LoggableComponent.TOPOLOGY, + options.MONGODB_LOG_TOPOLOGY ?? options.MONGODB_LOG_ALL + ); + this.componentSeverities.set( + LoggableComponent.SERVER_SELECTION, + options.MONGODB_LOG_SERVER_SELECTION ?? options.MONGODB_LOG_ALL + ); + this.componentSeverities.set( + LoggableComponent.CONNECTION, + options.MONGODB_LOG_CONNECTION ?? options.MONGODB_LOG_ALL + ); + + // fill max doc length + if (options.MONGODB_LOG_MAX_DOCUMENT_LENGTH < 0) + throw new MongoInvalidArgumentError('MONGODB_LOG_MAX_DOCUMENT_LENGTH must be >= 0'); + this.maxDocumentLength = options.MONGODB_LOG_MAX_DOCUMENT_LENGTH; } + /* eslint-disable @typescript-eslint/no-unused-vars */ + /* eslint-disable @typescript-eslint/no-empty-function */ emergency(component: any, message: any): void {} alert(component: any, message: any): void {} @@ -78,10 +103,6 @@ export class Logger { debug(component: any, message: any): void {} trace(component: any, message: any): void {} - - #validateOptions(): void { - if - } } /** From fae87060935c09bc7e588062b427ff06d4c8af30 Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Tue, 22 Nov 2022 13:01:37 -0500 Subject: [PATCH 03/50] feat: restructure Logger options --- src/connection_string.ts | 47 ++++--------------- src/logging_api.ts | 97 +++++++++++++++++++++++++++------------- src/mongo_client.ts | 10 ++--- 3 files changed, 79 insertions(+), 75 deletions(-) diff --git a/src/connection_string.ts b/src/connection_string.ts index 34dc0d8c86..19a1746326 100644 --- a/src/connection_string.ts +++ b/src/connection_string.ts @@ -1,7 +1,6 @@ import * as dns from 'dns'; import * as fs from 'fs'; import ConnectionString from 'mongodb-connection-string-url'; -import type { WritableStream } from 'stream/web'; import { URLSearchParams } from 'url'; import type { Document } from './bson'; @@ -16,11 +15,7 @@ import { MongoParseError } from './error'; import { Logger, LoggerLevel } from './logger'; -import { - Logger as LoggingApi, - LoggerOptions as LoggingApiOptions, - SeverityLevel -} from './logging_api'; +import { extractLoggerOptions, Logger as LoggingApi, SeverityLevel } from './logging_api'; import { DriverInfo, MongoClient, @@ -513,34 +508,18 @@ export function parseOptions( ); } - mongoOptions.loggingApiOptions = { - MONGODB_LOG_COMMAND: getSeverityForComponent(process.env.MONGODB_LOG_COMMAND ?? 'off'), - MONGODB_LOG_TOPOLOGY: getSeverityForComponent(process.env.MONGODB_LOG_TOPOLOGY ?? 'off'), - MONGODB_LOG_SERVER_SELECTION: getSeverityForComponent( - process.env.MONGODB_LOG_SERVER_SELECTION ?? 'off' - ), - MONGODB_LOG_CONNECTION: getSeverityForComponent(process.env.MONGODB_LOG_CONNECTION ?? 'off'), - MONGODB_LOG_ALL: getSeverityForComponent(process.env.MONGODB_LOG_ALL ?? 'off'), - MONGODB_LOG_MAX_DOCUMENT_LENGTH: - typeof process.env.MONGODB_LOG_MAX_DOCUMENT_LENGTH === 'string' - ? Number.parseInt(process.env.MONGODB_LOG_MAX_DOCUMENT_LENGTH) - : 1000, - MONGODB_LOG_PATH: - typeof process.env.MONGODB_LOG_PATH === 'string' - ? process.env.MONGODB_LOG_PATH - : mongoOptions.mongodbLogPath ?? 'stderr' - }; - + const loggingApiMongoClientOptions = { mongodbLogPath: mongoOptions.mongodbLogPath }; + const loggingApiOptions = extractLoggerOptions(loggingApiMongoClientOptions); const loggingComponents = [ - mongoOptions.loggingApiOptions.MONGODB_LOG_COMMAND, - mongoOptions.loggingApiOptions.MONGODB_LOG_TOPOLOGY, - mongoOptions.loggingApiOptions.MONGODB_LOG_SERVER_SELECTION, - mongoOptions.loggingApiOptions.MONGODB_LOG_CONNECTION, - mongoOptions.loggingApiOptions.MONGODB_LOG_ALL + loggingApiOptions.commandSeverity, + loggingApiOptions.topologySeverity, + loggingApiOptions.serverSelectionSeverity, + loggingApiOptions.connectionSeverity, + loggingApiOptions.defaultSeverity ]; if (loggingComponents.some(severity => severity !== SeverityLevel.OFF)) { - mongoOptions.loggingApi = new LoggingApi(mongoOptions.loggingApiOptions); + mongoOptions.loggingApi = new LoggingApi(loggingApiOptions); } return mongoOptions; @@ -628,11 +607,6 @@ function setOption( } } -function getSeverityForComponent(rawComponent: string): SeverityLevel | undefined { - const validSeverities = Object.values(SeverityLevel); - return validSeverities.find(severity => severity === rawComponent); -} - interface OptionDescriptor { target?: string; type?: 'boolean' | 'int' | 'uint' | 'record' | 'string' | 'any'; @@ -910,9 +884,6 @@ export const OPTIONS = { loggingApi: { target: 'loggingApi' }, - loggingApiOptions: { - target: 'loggingApi' - }, maxConnecting: { default: 2, transform({ name, values: [value] }): number { diff --git a/src/logging_api.ts b/src/logging_api.ts index b82ec40e4a..6eef418c90 100644 --- a/src/logging_api.ts +++ b/src/logging_api.ts @@ -21,7 +21,7 @@ export const SeverityLevel = Object.freeze({ export type SeverityLevel = typeof SeverityLevel[keyof typeof SeverityLevel]; /** @internal */ -const LoggableComponent = Object.freeze({ +export const LoggableComponent = Object.freeze({ COMMAND: 'command', TOPOLOGY: 'topology', SERVER_SELECTION: 'serverSelection', @@ -31,57 +31,104 @@ const LoggableComponent = Object.freeze({ /** @internal */ type LoggableComponent = typeof LoggableComponent[keyof typeof LoggableComponent]; +/** @public */ +export interface LoggerMongoClientOptions { + mongodbLogPath?: string | Writable; +} + /** @public */ export interface LoggerOptions { - MONGODB_LOG_COMMAND: SeverityLevel; - MONGODB_LOG_TOPOLOGY: SeverityLevel; - MONGODB_LOG_SERVER_SELECTION: SeverityLevel; - MONGODB_LOG_CONNECTION: SeverityLevel; - MONGODB_LOG_ALL: SeverityLevel; - MONGODB_LOG_MAX_DOCUMENT_LENGTH: number; - MONGODB_LOG_PATH: string | Writable; + commandSeverity: SeverityLevel; + topologySeverity: SeverityLevel; + serverSelectionSeverity: SeverityLevel; + connectionSeverity: SeverityLevel; + defaultSeverity: SeverityLevel; + maxDocumentLength: number; + logDestination: string | Writable; +} + +/** + * @public + * TODO(andymina): add docs for this + */ +export function extractLoggerOptions(clientOptions?: LoggerMongoClientOptions): LoggerOptions { + const validSeverities = Object.values(SeverityLevel); + + return { + commandSeverity: + validSeverities.find(severity => severity === process.env.MONGODB_LOG_COMMAND) ?? + SeverityLevel.OFF, + topologySeverity: + validSeverities.find(severity => severity === process.env.MONGODB_LOG_TOPOLOGY) ?? + SeverityLevel.OFF, + serverSelectionSeverity: + validSeverities.find(severity => severity === process.env.MONGODB_LOG_SERVER_SELECTION) ?? + SeverityLevel.OFF, + connectionSeverity: + validSeverities.find(severity => severity === process.env.MONGODB_LOG_CONNECTION) ?? + SeverityLevel.OFF, + defaultSeverity: + validSeverities.find(severity => severity === process.env.MONGODB_LOG_COMMAND) ?? + SeverityLevel.OFF, + maxDocumentLength: + typeof process.env.MONGODB_LOG_MAX_DOCUMENT_LENGTH === 'string' + ? Number.parseInt(process.env.MONGODB_LOG_MAX_DOCUMENT_LENGTH) + : 1000, + logDestination: + typeof process.env.MONGODB_LOG_PATH === 'string' + ? process.env.MONGODB_LOG_PATH + : clientOptions?.mongodbLogPath ?? 'stderr' + }; } /** @public */ export class Logger { /** @internal */ - componentSeverities: Map = new Map(); + componentSeverities: Map = new Map(); maxDocumentLength: number; logDestination: Writable; constructor(options: LoggerOptions) { // validate log path - if (typeof options.MONGODB_LOG_PATH === 'string') { + if (typeof options.logDestination === 'string') { this.logDestination = - options.MONGODB_LOG_PATH === 'stderr' || options.MONGODB_LOG_PATH === 'stdout' - ? process[options.MONGODB_LOG_PATH] - : fs.createWriteStream(options.MONGODB_LOG_PATH, { flags: 'a+' }); + options.logDestination === 'stderr' || options.logDestination === 'stdout' + ? process[options.logDestination] + : fs.createWriteStream(options.logDestination, { flags: 'a+' }); } else { - this.logDestination = options.MONGODB_LOG_PATH; + this.logDestination = options.logDestination; } // fill component severities this.componentSeverities.set( LoggableComponent.COMMAND, - options.MONGODB_LOG_COMMAND ?? options.MONGODB_LOG_ALL + options.commandSeverity !== SeverityLevel.OFF + ? options.commandSeverity + : options.defaultSeverity ); this.componentSeverities.set( LoggableComponent.TOPOLOGY, - options.MONGODB_LOG_TOPOLOGY ?? options.MONGODB_LOG_ALL + options.topologySeverity !== SeverityLevel.OFF + ? options.topologySeverity + : options.defaultSeverity ); this.componentSeverities.set( LoggableComponent.SERVER_SELECTION, - options.MONGODB_LOG_SERVER_SELECTION ?? options.MONGODB_LOG_ALL + options.serverSelectionSeverity !== SeverityLevel.OFF + ? options.serverSelectionSeverity + : options.defaultSeverity ); this.componentSeverities.set( LoggableComponent.CONNECTION, - options.MONGODB_LOG_CONNECTION ?? options.MONGODB_LOG_ALL + options.connectionSeverity !== SeverityLevel.OFF + ? options.connectionSeverity + : options.defaultSeverity ); // fill max doc length - if (options.MONGODB_LOG_MAX_DOCUMENT_LENGTH < 0) + if (options.maxDocumentLength < 0) throw new MongoInvalidArgumentError('MONGODB_LOG_MAX_DOCUMENT_LENGTH must be >= 0'); - this.maxDocumentLength = options.MONGODB_LOG_MAX_DOCUMENT_LENGTH; + this.maxDocumentLength = options.maxDocumentLength; } /* eslint-disable @typescript-eslint/no-unused-vars */ @@ -104,13 +151,3 @@ export class Logger { trace(component: any, message: any): void {} } - -/** - MONGODB_LOG_COMMAND - MONGODB_LOG_TOPOLOGY - MONGODB_LOG_SERVER_SELECTION - MONGODB_LOG_CONNECTION - MONGODB_LOG_ALL - MONGODB_LOG_MAX_DOCUMENT_LENGTH - MONGODB_LOG_PATH - */ diff --git a/src/mongo_client.ts b/src/mongo_client.ts index 119f9a71c7..0cfe6757cf 100644 --- a/src/mongo_client.ts +++ b/src/mongo_client.ts @@ -15,12 +15,8 @@ import { Db, DbOptions } from './db'; import type { AutoEncrypter, AutoEncryptionOptions } from './deps'; import type { Encrypter } from './encrypter'; import { MongoInvalidArgumentError } from './error'; -import type { Logger, LoggerLevel, LoggerOptions } from './logger'; -import type { - Logger as LoggingApi, - LoggerOptions as LoggingApiOptions, - SeverityLevel as LoggingApiSeverity -} from './logging_api'; +import type { Logger, LoggerLevel } from './logger'; +import type { Logger as LoggingApi } from './logging_api'; import { TypedEventEmitter } from './mongo_types'; import type { ReadConcern, ReadConcernLevel, ReadConcernLike } from './read_concern'; import { ReadPreference, ReadPreferenceMode } from './read_preference'; @@ -777,7 +773,7 @@ export interface MongoOptions proxyPort?: number; proxyUsername?: string; proxyPassword?: string; - loggerOptions?: LoggingApiOptions; + loggingApi?: LoggingApi; /** @internal */ connectionType?: typeof Connection; From b9d915c7eed0d6cb7cd8693107df5a1f0a0c3725 Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Tue, 22 Nov 2022 13:05:57 -0500 Subject: [PATCH 04/50] chore: remove sandbox file --- src/sandbox.ts | 54 -------------------------------------------------- 1 file changed, 54 deletions(-) delete mode 100644 src/sandbox.ts diff --git a/src/sandbox.ts b/src/sandbox.ts deleted file mode 100644 index 011a8c774b..0000000000 --- a/src/sandbox.ts +++ /dev/null @@ -1,54 +0,0 @@ -/** @public */ -export const SeverityLevel = Object.freeze({ - EMERGENCY: 'emergency', - ALERT: 'alert', - CRITICAL: 'critical', - ERROR: 'error', - WARNING: 'warn', - NOTICE: 'notice', - INFORMATIONAL: 'info', - DEBUG: 'debug', - TRACE: 'trace', - OFF: 'off' -} as const); - -/** @public */ -export type SeverityLevel = typeof SeverityLevel[keyof typeof SeverityLevel]; - -/** @public */ -export interface LoggerOptions { - [key: string]: SeverityLevel | string | number | undefined; - MONGODB_LOG_COMMAND?: SeverityLevel; - MONGODB_LOG_TOPOLOGY?: SeverityLevel; - MONGODB_LOG_SERVER_SELECTION?: SeverityLevel; - MONGODB_LOG_CONNECTION?: SeverityLevel; - MONGODB_LOG_ALL?: SeverityLevel; - MONGODB_LOG_MAX_DOCUMENT_LENGTH?: number; - MONGODB_LOG_PATH?: string; -} - -// function extractLoggerEnvOptions(): LoggerOptions { -// const loggerOptions: LoggerOptions = {}; - -// // set comp severities -// const validSeverities = Object.values(SeverityLevel); -// const loggingComponents = [ -// 'MONGODB_LOG_COMMAND', -// 'MONGODB_LOG_TOPOLOGY', -// 'MONGODB_LOG_SERVER_SELECTION', -// 'MONGODB_LOG_CONNECTION', -// 'MONGODB_LOG_ALL' -// ]; - -// for (const component of loggingComponents) { -// const severity = (process.env[component] ?? '').toLowerCase(); -// if (validSeverities.includes(severity as SeverityLevel)) -// loggerOptions[component] = severity as SeverityLevel; -// } - -// return loggerOptions; -// } - -const t = SeverityLevel.CRITICAL; - -console.log(typeof t); From c7f28f0d04dd4b994a8eb282f870c6b8bd303bf1 Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Tue, 22 Nov 2022 17:04:28 -0500 Subject: [PATCH 05/50] test: add test for MongoLogger --- src/connection_string.ts | 32 +++---- src/logging_api.ts | 153 --------------------------------- src/mongo_client.ts | 21 +++-- src/mongo_logger.ts | 131 ++++++++++++++++++++++++++++ test/unit/mongo_client.test.js | 32 ++++++- test/unit/mongo_logger.ts | 41 +++++++++ 6 files changed, 232 insertions(+), 178 deletions(-) delete mode 100644 src/logging_api.ts create mode 100644 src/mongo_logger.ts create mode 100644 test/unit/mongo_logger.ts diff --git a/src/connection_string.ts b/src/connection_string.ts index 19a1746326..3408063914 100644 --- a/src/connection_string.ts +++ b/src/connection_string.ts @@ -14,8 +14,7 @@ import { MongoMissingCredentialsError, MongoParseError } from './error'; -import { Logger, LoggerLevel } from './logger'; -import { extractLoggerOptions, Logger as LoggingApi, SeverityLevel } from './logging_api'; +import { Logger as LegacyLogger, LoggerLevel as LegacyLoggerLevel } from './logger'; import { DriverInfo, MongoClient, @@ -25,6 +24,7 @@ import { ServerApi, ServerApiVersion } from './mongo_client'; +import { Logger, SeverityLevel } from './mongo_logger'; import { PromiseProvider } from './promise_provider'; import { ReadConcern, ReadConcernLevel } from './read_concern'; import { ReadPreference, ReadPreferenceMode } from './read_preference'; @@ -207,7 +207,7 @@ function getInt(name: string, value: unknown): number { throw new MongoParseError(`Expected ${name} to be stringified int value, got: ${value}`); } -function getUint(name: string, value: unknown): number { +export function getUint(name: string, value: unknown): number { const parsedValue = getInt(name, value); if (parsedValue < 0) { throw new MongoParseError(`${name} can only be a positive int value, got: ${value}`); @@ -508,18 +508,18 @@ export function parseOptions( ); } - const loggingApiMongoClientOptions = { mongodbLogPath: mongoOptions.mongodbLogPath }; - const loggingApiOptions = extractLoggerOptions(loggingApiMongoClientOptions); + const loggerMongoClientOptions = { mongodbLogPath: mongoOptions.mongodbLogPath }; + const loggerOptions = Logger.resolveOptions(loggerMongoClientOptions); const loggingComponents = [ - loggingApiOptions.commandSeverity, - loggingApiOptions.topologySeverity, - loggingApiOptions.serverSelectionSeverity, - loggingApiOptions.connectionSeverity, - loggingApiOptions.defaultSeverity + loggerOptions.command, + loggerOptions.topology, + loggerOptions.serverSelection, + loggerOptions.connection, + loggerOptions.defaultSeverity ]; if (loggingComponents.some(severity => severity !== SeverityLevel.OFF)) { - mongoOptions.loggingApi = new LoggingApi(loggingApiOptions); + mongoOptions.mongoLogger = new Logger(loggerOptions); } return mongoOptions; @@ -864,9 +864,9 @@ export const OPTIONS = { type: 'uint' }, logger: { - default: new Logger('MongoClient'), + default: new LegacyLogger('MongoClient'), transform({ values: [value] }) { - if (value instanceof Logger) { + if (value instanceof LegacyLogger) { return value; } emitWarning('Alternative loggers might not be supported'); @@ -878,11 +878,11 @@ export const OPTIONS = { loggerLevel: { target: 'logger', transform({ values: [value] }) { - return new Logger('MongoClient', { loggerLevel: value as LoggerLevel }); + return new LegacyLogger('MongoClient', { loggerLevel: value as LegacyLoggerLevel }); } }, - loggingApi: { - target: 'loggingApi' + mongoLogger: { + target: 'mongoLogger' }, maxConnecting: { default: 2, diff --git a/src/logging_api.ts b/src/logging_api.ts deleted file mode 100644 index 6eef418c90..0000000000 --- a/src/logging_api.ts +++ /dev/null @@ -1,153 +0,0 @@ -import * as fs from 'fs'; -import type { Writable } from 'stream'; - -import { MongoInvalidArgumentError } from './error'; - -/** @public */ -export const SeverityLevel = Object.freeze({ - EMERGENCY: 'emergency', - ALERT: 'alert', - CRITICAL: 'critical', - ERROR: 'error', - WARNING: 'warn', - NOTICE: 'notice', - INFORMATIONAL: 'info', - DEBUG: 'debug', - TRACE: 'trace', - OFF: 'off' -} as const); - -/** @public */ -export type SeverityLevel = typeof SeverityLevel[keyof typeof SeverityLevel]; - -/** @internal */ -export const LoggableComponent = Object.freeze({ - COMMAND: 'command', - TOPOLOGY: 'topology', - SERVER_SELECTION: 'serverSelection', - CONNECTION: 'connection' -} as const); - -/** @internal */ -type LoggableComponent = typeof LoggableComponent[keyof typeof LoggableComponent]; - -/** @public */ -export interface LoggerMongoClientOptions { - mongodbLogPath?: string | Writable; -} - -/** @public */ -export interface LoggerOptions { - commandSeverity: SeverityLevel; - topologySeverity: SeverityLevel; - serverSelectionSeverity: SeverityLevel; - connectionSeverity: SeverityLevel; - defaultSeverity: SeverityLevel; - maxDocumentLength: number; - logDestination: string | Writable; -} - -/** - * @public - * TODO(andymina): add docs for this - */ -export function extractLoggerOptions(clientOptions?: LoggerMongoClientOptions): LoggerOptions { - const validSeverities = Object.values(SeverityLevel); - - return { - commandSeverity: - validSeverities.find(severity => severity === process.env.MONGODB_LOG_COMMAND) ?? - SeverityLevel.OFF, - topologySeverity: - validSeverities.find(severity => severity === process.env.MONGODB_LOG_TOPOLOGY) ?? - SeverityLevel.OFF, - serverSelectionSeverity: - validSeverities.find(severity => severity === process.env.MONGODB_LOG_SERVER_SELECTION) ?? - SeverityLevel.OFF, - connectionSeverity: - validSeverities.find(severity => severity === process.env.MONGODB_LOG_CONNECTION) ?? - SeverityLevel.OFF, - defaultSeverity: - validSeverities.find(severity => severity === process.env.MONGODB_LOG_COMMAND) ?? - SeverityLevel.OFF, - maxDocumentLength: - typeof process.env.MONGODB_LOG_MAX_DOCUMENT_LENGTH === 'string' - ? Number.parseInt(process.env.MONGODB_LOG_MAX_DOCUMENT_LENGTH) - : 1000, - logDestination: - typeof process.env.MONGODB_LOG_PATH === 'string' - ? process.env.MONGODB_LOG_PATH - : clientOptions?.mongodbLogPath ?? 'stderr' - }; -} - -/** @public */ -export class Logger { - /** @internal */ - componentSeverities: Map = new Map(); - maxDocumentLength: number; - logDestination: Writable; - - constructor(options: LoggerOptions) { - // validate log path - if (typeof options.logDestination === 'string') { - this.logDestination = - options.logDestination === 'stderr' || options.logDestination === 'stdout' - ? process[options.logDestination] - : fs.createWriteStream(options.logDestination, { flags: 'a+' }); - } else { - this.logDestination = options.logDestination; - } - - // fill component severities - this.componentSeverities.set( - LoggableComponent.COMMAND, - options.commandSeverity !== SeverityLevel.OFF - ? options.commandSeverity - : options.defaultSeverity - ); - this.componentSeverities.set( - LoggableComponent.TOPOLOGY, - options.topologySeverity !== SeverityLevel.OFF - ? options.topologySeverity - : options.defaultSeverity - ); - this.componentSeverities.set( - LoggableComponent.SERVER_SELECTION, - options.serverSelectionSeverity !== SeverityLevel.OFF - ? options.serverSelectionSeverity - : options.defaultSeverity - ); - this.componentSeverities.set( - LoggableComponent.CONNECTION, - options.connectionSeverity !== SeverityLevel.OFF - ? options.connectionSeverity - : options.defaultSeverity - ); - - // fill max doc length - if (options.maxDocumentLength < 0) - throw new MongoInvalidArgumentError('MONGODB_LOG_MAX_DOCUMENT_LENGTH must be >= 0'); - this.maxDocumentLength = options.maxDocumentLength; - } - - /* eslint-disable @typescript-eslint/no-unused-vars */ - /* eslint-disable @typescript-eslint/no-empty-function */ - emergency(component: any, message: any): void {} - - alert(component: any, message: any): void {} - - critical(component: any, message: any): void {} - - error(component: any, message: any): void {} - - warn(component: any, message: any): void {} - - notice(component: any, message: any): void {} - - info(component: any, message: any): void {} - - debug(component: any, message: any): void {} - - trace(component: any, message: any): void {} -} diff --git a/src/mongo_client.ts b/src/mongo_client.ts index 0cfe6757cf..746ec53fee 100644 --- a/src/mongo_client.ts +++ b/src/mongo_client.ts @@ -15,8 +15,8 @@ import { Db, DbOptions } from './db'; import type { AutoEncrypter, AutoEncryptionOptions } from './deps'; import type { Encrypter } from './encrypter'; import { MongoInvalidArgumentError } from './error'; -import type { Logger, LoggerLevel } from './logger'; -import type { Logger as LoggingApi } from './logging_api'; +import type { Logger as LegacyLogger, LoggerLevel as LegacyLoggerLevel } from './logger'; +import type { Logger } from './mongo_logger'; import { TypedEventEmitter } from './mongo_types'; import type { ReadConcern, ReadConcernLevel, ReadConcernLike } from './read_concern'; import { ReadPreference, ReadPreferenceMode } from './read_preference'; @@ -234,9 +234,9 @@ export interface MongoClientOptions extends BSONSerializeOptions, SupportedNodeC */ promiseLibrary?: any; /** The logging level */ - loggerLevel?: LoggerLevel; + loggerLevel?: LegacyLoggerLevel; /** Custom logger object */ - logger?: Logger; + logger?: LegacyLogger; /** Enable command monitoring for this client */ monitorCommands?: boolean; /** Server API version */ @@ -297,7 +297,7 @@ export interface MongoClientPrivate { readonly readConcern?: ReadConcern; readonly writeConcern?: WriteConcern; readonly readPreference: ReadPreference; - readonly logger: Logger; + readonly logger: LegacyLogger; readonly isMongoClient: true; } @@ -335,6 +335,8 @@ export class MongoClient extends TypedEventEmitter { s: MongoClientPrivate; /** @internal */ topology?: Topology; + /** @internal */ + readonly mongoLogger: Logger | null; /** * The consolidate, parsed, transformed and merged options. @@ -346,6 +348,7 @@ export class MongoClient extends TypedEventEmitter { super(); this[kOptions] = parseOptions(url, this, options); + this.mongoLogger = this[kOptions].mongoLogger ?? null; // eslint-disable-next-line @typescript-eslint/no-this-alias const client = this; @@ -418,7 +421,7 @@ export class MongoClient extends TypedEventEmitter { return this.s.bsonOptions; } - get logger(): Logger { + get logger(): LegacyLogger { return this.s.logger; } @@ -709,7 +712,7 @@ export class MongoClient extends TypedEventEmitter { } /** Return the mongo client logger */ - getLogger(): Logger { + getLogger(): LegacyLogger { return this.s.logger; } } @@ -773,7 +776,6 @@ export interface MongoOptions proxyPort?: number; proxyUsername?: string; proxyPassword?: string; - loggingApi?: LoggingApi; /** @internal */ connectionType?: typeof Connection; @@ -805,4 +807,7 @@ export interface MongoOptions /** @internal */ [featureFlag: symbol]: any; + + /** @internal */ + mongoLogger?: Logger; } diff --git a/src/mongo_logger.ts b/src/mongo_logger.ts new file mode 100644 index 0000000000..ee3fa2f38f --- /dev/null +++ b/src/mongo_logger.ts @@ -0,0 +1,131 @@ +import * as fs from 'fs'; +import { env } from 'process'; +import type { Writable } from 'stream'; + +import { getUint } from './connection_string'; + +/** @public */ +export const SeverityLevel = Object.freeze({ + EMERGENCY: 'emergency', + ALERT: 'alert', + CRITICAL: 'critical', + ERROR: 'error', + WARNING: 'warn', + NOTICE: 'notice', + INFORMATIONAL: 'info', + DEBUG: 'debug', + TRACE: 'trace', + OFF: 'off' +} as const); + +/** @public */ +export type SeverityLevel = typeof SeverityLevel[keyof typeof SeverityLevel]; + +/** @returns one of SeverityLevel or null if it is not a valid SeverityLevel */ +function toValidSeverity(severity?: string): SeverityLevel | null { + const validSeverities: string[] = Object.values(SeverityLevel); + const lowerSeverity = severity?.toLowerCase(); + + if (lowerSeverity != null && validSeverities.includes(lowerSeverity)) { + return lowerSeverity as SeverityLevel; + } + + return null; +} + +/** @internal */ +export const LoggableComponent = Object.freeze({ + COMMAND: 'command', + TOPOLOGY: 'topology', + SERVER_SELECTION: 'serverSelection', + CONNECTION: 'connection' +} as const); + +/** @internal */ +type LoggableComponent = typeof LoggableComponent[keyof typeof LoggableComponent]; + +/** @internal */ +export interface LoggerMongoClientOptions { + mongodbLogPath?: string | Writable; +} + +/** @public */ +export interface LoggerOptions { + command: SeverityLevel; + topology: SeverityLevel; + serverSelection: SeverityLevel; + connection: SeverityLevel; + defaultSeverity: SeverityLevel; + maxDocumentLength: number; + logPath: string | Writable; +} + +/** + * @internal + * TODO(andymina): add docs + */ +export class Logger { + /** @internal */ + componentSeverities: Record; + maxDocumentLength: number; + logPath: Writable; + + constructor(options: LoggerOptions) { + // validate log path + if (typeof options.logPath === 'string') { + this.logPath = + options.logPath === 'stderr' || options.logPath === 'stdout' + ? process[options.logPath] + : fs.createWriteStream(options.logPath, { flags: 'a+' }); + } else { + this.logPath = options.logPath; + } + + // extract comp severities + this.componentSeverities = options; + + // fill max doc length + this.maxDocumentLength = options.maxDocumentLength; + } + + /* eslint-disable @typescript-eslint/no-unused-vars */ + /* eslint-disable @typescript-eslint/no-empty-function */ + emergency(component: any, message: any): void {} + + alert(component: any, message: any): void {} + + critical(component: any, message: any): void {} + + error(component: any, message: any): void {} + + warn(component: any, message: any): void {} + + notice(component: any, message: any): void {} + + info(component: any, message: any): void {} + + debug(component: any, message: any): void {} + + trace(component: any, message: any): void {} + + static resolveOptions(clientOptions?: LoggerMongoClientOptions): LoggerOptions { + const defaultSeverity = toValidSeverity(env.MONGODB_LOG_ALL) ?? SeverityLevel.OFF; + + return { + command: toValidSeverity(env.MONGODB_LOG_COMMAND) ?? defaultSeverity, + topology: toValidSeverity(env.MONGODB_LOG_TOPOLOGY) ?? defaultSeverity, + serverSelection: toValidSeverity(env.MONGODB_LOG_SERVER_SELECTION) ?? defaultSeverity, + connection: toValidSeverity(env.MONGODB_LOG_CONNECTION) ?? defaultSeverity, + defaultSeverity, + maxDocumentLength: + typeof env.MONGODB_LOG_MAX_DOCUMENT_LENGTH === 'string' && + env.MONGODB_LOG_MAX_DOCUMENT_LENGTH !== '' + ? getUint('MONGODB_LOG_MAX_DOCUMENT_LENGTH', env.MONGODB_LOG_MAX_DOCUMENT_LENGTH) + : 1000, + logPath: + typeof env.MONGODB_LOG_PATH === 'string' && env.MONGODB_LOG_PATH !== '' + ? env.MONGODB_LOG_PATH + : clientOptions?.mongodbLogPath ?? 'stderr' + }; + } +} diff --git a/test/unit/mongo_client.test.js b/test/unit/mongo_client.test.js index 918b18c36e..aff5476aab 100644 --- a/test/unit/mongo_client.test.js +++ b/test/unit/mongo_client.test.js @@ -1,15 +1,17 @@ 'use strict'; const os = require('os'); const fs = require('fs'); +const { env } = require('process'); const { expect } = require('chai'); const { getSymbolFrom } = require('../tools/utils'); const { parseOptions, resolveSRVRecord } = require('../../src/connection_string'); const { ReadConcern } = require('../../src/read_concern'); const { WriteConcern } = require('../../src/write_concern'); const { ReadPreference } = require('../../src/read_preference'); -const { Logger } = require('../../src/logger'); +const { Logger } = require('../../src/mongo_logger'); const { MongoCredentials } = require('../../src/cmap/auth/mongo_credentials'); const { MongoClient, MongoParseError, ServerApiVersion } = require('../../src'); +const { SeverityLevel } = require('../../src/mongo_logger'); describe('MongoOptions', function () { it('MongoClient should always freeze public options', function () { @@ -847,4 +849,32 @@ describe('MongoOptions', function () { }); }); }); + + context('logger', function () { + const severityVars = [ + 'MONGODB_LOG_COMMAND', + 'MONGODB_LOG_TOPOLOGY', + 'MONGODB_LOG_SERVER_SELECTION', + 'MONGODB_LOG_CONNECTION', + 'MONGODB_LOG_ALL' + ]; + + for (const name of severityVars) { + it(`should enable logging if at least ${name} is set to a valid value`, function () { + env[name] = SeverityLevel.CRITICAL; + const client = new MongoClient('mongodb://localhost:27017'); + expect(client.mongoLogger).to.be.instanceOf(Logger); + env[name] = undefined; + }); + } + + for (const name of severityVars) { + it(`should not enable logging if ${name} is set to an invalid value`, function () { + env[name] = 'invalid'; + const client = new MongoClient('mongodb://localhost:27017'); + expect(client).property('mongoLogger', null); + env[name] = undefined; + }); + } + }); }); diff --git a/test/unit/mongo_logger.ts b/test/unit/mongo_logger.ts new file mode 100644 index 0000000000..8c91564e91 --- /dev/null +++ b/test/unit/mongo_logger.ts @@ -0,0 +1,41 @@ +import { expect } from 'chai'; + +import { LoggableComponent, Logger, LoggerOptions, SeverityLevel } from '../../src/mongo_logger'; + +describe('Logger', function () { + describe('options parsing', function () { + let loggerOptions: LoggerOptions; + + before(function () { + // MONGODB_LOG_COMMAND is not set so it defaults to undefined + process.env.MONGODB_LOG_TOPOLOGY = ''; + process.env.MONGODB_LOG_SERVER_SELECTION = 'invalid'; + process.env.MONGODB_LOG_CONNECTION = 'CRITICAL'; + process.env.MONGODB_LOG_ALL = 'eRrOr'; + process.env.MONGODB_LOG_MAX_DOCUMENT_LENGTH = '100'; + process.env.MONGODB_LOG_PATH = 'stderr'; + + loggerOptions = Logger.resolveOptions(); + }); + + it('treats severity values as case-insensitive', function () { + expect(loggerOptions.connection).to.equal(SeverityLevel.CRITICAL); + expect(loggerOptions.defaultSeverity).to.equal(SeverityLevel.ERROR); + }); + + it('will only use MONGODB_LOG_ALL for component severities that are not set or invalid', function () { + expect(loggerOptions.command).to.equal(loggerOptions.defaultSeverity); // empty str + expect(loggerOptions.topology).to.equal(loggerOptions.defaultSeverity); // undefined + expect(loggerOptions.serverSelection).to.equal(loggerOptions.defaultSeverity); // invalid + }); + + it('can set severity levels per component', function () { + const { componentSeverities } = new Logger(loggerOptions); + + expect(componentSeverities).property(LoggableComponent.COMMAND, SeverityLevel.ERROR); + expect(componentSeverities).property(LoggableComponent.TOPOLOGY, SeverityLevel.ERROR); + expect(componentSeverities).property(LoggableComponent.SERVER_SELECTION, SeverityLevel.ERROR); + expect(componentSeverities).property(LoggableComponent.CONNECTION, SeverityLevel.CRITICAL); + }); + }); +}); From 6cdb0c3434c9ec85e4bafc44a27f9c50bbc61751 Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Tue, 22 Nov 2022 17:06:56 -0500 Subject: [PATCH 06/50] refactor: use global process instead of importing --- src/mongo_logger.ts | 21 ++++++++++----------- test/unit/mongo_client.test.js | 9 ++++----- 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/src/mongo_logger.ts b/src/mongo_logger.ts index ee3fa2f38f..0107ab888d 100644 --- a/src/mongo_logger.ts +++ b/src/mongo_logger.ts @@ -1,5 +1,4 @@ import * as fs from 'fs'; -import { env } from 'process'; import type { Writable } from 'stream'; import { getUint } from './connection_string'; @@ -109,22 +108,22 @@ export class Logger { trace(component: any, message: any): void {} static resolveOptions(clientOptions?: LoggerMongoClientOptions): LoggerOptions { - const defaultSeverity = toValidSeverity(env.MONGODB_LOG_ALL) ?? SeverityLevel.OFF; + const defaultSeverity = toValidSeverity(process.env.MONGODB_LOG_ALL) ?? SeverityLevel.OFF; return { - command: toValidSeverity(env.MONGODB_LOG_COMMAND) ?? defaultSeverity, - topology: toValidSeverity(env.MONGODB_LOG_TOPOLOGY) ?? defaultSeverity, - serverSelection: toValidSeverity(env.MONGODB_LOG_SERVER_SELECTION) ?? defaultSeverity, - connection: toValidSeverity(env.MONGODB_LOG_CONNECTION) ?? defaultSeverity, + command: toValidSeverity(process.env.MONGODB_LOG_COMMAND) ?? defaultSeverity, + topology: toValidSeverity(process.env.MONGODB_LOG_TOPOLOGY) ?? defaultSeverity, + serverSelection: toValidSeverity(process.env.MONGODB_LOG_SERVER_SELECTION) ?? defaultSeverity, + connection: toValidSeverity(process.env.MONGODB_LOG_CONNECTION) ?? defaultSeverity, defaultSeverity, maxDocumentLength: - typeof env.MONGODB_LOG_MAX_DOCUMENT_LENGTH === 'string' && - env.MONGODB_LOG_MAX_DOCUMENT_LENGTH !== '' - ? getUint('MONGODB_LOG_MAX_DOCUMENT_LENGTH', env.MONGODB_LOG_MAX_DOCUMENT_LENGTH) + typeof process.env.MONGODB_LOG_MAX_DOCUMENT_LENGTH === 'string' && + process.env.MONGODB_LOG_MAX_DOCUMENT_LENGTH !== '' + ? getUint('MONGODB_LOG_MAX_DOCUMENT_LENGTH', process.env.MONGODB_LOG_MAX_DOCUMENT_LENGTH) : 1000, logPath: - typeof env.MONGODB_LOG_PATH === 'string' && env.MONGODB_LOG_PATH !== '' - ? env.MONGODB_LOG_PATH + typeof process.env.MONGODB_LOG_PATH === 'string' && process.env.MONGODB_LOG_PATH !== '' + ? process.env.MONGODB_LOG_PATH : clientOptions?.mongodbLogPath ?? 'stderr' }; } diff --git a/test/unit/mongo_client.test.js b/test/unit/mongo_client.test.js index aff5476aab..fa9197acb8 100644 --- a/test/unit/mongo_client.test.js +++ b/test/unit/mongo_client.test.js @@ -1,7 +1,6 @@ 'use strict'; const os = require('os'); const fs = require('fs'); -const { env } = require('process'); const { expect } = require('chai'); const { getSymbolFrom } = require('../tools/utils'); const { parseOptions, resolveSRVRecord } = require('../../src/connection_string'); @@ -861,19 +860,19 @@ describe('MongoOptions', function () { for (const name of severityVars) { it(`should enable logging if at least ${name} is set to a valid value`, function () { - env[name] = SeverityLevel.CRITICAL; + process.env[name] = SeverityLevel.CRITICAL; const client = new MongoClient('mongodb://localhost:27017'); expect(client.mongoLogger).to.be.instanceOf(Logger); - env[name] = undefined; + process.env[name] = undefined; }); } for (const name of severityVars) { it(`should not enable logging if ${name} is set to an invalid value`, function () { - env[name] = 'invalid'; + process.env[name] = 'invalid'; const client = new MongoClient('mongodb://localhost:27017'); expect(client).property('mongoLogger', null); - env[name] = undefined; + process.env[name] = undefined; }); } }); From 36ee2ade5ac8e5c212d22e1007edb4170531a818 Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Wed, 23 Nov 2022 11:30:02 -0500 Subject: [PATCH 07/50] refactor: rename logger to MongoLogger and add docs --- src/connection_string.ts | 6 ++--- src/index.ts | 7 ++++++ src/mongo_client.ts | 6 ++--- src/mongo_logger.ts | 53 +++++++++++++++++++++++++--------------- 4 files changed, 46 insertions(+), 26 deletions(-) diff --git a/src/connection_string.ts b/src/connection_string.ts index 3408063914..1b3ba46ac8 100644 --- a/src/connection_string.ts +++ b/src/connection_string.ts @@ -24,7 +24,7 @@ import { ServerApi, ServerApiVersion } from './mongo_client'; -import { Logger, SeverityLevel } from './mongo_logger'; +import { MongoLogger, SeverityLevel } from './mongo_logger'; import { PromiseProvider } from './promise_provider'; import { ReadConcern, ReadConcernLevel } from './read_concern'; import { ReadPreference, ReadPreferenceMode } from './read_preference'; @@ -509,7 +509,7 @@ export function parseOptions( } const loggerMongoClientOptions = { mongodbLogPath: mongoOptions.mongodbLogPath }; - const loggerOptions = Logger.resolveOptions(loggerMongoClientOptions); + const loggerOptions = MongoLogger.resolveOptions(loggerMongoClientOptions); const loggingComponents = [ loggerOptions.command, loggerOptions.topology, @@ -519,7 +519,7 @@ export function parseOptions( ]; if (loggingComponents.some(severity => severity !== SeverityLevel.OFF)) { - mongoOptions.mongoLogger = new Logger(loggerOptions); + mongoOptions.mongoLogger = new MongoLogger(loggerOptions); } return mongoOptions; diff --git a/src/index.ts b/src/index.ts index ff692a3b1b..b3cff9a49b 100644 --- a/src/index.ts +++ b/src/index.ts @@ -300,6 +300,13 @@ export type { SupportedTLSSocketOptions, WithSessionCallback } from './mongo_client'; +export type { + MongoLoggableComponent, + MongoLogger, + MongoLoggerMongoClientOptions, + MongoLoggerOptions, + SeverityLevel +} from './mongo_logger'; export type { CommonEvents, EventsDescription, diff --git a/src/mongo_client.ts b/src/mongo_client.ts index 746ec53fee..0248a0f666 100644 --- a/src/mongo_client.ts +++ b/src/mongo_client.ts @@ -16,7 +16,7 @@ import type { AutoEncrypter, AutoEncryptionOptions } from './deps'; import type { Encrypter } from './encrypter'; import { MongoInvalidArgumentError } from './error'; import type { Logger as LegacyLogger, LoggerLevel as LegacyLoggerLevel } from './logger'; -import type { Logger } from './mongo_logger'; +import type { MongoLogger } from './mongo_logger'; import { TypedEventEmitter } from './mongo_types'; import type { ReadConcern, ReadConcernLevel, ReadConcernLike } from './read_concern'; import { ReadPreference, ReadPreferenceMode } from './read_preference'; @@ -336,7 +336,7 @@ export class MongoClient extends TypedEventEmitter { /** @internal */ topology?: Topology; /** @internal */ - readonly mongoLogger: Logger | null; + readonly mongoLogger: MongoLogger | null; /** * The consolidate, parsed, transformed and merged options. @@ -809,5 +809,5 @@ export interface MongoOptions [featureFlag: symbol]: any; /** @internal */ - mongoLogger?: Logger; + mongoLogger?: MongoLogger; } diff --git a/src/mongo_logger.ts b/src/mongo_logger.ts index 0107ab888d..f643c39b94 100644 --- a/src/mongo_logger.ts +++ b/src/mongo_logger.ts @@ -3,7 +3,7 @@ import type { Writable } from 'stream'; import { getUint } from './connection_string'; -/** @public */ +/** @internal */ export const SeverityLevel = Object.freeze({ EMERGENCY: 'emergency', ALERT: 'alert', @@ -17,10 +17,10 @@ export const SeverityLevel = Object.freeze({ OFF: 'off' } as const); -/** @public */ +/** @internal */ export type SeverityLevel = typeof SeverityLevel[keyof typeof SeverityLevel]; -/** @returns one of SeverityLevel or null if it is not a valid SeverityLevel */ +/** @returns one of SeverityLevel or null if passsed severity is not a valid SeverityLevel */ function toValidSeverity(severity?: string): SeverityLevel | null { const validSeverities: string[] = Object.values(SeverityLevel); const lowerSeverity = severity?.toLowerCase(); @@ -33,7 +33,7 @@ function toValidSeverity(severity?: string): SeverityLevel | null { } /** @internal */ -export const LoggableComponent = Object.freeze({ +export const MongoLoggableComponent = Object.freeze({ COMMAND: 'command', TOPOLOGY: 'topology', SERVER_SELECTION: 'serverSelection', @@ -41,49 +41,51 @@ export const LoggableComponent = Object.freeze({ } as const); /** @internal */ -type LoggableComponent = typeof LoggableComponent[keyof typeof LoggableComponent]; +export type MongoLoggableComponent = + typeof MongoLoggableComponent[keyof typeof MongoLoggableComponent]; /** @internal */ -export interface LoggerMongoClientOptions { +export interface MongoLoggerMongoClientOptions { mongodbLogPath?: string | Writable; } -/** @public */ -export interface LoggerOptions { +/** @internal */ +export interface MongoLoggerOptions { + /** Severity level for command component */ command: SeverityLevel; + /** Severity level for SDAM */ topology: SeverityLevel; + /** Severity level for server selection component */ serverSelection: SeverityLevel; + /** Severity level for CMAP */ connection: SeverityLevel; + /** Default severity level to be if any of the above are unset */ defaultSeverity: SeverityLevel; + /** Max length of embedded EJSON docs. Setting to 0 disables truncation. Defaults to 1000. */ maxDocumentLength: number; + /** Destination for log messages. Must be 'stderr', 'stdout', a file path, or a Writable. Defaults to 'stderr'. */ logPath: string | Writable; } -/** - * @internal - * TODO(andymina): add docs - */ -export class Logger { - /** @internal */ - componentSeverities: Record; +/** @internal */ +export class MongoLogger { + componentSeverities: Record; maxDocumentLength: number; logPath: Writable; - constructor(options: LoggerOptions) { + constructor(options: MongoLoggerOptions) { // validate log path if (typeof options.logPath === 'string') { this.logPath = options.logPath === 'stderr' || options.logPath === 'stdout' ? process[options.logPath] : fs.createWriteStream(options.logPath, { flags: 'a+' }); + // TODO(NODE-4816): add error handling for creating a write stream } else { this.logPath = options.logPath; } - // extract comp severities this.componentSeverities = options; - - // fill max doc length this.maxDocumentLength = options.maxDocumentLength; } @@ -107,7 +109,18 @@ export class Logger { trace(component: any, message: any): void {} - static resolveOptions(clientOptions?: LoggerMongoClientOptions): LoggerOptions { + /** + * Merges options set through environment variables and the MongoClient, preferring envariables + * when both are set, and substituting defaults for values not set. Options set in constructor + * take precedence over both environment variables and MongoClient options. + * + * When parsing component severity levels, invalid values are treated as unset and replaced with + * the default severity. + * + * @param clientOptions - options set for the logger in the MongoClient options + * @returns a MongoLoggerOptions object to be used when instantiating a new MongoLogger + */ + static resolveOptions(clientOptions?: MongoLoggerMongoClientOptions): MongoLoggerOptions { const defaultSeverity = toValidSeverity(process.env.MONGODB_LOG_ALL) ?? SeverityLevel.OFF; return { From 007b1095971371261e993992225ee50c9e8f8c49 Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Wed, 23 Nov 2022 11:30:22 -0500 Subject: [PATCH 08/50] test: use MongoLogger name in tests --- .../{mongo_logger.ts => mongo_logger.test.ts} | 29 +++++++++++++------ 1 file changed, 20 insertions(+), 9 deletions(-) rename test/unit/{mongo_logger.ts => mongo_logger.test.ts} (62%) diff --git a/test/unit/mongo_logger.ts b/test/unit/mongo_logger.test.ts similarity index 62% rename from test/unit/mongo_logger.ts rename to test/unit/mongo_logger.test.ts index 8c91564e91..01c000a413 100644 --- a/test/unit/mongo_logger.ts +++ b/test/unit/mongo_logger.test.ts @@ -1,10 +1,15 @@ import { expect } from 'chai'; -import { LoggableComponent, Logger, LoggerOptions, SeverityLevel } from '../../src/mongo_logger'; +import { + MongoLoggableComponent, + MongoLogger, + MongoLoggerOptions, + SeverityLevel +} from '../../src/mongo_logger'; describe('Logger', function () { describe('options parsing', function () { - let loggerOptions: LoggerOptions; + let loggerOptions: MongoLoggerOptions; before(function () { // MONGODB_LOG_COMMAND is not set so it defaults to undefined @@ -15,7 +20,7 @@ describe('Logger', function () { process.env.MONGODB_LOG_MAX_DOCUMENT_LENGTH = '100'; process.env.MONGODB_LOG_PATH = 'stderr'; - loggerOptions = Logger.resolveOptions(); + loggerOptions = MongoLogger.resolveOptions(); }); it('treats severity values as case-insensitive', function () { @@ -30,12 +35,18 @@ describe('Logger', function () { }); it('can set severity levels per component', function () { - const { componentSeverities } = new Logger(loggerOptions); - - expect(componentSeverities).property(LoggableComponent.COMMAND, SeverityLevel.ERROR); - expect(componentSeverities).property(LoggableComponent.TOPOLOGY, SeverityLevel.ERROR); - expect(componentSeverities).property(LoggableComponent.SERVER_SELECTION, SeverityLevel.ERROR); - expect(componentSeverities).property(LoggableComponent.CONNECTION, SeverityLevel.CRITICAL); + const { componentSeverities } = new MongoLogger(loggerOptions); + + expect(componentSeverities).property(MongoLoggableComponent.COMMAND, SeverityLevel.ERROR); + expect(componentSeverities).property(MongoLoggableComponent.TOPOLOGY, SeverityLevel.ERROR); + expect(componentSeverities).property( + MongoLoggableComponent.SERVER_SELECTION, + SeverityLevel.ERROR + ); + expect(componentSeverities).property( + MongoLoggableComponent.CONNECTION, + SeverityLevel.CRITICAL + ); }); }); }); From de11ba4b17a1e525eb60557686d8c29f118348df Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Wed, 23 Nov 2022 11:31:51 -0500 Subject: [PATCH 09/50] docs: add remarks tag --- src/mongo_logger.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/mongo_logger.ts b/src/mongo_logger.ts index f643c39b94..01661ddfc6 100644 --- a/src/mongo_logger.ts +++ b/src/mongo_logger.ts @@ -114,6 +114,7 @@ export class MongoLogger { * when both are set, and substituting defaults for values not set. Options set in constructor * take precedence over both environment variables and MongoClient options. * + * @remarks * When parsing component severity levels, invalid values are treated as unset and replaced with * the default severity. * From 5dd9b9035c4b79811f0f0b100ac991dec3b05f7f Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Wed, 23 Nov 2022 11:41:34 -0500 Subject: [PATCH 10/50] fix: correct typo in test --- test/unit/mongo_client.test.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/unit/mongo_client.test.js b/test/unit/mongo_client.test.js index fa9197acb8..c7b0b79f1c 100644 --- a/test/unit/mongo_client.test.js +++ b/test/unit/mongo_client.test.js @@ -7,10 +7,10 @@ const { parseOptions, resolveSRVRecord } = require('../../src/connection_string' const { ReadConcern } = require('../../src/read_concern'); const { WriteConcern } = require('../../src/write_concern'); const { ReadPreference } = require('../../src/read_preference'); -const { Logger } = require('../../src/mongo_logger'); +const { Logger } = require('../../src/logger'); const { MongoCredentials } = require('../../src/cmap/auth/mongo_credentials'); const { MongoClient, MongoParseError, ServerApiVersion } = require('../../src'); -const { SeverityLevel } = require('../../src/mongo_logger'); +const { MongoLogger, SeverityLevel } = require('../../src/mongo_logger'); describe('MongoOptions', function () { it('MongoClient should always freeze public options', function () { @@ -849,7 +849,7 @@ describe('MongoOptions', function () { }); }); - context('logger', function () { + context('MongoLogger', function () { const severityVars = [ 'MONGODB_LOG_COMMAND', 'MONGODB_LOG_TOPOLOGY', @@ -862,7 +862,7 @@ describe('MongoOptions', function () { it(`should enable logging if at least ${name} is set to a valid value`, function () { process.env[name] = SeverityLevel.CRITICAL; const client = new MongoClient('mongodb://localhost:27017'); - expect(client.mongoLogger).to.be.instanceOf(Logger); + expect(client.mongoLogger).to.be.instanceOf(MongoLogger); process.env[name] = undefined; }); } From 07b81293004c23b4bb06d59fd6f803f9e5fd2f15 Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Wed, 23 Nov 2022 15:42:27 -0500 Subject: [PATCH 11/50] fix: address comments from review --- src/connection_string.ts | 39 ++++++++------- src/index.ts | 1 + src/mongo_logger.ts | 105 ++++++++++++++++++++++++++++----------- src/utils.ts | 15 ++++++ 4 files changed, 112 insertions(+), 48 deletions(-) diff --git a/src/connection_string.ts b/src/connection_string.ts index 1b3ba46ac8..d2fe901297 100644 --- a/src/connection_string.ts +++ b/src/connection_string.ts @@ -24,7 +24,12 @@ import { ServerApi, ServerApiVersion } from './mongo_client'; -import { MongoLogger, SeverityLevel } from './mongo_logger'; +import { + MongoLogger, + MongoLoggerEnvOptions, + MongoLoggerMongoClientOptions, + SeverityLevel +} from './mongo_logger'; import { PromiseProvider } from './promise_provider'; import { ReadConcern, ReadConcernLevel } from './read_concern'; import { ReadPreference, ReadPreferenceMode } from './read_preference'; @@ -33,6 +38,8 @@ import { DEFAULT_PK_FACTORY, emitWarning, emitWarningOnce, + getInt, + getUint, HostAddress, isRecord, makeClientMetadata, @@ -200,21 +207,6 @@ function getBoolean(name: string, value: unknown): boolean { throw new MongoParseError(`Expected ${name} to be stringified boolean value, got: ${value}`); } -function getInt(name: string, value: unknown): number { - if (typeof value === 'number') return Math.trunc(value); - const parsedValue = Number.parseInt(String(value), 10); - if (!Number.isNaN(parsedValue)) return parsedValue; - throw new MongoParseError(`Expected ${name} to be stringified int value, got: ${value}`); -} - -export function getUint(name: string, value: unknown): number { - const parsedValue = getInt(name, value); - if (parsedValue < 0) { - throw new MongoParseError(`${name} can only be a positive int value, got: ${value}`); - } - return parsedValue; -} - function* entriesFromString(value: string): Generator<[string, string]> { const keyValuePairs = value.split(','); for (const keyValue of keyValuePairs) { @@ -508,8 +500,19 @@ export function parseOptions( ); } - const loggerMongoClientOptions = { mongodbLogPath: mongoOptions.mongodbLogPath }; - const loggerOptions = MongoLogger.resolveOptions(loggerMongoClientOptions); + const loggerEnvOptions: MongoLoggerEnvOptions = { + MONGODB_LOG_COMMAND: process.env.MONGODB_LOG_COMMAND, + MONGODB_LOG_TOPOLOGY: process.env.MONGODB_LOG_TOPOLOGY, + MONGODB_LOG_SERVER_SELECTION: process.env.MONGODB_LOG_SERVER_SELECTION, + MONGODB_LOG_CONNECTION: process.env.MONGODB_LOG_CONNECTION, + MONGODB_LOG_ALL: process.env.MONGODB_LOG_ALL, + MONGODB_LOG_MAX_DOCUMENT_LENGTH: process.env.MONGODB_LOG_MAX_DOCUMENT_LENGTH, + MONGODB_LOG_PATH: process.env.MONGODB_LOG_PATH + }; + const loggerMongoClientOptions: MongoLoggerMongoClientOptions = { + mongodbLogPath: mongoOptions.mongodbLogPath + }; + const loggerOptions = MongoLogger.resolveOptions(loggerEnvOptions, loggerMongoClientOptions); const loggingComponents = [ loggerOptions.command, loggerOptions.topology, diff --git a/src/index.ts b/src/index.ts index b3cff9a49b..68fdd89b42 100644 --- a/src/index.ts +++ b/src/index.ts @@ -303,6 +303,7 @@ export type { export type { MongoLoggableComponent, MongoLogger, + MongoLoggerEnvOptions, MongoLoggerMongoClientOptions, MongoLoggerOptions, SeverityLevel diff --git a/src/mongo_logger.ts b/src/mongo_logger.ts index 01661ddfc6..fb726e5f8b 100644 --- a/src/mongo_logger.ts +++ b/src/mongo_logger.ts @@ -1,7 +1,7 @@ import * as fs from 'fs'; import type { Writable } from 'stream'; -import { getUint } from './connection_string'; +import { emitWarning, enumToString, getUint } from './utils'; /** @internal */ export const SeverityLevel = Object.freeze({ @@ -20,13 +20,23 @@ export const SeverityLevel = Object.freeze({ /** @internal */ export type SeverityLevel = typeof SeverityLevel[keyof typeof SeverityLevel]; -/** @returns one of SeverityLevel or null if passsed severity is not a valid SeverityLevel */ -function toValidSeverity(severity?: string): SeverityLevel | null { +/** + * Parses a string as one of SeverityLevel + * + * @param name - the name of the variable to be parsed + * @param s - the value to be parsed + * @returns one of SeverityLevel if value can be parsed as such, otherwise null + */ +function parseSeverityFromString(name: string, s?: string): SeverityLevel | null { const validSeverities: string[] = Object.values(SeverityLevel); - const lowerSeverity = severity?.toLowerCase(); + const lowerSeverity = s?.toLowerCase(); - if (lowerSeverity != null && validSeverities.includes(lowerSeverity)) { - return lowerSeverity as SeverityLevel; + if (lowerSeverity != null) { + if (validSeverities.includes(lowerSeverity)) { + return lowerSeverity as SeverityLevel; + } + + emitWarning(`Value for ${name} must be one of ${enumToString(SeverityLevel)}`); } return null; @@ -44,8 +54,27 @@ export const MongoLoggableComponent = Object.freeze({ export type MongoLoggableComponent = typeof MongoLoggableComponent[keyof typeof MongoLoggableComponent]; +/** @internal */ +export interface MongoLoggerEnvOptions { + /** Severity level for command component */ + MONGODB_LOG_COMMAND?: string; + /** Severity level for topology component */ + MONGODB_LOG_TOPOLOGY?: string; + /** Severity level for server selection component */ + MONGODB_LOG_SERVER_SELECTION?: string; + /** Severity level for CMAP */ + MONGODB_LOG_CONNECTION?: string; + /** Default severity level to be if any of the above are unset */ + MONGODB_LOG_ALL?: string; + /** Max length of embedded EJSON docs. Setting to 0 disables truncation. Defaults to 1000. */ + MONGODB_LOG_MAX_DOCUMENT_LENGTH?: string; + /** Destination for log messages. Must be 'stderr', 'stdout', or a file path. Defaults to 'stderr'. */ + MONGODB_LOG_PATH?: string; +} + /** @internal */ export interface MongoLoggerMongoClientOptions { + /** Destination for log messages. Must be 'stderr', 'stdout', a file path, or a Writable. Defaults to 'stderr'. */ mongodbLogPath?: string | Writable; } @@ -53,7 +82,7 @@ export interface MongoLoggerMongoClientOptions { export interface MongoLoggerOptions { /** Severity level for command component */ command: SeverityLevel; - /** Severity level for SDAM */ + /** Severity level for topology component */ topology: SeverityLevel; /** Severity level for server selection component */ serverSelection: SeverityLevel; @@ -64,25 +93,25 @@ export interface MongoLoggerOptions { /** Max length of embedded EJSON docs. Setting to 0 disables truncation. Defaults to 1000. */ maxDocumentLength: number; /** Destination for log messages. Must be 'stderr', 'stdout', a file path, or a Writable. Defaults to 'stderr'. */ - logPath: string | Writable; + logDestination: string | Writable; } /** @internal */ export class MongoLogger { componentSeverities: Record; maxDocumentLength: number; - logPath: Writable; + logDestination: Writable; constructor(options: MongoLoggerOptions) { // validate log path - if (typeof options.logPath === 'string') { - this.logPath = - options.logPath === 'stderr' || options.logPath === 'stdout' - ? process[options.logPath] - : fs.createWriteStream(options.logPath, { flags: 'a+' }); + if (typeof options.logDestination === 'string') { + this.logDestination = + options.logDestination === 'stderr' || options.logDestination === 'stdout' + ? process[options.logDestination] + : fs.createWriteStream(options.logDestination, { flags: 'a+' }); // TODO(NODE-4816): add error handling for creating a write stream } else { - this.logPath = options.logPath; + this.logDestination = options.logDestination; } this.componentSeverities = options; @@ -110,34 +139,50 @@ export class MongoLogger { trace(component: any, message: any): void {} /** - * Merges options set through environment variables and the MongoClient, preferring envariables - * when both are set, and substituting defaults for values not set. Options set in constructor - * take precedence over both environment variables and MongoClient options. + * Merges options set through environment variables and the MongoClient, preferring environment + * variables when both are set, and substituting defaults for values not set. Options set in + * constructor take precedence over both environment variables and MongoClient options. * * @remarks * When parsing component severity levels, invalid values are treated as unset and replaced with * the default severity. * + * @param envOptions - options set for the logger from the environment * @param clientOptions - options set for the logger in the MongoClient options * @returns a MongoLoggerOptions object to be used when instantiating a new MongoLogger + * @throws MongoParseError if MONGODB_LOG_MAX_DOCUMENT_LENGTH is not a non-negative number */ - static resolveOptions(clientOptions?: MongoLoggerMongoClientOptions): MongoLoggerOptions { - const defaultSeverity = toValidSeverity(process.env.MONGODB_LOG_ALL) ?? SeverityLevel.OFF; + static resolveOptions( + envOptions: MongoLoggerEnvOptions, + clientOptions: MongoLoggerMongoClientOptions + ): MongoLoggerOptions { + const defaultSeverity = + parseSeverityFromString('MONGODB_LOG_ALL', envOptions.MONGODB_LOG_ALL) ?? SeverityLevel.OFF; return { - command: toValidSeverity(process.env.MONGODB_LOG_COMMAND) ?? defaultSeverity, - topology: toValidSeverity(process.env.MONGODB_LOG_TOPOLOGY) ?? defaultSeverity, - serverSelection: toValidSeverity(process.env.MONGODB_LOG_SERVER_SELECTION) ?? defaultSeverity, - connection: toValidSeverity(process.env.MONGODB_LOG_CONNECTION) ?? defaultSeverity, + command: + parseSeverityFromString('MONGODB_LOG_COMMAND', envOptions.MONGODB_LOG_COMMAND) ?? + defaultSeverity, + topology: + parseSeverityFromString('MONGODB_LOG_TOPOLOGY', envOptions.MONGODB_LOG_TOPOLOGY) ?? + defaultSeverity, + serverSelection: + parseSeverityFromString( + 'MONGODB_LOG_SERVER_SELECTION', + envOptions.MONGODB_LOG_SERVER_SELECTION + ) ?? defaultSeverity, + connection: + parseSeverityFromString('MONGODB_LOG_CONNECTION', envOptions.MONGODB_LOG_CONNECTION) ?? + defaultSeverity, defaultSeverity, maxDocumentLength: - typeof process.env.MONGODB_LOG_MAX_DOCUMENT_LENGTH === 'string' && - process.env.MONGODB_LOG_MAX_DOCUMENT_LENGTH !== '' - ? getUint('MONGODB_LOG_MAX_DOCUMENT_LENGTH', process.env.MONGODB_LOG_MAX_DOCUMENT_LENGTH) + typeof envOptions.MONGODB_LOG_MAX_DOCUMENT_LENGTH === 'string' && + envOptions.MONGODB_LOG_MAX_DOCUMENT_LENGTH !== '' + ? getUint('MONGODB_LOG_MAX_DOCUMENT_LENGTH', envOptions.MONGODB_LOG_MAX_DOCUMENT_LENGTH) : 1000, - logPath: - typeof process.env.MONGODB_LOG_PATH === 'string' && process.env.MONGODB_LOG_PATH !== '' - ? process.env.MONGODB_LOG_PATH + logDestination: + typeof envOptions.MONGODB_LOG_PATH === 'string' && envOptions.MONGODB_LOG_PATH !== '' + ? envOptions.MONGODB_LOG_PATH : clientOptions?.mongodbLogPath ?? 'stderr' }; } diff --git a/src/utils.ts b/src/utils.ts index 9dc9348e9c..468a22fb6c 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -1421,3 +1421,18 @@ export function compareObjectId(oid1?: ObjectId | null, oid2?: ObjectId | null): return oid1.id.compare(oid2.id); } + +export function getInt(name: string, value: unknown): number { + if (typeof value === 'number') return Math.trunc(value); + const parsedValue = Number.parseInt(String(value), 10); + if (!Number.isNaN(parsedValue)) return parsedValue; + throw new MongoParseError(`Expected ${name} to be stringified int value, got: ${value}`); +} + +export function getUint(name: string, value: unknown): number { + const parsedValue = getInt(name, value); + if (parsedValue < 0) { + throw new MongoParseError(`${name} can only be a positive int value, got: ${value}`); + } + return parsedValue; +} From e62f0427bd6ec709af6d687f6d81acffe3cc729f Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Wed, 23 Nov 2022 15:42:45 -0500 Subject: [PATCH 12/50] test: restructure tests --- test/unit/mongo_client.test.js | 18 ++++----- test/unit/mongo_logger.test.ts | 71 +++++++++++++++------------------- 2 files changed, 41 insertions(+), 48 deletions(-) diff --git a/test/unit/mongo_client.test.js b/test/unit/mongo_client.test.js index c7b0b79f1c..6f47a65efc 100644 --- a/test/unit/mongo_client.test.js +++ b/test/unit/mongo_client.test.js @@ -858,21 +858,21 @@ describe('MongoOptions', function () { 'MONGODB_LOG_ALL' ]; - for (const name of severityVars) { - it(`should enable logging if at least ${name} is set to a valid value`, function () { - process.env[name] = SeverityLevel.CRITICAL; + for (const envName of severityVars) { + it(`should enable logging if ${envName} is set to a valid value`, function () { + const previousValue = process.env[envName]; + process.env[envName] = SeverityLevel.CRITICAL; const client = new MongoClient('mongodb://localhost:27017'); expect(client.mongoLogger).to.be.instanceOf(MongoLogger); - process.env[name] = undefined; + process.env[envName] = previousValue; }); - } - for (const name of severityVars) { - it(`should not enable logging if ${name} is set to an invalid value`, function () { - process.env[name] = 'invalid'; + it(`should not enable logging if ${envName} is the only variable set and it is set to an invalid value`, function () { + const previousValue = process.env[envName]; + process.env[envName] = 'invalid'; const client = new MongoClient('mongodb://localhost:27017'); expect(client).property('mongoLogger', null); - process.env[name] = undefined; + process.env[envName] = previousValue; }); } }); diff --git a/test/unit/mongo_logger.test.ts b/test/unit/mongo_logger.test.ts index 01c000a413..aaeefe25db 100644 --- a/test/unit/mongo_logger.test.ts +++ b/test/unit/mongo_logger.test.ts @@ -1,52 +1,45 @@ import { expect } from 'chai'; -import { - MongoLoggableComponent, - MongoLogger, - MongoLoggerOptions, - SeverityLevel -} from '../../src/mongo_logger'; +import { MongoLogger, SeverityLevel } from '../../src/mongo_logger'; -describe('Logger', function () { +describe('class MongoLogger', function () { describe('options parsing', function () { - let loggerOptions: MongoLoggerOptions; - - before(function () { - // MONGODB_LOG_COMMAND is not set so it defaults to undefined - process.env.MONGODB_LOG_TOPOLOGY = ''; - process.env.MONGODB_LOG_SERVER_SELECTION = 'invalid'; - process.env.MONGODB_LOG_CONNECTION = 'CRITICAL'; - process.env.MONGODB_LOG_ALL = 'eRrOr'; - process.env.MONGODB_LOG_MAX_DOCUMENT_LENGTH = '100'; - process.env.MONGODB_LOG_PATH = 'stderr'; - - loggerOptions = MongoLogger.resolveOptions(); - }); - it('treats severity values as case-insensitive', function () { - expect(loggerOptions.connection).to.equal(SeverityLevel.CRITICAL); - expect(loggerOptions.defaultSeverity).to.equal(SeverityLevel.ERROR); - }); - - it('will only use MONGODB_LOG_ALL for component severities that are not set or invalid', function () { - expect(loggerOptions.command).to.equal(loggerOptions.defaultSeverity); // empty str - expect(loggerOptions.topology).to.equal(loggerOptions.defaultSeverity); // undefined - expect(loggerOptions.serverSelection).to.equal(loggerOptions.defaultSeverity); // invalid + const loggerOptions = MongoLogger.resolveOptions({ MONGODB_LOG_COMMAND: 'EMERGENCY' }, {}); + expect(loggerOptions.connection).to.equal(SeverityLevel.EMERGENCY); }); it('can set severity levels per component', function () { - const { componentSeverities } = new MongoLogger(loggerOptions); - - expect(componentSeverities).property(MongoLoggableComponent.COMMAND, SeverityLevel.ERROR); - expect(componentSeverities).property(MongoLoggableComponent.TOPOLOGY, SeverityLevel.ERROR); - expect(componentSeverities).property( - MongoLoggableComponent.SERVER_SELECTION, - SeverityLevel.ERROR + const loggerOptions = MongoLogger.resolveOptions( + { + MONGODB_LOG_COMMAND: SeverityLevel.EMERGENCY, + MONGODB_LOG_TOPOLOGY: SeverityLevel.CRITICAL, + MONGODB_LOG_SERVER_SELECTION: SeverityLevel.ALERT, + MONGODB_LOG_CONNECTION: SeverityLevel.DEBUG + }, + {} ); - expect(componentSeverities).property( - MongoLoggableComponent.CONNECTION, - SeverityLevel.CRITICAL + expect(loggerOptions.command).to.equal(SeverityLevel.EMERGENCY); + expect(loggerOptions.topology).to.equal(SeverityLevel.CRITICAL); + expect(loggerOptions.serverSelection).to.equal(SeverityLevel.ALERT); + expect(loggerOptions.connection).to.equal(SeverityLevel.DEBUG); + }); + + it('will only use default severity for component severities that are not set or invalid', function () { + const loggerOptions = MongoLogger.resolveOptions( + { + MONGODB_LOG_COMMAND: '', + MONGODB_LOG_TOPOLOGY: undefined, + MONGODB_LOG_SERVER_SELECTION: 'invalid', + MONGODB_LOG_CONNECTION: SeverityLevel.EMERGENCY, + MONGODB_LOG_ALL: SeverityLevel.CRITICAL + }, + {} ); + expect(loggerOptions.command).to.equal(loggerOptions.defaultSeverity); + expect(loggerOptions.topology).to.equal(loggerOptions.defaultSeverity); + expect(loggerOptions.serverSelection).to.equal(loggerOptions.defaultSeverity); + expect(loggerOptions.connection).to.equal(SeverityLevel.EMERGENCY); }); }); }); From c24a6ec8132db806d7ded30e4d756c93f0d8df08 Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Mon, 28 Nov 2022 17:02:24 -0500 Subject: [PATCH 13/50] test: add more tests for MongoLogger --- src/mongo_logger.ts | 40 ++++++++--- test/unit/mongo_client.test.js | 21 +++++- test/unit/mongo_logger.test.ts | 123 +++++++++++++++++++++++++++++++-- 3 files changed, 168 insertions(+), 16 deletions(-) diff --git a/src/mongo_logger.ts b/src/mongo_logger.ts index fb726e5f8b..228c50bf4d 100644 --- a/src/mongo_logger.ts +++ b/src/mongo_logger.ts @@ -1,7 +1,7 @@ import * as fs from 'fs'; import type { Writable } from 'stream'; -import { emitWarning, enumToString, getUint } from './utils'; +import { emitWarning, emitWarningOnce, enumToString } from './utils'; /** @internal */ export const SeverityLevel = Object.freeze({ @@ -36,6 +36,7 @@ function parseSeverityFromString(name: string, s?: string): SeverityLevel | null return lowerSeverity as SeverityLevel; } + // TODO(andymina): revisit this after settling notifying about invalid values emitWarning(`Value for ${name} must be one of ${enumToString(SeverityLevel)}`); } @@ -105,9 +106,10 @@ export class MongoLogger { constructor(options: MongoLoggerOptions) { // validate log path if (typeof options.logDestination === 'string') { + const lowerLogDestination = options.logDestination.toLowerCase(); this.logDestination = - options.logDestination === 'stderr' || options.logDestination === 'stdout' - ? process[options.logDestination] + lowerLogDestination === 'stderr' || lowerLogDestination === 'stdout' + ? process[lowerLogDestination] : fs.createWriteStream(options.logDestination, { flags: 'a+' }); // TODO(NODE-4816): add error handling for creating a write stream } else { @@ -159,6 +161,22 @@ export class MongoLogger { const defaultSeverity = parseSeverityFromString('MONGODB_LOG_ALL', envOptions.MONGODB_LOG_ALL) ?? SeverityLevel.OFF; + let maxDocumentLength = 1000; + if ( + typeof envOptions.MONGODB_LOG_MAX_DOCUMENT_LENGTH === 'string' && + envOptions.MONGODB_LOG_MAX_DOCUMENT_LENGTH !== '' + ) { + const parsedValue = Number.parseInt(envOptions.MONGODB_LOG_MAX_DOCUMENT_LENGTH, 10); + if (!Number.isNaN(parsedValue) && parsedValue >= 0) { + maxDocumentLength = parsedValue; + } else { + // TODO(andymina): revisit this after settling notifying about invalid values + emitWarningOnce( + `MONGODB_MAX_DOCUMENT_LENGTH can only be a positive int value, got: ${envOptions.MONGODB_LOG_MAX_DOCUMENT_LENGTH}` + ); + } + } + return { command: parseSeverityFromString('MONGODB_LOG_COMMAND', envOptions.MONGODB_LOG_COMMAND) ?? @@ -175,15 +193,21 @@ export class MongoLogger { parseSeverityFromString('MONGODB_LOG_CONNECTION', envOptions.MONGODB_LOG_CONNECTION) ?? defaultSeverity, defaultSeverity, - maxDocumentLength: - typeof envOptions.MONGODB_LOG_MAX_DOCUMENT_LENGTH === 'string' && - envOptions.MONGODB_LOG_MAX_DOCUMENT_LENGTH !== '' - ? getUint('MONGODB_LOG_MAX_DOCUMENT_LENGTH', envOptions.MONGODB_LOG_MAX_DOCUMENT_LENGTH) - : 1000, + maxDocumentLength, logDestination: typeof envOptions.MONGODB_LOG_PATH === 'string' && envOptions.MONGODB_LOG_PATH !== '' ? envOptions.MONGODB_LOG_PATH : clientOptions?.mongodbLogPath ?? 'stderr' }; } + + parseMaxDocumentLength(s?: string): number { + if (typeof s === 'string' && s !== '') { + const parsedValue = Number.parseInt(s, 10); + if (!Number.isNaN(parsedValue) && parsedValue >= 0) return parsedValue; + emitWarningOnce(`MONGODB_MAX_DOCUMENT_LENGTH can only be a positive int value, got: ${s}`); + } + + return 1000; + } } diff --git a/test/unit/mongo_client.test.js b/test/unit/mongo_client.test.js index 6f47a65efc..5f4915fcf1 100644 --- a/test/unit/mongo_client.test.js +++ b/test/unit/mongo_client.test.js @@ -859,7 +859,7 @@ describe('MongoOptions', function () { ]; for (const envName of severityVars) { - it(`should enable logging if ${envName} is set to a valid value`, function () { + it(`should instantiate a MongoLogger if ${envName} is set to a valid value`, function () { const previousValue = process.env[envName]; process.env[envName] = SeverityLevel.CRITICAL; const client = new MongoClient('mongodb://localhost:27017'); @@ -867,7 +867,7 @@ describe('MongoOptions', function () { process.env[envName] = previousValue; }); - it(`should not enable logging if ${envName} is the only variable set and it is set to an invalid value`, function () { + it(`should not instatiate a MongoLogger if ${envName} is the only variable set and it is set to an invalid value`, function () { const previousValue = process.env[envName]; process.env[envName] = 'invalid'; const client = new MongoClient('mongodb://localhost:27017'); @@ -875,5 +875,22 @@ describe('MongoOptions', function () { process.env[envName] = previousValue; }); } + + it('should not instatiate a MongoLogger if environment variables are not set', function () { + const client = new MongoClient('mongodb://localhost:27017'); + expect(client).property('mongoLogger', null); + }); + + it('should instantiate a MongoLogger if there is a mix of environment vairables with valid and invalid options', function () { + const { MONGODB_LOG_COMMAND, MONGODB_LOG_TOPOLOGY } = process.env; + process.env['MONGODB_LOG_COMMAND'] = SeverityLevel.CRITICAL; + process.env['MONGODB_LOG_TOPOLOGY'] = 'invalid'; + + const client = new MongoClient('mongodb://localhost:27017'); + expect(client.mongoLogger).to.be.instanceOf(MongoLogger); + + process.env['MONGODB_LOG_COMMAND'] = MONGODB_LOG_COMMAND; + process.env['MONGODB_LOG_TOPOLOGY'] = MONGODB_LOG_TOPOLOGY; + }); }); }); diff --git a/test/unit/mongo_logger.test.ts b/test/unit/mongo_logger.test.ts index aaeefe25db..dc2b5050e3 100644 --- a/test/unit/mongo_logger.test.ts +++ b/test/unit/mongo_logger.test.ts @@ -2,11 +2,30 @@ import { expect } from 'chai'; import { MongoLogger, SeverityLevel } from '../../src/mongo_logger'; -describe('class MongoLogger', function () { - describe('options parsing', function () { +describe.only('class MongoLogger', function () { + describe('static #resolveOptions', function () { it('treats severity values as case-insensitive', function () { - const loggerOptions = MongoLogger.resolveOptions({ MONGODB_LOG_COMMAND: 'EMERGENCY' }, {}); - expect(loggerOptions.connection).to.equal(SeverityLevel.EMERGENCY); + const loggerOptions = MongoLogger.resolveOptions( + { + MONGODB_LOG_COMMAND: 'EMERGENCY', + MONGODB_LOG_TOPOLOGY: 'critical', + MONGODB_LOG_SERVER_SELECTION: 'aLeRt' + }, + {} + ); + expect(loggerOptions.command).to.equal(SeverityLevel.EMERGENCY); + expect(loggerOptions.topology).to.equal(SeverityLevel.CRITICAL); + expect(loggerOptions.serverSelection).to.equal(SeverityLevel.ALERT); + }); + + it('treats invalid severity values as off', function () { + const loggerOptions = MongoLogger.resolveOptions( + { + MONGODB_LOG_COMMAND: 'invalid' + }, + {} + ); + expect(loggerOptions.command).to.equal(SeverityLevel.OFF); }); it('can set severity levels per component', function () { @@ -15,7 +34,8 @@ describe('class MongoLogger', function () { MONGODB_LOG_COMMAND: SeverityLevel.EMERGENCY, MONGODB_LOG_TOPOLOGY: SeverityLevel.CRITICAL, MONGODB_LOG_SERVER_SELECTION: SeverityLevel.ALERT, - MONGODB_LOG_CONNECTION: SeverityLevel.DEBUG + MONGODB_LOG_CONNECTION: SeverityLevel.DEBUG, + MONGODB_LOG_ALL: SeverityLevel.WARNING }, {} ); @@ -23,9 +43,10 @@ describe('class MongoLogger', function () { expect(loggerOptions.topology).to.equal(SeverityLevel.CRITICAL); expect(loggerOptions.serverSelection).to.equal(SeverityLevel.ALERT); expect(loggerOptions.connection).to.equal(SeverityLevel.DEBUG); + expect(loggerOptions.defaultSeverity).to.equal(SeverityLevel.WARNING); }); - it('will only use default severity for component severities that are not set or invalid', function () { + it('only uses the default severity for component severities that are not set or invalid', function () { const loggerOptions = MongoLogger.resolveOptions( { MONGODB_LOG_COMMAND: '', @@ -41,5 +62,95 @@ describe('class MongoLogger', function () { expect(loggerOptions.serverSelection).to.equal(loggerOptions.defaultSeverity); expect(loggerOptions.connection).to.equal(SeverityLevel.EMERGENCY); }); + + it('gives precedence to environment variables over client options if both are set', function () { + const loggerOptions = MongoLogger.resolveOptions( + { MONGODB_LOG_PATH: 'env' }, + { mongodbLogPath: 'client' } + ); + expect(loggerOptions.logDestination).to.equal('env'); + }); + + context('maxDocumentLength', function () { + it('defaults to 1000 if MONGODB_LOG_MAX_DOCUMENT_LENGTH is undefined', function () { + const loggerOptions = MongoLogger.resolveOptions( + { + MONGODB_LOG_MAX_DOCUMENT_LENGTH: undefined + }, + {} + ); + expect(loggerOptions.maxDocumentLength).to.equal(1000); + }); + + it('defaults to 1000 if MONGODB_LOG_MAX_DOCUMENT_LENGTH is an empty string', function () { + const loggerOptions = MongoLogger.resolveOptions( + { + MONGODB_LOG_MAX_DOCUMENT_LENGTH: '' + }, + {} + ); + expect(loggerOptions.maxDocumentLength).to.equal(1000); + }); + + it('defaults to 1000 if MONGODB_LOG_MAX_DOCUMENT_LENGTH cannot be parsed as a uint', function () { + const loggerOptions = MongoLogger.resolveOptions( + { + MONGODB_LOG_MAX_DOCUMENT_LENGTH: 'invalid' + }, + {} + ); + expect(loggerOptions.maxDocumentLength).to.equal(1000); + }); + + it('uses the passed value if MONGODB_LOG_MAX_DOCUMENT_LENGTH can be parsed a uint', function () { + const loggerOptions = MongoLogger.resolveOptions( + { + MONGODB_LOG_MAX_DOCUMENT_LENGTH: '500' + }, + {} + ); + expect(loggerOptions.maxDocumentLength).to.equal(500); + }); + }); + + context('logDestination', function () { + it('defaults to stderr if MONGODB_LOG_PATH and mongodbLogPath are undefined', function () { + const loggerOptions = MongoLogger.resolveOptions( + { MONGODB_LOG_PATH: undefined }, + { mongodbLogPath: undefined } + ); + expect(loggerOptions.logDestination).to.equal('stderr'); + }); + + it('defaults to stderr if MONGODB_LOG_PATH is an empty string', function () { + const loggerOptions = MongoLogger.resolveOptions({ MONGODB_LOG_PATH: '' }, {}); + expect(loggerOptions.logDestination).to.equal('stderr'); + }); + + it('uses the passed value of MONGODB_LOG_PATH if it is not undefined and not an empty string', function () { + const loggerOptions = MongoLogger.resolveOptions({ MONGODB_LOG_PATH: 'file.txt' }, {}); + expect(loggerOptions.logDestination).to.equal('file.txt'); + }); + + it('uses the passed value of mongodbLogPath if MONGODB_LOG_PATH is undefined', function () { + const loggerOptions = MongoLogger.resolveOptions( + { MONGODB_LOG_PATH: undefined }, + { mongodbLogPath: 'file.txt' } + ); + expect(loggerOptions.logDestination).to.equal('file.txt'); + }); + }); + }); + + it('treats loggerOptions.logDestination value of stderr as case-insensitve', function () { + const loggerOptions = MongoLogger.resolveOptions({ MONGODB_LOG_PATH: 'STDERR' }, {}); + const logger = new MongoLogger(loggerOptions); + expect(logger.logDestination).to.equal(process['stderr']); + }); + + it('treats loggerOptions.logDestination value of stdout as case-insensitve', function () { + const loggerOptions = MongoLogger.resolveOptions({ MONGODB_LOG_PATH: 'STDOUT' }, {}); + const logger = new MongoLogger(loggerOptions); + expect(logger.logDestination).to.equal(process['stdout']); }); }); From 84a116fca2e0d984e118ffa95927900619ed9cd3 Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Mon, 28 Nov 2022 17:06:15 -0500 Subject: [PATCH 14/50] refactor: revert changes to getInt and getUint --- src/connection_string.ts | 17 +++++++++++++++-- src/utils.ts | 15 --------------- test/unit/mongo_client.test.js | 2 +- test/unit/mongo_logger.test.ts | 2 +- 4 files changed, 17 insertions(+), 19 deletions(-) diff --git a/src/connection_string.ts b/src/connection_string.ts index d2fe901297..e3946b6931 100644 --- a/src/connection_string.ts +++ b/src/connection_string.ts @@ -38,8 +38,6 @@ import { DEFAULT_PK_FACTORY, emitWarning, emitWarningOnce, - getInt, - getUint, HostAddress, isRecord, makeClientMetadata, @@ -610,6 +608,21 @@ function setOption( } } +function getInt(name: string, value: unknown): number { + if (typeof value === 'number') return Math.trunc(value); + const parsedValue = Number.parseInt(String(value), 10); + if (!Number.isNaN(parsedValue)) return parsedValue; + throw new MongoParseError(`Expected ${name} to be stringified int value, got: ${value}`); +} + +function getUint(name: string, value: unknown): number { + const parsedValue = getInt(name, value); + if (parsedValue < 0) { + throw new MongoParseError(`${name} can only be a positive int value, got: ${value}`); + } + return parsedValue; +} + interface OptionDescriptor { target?: string; type?: 'boolean' | 'int' | 'uint' | 'record' | 'string' | 'any'; diff --git a/src/utils.ts b/src/utils.ts index 468a22fb6c..9dc9348e9c 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -1421,18 +1421,3 @@ export function compareObjectId(oid1?: ObjectId | null, oid2?: ObjectId | null): return oid1.id.compare(oid2.id); } - -export function getInt(name: string, value: unknown): number { - if (typeof value === 'number') return Math.trunc(value); - const parsedValue = Number.parseInt(String(value), 10); - if (!Number.isNaN(parsedValue)) return parsedValue; - throw new MongoParseError(`Expected ${name} to be stringified int value, got: ${value}`); -} - -export function getUint(name: string, value: unknown): number { - const parsedValue = getInt(name, value); - if (parsedValue < 0) { - throw new MongoParseError(`${name} can only be a positive int value, got: ${value}`); - } - return parsedValue; -} diff --git a/test/unit/mongo_client.test.js b/test/unit/mongo_client.test.js index 5f4915fcf1..0d3460cef3 100644 --- a/test/unit/mongo_client.test.js +++ b/test/unit/mongo_client.test.js @@ -849,7 +849,7 @@ describe('MongoOptions', function () { }); }); - context('MongoLogger', function () { + context.only('MongoLogger', function () { const severityVars = [ 'MONGODB_LOG_COMMAND', 'MONGODB_LOG_TOPOLOGY', diff --git a/test/unit/mongo_logger.test.ts b/test/unit/mongo_logger.test.ts index dc2b5050e3..b386f2e0fb 100644 --- a/test/unit/mongo_logger.test.ts +++ b/test/unit/mongo_logger.test.ts @@ -2,7 +2,7 @@ import { expect } from 'chai'; import { MongoLogger, SeverityLevel } from '../../src/mongo_logger'; -describe.only('class MongoLogger', function () { +describe('class MongoLogger', function () { describe('static #resolveOptions', function () { it('treats severity values as case-insensitive', function () { const loggerOptions = MongoLogger.resolveOptions( From 67882434d2877bdd11cf6c769db9c1235b0106e7 Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Mon, 28 Nov 2022 17:08:16 -0500 Subject: [PATCH 15/50] fix: remove .only from tests --- test/unit/mongo_client.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/mongo_client.test.js b/test/unit/mongo_client.test.js index 0d3460cef3..5f4915fcf1 100644 --- a/test/unit/mongo_client.test.js +++ b/test/unit/mongo_client.test.js @@ -849,7 +849,7 @@ describe('MongoOptions', function () { }); }); - context.only('MongoLogger', function () { + context('MongoLogger', function () { const severityVars = [ 'MONGODB_LOG_COMMAND', 'MONGODB_LOG_TOPOLOGY', From 24010a61807fcd7153f39e3d46539a097dcc9f1f Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Tue, 29 Nov 2022 14:15:35 -0500 Subject: [PATCH 16/50] refactor: ignore invalid env vars --- src/mongo_logger.ts | 73 ++++++++++++++------------------------------- 1 file changed, 23 insertions(+), 50 deletions(-) diff --git a/src/mongo_logger.ts b/src/mongo_logger.ts index 228c50bf4d..a5b7ab405f 100644 --- a/src/mongo_logger.ts +++ b/src/mongo_logger.ts @@ -27,22 +27,31 @@ export type SeverityLevel = typeof SeverityLevel[keyof typeof SeverityLevel]; * @param s - the value to be parsed * @returns one of SeverityLevel if value can be parsed as such, otherwise null */ -function parseSeverityFromString(name: string, s?: string): SeverityLevel | null { +function parseSeverityFromString(s?: string): SeverityLevel | null { const validSeverities: string[] = Object.values(SeverityLevel); const lowerSeverity = s?.toLowerCase(); - if (lowerSeverity != null) { - if (validSeverities.includes(lowerSeverity)) { - return lowerSeverity as SeverityLevel; - } - - // TODO(andymina): revisit this after settling notifying about invalid values - emitWarning(`Value for ${name} must be one of ${enumToString(SeverityLevel)}`); + if (lowerSeverity != null && validSeverities.includes(lowerSeverity)) { + return lowerSeverity as SeverityLevel; } return null; } +/** + * Parses a string to be a number greater than or equal to 0 for maxDocumentLength. + * + * @param s - the value to be parsed + * @returns the int value parsed or 1000 if the value could not be parsed + */ +function parseMaxDocumentLength(s?: string): number { + if (typeof s === 'string' && s !== '') { + const parsedValue = Number.parseInt(s, 10); + return !Number.isNaN(parsedValue) && parsedValue >= 0 ? parsedValue : 1000; + } + return 1000; +} + /** @internal */ export const MongoLoggableComponent = Object.freeze({ COMMAND: 'command', @@ -152,62 +161,26 @@ export class MongoLogger { * @param envOptions - options set for the logger from the environment * @param clientOptions - options set for the logger in the MongoClient options * @returns a MongoLoggerOptions object to be used when instantiating a new MongoLogger - * @throws MongoParseError if MONGODB_LOG_MAX_DOCUMENT_LENGTH is not a non-negative number */ static resolveOptions( envOptions: MongoLoggerEnvOptions, clientOptions: MongoLoggerMongoClientOptions ): MongoLoggerOptions { const defaultSeverity = - parseSeverityFromString('MONGODB_LOG_ALL', envOptions.MONGODB_LOG_ALL) ?? SeverityLevel.OFF; - - let maxDocumentLength = 1000; - if ( - typeof envOptions.MONGODB_LOG_MAX_DOCUMENT_LENGTH === 'string' && - envOptions.MONGODB_LOG_MAX_DOCUMENT_LENGTH !== '' - ) { - const parsedValue = Number.parseInt(envOptions.MONGODB_LOG_MAX_DOCUMENT_LENGTH, 10); - if (!Number.isNaN(parsedValue) && parsedValue >= 0) { - maxDocumentLength = parsedValue; - } else { - // TODO(andymina): revisit this after settling notifying about invalid values - emitWarningOnce( - `MONGODB_MAX_DOCUMENT_LENGTH can only be a positive int value, got: ${envOptions.MONGODB_LOG_MAX_DOCUMENT_LENGTH}` - ); - } - } + parseSeverityFromString(envOptions.MONGODB_LOG_ALL) ?? SeverityLevel.OFF; return { - command: - parseSeverityFromString('MONGODB_LOG_COMMAND', envOptions.MONGODB_LOG_COMMAND) ?? - defaultSeverity, - topology: - parseSeverityFromString('MONGODB_LOG_TOPOLOGY', envOptions.MONGODB_LOG_TOPOLOGY) ?? - defaultSeverity, + command: parseSeverityFromString(envOptions.MONGODB_LOG_COMMAND) ?? defaultSeverity, + topology: parseSeverityFromString(envOptions.MONGODB_LOG_TOPOLOGY) ?? defaultSeverity, serverSelection: - parseSeverityFromString( - 'MONGODB_LOG_SERVER_SELECTION', - envOptions.MONGODB_LOG_SERVER_SELECTION - ) ?? defaultSeverity, - connection: - parseSeverityFromString('MONGODB_LOG_CONNECTION', envOptions.MONGODB_LOG_CONNECTION) ?? - defaultSeverity, + parseSeverityFromString(envOptions.MONGODB_LOG_SERVER_SELECTION) ?? defaultSeverity, + connection: parseSeverityFromString(envOptions.MONGODB_LOG_CONNECTION) ?? defaultSeverity, defaultSeverity, - maxDocumentLength, + maxDocumentLength: parseMaxDocumentLength(envOptions.MONGODB_LOG_MAX_DOCUMENT_LENGTH), logDestination: typeof envOptions.MONGODB_LOG_PATH === 'string' && envOptions.MONGODB_LOG_PATH !== '' ? envOptions.MONGODB_LOG_PATH : clientOptions?.mongodbLogPath ?? 'stderr' }; } - - parseMaxDocumentLength(s?: string): number { - if (typeof s === 'string' && s !== '') { - const parsedValue = Number.parseInt(s, 10); - if (!Number.isNaN(parsedValue) && parsedValue >= 0) return parsedValue; - emitWarningOnce(`MONGODB_MAX_DOCUMENT_LENGTH can only be a positive int value, got: ${s}`); - } - - return 1000; - } } From 7dafcfa622addb949ed63aaedd55b367fc72b8c3 Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Tue, 29 Nov 2022 14:15:58 -0500 Subject: [PATCH 17/50] fix: remove unused imports --- src/mongo_logger.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/mongo_logger.ts b/src/mongo_logger.ts index a5b7ab405f..bf00251a0f 100644 --- a/src/mongo_logger.ts +++ b/src/mongo_logger.ts @@ -1,8 +1,6 @@ import * as fs from 'fs'; import type { Writable } from 'stream'; -import { emitWarning, emitWarningOnce, enumToString } from './utils'; - /** @internal */ export const SeverityLevel = Object.freeze({ EMERGENCY: 'emergency', From d41bcdaaca060c808a647ce168b8ec36a9cb06c3 Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Tue, 29 Nov 2022 14:49:30 -0500 Subject: [PATCH 18/50] refactor: only accept out or err for logDestination --- src/mongo_logger.ts | 23 +++++++---------------- test/unit/mongo_logger.test.ts | 10 ++++++++-- 2 files changed, 15 insertions(+), 18 deletions(-) diff --git a/src/mongo_logger.ts b/src/mongo_logger.ts index bf00251a0f..dda7f50b45 100644 --- a/src/mongo_logger.ts +++ b/src/mongo_logger.ts @@ -82,8 +82,8 @@ export interface MongoLoggerEnvOptions { /** @internal */ export interface MongoLoggerMongoClientOptions { - /** Destination for log messages. Must be 'stderr', 'stdout', a file path, or a Writable. Defaults to 'stderr'. */ - mongodbLogPath?: string | Writable; + /** Destination for log messages. Must be 'stderr' or 'stdout'. Defaults to 'stderr'. */ + mongodbLogPath?: string; } /** @internal */ @@ -100,8 +100,8 @@ export interface MongoLoggerOptions { defaultSeverity: SeverityLevel; /** Max length of embedded EJSON docs. Setting to 0 disables truncation. Defaults to 1000. */ maxDocumentLength: number; - /** Destination for log messages. Must be 'stderr', 'stdout', a file path, or a Writable. Defaults to 'stderr'. */ - logDestination: string | Writable; + /** Destination for log messages. Must be 'stderr' or 'stdout'. Defaults to 'stderr'. */ + logDestination: string; } /** @internal */ @@ -111,18 +111,9 @@ export class MongoLogger { logDestination: Writable; constructor(options: MongoLoggerOptions) { - // validate log path - if (typeof options.logDestination === 'string') { - const lowerLogDestination = options.logDestination.toLowerCase(); - this.logDestination = - lowerLogDestination === 'stderr' || lowerLogDestination === 'stdout' - ? process[lowerLogDestination] - : fs.createWriteStream(options.logDestination, { flags: 'a+' }); - // TODO(NODE-4816): add error handling for creating a write stream - } else { - this.logDestination = options.logDestination; - } - + // TODO(NODE-4849): add filepath and Writable support + this.logDestination = + options.logDestination.toLowerCase() === 'stdout' ? process['stdout'] : process['stderr']; this.componentSeverities = options; this.maxDocumentLength = options.maxDocumentLength; } diff --git a/test/unit/mongo_logger.test.ts b/test/unit/mongo_logger.test.ts index b386f2e0fb..bab724db71 100644 --- a/test/unit/mongo_logger.test.ts +++ b/test/unit/mongo_logger.test.ts @@ -142,15 +142,21 @@ describe('class MongoLogger', function () { }); }); - it('treats loggerOptions.logDestination value of stderr as case-insensitve', function () { + it('treats the log destination value of stderr as case-insensitve', function () { const loggerOptions = MongoLogger.resolveOptions({ MONGODB_LOG_PATH: 'STDERR' }, {}); const logger = new MongoLogger(loggerOptions); expect(logger.logDestination).to.equal(process['stderr']); }); - it('treats loggerOptions.logDestination value of stdout as case-insensitve', function () { + it('treats the log destination value of stdout as case-insensitve', function () { const loggerOptions = MongoLogger.resolveOptions({ MONGODB_LOG_PATH: 'STDOUT' }, {}); const logger = new MongoLogger(loggerOptions); expect(logger.logDestination).to.equal(process['stdout']); }); + + it('sets the log destination to stderr if an invalid value is passed', function () { + const loggerOptions = MongoLogger.resolveOptions({ MONGODB_LOG_PATH: 'invalid' }, {}); + const logger = new MongoLogger(loggerOptions); + expect(logger.logDestination).to.equal(process['stderr']); + }); }); From 893e464519a637bece2b1dba74d1bbf92ddcf521 Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Tue, 29 Nov 2022 15:41:00 -0500 Subject: [PATCH 19/50] feat: add logger feature flag --- src/connection_string.ts | 56 +++++++++++-------- .../node-specific/feature_flags.test.ts | 2 + test/unit/mongo_client.test.js | 31 +++++----- 3 files changed, 52 insertions(+), 37 deletions(-) diff --git a/src/connection_string.ts b/src/connection_string.ts index e3946b6931..76b152e853 100644 --- a/src/connection_string.ts +++ b/src/connection_string.ts @@ -498,29 +498,34 @@ export function parseOptions( ); } - const loggerEnvOptions: MongoLoggerEnvOptions = { - MONGODB_LOG_COMMAND: process.env.MONGODB_LOG_COMMAND, - MONGODB_LOG_TOPOLOGY: process.env.MONGODB_LOG_TOPOLOGY, - MONGODB_LOG_SERVER_SELECTION: process.env.MONGODB_LOG_SERVER_SELECTION, - MONGODB_LOG_CONNECTION: process.env.MONGODB_LOG_CONNECTION, - MONGODB_LOG_ALL: process.env.MONGODB_LOG_ALL, - MONGODB_LOG_MAX_DOCUMENT_LENGTH: process.env.MONGODB_LOG_MAX_DOCUMENT_LENGTH, - MONGODB_LOG_PATH: process.env.MONGODB_LOG_PATH - }; - const loggerMongoClientOptions: MongoLoggerMongoClientOptions = { - mongodbLogPath: mongoOptions.mongodbLogPath - }; - const loggerOptions = MongoLogger.resolveOptions(loggerEnvOptions, loggerMongoClientOptions); - const loggingComponents = [ - loggerOptions.command, - loggerOptions.topology, - loggerOptions.serverSelection, - loggerOptions.connection, - loggerOptions.defaultSeverity - ]; - - if (loggingComponents.some(severity => severity !== SeverityLevel.OFF)) { - mongoOptions.mongoLogger = new MongoLogger(loggerOptions); + const loggerFeatureFlag = Symbol.for('@@mdb.enableMongoLogger'); + mongoOptions[loggerFeatureFlag] = mongoOptions[loggerFeatureFlag] ?? false; + + if (mongoOptions[loggerFeatureFlag]) { + const loggerEnvOptions: MongoLoggerEnvOptions = { + MONGODB_LOG_COMMAND: process.env.MONGODB_LOG_COMMAND, + MONGODB_LOG_TOPOLOGY: process.env.MONGODB_LOG_TOPOLOGY, + MONGODB_LOG_SERVER_SELECTION: process.env.MONGODB_LOG_SERVER_SELECTION, + MONGODB_LOG_CONNECTION: process.env.MONGODB_LOG_CONNECTION, + MONGODB_LOG_ALL: process.env.MONGODB_LOG_ALL, + MONGODB_LOG_MAX_DOCUMENT_LENGTH: process.env.MONGODB_LOG_MAX_DOCUMENT_LENGTH, + MONGODB_LOG_PATH: process.env.MONGODB_LOG_PATH + }; + const loggerMongoClientOptions: MongoLoggerMongoClientOptions = { + mongodbLogPath: mongoOptions.mongodbLogPath + }; + const loggerOptions = MongoLogger.resolveOptions(loggerEnvOptions, loggerMongoClientOptions); + const loggingComponents = [ + loggerOptions.command, + loggerOptions.topology, + loggerOptions.serverSelection, + loggerOptions.connection, + loggerOptions.defaultSeverity + ]; + + if (loggingComponents.some(severity => severity !== SeverityLevel.OFF)) { + mongoOptions.mongoLogger = new MongoLogger(loggerOptions); + } } return mongoOptions; @@ -1308,4 +1313,7 @@ export const DEFAULT_OPTIONS = new CaseInsensitiveMap( * Set of permitted feature flags * @internal */ -export const FEATURE_FLAGS = new Set([Symbol.for('@@mdb.skipPingOnConnect')]); +export const FEATURE_FLAGS = new Set([ + Symbol.for('@@mdb.skipPingOnConnect'), + Symbol.for('@@mdb.enableMongoLogger') +]); diff --git a/test/integration/node-specific/feature_flags.test.ts b/test/integration/node-specific/feature_flags.test.ts index 6bf9a9a091..3c13eec63f 100644 --- a/test/integration/node-specific/feature_flags.test.ts +++ b/test/integration/node-specific/feature_flags.test.ts @@ -42,4 +42,6 @@ describe('Feature Flags', () => { }); } }); + + describe('@@mdb.enableMongoLogger', () => {}); }); diff --git a/test/unit/mongo_client.test.js b/test/unit/mongo_client.test.js index 5f4915fcf1..9bdf5b49b0 100644 --- a/test/unit/mongo_client.test.js +++ b/test/unit/mongo_client.test.js @@ -850,6 +850,8 @@ describe('MongoOptions', function () { }); context('MongoLogger', function () { + let cachedEnvironment; + const loggerFeatureFlag = Symbol.for('@@mdb.enableMongoLogger'); const severityVars = [ 'MONGODB_LOG_COMMAND', 'MONGODB_LOG_TOPOLOGY', @@ -858,39 +860,42 @@ describe('MongoOptions', function () { 'MONGODB_LOG_ALL' ]; + before(() => { + cachedEnvironment = process.env; + }); + + after(() => { + process.env = cachedEnvironment; + }); + + beforeEach(() => { + process.env = {}; + }); + for (const envName of severityVars) { it(`should instantiate a MongoLogger if ${envName} is set to a valid value`, function () { - const previousValue = process.env[envName]; process.env[envName] = SeverityLevel.CRITICAL; - const client = new MongoClient('mongodb://localhost:27017'); + const client = new MongoClient('mongodb://localhost:27017', { [loggerFeatureFlag]: true }); expect(client.mongoLogger).to.be.instanceOf(MongoLogger); - process.env[envName] = previousValue; }); it(`should not instatiate a MongoLogger if ${envName} is the only variable set and it is set to an invalid value`, function () { - const previousValue = process.env[envName]; process.env[envName] = 'invalid'; - const client = new MongoClient('mongodb://localhost:27017'); + const client = new MongoClient('mongodb://localhost:27017', { [loggerFeatureFlag]: true }); expect(client).property('mongoLogger', null); - process.env[envName] = previousValue; }); } it('should not instatiate a MongoLogger if environment variables are not set', function () { - const client = new MongoClient('mongodb://localhost:27017'); + const client = new MongoClient('mongodb://localhost:27017', { [loggerFeatureFlag]: true }); expect(client).property('mongoLogger', null); }); it('should instantiate a MongoLogger if there is a mix of environment vairables with valid and invalid options', function () { - const { MONGODB_LOG_COMMAND, MONGODB_LOG_TOPOLOGY } = process.env; process.env['MONGODB_LOG_COMMAND'] = SeverityLevel.CRITICAL; process.env['MONGODB_LOG_TOPOLOGY'] = 'invalid'; - - const client = new MongoClient('mongodb://localhost:27017'); + const client = new MongoClient('mongodb://localhost:27017', { [loggerFeatureFlag]: true }); expect(client.mongoLogger).to.be.instanceOf(MongoLogger); - - process.env['MONGODB_LOG_COMMAND'] = MONGODB_LOG_COMMAND; - process.env['MONGODB_LOG_TOPOLOGY'] = MONGODB_LOG_TOPOLOGY; }); }); }); From 0b7b6e369ee46186feeaf22d7d9e0c383bbf893e Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Tue, 29 Nov 2022 16:48:51 -0500 Subject: [PATCH 20/50] test: add feature flag tests --- .../node-specific/feature_flags.test.ts | 34 ++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/test/integration/node-specific/feature_flags.test.ts b/test/integration/node-specific/feature_flags.test.ts index 3c13eec63f..cfd2f6f365 100644 --- a/test/integration/node-specific/feature_flags.test.ts +++ b/test/integration/node-specific/feature_flags.test.ts @@ -1,5 +1,8 @@ import { expect } from 'chai'; +import { MongoClient } from '../../../src'; +import { MongoLogger } from '../../../src/mongo_logger'; + describe('Feature Flags', () => { describe('@@mdb.skipPingOnConnect', () => { beforeEach(function () { @@ -43,5 +46,34 @@ describe('Feature Flags', () => { } }); - describe('@@mdb.enableMongoLogger', () => {}); + describe('@@mdb.enableMongoLogger', () => { + let cachedEnv; + const loggerFeatureFlag = Symbol.for('@@mdb.enableMongoLogger'); + + before(() => { + cachedEnv = process.env; + process.env['MONGODB_LOG_COMMAND'] = 'emergency'; + }); + + after(() => { + process.env = cachedEnv; + }); + + it('should instantiate a MongoLogger when set to true', () => { + const client = new MongoClient('mongodb://localhost:27017', { [loggerFeatureFlag]: true }); + expect(client.mongoLogger).to.be.instanceOf(MongoLogger); + }); + + it('should not instantiate a MongoLogger when set to false', () => { + const client = new MongoClient('mongodb://localhost:27017', { [loggerFeatureFlag]: false }); + expect(client).property('mongoLogger', null); + }); + + it('should instantiate a MongoLogger when set to undefined', () => { + const client = new MongoClient('mongodb://localhost:27017', { + [loggerFeatureFlag]: undefined + }); + expect(client).property('mongoLogger', null); + }); + }); }); From 0cfda96779498761ec552ee5900c67635e17d9b2 Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Tue, 29 Nov 2022 16:50:42 -0500 Subject: [PATCH 21/50] fix: remove unused import in mongo_logger --- src/mongo_logger.ts | 1 - test/integration/node-specific/feature_flags.test.ts | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/mongo_logger.ts b/src/mongo_logger.ts index dda7f50b45..b47319d0e5 100644 --- a/src/mongo_logger.ts +++ b/src/mongo_logger.ts @@ -1,4 +1,3 @@ -import * as fs from 'fs'; import type { Writable } from 'stream'; /** @internal */ diff --git a/test/integration/node-specific/feature_flags.test.ts b/test/integration/node-specific/feature_flags.test.ts index cfd2f6f365..a09f76c485 100644 --- a/test/integration/node-specific/feature_flags.test.ts +++ b/test/integration/node-specific/feature_flags.test.ts @@ -25,7 +25,7 @@ describe('Feature Flags', () => { for (const { description, value, expectEvents } of tests) { it(description, async function () { const options = - value === undefined ? {} : { [Symbol.for('@@mdb.skipPingOnConnect')]: value }; + value === undefined ? {} : { [Symbol.for('@@mdb.skipPingOnConnect')]: value }; const client = this.configuration.newClient({}, { ...options, monitorCommands: true }); const events = []; client.on('commandStarted', event => events.push(event)); From f66f973fb1065084542b4e070a3f27caf6a5d750 Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Tue, 29 Nov 2022 18:12:54 -0500 Subject: [PATCH 22/50] fix: manually add feature flag to test --- test/integration/node-specific/feature_flags.test.ts | 2 +- test/unit/connection_string.test.ts | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/test/integration/node-specific/feature_flags.test.ts b/test/integration/node-specific/feature_flags.test.ts index a09f76c485..cfd2f6f365 100644 --- a/test/integration/node-specific/feature_flags.test.ts +++ b/test/integration/node-specific/feature_flags.test.ts @@ -25,7 +25,7 @@ describe('Feature Flags', () => { for (const { description, value, expectEvents } of tests) { it(description, async function () { const options = - value === undefined ? {} : { [Symbol.for('@@mdb.skipPingOnConnect')]: value }; + value === undefined ? {} : { [Symbol.for('@@mdb.skipPingOnConnect')]: value }; const client = this.configuration.newClient({}, { ...options, monitorCommands: true }); const events = []; client.on('commandStarted', event => events.push(event)); diff --git a/test/unit/connection_string.test.ts b/test/unit/connection_string.test.ts index f2a825db7d..62d0cee692 100644 --- a/test/unit/connection_string.test.ts +++ b/test/unit/connection_string.test.ts @@ -539,8 +539,9 @@ describe('Connection String', function () { describe('feature flags', () => { it('should be stored in the FEATURE_FLAGS Set', () => { - expect(FEATURE_FLAGS.size).to.equal(1); + expect(FEATURE_FLAGS.size).to.equal(2); expect(FEATURE_FLAGS.has(Symbol.for('@@mdb.skipPingOnConnect'))).to.be.true; + expect(FEATURE_FLAGS.has(Symbol.for('@@mdb.enableMongoLogger'))).to.be.true; // Add more flags here }); From 159aac0510011e4f41fb0bfcf6577929c49d2865 Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Wed, 30 Nov 2022 12:01:27 -0500 Subject: [PATCH 23/50] test: reformat testing structure --- src/connection_string.ts | 17 +------ src/mongo_logger.ts | 13 +++-- src/utils.ts | 15 ++++++ .../node-specific/feature_flags.test.ts | 2 +- test/unit/mongo_client.test.js | 50 ++++++++++++------- test/unit/mongo_logger.test.ts | 38 +++++++------- 6 files changed, 78 insertions(+), 57 deletions(-) diff --git a/src/connection_string.ts b/src/connection_string.ts index 76b152e853..c53050c626 100644 --- a/src/connection_string.ts +++ b/src/connection_string.ts @@ -38,6 +38,8 @@ import { DEFAULT_PK_FACTORY, emitWarning, emitWarningOnce, + getInt, + getUint, HostAddress, isRecord, makeClientMetadata, @@ -613,21 +615,6 @@ function setOption( } } -function getInt(name: string, value: unknown): number { - if (typeof value === 'number') return Math.trunc(value); - const parsedValue = Number.parseInt(String(value), 10); - if (!Number.isNaN(parsedValue)) return parsedValue; - throw new MongoParseError(`Expected ${name} to be stringified int value, got: ${value}`); -} - -function getUint(name: string, value: unknown): number { - const parsedValue = getInt(name, value); - if (parsedValue < 0) { - throw new MongoParseError(`${name} can only be a positive int value, got: ${value}`); - } - return parsedValue; -} - interface OptionDescriptor { target?: string; type?: 'boolean' | 'int' | 'uint' | 'record' | 'string' | 'any'; diff --git a/src/mongo_logger.ts b/src/mongo_logger.ts index b47319d0e5..1c9733712a 100644 --- a/src/mongo_logger.ts +++ b/src/mongo_logger.ts @@ -1,5 +1,7 @@ import type { Writable } from 'stream'; +import { getUint } from './utils'; + /** @internal */ export const SeverityLevel = Object.freeze({ EMERGENCY: 'emergency', @@ -42,11 +44,12 @@ function parseSeverityFromString(s?: string): SeverityLevel | null { * @returns the int value parsed or 1000 if the value could not be parsed */ function parseMaxDocumentLength(s?: string): number { - if (typeof s === 'string' && s !== '') { - const parsedValue = Number.parseInt(s, 10); - return !Number.isNaN(parsedValue) && parsedValue >= 0 ? parsedValue : 1000; + try { + const maxDocumentLength = getUint('MONGODB_LOG_MAX_DOCUMENT_LENGTH', s); + return maxDocumentLength; + } catch { + return 1000; } - return 1000; } /** @internal */ @@ -168,7 +171,7 @@ export class MongoLogger { logDestination: typeof envOptions.MONGODB_LOG_PATH === 'string' && envOptions.MONGODB_LOG_PATH !== '' ? envOptions.MONGODB_LOG_PATH - : clientOptions?.mongodbLogPath ?? 'stderr' + : clientOptions.mongodbLogPath ?? 'stderr' }; } } diff --git a/src/utils.ts b/src/utils.ts index 9dc9348e9c..468a22fb6c 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -1421,3 +1421,18 @@ export function compareObjectId(oid1?: ObjectId | null, oid2?: ObjectId | null): return oid1.id.compare(oid2.id); } + +export function getInt(name: string, value: unknown): number { + if (typeof value === 'number') return Math.trunc(value); + const parsedValue = Number.parseInt(String(value), 10); + if (!Number.isNaN(parsedValue)) return parsedValue; + throw new MongoParseError(`Expected ${name} to be stringified int value, got: ${value}`); +} + +export function getUint(name: string, value: unknown): number { + const parsedValue = getInt(name, value); + if (parsedValue < 0) { + throw new MongoParseError(`${name} can only be a positive int value, got: ${value}`); + } + return parsedValue; +} diff --git a/test/integration/node-specific/feature_flags.test.ts b/test/integration/node-specific/feature_flags.test.ts index cfd2f6f365..0d833eb89b 100644 --- a/test/integration/node-specific/feature_flags.test.ts +++ b/test/integration/node-specific/feature_flags.test.ts @@ -69,7 +69,7 @@ describe('Feature Flags', () => { expect(client).property('mongoLogger', null); }); - it('should instantiate a MongoLogger when set to undefined', () => { + it('should not instantiate a MongoLogger when set to undefined', () => { const client = new MongoClient('mongodb://localhost:27017', { [loggerFeatureFlag]: undefined }); diff --git a/test/unit/mongo_client.test.js b/test/unit/mongo_client.test.js index 9bdf5b49b0..0a82d17683 100644 --- a/test/unit/mongo_client.test.js +++ b/test/unit/mongo_client.test.js @@ -872,30 +872,44 @@ describe('MongoOptions', function () { process.env = {}; }); - for (const envName of severityVars) { - it(`should instantiate a MongoLogger if ${envName} is set to a valid value`, function () { - process.env[envName] = SeverityLevel.CRITICAL; - const client = new MongoClient('mongodb://localhost:27017', { [loggerFeatureFlag]: true }); - expect(client.mongoLogger).to.be.instanceOf(MongoLogger); - }); + context('when only a single severity enviroment variable is set', function () { + for (const envName of severityVars) { + context(`when ${envName} is set to a valid value`, function () { + it('instantiates a MongoLogger', function () { + process.env[envName] = SeverityLevel.CRITICAL; + const client = new MongoClient('mongodb://localhost:27017', { + [loggerFeatureFlag]: true + }); + expect(client.mongoLogger).to.be.instanceOf(MongoLogger); + }); + }); + + context(`when ${envName} is set to an invalid value`, function () { + it('does not instatiate a MongoLogger', function () { + process.env[envName] = 'invalid'; + const client = new MongoClient('mongodb://localhost:27017', { + [loggerFeatureFlag]: true + }); + expect(client).property('mongoLogger', null); + }); + }); + } + }); - it(`should not instatiate a MongoLogger if ${envName} is the only variable set and it is set to an invalid value`, function () { - process.env[envName] = 'invalid'; + context('when there are no environment variables set', function () { + it('should not instatiate a MongoLogger if environment variables are not set', function () { const client = new MongoClient('mongodb://localhost:27017', { [loggerFeatureFlag]: true }); expect(client).property('mongoLogger', null); }); - } - - it('should not instatiate a MongoLogger if environment variables are not set', function () { - const client = new MongoClient('mongodb://localhost:27017', { [loggerFeatureFlag]: true }); - expect(client).property('mongoLogger', null); }); - it('should instantiate a MongoLogger if there is a mix of environment vairables with valid and invalid options', function () { - process.env['MONGODB_LOG_COMMAND'] = SeverityLevel.CRITICAL; - process.env['MONGODB_LOG_TOPOLOGY'] = 'invalid'; - const client = new MongoClient('mongodb://localhost:27017', { [loggerFeatureFlag]: true }); - expect(client.mongoLogger).to.be.instanceOf(MongoLogger); + context('when there are environment variables with both valid and invalid options', () => { + it('instantiates a MongoLogger', function () { + process.env['MONGODB_LOG_COMMAND'] = SeverityLevel.CRITICAL; + process.env['MONGODB_LOG_TOPOLOGY'] = 'invalid'; + const client = new MongoClient('mongodb://localhost:27017', { [loggerFeatureFlag]: true }); + expect(client.mongoLogger).to.be.instanceOf(MongoLogger); + }); }); }); }); diff --git a/test/unit/mongo_logger.test.ts b/test/unit/mongo_logger.test.ts index bab724db71..49b8075636 100644 --- a/test/unit/mongo_logger.test.ts +++ b/test/unit/mongo_logger.test.ts @@ -3,6 +3,26 @@ import { expect } from 'chai'; import { MongoLogger, SeverityLevel } from '../../src/mongo_logger'; describe('class MongoLogger', function () { + describe('#constructor', function () { + it('treats the log destination value of stderr as case-insensitve', function () { + const loggerOptions = MongoLogger.resolveOptions({ MONGODB_LOG_PATH: 'STDERR' }, {}); + const logger = new MongoLogger(loggerOptions); + expect(logger.logDestination).to.equal(process['stderr']); + }); + + it('treats the log destination value of stdout as case-insensitve', function () { + const loggerOptions = MongoLogger.resolveOptions({ MONGODB_LOG_PATH: 'STDOUT' }, {}); + const logger = new MongoLogger(loggerOptions); + expect(logger.logDestination).to.equal(process['stdout']); + }); + + it('sets the log destination to stderr if an invalid value is passed', function () { + const loggerOptions = MongoLogger.resolveOptions({ MONGODB_LOG_PATH: 'invalid' }, {}); + const logger = new MongoLogger(loggerOptions); + expect(logger.logDestination).to.equal(process['stderr']); + }); + }); + describe('static #resolveOptions', function () { it('treats severity values as case-insensitive', function () { const loggerOptions = MongoLogger.resolveOptions( @@ -141,22 +161,4 @@ describe('class MongoLogger', function () { }); }); }); - - it('treats the log destination value of stderr as case-insensitve', function () { - const loggerOptions = MongoLogger.resolveOptions({ MONGODB_LOG_PATH: 'STDERR' }, {}); - const logger = new MongoLogger(loggerOptions); - expect(logger.logDestination).to.equal(process['stderr']); - }); - - it('treats the log destination value of stdout as case-insensitve', function () { - const loggerOptions = MongoLogger.resolveOptions({ MONGODB_LOG_PATH: 'STDOUT' }, {}); - const logger = new MongoLogger(loggerOptions); - expect(logger.logDestination).to.equal(process['stdout']); - }); - - it('sets the log destination to stderr if an invalid value is passed', function () { - const loggerOptions = MongoLogger.resolveOptions({ MONGODB_LOG_PATH: 'invalid' }, {}); - const logger = new MongoLogger(loggerOptions); - expect(logger.logDestination).to.equal(process['stderr']); - }); }); From c1c04210c263c500a613d46a0facf2a8982550e5 Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Wed, 30 Nov 2022 13:02:32 -0500 Subject: [PATCH 24/50] test: restructure tests to use context --- src/mongo_logger.ts | 15 ++- test/unit/mongo_logger.test.ts | 182 ++++++++++++++++++++------------- 2 files changed, 122 insertions(+), 75 deletions(-) diff --git a/src/mongo_logger.ts b/src/mongo_logger.ts index 1c9733712a..ceff3c7184 100644 --- a/src/mongo_logger.ts +++ b/src/mongo_logger.ts @@ -52,6 +52,16 @@ function parseMaxDocumentLength(s?: string): number { } } +function parseLogDestination(envDestination?: string, clientDestination?: string): string { + if (typeof envDestination === 'string' && envDestination !== '') { + return envDestination; + } else if (typeof clientDestination === 'string' && clientDestination !== '') { + return clientDestination; + } + + return 'stderr'; +} + /** @internal */ export const MongoLoggableComponent = Object.freeze({ COMMAND: 'command', @@ -168,10 +178,7 @@ export class MongoLogger { connection: parseSeverityFromString(envOptions.MONGODB_LOG_CONNECTION) ?? defaultSeverity, defaultSeverity, maxDocumentLength: parseMaxDocumentLength(envOptions.MONGODB_LOG_MAX_DOCUMENT_LENGTH), - logDestination: - typeof envOptions.MONGODB_LOG_PATH === 'string' && envOptions.MONGODB_LOG_PATH !== '' - ? envOptions.MONGODB_LOG_PATH - : clientOptions.mongodbLogPath ?? 'stderr' + logDestination: parseLogDestination(envOptions.MONGODB_LOG_PATH, clientOptions.mongodbLogPath) }; } } diff --git a/test/unit/mongo_logger.test.ts b/test/unit/mongo_logger.test.ts index 49b8075636..2ab6efc18e 100644 --- a/test/unit/mongo_logger.test.ts +++ b/test/unit/mongo_logger.test.ts @@ -66,98 +66,138 @@ describe('class MongoLogger', function () { expect(loggerOptions.defaultSeverity).to.equal(SeverityLevel.WARNING); }); - it('only uses the default severity for component severities that are not set or invalid', function () { - const loggerOptions = MongoLogger.resolveOptions( - { - MONGODB_LOG_COMMAND: '', - MONGODB_LOG_TOPOLOGY: undefined, - MONGODB_LOG_SERVER_SELECTION: 'invalid', - MONGODB_LOG_CONNECTION: SeverityLevel.EMERGENCY, - MONGODB_LOG_ALL: SeverityLevel.CRITICAL - }, - {} - ); - expect(loggerOptions.command).to.equal(loggerOptions.defaultSeverity); - expect(loggerOptions.topology).to.equal(loggerOptions.defaultSeverity); - expect(loggerOptions.serverSelection).to.equal(loggerOptions.defaultSeverity); - expect(loggerOptions.connection).to.equal(SeverityLevel.EMERGENCY); - }); - - it('gives precedence to environment variables over client options if both are set', function () { - const loggerOptions = MongoLogger.resolveOptions( - { MONGODB_LOG_PATH: 'env' }, - { mongodbLogPath: 'client' } - ); - expect(loggerOptions.logDestination).to.equal('env'); - }); - - context('maxDocumentLength', function () { - it('defaults to 1000 if MONGODB_LOG_MAX_DOCUMENT_LENGTH is undefined', function () { + context('when component severities are not set or invalid', function () { + it('only uses the default severity for those components', function () { const loggerOptions = MongoLogger.resolveOptions( { - MONGODB_LOG_MAX_DOCUMENT_LENGTH: undefined + MONGODB_LOG_COMMAND: '', + MONGODB_LOG_TOPOLOGY: undefined, + MONGODB_LOG_SERVER_SELECTION: 'invalid', + MONGODB_LOG_CONNECTION: SeverityLevel.EMERGENCY, + MONGODB_LOG_ALL: SeverityLevel.CRITICAL }, {} ); - expect(loggerOptions.maxDocumentLength).to.equal(1000); + expect(loggerOptions.command).to.equal(loggerOptions.defaultSeverity); + expect(loggerOptions.topology).to.equal(loggerOptions.defaultSeverity); + expect(loggerOptions.serverSelection).to.equal(loggerOptions.defaultSeverity); + expect(loggerOptions.connection).to.equal(SeverityLevel.EMERGENCY); }); + }); - it('defaults to 1000 if MONGODB_LOG_MAX_DOCUMENT_LENGTH is an empty string', function () { + context('when environment variables and client options are both set', function () { + it('gives precedence to environment variables', function () { const loggerOptions = MongoLogger.resolveOptions( - { - MONGODB_LOG_MAX_DOCUMENT_LENGTH: '' - }, - {} + { MONGODB_LOG_PATH: 'env' }, + { mongodbLogPath: 'client' } ); - expect(loggerOptions.maxDocumentLength).to.equal(1000); + expect(loggerOptions.logDestination).to.equal('env'); }); + }); - it('defaults to 1000 if MONGODB_LOG_MAX_DOCUMENT_LENGTH cannot be parsed as a uint', function () { - const loggerOptions = MongoLogger.resolveOptions( - { - MONGODB_LOG_MAX_DOCUMENT_LENGTH: 'invalid' - }, - {} - ); - expect(loggerOptions.maxDocumentLength).to.equal(1000); + context('maxDocumentLength', function () { + context('when MONGODB_LOG_MAX_DOCUMENT_LENGTH is undefined', function () { + it('defaults to 1000', function () { + const loggerOptions = MongoLogger.resolveOptions( + { + MONGODB_LOG_MAX_DOCUMENT_LENGTH: undefined + }, + {} + ); + expect(loggerOptions.maxDocumentLength).to.equal(1000); + }); }); - it('uses the passed value if MONGODB_LOG_MAX_DOCUMENT_LENGTH can be parsed a uint', function () { - const loggerOptions = MongoLogger.resolveOptions( - { - MONGODB_LOG_MAX_DOCUMENT_LENGTH: '500' - }, - {} - ); - expect(loggerOptions.maxDocumentLength).to.equal(500); + context('when MONGODB_LOG_MAX_DOCUMENT_LENGTH is an empty string', function () { + it('defaults to 1000', function () { + const loggerOptions = MongoLogger.resolveOptions( + { + MONGODB_LOG_MAX_DOCUMENT_LENGTH: '' + }, + {} + ); + expect(loggerOptions.maxDocumentLength).to.equal(1000); + }); }); - }); - context('logDestination', function () { - it('defaults to stderr if MONGODB_LOG_PATH and mongodbLogPath are undefined', function () { - const loggerOptions = MongoLogger.resolveOptions( - { MONGODB_LOG_PATH: undefined }, - { mongodbLogPath: undefined } - ); - expect(loggerOptions.logDestination).to.equal('stderr'); + context('when MONGODB_LOG_MAX_DOCUMENT_LENGTH cannot be parsed as a uint', function () { + it('defaults to 1000', function () { + const loggerOptions = MongoLogger.resolveOptions( + { + MONGODB_LOG_MAX_DOCUMENT_LENGTH: 'invalid' + }, + {} + ); + expect(loggerOptions.maxDocumentLength).to.equal(1000); + }); }); - it('defaults to stderr if MONGODB_LOG_PATH is an empty string', function () { - const loggerOptions = MongoLogger.resolveOptions({ MONGODB_LOG_PATH: '' }, {}); - expect(loggerOptions.logDestination).to.equal('stderr'); + context('when MONGODB_LOG_MAX_DOCUMENT_LENGTH can be parsed a uint', function () { + it('uses the passed value', function () { + const loggerOptions = MongoLogger.resolveOptions( + { + MONGODB_LOG_MAX_DOCUMENT_LENGTH: '500' + }, + {} + ); + expect(loggerOptions.maxDocumentLength).to.equal(500); + }); }); + }); - it('uses the passed value of MONGODB_LOG_PATH if it is not undefined and not an empty string', function () { - const loggerOptions = MongoLogger.resolveOptions({ MONGODB_LOG_PATH: 'file.txt' }, {}); - expect(loggerOptions.logDestination).to.equal('file.txt'); + context('logDestination', function () { + context('when mongodbLogPath is undefined', function () { + context('when MONGODB_LOG_PATH is undefined', function () { + it('defaults to stderr', function () { + const loggerOptions = MongoLogger.resolveOptions( + { MONGODB_LOG_PATH: undefined }, + { mongodbLogPath: undefined } + ); + expect(loggerOptions.logDestination).to.equal('stderr'); + }); + }); + + context('when MONGODB_LOG_PATH is an empty string', function () { + it('defaults to stderr', function () { + const loggerOptions = MongoLogger.resolveOptions( + { MONGODB_LOG_PATH: '' }, + { mongodbLogPath: undefined } + ); + expect(loggerOptions.logDestination).to.equal('stderr'); + }); + }); + + context('when MONGODB_LOG_PATH is not an empty string', function () { + it('uses the passed value', function () { + const loggerOptions = MongoLogger.resolveOptions( + { MONGODB_LOG_PATH: 'stdout' }, + { mongodbLogPath: undefined } + ); + expect(loggerOptions.logDestination).to.equal('stdout'); + }); + }); }); - it('uses the passed value of mongodbLogPath if MONGODB_LOG_PATH is undefined', function () { - const loggerOptions = MongoLogger.resolveOptions( - { MONGODB_LOG_PATH: undefined }, - { mongodbLogPath: 'file.txt' } - ); - expect(loggerOptions.logDestination).to.equal('file.txt'); + context('when MONGODB_LOG_PATH is undefined', function () { + context('when mongodbLogPath is an empty string', function () { + it('defaults to stderr', function () { + const loggerOptions = MongoLogger.resolveOptions( + { MONGODB_LOG_PATH: undefined }, + { mongodbLogPath: '' } + ); + expect(loggerOptions.logDestination).to.equal('stderr'); + }); + }); + + context('when mongodbLogPath is not an empty string', function () { + it('uses the passed value ', function () { + const loggerOptions = MongoLogger.resolveOptions( + { MONGODB_LOG_PATH: undefined }, + { mongodbLogPath: 'stdout' } + ); + expect(loggerOptions.logDestination).to.equal('stdout'); + }); + }); }); }); }); From 1a022fe985cd7b1421b9a4b273203315ee9d1ac6 Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Thu, 1 Dec 2022 16:00:51 -0500 Subject: [PATCH 25/50] refactor: address comments --- src/connection_string.ts | 26 +--- src/mongo_client.ts | 6 +- src/mongo_logger.ts | 74 ++++++---- .../node-specific/feature_flags.test.ts | 46 ++++-- test/unit/mongo_client.test.js | 6 +- test/unit/mongo_logger.test.ts | 132 +++++++++++++----- 6 files changed, 190 insertions(+), 100 deletions(-) diff --git a/src/connection_string.ts b/src/connection_string.ts index c53050c626..642900bb2b 100644 --- a/src/connection_string.ts +++ b/src/connection_string.ts @@ -24,12 +24,7 @@ import { ServerApi, ServerApiVersion } from './mongo_client'; -import { - MongoLogger, - MongoLoggerEnvOptions, - MongoLoggerMongoClientOptions, - SeverityLevel -} from './mongo_logger'; +import { MongoLogger, MongoLoggerEnvOptions, MongoLoggerMongoClientOptions } from './mongo_logger'; import { PromiseProvider } from './promise_provider'; import { ReadConcern, ReadConcernLevel } from './read_concern'; import { ReadPreference, ReadPreferenceMode } from './read_preference'; @@ -516,18 +511,10 @@ export function parseOptions( const loggerMongoClientOptions: MongoLoggerMongoClientOptions = { mongodbLogPath: mongoOptions.mongodbLogPath }; - const loggerOptions = MongoLogger.resolveOptions(loggerEnvOptions, loggerMongoClientOptions); - const loggingComponents = [ - loggerOptions.command, - loggerOptions.topology, - loggerOptions.serverSelection, - loggerOptions.connection, - loggerOptions.defaultSeverity - ]; - - if (loggingComponents.some(severity => severity !== SeverityLevel.OFF)) { - mongoOptions.mongoLogger = new MongoLogger(loggerOptions); - } + mongoOptions.mongoLoggerOptions = MongoLogger.resolveOptions( + loggerEnvOptions, + loggerMongoClientOptions + ); } return mongoOptions; @@ -889,9 +876,6 @@ export const OPTIONS = { return new LegacyLogger('MongoClient', { loggerLevel: value as LegacyLoggerLevel }); } }, - mongoLogger: { - target: 'mongoLogger' - }, maxConnecting: { default: 2, transform({ name, values: [value] }): number { diff --git a/src/mongo_client.ts b/src/mongo_client.ts index 0248a0f666..cb472fe4da 100644 --- a/src/mongo_client.ts +++ b/src/mongo_client.ts @@ -16,7 +16,7 @@ import type { AutoEncrypter, AutoEncryptionOptions } from './deps'; import type { Encrypter } from './encrypter'; import { MongoInvalidArgumentError } from './error'; import type { Logger as LegacyLogger, LoggerLevel as LegacyLoggerLevel } from './logger'; -import type { MongoLogger } from './mongo_logger'; +import { MongoLogger, MongoLoggerOptions } from './mongo_logger'; import { TypedEventEmitter } from './mongo_types'; import type { ReadConcern, ReadConcernLevel, ReadConcernLike } from './read_concern'; import { ReadPreference, ReadPreferenceMode } from './read_preference'; @@ -348,7 +348,7 @@ export class MongoClient extends TypedEventEmitter { super(); this[kOptions] = parseOptions(url, this, options); - this.mongoLogger = this[kOptions].mongoLogger ?? null; + this.mongoLogger = MongoLogger.create(this[kOptions].mongoLoggerOptions); // eslint-disable-next-line @typescript-eslint/no-this-alias const client = this; @@ -809,5 +809,5 @@ export interface MongoOptions [featureFlag: symbol]: any; /** @internal */ - mongoLogger?: MongoLogger; + mongoLoggerOptions?: MongoLoggerOptions; } diff --git a/src/mongo_logger.ts b/src/mongo_logger.ts index ceff3c7184..04dbf3ce3a 100644 --- a/src/mongo_logger.ts +++ b/src/mongo_logger.ts @@ -22,7 +22,6 @@ export type SeverityLevel = typeof SeverityLevel[keyof typeof SeverityLevel]; /** * Parses a string as one of SeverityLevel * - * @param name - the name of the variable to be parsed * @param s - the value to be parsed * @returns one of SeverityLevel if value can be parsed as such, otherwise null */ @@ -52,11 +51,12 @@ function parseMaxDocumentLength(s?: string): number { } } -function parseLogDestination(envDestination?: string, clientDestination?: string): string { - if (typeof envDestination === 'string' && envDestination !== '') { - return envDestination; - } else if (typeof clientDestination === 'string' && clientDestination !== '') { - return clientDestination; +function parseLogDestination(logDestination?: string): string { + if (typeof logDestination === 'string' && logDestination !== '') { + const lowerLogDestination = logDestination.toLowerCase(); + return lowerLogDestination === 'stdout' || lowerLogDestination === 'stderr' + ? lowerLogDestination + : logDestination; } return 'stderr'; @@ -100,16 +100,19 @@ export interface MongoLoggerMongoClientOptions { /** @internal */ export interface MongoLoggerOptions { - /** Severity level for command component */ - command: SeverityLevel; - /** Severity level for topology component */ - topology: SeverityLevel; - /** Severity level for server selection component */ - serverSelection: SeverityLevel; - /** Severity level for CMAP */ - connection: SeverityLevel; - /** Default severity level to be if any of the above are unset */ - defaultSeverity: SeverityLevel; + severitySettings: { + /** Severity level for command component */ + command: SeverityLevel; + /** Severity level for topology component */ + topology: SeverityLevel; + /** Severity level for server selection component */ + serverSelection: SeverityLevel; + /** Severity level for connection component */ + connection: SeverityLevel; + /** Default severity level to be used if any of the above are unset */ + default: SeverityLevel; + }; + /** Max length of embedded EJSON docs. Setting to 0 disables truncation. Defaults to 1000. */ maxDocumentLength: number; /** Destination for log messages. Must be 'stderr' or 'stdout'. Defaults to 'stderr'. */ @@ -123,11 +126,9 @@ export class MongoLogger { logDestination: Writable; constructor(options: MongoLoggerOptions) { - // TODO(NODE-4849): add filepath and Writable support - this.logDestination = - options.logDestination.toLowerCase() === 'stdout' ? process['stdout'] : process['stderr']; - this.componentSeverities = options; + this.componentSeverities = options.severitySettings; this.maxDocumentLength = options.maxDocumentLength; + this.logDestination = options.logDestination === 'stdout' ? process.stdout : process.stderr; } /* eslint-disable @typescript-eslint/no-unused-vars */ @@ -150,6 +151,20 @@ export class MongoLogger { trace(component: any, message: any): void {} + /** + * Creates and returns a new MongoLogger object if logging is enabled by setting + * at least one value in options.severitySetting to a SeverityLevel other than SeverityLevel.OFF. + * + * @param options - a well-formatted MongoLoggerOptions object + * @returns a new MongoLogger if logging is enabled, otherwise null + */ + static create(options?: MongoLoggerOptions): MongoLogger | null { + if (options == null) return null; + return Object.values(options.severitySettings).some(severity => severity !== SeverityLevel.OFF) + ? new MongoLogger(options) + : null; + } + /** * Merges options set through environment variables and the MongoClient, preferring environment * variables when both are set, and substituting defaults for values not set. Options set in @@ -171,14 +186,19 @@ export class MongoLogger { parseSeverityFromString(envOptions.MONGODB_LOG_ALL) ?? SeverityLevel.OFF; return { - command: parseSeverityFromString(envOptions.MONGODB_LOG_COMMAND) ?? defaultSeverity, - topology: parseSeverityFromString(envOptions.MONGODB_LOG_TOPOLOGY) ?? defaultSeverity, - serverSelection: - parseSeverityFromString(envOptions.MONGODB_LOG_SERVER_SELECTION) ?? defaultSeverity, - connection: parseSeverityFromString(envOptions.MONGODB_LOG_CONNECTION) ?? defaultSeverity, - defaultSeverity, + severitySettings: { + command: parseSeverityFromString(envOptions.MONGODB_LOG_COMMAND) ?? defaultSeverity, + topology: parseSeverityFromString(envOptions.MONGODB_LOG_TOPOLOGY) ?? defaultSeverity, + serverSelection: + parseSeverityFromString(envOptions.MONGODB_LOG_SERVER_SELECTION) ?? defaultSeverity, + connection: parseSeverityFromString(envOptions.MONGODB_LOG_CONNECTION) ?? defaultSeverity, + default: defaultSeverity + }, maxDocumentLength: parseMaxDocumentLength(envOptions.MONGODB_LOG_MAX_DOCUMENT_LENGTH), - logDestination: parseLogDestination(envOptions.MONGODB_LOG_PATH, clientOptions.mongodbLogPath) + logDestination: + envOptions != null + ? parseLogDestination(envOptions.MONGODB_LOG_PATH) + : parseLogDestination(clientOptions.mongodbLogPath) }; } } diff --git a/test/integration/node-specific/feature_flags.test.ts b/test/integration/node-specific/feature_flags.test.ts index 0d833eb89b..e98dc10ae1 100644 --- a/test/integration/node-specific/feature_flags.test.ts +++ b/test/integration/node-specific/feature_flags.test.ts @@ -1,7 +1,7 @@ import { expect } from 'chai'; import { MongoClient } from '../../../src'; -import { MongoLogger } from '../../../src/mongo_logger'; +import { MongoLogger, SeverityLevel } from '../../../src/mongo_logger'; describe('Feature Flags', () => { describe('@@mdb.skipPingOnConnect', () => { @@ -52,28 +52,48 @@ describe('Feature Flags', () => { before(() => { cachedEnv = process.env; - process.env['MONGODB_LOG_COMMAND'] = 'emergency'; }); after(() => { process.env = cachedEnv; }); - it('should instantiate a MongoLogger when set to true', () => { - const client = new MongoClient('mongodb://localhost:27017', { [loggerFeatureFlag]: true }); - expect(client.mongoLogger).to.be.instanceOf(MongoLogger); - }); + context('when logging for a component is enabled', () => { + before(() => { + process.env['MONGODB_LOG_COMMAND'] = SeverityLevel.EMERGENCY; + }); - it('should not instantiate a MongoLogger when set to false', () => { - const client = new MongoClient('mongodb://localhost:27017', { [loggerFeatureFlag]: false }); - expect(client).property('mongoLogger', null); + it('should instantiate a MongoLogger when set to true', () => { + const client = new MongoClient('mongodb://localhost:27017', { [loggerFeatureFlag]: true }); + expect(client.mongoLogger).to.be.instanceOf(MongoLogger); + }); + + it('should not instantiate a MongoLogger when set to false', () => { + const client = new MongoClient('mongodb://localhost:27017', { [loggerFeatureFlag]: false }); + expect(client).property('mongoLogger', null); + }); + + it('should not instantiate a MongoLogger when set to undefined', () => { + const client = new MongoClient('mongodb://localhost:27017', { + [loggerFeatureFlag]: undefined + }); + expect(client).property('mongoLogger', null); + }); }); - it('should not instantiate a MongoLogger when set to undefined', () => { - const client = new MongoClient('mongodb://localhost:27017', { - [loggerFeatureFlag]: undefined + context('when logging for a component is not enabled', () => { + before(() => { + process.env['MONGODB_LOG_COMMAND'] = SeverityLevel.OFF; }); - expect(client).property('mongoLogger', null); + + for (const featureFlagValue of [true, false, undefined]) { + it(`should not instantiate a MongoLogger when set to ${featureFlagValue}`, () => { + const client = new MongoClient('mongodb://localhost:27017', { + [loggerFeatureFlag]: featureFlagValue + }); + expect(client).property('mongoLogger', null); + }); + } }); }); }); diff --git a/test/unit/mongo_client.test.js b/test/unit/mongo_client.test.js index 0a82d17683..3ec83a4242 100644 --- a/test/unit/mongo_client.test.js +++ b/test/unit/mongo_client.test.js @@ -872,7 +872,7 @@ describe('MongoOptions', function () { process.env = {}; }); - context('when only a single severity enviroment variable is set', function () { + context('when only a single severity environment variable is set', function () { for (const envName of severityVars) { context(`when ${envName} is set to a valid value`, function () { it('instantiates a MongoLogger', function () { @@ -885,7 +885,7 @@ describe('MongoOptions', function () { }); context(`when ${envName} is set to an invalid value`, function () { - it('does not instatiate a MongoLogger', function () { + it('does not instantiate a MongoLogger', function () { process.env[envName] = 'invalid'; const client = new MongoClient('mongodb://localhost:27017', { [loggerFeatureFlag]: true @@ -897,7 +897,7 @@ describe('MongoOptions', function () { }); context('when there are no environment variables set', function () { - it('should not instatiate a MongoLogger if environment variables are not set', function () { + it('should not instantiate a MongoLogger', function () { const client = new MongoClient('mongodb://localhost:27017', { [loggerFeatureFlag]: true }); expect(client).property('mongoLogger', null); }); diff --git a/test/unit/mongo_logger.test.ts b/test/unit/mongo_logger.test.ts index 2ab6efc18e..9ccd041ef5 100644 --- a/test/unit/mongo_logger.test.ts +++ b/test/unit/mongo_logger.test.ts @@ -4,22 +4,12 @@ import { MongoLogger, SeverityLevel } from '../../src/mongo_logger'; describe('class MongoLogger', function () { describe('#constructor', function () { - it('treats the log destination value of stderr as case-insensitve', function () { - const loggerOptions = MongoLogger.resolveOptions({ MONGODB_LOG_PATH: 'STDERR' }, {}); - const logger = new MongoLogger(loggerOptions); - expect(logger.logDestination).to.equal(process['stderr']); - }); - - it('treats the log destination value of stdout as case-insensitve', function () { - const loggerOptions = MongoLogger.resolveOptions({ MONGODB_LOG_PATH: 'STDOUT' }, {}); - const logger = new MongoLogger(loggerOptions); - expect(logger.logDestination).to.equal(process['stdout']); - }); - - it('sets the log destination to stderr if an invalid value is passed', function () { - const loggerOptions = MongoLogger.resolveOptions({ MONGODB_LOG_PATH: 'invalid' }, {}); - const logger = new MongoLogger(loggerOptions); - expect(logger.logDestination).to.equal(process['stderr']); + context('when an invalid value is passed', function () { + it('sets the log destination to stderr if an invalid value is passed', function () { + const loggerOptions = MongoLogger.resolveOptions({ MONGODB_LOG_PATH: 'invalid' }, {}); + const logger = new MongoLogger(loggerOptions); + expect(logger.logDestination).to.equal(process.stderr); + }); }); }); @@ -33,9 +23,18 @@ describe('class MongoLogger', function () { }, {} ); - expect(loggerOptions.command).to.equal(SeverityLevel.EMERGENCY); - expect(loggerOptions.topology).to.equal(SeverityLevel.CRITICAL); - expect(loggerOptions.serverSelection).to.equal(SeverityLevel.ALERT); + expect(loggerOptions).to.have.nested.property( + 'severitySettings.command', + SeverityLevel.EMERGENCY + ); + expect(loggerOptions).to.have.nested.property( + 'severitySettings.topology', + SeverityLevel.CRITICAL + ); + expect(loggerOptions).to.have.nested.property( + 'severitySettings.serverSelection', + SeverityLevel.ALERT + ); }); it('treats invalid severity values as off', function () { @@ -45,7 +44,7 @@ describe('class MongoLogger', function () { }, {} ); - expect(loggerOptions.command).to.equal(SeverityLevel.OFF); + expect(loggerOptions).to.have.nested.property('severitySettings.command', SeverityLevel.OFF); }); it('can set severity levels per component', function () { @@ -59,11 +58,26 @@ describe('class MongoLogger', function () { }, {} ); - expect(loggerOptions.command).to.equal(SeverityLevel.EMERGENCY); - expect(loggerOptions.topology).to.equal(SeverityLevel.CRITICAL); - expect(loggerOptions.serverSelection).to.equal(SeverityLevel.ALERT); - expect(loggerOptions.connection).to.equal(SeverityLevel.DEBUG); - expect(loggerOptions.defaultSeverity).to.equal(SeverityLevel.WARNING); + expect(loggerOptions).to.have.nested.property( + 'severitySettings.command', + SeverityLevel.EMERGENCY + ); + expect(loggerOptions).to.have.nested.property( + 'severitySettings.topology', + SeverityLevel.CRITICAL + ); + expect(loggerOptions).to.have.nested.property( + 'severitySettings.serverSelection', + SeverityLevel.ALERT + ); + expect(loggerOptions).to.have.nested.property( + 'severitySettings.connection', + SeverityLevel.DEBUG + ); + expect(loggerOptions).to.have.nested.property( + 'severitySettings.default', + SeverityLevel.WARNING + ); }); context('when component severities are not set or invalid', function () { @@ -78,10 +92,22 @@ describe('class MongoLogger', function () { }, {} ); - expect(loggerOptions.command).to.equal(loggerOptions.defaultSeverity); - expect(loggerOptions.topology).to.equal(loggerOptions.defaultSeverity); - expect(loggerOptions.serverSelection).to.equal(loggerOptions.defaultSeverity); - expect(loggerOptions.connection).to.equal(SeverityLevel.EMERGENCY); + expect(loggerOptions).to.nested.property( + 'severitySettings.command', + loggerOptions.severitySettings.default + ); + expect(loggerOptions).to.nested.property( + 'severitySettings.topology', + loggerOptions.severitySettings.default + ); + expect(loggerOptions).to.nested.property( + 'severitySettings.serverSelection', + loggerOptions.severitySettings.default + ); + expect(loggerOptions).to.nested.property( + 'severitySettings.connection', + loggerOptions.severitySettings.default + ); }); }); @@ -167,13 +193,33 @@ describe('class MongoLogger', function () { }); }); - context('when MONGODB_LOG_PATH is not an empty string', function () { + context('when MONGODB_LOG_PATH is stdout', function () { + it('treats the value as case-insensitive', function () { + const loggerOptions = MongoLogger.resolveOptions( + { MONGODB_LOG_PATH: 'STDOUT' }, + { mongodbLogPath: undefined } + ); + expect(loggerOptions).to.have.property('logDestination', 'stdout'); + }); + }); + + context('when MONGODB_LOG_PATH is stderr', function () { + it('treats the value as case-insensitive', function () { + const loggerOptions = MongoLogger.resolveOptions( + { MONGODB_LOG_PATH: 'STDERR' }, + { mongodbLogPath: undefined } + ); + expect(loggerOptions).to.have.property('logDestination', 'stderr'); + }); + }); + + context('when MONGODB_LOG_PATH is a non-empty string', function () { it('uses the passed value', function () { const loggerOptions = MongoLogger.resolveOptions( - { MONGODB_LOG_PATH: 'stdout' }, + { MONGODB_LOG_PATH: '/path/to/file.txt' }, { mongodbLogPath: undefined } ); - expect(loggerOptions.logDestination).to.equal('stdout'); + expect(loggerOptions.logDestination).to.equal('/path/to/file.txt'); }); }); }); @@ -189,7 +235,27 @@ describe('class MongoLogger', function () { }); }); - context('when mongodbLogPath is not an empty string', function () { + context('when mongodbLogPath is stdout', function () { + it('treats the value as case-insensitive', function () { + const loggerOptions = MongoLogger.resolveOptions( + { MONGODB_LOG_PATH: undefined }, + { mongodbLogPath: 'STDOUT' } + ); + expect(loggerOptions).to.have.property('logDestination', 'stdout'); + }); + }); + + context('when mongodbLogPath is stderr', function () { + it('treats the value as case-insensitive', function () { + const loggerOptions = MongoLogger.resolveOptions( + { MONGODB_LOG_PATH: undefined }, + { mongodbLogPath: 'STDERR' } + ); + expect(loggerOptions).to.have.property('logDestination', 'stderr'); + }); + }); + + context('when mongodbLogPath is a non-empty string', function () { it('uses the passed value ', function () { const loggerOptions = MongoLogger.resolveOptions( { MONGODB_LOG_PATH: undefined }, From dca979676dc2f8116929474772845aca7196163c Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Fri, 2 Dec 2022 10:59:12 -0500 Subject: [PATCH 26/50] refactor: make mongoLogger non-existent --- src/mongo_client.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/mongo_client.ts b/src/mongo_client.ts index cb472fe4da..bf931e1ff0 100644 --- a/src/mongo_client.ts +++ b/src/mongo_client.ts @@ -336,7 +336,7 @@ export class MongoClient extends TypedEventEmitter { /** @internal */ topology?: Topology; /** @internal */ - readonly mongoLogger: MongoLogger | null; + readonly mongoLogger?: MongoLogger; /** * The consolidate, parsed, transformed and merged options. @@ -348,7 +348,8 @@ export class MongoClient extends TypedEventEmitter { super(); this[kOptions] = parseOptions(url, this, options); - this.mongoLogger = MongoLogger.create(this[kOptions].mongoLoggerOptions); + const mongoLogger = MongoLogger.create(this[kOptions].mongoLoggerOptions); + if (mongoLogger != null) this.mongoLogger = mongoLogger; // eslint-disable-next-line @typescript-eslint/no-this-alias const client = this; From a69b493252d4aa217284c155d4f02edee565f4d3 Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Fri, 2 Dec 2022 15:09:12 -0500 Subject: [PATCH 27/50] test: refactor feature flag tests --- src/connection_string.ts | 14 ++- src/mongo_client.ts | 7 +- .../node-specific/feature_flags.test.ts | 117 +++++++++++++----- 3 files changed, 99 insertions(+), 39 deletions(-) diff --git a/src/connection_string.ts b/src/connection_string.ts index 642900bb2b..7009e8f76f 100644 --- a/src/connection_string.ts +++ b/src/connection_string.ts @@ -498,8 +498,10 @@ export function parseOptions( const loggerFeatureFlag = Symbol.for('@@mdb.enableMongoLogger'); mongoOptions[loggerFeatureFlag] = mongoOptions[loggerFeatureFlag] ?? false; + let loggerEnvOptions: MongoLoggerEnvOptions = {}; + let loggerClientOptions: MongoLoggerMongoClientOptions = {}; if (mongoOptions[loggerFeatureFlag]) { - const loggerEnvOptions: MongoLoggerEnvOptions = { + loggerEnvOptions = { MONGODB_LOG_COMMAND: process.env.MONGODB_LOG_COMMAND, MONGODB_LOG_TOPOLOGY: process.env.MONGODB_LOG_TOPOLOGY, MONGODB_LOG_SERVER_SELECTION: process.env.MONGODB_LOG_SERVER_SELECTION, @@ -508,14 +510,14 @@ export function parseOptions( MONGODB_LOG_MAX_DOCUMENT_LENGTH: process.env.MONGODB_LOG_MAX_DOCUMENT_LENGTH, MONGODB_LOG_PATH: process.env.MONGODB_LOG_PATH }; - const loggerMongoClientOptions: MongoLoggerMongoClientOptions = { + loggerClientOptions = { mongodbLogPath: mongoOptions.mongodbLogPath }; - mongoOptions.mongoLoggerOptions = MongoLogger.resolveOptions( - loggerEnvOptions, - loggerMongoClientOptions - ); } + mongoOptions.mongoLoggerOptions = MongoLogger.resolveOptions( + loggerEnvOptions, + loggerClientOptions + ); return mongoOptions; } diff --git a/src/mongo_client.ts b/src/mongo_client.ts index bf931e1ff0..4d94d87747 100644 --- a/src/mongo_client.ts +++ b/src/mongo_client.ts @@ -336,7 +336,7 @@ export class MongoClient extends TypedEventEmitter { /** @internal */ topology?: Topology; /** @internal */ - readonly mongoLogger?: MongoLogger; + readonly mongoLogger: MongoLogger; /** * The consolidate, parsed, transformed and merged options. @@ -348,8 +348,7 @@ export class MongoClient extends TypedEventEmitter { super(); this[kOptions] = parseOptions(url, this, options); - const mongoLogger = MongoLogger.create(this[kOptions].mongoLoggerOptions); - if (mongoLogger != null) this.mongoLogger = mongoLogger; + this.mongoLogger = new MongoLogger(this[kOptions].mongoLoggerOptions); // eslint-disable-next-line @typescript-eslint/no-this-alias const client = this; @@ -810,5 +809,5 @@ export interface MongoOptions [featureFlag: symbol]: any; /** @internal */ - mongoLoggerOptions?: MongoLoggerOptions; + mongoLoggerOptions: MongoLoggerOptions; } diff --git a/test/integration/node-specific/feature_flags.test.ts b/test/integration/node-specific/feature_flags.test.ts index e98dc10ae1..dd0d16b3e2 100644 --- a/test/integration/node-specific/feature_flags.test.ts +++ b/test/integration/node-specific/feature_flags.test.ts @@ -1,7 +1,7 @@ import { expect } from 'chai'; import { MongoClient } from '../../../src'; -import { MongoLogger, SeverityLevel } from '../../../src/mongo_logger'; +import { SeverityLevel } from '../../../src/mongo_logger'; describe('Feature Flags', () => { describe('@@mdb.skipPingOnConnect', () => { @@ -46,9 +46,20 @@ describe('Feature Flags', () => { } }); - describe('@@mdb.enableMongoLogger', () => { + describe.only('@@mdb.enableMongoLogger', () => { let cachedEnv; const loggerFeatureFlag = Symbol.for('@@mdb.enableMongoLogger'); + const severityMethods = [ + 'emergency', + 'alert', + 'critical', + 'error', + 'warn', + 'notice', + 'info', + 'debug', + 'trace' + ]; before(() => { cachedEnv = process.env; @@ -58,42 +69,90 @@ describe('Feature Flags', () => { process.env = cachedEnv; }); - context('when logging for a component is enabled', () => { - before(() => { - process.env['MONGODB_LOG_COMMAND'] = SeverityLevel.EMERGENCY; - }); - - it('should instantiate a MongoLogger when set to true', () => { - const client = new MongoClient('mongodb://localhost:27017', { [loggerFeatureFlag]: true }); - expect(client.mongoLogger).to.be.instanceOf(MongoLogger); - }); + context('when set to true', () => { + context('when logging for a component is enabled', () => { + before(() => { + process.env['MONGODB_LOG_COMMAND'] = SeverityLevel.EMERGENCY; + }); - it('should not instantiate a MongoLogger when set to false', () => { - const client = new MongoClient('mongodb://localhost:27017', { [loggerFeatureFlag]: false }); - expect(client).property('mongoLogger', null); + for (const severity of severityMethods) { + context(`${severity} severity logging method`, () => { + const skipReason = + severity === SeverityLevel.EMERGENCY + ? 'TODO(NODE-4813): implement the emergency severity logging method' + : 'TODO(NODE-4814): implement the remaining severity loggers'; + it.skip('should not be a no-op', () => { + const client = new MongoClient('mongodb://localhost:27017', { + [loggerFeatureFlag]: true + }); + const stringifiedMethod = client.mongoLogger[severity].toString(); + const expectedStringifiedMethod = `${severity}(component, message) { }`; + expect(stringifiedMethod).to.not.equal(expectedStringifiedMethod); + }).skipReason = skipReason; + }); + } }); - it('should not instantiate a MongoLogger when set to undefined', () => { - const client = new MongoClient('mongodb://localhost:27017', { - [loggerFeatureFlag]: undefined + context('when logging for a component is not enabled', () => { + before(() => { + process.env['MONGODB_LOG_COMMAND'] = SeverityLevel.OFF; }); - expect(client).property('mongoLogger', null); + + for (const severity of severityMethods) { + context(`${severity} severity logging method`, () => { + it('should be a no-op', () => { + const client = new MongoClient('mongodb://localhost:27017', { + [loggerFeatureFlag]: true + }); + const stringifiedMethod = client.mongoLogger[severity].toString(); + const expectedStringifiedMethod = `${severity}(component, message) { }`; + expect(stringifiedMethod).to.equal(expectedStringifiedMethod); + }); + }); + } }); }); - context('when logging for a component is not enabled', () => { - before(() => { - process.env['MONGODB_LOG_COMMAND'] = SeverityLevel.OFF; - }); + for (const featureFlagValue of [false, undefined]) { + context(`when set to ${featureFlagValue}`, () => { + context('when logging for a component is enabled', () => { + before(() => { + process.env['MONGODB_LOG_COMMAND'] = SeverityLevel.EMERGENCY; + }); - for (const featureFlagValue of [true, false, undefined]) { - it(`should not instantiate a MongoLogger when set to ${featureFlagValue}`, () => { - const client = new MongoClient('mongodb://localhost:27017', { - [loggerFeatureFlag]: featureFlagValue + for (const severity of severityMethods) { + context(`${severity} severity logging method`, () => { + it('should be a no-op', () => { + const client = new MongoClient('mongodb://localhost:27017', { + [loggerFeatureFlag]: true + }); + const stringifiedMethod = client.mongoLogger[severity].toString(); + const expectedStringifiedMethod = `${severity}(component, message) { }`; + expect(stringifiedMethod).to.equal(expectedStringifiedMethod); + }); + }); + } + }); + + context('when logging for a component is not enabled', () => { + before(() => { + process.env['MONGODB_LOG_COMMAND'] = SeverityLevel.OFF; }); - expect(client).property('mongoLogger', null); + + for (const severity of severityMethods) { + context(`${severity} severity logging method`, () => { + it('should be a no-op', () => { + const client = new MongoClient('mongodb://localhost:27017', { + [loggerFeatureFlag]: true + }); + const stringifiedMethod = client.mongoLogger[severity].toString(); + const expectedStringifiedMethod = `${severity}(component, message) { }`; + expect(stringifiedMethod).to.equal(expectedStringifiedMethod); + }); + }); + } }); - } - }); + }); + } }); }); From 8db2be514bcdbc7871cbeae9bb766c69180ea25b Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Fri, 2 Dec 2022 15:20:07 -0500 Subject: [PATCH 28/50] fix: adjust typo in tests --- src/mongo_logger.ts | 21 +++++++-------------- test/unit/mongo_logger.test.ts | 8 ++++---- 2 files changed, 11 insertions(+), 18 deletions(-) diff --git a/src/mongo_logger.ts b/src/mongo_logger.ts index 04dbf3ce3a..d87c53626f 100644 --- a/src/mongo_logger.ts +++ b/src/mongo_logger.ts @@ -51,6 +51,13 @@ function parseMaxDocumentLength(s?: string): number { } } +/** + * Parses a string for logDestination. Strings containing 'stderr' or 'stdout' are treated as + * case-insensitive. + * + * @param logDestination - the destination to write logs to + * @returns the original string if a file path was passed. Otherwise one of ['stderr', 'stdout']. + */ function parseLogDestination(logDestination?: string): string { if (typeof logDestination === 'string' && logDestination !== '') { const lowerLogDestination = logDestination.toLowerCase(); @@ -151,20 +158,6 @@ export class MongoLogger { trace(component: any, message: any): void {} - /** - * Creates and returns a new MongoLogger object if logging is enabled by setting - * at least one value in options.severitySetting to a SeverityLevel other than SeverityLevel.OFF. - * - * @param options - a well-formatted MongoLoggerOptions object - * @returns a new MongoLogger if logging is enabled, otherwise null - */ - static create(options?: MongoLoggerOptions): MongoLogger | null { - if (options == null) return null; - return Object.values(options.severitySettings).some(severity => severity !== SeverityLevel.OFF) - ? new MongoLogger(options) - : null; - } - /** * Merges options set through environment variables and the MongoClient, preferring environment * variables when both are set, and substituting defaults for values not set. Options set in diff --git a/test/unit/mongo_logger.test.ts b/test/unit/mongo_logger.test.ts index 9ccd041ef5..6b9fb71af3 100644 --- a/test/unit/mongo_logger.test.ts +++ b/test/unit/mongo_logger.test.ts @@ -92,19 +92,19 @@ describe('class MongoLogger', function () { }, {} ); - expect(loggerOptions).to.nested.property( + expect(loggerOptions).to.have.nested.property( 'severitySettings.command', loggerOptions.severitySettings.default ); - expect(loggerOptions).to.nested.property( + expect(loggerOptions).to.have.nested.property( 'severitySettings.topology', loggerOptions.severitySettings.default ); - expect(loggerOptions).to.nested.property( + expect(loggerOptions).to.have.nested.property( 'severitySettings.serverSelection', loggerOptions.severitySettings.default ); - expect(loggerOptions).to.nested.property( + expect(loggerOptions).to.have.nested.property( 'severitySettings.connection', loggerOptions.severitySettings.default ); From c676b217011056f96d4440b496cb8d9f1a42840d Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Fri, 2 Dec 2022 15:28:22 -0500 Subject: [PATCH 29/50] fix: remove .only from test --- test/integration/node-specific/feature_flags.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/node-specific/feature_flags.test.ts b/test/integration/node-specific/feature_flags.test.ts index dd0d16b3e2..acbe4ddab0 100644 --- a/test/integration/node-specific/feature_flags.test.ts +++ b/test/integration/node-specific/feature_flags.test.ts @@ -46,7 +46,7 @@ describe('Feature Flags', () => { } }); - describe.only('@@mdb.enableMongoLogger', () => { + describe('@@mdb.enableMongoLogger', () => { let cachedEnv; const loggerFeatureFlag = Symbol.for('@@mdb.enableMongoLogger'); const severityMethods = [ From 3e1e33d93f0d206d024b6ffc8851730cdf731039 Mon Sep 17 00:00:00 2001 From: Bailey Pearson Date: Tue, 6 Dec 2022 12:48:06 -0500 Subject: [PATCH 30/50] Update test/unit/mongo_logger.test.ts Co-authored-by: Daria Pardue --- test/unit/mongo_logger.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/mongo_logger.test.ts b/test/unit/mongo_logger.test.ts index 6b9fb71af3..61328a789d 100644 --- a/test/unit/mongo_logger.test.ts +++ b/test/unit/mongo_logger.test.ts @@ -158,7 +158,7 @@ describe('class MongoLogger', function () { }); }); - context('when MONGODB_LOG_MAX_DOCUMENT_LENGTH can be parsed a uint', function () { + context('when MONGODB_LOG_MAX_DOCUMENT_LENGTH can be parsed as a uint', function () { it('uses the passed value', function () { const loggerOptions = MongoLogger.resolveOptions( { From f236d5589ba9f29de9fbc45b5e5bc94a93fd1ab8 Mon Sep 17 00:00:00 2001 From: Bailey Pearson Date: Tue, 6 Dec 2022 15:15:41 -0500 Subject: [PATCH 31/50] refactor: clean up mongo_logger tests --- src/mongo_logger.ts | 51 ++-- test/unit/mongo_logger.test.ts | 502 ++++++++++++++++++--------------- 2 files changed, 300 insertions(+), 253 deletions(-) diff --git a/src/mongo_logger.ts b/src/mongo_logger.ts index d87c53626f..0f67b936bc 100644 --- a/src/mongo_logger.ts +++ b/src/mongo_logger.ts @@ -101,13 +101,13 @@ export interface MongoLoggerEnvOptions { /** @internal */ export interface MongoLoggerMongoClientOptions { - /** Destination for log messages. Must be 'stderr' or 'stdout'. Defaults to 'stderr'. */ - mongodbLogPath?: string; + /** Destination for log messages */ + mongodbLogPath?: 'stdout' | 'stderr' | Writable; } /** @internal */ export interface MongoLoggerOptions { - severitySettings: { + componentSeverities: { /** Severity level for command component */ command: SeverityLevel; /** Severity level for topology component */ @@ -122,8 +122,8 @@ export interface MongoLoggerOptions { /** Max length of embedded EJSON docs. Setting to 0 disables truncation. Defaults to 1000. */ maxDocumentLength: number; - /** Destination for log messages. Must be 'stderr' or 'stdout'. Defaults to 'stderr'. */ - logDestination: string; + /** Destination for log messages. */ + logDestination: Writable; } /** @internal */ @@ -133,9 +133,9 @@ export class MongoLogger { logDestination: Writable; constructor(options: MongoLoggerOptions) { - this.componentSeverities = options.severitySettings; + this.componentSeverities = options.componentSeverities; this.maxDocumentLength = options.maxDocumentLength; - this.logDestination = options.logDestination === 'stdout' ? process.stdout : process.stderr; + this.logDestination = options.logDestination; } /* eslint-disable @typescript-eslint/no-unused-vars */ @@ -175,23 +175,38 @@ export class MongoLogger { envOptions: MongoLoggerEnvOptions, clientOptions: MongoLoggerMongoClientOptions ): MongoLoggerOptions { + // client options take precedence over env options + const combinedOptions = { + ...envOptions, + ...clientOptions, + mongodbLogPath: + clientOptions.mongodbLogPath != null + ? clientOptions.mongodbLogPath + : envOptions.MONGODB_LOG_PATH != null + ? envOptions.MONGODB_LOG_PATH + : 'stderr' + }; const defaultSeverity = - parseSeverityFromString(envOptions.MONGODB_LOG_ALL) ?? SeverityLevel.OFF; + parseSeverityFromString(combinedOptions.MONGODB_LOG_ALL) ?? SeverityLevel.OFF; + const logDestination: MongoLoggerOptions['logDestination'] = + typeof combinedOptions.mongodbLogPath !== 'string' + ? combinedOptions.mongodbLogPath + : combinedOptions.mongodbLogPath === 'stderr' + ? process.stderr + : process.stdout; return { - severitySettings: { - command: parseSeverityFromString(envOptions.MONGODB_LOG_COMMAND) ?? defaultSeverity, - topology: parseSeverityFromString(envOptions.MONGODB_LOG_TOPOLOGY) ?? defaultSeverity, + componentSeverities: { + command: parseSeverityFromString(combinedOptions.MONGODB_LOG_COMMAND) ?? defaultSeverity, + topology: parseSeverityFromString(combinedOptions.MONGODB_LOG_TOPOLOGY) ?? defaultSeverity, serverSelection: - parseSeverityFromString(envOptions.MONGODB_LOG_SERVER_SELECTION) ?? defaultSeverity, - connection: parseSeverityFromString(envOptions.MONGODB_LOG_CONNECTION) ?? defaultSeverity, + parseSeverityFromString(combinedOptions.MONGODB_LOG_SERVER_SELECTION) ?? defaultSeverity, + connection: + parseSeverityFromString(combinedOptions.MONGODB_LOG_CONNECTION) ?? defaultSeverity, default: defaultSeverity }, - maxDocumentLength: parseMaxDocumentLength(envOptions.MONGODB_LOG_MAX_DOCUMENT_LENGTH), - logDestination: - envOptions != null - ? parseLogDestination(envOptions.MONGODB_LOG_PATH) - : parseLogDestination(clientOptions.mongodbLogPath) + maxDocumentLength: parseMaxDocumentLength(combinedOptions.MONGODB_LOG_MAX_DOCUMENT_LENGTH), + logDestination }; } } diff --git a/test/unit/mongo_logger.test.ts b/test/unit/mongo_logger.test.ts index 61328a789d..b7eba738f6 100644 --- a/test/unit/mongo_logger.test.ts +++ b/test/unit/mongo_logger.test.ts @@ -1,270 +1,302 @@ import { expect } from 'chai'; +import { Writable } from 'stream'; -import { MongoLogger, SeverityLevel } from '../../src/mongo_logger'; +import { + MongoLogger, + MongoLoggerMongoClientOptions, + MongoLoggerOptions, + SeverityLevel +} from '../../src/mongo_logger'; describe('class MongoLogger', function () { - describe('#constructor', function () { - context('when an invalid value is passed', function () { - it('sets the log destination to stderr if an invalid value is passed', function () { - const loggerOptions = MongoLogger.resolveOptions({ MONGODB_LOG_PATH: 'invalid' }, {}); - const logger = new MongoLogger(loggerOptions); - expect(logger.logDestination).to.equal(process.stderr); + describe('#constructor()', function () { + it('assigns each property from the options object onto the logging class', function () { + const componentSeverities: MongoLoggerOptions['componentSeverities'] = { + command: 'alert' + } as any; + const stream = new Writable(); + const logger = new MongoLogger({ + componentSeverities, + maxDocumentLength: 10, + logDestination: stream }); - }); - }); - - describe('static #resolveOptions', function () { - it('treats severity values as case-insensitive', function () { - const loggerOptions = MongoLogger.resolveOptions( - { - MONGODB_LOG_COMMAND: 'EMERGENCY', - MONGODB_LOG_TOPOLOGY: 'critical', - MONGODB_LOG_SERVER_SELECTION: 'aLeRt' - }, - {} - ); - expect(loggerOptions).to.have.nested.property( - 'severitySettings.command', - SeverityLevel.EMERGENCY - ); - expect(loggerOptions).to.have.nested.property( - 'severitySettings.topology', - SeverityLevel.CRITICAL - ); - expect(loggerOptions).to.have.nested.property( - 'severitySettings.serverSelection', - SeverityLevel.ALERT - ); - }); - it('treats invalid severity values as off', function () { - const loggerOptions = MongoLogger.resolveOptions( - { - MONGODB_LOG_COMMAND: 'invalid' - }, - {} - ); - expect(loggerOptions).to.have.nested.property('severitySettings.command', SeverityLevel.OFF); - }); - - it('can set severity levels per component', function () { - const loggerOptions = MongoLogger.resolveOptions( - { - MONGODB_LOG_COMMAND: SeverityLevel.EMERGENCY, - MONGODB_LOG_TOPOLOGY: SeverityLevel.CRITICAL, - MONGODB_LOG_SERVER_SELECTION: SeverityLevel.ALERT, - MONGODB_LOG_CONNECTION: SeverityLevel.DEBUG, - MONGODB_LOG_ALL: SeverityLevel.WARNING - }, - {} - ); - expect(loggerOptions).to.have.nested.property( - 'severitySettings.command', - SeverityLevel.EMERGENCY - ); - expect(loggerOptions).to.have.nested.property( - 'severitySettings.topology', - SeverityLevel.CRITICAL - ); - expect(loggerOptions).to.have.nested.property( - 'severitySettings.serverSelection', - SeverityLevel.ALERT - ); - expect(loggerOptions).to.have.nested.property( - 'severitySettings.connection', - SeverityLevel.DEBUG - ); - expect(loggerOptions).to.have.nested.property( - 'severitySettings.default', - SeverityLevel.WARNING - ); - }); - - context('when component severities are not set or invalid', function () { - it('only uses the default severity for those components', function () { - const loggerOptions = MongoLogger.resolveOptions( - { - MONGODB_LOG_COMMAND: '', - MONGODB_LOG_TOPOLOGY: undefined, - MONGODB_LOG_SERVER_SELECTION: 'invalid', - MONGODB_LOG_CONNECTION: SeverityLevel.EMERGENCY, - MONGODB_LOG_ALL: SeverityLevel.CRITICAL - }, - {} - ); - expect(loggerOptions).to.have.nested.property( - 'severitySettings.command', - loggerOptions.severitySettings.default - ); - expect(loggerOptions).to.have.nested.property( - 'severitySettings.topology', - loggerOptions.severitySettings.default - ); - expect(loggerOptions).to.have.nested.property( - 'severitySettings.serverSelection', - loggerOptions.severitySettings.default - ); - expect(loggerOptions).to.have.nested.property( - 'severitySettings.connection', - loggerOptions.severitySettings.default - ); - }); - }); - - context('when environment variables and client options are both set', function () { - it('gives precedence to environment variables', function () { - const loggerOptions = MongoLogger.resolveOptions( - { MONGODB_LOG_PATH: 'env' }, - { mongodbLogPath: 'client' } - ); - expect(loggerOptions.logDestination).to.equal('env'); - }); + expect(logger).to.have.property('componentSeverities', componentSeverities); + expect(logger).to.have.property('maxDocumentLength', 10); + expect(logger).to.have.property('logDestination', stream); }); + }); - context('maxDocumentLength', function () { - context('when MONGODB_LOG_MAX_DOCUMENT_LENGTH is undefined', function () { - it('defaults to 1000', function () { - const loggerOptions = MongoLogger.resolveOptions( - { - MONGODB_LOG_MAX_DOCUMENT_LENGTH: undefined - }, - {} - ); - expect(loggerOptions.maxDocumentLength).to.equal(1000); - }); - }); + describe('static #resolveOptions()', function () { + describe('componentSeverities', function () { + const components = [ + 'MONGODB_LOG_COMMAND', + 'MONGODB_LOG_TOPOLOGY', + 'MONGODB_LOG_SERVER_SELECTION', + 'MONGODB_LOG_CONNECTION' + ]; + const componentToInternalRepresentation = (component: string) => { + const options: Record = { + MONGODB_LOG_COMMAND: 'command', + MONGODB_LOG_TOPOLOGY: 'topology', + MONGODB_LOG_SERVER_SELECTION: 'serverSelection', + MONGODB_LOG_CONNECTION: 'connection' + }; + return options[component]; + }; - context('when MONGODB_LOG_MAX_DOCUMENT_LENGTH is an empty string', function () { - it('defaults to 1000', function () { - const loggerOptions = MongoLogger.resolveOptions( - { - MONGODB_LOG_MAX_DOCUMENT_LENGTH: '' - }, - {} - ); - expect(loggerOptions.maxDocumentLength).to.equal(1000); + context('MONGODB_LOG_ALL', () => { + context('when a default is provided', () => { + it('sets default to the provided value', () => { + const options = MongoLogger.resolveOptions( + { MONGODB_LOG_ALL: SeverityLevel.ALERT }, + {} + ); + expect(options.componentSeverities).to.have.property('default', SeverityLevel.ALERT); + }); }); - }); - - context('when MONGODB_LOG_MAX_DOCUMENT_LENGTH cannot be parsed as a uint', function () { - it('defaults to 1000', function () { - const loggerOptions = MongoLogger.resolveOptions( - { - MONGODB_LOG_MAX_DOCUMENT_LENGTH: 'invalid' - }, - {} - ); - expect(loggerOptions.maxDocumentLength).to.equal(1000); + context('when no value is provided', () => { + it('sets default to off', () => { + const options = MongoLogger.resolveOptions({ MONGODB_LOG_ALL: SeverityLevel.OFF }, {}); + expect(options.componentSeverities).to.have.property('default', SeverityLevel.OFF); + }); }); - }); - context('when MONGODB_LOG_MAX_DOCUMENT_LENGTH can be parsed as a uint', function () { - it('uses the passed value', function () { - const loggerOptions = MongoLogger.resolveOptions( - { - MONGODB_LOG_MAX_DOCUMENT_LENGTH: '500' - }, - {} - ); - expect(loggerOptions.maxDocumentLength).to.equal(500); + it('is case insensitive', () => { + const options = MongoLogger.resolveOptions({ MONGODB_LOG_ALL: 'dEbUg' }, {}); + expect(options.componentSeverities).to.have.property('default', SeverityLevel.DEBUG); }); }); - }); - context('logDestination', function () { - context('when mongodbLogPath is undefined', function () { - context('when MONGODB_LOG_PATH is undefined', function () { - it('defaults to stderr', function () { - const loggerOptions = MongoLogger.resolveOptions( - { MONGODB_LOG_PATH: undefined }, - { mongodbLogPath: undefined } - ); - expect(loggerOptions.logDestination).to.equal('stderr'); + for (const component of components) { + const internalRepresentationOfLogComponent = componentToInternalRepresentation(component); + context(`${component}`, function () { + context(`when set to a valid value in the environment`, function () { + context('when there is a default provided', function () { + it(`sets ${internalRepresentationOfLogComponent} to the provided value and ignores the default`, function () { + const options = MongoLogger.resolveOptions( + { [component]: SeverityLevel.ALERT, MONGODB_LOG_ALL: SeverityLevel.OFF }, + {} + ); + expect(options.componentSeverities).to.have.property( + internalRepresentationOfLogComponent, + SeverityLevel.ALERT + ); + }); + }); + context('when there is no default provided', function () { + it(`sets ${internalRepresentationOfLogComponent} to the provided value`, function () { + const options = MongoLogger.resolveOptions( + { [component]: SeverityLevel.ALERT, MONGODB_LOG_ALL: SeverityLevel.OFF }, + {} + ); + expect(options.componentSeverities).to.have.property( + internalRepresentationOfLogComponent, + SeverityLevel.ALERT + ); + }); + }); }); - }); - context('when MONGODB_LOG_PATH is an empty string', function () { - it('defaults to stderr', function () { - const loggerOptions = MongoLogger.resolveOptions( - { MONGODB_LOG_PATH: '' }, - { mongodbLogPath: undefined } - ); - expect(loggerOptions.logDestination).to.equal('stderr'); + context(`when set to an invalid value in the environment`, function () { + context('when there is a default provided', function () { + it(`sets ${internalRepresentationOfLogComponent} to the the default`, function () { + const options = MongoLogger.resolveOptions( + { [component]: 'invalid value' as any, MONGODB_LOG_ALL: SeverityLevel.ALERT }, + {} + ); + expect(options.componentSeverities).to.have.property( + internalRepresentationOfLogComponent, + SeverityLevel.ALERT + ); + }); + }); + context('when there is no default provided', function () { + it(`sets ${internalRepresentationOfLogComponent} to the off`, function () { + const options = MongoLogger.resolveOptions( + { [component]: 'invalid value' as any }, + {} + ); + expect(options.componentSeverities).to.have.property( + internalRepresentationOfLogComponent, + SeverityLevel.OFF + ); + }); + }); }); - }); - context('when MONGODB_LOG_PATH is stdout', function () { - it('treats the value as case-insensitive', function () { - const loggerOptions = MongoLogger.resolveOptions( - { MONGODB_LOG_PATH: 'STDOUT' }, - { mongodbLogPath: undefined } - ); - expect(loggerOptions).to.have.property('logDestination', 'stdout'); - }); - }); + context(`when unset`, () => { + context(`when there is no default set`, () => { + it(`does not set ${internalRepresentationOfLogComponent}`, () => { + const options = MongoLogger.resolveOptions({}, {}); + expect(options.componentSeverities).to.have.property( + internalRepresentationOfLogComponent, + SeverityLevel.OFF + ); + }); + }); - context('when MONGODB_LOG_PATH is stderr', function () { - it('treats the value as case-insensitive', function () { - const loggerOptions = MongoLogger.resolveOptions( - { MONGODB_LOG_PATH: 'STDERR' }, - { mongodbLogPath: undefined } - ); - expect(loggerOptions).to.have.property('logDestination', 'stderr'); + context(`when there is a default set`, () => { + it(`sets ${internalRepresentationOfLogComponent} to the default`, () => { + const options = MongoLogger.resolveOptions( + { MONGODB_LOG_ALL: SeverityLevel.DEBUG }, + {} + ); + expect(options.componentSeverities).to.have.property( + internalRepresentationOfLogComponent, + SeverityLevel.DEBUG + ); + }); + }); }); - }); - context('when MONGODB_LOG_PATH is a non-empty string', function () { - it('uses the passed value', function () { - const loggerOptions = MongoLogger.resolveOptions( - { MONGODB_LOG_PATH: '/path/to/file.txt' }, - { mongodbLogPath: undefined } + it('is case insensitive', function () { + const options = MongoLogger.resolveOptions({ MONGODB_LOG_ALL: 'dEbUg' as any }, {}); + expect(options.componentSeverities).to.have.property( + internalRepresentationOfLogComponent, + SeverityLevel.DEBUG ); - expect(loggerOptions.logDestination).to.equal('/path/to/file.txt'); }); }); - }); + } + }); - context('when MONGODB_LOG_PATH is undefined', function () { - context('when mongodbLogPath is an empty string', function () { - it('defaults to stderr', function () { - const loggerOptions = MongoLogger.resolveOptions( - { MONGODB_LOG_PATH: undefined }, - { mongodbLogPath: '' } - ); - expect(loggerOptions.logDestination).to.equal('stderr'); - }); - }); + context('maxDocumentLength', function () { + const tests: Array<{ + input: undefined | string; + expected: number; + context: string; + outcome: string; + }> = [ + { + input: undefined, + expected: 1000, + context: 'when unset', + outcome: 'defaults to 1000' + }, + { + input: '33', + context: 'when set to parsable uint', + outcome: 'sets `maxDocumentLength` to the parsed value', + expected: 33 + }, + { + input: '', + context: 'when set to an empty string', + outcome: 'defaults to 1000', + expected: 1000 + }, + { + input: 'asdf', + context: 'when set to a non-integer string', + outcome: 'defaults to 1000', + expected: 1000 + } + ]; - context('when mongodbLogPath is stdout', function () { - it('treats the value as case-insensitive', function () { - const loggerOptions = MongoLogger.resolveOptions( - { MONGODB_LOG_PATH: undefined }, - { mongodbLogPath: 'STDOUT' } + for (const { input, outcome, expected, context: _context } of tests) { + context(_context, () => { + it(outcome, () => { + const options = MongoLogger.resolveOptions( + { MONGODB_LOG_MAX_DOCUMENT_LENGTH: input }, + {} ); - expect(loggerOptions).to.have.property('logDestination', 'stdout'); + expect(options.maxDocumentLength).to.equal(expected); }); }); + } + }); - context('when mongodbLogPath is stderr', function () { - it('treats the value as case-insensitive', function () { - const loggerOptions = MongoLogger.resolveOptions( - { MONGODB_LOG_PATH: undefined }, - { mongodbLogPath: 'STDERR' } - ); - expect(loggerOptions).to.have.property('logDestination', 'stderr'); - }); - }); + context('logDestination', function () { + const stream = new Writable(); + const tests: Array<{ + env: 'stderr' | 'stdout' | undefined; + client: MongoLoggerMongoClientOptions['mongodbLogPath'] | undefined; + expectedLogDestination: MongoLoggerOptions['logDestination']; + }> = [ + { + env: undefined, + client: undefined, + expectedLogDestination: process.stderr + }, + { + env: 'stderr', + client: undefined, + expectedLogDestination: process.stderr + }, + { + env: 'stdout', + client: undefined, + expectedLogDestination: process.stdout + }, + { + env: undefined, + client: 'stdout', + expectedLogDestination: process.stdout + }, + { + env: 'stderr', + client: 'stdout', + expectedLogDestination: process.stdout + }, + { + env: 'stdout', + client: 'stdout', + expectedLogDestination: process.stdout + }, + { + env: undefined, + client: 'stderr', + expectedLogDestination: process.stderr + }, + { + env: 'stderr', + client: 'stderr', + expectedLogDestination: process.stderr + }, + { + env: 'stdout', + client: 'stderr', + expectedLogDestination: process.stderr + }, + { + env: undefined, + client: stream, + expectedLogDestination: stream + }, + { + env: 'stderr', + client: stream, + expectedLogDestination: stream + }, + { + env: 'stdout', + client: stream, + expectedLogDestination: stream + } + ]; - context('when mongodbLogPath is a non-empty string', function () { - it('uses the passed value ', function () { - const loggerOptions = MongoLogger.resolveOptions( - { MONGODB_LOG_PATH: undefined }, - { mongodbLogPath: 'stdout' } - ); - expect(loggerOptions.logDestination).to.equal('stdout'); - }); - }); - }); + for (const { env, client, expectedLogDestination } of tests) { + context( + `environment option=${env}, client option=${ + client instanceof Writable ? 'a writable stream' : client + }`, + () => { + it(`sets the log destination to ${ + expectedLogDestination instanceof Writable + ? 'the provided writable stream' + : expectedLogDestination + }`, () => { + const options = MongoLogger.resolveOptions( + { MONGODB_LOG_PATH: env }, + { mongodbLogPath: client } + ); + + expect(options).to.have.property('logDestination', expectedLogDestination); + }); + } + ); + } }); }); }); From 18db4491ea854b4eb19f394570d568c9ed28dde5 Mon Sep 17 00:00:00 2001 From: Bailey Pearson Date: Tue, 6 Dec 2022 15:39:37 -0500 Subject: [PATCH 32/50] better name --- test/unit/mongo_logger.test.ts | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/test/unit/mongo_logger.test.ts b/test/unit/mongo_logger.test.ts index b7eba738f6..7549e36476 100644 --- a/test/unit/mongo_logger.test.ts +++ b/test/unit/mongo_logger.test.ts @@ -35,7 +35,7 @@ describe('class MongoLogger', function () { 'MONGODB_LOG_SERVER_SELECTION', 'MONGODB_LOG_CONNECTION' ]; - const componentToInternalRepresentation = (component: string) => { + const mapToInternalRepresentation = (component: string) => { const options: Record = { MONGODB_LOG_COMMAND: 'command', MONGODB_LOG_TOPOLOGY: 'topology', @@ -69,29 +69,29 @@ describe('class MongoLogger', function () { }); for (const component of components) { - const internalRepresentationOfLogComponent = componentToInternalRepresentation(component); + const mappedComponent = mapToInternalRepresentation(component); context(`${component}`, function () { context(`when set to a valid value in the environment`, function () { context('when there is a default provided', function () { - it(`sets ${internalRepresentationOfLogComponent} to the provided value and ignores the default`, function () { + it(`sets ${mappedComponent} to the provided value and ignores the default`, function () { const options = MongoLogger.resolveOptions( { [component]: SeverityLevel.ALERT, MONGODB_LOG_ALL: SeverityLevel.OFF }, {} ); expect(options.componentSeverities).to.have.property( - internalRepresentationOfLogComponent, + mappedComponent, SeverityLevel.ALERT ); }); }); context('when there is no default provided', function () { - it(`sets ${internalRepresentationOfLogComponent} to the provided value`, function () { + it(`sets ${mappedComponent} to the provided value`, function () { const options = MongoLogger.resolveOptions( { [component]: SeverityLevel.ALERT, MONGODB_LOG_ALL: SeverityLevel.OFF }, {} ); expect(options.componentSeverities).to.have.property( - internalRepresentationOfLogComponent, + mappedComponent, SeverityLevel.ALERT ); }); @@ -100,25 +100,25 @@ describe('class MongoLogger', function () { context(`when set to an invalid value in the environment`, function () { context('when there is a default provided', function () { - it(`sets ${internalRepresentationOfLogComponent} to the the default`, function () { + it(`sets ${mappedComponent} to the the default`, function () { const options = MongoLogger.resolveOptions( { [component]: 'invalid value' as any, MONGODB_LOG_ALL: SeverityLevel.ALERT }, {} ); expect(options.componentSeverities).to.have.property( - internalRepresentationOfLogComponent, + mappedComponent, SeverityLevel.ALERT ); }); }); context('when there is no default provided', function () { - it(`sets ${internalRepresentationOfLogComponent} to the off`, function () { + it(`sets ${mappedComponent} to the off`, function () { const options = MongoLogger.resolveOptions( { [component]: 'invalid value' as any }, {} ); expect(options.componentSeverities).to.have.property( - internalRepresentationOfLogComponent, + mappedComponent, SeverityLevel.OFF ); }); @@ -127,23 +127,23 @@ describe('class MongoLogger', function () { context(`when unset`, () => { context(`when there is no default set`, () => { - it(`does not set ${internalRepresentationOfLogComponent}`, () => { + it(`does not set ${mappedComponent}`, () => { const options = MongoLogger.resolveOptions({}, {}); expect(options.componentSeverities).to.have.property( - internalRepresentationOfLogComponent, + mappedComponent, SeverityLevel.OFF ); }); }); context(`when there is a default set`, () => { - it(`sets ${internalRepresentationOfLogComponent} to the default`, () => { + it(`sets ${mappedComponent} to the default`, () => { const options = MongoLogger.resolveOptions( { MONGODB_LOG_ALL: SeverityLevel.DEBUG }, {} ); expect(options.componentSeverities).to.have.property( - internalRepresentationOfLogComponent, + mappedComponent, SeverityLevel.DEBUG ); }); @@ -153,7 +153,7 @@ describe('class MongoLogger', function () { it('is case insensitive', function () { const options = MongoLogger.resolveOptions({ MONGODB_LOG_ALL: 'dEbUg' as any }, {}); expect(options.componentSeverities).to.have.property( - internalRepresentationOfLogComponent, + mappedComponent, SeverityLevel.DEBUG ); }); From 2ca2cac9c8ae0e2afa6b05fc91de8fdd38a4ce93 Mon Sep 17 00:00:00 2001 From: Bailey Pearson Date: Tue, 6 Dec 2022 15:40:01 -0500 Subject: [PATCH 33/50] add mongo client test --- test/unit/mongo_client.test.js | 70 ++++++++-------------------------- 1 file changed, 15 insertions(+), 55 deletions(-) diff --git a/test/unit/mongo_client.test.js b/test/unit/mongo_client.test.js index 3ec83a4242..1be9fd2add 100644 --- a/test/unit/mongo_client.test.js +++ b/test/unit/mongo_client.test.js @@ -10,7 +10,9 @@ const { ReadPreference } = require('../../src/read_preference'); const { Logger } = require('../../src/logger'); const { MongoCredentials } = require('../../src/cmap/auth/mongo_credentials'); const { MongoClient, MongoParseError, ServerApiVersion } = require('../../src'); -const { MongoLogger, SeverityLevel } = require('../../src/mongo_logger'); +const { MongoLogger } = require('../../src/mongo_logger'); +const sinon = require('sinon'); +const { Writable } = require('stream'); describe('MongoOptions', function () { it('MongoClient should always freeze public options', function () { @@ -849,67 +851,25 @@ describe('MongoOptions', function () { }); }); - context('MongoLogger', function () { - let cachedEnvironment; - const loggerFeatureFlag = Symbol.for('@@mdb.enableMongoLogger'); - const severityVars = [ - 'MONGODB_LOG_COMMAND', - 'MONGODB_LOG_TOPOLOGY', - 'MONGODB_LOG_SERVER_SELECTION', - 'MONGODB_LOG_CONNECTION', - 'MONGODB_LOG_ALL' - ]; + context('loggingOptions', function () { + const expectedLoggingObject = { + maxDocumentLength: 20, + logDestination: new Writable() + }; before(() => { - cachedEnvironment = process.env; + sinon.stub(MongoLogger, 'resolveOptions').callsFake(() => { + return expectedLoggingObject; + }); }); after(() => { - process.env = cachedEnvironment; - }); - - beforeEach(() => { - process.env = {}; + sinon.restore(); }); - context('when only a single severity environment variable is set', function () { - for (const envName of severityVars) { - context(`when ${envName} is set to a valid value`, function () { - it('instantiates a MongoLogger', function () { - process.env[envName] = SeverityLevel.CRITICAL; - const client = new MongoClient('mongodb://localhost:27017', { - [loggerFeatureFlag]: true - }); - expect(client.mongoLogger).to.be.instanceOf(MongoLogger); - }); - }); - - context(`when ${envName} is set to an invalid value`, function () { - it('does not instantiate a MongoLogger', function () { - process.env[envName] = 'invalid'; - const client = new MongoClient('mongodb://localhost:27017', { - [loggerFeatureFlag]: true - }); - expect(client).property('mongoLogger', null); - }); - }); - } - }); - - context('when there are no environment variables set', function () { - it('should not instantiate a MongoLogger', function () { - const client = new MongoClient('mongodb://localhost:27017', { [loggerFeatureFlag]: true }); - expect(client).property('mongoLogger', null); - }); - }); - - context('when there are environment variables with both valid and invalid options', () => { - it('instantiates a MongoLogger', function () { - process.env['MONGODB_LOG_COMMAND'] = SeverityLevel.CRITICAL; - process.env['MONGODB_LOG_TOPOLOGY'] = 'invalid'; - const client = new MongoClient('mongodb://localhost:27017', { [loggerFeatureFlag]: true }); - expect(client.mongoLogger).to.be.instanceOf(MongoLogger); - }); + it('assigns the parsed options to the mongoLoggerOptions option', function () { + const client = new MongoClient('mongodb://localhost:27017'); + expect(client.options).to.have.property('mongoLoggerOptions').to.equal(expectedLoggingObject); }); }); }); From 73b3535b96cd47a86719e495da90315d99359df1 Mon Sep 17 00:00:00 2001 From: Bailey Pearson Date: Tue, 6 Dec 2022 16:10:59 -0500 Subject: [PATCH 34/50] add feature flag tests --- .../node-specific/feature_flags.test.ts | 107 +++++++----------- 1 file changed, 40 insertions(+), 67 deletions(-) diff --git a/test/integration/node-specific/feature_flags.test.ts b/test/integration/node-specific/feature_flags.test.ts index acbe4ddab0..58dba9d3f6 100644 --- a/test/integration/node-specific/feature_flags.test.ts +++ b/test/integration/node-specific/feature_flags.test.ts @@ -49,17 +49,6 @@ describe('Feature Flags', () => { describe('@@mdb.enableMongoLogger', () => { let cachedEnv; const loggerFeatureFlag = Symbol.for('@@mdb.enableMongoLogger'); - const severityMethods = [ - 'emergency', - 'alert', - 'critical', - 'error', - 'warn', - 'notice', - 'info', - 'debug', - 'trace' - ]; before(() => { cachedEnv = process.env; @@ -69,88 +58,72 @@ describe('Feature Flags', () => { process.env = cachedEnv; }); - context('when set to true', () => { - context('when logging for a component is enabled', () => { + context('when enabled', () => { + context('when logging is enabled for any component', () => { before(() => { - process.env['MONGODB_LOG_COMMAND'] = SeverityLevel.EMERGENCY; + process.env.MONGODB_LOG_COMMAND = SeverityLevel.EMERGENCY; }); - for (const severity of severityMethods) { - context(`${severity} severity logging method`, () => { - const skipReason = - severity === SeverityLevel.EMERGENCY - ? 'TODO(NODE-4813): implement the emergency severity logging method' - : 'TODO(NODE-4814): implement the remaining severity loggers'; - it.skip('should not be a no-op', () => { - const client = new MongoClient('mongodb://localhost:27017', { - [loggerFeatureFlag]: true - }); - const stringifiedMethod = client.mongoLogger[severity].toString(); - const expectedStringifiedMethod = `${severity}(component, message) { }`; - expect(stringifiedMethod).to.not.equal(expectedStringifiedMethod); - }).skipReason = skipReason; + it('enables logging for MONGODB_LOG_COMMAND', () => { + const client = new MongoClient('mongodb://localhost:27017', { + [loggerFeatureFlag]: true }); - } + expect(client.mongoLogger.componentSeverities).to.have.property( + 'command', + SeverityLevel.EMERGENCY + ); + }); }); - context('when logging for a component is not enabled', () => { + context('when logging is not enabled for any component', () => { before(() => { - process.env['MONGODB_LOG_COMMAND'] = SeverityLevel.OFF; + process.env = {}; }); - for (const severity of severityMethods) { - context(`${severity} severity logging method`, () => { - it('should be a no-op', () => { - const client = new MongoClient('mongodb://localhost:27017', { - [loggerFeatureFlag]: true - }); - const stringifiedMethod = client.mongoLogger[severity].toString(); - const expectedStringifiedMethod = `${severity}(component, message) { }`; - expect(stringifiedMethod).to.equal(expectedStringifiedMethod); - }); + it('does not enable logging', () => { + const client = new MongoClient('mongodb://localhost:27017', { + [loggerFeatureFlag]: true }); - } + expect(client.mongoLogger.componentSeverities).to.have.property( + 'command', + SeverityLevel.OFF + ); + }); }); }); for (const featureFlagValue of [false, undefined]) { context(`when set to ${featureFlagValue}`, () => { - context('when logging for a component is enabled', () => { + context('when logging is enabled for any component', () => { before(() => { process.env['MONGODB_LOG_COMMAND'] = SeverityLevel.EMERGENCY; }); - for (const severity of severityMethods) { - context(`${severity} severity logging method`, () => { - it('should be a no-op', () => { - const client = new MongoClient('mongodb://localhost:27017', { - [loggerFeatureFlag]: true - }); - const stringifiedMethod = client.mongoLogger[severity].toString(); - const expectedStringifiedMethod = `${severity}(component, message) { }`; - expect(stringifiedMethod).to.equal(expectedStringifiedMethod); - }); + it('does not enable logging', () => { + const client = new MongoClient('mongodb://localhost:27017', { + [loggerFeatureFlag]: featureFlagValue }); - } + expect(client.mongoLogger.componentSeverities).to.have.property( + 'command', + SeverityLevel.OFF + ); + }); }); - context('when logging for a component is not enabled', () => { + context('when logging is not enabled for any component', () => { before(() => { - process.env['MONGODB_LOG_COMMAND'] = SeverityLevel.OFF; + process.env = {}; }); - for (const severity of severityMethods) { - context(`${severity} severity logging method`, () => { - it('should be a no-op', () => { - const client = new MongoClient('mongodb://localhost:27017', { - [loggerFeatureFlag]: true - }); - const stringifiedMethod = client.mongoLogger[severity].toString(); - const expectedStringifiedMethod = `${severity}(component, message) { }`; - expect(stringifiedMethod).to.equal(expectedStringifiedMethod); - }); + it('does not enable logging', () => { + const client = new MongoClient('mongodb://localhost:27017', { + [loggerFeatureFlag]: featureFlagValue }); - } + expect(client.mongoLogger.componentSeverities).to.have.property( + 'command', + SeverityLevel.OFF + ); + }); }); }); } From a52493b0d14c8db226012089d3fdfff43753155d Mon Sep 17 00:00:00 2001 From: Bailey Pearson Date: Tue, 6 Dec 2022 16:11:26 -0500 Subject: [PATCH 35/50] add severity helper stream tests when helpers are off --- test/unit/mongo_logger.test.ts | 45 ++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/test/unit/mongo_logger.test.ts b/test/unit/mongo_logger.test.ts index 7549e36476..3d48405e31 100644 --- a/test/unit/mongo_logger.test.ts +++ b/test/unit/mongo_logger.test.ts @@ -8,6 +8,31 @@ import { SeverityLevel } from '../../src/mongo_logger'; +class BufferingStream extends Writable { + buffer: string[] = []; + + constructor(options = {}) { + super({ ...options, objectMode: true }); + } + + override _write(chunk, encoding, callback) { + this.buffer.push(chunk); + callback(); + } +} + +describe('meta tests for BufferingStream', function () { + it('the buffer is empty on construction', function () { + const stream = new BufferingStream(); + expect(stream.buffer).to.have.lengthOf(0); + }); + it('pushes messages to the buffer when written to', function () { + const stream = new BufferingStream(); + stream.write('message'); + expect(stream.buffer).to.deep.equal(['message']); + }); +}); + describe('class MongoLogger', function () { describe('#constructor()', function () { it('assigns each property from the options object onto the logging class', function () { @@ -299,4 +324,24 @@ describe('class MongoLogger', function () { } }); }); + + describe('severity helpers', function () { + const severities = Object.values(SeverityLevel).filter(severity => severity !== 'off'); + for (const severityLevel of severities) { + describe(`${severityLevel}()`, function () { + it('does not log when logging for the component is disabled', () => { + const stream = new BufferingStream(); + const logger = new MongoLogger({ + componentSeverities: { + topology: 'off' + } as any, + logDestination: stream + } as any); + + logger[severityLevel]('topology', 'message'); + expect(stream.buffer).to.have.lengthOf(0); + }); + }); + } + }); }); From e7491100d318c0a4e16c3b2a9b9a5a2be080103e Mon Sep 17 00:00:00 2001 From: Bailey Pearson Date: Tue, 6 Dec 2022 16:17:45 -0500 Subject: [PATCH 36/50] clean up mongo logger --- src/mongo_logger.ts | 20 +------------------- 1 file changed, 1 insertion(+), 19 deletions(-) diff --git a/src/mongo_logger.ts b/src/mongo_logger.ts index 0f67b936bc..7090269d13 100644 --- a/src/mongo_logger.ts +++ b/src/mongo_logger.ts @@ -51,24 +51,6 @@ function parseMaxDocumentLength(s?: string): number { } } -/** - * Parses a string for logDestination. Strings containing 'stderr' or 'stdout' are treated as - * case-insensitive. - * - * @param logDestination - the destination to write logs to - * @returns the original string if a file path was passed. Otherwise one of ['stderr', 'stdout']. - */ -function parseLogDestination(logDestination?: string): string { - if (typeof logDestination === 'string' && logDestination !== '') { - const lowerLogDestination = logDestination.toLowerCase(); - return lowerLogDestination === 'stdout' || lowerLogDestination === 'stderr' - ? lowerLogDestination - : logDestination; - } - - return 'stderr'; -} - /** @internal */ export const MongoLoggableComponent = Object.freeze({ COMMAND: 'command', @@ -95,7 +77,7 @@ export interface MongoLoggerEnvOptions { MONGODB_LOG_ALL?: string; /** Max length of embedded EJSON docs. Setting to 0 disables truncation. Defaults to 1000. */ MONGODB_LOG_MAX_DOCUMENT_LENGTH?: string; - /** Destination for log messages. Must be 'stderr', 'stdout', or a file path. Defaults to 'stderr'. */ + /** Destination for log messages. Must be 'stderr', 'stdout'. Defaults to 'stderr'. */ MONGODB_LOG_PATH?: string; } From 81b62fdc655c93a774968d45f3d06e1aafc1491b Mon Sep 17 00:00:00 2001 From: Bailey Pearson Date: Tue, 6 Dec 2022 22:53:01 -0500 Subject: [PATCH 37/50] Update test/integration/node-specific/feature_flags.test.ts Co-authored-by: Daria Pardue --- test/integration/node-specific/feature_flags.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/node-specific/feature_flags.test.ts b/test/integration/node-specific/feature_flags.test.ts index 58dba9d3f6..48fd197924 100644 --- a/test/integration/node-specific/feature_flags.test.ts +++ b/test/integration/node-specific/feature_flags.test.ts @@ -115,7 +115,7 @@ describe('Feature Flags', () => { process.env = {}; }); - it('does not enable logging', () => { + it('does not enable logging for any component', () => { const client = new MongoClient('mongodb://localhost:27017', { [loggerFeatureFlag]: featureFlagValue }); From a6eee378b5468592d2f8ec6e2904c76e7679e8fb Mon Sep 17 00:00:00 2001 From: Bailey Pearson Date: Tue, 6 Dec 2022 22:53:12 -0500 Subject: [PATCH 38/50] Update test/integration/node-specific/feature_flags.test.ts Co-authored-by: Daria Pardue --- test/integration/node-specific/feature_flags.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/node-specific/feature_flags.test.ts b/test/integration/node-specific/feature_flags.test.ts index 48fd197924..bed887e92f 100644 --- a/test/integration/node-specific/feature_flags.test.ts +++ b/test/integration/node-specific/feature_flags.test.ts @@ -80,7 +80,7 @@ describe('Feature Flags', () => { process.env = {}; }); - it('does not enable logging', () => { + it('does not enable logging for any component', () => { const client = new MongoClient('mongodb://localhost:27017', { [loggerFeatureFlag]: true }); From 1c86ce6c9c678d86019b06c64976b189cdcba6da Mon Sep 17 00:00:00 2001 From: Bailey Pearson Date: Tue, 6 Dec 2022 22:53:44 -0500 Subject: [PATCH 39/50] Update test/integration/node-specific/feature_flags.test.ts Co-authored-by: Daria Pardue --- test/integration/node-specific/feature_flags.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/node-specific/feature_flags.test.ts b/test/integration/node-specific/feature_flags.test.ts index bed887e92f..59560b57e9 100644 --- a/test/integration/node-specific/feature_flags.test.ts +++ b/test/integration/node-specific/feature_flags.test.ts @@ -64,7 +64,7 @@ describe('Feature Flags', () => { process.env.MONGODB_LOG_COMMAND = SeverityLevel.EMERGENCY; }); - it('enables logging for MONGODB_LOG_COMMAND', () => { + it('enables logging for the specified component', () => { const client = new MongoClient('mongodb://localhost:27017', { [loggerFeatureFlag]: true }); From 36bd25910029f0a39cc656a3f7b5066fa494e622 Mon Sep 17 00:00:00 2001 From: Bailey Pearson Date: Tue, 6 Dec 2022 22:45:12 -0500 Subject: [PATCH 40/50] cleanup test wording --- test/unit/mongo_logger.test.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/test/unit/mongo_logger.test.ts b/test/unit/mongo_logger.test.ts index 3d48405e31..3cf44537d9 100644 --- a/test/unit/mongo_logger.test.ts +++ b/test/unit/mongo_logger.test.ts @@ -307,11 +307,7 @@ describe('class MongoLogger', function () { client instanceof Writable ? 'a writable stream' : client }`, () => { - it(`sets the log destination to ${ - expectedLogDestination instanceof Writable - ? 'the provided writable stream' - : expectedLogDestination - }`, () => { + it('sets the log destination to the provided writable stream', () => { const options = MongoLogger.resolveOptions( { MONGODB_LOG_PATH: env }, { mongodbLogPath: client } From bd10d81674507afa7d58a37aa56228eae5d4cdba Mon Sep 17 00:00:00 2001 From: Bailey Pearson Date: Wed, 7 Dec 2022 17:44:27 -0500 Subject: [PATCH 41/50] rework mongodb log destination tests --- src/mongo_logger.ts | 48 +++++-- test/unit/mongo_logger.test.ts | 252 ++++++++++++++++++++++----------- 2 files changed, 206 insertions(+), 94 deletions(-) diff --git a/src/mongo_logger.ts b/src/mongo_logger.ts index 7090269d13..a9579d9acb 100644 --- a/src/mongo_logger.ts +++ b/src/mongo_logger.ts @@ -1,4 +1,4 @@ -import type { Writable } from 'stream'; +import { Writable } from 'stream'; import { getUint } from './utils'; @@ -51,6 +51,37 @@ function parseMaxDocumentLength(s?: string): number { } } +/** + * resolves the MONGODB_LOG_PATH and mongodbLogPath options from the environment and the + * mongo client options respectively. + * + * @returns the Writable stream to write logs to + */ +function resolveLogPath( + { MONGODB_LOG_PATH }: MongoLoggerEnvOptions, + { + mongodbLogPath + }: { + mongodbLogPath?: unknown; + } +): Writable { + const isValidLogDestinationString = (destination: string) => + ['stdout', 'stderr'].includes(destination.toLowerCase()); + if (typeof mongodbLogPath === 'string' && isValidLogDestinationString(mongodbLogPath)) { + return mongodbLogPath.toLowerCase() === 'stderr' ? process.stderr : process.stdout; + } + + if (typeof mongodbLogPath === 'object' && mongodbLogPath instanceof Writable) { + return mongodbLogPath; + } + + if (typeof MONGODB_LOG_PATH === 'string' && isValidLogDestinationString(MONGODB_LOG_PATH)) { + return MONGODB_LOG_PATH.toLowerCase() === 'stderr' ? process.stderr : process.stdout; + } + + return process.stderr; +} + /** @internal */ export const MongoLoggableComponent = Object.freeze({ COMMAND: 'command', @@ -161,21 +192,10 @@ export class MongoLogger { const combinedOptions = { ...envOptions, ...clientOptions, - mongodbLogPath: - clientOptions.mongodbLogPath != null - ? clientOptions.mongodbLogPath - : envOptions.MONGODB_LOG_PATH != null - ? envOptions.MONGODB_LOG_PATH - : 'stderr' + mongodbLogPath: resolveLogPath(envOptions, clientOptions) }; const defaultSeverity = parseSeverityFromString(combinedOptions.MONGODB_LOG_ALL) ?? SeverityLevel.OFF; - const logDestination: MongoLoggerOptions['logDestination'] = - typeof combinedOptions.mongodbLogPath !== 'string' - ? combinedOptions.mongodbLogPath - : combinedOptions.mongodbLogPath === 'stderr' - ? process.stderr - : process.stdout; return { componentSeverities: { @@ -188,7 +208,7 @@ export class MongoLogger { default: defaultSeverity }, maxDocumentLength: parseMaxDocumentLength(combinedOptions.MONGODB_LOG_MAX_DOCUMENT_LENGTH), - logDestination + logDestination: combinedOptions.mongodbLogPath }; } } diff --git a/test/unit/mongo_logger.test.ts b/test/unit/mongo_logger.test.ts index 3cf44537d9..2dcf42f15c 100644 --- a/test/unit/mongo_logger.test.ts +++ b/test/unit/mongo_logger.test.ts @@ -1,6 +1,7 @@ import { expect } from 'chai'; -import { Writable } from 'stream'; +import { Readable, Writable } from 'stream'; +import { CaseInsensitiveMap } from '../../src/connection_string'; import { MongoLogger, MongoLoggerMongoClientOptions, @@ -234,90 +235,181 @@ describe('class MongoLogger', function () { context('logDestination', function () { const stream = new Writable(); - const tests: Array<{ - env: 'stderr' | 'stdout' | undefined; - client: MongoLoggerMongoClientOptions['mongodbLogPath'] | undefined; - expectedLogDestination: MongoLoggerOptions['logDestination']; - }> = [ - { - env: undefined, - client: undefined, - expectedLogDestination: process.stderr - }, - { - env: 'stderr', - client: undefined, - expectedLogDestination: process.stderr - }, - { - env: 'stdout', - client: undefined, - expectedLogDestination: process.stdout - }, - { - env: undefined, - client: 'stdout', - expectedLogDestination: process.stdout - }, - { - env: 'stderr', - client: 'stdout', - expectedLogDestination: process.stdout - }, - { - env: 'stdout', - client: 'stdout', - expectedLogDestination: process.stdout - }, - { - env: undefined, - client: 'stderr', - expectedLogDestination: process.stderr - }, - { - env: 'stderr', - client: 'stderr', - expectedLogDestination: process.stderr - }, - { - env: 'stdout', - client: 'stderr', - expectedLogDestination: process.stderr - }, - { - env: undefined, - client: stream, - expectedLogDestination: stream - }, - { - env: 'stderr', - client: stream, - expectedLogDestination: stream - }, - { - env: 'stdout', - client: stream, - expectedLogDestination: stream + const validOptions: Map = new Map([ + ['stdout', process.stdout], + ['stderr', process.stderr], + [stream, stream], + ['stdOut', process.stdout], + ['stdErr', process.stderr] + ] as Array<[any, Writable]>); + const unsetOptions = ['', undefined]; + const invalidEnvironmentOptions = ['non-acceptable-string']; + const invalidClientOptions = ['', ' ', undefined, null, 0, false, new Readable()]; + const validClientOptions = ['stderr', 'stdout', stream, 'stdErr', 'stdOut']; + const validEnvironmentOptions = ['stderr', 'stdout', 'stdOut', 'stdErr']; + context('when MONGODB_LOG_DESTINATION is unset in the environment', function () { + context('when mongodbLogPath is unset as a client option', function () { + for (const unsetEnvironmentOption of unsetOptions) { + for (const unsetOption of unsetOptions) { + it(`{environment: "${unsetEnvironmentOption}", client: "${unsetOption}"} defaults to process.stderr`, function () { + const options = MongoLogger.resolveOptions( + { + MONGODB_LOG_PATH: unsetEnvironmentOption + }, + { mongodbLogPath: unsetOption as any } + ); + expect(options.logDestination).to.equal(process.stderr); + }); + } + } + }); + + context('when mongodbLogPath is an invalid client option', function () { + for (const unsetEnvironmentOption of unsetOptions) { + for (const invalidOption of invalidClientOptions) { + it(`{environment: "${unsetEnvironmentOption}", client: "${invalidOption}"} defaults to process.stderr`, function () { + const options = MongoLogger.resolveOptions( + { + MONGODB_LOG_PATH: unsetEnvironmentOption + }, + { mongodbLogPath: invalidOption as any } + ); + expect(options.logDestination).to.equal(process.stderr); + }); + } + } + }); + + context('when mongodbLogPath is a valid client option', function () { + for (const unsetEnvironmentOption of unsetOptions) { + for (const validOption of validClientOptions) { + it(`{environment: "${unsetEnvironmentOption}", client: "${validOption}"} uses the value from the client options`, function () { + const options = MongoLogger.resolveOptions( + { + MONGODB_LOG_PATH: unsetEnvironmentOption + }, + { mongodbLogPath: validOption as any } + ); + const correctDestination = validOptions.get(validOption); + expect(options.logDestination).to.equal(correctDestination); + }); + } + } + }); + }); + + context( + 'when MONGODB_LOG_DESTINATION is set to an invalid value in the environment', + function () { + context('when mongodbLogPath is unset on the client options', function () { + for (const invalidEnvironmentOption of invalidEnvironmentOptions) { + for (const unsetClientOption of unsetOptions) { + it(`{environment: "${invalidEnvironmentOption}", client: "${unsetClientOption}"} defaults to process.stderr`, function () { + const options = MongoLogger.resolveOptions( + { + MONGODB_LOG_PATH: invalidEnvironmentOption + }, + { mongodbLogPath: unsetClientOption as any } + ); + expect(options.logDestination).to.equal(process.stderr); + }); + } + } + }); + + context( + 'when mongodbLogPath is set to an invalid value on the client options', + function () { + for (const invalidEnvironmentOption of invalidEnvironmentOptions) { + for (const invalidOption of invalidClientOptions) { + it(`{environment: "${invalidEnvironmentOption}", client: "${invalidOption}"} defaults to process.stderr`, function () { + const options = MongoLogger.resolveOptions( + { + MONGODB_LOG_PATH: invalidEnvironmentOption + }, + { mongodbLogPath: invalidOption as any } + ); + expect(options.logDestination).to.equal(process.stderr); + }); + } + } + } + ); + + context('when mongodbLogPath is set to a valid value on the client options', function () { + for (const invalidEnvironmentOption of invalidEnvironmentOptions) { + for (const validOption of validClientOptions) { + it(`{environment: "${invalidEnvironmentOption}", client: "${validOption}"} uses the value from the client options`, function () { + const options = MongoLogger.resolveOptions( + { + MONGODB_LOG_PATH: invalidEnvironmentOption + }, + { mongodbLogPath: validOption as any } + ); + const correctDestination = validOptions.get(validOption); + expect(options.logDestination).to.equal(correctDestination); + }); + } + } + }); } - ]; + ); - for (const { env, client, expectedLogDestination } of tests) { - context( - `environment option=${env}, client option=${ - client instanceof Writable ? 'a writable stream' : client - }`, - () => { - it('sets the log destination to the provided writable stream', () => { - const options = MongoLogger.resolveOptions( - { MONGODB_LOG_PATH: env }, - { mongodbLogPath: client } - ); + context('when MONGODB_LOG_PATH is set to a valid option in the environment', function () { + context('when mongodbLogPath is unset on the client options', function () { + for (const validEnvironmentOption of validEnvironmentOptions) { + for (const unsetOption of unsetOptions) { + it(`{environment: "${validEnvironmentOption}", client: "${unsetOption}"} uses process.${validEnvironmentOption}`, function () { + const options = MongoLogger.resolveOptions( + { + MONGODB_LOG_PATH: validEnvironmentOption + }, + { mongodbLogPath: unsetOption as any } + ); + const correctDestination = validOptions.get(validEnvironmentOption); + expect(options.logDestination).to.equal(correctDestination); + }); + } + } + }); - expect(options).to.have.property('logDestination', expectedLogDestination); - }); + context( + 'when mongodbLogPath is set to an invalid value on the client options', + function () { + for (const validEnvironmentOption of validEnvironmentOptions) { + for (const invalidValue of invalidClientOptions) { + it(`{environment: "${validEnvironmentOption}", client: "${invalidValue}"} uses process.${validEnvironmentOption}`, function () { + const options = MongoLogger.resolveOptions( + { + MONGODB_LOG_PATH: validEnvironmentOption + }, + { mongodbLogPath: invalidValue as any } + ); + const correctDestination = validOptions.get(validEnvironmentOption); + expect(options.logDestination).to.equal(correctDestination); + }); + } + } } ); - } + + context('when mongodbLogPath is set to valid client option', function () { + for (const validEnvironmentOption of validEnvironmentOptions) { + for (const validValue of validClientOptions) { + it(`{environment: "${validEnvironmentOption}", client: "${validValue}"} uses the value from the client options`, function () { + const options = MongoLogger.resolveOptions( + { + MONGODB_LOG_PATH: validEnvironmentOption + }, + { mongodbLogPath: validValue as any } + ); + const correctDestination = validOptions.get(validValue); + expect(options.logDestination).to.equal(correctDestination); + }); + } + } + }); + }); }); }); From 67fd13d4fbe9c6108497e40ae962134c521eef9f Mon Sep 17 00:00:00 2001 From: Bailey Pearson Date: Thu, 8 Dec 2022 10:19:41 -0500 Subject: [PATCH 42/50] add tests for componentSeverities --- test/unit/mongo_logger.test.ts | 305 ++++++++++++++++++++++----------- 1 file changed, 201 insertions(+), 104 deletions(-) diff --git a/test/unit/mongo_logger.test.ts b/test/unit/mongo_logger.test.ts index 2dcf42f15c..c1b051d522 100644 --- a/test/unit/mongo_logger.test.ts +++ b/test/unit/mongo_logger.test.ts @@ -1,4 +1,5 @@ import { expect } from 'chai'; +import { valid } from 'semver'; import { Readable, Writable } from 'stream'; import { CaseInsensitiveMap } from '../../src/connection_string'; @@ -55,136 +56,232 @@ describe('class MongoLogger', function () { describe('static #resolveOptions()', function () { describe('componentSeverities', function () { - const components = [ - 'MONGODB_LOG_COMMAND', - 'MONGODB_LOG_TOPOLOGY', - 'MONGODB_LOG_SERVER_SELECTION', - 'MONGODB_LOG_CONNECTION' - ]; - const mapToInternalRepresentation = (component: string) => { - const options: Record = { - MONGODB_LOG_COMMAND: 'command', - MONGODB_LOG_TOPOLOGY: 'topology', - MONGODB_LOG_SERVER_SELECTION: 'serverSelection', - MONGODB_LOG_CONNECTION: 'connection' - }; - return options[component]; - }; + const components = new Map([ + ['MONGODB_LOG_COMMAND', 'command'], + ['MONGODB_LOG_TOPOLOGY', 'topology'], + ['MONGODB_LOG_SERVER_SELECTION', 'serverSelection'], + ['MONGODB_LOG_CONNECTION', 'connection'] + ]); - context('MONGODB_LOG_ALL', () => { - context('when a default is provided', () => { - it('sets default to the provided value', () => { - const options = MongoLogger.resolveOptions( - { MONGODB_LOG_ALL: SeverityLevel.ALERT }, - {} - ); - expect(options.componentSeverities).to.have.property('default', SeverityLevel.ALERT); - }); - }); - context('when no value is provided', () => { - it('sets default to off', () => { - const options = MongoLogger.resolveOptions({ MONGODB_LOG_ALL: SeverityLevel.OFF }, {}); - expect(options.componentSeverities).to.have.property('default', SeverityLevel.OFF); - }); - }); + function* makeValidOptions(): Generator<[string, string]> { + const validOptions = Object.values(SeverityLevel).filter( + option => option !== SeverityLevel.OFF + ); + for (const option of validOptions) { + yield [option, option]; + yield [option.toUpperCase(), option]; + } + } - it('is case insensitive', () => { - const options = MongoLogger.resolveOptions({ MONGODB_LOG_ALL: 'dEbUg' }, {}); - expect(options.componentSeverities).to.have.property('default', SeverityLevel.DEBUG); - }); - }); + const invalidOptions = ['', 'invalid-string']; + const validNonDefaultOptions = new Map(makeValidOptions()); - for (const component of components) { - const mappedComponent = mapToInternalRepresentation(component); - context(`${component}`, function () { - context(`when set to a valid value in the environment`, function () { - context('when there is a default provided', function () { - it(`sets ${mappedComponent} to the provided value and ignores the default`, function () { - const options = MongoLogger.resolveOptions( - { [component]: SeverityLevel.ALERT, MONGODB_LOG_ALL: SeverityLevel.OFF }, + context('default', () => { + context('when MONGODB_LOG_ALL is unset', () => { + it('sets default to OFF', () => { + const { componentSeverities } = MongoLogger.resolveOptions({}, {}); + expect(componentSeverities.default).to.equal(SeverityLevel.OFF); + }); + }); + context('when MONGODB_LOG_ALL is invalid', () => { + for (const invalidOption of invalidOptions) { + context(`{ MONGODB_LOG_ALL: '${invalidOption} }'`, () => { + it('sets default to OFF', () => { + const { componentSeverities } = MongoLogger.resolveOptions( + { + MONGODB_LOG_ALL: invalidOption + }, {} ); - expect(options.componentSeverities).to.have.property( - mappedComponent, - SeverityLevel.ALERT - ); + expect(componentSeverities.default).to.equal(SeverityLevel.OFF); }); }); - context('when there is no default provided', function () { - it(`sets ${mappedComponent} to the provided value`, function () { - const options = MongoLogger.resolveOptions( - { [component]: SeverityLevel.ALERT, MONGODB_LOG_ALL: SeverityLevel.OFF }, + } + }); + context('when MONGODB_LOG_ALL is valid', () => { + for (const [validOption, expectedValue] of validNonDefaultOptions) { + context(`{ MONGODB_LOG_ALL: '${validOption} }'`, () => { + it('sets default to the value of MONGODB_LOG_ALL', () => { + const { componentSeverities } = MongoLogger.resolveOptions( + { + MONGODB_LOG_ALL: validOption + }, {} ); - expect(options.componentSeverities).to.have.property( - mappedComponent, - SeverityLevel.ALERT - ); + expect(componentSeverities.default).to.equal(expectedValue); }); }); + } + }); + for (const [loggingComponent, componentSeverityOption] of components) { + context(`when ${loggingComponent} is unset`, () => { + context(`when MONGODB_LOG_ALL is unset`, () => { + it(`sets ${componentSeverityOption} to OFF`, () => { + const { componentSeverities } = MongoLogger.resolveOptions({}, {}); + expect(componentSeverities[componentSeverityOption]).to.equal(SeverityLevel.OFF); + }); + }); + context(`when MONGODB_LOG_ALL is set to an invalid value`, () => { + for (const invalidOption of invalidOptions) { + context(`{ MONGODB_LOG_ALL: ${invalidOption} }`, () => { + it(`sets ${invalidOption} to OFF`, () => { + const { componentSeverities } = MongoLogger.resolveOptions( + { + MONGODB_LOG_ALL: invalidOption + }, + {} + ); + expect(componentSeverities[componentSeverityOption]).to.equal( + SeverityLevel.OFF + ); + }); + }); + } + }); + context(`when MONGODB_LOG_ALL is set to a valid value`, () => { + for (const [option, expectedValue] of validNonDefaultOptions) { + context(`{ MONGODB_LOG_ALL: ${option} }`, () => { + it(`sets ${option} to the value of MONGODB_LOG_ALL`, () => { + const { componentSeverities } = MongoLogger.resolveOptions( + { + MONGODB_LOG_ALL: option + }, + {} + ); + expect(componentSeverities[componentSeverityOption]).to.equal(expectedValue); + }); + }); + } + }); }); + context(`when ${loggingComponent} is set to an invalid value in the environment`, () => { + context(`when MONGODB_LOG_ALL is unset`, () => { + for (const invalidOption of invalidOptions) { + context(`{ ${loggingComponent}: ${invalidOption} }`, () => { + it(`sets ${componentSeverityOption} to OFF`, () => { + const { componentSeverities } = MongoLogger.resolveOptions( + { + [loggingComponent]: invalidOption + }, + {} + ); - context(`when set to an invalid value in the environment`, function () { - context('when there is a default provided', function () { - it(`sets ${mappedComponent} to the the default`, function () { - const options = MongoLogger.resolveOptions( - { [component]: 'invalid value' as any, MONGODB_LOG_ALL: SeverityLevel.ALERT }, - {} - ); - expect(options.componentSeverities).to.have.property( - mappedComponent, - SeverityLevel.ALERT - ); - }); + expect(componentSeverities[componentSeverityOption]).to.equal( + SeverityLevel.OFF + ); + }); + }); + } }); - context('when there is no default provided', function () { - it(`sets ${mappedComponent} to the off`, function () { - const options = MongoLogger.resolveOptions( - { [component]: 'invalid value' as any }, - {} + context(`when MONGODB_LOG_ALL is set to an invalid value`, () => { + for (const invalidOption of invalidOptions) { + context( + `{ ${loggingComponent}: ${invalidOption}, MONGODB_LOG_ALL: ${invalidOption} }`, + () => { + it(`sets ${componentSeverityOption} to OFF`, () => { + const { componentSeverities } = MongoLogger.resolveOptions( + { + [loggingComponent]: invalidOption, + MONGODB_LOG_ALL: invalidOption + }, + {} + ); + + expect(componentSeverities[componentSeverityOption]).to.equal( + SeverityLevel.OFF + ); + }); + } ); - expect(options.componentSeverities).to.have.property( - mappedComponent, - SeverityLevel.OFF + } + }); + context(`when MONGODB_LOG_ALL is set to a valid value`, () => { + const invalidOption = invalidOptions[0]; + + for (const [option, expectedValue] of validNonDefaultOptions) { + context( + `{ MONGODB_LOG_ALL: ${option}, ${componentSeverityOption}: ${option} }`, + () => { + it(`sets ${componentSeverityOption} to the value of MONGODB_LOG_ALL`, () => { + const { componentSeverities } = MongoLogger.resolveOptions( + { + [loggingComponent]: invalidOption, + MONGODB_LOG_ALL: option + }, + {} + ); + expect(componentSeverities[componentSeverityOption]).to.equal(expectedValue); + }); + } ); + } + it(`sets ${componentSeverityOption} to the value of MONGODB_LOG_ALL`, () => { + expect(true).to.be.true; }); }); }); + context(`when ${loggingComponent} is set to a valid value in the environment`, () => { + context(`when MONGODB_LOG_ALL is unset`, () => { + for (const [option, expectedValue] of validNonDefaultOptions) { + context(`{ ${loggingComponent}: ${option} }`, () => { + it(`sets ${componentSeverityOption} to the value of ${loggingComponent}`, () => { + const { componentSeverities } = MongoLogger.resolveOptions( + { + [loggingComponent]: option + }, + {} + ); - context(`when unset`, () => { - context(`when there is no default set`, () => { - it(`does not set ${mappedComponent}`, () => { - const options = MongoLogger.resolveOptions({}, {}); - expect(options.componentSeverities).to.have.property( - mappedComponent, - SeverityLevel.OFF - ); - }); + expect(componentSeverities[componentSeverityOption]).to.equal(expectedValue); + }); + }); + } }); + context(`when MONGODB_LOG_ALL is set to an invalid value`, () => { + const invalidValue = invalidOptions[0]; + for (const [option, expectedValue] of validNonDefaultOptions) { + context( + `{ ${loggingComponent}: ${option}, MONGODB_LOG_ALL: ${invalidValue} }`, + () => { + it(`sets ${componentSeverityOption} to the value of ${loggingComponent}`, () => { + const { componentSeverities } = MongoLogger.resolveOptions( + { + [loggingComponent]: option, + MONGODB_LOG_ALL: invalidValue + }, + {} + ); - context(`when there is a default set`, () => { - it(`sets ${mappedComponent} to the default`, () => { - const options = MongoLogger.resolveOptions( - { MONGODB_LOG_ALL: SeverityLevel.DEBUG }, - {} + expect(componentSeverities[componentSeverityOption]).to.equal(expectedValue); + }); + } ); - expect(options.componentSeverities).to.have.property( - mappedComponent, - SeverityLevel.DEBUG - ); - }); + } }); - }); + context(`when MONGODB_LOG_ALL is set to a valid value`, () => { + const validOption = validNonDefaultOptions.keys()[0]; + for (const [option, expectedValue] of validNonDefaultOptions) { + context( + `{ ${loggingComponent}: ${option}, MONGODB_LOG_ALL: ${validOption} }`, + () => { + it(`sets ${componentSeverityOption} to the value of ${loggingComponent}`, () => { + const { componentSeverities } = MongoLogger.resolveOptions( + { + [loggingComponent]: option, + MONGODB_LOG_ALL: validOption + }, + {} + ); - it('is case insensitive', function () { - const options = MongoLogger.resolveOptions({ MONGODB_LOG_ALL: 'dEbUg' as any }, {}); - expect(options.componentSeverities).to.have.property( - mappedComponent, - SeverityLevel.DEBUG - ); + expect(componentSeverities[componentSeverityOption]).to.equal(expectedValue); + }); + } + ); + } + }); }); - }); - } + } + }); }); context('maxDocumentLength', function () { From 85d598ec397f86e7525c546c166b738730fe720f Mon Sep 17 00:00:00 2001 From: Bailey Pearson Date: Thu, 8 Dec 2022 10:20:12 -0500 Subject: [PATCH 43/50] restructure mongo_logger file slightly --- src/mongo_logger.ts | 114 ++++++++++++++++++++++---------------------- 1 file changed, 57 insertions(+), 57 deletions(-) diff --git a/src/mongo_logger.ts b/src/mongo_logger.ts index a9579d9acb..1f2d49a7ff 100644 --- a/src/mongo_logger.ts +++ b/src/mongo_logger.ts @@ -19,6 +19,63 @@ export const SeverityLevel = Object.freeze({ /** @internal */ export type SeverityLevel = typeof SeverityLevel[keyof typeof SeverityLevel]; +/** @internal */ +export const MongoLoggableComponent = Object.freeze({ + COMMAND: 'command', + TOPOLOGY: 'topology', + SERVER_SELECTION: 'serverSelection', + CONNECTION: 'connection' +} as const); + +/** @internal */ +export type MongoLoggableComponent = + typeof MongoLoggableComponent[keyof typeof MongoLoggableComponent]; + +/** @internal */ +export interface MongoLoggerEnvOptions { + /** Severity level for command component */ + MONGODB_LOG_COMMAND?: string; + /** Severity level for topology component */ + MONGODB_LOG_TOPOLOGY?: string; + /** Severity level for server selection component */ + MONGODB_LOG_SERVER_SELECTION?: string; + /** Severity level for CMAP */ + MONGODB_LOG_CONNECTION?: string; + /** Default severity level to be if any of the above are unset */ + MONGODB_LOG_ALL?: string; + /** Max length of embedded EJSON docs. Setting to 0 disables truncation. Defaults to 1000. */ + MONGODB_LOG_MAX_DOCUMENT_LENGTH?: string; + /** Destination for log messages. Must be 'stderr', 'stdout'. Defaults to 'stderr'. */ + MONGODB_LOG_PATH?: string; +} + +/** @internal */ +export interface MongoLoggerMongoClientOptions { + /** Destination for log messages */ + mongodbLogPath?: 'stdout' | 'stderr' | Writable; +} + +/** @internal */ +export interface MongoLoggerOptions { + componentSeverities: { + /** Severity level for command component */ + command: SeverityLevel; + /** Severity level for topology component */ + topology: SeverityLevel; + /** Severity level for server selection component */ + serverSelection: SeverityLevel; + /** Severity level for connection component */ + connection: SeverityLevel; + /** Default severity level to be used if any of the above are unset */ + default: SeverityLevel; + }; + + /** Max length of embedded EJSON docs. Setting to 0 disables truncation. Defaults to 1000. */ + maxDocumentLength: number; + /** Destination for log messages. */ + logDestination: Writable; +} + /** * Parses a string as one of SeverityLevel * @@ -82,63 +139,6 @@ function resolveLogPath( return process.stderr; } -/** @internal */ -export const MongoLoggableComponent = Object.freeze({ - COMMAND: 'command', - TOPOLOGY: 'topology', - SERVER_SELECTION: 'serverSelection', - CONNECTION: 'connection' -} as const); - -/** @internal */ -export type MongoLoggableComponent = - typeof MongoLoggableComponent[keyof typeof MongoLoggableComponent]; - -/** @internal */ -export interface MongoLoggerEnvOptions { - /** Severity level for command component */ - MONGODB_LOG_COMMAND?: string; - /** Severity level for topology component */ - MONGODB_LOG_TOPOLOGY?: string; - /** Severity level for server selection component */ - MONGODB_LOG_SERVER_SELECTION?: string; - /** Severity level for CMAP */ - MONGODB_LOG_CONNECTION?: string; - /** Default severity level to be if any of the above are unset */ - MONGODB_LOG_ALL?: string; - /** Max length of embedded EJSON docs. Setting to 0 disables truncation. Defaults to 1000. */ - MONGODB_LOG_MAX_DOCUMENT_LENGTH?: string; - /** Destination for log messages. Must be 'stderr', 'stdout'. Defaults to 'stderr'. */ - MONGODB_LOG_PATH?: string; -} - -/** @internal */ -export interface MongoLoggerMongoClientOptions { - /** Destination for log messages */ - mongodbLogPath?: 'stdout' | 'stderr' | Writable; -} - -/** @internal */ -export interface MongoLoggerOptions { - componentSeverities: { - /** Severity level for command component */ - command: SeverityLevel; - /** Severity level for topology component */ - topology: SeverityLevel; - /** Severity level for server selection component */ - serverSelection: SeverityLevel; - /** Severity level for connection component */ - connection: SeverityLevel; - /** Default severity level to be used if any of the above are unset */ - default: SeverityLevel; - }; - - /** Max length of embedded EJSON docs. Setting to 0 disables truncation. Defaults to 1000. */ - maxDocumentLength: number; - /** Destination for log messages. */ - logDestination: Writable; -} - /** @internal */ export class MongoLogger { componentSeverities: Record; From fdcfc3e2e13fec6021fad97353f7571602a6113a Mon Sep 17 00:00:00 2001 From: Bailey Pearson Date: Mon, 12 Dec 2022 15:16:56 -0500 Subject: [PATCH 44/50] chore: address Daria's comments in feature flags tests --- .../node-specific/feature_flags.test.ts | 43 ++++++++++++------- 1 file changed, 28 insertions(+), 15 deletions(-) diff --git a/test/integration/node-specific/feature_flags.test.ts b/test/integration/node-specific/feature_flags.test.ts index 59560b57e9..aba7103467 100644 --- a/test/integration/node-specific/feature_flags.test.ts +++ b/test/integration/node-specific/feature_flags.test.ts @@ -1,7 +1,7 @@ import { expect } from 'chai'; import { MongoClient } from '../../../src'; -import { SeverityLevel } from '../../../src/mongo_logger'; +import { MongoLoggableComponent, SeverityLevel } from '../../../src/mongo_logger'; describe('Feature Flags', () => { describe('@@mdb.skipPingOnConnect', () => { @@ -49,6 +49,13 @@ 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; @@ -84,17 +91,19 @@ describe('Feature Flags', () => { const client = new MongoClient('mongodb://localhost:27017', { [loggerFeatureFlag]: true }); - expect(client.mongoLogger.componentSeverities).to.have.property( - 'command', - SeverityLevel.OFF - ); + for (const component of components) { + expect(client.mongoLogger.componentSeverities).to.have.property( + component, + SeverityLevel.OFF + ); + } }); }); }); for (const featureFlagValue of [false, undefined]) { context(`when set to ${featureFlagValue}`, () => { - context('when logging is enabled for any component', () => { + context('when logging is enabled for a component', () => { before(() => { process.env['MONGODB_LOG_COMMAND'] = SeverityLevel.EMERGENCY; }); @@ -103,10 +112,12 @@ describe('Feature Flags', () => { const client = new MongoClient('mongodb://localhost:27017', { [loggerFeatureFlag]: featureFlagValue }); - expect(client.mongoLogger.componentSeverities).to.have.property( - 'command', - SeverityLevel.OFF - ); + for (const component of components) { + expect(client.mongoLogger.componentSeverities).to.have.property( + component, + SeverityLevel.OFF + ); + } }); }); @@ -115,14 +126,16 @@ describe('Feature Flags', () => { process.env = {}; }); - it('does not enable logging for any component', () => { + it('does not enable logging', () => { const client = new MongoClient('mongodb://localhost:27017', { [loggerFeatureFlag]: featureFlagValue }); - expect(client.mongoLogger.componentSeverities).to.have.property( - 'command', - SeverityLevel.OFF - ); + for (const component of components) { + expect(client.mongoLogger.componentSeverities).to.have.property( + component, + SeverityLevel.OFF + ); + } }); }); }); From 32f1f2635414881c1630935b6c054bb049d78ab0 Mon Sep 17 00:00:00 2001 From: Bailey Pearson Date: Mon, 12 Dec 2022 15:28:30 -0500 Subject: [PATCH 45/50] chore: address Daria's comments in mongo logger tests --- test/unit/mongo_logger.test.ts | 278 ++++++++++++++++----------------- 1 file changed, 137 insertions(+), 141 deletions(-) diff --git a/test/unit/mongo_logger.test.ts b/test/unit/mongo_logger.test.ts index c1b051d522..98cbc5d109 100644 --- a/test/unit/mongo_logger.test.ts +++ b/test/unit/mongo_logger.test.ts @@ -1,14 +1,7 @@ import { expect } from 'chai'; -import { valid } from 'semver'; import { Readable, Writable } from 'stream'; -import { CaseInsensitiveMap } from '../../src/connection_string'; -import { - MongoLogger, - MongoLoggerMongoClientOptions, - MongoLoggerOptions, - SeverityLevel -} from '../../src/mongo_logger'; +import { MongoLogger, MongoLoggerOptions, SeverityLevel } from '../../src/mongo_logger'; class BufferingStream extends Writable { buffer: string[] = []; @@ -83,6 +76,7 @@ describe('class MongoLogger', function () { expect(componentSeverities.default).to.equal(SeverityLevel.OFF); }); }); + context('when MONGODB_LOG_ALL is invalid', () => { for (const invalidOption of invalidOptions) { context(`{ MONGODB_LOG_ALL: '${invalidOption} }'`, () => { @@ -98,6 +92,7 @@ describe('class MongoLogger', function () { }); } }); + context('when MONGODB_LOG_ALL is valid', () => { for (const [validOption, expectedValue] of validNonDefaultOptions) { context(`{ MONGODB_LOG_ALL: '${validOption} }'`, () => { @@ -113,175 +108,176 @@ describe('class MongoLogger', function () { }); } }); - for (const [loggingComponent, componentSeverityOption] of components) { - context(`when ${loggingComponent} is unset`, () => { - context(`when MONGODB_LOG_ALL is unset`, () => { - it(`sets ${componentSeverityOption} to OFF`, () => { - const { componentSeverities } = MongoLogger.resolveOptions({}, {}); - expect(componentSeverities[componentSeverityOption]).to.equal(SeverityLevel.OFF); - }); + }); + for (const [loggingComponent, componentSeverityOption] of components) { + context(`when ${loggingComponent} is unset`, () => { + context(`when MONGODB_LOG_ALL is unset`, () => { + it(`sets ${componentSeverityOption} to OFF`, () => { + const { componentSeverities } = MongoLogger.resolveOptions({}, {}); + expect(componentSeverities[componentSeverityOption]).to.equal(SeverityLevel.OFF); }); - context(`when MONGODB_LOG_ALL is set to an invalid value`, () => { - for (const invalidOption of invalidOptions) { - context(`{ MONGODB_LOG_ALL: ${invalidOption} }`, () => { - it(`sets ${invalidOption} to OFF`, () => { + }); + + context(`when MONGODB_LOG_ALL is set to an invalid value`, () => { + for (const invalidOption of invalidOptions) { + context(`{ MONGODB_LOG_ALL: ${invalidOption} }`, () => { + it(`sets ${invalidOption} to OFF`, () => { + const { componentSeverities } = MongoLogger.resolveOptions( + { + MONGODB_LOG_ALL: invalidOption + }, + {} + ); + expect(componentSeverities[componentSeverityOption]).to.equal(SeverityLevel.OFF); + }); + }); + } + }); + + context(`when MONGODB_LOG_ALL is set to a valid value`, () => { + for (const [option, expectedValue] of validNonDefaultOptions) { + context(`{ MONGODB_LOG_ALL: ${option} }`, () => { + it(`sets ${option} to the value of MONGODB_LOG_ALL`, () => { + const { componentSeverities } = MongoLogger.resolveOptions( + { + MONGODB_LOG_ALL: option + }, + {} + ); + expect(componentSeverities[componentSeverityOption]).to.equal(expectedValue); + }); + }); + } + }); + }); + + context(`when ${loggingComponent} is set to an invalid value in the environment`, () => { + context(`when MONGODB_LOG_ALL is unset`, () => { + for (const invalidOption of invalidOptions) { + context(`{ ${loggingComponent}: ${invalidOption} }`, () => { + it(`sets ${componentSeverityOption} to OFF`, () => { + const { componentSeverities } = MongoLogger.resolveOptions( + { + [loggingComponent]: invalidOption + }, + {} + ); + + expect(componentSeverities[componentSeverityOption]).to.equal(SeverityLevel.OFF); + }); + }); + } + }); + + context(`when MONGODB_LOG_ALL is set to an invalid value`, () => { + for (const invalidOption of invalidOptions) { + context( + `{ ${loggingComponent}: ${invalidOption}, MONGODB_LOG_ALL: ${invalidOption} }`, + () => { + it(`sets ${componentSeverityOption} to OFF`, () => { const { componentSeverities } = MongoLogger.resolveOptions( { + [loggingComponent]: invalidOption, MONGODB_LOG_ALL: invalidOption }, {} ); + expect(componentSeverities[componentSeverityOption]).to.equal( SeverityLevel.OFF ); }); - }); - } - }); - context(`when MONGODB_LOG_ALL is set to a valid value`, () => { - for (const [option, expectedValue] of validNonDefaultOptions) { - context(`{ MONGODB_LOG_ALL: ${option} }`, () => { - it(`sets ${option} to the value of MONGODB_LOG_ALL`, () => { + } + ); + } + }); + + context(`when MONGODB_LOG_ALL is set to a valid value`, () => { + const invalidOption = invalidOptions[0]; + + for (const [option, expectedValue] of validNonDefaultOptions) { + context( + `{ MONGODB_LOG_ALL: ${option}, ${componentSeverityOption}: ${option} }`, + () => { + it(`sets ${componentSeverityOption} to the value of MONGODB_LOG_ALL`, () => { const { componentSeverities } = MongoLogger.resolveOptions( { + [loggingComponent]: invalidOption, MONGODB_LOG_ALL: option }, {} ); expect(componentSeverities[componentSeverityOption]).to.equal(expectedValue); }); - }); - } + } + ); + } + it(`sets ${componentSeverityOption} to the value of MONGODB_LOG_ALL`, () => { + expect(true).to.be.true; }); }); - context(`when ${loggingComponent} is set to an invalid value in the environment`, () => { - context(`when MONGODB_LOG_ALL is unset`, () => { - for (const invalidOption of invalidOptions) { - context(`{ ${loggingComponent}: ${invalidOption} }`, () => { - it(`sets ${componentSeverityOption} to OFF`, () => { - const { componentSeverities } = MongoLogger.resolveOptions( - { - [loggingComponent]: invalidOption - }, - {} - ); + }); - expect(componentSeverities[componentSeverityOption]).to.equal( - SeverityLevel.OFF - ); - }); + context(`when ${loggingComponent} is set to a valid value in the environment`, () => { + context(`when MONGODB_LOG_ALL is unset`, () => { + for (const [option, expectedValue] of validNonDefaultOptions) { + context(`{ ${loggingComponent}: ${option} }`, () => { + it(`sets ${componentSeverityOption} to the value of ${loggingComponent}`, () => { + const { componentSeverities } = MongoLogger.resolveOptions( + { + [loggingComponent]: option + }, + {} + ); + + expect(componentSeverities[componentSeverityOption]).to.equal(expectedValue); }); - } - }); - context(`when MONGODB_LOG_ALL is set to an invalid value`, () => { - for (const invalidOption of invalidOptions) { - context( - `{ ${loggingComponent}: ${invalidOption}, MONGODB_LOG_ALL: ${invalidOption} }`, - () => { - it(`sets ${componentSeverityOption} to OFF`, () => { - const { componentSeverities } = MongoLogger.resolveOptions( - { - [loggingComponent]: invalidOption, - MONGODB_LOG_ALL: invalidOption - }, - {} - ); - - expect(componentSeverities[componentSeverityOption]).to.equal( - SeverityLevel.OFF - ); - }); - } - ); - } - }); - context(`when MONGODB_LOG_ALL is set to a valid value`, () => { - const invalidOption = invalidOptions[0]; - - for (const [option, expectedValue] of validNonDefaultOptions) { - context( - `{ MONGODB_LOG_ALL: ${option}, ${componentSeverityOption}: ${option} }`, - () => { - it(`sets ${componentSeverityOption} to the value of MONGODB_LOG_ALL`, () => { - const { componentSeverities } = MongoLogger.resolveOptions( - { - [loggingComponent]: invalidOption, - MONGODB_LOG_ALL: option - }, - {} - ); - expect(componentSeverities[componentSeverityOption]).to.equal(expectedValue); - }); - } - ); - } - it(`sets ${componentSeverityOption} to the value of MONGODB_LOG_ALL`, () => { - expect(true).to.be.true; }); - }); + } }); - context(`when ${loggingComponent} is set to a valid value in the environment`, () => { - context(`when MONGODB_LOG_ALL is unset`, () => { - for (const [option, expectedValue] of validNonDefaultOptions) { - context(`{ ${loggingComponent}: ${option} }`, () => { + + context(`when MONGODB_LOG_ALL is set to an invalid value`, () => { + const invalidValue = invalidOptions[0]; + for (const [option, expectedValue] of validNonDefaultOptions) { + context( + `{ ${loggingComponent}: ${option}, MONGODB_LOG_ALL: ${invalidValue} }`, + () => { it(`sets ${componentSeverityOption} to the value of ${loggingComponent}`, () => { const { componentSeverities } = MongoLogger.resolveOptions( { - [loggingComponent]: option + [loggingComponent]: option, + MONGODB_LOG_ALL: invalidValue }, {} ); expect(componentSeverities[componentSeverityOption]).to.equal(expectedValue); }); + } + ); + } + }); + + context(`when MONGODB_LOG_ALL is set to a valid value`, () => { + const validOption = validNonDefaultOptions.keys()[0]; + for (const [option, expectedValue] of validNonDefaultOptions) { + context(`{ ${loggingComponent}: ${option}, MONGODB_LOG_ALL: ${validOption} }`, () => { + it(`sets ${componentSeverityOption} to the value of ${loggingComponent}`, () => { + const { componentSeverities } = MongoLogger.resolveOptions( + { + [loggingComponent]: option, + MONGODB_LOG_ALL: validOption + }, + {} + ); + + expect(componentSeverities[componentSeverityOption]).to.equal(expectedValue); }); - } - }); - context(`when MONGODB_LOG_ALL is set to an invalid value`, () => { - const invalidValue = invalidOptions[0]; - for (const [option, expectedValue] of validNonDefaultOptions) { - context( - `{ ${loggingComponent}: ${option}, MONGODB_LOG_ALL: ${invalidValue} }`, - () => { - it(`sets ${componentSeverityOption} to the value of ${loggingComponent}`, () => { - const { componentSeverities } = MongoLogger.resolveOptions( - { - [loggingComponent]: option, - MONGODB_LOG_ALL: invalidValue - }, - {} - ); - - expect(componentSeverities[componentSeverityOption]).to.equal(expectedValue); - }); - } - ); - } - }); - context(`when MONGODB_LOG_ALL is set to a valid value`, () => { - const validOption = validNonDefaultOptions.keys()[0]; - for (const [option, expectedValue] of validNonDefaultOptions) { - context( - `{ ${loggingComponent}: ${option}, MONGODB_LOG_ALL: ${validOption} }`, - () => { - it(`sets ${componentSeverityOption} to the value of ${loggingComponent}`, () => { - const { componentSeverities } = MongoLogger.resolveOptions( - { - [loggingComponent]: option, - MONGODB_LOG_ALL: validOption - }, - {} - ); - - expect(componentSeverities[componentSeverityOption]).to.equal(expectedValue); - }); - } - ); - } - }); + }); + } }); - } - }); + }); + } }); context('maxDocumentLength', function () { From 7b07723d3417a5acce20c6402d77b7a2b71dfb27 Mon Sep 17 00:00:00 2001 From: Bailey Pearson Date: Tue, 13 Dec 2022 08:14:05 -0500 Subject: [PATCH 46/50] remove unnecessary block --- test/unit/mongo_logger.test.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/test/unit/mongo_logger.test.ts b/test/unit/mongo_logger.test.ts index 98cbc5d109..e9a28b41a4 100644 --- a/test/unit/mongo_logger.test.ts +++ b/test/unit/mongo_logger.test.ts @@ -212,9 +212,6 @@ describe('class MongoLogger', function () { } ); } - it(`sets ${componentSeverityOption} to the value of MONGODB_LOG_ALL`, () => { - expect(true).to.be.true; - }); }); }); From 5b18ef0e8a05e8f0977ba6bba3f0083f48074869 Mon Sep 17 00:00:00 2001 From: Bailey Pearson Date: Wed, 14 Dec 2022 10:18:52 -0500 Subject: [PATCH 47/50] chore: add todo for writable in mongo_logger --- src/mongo_logger.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/mongo_logger.ts b/src/mongo_logger.ts index 1f2d49a7ff..5932a3fdf2 100644 --- a/src/mongo_logger.ts +++ b/src/mongo_logger.ts @@ -128,6 +128,7 @@ function resolveLogPath( return mongodbLogPath.toLowerCase() === 'stderr' ? process.stderr : process.stdout; } + // TODO(NODE-4886): check for minimal interface instead of instanceof Writable if (typeof mongodbLogPath === 'object' && mongodbLogPath instanceof Writable) { return mongodbLogPath; } From 5262ca0cae4d919a0a697d5851dc4f24447eceb6 Mon Sep 17 00:00:00 2001 From: Bailey Pearson Date: Wed, 14 Dec 2022 16:25:54 -0500 Subject: [PATCH 48/50] refactor uint parsing --- src/connection_string.ts | 35 ++++++++++++++++++++++++++--------- src/mongo_logger.ts | 21 +++------------------ src/utils.ts | 16 +++++++--------- 3 files changed, 36 insertions(+), 36 deletions(-) diff --git a/src/connection_string.ts b/src/connection_string.ts index 7009e8f76f..c7f2c52394 100644 --- a/src/connection_string.ts +++ b/src/connection_string.ts @@ -33,11 +33,12 @@ import { DEFAULT_PK_FACTORY, emitWarning, emitWarningOnce, - getInt, - getUint, + getInt as getIntFromOptions, + getUint as getUIntFromOptions, HostAddress, isRecord, makeClientMetadata, + parseInt, setDifference } from './utils'; import { W, WriteConcern } from './write_concern'; @@ -202,6 +203,22 @@ function getBoolean(name: string, value: unknown): boolean { throw new MongoParseError(`Expected ${name} to be stringified boolean value, got: ${value}`); } +function getIntFromOptions(name: string, value: unknown): number { + const parsedInt = parseInt(value); + if (parsedInt != null) { + return parsedInt; + } + throw new MongoParseError(`Expected ${name} to be stringified int value, got: ${value}`); +} + +function getUIntFromOptions(name: string, value: unknown): number { + const parsedValue = getIntFromOptions(name, value); + if (parsedValue < 0) { + throw new MongoParseError(`${name} can only be a positive int value, got: ${value}`); + } + return parsedValue; +} + function* entriesFromString(value: string): Generator<[string, string]> { const keyValuePairs = value.split(','); for (const keyValue of keyValuePairs) { @@ -573,10 +590,10 @@ function setOption( mongoOptions[name] = getBoolean(name, values[0]); break; case 'int': - mongoOptions[name] = getInt(name, values[0]); + mongoOptions[name] = getIntFromOptions(name, values[0]); break; case 'uint': - mongoOptions[name] = getUint(name, values[0]); + mongoOptions[name] = getUIntFromOptions(name, values[0]); break; case 'string': if (values[0] == null) { @@ -782,7 +799,7 @@ export const OPTIONS = { enableUtf8Validation: { type: 'boolean', default: true }, family: { transform({ name, values: [value] }): 4 | 6 { - const transformValue = getInt(name, value); + const transformValue = getIntFromOptions(name, value); if (transformValue === 4 || transformValue === 6) { return transformValue; } @@ -881,7 +898,7 @@ export const OPTIONS = { maxConnecting: { default: 2, transform({ name, values: [value] }): number { - const maxConnecting = getUint(name, value); + const maxConnecting = getUIntFromOptions(name, value); if (maxConnecting === 0) { throw new MongoInvalidArgumentError('maxConnecting must be > 0 if specified'); } @@ -899,7 +916,7 @@ export const OPTIONS = { maxStalenessSeconds: { target: 'readPreference', transform({ name, options, values: [value] }) { - const maxStalenessSeconds = getUint(name, value); + const maxStalenessSeconds = getUIntFromOptions(name, value); if (options.readPreference) { return ReadPreference.fromOptions({ readPreference: { ...options.readPreference, maxStalenessSeconds } @@ -1218,7 +1235,7 @@ export const OPTIONS = { const wc = WriteConcern.fromOptions({ writeConcern: { ...options.writeConcern, - wtimeout: getUint('wtimeout', value) + wtimeout: getUIntFromOptions('wtimeout', value) } }); if (wc) return wc; @@ -1231,7 +1248,7 @@ export const OPTIONS = { const wc = WriteConcern.fromOptions({ writeConcern: { ...options.writeConcern, - wtimeoutMS: getUint('wtimeoutMS', value) + wtimeoutMS: getUIntFromOptions('wtimeoutMS', value) } }); if (wc) return wc; diff --git a/src/mongo_logger.ts b/src/mongo_logger.ts index 5932a3fdf2..a45ff16847 100644 --- a/src/mongo_logger.ts +++ b/src/mongo_logger.ts @@ -1,6 +1,6 @@ import { Writable } from 'stream'; -import { getUint } from './utils'; +import { getUint, parseUInt } from './utils'; /** @internal */ export const SeverityLevel = Object.freeze({ @@ -93,21 +93,6 @@ function parseSeverityFromString(s?: string): SeverityLevel | null { return null; } -/** - * Parses a string to be a number greater than or equal to 0 for maxDocumentLength. - * - * @param s - the value to be parsed - * @returns the int value parsed or 1000 if the value could not be parsed - */ -function parseMaxDocumentLength(s?: string): number { - try { - const maxDocumentLength = getUint('MONGODB_LOG_MAX_DOCUMENT_LENGTH', s); - return maxDocumentLength; - } catch { - return 1000; - } -} - /** * resolves the MONGODB_LOG_PATH and mongodbLogPath options from the environment and the * mongo client options respectively. @@ -128,7 +113,7 @@ function resolveLogPath( return mongodbLogPath.toLowerCase() === 'stderr' ? process.stderr : process.stdout; } - // TODO(NODE-4886): check for minimal interface instead of instanceof Writable + // TODO(NODE-4813): check for minimal interface instead of instanceof Writable if (typeof mongodbLogPath === 'object' && mongodbLogPath instanceof Writable) { return mongodbLogPath; } @@ -208,7 +193,7 @@ export class MongoLogger { parseSeverityFromString(combinedOptions.MONGODB_LOG_CONNECTION) ?? defaultSeverity, default: defaultSeverity }, - maxDocumentLength: parseMaxDocumentLength(combinedOptions.MONGODB_LOG_MAX_DOCUMENT_LENGTH), + maxDocumentLength: parseUInt(combinedOptions.MONGODB_LOG_MAX_DOCUMENT_LENGTH) ?? 1000, logDestination: combinedOptions.mongodbLogPath }; } diff --git a/src/utils.ts b/src/utils.ts index 468a22fb6c..dd3a6d23cc 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -1422,17 +1422,15 @@ export function compareObjectId(oid1?: ObjectId | null, oid2?: ObjectId | null): return oid1.id.compare(oid2.id); } -export function getInt(name: string, value: unknown): number { +export function parseInt(value: unknown): number | null { if (typeof value === 'number') return Math.trunc(value); const parsedValue = Number.parseInt(String(value), 10); - if (!Number.isNaN(parsedValue)) return parsedValue; - throw new MongoParseError(`Expected ${name} to be stringified int value, got: ${value}`); + + return Number.isNaN(parsedValue) ? null : parsedValue; } -export function getUint(name: string, value: unknown): number { - const parsedValue = getInt(name, value); - if (parsedValue < 0) { - throw new MongoParseError(`${name} can only be a positive int value, got: ${value}`); - } - return parsedValue; +export function parseUInt(value: unknown): number | null { + const parsedInt = parseInt(value); + + return parsedInt != null && parsedInt >= 0 ? parsedInt : null; } From e72a8c1eaacdaddf79f60b24fa020e0c33c818b6 Mon Sep 17 00:00:00 2001 From: Bailey Pearson Date: Wed, 14 Dec 2022 16:56:02 -0500 Subject: [PATCH 49/50] fix imports --- src/connection_string.ts | 2 -- src/mongo_logger.ts | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/src/connection_string.ts b/src/connection_string.ts index c7f2c52394..79cf51955d 100644 --- a/src/connection_string.ts +++ b/src/connection_string.ts @@ -33,8 +33,6 @@ import { DEFAULT_PK_FACTORY, emitWarning, emitWarningOnce, - getInt as getIntFromOptions, - getUint as getUIntFromOptions, HostAddress, isRecord, makeClientMetadata, diff --git a/src/mongo_logger.ts b/src/mongo_logger.ts index a45ff16847..4bbfb9984a 100644 --- a/src/mongo_logger.ts +++ b/src/mongo_logger.ts @@ -1,6 +1,6 @@ import { Writable } from 'stream'; -import { getUint, parseUInt } from './utils'; +import { parseUInt } from './utils'; /** @internal */ export const SeverityLevel = Object.freeze({ From 997a7f2581fb6c687404a61c96f8daed52f17fb4 Mon Sep 17 00:00:00 2001 From: Bailey Pearson Date: Wed, 14 Dec 2022 19:25:50 -0500 Subject: [PATCH 50/50] better naming for parseInt and parseUInt --- src/connection_string.ts | 4 ++-- src/mongo_logger.ts | 5 +++-- src/utils.ts | 6 +++--- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/connection_string.ts b/src/connection_string.ts index 79cf51955d..a1a177433d 100644 --- a/src/connection_string.ts +++ b/src/connection_string.ts @@ -36,7 +36,7 @@ import { HostAddress, isRecord, makeClientMetadata, - parseInt, + parseInteger, setDifference } from './utils'; import { W, WriteConcern } from './write_concern'; @@ -202,7 +202,7 @@ function getBoolean(name: string, value: unknown): boolean { } function getIntFromOptions(name: string, value: unknown): number { - const parsedInt = parseInt(value); + const parsedInt = parseInteger(value); if (parsedInt != null) { return parsedInt; } diff --git a/src/mongo_logger.ts b/src/mongo_logger.ts index 4bbfb9984a..81acd2af70 100644 --- a/src/mongo_logger.ts +++ b/src/mongo_logger.ts @@ -1,6 +1,6 @@ import { Writable } from 'stream'; -import { parseUInt } from './utils'; +import { parseUnsignedInteger } from './utils'; /** @internal */ export const SeverityLevel = Object.freeze({ @@ -193,7 +193,8 @@ export class MongoLogger { parseSeverityFromString(combinedOptions.MONGODB_LOG_CONNECTION) ?? defaultSeverity, default: defaultSeverity }, - maxDocumentLength: parseUInt(combinedOptions.MONGODB_LOG_MAX_DOCUMENT_LENGTH) ?? 1000, + maxDocumentLength: + parseUnsignedInteger(combinedOptions.MONGODB_LOG_MAX_DOCUMENT_LENGTH) ?? 1000, logDestination: combinedOptions.mongodbLogPath }; } diff --git a/src/utils.ts b/src/utils.ts index dd3a6d23cc..3fce528b4a 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -1422,15 +1422,15 @@ export function compareObjectId(oid1?: ObjectId | null, oid2?: ObjectId | null): return oid1.id.compare(oid2.id); } -export function parseInt(value: unknown): number | null { +export function parseInteger(value: unknown): number | null { if (typeof value === 'number') return Math.trunc(value); const parsedValue = Number.parseInt(String(value), 10); return Number.isNaN(parsedValue) ? null : parsedValue; } -export function parseUInt(value: unknown): number | null { - const parsedInt = parseInt(value); +export function parseUnsignedInteger(value: unknown): number | null { + const parsedInt = parseInteger(value); return parsedInt != null && parsedInt >= 0 ? parsedInt : null; }