Skip to content

feat(cli-repl): add configuration to enable log compression MONGOSH-1986 #2362

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
gagik merged 1 commit into from
Feb 10, 2025
Merged

feat(cli-repl): add configuration to enable log compression MONGOSH-1986 #2362

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions packages/cli-repl/src/cli-repl.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,7 @@ describe('CliRepl', function () {
'logLocation',
'logRetentionDays',
'logMaxFileCount',
'logCompressionEnabled',
Copy link
Copy Markdown
Contributor Author

@gagik gagik Feb 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could naming this enableLogCompression be more consistent with disableGreetingMessage, disableLogging, enableTelemetry?

Copy link
Copy Markdown
Collaborator

@addaleax addaleax Feb 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We've had this conversation in the tech design and decided that we'd go for disableXYZ/enableXYZ only for the top-level features, but use the xyzDetailedOption for the detailed configuration of each individual feature

] satisfies (keyof CliUserConfig)[]);
});

Expand Down Expand Up @@ -1477,6 +1478,16 @@ describe('CliRepl', function () {
process.env.MONGOSH_TEST_ONLY_MAX_LOG_FILE_COUNT =
oldEnvironmentLimit;
});

it('can set log compression', async function () {
cliRepl.config.logCompressionEnabled = true;
await cliRepl.start(await testServer.connectionString(), {});

expect(await cliRepl.getConfig('logCompressionEnabled')).equals(
true
);
expect(cliRepl.logManager?._options.gzip).equals(true);
});
});

it('times out fast', async function () {
Expand Down
1 change: 1 addition & 0 deletions packages/cli-repl/src/cli-repl.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,7 @@ export class CliRepl implements MongoshIOProvider {
(await this.getConfig('logLocation')) ||
this.shellHomeDirectory.localPath('.'),
retentionDays: await this.getConfig('logRetentionDays'),
gzip: await this.getConfig('logCompressionEnabled'),
maxLogFileCount: +(
process.env.MONGOSH_TEST_ONLY_MAX_LOG_FILE_COUNT ||
(await this.getConfig('logMaxFileCount'))
Expand Down
43 changes: 43 additions & 0 deletions packages/e2e-tests/test/e2e.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1691,6 +1691,49 @@ describe('e2e', function () {
).join('');
};

describe('with custom log compression', function () {
const customLogDir = useTmpdir();

it('should created compressed files when enabled', async function () {
const globalConfig = path.join(homedir, 'globalconfig.conf');
await fs.writeFile(
globalConfig,
`mongosh:\n logLocation: ${JSON.stringify(
customLogDir.path
)}\n logCompressionEnabled: true`
);

shell = this.startTestShell({
args: ['--nodb'],
env: {
...env,
MONGOSH_GLOBAL_CONFIG_FILE_FOR_TESTING: globalConfig,
},
forceTerminal: true,
});

await shell.waitForPrompt();

const logFile = path.join(
customLogDir.path,
`${shell.logId as string}_log`
);
const logFileGzip = path.join(
customLogDir.path,
`${shell.logId as string}_log.gz`
);

// Only the gzipped file should exist
expect(await getFilesState([logFile, logFileGzip])).equals('01');

const logContent = await fs.readFile(logFileGzip);

// gzipped files start with 0x1f 0x8b
expect(logContent[0]).equals(0x1f);
expect(logContent[1]).equals(0x8b);
});
});

describe('with custom log retention days', function () {
const customLogDir = useTmpdir();

Expand Down
7 changes: 7 additions & 0 deletions packages/types/src/index.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,13 @@ describe('config validation', function () {
expect(await validate('logMaxFileCount', -1)).to.equal(
'logMaxFileCount must be a positive integer'
);
expect(await validate('logCompressionEnabled', 'foo')).to.equal(
'logCompressionEnabled must be a boolean'
);
expect(await validate('logCompressionEnabled', -1)).to.equal(
'logCompressionEnabled must be a boolean'
);
expect(await validate('logCompressionEnabled', false)).to.equal(null);
expect(await validate('showStackTraces', 'foo')).to.equal(
'showStackTraces must be a boolean'
);
Expand Down
2 changes: 2 additions & 0 deletions packages/types/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -511,6 +511,7 @@ export class CliUserConfig extends SnippetShellUserConfig {
logLocation: string | undefined = undefined;
logRetentionDays = 30;
logMaxFileCount = 100;
logCompressionEnabled = false;
}

export class CliUserConfigValidator extends SnippetShellUserConfigValidator {
Expand Down Expand Up @@ -542,6 +543,7 @@ export class CliUserConfigValidator extends SnippetShellUserConfigValidator {
case 'disableLogging':
case 'forceDisableTelemetry':
case 'showStackTraces':
case 'logCompressionEnabled':
if (typeof value !== 'boolean') {
return `${key} must be a boolean`;
}
Expand Down