Skip to content

refactor(NODE-4685): unified spec runner to support logging tests #3578

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

Merged
merged 57 commits into from
Mar 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
57 commits
Select commit Hold shift + click to select a range
80f183b
ci(NODE-4685): WIP test runner changes
W-A-James Feb 27, 2023
1837eb4
ci(NODE-4685): Start implementign new operators
W-A-James Feb 27, 2023
f86a280
ci(NODE-4685): Start adding log capture facilities to ClientEntity
W-A-James Feb 27, 2023
55e9eb3
ci(NODE-4685): continue progress on unified spec runnner
W-A-James Mar 1, 2023
52ed51d
ci(NODE-4685): Implement failureIsRedactedHandling
W-A-James Mar 1, 2023
c714020
ci(NODE-4685): Change field names
W-A-James Mar 1, 2023
53b4d39
style(NODE-4685): Remove todo
W-A-James Mar 1, 2023
6c135e9
fix(NODE-4685): TO BE REVERTED
W-A-James Mar 1, 2023
4321391
test(NODE-4685): Add unit tests for new functionality
W-A-James Mar 3, 2023
cb57476
ci(NODE-4685): Changes to fit 1088matchAsDocument and 1088matchAsRoot…
W-A-James Mar 3, 2023
75dd3ae
Merge branch 'main' into NODE-4685/Easier_debugging_with_standardized…
W-A-James Mar 13, 2023
bc0b66c
test(NODE-4685): Update tests
W-A-James Mar 13, 2023
420481f
ci(NODE-4685): Change type of Logs
W-A-James Mar 13, 2023
a76868b
ci(NODE-4685): run command logging spec tests
W-A-James Mar 13, 2023
a83ae0b
ci(NODE-4685): remove extraneous test file
W-A-James Mar 13, 2023
1cf7a99
ci(NODE-4685): Update LogCollector
W-A-James Mar 13, 2023
abd5be9
style(NODE-4685): remove todo comment
W-A-James Mar 13, 2023
fedaeb8
style(NODE-4685): remove todo comment
W-A-James Mar 13, 2023
94ba6c0
ci(NODE-4685): fix log capture
W-A-James Mar 13, 2023
c291584
ci(NODE-4685): Update conditional
W-A-James Mar 13, 2023
7b9e7f5
ci(NODE-4685): remove unneeded field
W-A-James Mar 13, 2023
48ac7ce
ci(NODE-4685): Fix up log collection
W-A-James Mar 13, 2023
c7a36a8
ci(NODE-4685): Remove unneeded conditon
W-A-James Mar 13, 2023
b0796cf
style(NODE-4685): remove todo
W-A-James Mar 13, 2023
24321f8
ci(NODE-4685): fix condition
W-A-James Mar 13, 2023
a264ba9
Merge branch 'main' into NODE-4685/Easier_debugging_with_standardized…
W-A-James Mar 13, 2023
3428f0c
style(NODE-4685): Remove todo comment
W-A-James Mar 14, 2023
6234beb
style(NODE-4685): eslint
W-A-James Mar 14, 2023
f1d80ac
test(NODE-4685): Add regex check to Error assertion
W-A-James Mar 15, 2023
9950d12
test(NODE-4685): Add regex
W-A-James Mar 15, 2023
4ed6731
ci(NODE-4685): Update assertions
W-A-James Mar 15, 2023
ccd05a9
test(NODE-4685): Update test name
W-A-James Mar 15, 2023
9369b42
ci(NODE-4685): change field name
W-A-James Mar 15, 2023
16adbcf
fix(NODE-4685): remove call to entries()
W-A-James Mar 15, 2023
6fa85e5
style(NODE-4685): eslint
W-A-James Mar 15, 2023
5065ca7
ci(NODE-4685): Update log collector
W-A-James Mar 15, 2023
0537cd5
Merge branch 'main' into NODE-4685/Easier_debugging_with_standardized…
W-A-James Mar 15, 2023
5253a66
test(NODE-4685): skip command logging tests
W-A-James Mar 16, 2023
a7279ec
ci(NODE-4685): Add callback to write method and eslint fixes
W-A-James Mar 16, 2023
4d4d1a3
style(NODE-4685): eslint
W-A-James Mar 16, 2023
b526b8e
test(NODE-4685): Add review fixes
W-A-James Mar 16, 2023
36efc36
ci(NODE-4685): Fix field access
W-A-James Mar 17, 2023
bb4b944
test(NODE-4685): Fix resultCheckSpy
W-A-James Mar 17, 2023
1ce04b4
test(NODE-4685): Move commmand monitoring and command logging spec te…
W-A-James Mar 17, 2023
1be583c
style(NODE-4685): ADd todo comment
W-A-James Mar 17, 2023
a9c1f1d
test(NODE-4685): addressing review comments
W-A-James Mar 17, 2023
f0ee322
refactor(NODE-4685): restore bang operator
W-A-James Mar 20, 2023
0af5a86
refactor(NODE-4685): Add internal symbol property to be able to progr…
W-A-James Mar 20, 2023
b71217a
style(NODE-4685): Rename variable
W-A-James Mar 20, 2023
37277ac
style(NODE-4685): rename variable
W-A-James Mar 20, 2023
efd1c02
refactor(NODE-4685): remove duplicate types
W-A-James Mar 20, 2023
7a21fac
style(NODE-4685): rename symbol property
W-A-James Mar 20, 2023
993958c
Merge branch 'main' into NODE-4685/Easier_debugging_with_standardized…
W-A-James Mar 20, 2023
6f8999c
refactor(NODE-4685): override logger environment variables in Unified…
W-A-James Mar 20, 2023
8ea8ce8
Merge branch 'NODE-4685/Easier_debugging_with_standardized_logging' o…
W-A-James Mar 20, 2023
a61b8cd
refactor(NODE-4685): hardcode component to env var mapping
W-A-James Mar 20, 2023
f60f3bb
test(NODE-4685): Fix test failures
W-A-James Mar 21, 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/connection_string.ts
Original file line number Diff line number Diff line change
Expand Up @@ -531,7 +531,8 @@ export function parseOptions(
MONGODB_LOG_CONNECTION: process.env.MONGODB_LOG_CONNECTION,
MONGODB_LOG_ALL: process.env.MONGODB_LOG_ALL,
MONGODB_LOG_MAX_DOCUMENT_LENGTH: process.env.MONGODB_LOG_MAX_DOCUMENT_LENGTH,
MONGODB_LOG_PATH: process.env.MONGODB_LOG_PATH
MONGODB_LOG_PATH: process.env.MONGODB_LOG_PATH,
...mongoOptions[Symbol.for('@@mdb.internalLoggerConfig')]
};
loggerClientOptions = {
mongodbLogPath: mongoOptions.mongodbLogPath
Expand Down Expand Up @@ -1282,7 +1283,10 @@ export const OPTIONS = {
index: { type: 'any' },
// Legacy Options, these are unused but left here to avoid errors with CSFLE lib
useNewUrlParser: { type: 'boolean' } as OptionDescriptor,
useUnifiedTopology: { type: 'boolean' } as OptionDescriptor
useUnifiedTopology: { type: 'boolean' } as OptionDescriptor,
// MongoLogger
// TODO(NODE-4849): Tighten the type of mongodbLogPath
mongodbLogPath: { type: 'any' }
} as Record<keyof MongoClientOptions, OptionDescriptor>;

export const DEFAULT_OPTIONS = new CaseInsensitiveMap(
Expand Down
18 changes: 18 additions & 0 deletions src/mongo_logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,24 @@ export const SeverityLevel = Object.freeze({
/** @internal */
export type SeverityLevel = (typeof SeverityLevel)[keyof typeof SeverityLevel];

/** @internal */
export const SeverityLevelMap: Map<string | number, string | number> = new Map([
[SeverityLevel.OFF, -Infinity],
[SeverityLevel.EMERGENCY, 0],
[SeverityLevel.ALERT, 1],
[SeverityLevel.CRITICAL, 2],
[SeverityLevel.ERROR, 3],
[SeverityLevel.WARNING, 4],
[SeverityLevel.NOTICE, 5],
[SeverityLevel.INFORMATIONAL, 6],
[SeverityLevel.DEBUG, 7],
[SeverityLevel.TRACE, 8]
]);

for (const [level, value] of SeverityLevelMap) {
SeverityLevelMap.set(value, level);
}

/** @internal */
export const MongoLoggableComponent = Object.freeze({
COMMAND: 'command',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,18 @@ import { runUnifiedSuite } from '../../tools/unified-spec-runner/runner';
// the server. There's nothing we can change here.
const SKIP = ['A successful unordered bulk write with an unacknowledged write concern'];

describe('Command Monitoring Spec (unified)', () => {
runUnifiedSuite(
loadSpecTests(path.join('command-logging-and-monitoring', 'monitoring')),
({ description }) =>
SKIP.includes(description) ? `TODO(NODE-4261): support skip reasons in unified tests` : false
);
describe('Command Logging and Monitoring Spec', function () {
describe('Command Monitoring Spec (unified)', () => {
runUnifiedSuite(
loadSpecTests(path.join('command-logging-and-monitoring', 'monitoring')),
({ description }) =>
SKIP.includes(description)
? `TODO(NODE-4261): support skip reasons in unified tests`
: false
);
});

describe.skip('Command Logging Spec', () => {
runUnifiedSuite(loadSpecTests(path.join('command-logging-and-monitoring', 'logging')));
}).skipReason = 'TODO(NODE-4686): Unskip these tests';
});
58 changes: 57 additions & 1 deletion test/tools/unified-spec-runner/entities.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/* eslint-disable @typescript-eslint/no-non-null-assertion */
import { expect } from 'chai';
import { EventEmitter } from 'events';
import { Writable } from 'stream';

import {
AbstractCursor,
Expand Down Expand Up @@ -37,7 +38,12 @@ import {
import { ejson, getEnvironmentalOptions } from '../../tools/utils';
import type { TestConfiguration } from '../runner/config';
import { trace } from './runner';
import type { ClientEncryption, ClientEntity, EntityDescription } from './schema';
import type {
ClientEncryption,
ClientEntity,
EntityDescription,
ExpectedLogMessage
} from './schema';
import {
createClientEncryption,
makeConnectionString,
Expand Down Expand Up @@ -95,16 +101,40 @@ export type CmapEvent =
| ConnectionCheckedInEvent
| ConnectionPoolClearedEvent;
export type SdamEvent = ServerDescriptionChangedEvent;
export type LogMessage = Omit<ExpectedLogMessage, 'failureIsRedacted'>;

function getClient(address) {
return new MongoClient(`mongodb://${address}`, getEnvironmentalOptions());
}

// TODO(NODE-4813): Remove this class in favour of a simple object with a write method
/* TODO(NODE-4813): Ensure that the object that we replace this with has logic to convert the
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added this TODO instead of adding the logic here since we are still solidifying what the shape of the logs will be

* collected log into the format require by the unified spec runner
* (see ExpectedLogMessage type in schema.ts) */
export class UnifiedLogCollector extends Writable {
collectedLogs: LogMessage[] = [];

constructor() {
super({ objectMode: true });
}

_write(
log: LogMessage,
_: string,
callback: (e: Error | null, l: LogMessage | undefined) => void
) {
this.collectedLogs.push(log);
callback(null, log);
}
}

export class UnifiedMongoClient extends MongoClient {
commandEvents: CommandEvent[] = [];
cmapEvents: CmapEvent[] = [];
sdamEvents: SdamEvent[] = [];
failPoints: Document[] = [];
logCollector: UnifiedLogCollector;

ignoredEvents: string[];
observedCommandEvents: ('commandStarted' | 'commandSucceeded' | 'commandFailed')[];
observedCmapEvents: (
Expand Down Expand Up @@ -148,13 +178,35 @@ export class UnifiedMongoClient extends MongoClient {
serverDescriptionChangedEvent: 'serverDescriptionChanged'
} as const;

static LOGGING_COMPONENT_TO_ENV_VAR_NAME = {
command: 'MONGODB_LOG_COMMAND',
serverSelection: 'MONGODB_LOG_SERVER_SELECTION',
connection: 'MONGODB_LOG_CONNECTION',
topology: 'MONGODB_LOG_TOPOLOGY'
} as const;

constructor(uri: string, description: ClientEntity) {
const logCollector = new UnifiedLogCollector();
const componentSeverities = {
MONGODB_LOG_ALL: 'off'
};

// NOTE: this is done to override the logger environment variables
for (const key in description.observeLogMessages) {
componentSeverities[UnifiedMongoClient.LOGGING_COMPONENT_TO_ENV_VAR_NAME[key]] =
description.observeLogMessages[key];
}

super(uri, {
monitorCommands: true,
[Symbol.for('@@mdb.skipPingOnConnect')]: true,
[Symbol.for('@@mdb.enableMongoLogger')]: true,
[Symbol.for('@@mdb.internalMongoLoggerConfig')]: componentSeverities,
mongodbLogPath: logCollector,
...getEnvironmentalOptions(),
...(description.serverApi ? { serverApi: description.serverApi } : {})
});
this.logCollector = logCollector;

this.ignoredEvents = [
...(description.ignoreCommandMonitoringEvents ?? []),
Expand Down Expand Up @@ -242,6 +294,10 @@ export class UnifiedMongoClient extends MongoClient {
this.off(eventName, this.pushSdamEvent);
}
}

get collectedLogs(): LogMessage[] {
return this.logCollector.collectedLogs;
}
}

export class FailPointMap extends Map<string, Document> {
Expand Down
90 changes: 86 additions & 4 deletions test/tools/unified-spec-runner/match.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
/* eslint-disable @typescript-eslint/no-non-null-assertion */
import { EJSON } from 'bson';
import { expect } from 'chai';
import { inspect } from 'util';

Expand Down Expand Up @@ -34,6 +35,7 @@ import {
ExpectedCommandEvent,
ExpectedError,
ExpectedEventsForClient,
ExpectedLogMessage,
ExpectedSdamEvent
} from './schema';

Expand Down Expand Up @@ -73,12 +75,26 @@ export interface SessionLsidOperator {
export function isSessionLsidOperator(value: unknown): value is SessionLsidOperator {
return typeof value === 'object' && value != null && '$$sessionLsid' in value;
}
export interface MatchAsDocumentOperator {
$$matchAsDocument: unknown;
}
export function isMatchAsDocumentOperator(value: unknown): value is MatchAsDocumentOperator {
return typeof value === 'object' && value != null && '$$matchAsDocument' in value;
}
export interface MatchAsRootOperator {
$$matchAsRoot: unknown;
}
export function isMatchAsRootOperator(value: unknown): value is MatchAsRootOperator {
return typeof value === 'object' && value != null && '$$matchAsRoot' in value;
}

export const SpecialOperatorKeys = [
'$$exists',
'$$type',
'$$matchesEntity',
'$$matchesHexBytes',
'$$matchAsRoot',
'$$matchAsDocument',
'$$unsetOrMatches',
'$$sessionLsid'
];
Expand All @@ -89,7 +105,9 @@ export type SpecialOperator =
| MatchesEntityOperator
| MatchesHexBytesOperator
| UnsetOrMatchesOperator
| SessionLsidOperator;
| SessionLsidOperator
| MatchAsDocumentOperator
| MatchAsRootOperator;

type KeysOfUnion<T> = T extends object ? keyof T : never;
export type SpecialOperatorKey = KeysOfUnion<SpecialOperator>;
Expand All @@ -100,7 +118,9 @@ export function isSpecialOperator(value: unknown): value is SpecialOperator {
isMatchesEntityOperator(value) ||
isMatchesHexBytesOperator(value) ||
isUnsetOrMatchesOperator(value) ||
isSessionLsidOperator(value)
isSessionLsidOperator(value) ||
isMatchAsRootOperator(value) ||
isMatchAsDocumentOperator(value)
);
}

Expand Down Expand Up @@ -199,6 +219,10 @@ export function resultCheck(
return;
}

if (typeof actual !== 'object') {
expect.fail('Expected actual value to be an object');
}

const expectedEntries = Object.entries(expected);

if (Array.isArray(expected)) {
Expand Down Expand Up @@ -294,8 +318,9 @@ export function specialCheck(
// $$sessionLsid
const session = entities.getEntity('session', expected.$$sessionLsid, false);
expect(session, `Session ${expected.$$sessionLsid} does not exist in entities`).to.exist;
const entitySessionHex = session.id!.id.buffer.toString('hex').toUpperCase();
const actualSessionHex = actual.id.buffer.toString('hex').toUpperCase();
const entitySessionHex = session.id!.id.toString('hex').toUpperCase();
const actualSessionHex = actual.id!.toString('hex').toUpperCase();

expect(
entitySessionHex,
`Session entity ${expected.$$sessionLsid} does not match lsid`
Expand Down Expand Up @@ -323,6 +348,25 @@ export function specialCheck(
ejson`expected value at path ${path.join('')} NOT to exist, but received ${actual}`
).to.be.false;
}
} else if (isMatchAsDocumentOperator(expected)) {
if (typeof actual === 'string') {
const actualDoc = EJSON.parse(actual, { relaxed: false });
resultCheck(actualDoc, expected.$$matchAsDocument as any, entities, path, true);
} else {
expect.fail(
`Expected value at path '${path.join('')}' to be string, but received ${inspect(actual)}`
);
}
} else if (isMatchAsRootOperator(expected)) {
expect(
typeof actual,
`Expected value at path '${path.join('')}' to be object, but received ${inspect(actual)}`
).to.equal('object');
expect(typeof expected.$$matchAsRoot, 'Value of $$matchAsRoot must be an object').to.equal(
'object'
);

resultCheck(actual, expected.$$matchAsRoot as any, entities, path, false);
} else {
expect.fail(`Unknown special operator: ${JSON.stringify(expected)}`);
}
Expand Down Expand Up @@ -523,6 +567,44 @@ export function matchesEvents(
}
}

export function compareLogs(
expected: ExpectedLogMessage[],
actual: ExpectedLogMessage[],
entities: EntitiesMap
): void {
expect(actual).to.have.lengthOf(expected.length);

for (const [index, actualLog] of actual.entries()) {
const rootPrefix = `expectLogMessages[${index}]`;
const expectedLog = expected[index];

// Check that log levels match
expect(actualLog).to.have.property('level', expectedLog.level);

// Check that components match
expect(actualLog).to.have.property('component', expectedLog.component);

// NOTE: The spec states that if the failureIsRedacted flag is present, we
// must assert that a failure occurred.
if (expectedLog.failureIsRedacted !== undefined) {
expect(expectedLog.failureIsRedacted).to.be.a('boolean');
expect(actualLog.data.failure, 'Expected failure to exist').to.exist;
if (expectedLog.failureIsRedacted) {
// Assert that failure has been redacted
expect(actualLog.data.failure, 'Expected failure to have been redacted').to.deep.equal({});
} else {
// Assert that failure has not been redacted
expect(
actualLog.data.failure,
'Expected failure to have not been redacted'
).to.not.deep.equal({});
}
}

resultCheck(actualLog.data, expectedLog.data, entities, [rootPrefix], false);
}
}

function isMongoCryptError(err): boolean {
if (err.constructor.name === 'MongoCryptError') {
return true;
Expand Down
2 changes: 1 addition & 1 deletion test/tools/unified-spec-runner/operations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import { getSymbolFrom, sleep } from '../../tools/utils';
import { TestConfiguration } from '../runner/config';
import { EntitiesMap, UnifiedChangeStream } from './entities';
import { expectErrorCheck, resultCheck } from './match';
import type { ExpectedEvent, OperationDescription } from './schema';
import type { ExpectedEvent, ExpectedLogMessage, OperationDescription } from './schema';
import { getMatchingEventCount, translateOptions } from './unified-utils';

interface OperationFunctionParams {
Expand Down
12 changes: 11 additions & 1 deletion test/tools/unified-spec-runner/runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import type { MongoClient } from '../../mongodb';
import { MONGODB_ERROR_CODES, ns, ReadPreference, TopologyType } from '../../mongodb';
import { ejson } from '../utils';
import { EntitiesMap, UnifiedMongoClient } from './entities';
import { matchesEvents } from './match';
import { compareLogs, matchesEvents } from './match';
import { executeOperationAndCheck } from './operations';
import * as uni from './schema';
import { isAnyRequirementSatisfied, patchVersion, zip } from './unified-utils';
Expand Down Expand Up @@ -219,6 +219,16 @@ async function runUnifiedTest(
}
}

if (test.expectLogMessages) {
for (const expectedLogsForClient of test.expectLogMessages) {
const clientId = expectedLogsForClient.client;
const testClient = clientList.get(clientId);

expect(testClient, `No client entity found with id ${clientId}`).to.exist;
compareLogs(expectedLogsForClient.messages, testClient!.collectedLogs, entities);
}
}

if (test.outcome) {
for (const collectionData of test.outcome) {
const collection = utilClient
Expand Down
Loading