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

GODRIVER-2881 Enable logging for ComponentAll #1340

Merged
merged 4 commits into from Aug 1, 2023

Conversation

prestonvasquez
Copy link
Collaborator

GODRIVER-2881

Summary

Update the considerations for enabling a log to

  1. Disable with any case where the level if "off"
  2. Disable when ComponentLevels are undefined
  3. Enable if the component "ComponentAll" is defined and it bounds the level passed to the function

Background & Motivation

There is a logging design oversight in which enabling logging with "options.LogComponentAll" does not result in the publication of logs. This issue arises because the "LevelComponentEnabled" method on the internal logger object fails to account for this specific use case.

@prestonvasquez prestonvasquez marked this pull request as ready for review July 31, 2023 23:48
@prestonvasquez prestonvasquez requested a review from a team as a code owner July 31, 2023 23:48
@prestonvasquez prestonvasquez requested review from matthewdale and qingyang-hu and removed request for a team and matthewdale July 31, 2023 23:48
Comment on lines +395 to +402
name: "component mismatch",
logger: Logger{
ComponentLevels: map[Component]Level{
ComponentCommand: LevelDebug,
},
},
level: LevelDebug,
component: ComponentTopology,
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it will be better to have want explicitly.

Comment on lines 351 to 352
name: "nil",
want: false,
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it will be better to have all zero values explicitly.

Copy link
Collaborator

@qingyang-hu qingyang-hu left a comment

Choose a reason for hiding this comment

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

Awesome!

@prestonvasquez prestonvasquez merged commit 1430662 into mongodb:master Aug 1, 2023
14 of 19 checks passed
@prestonvasquez prestonvasquez deleted the GODRIVER-2881 branch August 1, 2023 16:29
prestonvasquez added a commit to prestonvasquez/mongo-go-driver that referenced this pull request Aug 1, 2023
prestonvasquez added a commit to prestonvasquez/mongo-go-driver that referenced this pull request Aug 1, 2023
prestonvasquez added a commit to prestonvasquez/mongo-go-driver that referenced this pull request Aug 1, 2023
prestonvasquez added a commit to prestonvasquez/mongo-go-driver that referenced this pull request Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants