From 2eb3d4ac69e7060f23754d73714428c8f82ed8bf Mon Sep 17 00:00:00 2001 From: gagik Date: Thu, 6 Feb 2025 15:45:23 +0100 Subject: [PATCH 1/2] feat(mongodb-log-writer): add ability to specify a prefix for log files This is needed for mongosh to use a `mongosh_` prefix for custom log directories. --- .../src/mongo-log-manager.spec.ts | 67 ++++++++++++++++++- .../src/mongo-log-manager.ts | 19 ++++-- 2 files changed, 79 insertions(+), 7 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 eb63b50b..c1251557 100644 --- a/packages/mongodb-log-writer/src/mongo-log-manager.spec.ts +++ b/packages/mongodb-log-writer/src/mongo-log-manager.spec.ts @@ -61,6 +61,19 @@ describe('MongoLogManager', function () { expect(log[0].t.$date).to.be.a('string'); }); + it('can take a custom prefix for log files', async function () { + const manager = new MongoLogManager({ + directory, + retentionDays, + prefix: 'custom_', + onwarn, + onerror, + }); + + const writer = await manager.createLogWriter(); + expect(writer.logFilePath as string).to.match(/custom_/); + }); + it('cleans up old log files when requested', async function () { retentionDays = 0.000001; // 86.4 ms const manager = new MongoLogManager({ @@ -131,8 +144,8 @@ describe('MongoLogManager', function () { directory, retentionDays, retentionGB: 3, - onwarn, onerror, + onwarn, }); const offset = Math.floor(Date.now() / 1000); @@ -178,7 +191,57 @@ describe('MongoLogManager', function () { expect(leftoverFiles).deep.equals([faultyFile, ...validFiles.slice(3)]); }); - it('cleans up least recent log files when over a storage limit', async function () { + it('cleanup only applies to files with the prefix, if set', async function () { + const manager = new MongoLogManager({ + directory, + retentionDays, + maxLogFileCount: 7, + prefix: 'custom_', + onwarn, + onerror, + }); + + const paths: string[] = []; + const offset = Math.floor(Date.now() / 1000); + + // Create 4 files: 2 with a different prefix and 2 with no prefix + for (let i = 1; i >= 0; i--) { + const withoutPrefix = path.join( + directory, + ObjectId.createFromTime(offset - i).toHexString() + '_log' + ); + await fs.writeFile(withoutPrefix, ''); + paths.push(withoutPrefix); + + const withDifferentPrefix = path.join( + directory, + 'different_' + + ObjectId.createFromTime(offset - i).toHexString() + + '_log' + ); + await fs.writeFile(withDifferentPrefix, ''); + paths.push(withDifferentPrefix); + } + + // Create 10 files with the prefix + for (let i = 9; i >= 0; i--) { + const filename = path.join( + directory, + `custom_${ObjectId.createFromTime(offset - i).toHexString()}_log` + ); + await fs.writeFile(filename, ''); + paths.push(filename); + } + + expect(await getFilesState(paths)).to.equal('11111111111111'); + await manager.cleanupOldLogFiles(); + + // The first 4 files without the right prefix should still be there. + // The next (oldest) 3 files with the prefix should be deleted. + expect(await getFilesState(paths)).to.equal('11110001111111'); + }); + + it('cleans up least recent log files when requested with a storage limit', async function () { const manager = new MongoLogManager({ directory, retentionDays, diff --git a/packages/mongodb-log-writer/src/mongo-log-manager.ts b/packages/mongodb-log-writer/src/mongo-log-manager.ts index 180cf517..2626dc28 100644 --- a/packages/mongodb-log-writer/src/mongo-log-manager.ts +++ b/packages/mongodb-log-writer/src/mongo-log-manager.ts @@ -19,10 +19,12 @@ interface MongoLogOptions { maxLogFileCount?: number; /** The maximal size of log files which are kept. */ retentionGB?: number; + /** Prefix to use for the log files */ + prefix?: string; + /** A handler for warnings related to a specific filesystem path. */ + onerror: (err: Error, path: string) => unknown | Promise; /** A handler for errors related to a specific filesystem path. */ onerror: (err: Error, path: string) => unknown | Promise; - /** A handler for warnings related to a specific filesystem path. */ - onwarn: (err: Error, path: string) => unknown | Promise; } /** @@ -48,6 +50,10 @@ export class MongoLogManager { } } + private get prefix() { + return this._options.prefix ?? ''; + } + /** Clean up log files older than `retentionDays`. */ async cleanupOldLogFiles(maxDurationMs = 5_000): Promise { const dir = this._options.directory; @@ -80,8 +86,11 @@ export class MongoLogManager { if (Date.now() - deletionStartTimestamp > maxDurationMs) break; if (!dirent.isFile()) continue; - const { id } = - /^(?[a-f0-9]{24})_log(\.gz)?$/i.exec(dirent.name)?.groups ?? {}; + const logRegExp = new RegExp( + `^${this.prefix}(?[a-f0-9]{24})_log(\\.gz)?$`, + 'i' + ); + const { id } = logRegExp.exec(dirent.name)?.groups ?? {}; if (!id) continue; const fileTimestamp = +new ObjectId(id).getTimestamp(); @@ -143,7 +152,7 @@ export class MongoLogManager { const doGzip = !!this._options.gzip; const logFilePath = path.join( this._options.directory, - `${logId}_log${doGzip ? '.gz' : ''}` + `${this.prefix}${logId}_log${doGzip ? '.gz' : ''}` ); let originalTarget: Writable; From d65386bb1315d532c3842c2195bfa637d807bd26 Mon Sep 17 00:00:00 2001 From: gagik Date: Mon, 10 Feb 2025 16:14:17 +0100 Subject: [PATCH 2/2] feat: add validation of the prefix --- .../src/mongo-log-manager.spec.ts | 44 ++++++++++++++++++- .../src/mongo-log-manager.ts | 11 ++++- 2 files changed, 52 insertions(+), 3 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 c1251557..4772c36f 100644 --- a/packages/mongodb-log-writer/src/mongo-log-manager.spec.ts +++ b/packages/mongodb-log-writer/src/mongo-log-manager.spec.ts @@ -31,6 +31,48 @@ describe('MongoLogManager', function () { sinon.restore(); }); + it('constructor throws with invalid prefixes', function () { + expect(() => { + new MongoLogManager({ + directory, + retentionDays, + prefix: '%asdabs/', + onwarn, + onerror, + }); + }).to.throw(); + + expect(() => { + new MongoLogManager({ + directory, + retentionDays, + prefix: '$$$$', + onwarn, + onerror, + }); + }).to.throw(); + + expect(() => { + new MongoLogManager({ + directory, + retentionDays, + prefix: 'abc_', + onwarn, + onerror, + }); + }).not.to.throw(); + + expect(() => { + new MongoLogManager({ + directory, + retentionDays, + prefix: 'something', + onwarn, + onerror, + }); + }).not.to.throw(); + }); + it('allows creating and writing to log files', async function () { const manager = new MongoLogManager({ directory, @@ -144,8 +186,8 @@ describe('MongoLogManager', function () { directory, retentionDays, retentionGB: 3, - onerror, onwarn, + onerror, }); const offset = Math.floor(Date.now() / 1000); diff --git a/packages/mongodb-log-writer/src/mongo-log-manager.ts b/packages/mongodb-log-writer/src/mongo-log-manager.ts index 2626dc28..01d07246 100644 --- a/packages/mongodb-log-writer/src/mongo-log-manager.ts +++ b/packages/mongodb-log-writer/src/mongo-log-manager.ts @@ -21,10 +21,10 @@ interface MongoLogOptions { retentionGB?: number; /** Prefix to use for the log files */ prefix?: string; - /** A handler for warnings related to a specific filesystem path. */ - onerror: (err: Error, path: string) => unknown | Promise; /** A handler for errors related to a specific filesystem path. */ onerror: (err: Error, path: string) => unknown | Promise; + /** A handler for warnings related to a specific filesystem path. */ + onwarn: (err: Error, path: string) => unknown | Promise; } /** @@ -36,6 +36,13 @@ export class MongoLogManager { _options: MongoLogOptions; constructor(options: MongoLogOptions) { + if (options.prefix) { + if (!/^[a-z0-9_]+$/i.test(options.prefix)) { + throw new Error( + 'Prefix must only contain letters, numbers, and underscores' + ); + } + } this._options = options; }