refactor(NODE-4685): unified spec runner to support logging tests #3578
Conversation
test/integration/command-logging-and-monitoring/command_logging.spec.test.ts
Outdated
Show resolved
Hide resolved
| @@ -169,6 +208,7 @@ export class UnifiedMongoClient extends MongoClient { | |||
| this.observedSdamEvents = (description.observeEvents ?? []) | |||
| .map(e => UnifiedMongoClient.SDAM_EVENT_NAME_LOOKUP[e]) | |||
| .filter(e => !!e); | |||
| this.observedLogEvents = new Map(); | |||
There was a problem hiding this comment.
Will initialize this with the observedLogEvents passed in to the constructor in future, but need to implement NODE-4849 to have them take effect
test/integration/command-logging-and-monitoring/command_logging.spec.test.ts
Outdated
Show resolved
Hide resolved
W-A-James
changed the title
src/connection_string.ts
Outdated
| @@ -521,6 +521,9 @@ export function parseOptions( | |||
| const loggerFeatureFlag = Symbol.for('@@mdb.enableMongoLogger'); | |||
| mongoOptions[loggerFeatureFlag] = mongoOptions[loggerFeatureFlag] ?? false; | |||
|
|
|||
| const internalLoggerConfigSymbol = Symbol.for('@@mdb.internalLoggerConfig'); | |||
There was a problem hiding this comment.
Is there anywhere I should declare this symbol outside of this function? I didn't declare it anywhere else simply because it is something we only intend to use for testing.
| [Symbol.for('@@mdb.enableMongoLogger')]: true, | ||
| [Symbol.for('@@mdb.internalLoggerConfig')]: { | ||
| componentSeverities: description.observeLogMessages, | ||
| logDestination: logCollector | ||
| }, |
There was a problem hiding this comment.
Passing in the logDestination like this comes with the bonus that TS doesn't complain about options that don't exist
|
|
||
| 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 |
There was a problem hiding this comment.
Added this TODO instead of adding the logic here since we are still solidifying what the shape of the logs will be
…MongoClient constructor
…f github.com:mongodb/node-mongodb-native into NODE-4685/Easier_debugging_with_standardized_logging
| const camelToUpperSnake = (s: string) => { | ||
| const output: string[] = []; | ||
| for (const c of s) { | ||
| if (/[A-Z]/.test(c)) { | ||
| output.push('_'); | ||
| output.push(c); | ||
| } else { | ||
| output.push(c); | ||
| } | ||
| } | ||
| return output.join('').toUpperCase(); | ||
| }; | ||
|
|
||
| // NOTE: this is done to override the logger environment variables | ||
| for (const key in description.observeLogMessages) { | ||
| componentSeverities['MONGODB_LOG_' + camelToUpperSnake(key)] = | ||
| description.observeLogMessages[key]; | ||
| } |
There was a problem hiding this comment.
This is safe as we know that all the keys in description.observeLogMessages are the same as the spec-described loggable components and we want to be able to override our environment variables which are described in the spec to be in upper snake case (UPPER_SNAKE_CASE) with MONGODB_LOG_ as a prefix.
There was a problem hiding this comment.
I would hard code the mapping given its a know limited set of keys and then we can find out if there are ones we are unaware of during test sync. But I don't feel strongly
| const camelToUpperSnake = (s: string) => { | ||
| const output: string[] = []; | ||
| for (const c of s) { | ||
| if (/[A-Z]/.test(c)) { | ||
| output.push('_'); | ||
| output.push(c); | ||
| } else { | ||
| output.push(c); | ||
| } | ||
| } | ||
| return output.join('').toUpperCase(); | ||
| }; | ||
|
|
||
| // NOTE: this is done to override the logger environment variables | ||
| for (const key in description.observeLogMessages) { | ||
| componentSeverities['MONGODB_LOG_' + camelToUpperSnake(key)] = | ||
| description.observeLogMessages[key]; | ||
| } |
There was a problem hiding this comment.
I would hard code the mapping given its a know limited set of keys and then we can find out if there are ones we are unaware of during test sync. But I don't feel strongly
nbbeeken
deleted the
NODE-4685/Easier_debugging_with_standardized_logging
branch
Description
Implement changes to unified spec runner to facilitate testing of logging spec implementation
What is changing?
$$matchAsRootoperator$$matchAsDocumentoperatorexpectLogMessagesexpectedLogMessagesexpectedLogMessagesForClientobserveLogMessagesUnifiedMongoClientto be able to collect logs fromMongoLoggerIs there new documentation needed for these changes?
No
TODOs
Update AC of NODE-4849/file new ticket to implement total ordering forLogSeverity$$matchAsRootand$$matchAsDocumentoperatorsDouble check the following
npm run check:lintscripttype(NODE-xxxx)[!]: descriptionfeat(NODE-1234)!: rewriting everything in coffeescript