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-5169): Implement emergency logger #3610

Merged

nbbeeken merged 48 commits into main from NODE-4813/Implement_severity_logging_methods

Apr 14, 2023

Contributor

W-A-James commented Mar 27, 2023 •

edited

Description

What is changing?

Implement MongoLogger.emergency severity logger, the private MongoLogger.log method and related private helpers
Change the requirement for MongoLoggerMongoClientOptions.mongodbLogPath to be a MongoDBLogWritable
All command monitoring and connection monitoring events now have a public name field.
ConnectionClosedEvent's constructor now takes in an optional error that is used in the logger
Adds a new integration test to ensure that the redaction of sensitive commands in the CommandXXXXEvent constructors works when the events are passed to the logger.

Is there new documentation needed for these changes?

No

What is the motivation for this change?

Get initial implementation of one severity logging method complete to enable quick completion of the remainder of the logging methods

Double check the following

Ran npm run check:lint script
Self-review completed using the steps outlined here
PR title follows the correct format: type(NODE-xxxx)[!]: description
- Example: feat(NODE-1234)!: rewriting everything in coffeescript
Changes are covered by tests
New TODOs have a related JIRA ticket

W-A-James and others added 6 commits

March 27, 2023 14:39


          refactor(NODE-4813): WIP

a8d42ad


          Merge branch 'main' into NODE-4813/Implement_severity_logging_methods

6b053d9


          refactor(NODE-4813): start first draft of logger implementation

e0b1db2


          style(NODE-4813): eslint

d2ff692


          refactor(NODE-4813): Add changes to MongoLogger and ConnectionClosedE…

a52b1ae

…vent


          test(NODE-4813): WIP - start adding tests

16fee3f

W-A-James commented

View reviewed changes

src/cmap/connection_pool_events.ts Outdated Show resolved Hide resolved

W-A-James commented

View reviewed changes

src/cmap/connection_pool_events.ts Outdated Show resolved Hide resolved

W-A-James commented

View reviewed changes

src/mongo_logger.ts Outdated Show resolved Hide resolved

W-A-James commented

View reviewed changes

src/mongo_logger.ts Outdated Show resolved Hide resolved

W-A-James commented

View reviewed changes

src/mongo_logger.ts Show resolved Hide resolved

W-A-James commented

View reviewed changes

test/unit/mongo_logger.test.ts Outdated Show resolved Hide resolved

W-A-James commented

View reviewed changes

test/unit/mongo_logger.test.ts Outdated Show resolved Hide resolved

W-A-James commented

View reviewed changes

test/unit/mongo_logger.test.ts Outdated Show resolved Hide resolved


          fix(NODE-4813): use HostAddress

b88003e

W-A-James commented

View reviewed changes

src/cmap/command_monitoring_events.ts Outdated Show resolved Hide resolved

W-A-James added 2 commits

April 3, 2023 15:36


          fix(NODE-4813): Add Loggable interface

06a1bf0


          test(NODE-4813): Add connection events tests

3501a35

W-A-James commented

View reviewed changes

test/unit/mongo_logger.test.ts Show resolved Hide resolved

W-A-James added 2 commits

April 3, 2023 16:45


          fix(NODE-4813): omit nullish fields

bf591b9


          test(NODE-4813): update tests

36e2e32

W-A-James commented

View reviewed changes

test/unit/mongo_logger.test.ts Show resolved Hide resolved

W-A-James commented

View reviewed changes

test/unit/mongo_logger.test.ts Outdated Show resolved Hide resolved

W-A-James commented

View reviewed changes

test/unit/mongo_logger.test.ts Show resolved Hide resolved

W-A-James changed the title ~~refactor(NODE-4813): WIP~~ feat(NODE-5169): Implement emergency logger

W-A-James added 2 commits

April 4, 2023 15:25


          refactor(NODE-4813): Update mongo client to accept @@mdb.internalLogC…

80fbecd

…onfig feature flag and remove strict writable requirement for log sink in mongologger


          test(NODE-4813): Add integration tests

da4f89c

W-A-James commented

View reviewed changes

test/integration/command-logging-and-monitoring/command_logging.test.ts Outdated Show resolved Hide resolved

W-A-James added 2 commits

April 4, 2023 15:55


          test(NODE-4813): ensure that mongo logger works with MongoDBLogWritab…

3c76aeb

…le implementors


          style(NODE-4813): remove todo comment

a39965f

W-A-James and others added 2 commits

April 12, 2023 09:25


          Merge branch 'main' into NODE-4813/Implement_severity_logging_methods

666221b


          fix(NODE-5169): Remove createdStdioLogger from export

b76ba6d

baileympearson reviewed

View reviewed changes

src/utils.ts Show resolved Hide resolved


          refactor(NODE-5169): Addres review comments

4db4913

W-A-James requested a review from nbbeeken

April 12, 2023 15:59

nbbeeken added Team Review and removed Primary Review labels

W-A-James commented

View reviewed changes

test/integration/command-logging-and-monitoring/command_logging.test.ts Outdated Show resolved Hide resolved

W-A-James and others added 2 commits

April 12, 2023 13:09


          test(NODE-5169): remove unneeded test file

be04325


          Merge branch 'main' into NODE-4813/Implement_severity_logging_methods

nbbeeken approved these changes

View reviewed changes

nbbeeken requested a review from baileympearson

April 12, 2023 17:20

baileympearson requested changes

View reviewed changes

src/cmap/connection_pool_events.ts Show resolved Hide resolved

src/constants.ts Outdated

               export const TOPOLOGY_OPENING = 'topologyOpening' as const;
               export const TOPOLOGY_CLOSED = 'topologyClosed' as const;
               export const TOPOLOGY_DESCRIPTION_CHANGED = 'topologyDescriptionChanged' as const;
+              /** @public */

Contributor

baileympearson Apr 13, 2023

we do we need to export our constants?

Contributor Author

W-A-James Apr 13, 2023

This was done since the the name property that we have on our Command Events and ConnectionPool Events is public so we need to have these constants exported. I'm open to doing this in a more elegant way.

Contributor

baileympearson Apr 13, 2023

Well one option is to make the name property on the command events internal. There's no reason it needs to be public, right? It's only used by the logger.

src/utils.ts Show resolved Hide resolved

src/mongo_logger.ts Outdated Show resolved Hide resolved

nbbeeken requested a review from dariakp

April 13, 2023 15:59

nbbeeken requested changes

View reviewed changes

src/cmap/command_monitoring_events.ts Show resolved Hide resolved

src/cmap/connection_pool_events.ts Show resolved Hide resolved

src/cmap/connection_pool_events.ts Outdated Show resolved Hide resolved


          fix(NODE-5169): Addres review comments and fix ConnectionCheckOutFail…

57a643c

…ed case

W-A-James requested review from nbbeeken and baileympearson

April 13, 2023 19:38

dariakp requested changes

View reviewed changes

src/mongo_logger.ts Outdated Show resolved Hide resolved

nbbeeken requested changes

View reviewed changes

src/cmap/connection_pool.ts Outdated Show resolved Hide resolved

src/cmap/connection_pool_events.ts Outdated Show resolved Hide resolved

W-A-James and others added 5 commits

April 14, 2023 10:49


          fix(NODE-5169): update signature

c456ce9


          fix(NODE-5169): convert to MongoError

f258901


          style(NODE-5169): eslint

442557b


          refactor(NODE-5169): update function signature

dd66ea3


          Merge branch 'main' into NODE-4813/Implement_severity_logging_methods

369216d

W-A-James requested a review from nbbeeken

April 14, 2023 18:15

nbbeeken approved these changes

View reviewed changes

baileympearson approved these changes

View reviewed changes

nbbeeken merged commit d502eb0 into main

16 of 21 checks passed

nbbeeken deleted the NODE-4813/Implement_severity_logging_methods branch

April 14, 2023 19:58

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment