-
Notifications
You must be signed in to change notification settings - Fork 912
GODRIVER-2085 Add serverConnectionId to command monitoring events #724
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-2085 Add serverConnectionId to command monitoring events #724
Conversation
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.
Overall looking good! I have not fully reviewed yet, but I think the connection ID may need to be set from the connection's server description instead of the selected server.
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.
Apologies for the delayed review. I have a question about storing connectionID
on description.Server
, but otherwise 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.
I've removed ServerConnectionID
from description.Server
and added it to HandshakeInformation
.
I also required a new field serverConnectionID
on topology.Connection
and a new method on the Connection
interface: ServerConnectionID
. The new ServerConnectionID
method means that any user implementing driver.Connection
will see an error (this interface is under x
, though).
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.
LGTM! Thank you for addressing the concerns about exposing connectionId
as part of server.Description
.
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.
👍 pending resolving the comment about rebasing on master
.
Failing tests are just due to GODRIVER-2155. |
…ngodb#724) Bump UTF schema version to 1.6
GODRIVER-2085
Adds a
ServerConnectionID
field toCommandStartedEvent
,CommandSucceededEvent
andCommandFailedEvent
to track theserverConnectionId
returned in the server'shello
or legacy hello response (on 4.2+). Syncs command monitoring spec tests. Bumps supported schema version of the unified test runner to 1.6.