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.

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

Jump to bottom

feat(NODE-4814): implement remaining severity logging methods #3629

Merged
merged 11 commits into from
Apr 19, 2023
Merged

feat(NODE-4814): implement remaining severity logging methods #3629

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
c2324c9
feat(NODE-4814): Add severity loggers and unskip tests
W-A-James Apr 14, 2023
f745a8c
refactor(NODE-4814): remove unneeded loggers
W-A-James Apr 17, 2023
34b1523
Merge branch 'main' into NODE-4814/implement_remaining_severity_loggi…
W-A-James Apr 17, 2023
33c5a04
test(NODE-4814): remove todos
W-A-James Apr 17, 2023
5d4af37
Merge branch 'NODE-4814/implement_remaining_severity_logging_methods'…
W-A-James Apr 17, 2023
4bcb802
test(NODE-4814): type narrowing
W-A-James Apr 17, 2023
2ca6b39
docs(NODE-4814): add tsdoc comments
W-A-James Apr 18, 2023
8f6f8c8
docs(NODE-4814): Update doc comments
W-A-James Apr 18, 2023
e3c363f
docs(NODE-4814): Update doc comments
W-A-James Apr 18, 2023
70fc2b4
Merge branch 'main' into NODE-4814/implement_remaining_severity_loggi…
W-A-James Apr 19, 2023
170d70a
style(NODE-4814): eslint
W-A-James Apr 19, 2023
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
8 changes: 6 additions & 2 deletions src/mongo_logger.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -430,14 +430,18 @@ export class MongoLogger {
maxDocumentLength: number;
logDestination: MongoDBLogWritable | Writable;

error = this.log.bind(this, 'error');
warn = this.log.bind(this, 'warn');
info = this.log.bind(this, 'info');
debug = this.log.bind(this, 'debug');
trace = this.log.bind(this, 'trace');
nbbeeken marked this conversation as resolved.
Show resolved Hide resolved

constructor(options: MongoLoggerOptions) {
this.componentSeverities = options.componentSeverities;
this.maxDocumentLength = options.maxDocumentLength;
this.logDestination = options.logDestination;
}

emergency = this.log.bind(this, 'emergency');

private log(
severity: SeverityLevel,
component: MongoLoggableComponent,
Expand Down
81 changes: 34 additions & 47 deletions test/unit/mongo_logger.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import {
MongoDBLogWritable,
MongoLogger,
MongoLoggerOptions,
SEVERITY_LEVEL_MAP,
SeverityLevel
} from '../mongodb';

Expand Down Expand Up @@ -79,11 +78,11 @@ describe('class MongoLogger', function () {
}
} as { buffer: any[]; write: (log: Log) => void };
const logger = new MongoLogger({
componentSeverities: { command: 'emergency' } as any,
componentSeverities: { command: 'error' } as any,
logDestination
} as any);

logger.emergency('command', 'Hello world!');
logger.error('command', 'Hello world!');
expect(logDestination.buffer).to.have.lengthOf(1);
});
});
Expand All @@ -99,11 +98,11 @@ describe('class MongoLogger', function () {
});

const logger = new MongoLogger({
componentSeverities: { command: 'emergency' } as any,
componentSeverities: { command: 'error' } as any,
logDestination
} as any);

logger.emergency('command', 'Hello world!');
logger.error('command', 'Hello world!');
expect(buffer).to.have.lengthOf(1);
});
});
Expand All @@ -119,8 +118,8 @@ describe('class MongoLogger', function () {
]);

function* makeValidOptions(): Generator<[string, string]> {
const validOptions = Object.values(SeverityLevel).filter(
option => option !== SeverityLevel.OFF
const validOptions = Object.values(SeverityLevel).filter(option =>
['error', 'warn', 'info', 'debug', 'trace'].includes(option)
);
for (const option of validOptions) {
yield [option, option];
Expand Down Expand Up @@ -427,11 +426,11 @@ describe('class MongoLogger', function () {
const options = MongoLogger.resolveOptions(
{
MONGODB_LOG_PATH: unsetEnvironmentOption,
MONGODB_LOG_COMMAND: 'emergency'
MONGODB_LOG_COMMAND: 'error'
},
{ mongodbLogPath: unsetOption as any }
);
const log: Log = { t: new Date(), c: 'command', s: 'emergency' };
const log: Log = { t: new Date(), c: 'command', s: 'error' };
options.logDestination.write(log);

expect(stderrStub.write).to.have.been.calledOnceWith(
Expand All @@ -449,11 +448,11 @@ describe('class MongoLogger', function () {
const options = MongoLogger.resolveOptions(
{
MONGODB_LOG_PATH: unsetEnvironmentOption,
MONGODB_LOG_COMMAND: 'emergency'
MONGODB_LOG_COMMAND: 'error'
},
{ mongodbLogPath: invalidOption as any }
);
const log: Log = { t: new Date(), c: 'command', s: 'emergency' };
const log: Log = { t: new Date(), c: 'command', s: 'error' };
options.logDestination.write(log);

expect(stderrStub.write).to.have.been.calledOnceWith(
Expand All @@ -471,12 +470,12 @@ describe('class MongoLogger', function () {
const options = MongoLogger.resolveOptions(
{
MONGODB_LOG_PATH: unsetEnvironmentOption,
MONGODB_LOG_COMMAND: 'emergency'
MONGODB_LOG_COMMAND: 'error'
},
{ mongodbLogPath: validOption as any }
);

const log: Log = { t: new Date(), c: 'command', s: 'emergency' };
const log: Log = { t: new Date(), c: 'command', s: 'error' };
options.logDestination.write(log);
const correctDestination = validOptions.get(validOption);
expect(correctDestination?.write).to.have.been.calledOnce;
Expand All @@ -496,11 +495,11 @@ describe('class MongoLogger', function () {
const options = MongoLogger.resolveOptions(
{
MONGODB_LOG_PATH: invalidEnvironmentOption,
MONGODB_LOG_COMMAND: 'emergency'
MONGODB_LOG_COMMAND: 'error'
},
{ mongodbLogPath: unsetClientOption as any }
);
const log: Log = { t: new Date(), c: 'command', s: 'emergency' };
const log: Log = { t: new Date(), c: 'command', s: 'error' };
options.logDestination.write(log);

expect(stderrStub.write).to.have.been.calledOnceWith(
Expand All @@ -520,11 +519,11 @@ describe('class MongoLogger', function () {
const options = MongoLogger.resolveOptions(
{
MONGODB_LOG_PATH: invalidEnvironmentOption,
MONGODB_LOG_COMMAND: 'emergency'
MONGODB_LOG_COMMAND: 'error'
},
{ mongodbLogPath: invalidOption as any }
);
const log: Log = { t: new Date(), c: 'command', s: 'emergency' };
const log: Log = { t: new Date(), c: 'command', s: 'error' };
options.logDestination.write(log);

expect(stderrStub.write).to.have.been.calledOnceWith(
Expand All @@ -543,12 +542,12 @@ describe('class MongoLogger', function () {
const options = MongoLogger.resolveOptions(
{
MONGODB_LOG_PATH: invalidEnvironmentOption,
MONGODB_LOG_COMMAND: 'emergency'
MONGODB_LOG_COMMAND: 'error'
},
{ mongodbLogPath: validOption as any }
);
const correctDestination = validOptions.get(validOption);
const log: Log = { t: new Date(), c: 'command', s: 'emergency' };
const log: Log = { t: new Date(), c: 'command', s: 'error' };
options.logDestination.write(log);
expect(correctDestination?.write).to.have.been.calledOnce;
});
Expand All @@ -566,12 +565,12 @@ describe('class MongoLogger', function () {
const options = MongoLogger.resolveOptions(
{
MONGODB_LOG_PATH: validEnvironmentOption,
MONGODB_LOG_COMMAND: 'emergency'
MONGODB_LOG_COMMAND: 'error'
},
{ mongodbLogPath: unsetOption as any }
);
const correctDestination = validOptions.get(validEnvironmentOption);
options.logDestination.write({ t: new Date(), c: 'command', s: 'emergency' });
options.logDestination.write({ t: new Date(), c: 'command', s: 'error' });

expect(correctDestination?.write).to.have.been.calledOnce;
});
Expand All @@ -588,13 +587,13 @@ describe('class MongoLogger', function () {
const options = MongoLogger.resolveOptions(
{
MONGODB_LOG_PATH: validEnvironmentOption,
MONGODB_LOG_COMMAND: 'emergency'
MONGODB_LOG_COMMAND: 'error'
},
{ mongodbLogPath: invalidValue as any }
);

const correctDestination = validOptions.get(validEnvironmentOption);
const log: Log = { t: new Date(), c: 'command', s: 'emergency' };
const log: Log = { t: new Date(), c: 'command', s: 'error' };
options.logDestination.write(log);

expect(correctDestination?.write).to.have.been.calledOnce;
Expand All @@ -615,12 +614,12 @@ describe('class MongoLogger', function () {
const options = MongoLogger.resolveOptions(
{
MONGODB_LOG_PATH: validEnvironmentOption,
MONGODB_LOG_COMMAND: 'emergency'
MONGODB_LOG_COMMAND: 'error'
},
{ mongodbLogPath: validValue as any }
);
const correctDestination = validOptions.get(validValue);
options.logDestination.write({ t: new Date(), c: 'command', s: 'emergency' });
options.logDestination.write({ t: new Date(), c: 'command', s: 'error' });
expect(correctDestination?.write).to.have.been.calledOnce;
});
}
Expand All @@ -631,9 +630,10 @@ describe('class MongoLogger', function () {
});

describe('severity helpers', function () {
// TODO(NODE-4814): Ensure we test on all valid severity levels
const severities = Object.values(SeverityLevel).filter(severity => severity === 'emergency');
for (const severityLevel of severities) {
const severities = Object.values(SeverityLevel).filter(severity =>
['error', 'warn', 'info', 'debug', 'trace'].includes(severity)
);
for (const [index, severityLevel] of severities.entries()) {
describe(`${severityLevel}()`, function () {
it('does not log when logging for the component is disabled', () => {
const stream = new BufferingStream();
Expand All @@ -648,8 +648,7 @@ describe('class MongoLogger', function () {
expect(stream.buffer).to.have.lengthOf(0);
});

// TODO(NODE-4814): Unskip this test
context.skip('when the log severity is greater than what was configured', function () {
context('when the log severity is greater than what was configured', function () {
it('does not write to logDestination', function () {
const stream = new BufferingStream();
const logger = new MongoLogger({
Expand All @@ -659,13 +658,8 @@ describe('class MongoLogger', function () {
logDestination: stream
} as any);

const TRACE = 8;
for (
let l = SEVERITY_LEVEL_MAP.getNumericSeverityLevel(severityLevel) + 1;
l <= TRACE;
l++
) {
const severity = SEVERITY_LEVEL_MAP.getSeverityLevelName(l);
for (let i = index + 1; i < severities.length; i++) {
const severity = severities[i];
logger[severity as SeverityLevel]('command', 'Hello');
}

Expand All @@ -683,20 +677,13 @@ describe('class MongoLogger', function () {
logDestination: stream
} as any);

const EMERGENCY = 0;
// Calls all severity logging methods with a level less than or equal to what severityLevel
for (
let l = SEVERITY_LEVEL_MAP.getNumericSeverityLevel(severityLevel);
l >= EMERGENCY;
l--
) {
const severity = SEVERITY_LEVEL_MAP.getSeverityLevelName(l);
for (let i = index; i >= 0; i--) {
const severity = severities[i];
Copy link
Contributor

Choose a reason for hiding this comment

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

if we type severities as SeverityLevel[], we should be able to remove the cast in line 683

logger[severity as SeverityLevel]('command', 'Hello');
}

expect(stream.buffer).to.have.lengthOf(
SEVERITY_LEVEL_MAP.getNumericSeverityLevel(severityLevel) + 1
);
expect(stream.buffer).to.have.lengthOf(index + 1);
});
});

Expand Down