-
Notifications
You must be signed in to change notification settings - Fork 901
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-2586 Add log messages to CMAP spec #1165
GODRIVER-2586 Add log messages to CMAP spec #1165
Conversation
…iver into GODRIVER-2586
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some comments about synchronization/buffering of json.Encoder
, but everything else looks great!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One suggestion to fix the ExampleClientOptions_SetLoggerOptions_customLogger
test failures. Otherwise, looks good 👍
Co-authored-by: Matt Dale <9760375+matthewdale@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job!
GODRIVER-2586
Summary
This PR adds log messages to CMAP operations.
Additional Updates
1. Add
Reason
type to the "event" and "logging" packagesThere is an unexported function in the x "topology" package used to source the event reason for the event monitor called "connectionPerished". The exact strings for the event reason differ between the "event" and "logging" packages. In order to unify the two packages, this PR proposes that we update the function signature to return a struct with fields for various reasons:
2. Unify Component Keys
Add constants to the logger/component.go file that represent "key names". This will unify the IO Sink, the serialization methods, and the key/values appended to logs messages through pool and command operations.
3. Closing v. Checkout Failure
The Go Driver creates it’s pool of connections in the background and there is always a realistic chance that a connection will close after a checkout fails. After careful consideration, the team doesn’t believe this behavior is a bug but a consequence of gracefully trying to concurrently deliver checkouts.
These issues will almost certainly come up again in the specifications PR #1369.
Multiple solutions have been proposed to resolve this issue:
To prevent GODRIVER-2586 from being blocked, this PR suggests that for the moment we relax the order requirements for logs with "ConnectionClosed" and "ConnectionCheckoutFailed" messages.
Background & Motivation
To add log messages and corresponding tests for connection pool logging.