diff --git a/package-lock.json b/package-lock.json index d2800bb01d..0df8f6fa5b 100644 --- a/package-lock.json +++ b/package-lock.json @@ -21445,6 +21445,7 @@ "resolved": "https://registry.npmjs.org/mongodb-log-writer/-/mongodb-log-writer-2.1.0.tgz", "integrity": "sha512-ZlNts/L9fs6gQNRuqLcB0+yjjfeyapbxjdkDpeb2bEYOUUThG0iOEhzIFejv0g3TX1SSAsdrT2aGYnFqoQILgQ==", "license": "Apache-2.0", + "peer": true, "dependencies": { "heap-js": "^2.3.0" }, @@ -29304,7 +29305,7 @@ "is-recoverable-error": "^1.0.3", "js-yaml": "^4.1.0", "mongodb-connection-string-url": "^3.0.1", - "mongodb-log-writer": "^2.1.0", + "mongodb-log-writer": "^2.3.0", "numeral": "^2.0.6", "pretty-repl": "^4.0.1", "semver": "^7.5.4", @@ -29360,6 +29361,18 @@ "js-yaml": "bin/js-yaml.js" } }, + "packages/cli-repl/node_modules/mongodb-log-writer": { + "version": "2.3.0", + "resolved": "https://registry.npmjs.org/mongodb-log-writer/-/mongodb-log-writer-2.3.0.tgz", + "integrity": "sha512-EfraRB3vfzxRUSPKlrO2g5kSdEy8wwD49jxX3FP4Yye1R88aQZAQ3g+0VACyJvJ4cWU1yqZcFpbvRaLQK4GC3g==", + "license": "Apache-2.0", + "dependencies": { + "heap-js": "^2.3.0" + }, + "peerDependencies": { + "bson": "6.x" + } + }, "packages/connectivity-tests": { "name": "@mongosh/connectivity-tests", "version": "2.4.0", @@ -29393,7 +29406,7 @@ "lodash": "^4.17.21", "moment": "^2.29.1", "mongodb": "^6.12.0", - "mongodb-log-writer": "^2.1.0", + "mongodb-log-writer": "^2.3.0", "node-fetch": "^3.3.2", "prettier": "^2.8.8", "rimraf": "^3.0.2" @@ -29440,6 +29453,19 @@ "node": ">= 12" } }, + "packages/e2e-tests/node_modules/mongodb-log-writer": { + "version": "2.3.0", + "resolved": "https://registry.npmjs.org/mongodb-log-writer/-/mongodb-log-writer-2.3.0.tgz", + "integrity": "sha512-EfraRB3vfzxRUSPKlrO2g5kSdEy8wwD49jxX3FP4Yye1R88aQZAQ3g+0VACyJvJ4cWU1yqZcFpbvRaLQK4GC3g==", + "dev": true, + "license": "Apache-2.0", + "dependencies": { + "heap-js": "^2.3.0" + }, + "peerDependencies": { + "bson": "6.x" + } + }, "packages/e2e-tests/node_modules/node-fetch": { "version": "3.3.2", "resolved": "https://registry.npmjs.org/node-fetch/-/node-fetch-3.3.2.tgz", @@ -29671,7 +29697,7 @@ "@mongosh/errors": "2.4.0", "@mongosh/history": "2.4.2", "@mongosh/types": "3.2.0", - "mongodb-log-writer": "^2.1.0", + "mongodb-log-writer": "^2.3.0", "mongodb-redact": "^1.1.5" }, "devDependencies": { @@ -29686,6 +29712,18 @@ "node": ">=14.15.1" } }, + "packages/logging/node_modules/mongodb-log-writer": { + "version": "2.3.0", + "resolved": "https://registry.npmjs.org/mongodb-log-writer/-/mongodb-log-writer-2.3.0.tgz", + "integrity": "sha512-EfraRB3vfzxRUSPKlrO2g5kSdEy8wwD49jxX3FP4Yye1R88aQZAQ3g+0VACyJvJ4cWU1yqZcFpbvRaLQK4GC3g==", + "license": "Apache-2.0", + "dependencies": { + "heap-js": "^2.3.0" + }, + "peerDependencies": { + "bson": "6.x" + } + }, "packages/logging/node_modules/mongodb-redact": { "version": "1.1.5", "resolved": "https://registry.npmjs.org/mongodb-redact/-/mongodb-redact-1.1.5.tgz", diff --git a/packages/cli-repl/package.json b/packages/cli-repl/package.json index 4c44aed24d..9e2501cba3 100644 --- a/packages/cli-repl/package.json +++ b/packages/cli-repl/package.json @@ -85,7 +85,7 @@ "is-recoverable-error": "^1.0.3", "js-yaml": "^4.1.0", "mongodb-connection-string-url": "^3.0.1", - "mongodb-log-writer": "^2.1.0", + "mongodb-log-writer": "^2.3.0", "numeral": "^2.0.6", "pretty-repl": "^4.0.1", "semver": "^7.5.4", diff --git a/packages/cli-repl/src/cli-repl.spec.ts b/packages/cli-repl/src/cli-repl.spec.ts index 35ac806535..99014f6f67 100644 --- a/packages/cli-repl/src/cli-repl.spec.ts +++ b/packages/cli-repl/src/cli-repl.spec.ts @@ -326,6 +326,7 @@ describe('CliRepl', function () { 'logRetentionDays', 'logMaxFileCount', 'logCompressionEnabled', + 'logRetentionGB', ] satisfies (keyof CliUserConfig)[]); }); @@ -1460,6 +1461,19 @@ describe('CliRepl', function () { ); }); + it('can set log retention GB', async function () { + const testLogRetentionGB = 10; + cliRepl.config.logRetentionGB = testLogRetentionGB; + await cliRepl.start(await testServer.connectionString(), {}); + + expect(await cliRepl.getConfig('logRetentionGB')).equals( + testLogRetentionGB + ); + expect(cliRepl.logManager?._options.retentionGB).equals( + testLogRetentionGB + ); + }); + it('can set log max file count', async function () { const testMaxFileCount = 123; cliRepl.config.logMaxFileCount = testMaxFileCount; diff --git a/packages/cli-repl/src/cli-repl.ts b/packages/cli-repl/src/cli-repl.ts index d040b189c8..67179d1d58 100644 --- a/packages/cli-repl/src/cli-repl.ts +++ b/packages/cli-repl/src/cli-repl.ts @@ -262,6 +262,7 @@ export class CliRepl implements MongoshIOProvider { retentionDays: await this.getConfig('logRetentionDays'), gzip: await this.getConfig('logCompressionEnabled'), maxLogFileCount: await this.getConfig('logMaxFileCount'), + retentionGB: await this.getConfig('logRetentionGB'), onerror: (err: Error) => this.bus.emit('mongosh:error', err, 'log'), onwarn: (err: Error, path: string) => this.warnAboutInaccessibleFile(err, path), diff --git a/packages/e2e-tests/package.json b/packages/e2e-tests/package.json index 35ae98d88c..851668fe88 100644 --- a/packages/e2e-tests/package.json +++ b/packages/e2e-tests/package.json @@ -33,7 +33,7 @@ "strip-ansi": "^6.0.0" }, "devDependencies": { - "mongodb-log-writer": "^2.1.0", + "mongodb-log-writer": "^2.3.0", "@mongodb-js/eslint-config-mongosh": "^1.0.0", "@mongodb-js/oidc-mock-provider": "^0.10.2", "@mongodb-js/prettier-config-devtools": "^1.0.1", diff --git a/packages/e2e-tests/test/e2e.spec.ts b/packages/e2e-tests/test/e2e.spec.ts index 9316dfa166..1ae15b1c30 100644 --- a/packages/e2e-tests/test/e2e.spec.ts +++ b/packages/e2e-tests/test/e2e.spec.ts @@ -1581,7 +1581,7 @@ describe('e2e', function () { await shell.waitForPrompt(); shell.assertContainsOutput('Ignoring config option "logLocation"'); shell.assertContainsOutput( - 'must be a valid absolute path or empty' + 'must be a valid absolute path or undefined' ); expect( @@ -1589,7 +1589,7 @@ describe('e2e', function () { 'config.set("logLocation", "[123123123123]")' ) ).contains( - 'Cannot set option "logLocation": logLocation must be a valid absolute path or empty' + 'Cannot set option "logLocation": logLocation must be a valid absolute path or undefined' ); }); @@ -1722,10 +1722,10 @@ describe('e2e', function () { }); }); - describe('with custom log retention days', function () { + describe('with logRetentionDays', function () { const customLogDir = useTmpdir(); - it('should delete older files according to the setting', async function () { + it('should delete older files older than logRetentionDays', async function () { const paths: string[] = []; const today = Math.floor(Date.now() / 1000); const tenDaysAgo = today - 10 * 24 * 60 * 60; @@ -1776,10 +1776,10 @@ describe('e2e', function () { }); }); - describe('with custom log retention max file count', function () { + describe('with logMaxFileCount', function () { const customLogDir = useTmpdir(); - it('should delete files once it is above the max file count limit', async function () { + it('should delete files once it is above logMaxFileCount', async function () { const globalConfig = path.join(homedir, 'globalconfig.conf'); await fs.writeFile( globalConfig, @@ -1825,6 +1825,57 @@ describe('e2e', function () { }); }); + describe('with logRetentionGB', function () { + const customLogDir = useTmpdir(); + + it('should delete files once it is above logRetentionGB', async function () { + const globalConfig = path.join(homedir, 'globalconfig.conf'); + await fs.writeFile( + globalConfig, + // Set logRetentionGB to 4 MB and we will create prior 10 log files, 1 MB each + `mongosh:\n logLocation: ${JSON.stringify( + customLogDir.path + )}\n logRetentionGB: ${4 / 1024}` + ); + const paths: string[] = []; + const offset = Math.floor(Date.now() / 1000); + + // Create 10 log files, around 1 mb each + for (let i = 9; i >= 0; i--) { + const filename = path.join( + customLogDir.path, + ObjectId.createFromTime(offset - i).toHexString() + '_log' + ); + await fs.writeFile(filename, '0'.repeat(1024 * 1024)); + paths.push(filename); + } + + // All 10 existing log files exist. + expect(await getFilesState(paths)).to.equal('1111111111'); + shell = this.startTestShell({ + args: ['--nodb'], + env, + globalConfigPath: globalConfig, + forceTerminal: true, + }); + + await shell.waitForPrompt(); + + // Add the newly created log to the file list. + paths.push( + path.join(customLogDir.path, `${shell.logId as string}_log`) + ); + + expect( + await shell.executeLine('config.get("logRetentionGB")') + ).contains(`${4 / 1024}`); + + // Expect 6 files to be deleted and 4 to remain + // (including the new log file which should be <1 MB) + expect(await getFilesState(paths)).to.equal('00000001111'); + }); + }); + it('creates a log file that keeps track of session events', async function () { expect(await shell.executeLine('print(123 + 456)')).to.include('579'); const log = await readLogFile(); diff --git a/packages/logging/package.json b/packages/logging/package.json index 87f1a21115..f845c5f368 100644 --- a/packages/logging/package.json +++ b/packages/logging/package.json @@ -21,7 +21,7 @@ "@mongosh/errors": "2.4.0", "@mongosh/history": "2.4.2", "@mongosh/types": "3.2.0", - "mongodb-log-writer": "^2.1.0", + "mongodb-log-writer": "^2.3.0", "mongodb-redact": "^1.1.5" }, "devDependencies": { diff --git a/packages/types/src/index.spec.ts b/packages/types/src/index.spec.ts index cb4d5ad57f..4cd08156a4 100644 --- a/packages/types/src/index.spec.ts +++ b/packages/types/src/index.spec.ts @@ -31,6 +31,14 @@ describe('config validation', function () { expect(await validate('logRetentionDays', -1)).to.equal( 'logRetentionDays must be a positive integer' ); + expect(await validate('logRetentionGB', 'foo')).to.equal( + 'logRetentionGB must be a positive number or undefined' + ); + expect(await validate('logRetentionGB', -1)).to.equal( + 'logRetentionGB must be a positive number or undefined' + ); + expect(await validate('logRetentionGB', undefined)).to.equal(null); + expect(await validate('logRetentionGB', 100)).to.equal(null); expect(await validate('logMaxFileCount', 'foo')).to.equal( 'logMaxFileCount must be a positive integer' ); diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index e6ad502efb..361061acfd 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -515,6 +515,7 @@ export class CliUserConfig extends SnippetShellUserConfig { logRetentionDays = 30; logMaxFileCount = 100; logCompressionEnabled = false; + logRetentionGB: number | undefined = undefined; } export class CliUserConfigValidator extends SnippetShellUserConfigValidator { @@ -543,6 +544,11 @@ export class CliUserConfigValidator extends SnippetShellUserConfigValidator { return `${key} must be a positive integer`; } return null; + case 'logRetentionGB': + if (value !== undefined && (typeof value !== 'number' || value < 0)) { + return `${key} must be a positive number or undefined`; + } + return null; case 'disableLogging': case 'forceDisableTelemetry': case 'showStackTraces': @@ -595,7 +601,7 @@ export class CliUserConfigValidator extends SnippetShellUserConfigValidator { value !== undefined && (typeof value !== 'string' || !path.isAbsolute(value)) ) { - return `${key} must be a valid absolute path or empty`; + return `${key} must be a valid absolute path or undefined`; } return null; default: