From 6f07729d2a3ebd1e772904770ac5690230667fce Mon Sep 17 00:00:00 2001 From: gagik Date: Wed, 22 Jan 2025 00:49:09 +0100 Subject: [PATCH 1/4] feat(mongodb-log-writer): add ability to disable and enable logging Adds a new isDisabled field to disable the writing of logs. This can be modified using the new enable() and disable() methods. --- .../src/mongo-log-writer.spec.ts | 98 ++++++++++++++++++- .../src/mongo-log-writer.ts | 49 ++++++++-- 2 files changed, 136 insertions(+), 11 deletions(-) diff --git a/packages/mongodb-log-writer/src/mongo-log-writer.spec.ts b/packages/mongodb-log-writer/src/mongo-log-writer.spec.ts index 7f03f654..69ecd018 100644 --- a/packages/mongodb-log-writer/src/mongo-log-writer.spec.ts +++ b/packages/mongodb-log-writer/src/mongo-log-writer.spec.ts @@ -5,11 +5,107 @@ import stream from 'stream'; import { inspect } from 'util'; import chai, { expect } from 'chai'; import sinonChai from 'sinon-chai'; +import sinon from 'sinon'; chai.use(sinonChai); describe('MongoLogWriter', function () { + const now = new Date(1628591965386); + + describe('enabling and disabling', function () { + let writer: MongoLogWriter; + let target: stream.PassThrough; + let writeSpy: sinon.SinonSpy; + + const SEVERITIES_COUNT = 5; + + function logAllSeverities(writer: MongoLogWriter) { + writer.info('component', mongoLogId(12345), 'context', 'message', {}); + writer.warn('component', mongoLogId(12345), 'context', 'message', {}); + writer.error('component', mongoLogId(12345), 'context', 'message', {}); + writer.debug('component', mongoLogId(12345), 'context', 'message', {}); + writer.fatal('component', mongoLogId(12345), 'context', 'message', {}); + } + + beforeEach(function () { + target = new stream.PassThrough().setEncoding('utf8'); + writer = new MongoLogWriter('logid', null, target, () => now); + writeSpy = sinon.spy(writer, 'write'); + }); + + afterEach(function () { + sinon.restore(); + }); + + it('is enabled by default', async function () { + expect(writer.isDisabled).to.equal(false); + + writer.info('component', mongoLogId(12345), 'context', 'message', {}); + + await writer.flush(); + + expect(target.read()).is.not.null; + expect(writeSpy).callCount(1); + }); + + it('can be disabled on initialization', async function () { + const disabledWriter = new MongoLogWriter( + 'logid', + null, + target, + () => now, + { + isDisabled: true, + } + ); + + expect(disabledWriter.isDisabled).to.equal(true); + logAllSeverities(disabledWriter); + + await disabledWriter.flush(); + + expect(target.read()).is.null; + expect(writeSpy).not.called; + }); + + it('can run disable() to disable logging across all severities', function () { + expect(writer.isDisabled).to.equal(false); + + logAllSeverities(writer); + + expect(writeSpy).callCount(SEVERITIES_COUNT); + + writer.disable(); + + expect(writer.isDisabled).to.equal(true); + + logAllSeverities(writer); + + expect(writeSpy).callCount(SEVERITIES_COUNT); + }); + + it('can run enable() after being disabled', async function () { + expect(writer.isDisabled).to.equal(false); + + writer.disable(); + expect(writer.isDisabled).to.equal(true); + + logAllSeverities(writer); + + expect(writeSpy).not.called; + + writer.enable(); + expect(writer.isDisabled).to.equal(false); + + logAllSeverities(writer); + + await writer.flush(); + + expect(target.read()).not.null; + expect(writeSpy).callCount(SEVERITIES_COUNT); + }); + }); + it('allows writing log messages to a stream', async function () { - const now = new Date(1628591965386); const target = new stream.PassThrough().setEncoding('utf8'); const writer = new MongoLogWriter('logid', null, target, () => now); const logEvents: MongoLogEntry[] = []; diff --git a/packages/mongodb-log-writer/src/mongo-log-writer.ts b/packages/mongodb-log-writer/src/mongo-log-writer.ts index befabacf..7e9e5c0f 100644 --- a/packages/mongodb-log-writer/src/mongo-log-writer.ts +++ b/packages/mongodb-log-writer/src/mongo-log-writer.ts @@ -63,6 +63,10 @@ function validateLogEntry(info: MongoLogEntry): Error | null { return null; } +export type MongoLogWriterOptions = { + isDisabled?: boolean; +}; + /** * A helper class for writing formatted log information to an output stream. * This class itself is an object-mode Writable stream to which @@ -72,10 +76,11 @@ function validateLogEntry(info: MongoLogEntry): Error | null { * the target stream. */ export class MongoLogWriter extends Writable { - _logId: string; - _logFilePath: string | null; - _target: PlainWritable; - _now: () => Date; + private _logId: string; + private _logFilePath: string | null; + private _target: PlainWritable; + private _now: () => Date; + private _isDisabled: boolean; /** * @param logId A unique identifier for this log file. This is not used outside the `logId` getter. @@ -87,13 +92,15 @@ export class MongoLogWriter extends Writable { logId: string, logFilePath: string | null, target: PlainWritable, - now?: () => Date + now?: () => Date, + { isDisabled = false }: MongoLogWriterOptions = {} ) { super({ objectMode: true }); this._logId = logId; this._logFilePath = logFilePath; this._target = target; this._now = now ?? (() => new Date()); + this._isDisabled = isDisabled; } /** Return the logId passed to the constructor. */ @@ -111,6 +118,21 @@ export class MongoLogWriter extends Writable { return this._target; } + /** + * Return whether the logger is currently disabled. + */ + get isDisabled() { + return this._isDisabled; + } + + disable(): void { + this._isDisabled = true; + } + + enable(): void { + this._isDisabled = false; + } + _write( info: MongoLogEntry, encoding: unknown, @@ -186,6 +208,13 @@ export class MongoLogWriter extends Writable { await new Promise((resolve) => this._target.write('', resolve)); } + writeIfEnabled(chunk: Parameters[0]): void { + if (this._isDisabled) { + return; + } + this.write(chunk); + } + /** * Write a log entry with severity 'I'. */ @@ -204,7 +233,7 @@ export class MongoLogWriter extends Writable { msg: message, attr: attr, }; - this.write(logEntry); + this.writeIfEnabled(logEntry); } /** @@ -225,7 +254,7 @@ export class MongoLogWriter extends Writable { msg: message, attr: attr, }; - this.write(logEntry); + this.writeIfEnabled(logEntry); } /** @@ -246,7 +275,7 @@ export class MongoLogWriter extends Writable { msg: message, attr: attr, }; - this.write(logEntry); + this.writeIfEnabled(logEntry); } /** @@ -267,7 +296,7 @@ export class MongoLogWriter extends Writable { msg: message, attr: attr, }; - this.write(logEntry); + this.writeIfEnabled(logEntry); } /** @@ -289,7 +318,7 @@ export class MongoLogWriter extends Writable { msg: message, attr: attr, }; - this.write(logEntry); + this.writeIfEnabled(logEntry); } /** From 447705f05711825c481d34c6b81aa3b856d7d034 Mon Sep 17 00:00:00 2001 From: gagik Date: Wed, 22 Jan 2025 12:33:04 +0100 Subject: [PATCH 2/4] feat!: add log writer options to LogManager and use settings objects instead of parameters This is a breaking change which I believe would be useful for the future to create better consistency between LogManager and LogWriter and scale better as we add more parameters to log writer settings. --- .../src/mongo-log-manager.spec.ts | 13 +++++ .../src/mongo-log-manager.ts | 20 +++++--- .../src/mongo-log-writer.spec.ts | 47 ++++++++++++++----- .../src/mongo-log-writer.ts | 18 ++++--- 4 files changed, 72 insertions(+), 26 deletions(-) diff --git a/packages/mongodb-log-writer/src/mongo-log-manager.spec.ts b/packages/mongodb-log-writer/src/mongo-log-manager.spec.ts index b4652864..94969b9e 100644 --- a/packages/mongodb-log-writer/src/mongo-log-manager.spec.ts +++ b/packages/mongodb-log-writer/src/mongo-log-manager.spec.ts @@ -213,4 +213,17 @@ describe('MongoLogManager', function () { writer.end(); await once(writer, 'finish'); }); + + it('can create a disabled writer', async function () { + const manager = new MongoLogManager({ + directory, + retentionDays, + onwarn, + onerror, + gzip: true, + }); + const logWriter = await manager.createLogWriter({ isDisabled: true }); + + expect(logWriter.isDisabled).is.true; + }); }); diff --git a/packages/mongodb-log-writer/src/mongo-log-manager.ts b/packages/mongodb-log-writer/src/mongo-log-manager.ts index 487c6a2d..cc65be57 100644 --- a/packages/mongodb-log-writer/src/mongo-log-manager.ts +++ b/packages/mongodb-log-writer/src/mongo-log-manager.ts @@ -4,11 +4,12 @@ import { once } from 'events'; import { createWriteStream, promises as fs } from 'fs'; import { createGzip, constants as zlibConstants } from 'zlib'; import { Heap } from 'heap-js'; +import type { MongoLogWriterOptions } from './mongo-log-writer'; import { MongoLogWriter } from './mongo-log-writer'; import { Writable } from 'stream'; /** Options used by MongoLogManager instances. */ -interface MongoLogOptions { +export interface MongoLogManagerOptions { /** A base directory in which log files are stored. */ directory: string; /** Whether to write files as .gz files or not. */ @@ -29,9 +30,9 @@ interface MongoLogOptions { * naming convention `${logId}_log`. */ export class MongoLogManager { - _options: MongoLogOptions; + _options: MongoLogManagerOptions; - constructor(options: MongoLogOptions) { + constructor(options: MongoLogManagerOptions) { this._options = options; } @@ -97,7 +98,9 @@ export class MongoLogManager { } /** Create a MongoLogWriter stream for a new log file. */ - async createLogWriter(): Promise { + async createLogWriter( + writerOptions: Pick = {} + ): Promise { const logId = new ObjectId().toString(); const doGzip = !!this._options.gzip; const logFilePath = path.join( @@ -132,10 +135,15 @@ export class MongoLogManager { }, }); originalTarget = stream; - logWriter = new MongoLogWriter(logId, null, stream); + logWriter = new MongoLogWriter({ + ...writerOptions, + logId, + logFilePath: null, + target: stream, + }); } if (!logWriter) { - logWriter = new MongoLogWriter(logId, logFilePath, stream); + logWriter = new MongoLogWriter({ logId, logFilePath, target: stream }); } // We use 'log-finish' to give consumers an event that they can diff --git a/packages/mongodb-log-writer/src/mongo-log-writer.spec.ts b/packages/mongodb-log-writer/src/mongo-log-writer.spec.ts index 69ecd018..51e0ea6d 100644 --- a/packages/mongodb-log-writer/src/mongo-log-writer.spec.ts +++ b/packages/mongodb-log-writer/src/mongo-log-writer.spec.ts @@ -28,7 +28,12 @@ describe('MongoLogWriter', function () { beforeEach(function () { target = new stream.PassThrough().setEncoding('utf8'); - writer = new MongoLogWriter('logid', null, target, () => now); + writer = new MongoLogWriter({ + logId: 'logid', + logFilePath: null, + target, + now: () => now, + }); writeSpy = sinon.spy(writer, 'write'); }); @@ -48,15 +53,12 @@ describe('MongoLogWriter', function () { }); it('can be disabled on initialization', async function () { - const disabledWriter = new MongoLogWriter( - 'logid', - null, + const disabledWriter = new MongoLogWriter({ + logId: 'logid', + logFilePath: null, target, - () => now, - { - isDisabled: true, - } - ); + now: () => now, + }); expect(disabledWriter.isDisabled).to.equal(true); logAllSeverities(disabledWriter); @@ -107,7 +109,12 @@ describe('MongoLogWriter', function () { it('allows writing log messages to a stream', async function () { const target = new stream.PassThrough().setEncoding('utf8'); - const writer = new MongoLogWriter('logid', null, target, () => now); + const writer = new MongoLogWriter({ + logId: 'logid', + logFilePath: null, + target, + now: () => now, + }); const logEvents: MongoLogEntry[] = []; // eslint-disable-next-line @typescript-eslint/no-unsafe-argument writer.on('log', (entry) => logEvents.push(entry)); @@ -207,7 +214,12 @@ describe('MongoLogWriter', function () { it('can log error object as data as-is', async function () { const now = new Date(1628591965386); const target = new stream.PassThrough().setEncoding('utf8'); - const writer = new MongoLogWriter('logid', null, target, () => now); + const writer = new MongoLogWriter({ + logId: 'logid', + logFilePath: null, + target, + now: () => now, + }); writer.error( 'component', mongoLogId(12345), @@ -233,7 +245,12 @@ describe('MongoLogWriter', function () { it('can log non-trivial data', async function () { const now = new Date(1628591965386); const target = new stream.PassThrough().setEncoding('utf8'); - const writer = new MongoLogWriter('logid', null, target, () => now); + const writer = new MongoLogWriter({ + logId: 'logid', + logFilePath: null, + target, + now: () => now, + }); const cyclic: any = {}; cyclic.cyclic = cyclic; @@ -256,7 +273,11 @@ describe('MongoLogWriter', function () { const errors: Error[] = []; function tryWrite(input: any) { const target = new stream.PassThrough().setEncoding('utf8'); - const writer = new MongoLogWriter('logid', null, target); + const writer = new MongoLogWriter({ + logId: 'logid', + logFilePath: null, + target, + }); writer.on('error', (err) => errors.push(err)); writer.write(input); } diff --git a/packages/mongodb-log-writer/src/mongo-log-writer.ts b/packages/mongodb-log-writer/src/mongo-log-writer.ts index 7e9e5c0f..238452b2 100644 --- a/packages/mongodb-log-writer/src/mongo-log-writer.ts +++ b/packages/mongodb-log-writer/src/mongo-log-writer.ts @@ -65,6 +65,10 @@ function validateLogEntry(info: MongoLogEntry): Error | null { export type MongoLogWriterOptions = { isDisabled?: boolean; + logId: string; + logFilePath: string | null; + target: PlainWritable; + now?: () => Date; }; /** @@ -88,13 +92,13 @@ export class MongoLogWriter extends Writable { * @param target The Writable stream to write data to. * @param now An optional function that overrides computation of the current time. This is used for testing. */ - constructor( - logId: string, - logFilePath: string | null, - target: PlainWritable, - now?: () => Date, - { isDisabled = false }: MongoLogWriterOptions = {} - ) { + constructor({ + logId, + logFilePath, + target, + now, + isDisabled = false, + }: MongoLogWriterOptions) { super({ objectMode: true }); this._logId = logId; this._logFilePath = logFilePath; From adf93689127f8536cc035385d4ae754b13d650bd Mon Sep 17 00:00:00 2001 From: gagik Date: Wed, 22 Jan 2025 12:56:55 +0100 Subject: [PATCH 3/4] chore(ci): define test-ci script --- packages/mongodb-log-writer/package.json | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/mongodb-log-writer/package.json b/packages/mongodb-log-writer/package.json index e8ffed13..1649d9ae 100644 --- a/packages/mongodb-log-writer/package.json +++ b/packages/mongodb-log-writer/package.json @@ -56,6 +56,7 @@ "lint": "eslint src/**/*.ts", "test-only": "nyc mocha --colors -r ts-node/register src/**.spec.ts", "test": "npm run lint && npm run build && npm run test-only", + "test-ci": "npm run test", "build": "npm run compile-ts && gen-esm-wrapper . ./.esm-wrapper.mjs", "prepack": "npm run build", "compile-ts": "tsc -p tsconfig.json" From 5f383d001889aab1c94b409b0ddf42263011603b Mon Sep 17 00:00:00 2001 From: gagik Date: Wed, 22 Jan 2025 15:14:59 +0100 Subject: [PATCH 4/4] test: fix log writer disabling test and cleanup after creating writer --- packages/devtools-connect/src/log-hook.spec.ts | 16 ++++++++-------- .../src/mongo-log-manager.spec.ts | 7 +++++-- .../mongodb-log-writer/src/mongo-log-manager.ts | 7 ++++++- .../src/mongo-log-writer.spec.ts | 6 +++++- 4 files changed, 24 insertions(+), 12 deletions(-) diff --git a/packages/devtools-connect/src/log-hook.spec.ts b/packages/devtools-connect/src/log-hook.spec.ts index 2561573f..ae75a4d0 100644 --- a/packages/devtools-connect/src/log-hook.spec.ts +++ b/packages/devtools-connect/src/log-hook.spec.ts @@ -1,5 +1,5 @@ -import { hookLogger } from '../'; -import type { ConnectLogEmitter } from '../'; +import { hookLogger } from './'; +import type { ConnectLogEmitter } from './'; import { EventEmitter } from 'events'; import { MongoLogWriter } from 'mongodb-log-writer'; import { redactConnectionString } from 'mongodb-connection-string-url'; @@ -9,12 +9,12 @@ import { expect } from 'chai'; describe('Logging setup', function () { it('logs events', async function () { const pt = new PassThrough(); - const log = new MongoLogWriter( - 'logid', - null, - pt, - () => new Date('2021-12-16T14:35:08.763Z') - ); + const log = new MongoLogWriter({ + logId: 'logid', + logFilePath: null, + target: pt, + now: () => new Date('2021-12-16T14:35:08.763Z'), + }); const emitter: ConnectLogEmitter = new EventEmitter(); hookLogger(emitter, log, 'prefix', redactConnectionString); diff --git a/packages/mongodb-log-writer/src/mongo-log-manager.spec.ts b/packages/mongodb-log-writer/src/mongo-log-manager.spec.ts index 94969b9e..8f706a69 100644 --- a/packages/mongodb-log-writer/src/mongo-log-manager.spec.ts +++ b/packages/mongodb-log-writer/src/mongo-log-manager.spec.ts @@ -222,8 +222,11 @@ describe('MongoLogManager', function () { onerror, gzip: true, }); - const logWriter = await manager.createLogWriter({ isDisabled: true }); + const writer = await manager.createLogWriter({ isDisabled: true }); - expect(logWriter.isDisabled).is.true; + expect(writer.isDisabled).is.true; + + writer.end(); + await once(writer, 'finish'); }); }); diff --git a/packages/mongodb-log-writer/src/mongo-log-manager.ts b/packages/mongodb-log-writer/src/mongo-log-manager.ts index cc65be57..6caa9538 100644 --- a/packages/mongodb-log-writer/src/mongo-log-manager.ts +++ b/packages/mongodb-log-writer/src/mongo-log-manager.ts @@ -143,7 +143,12 @@ export class MongoLogManager { }); } if (!logWriter) { - logWriter = new MongoLogWriter({ logId, logFilePath, target: stream }); + logWriter = new MongoLogWriter({ + ...writerOptions, + logId, + logFilePath, + target: stream, + }); } // We use 'log-finish' to give consumers an event that they can diff --git a/packages/mongodb-log-writer/src/mongo-log-writer.spec.ts b/packages/mongodb-log-writer/src/mongo-log-writer.spec.ts index 51e0ea6d..56ccaa03 100644 --- a/packages/mongodb-log-writer/src/mongo-log-writer.spec.ts +++ b/packages/mongodb-log-writer/src/mongo-log-writer.spec.ts @@ -2,6 +2,7 @@ import type { MongoLogEntry } from '.'; import { MongoLogWriter, mongoLogId } from '.'; import { EJSON } from 'bson'; import stream from 'stream'; +import { once } from 'events'; import { inspect } from 'util'; import chai, { expect } from 'chai'; import sinonChai from 'sinon-chai'; @@ -37,7 +38,9 @@ describe('MongoLogWriter', function () { writeSpy = sinon.spy(writer, 'write'); }); - afterEach(function () { + afterEach(async function () { + writer.end(); + await once(writer, 'finish'); sinon.restore(); }); @@ -58,6 +61,7 @@ describe('MongoLogWriter', function () { logFilePath: null, target, now: () => now, + isDisabled: true, }); expect(disabledWriter.isDisabled).to.equal(true);