Skip to content
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.

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

feat(cli-repl): add configuration to set log location MONGOSH-1983 #2326

Merged
merged 7 commits into from
Feb 7, 2025
Merged
17 changes: 17 additions & 0 deletions packages/cli-repl/src/cli-repl.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,7 @@ describe('CliRepl', function () {
'browser',
'updateURL',
'disableLogging',
'logLocation',
] satisfies (keyof CliUserConfig)[]);
});

Expand Down Expand Up @@ -1426,6 +1427,22 @@ describe('CliRepl', function () {
)
).to.have.lengthOf(1);
});

const customLogLocation = useTmpdir();
it('can set the log location', async function () {
cliRepl.config.logLocation = customLogLocation.path;
await cliRepl.start(await testServer.connectionString(), {});

expect(await cliRepl.getConfig('logLocation')).equals(
customLogLocation.path
);
expect(cliRepl.logWriter?.logFilePath).equals(
path.join(
customLogLocation.path,
(cliRepl.logWriter?.logId as string) + '_log'
)
);
});
});

it('times out fast', async function () {
Expand Down
4 changes: 3 additions & 1 deletion packages/cli-repl/src/cli-repl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,9 @@ export class CliRepl implements MongoshIOProvider {
}

this.logManager ??= new MongoLogManager({
directory: this.shellHomeDirectory.localPath('.'),
directory:
(await this.getConfig('logLocation')) ||
this.shellHomeDirectory.localPath('.'),
retentionDays: 30,
maxLogFileCount: +(
process.env.MONGOSH_TEST_ONLY_MAX_LOG_FILE_COUNT || 100
Expand Down
127 changes: 123 additions & 4 deletions packages/e2e-tests/test/e2e.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,11 @@ import { promisify } from 'util';
import path from 'path';
import os from 'os';
import type { MongoLogEntryFromFile } from './repl-helpers';
import { readReplLogFile, setTemporaryHomeDirectory } from './repl-helpers';
import {
readReplLogFile,
setTemporaryHomeDirectory,
useTmpdir,
} from './repl-helpers';
import { bson } from '@mongosh/service-provider-core';
import type { Server as HTTPServer } from 'http';
import { createServer as createHTTPServer } from 'http';
Expand Down Expand Up @@ -1356,7 +1360,9 @@ describe('e2e', function () {
let logBasePath: string;
let historyPath: string;
let readConfig: () => Promise<any>;
let readLogFile: <T extends MongoLogEntryFromFile>() => Promise<T[]>;
let readLogFile: <T extends MongoLogEntryFromFile>(
customBasePath?: string
) => Promise<T[]>;
let startTestShell: (...extraArgs: string[]) => Promise<TestShell>;

beforeEach(function () {
Expand Down Expand Up @@ -1393,11 +1399,16 @@ describe('e2e', function () {
}
readConfig = async () =>
EJSON.parse(await fs.readFile(configPath, 'utf8'));
readLogFile = async <T extends MongoLogEntryFromFile>(): Promise<T[]> => {
readLogFile = async <T extends MongoLogEntryFromFile>(
customBasePath?: string
): Promise<T[]> => {
if (!shell.logId) {
throw new Error('Shell does not have a logId associated with it');
}
const logPath = path.join(logBasePath, `${shell.logId}_log`);
const logPath = path.join(
customBasePath ?? logBasePath,
`${shell.logId}_log`
);
return readReplLogFile<T>(logPath);
};
startTestShell = async (...extraArgs: string[]) => {
Expand Down Expand Up @@ -1557,6 +1568,114 @@ describe('e2e', function () {
).to.have.lengthOf(1);
});

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

it('fails with relative or invalid paths', async function () {
const globalConfig = path.join(homedir, 'globalconfig.conf');
await fs.writeFile(
globalConfig,
`mongosh:\n logLocation: "./some-relative-path"`
);

shell = this.startTestShell({
args: ['--nodb'],
env: {
...env,
MONGOSH_GLOBAL_CONFIG_FILE_FOR_TESTING: globalConfig,
},
forceTerminal: true,
});
await shell.waitForPrompt();
shell.assertContainsOutput('Ignoring config option "logLocation"');
shell.assertContainsOutput(
'must be a valid absolute path or empty'
);

expect(
await shell.executeLine(
'config.set("logLocation", "[123123123123]")'
)
).contains(
'Cannot set option "logLocation": logLocation must be a valid absolute path or empty'
);
});

it('gets created according to logLocation, if set', async function () {
const globalConfig = path.join(homedir, 'globalconfig.conf');
await fs.writeFile(
globalConfig,
`mongosh:\n logLocation: ${JSON.stringify(customLogDir.path)}`
);

shell = this.startTestShell({
args: ['--nodb'],
env: {
...env,
MONGOSH_GLOBAL_CONFIG_FILE_FOR_TESTING: globalConfig,
},
forceTerminal: true,
});
await shell.waitForPrompt();
expect(
await shell.executeLine('config.get("logLocation")')
).contains(customLogDir.path);

try {
await readLogFile();
expect.fail('expected to throw');
} catch (error) {
expect((error as Error).message).includes(
'no such file or directory'
);
}

expect(
(await readLogFile(customLogDir.path)).some(
(log) => log.attr?.input === 'config.get("logLocation")'
)
).is.true;
});

it('setting location while running mongosh does not have an immediate effect on logging', async function () {
expect(
await shell.executeLine('config.get("logLocation")')
).does.not.contain(customLogDir.path);
const oldLogId = shell.logId;

const oldLogEntries = await readLogFile();
await shell.executeLine(
`config.set("logLocation", ${JSON.stringify(customLogDir.path)})`
);

await shell.waitForPrompt();
expect(
await shell.executeLine('config.get("logLocation")')
).contains(customLogDir.path);

expect(shell.logId).equals(oldLogId);

const currentLogEntries = await readLogFile();

try {
await readLogFile(customLogDir.path);
expect.fail('expected to throw');
} catch (error) {
expect((error as Error).message).includes(
'no such file or directory'
);
}
expect(
currentLogEntries.some(
(log) => log.attr?.input === 'config.get("logLocation")'
)
).is.true;
expect(currentLogEntries.length).is.greaterThanOrEqual(
oldLogEntries.length
);
});
});

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();
Expand Down
10 changes: 10 additions & 0 deletions packages/types/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import type { ConnectEventMap } from '@mongodb-js/devtools-connect';
import path from 'path';

export interface ApiEventArguments {
pipeline?: any[];
Expand Down Expand Up @@ -507,6 +508,7 @@ export class CliUserConfig extends SnippetShellUserConfig {
browser: undefined | false | string = undefined;
updateURL = 'https://downloads.mongodb.com/compass/mongosh.json';
disableLogging = false;
logLocation: string | undefined = undefined;
}

export class CliUserConfigValidator extends SnippetShellUserConfigValidator {
Expand Down Expand Up @@ -579,6 +581,14 @@ export class CliUserConfigValidator extends SnippetShellUserConfigValidator {
return `${key} must be a valid URL or empty`;
}
return null;
case 'logLocation':
if (
value !== undefined &&
(typeof value !== 'string' || !path.isAbsolute(value))
) {
return `${key} must be a valid absolute path or empty`;
}
return null;
default:
return super.validate(
key as keyof SnippetShellUserConfig,
Expand Down