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(NODE-4878): Add remaining log configurable client options #3908

Merged
merged 22 commits into from
Nov 8, 2023

Conversation

aditi-khare-mongoDB
Copy link
Contributor

@aditi-khare-mongoDB aditi-khare-mongoDB commented Nov 1, 2023

Description

mongodbLogComponentSeverities and mongodbLogMaxDocumentLength are now supported options in client options through the MongoClient constructor.

Users can now provide severity levels for loggable components through the MongoClient constructor, as well as max document length for their logs.

This feature is currently flagged as internal until Logging is released.

What is changing?

The user can provide component severities through clientOptions. If environment variables as well as client options are present, the client options will take precedence. If certain components are not specified in either, their severity level will default to the value of 'default.' If default is not specified, the severity level for all components will be 'off.'

These client options are only supported for the constructor, not for connection string.

Example:

 new MongoClient('mongodb://localhost:27017', 
     {  mongodbLogComponentSeverities:  {
          client: 'trace',
          topology: 'trace',
          default: 'warn',
        },
        mongodbLogMaxDocumentLength: 800
    }
);
Is there new documentation needed for these changes?

Yes, documentation for MongoClient options should be updated with updated logging information.

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

@aditi-khare-mongoDB aditi-khare-mongoDB changed the title Node 4878/log client configurable options feat(Node 4878): Add remaining log configurable client options Nov 1, 2023
@aditi-khare-mongoDB aditi-khare-mongoDB marked this pull request as ready for review November 2, 2023 21:02
@aditi-khare-mongoDB aditi-khare-mongoDB force-pushed the NODE-4878/log-client-configurable-options branch from a6ee6fa to 8810c5f Compare November 3, 2023 19:26
@W-A-James W-A-James self-assigned this Nov 6, 2023
@W-A-James W-A-James added the Primary Review In Review with primary reviewer, not yet ready for team's eyes label Nov 6, 2023
@W-A-James W-A-James self-requested a review November 6, 2023 18:22
Copy link
Contributor

@W-A-James W-A-James left a comment

Choose a reason for hiding this comment

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

Can we also add a unit test for the mongodbLogMaxDocumentLength?

src/connection_string.ts Outdated Show resolved Hide resolved
src/connection_string.ts Outdated Show resolved Hide resolved
src/mongo_logger.ts Outdated Show resolved Hide resolved
test/unit/mongo_client.test.js Outdated Show resolved Hide resolved
src/mongo_client.ts Outdated Show resolved Hide resolved
src/mongo_logger.ts Outdated Show resolved Hide resolved
@nbbeeken nbbeeken changed the title feat(Node 4878): Add remaining log configurable client options feat(NODE-4878): Add remaining log configurable client options Nov 6, 2023
Copy link
Contributor

@W-A-James W-A-James left a comment

Choose a reason for hiding this comment

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

Current changes look good. Approval pending changing mongodbLogMaxDocumentLength to number and the surrounding code to treat it as an integer specifically from outcome of Slack thread.

test/unit/mongo_client.test.js Outdated Show resolved Hide resolved
Copy link
Contributor

@W-A-James W-A-James left a comment

Choose a reason for hiding this comment

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

LGTM. Failures are unrelated

@W-A-James W-A-James added Team Review Needs review from team and removed Primary Review In Review with primary reviewer, not yet ready for team's eyes labels Nov 7, 2023
@W-A-James W-A-James merged commit 54adc9f into main Nov 8, 2023
1 check passed
@W-A-James W-A-James deleted the NODE-4878/log-client-configurable-options branch November 8, 2023 18:57
aditi-khare-mongoDB added a commit that referenced this pull request Nov 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team Review Needs review from team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants