From 2678e1bc47b620d8907469432a611ddc9857e88d Mon Sep 17 00:00:00 2001 From: Preston Vasquez <24281431+prestonvasquez@users.noreply.github.com> Date: Mon, 6 Nov 2023 15:40:41 -0700 Subject: [PATCH] GODRIVER-2824 Remove non-int64 Exported Connection IDs --- event/monitoring.go | 6 ++-- internal/logger/component.go | 5 ++- internal/logger/component_test.go | 8 ++--- mongo/integration/mtest/opmsg_deployment.go | 3 +- .../server_selection_prose_test.go | 4 +-- .../integration/unified/event_verification.go | 6 ++-- x/mongo/driver/driver.go | 2 +- x/mongo/driver/drivertest/channel_conn.go | 3 +- x/mongo/driver/operation.go | 32 +++++++++---------- x/mongo/driver/operation_test.go | 3 +- x/mongo/driver/session/client_session.go | 2 +- x/mongo/driver/topology/CMAP_spec_test.go | 2 +- x/mongo/driver/topology/connection.go | 9 ++---- x/mongo/driver/topology/pool.go | 12 +++---- x/mongo/driver/topology/server.go | 2 +- 15 files changed, 46 insertions(+), 53 deletions(-) diff --git a/event/monitoring.go b/event/monitoring.go index edfca3fa68..e6037d5c14 100644 --- a/event/monitoring.go +++ b/event/monitoring.go @@ -25,7 +25,7 @@ type CommandStartedEvent struct { ConnectionID string // ServerConnectionID64 contains the connection ID from the server of the operation. If the server does not // return this value (e.g. on MDB < 4.2), it is unset. - ServerConnectionID64 *int64 + ServerConnectionID *int64 // ServiceID contains the ID of the server to which the command was sent if it is running behind a load balancer. // Otherwise, it is unset. ServiceID *primitive.ObjectID @@ -40,7 +40,7 @@ type CommandFinishedEvent struct { ConnectionID string // ServerConnectionID64 contains the connection ID from the server of the operation. If the server does not // return this value (e.g. on MDB < 4.2), it is unset. - ServerConnectionID64 *int64 + ServerConnectionID *int64 // ServiceID contains the ID of the server to which the command was sent if it is running behind a load balancer. // Otherwise, it is unset. ServiceID *primitive.ObjectID @@ -101,7 +101,7 @@ type MonitorPoolOptions struct { type PoolEvent struct { Type string `json:"type"` Address string `json:"address"` - ConnectionID uint64 `json:"connectionId"` + ConnectionID int64 `json:"connectionId"` PoolOptions *MonitorPoolOptions `json:"options"` Reason string `json:"reason"` // ServiceID is only set if the Type is PoolCleared and the server is deployed behind a load balancer. This field diff --git a/internal/logger/component.go b/internal/logger/component.go index 0a3d553208..532162dcc3 100644 --- a/internal/logger/component.go +++ b/internal/logger/component.go @@ -144,8 +144,7 @@ func EnvHasComponentVariables() bool { // Command is a struct defining common fields that must be included in all // commands. type Command struct { - // TODO(GODRIVER-2824): change the DriverConnectionID type to int64. - DriverConnectionID uint64 // Driver's ID for the connection + DriverConnectionID int64 // Driver's ID for the connection Name string // Command name DatabaseName string // Database name Message string // Message associated with the command @@ -226,7 +225,7 @@ func SerializeConnection(conn Connection, extraKeysAndValues ...interface{}) Key // Server contains data that all server messages MAY contain. type Server struct { - DriverConnectionID uint64 // Driver's ID for the connection + DriverConnectionID int64 // Driver's ID for the connection TopologyID primitive.ObjectID // Driver's unique ID for this topology Message string // Message associated with the topology ServerConnectionID *int64 // Server's ID for the connection diff --git a/internal/logger/component_test.go b/internal/logger/component_test.go index cf752d2ad7..91bd98dc8d 100644 --- a/internal/logger/component_test.go +++ b/internal/logger/component_test.go @@ -39,7 +39,7 @@ func TestSerializeCommand(t *testing.T) { want: KeyValues{ KeyCommandName, "", KeyDatabaseName, "", - KeyDriverConnectionID, uint64(0), + KeyDriverConnectionID, int64(0), KeyMessage, "", KeyOperationID, int32(0), KeyRequestID, int64(0), @@ -63,7 +63,7 @@ func TestSerializeCommand(t *testing.T) { want: KeyValues{ KeyCommandName, "foo", KeyDatabaseName, "db", - KeyDriverConnectionID, uint64(1), + KeyDriverConnectionID, int64(1), KeyMessage, "bar", KeyOperationID, int32(2), KeyRequestID, int64(3), @@ -145,7 +145,7 @@ func TestSerializeServer(t *testing.T) { { name: "empty", want: KeyValues{ - KeyDriverConnectionID, uint64(0), + KeyDriverConnectionID, int64(0), KeyMessage, "", KeyServerHost, "", KeyTopologyID, primitive.ObjectID{}.Hex(), @@ -162,7 +162,7 @@ func TestSerializeServer(t *testing.T) { ServerPort: "27017", }, want: KeyValues{ - KeyDriverConnectionID, uint64(1), + KeyDriverConnectionID, int64(1), KeyMessage, "foo", KeyServerHost, "localhost", KeyTopologyID, topologyID.Hex(), diff --git a/mongo/integration/mtest/opmsg_deployment.go b/mongo/integration/mtest/opmsg_deployment.go index 63947d3919..04481ebd56 100644 --- a/mongo/integration/mtest/opmsg_deployment.go +++ b/mongo/integration/mtest/opmsg_deployment.go @@ -92,8 +92,7 @@ func (*connection) ID() string { } // DriverConnectionID returns a fixed identifier for the driver pool connection. -// TODO(GODRIVER-2824): replace return type with int64. -func (*connection) DriverConnectionID() uint64 { +func (*connection) DriverConnectionID() int64 { return 0 } diff --git a/mongo/integration/server_selection_prose_test.go b/mongo/integration/server_selection_prose_test.go index e6486ae123..8b17d0e6a4 100644 --- a/mongo/integration/server_selection_prose_test.go +++ b/mongo/integration/server_selection_prose_test.go @@ -22,12 +22,12 @@ import ( "go.mongodb.org/mongo-driver/mongo/options" ) -type saturatedConnections map[uint64]bool +type saturatedConnections map[int64]bool // saturatedHosts is used to maintain information about events with specific host+pool combinations. type saturatedHosts map[string]saturatedConnections -func (set saturatedHosts) add(host string, connectionID uint64) { +func (set saturatedHosts) add(host string, connectionID int64) { if set[host] == nil { set[host] = make(saturatedConnections) } diff --git a/mongo/integration/unified/event_verification.go b/mongo/integration/unified/event_verification.go index 1d54e3fb2a..a3e1ab4e8f 100644 --- a/mongo/integration/unified/event_verification.go +++ b/mongo/integration/unified/event_verification.go @@ -215,7 +215,7 @@ func verifyCommandEvents(ctx context.Context, client *clientEntity, expectedEven } } if expected.HasServerConnectionID != nil { - if err := verifyServerConnectionID(*expected.HasServerConnectionID, actual.ServerConnectionID64); err != nil { + if err := verifyServerConnectionID(*expected.HasServerConnectionID, actual.ServerConnectionID); err != nil { return newEventVerificationError(idx, client, "error verifying serverConnectionID: %v", err) } } @@ -259,7 +259,7 @@ func verifyCommandEvents(ctx context.Context, client *clientEntity, expectedEven } } if expected.HasServerConnectionID != nil { - if err := verifyServerConnectionID(*expected.HasServerConnectionID, actual.ServerConnectionID64); err != nil { + if err := verifyServerConnectionID(*expected.HasServerConnectionID, actual.ServerConnectionID); err != nil { return newEventVerificationError(idx, client, "error verifying serverConnectionID: %v", err) } } @@ -286,7 +286,7 @@ func verifyCommandEvents(ctx context.Context, client *clientEntity, expectedEven } } if expected.HasServerConnectionID != nil { - if err := verifyServerConnectionID(*expected.HasServerConnectionID, actual.ServerConnectionID64); err != nil { + if err := verifyServerConnectionID(*expected.HasServerConnectionID, actual.ServerConnectionID); err != nil { return newEventVerificationError(idx, client, "error verifying serverConnectionID: %v", err) } } diff --git a/x/mongo/driver/driver.go b/x/mongo/driver/driver.go index 5fd3ddcb42..83eaf99fa6 100644 --- a/x/mongo/driver/driver.go +++ b/x/mongo/driver/driver.go @@ -69,7 +69,7 @@ type Connection interface { ID() string ServerConnectionID() *int64 - DriverConnectionID() uint64 // TODO(GODRIVER-2824): change type to int64. + DriverConnectionID() int64 Address() address.Address Stale() bool } diff --git a/x/mongo/driver/drivertest/channel_conn.go b/x/mongo/driver/drivertest/channel_conn.go index 27be4c264d..85471bc94e 100644 --- a/x/mongo/driver/drivertest/channel_conn.go +++ b/x/mongo/driver/drivertest/channel_conn.go @@ -68,8 +68,7 @@ func (c *ChannelConn) ID() string { } // DriverConnectionID implements the driver.Connection interface. -// TODO(GODRIVER-2824): replace return type with int64. -func (c *ChannelConn) DriverConnectionID() uint64 { +func (c *ChannelConn) DriverConnectionID() int64 { return 0 } diff --git a/x/mongo/driver/operation.go b/x/mongo/driver/operation.go index d7b24e90e2..1973ebafac 100644 --- a/x/mongo/driver/operation.go +++ b/x/mongo/driver/operation.go @@ -96,7 +96,7 @@ type startedInformation struct { cmdName string documentSequenceIncluded bool connID string - driverConnectionID uint64 // TODO(GODRIVER-2824): change type to int64. + driverConnectionID int64 serverConnID *int64 redacted bool serviceID *primitive.ObjectID @@ -110,7 +110,7 @@ type finishedInformation struct { response bsoncore.Document cmdErr error connID string - driverConnectionID uint64 // TODO(GODRIVER-2824): change type to int64. + driverConnectionID int64 serverConnID *int64 redacted bool serviceID *primitive.ObjectID @@ -1933,13 +1933,13 @@ func (op Operation) publishStartedEvent(ctx context.Context, info startedInforma if op.canPublishStartedEvent() { started := &event.CommandStartedEvent{ - Command: redactStartedInformationCmd(op, info), - DatabaseName: op.Database, - CommandName: info.cmdName, - RequestID: int64(info.requestID), - ConnectionID: info.connID, - ServerConnectionID64: info.serverConnID, - ServiceID: info.serviceID, + Command: redactStartedInformationCmd(op, info), + DatabaseName: op.Database, + CommandName: info.cmdName, + RequestID: int64(info.requestID), + ConnectionID: info.connID, + ServerConnectionID: info.serverConnID, + ServiceID: info.serviceID, } op.CommandMonitor.Started(ctx, started) } @@ -2012,13 +2012,13 @@ func (op Operation) publishFinishedEvent(ctx context.Context, info finishedInfor } finished := event.CommandFinishedEvent{ - CommandName: info.cmdName, - DatabaseName: op.Database, - RequestID: int64(info.requestID), - ConnectionID: info.connID, - Duration: info.duration, - ServerConnectionID64: info.serverConnID, - ServiceID: info.serviceID, + CommandName: info.cmdName, + DatabaseName: op.Database, + RequestID: int64(info.requestID), + ConnectionID: info.connID, + Duration: info.duration, + ServerConnectionID: info.serverConnID, + ServiceID: info.serviceID, } if info.success() { diff --git a/x/mongo/driver/operation_test.go b/x/mongo/driver/operation_test.go index 6e28ab90bc..be16be3f50 100644 --- a/x/mongo/driver/operation_test.go +++ b/x/mongo/driver/operation_test.go @@ -736,8 +736,7 @@ func (m *mockConnection) CurrentlyStreaming() bool { return m.rStreaming func (m *mockConnection) SetStreaming(streaming bool) { m.rStreaming = streaming } func (m *mockConnection) Stale() bool { return false } -// TODO:(GODRIVER-2824) replace return type with int64. -func (m *mockConnection) DriverConnectionID() uint64 { return 0 } +func (m *mockConnection) DriverConnectionID() int64 { return 0 } func (m *mockConnection) WriteWireMessage(_ context.Context, wm []byte) error { m.pWriteWM = wm diff --git a/x/mongo/driver/session/client_session.go b/x/mongo/driver/session/client_session.go index 4181b6d3de..f48348a687 100644 --- a/x/mongo/driver/session/client_session.go +++ b/x/mongo/driver/session/client_session.go @@ -87,7 +87,7 @@ type LoadBalancedTransactionConnection interface { Close() error ID() string ServerConnectionID() *int64 - DriverConnectionID() uint64 // TODO(GODRIVER-2824): change type to int64. + DriverConnectionID() int64 Address() address.Address Stale() bool diff --git a/x/mongo/driver/topology/CMAP_spec_test.go b/x/mongo/driver/topology/CMAP_spec_test.go index d752af71e2..0bc95254ac 100644 --- a/x/mongo/driver/topology/CMAP_spec_test.go +++ b/x/mongo/driver/topology/CMAP_spec_test.go @@ -63,7 +63,7 @@ var skippedTestDescriptions = map[string]string{ type cmapEvent struct { EventType string `json:"type"` Address interface{} `json:"address"` - ConnectionID uint64 `json:"connectionId"` + ConnectionID int64 `json:"connectionId"` Options interface{} `json:"options"` Reason string `json:"reason"` } diff --git a/x/mongo/driver/topology/connection.go b/x/mongo/driver/topology/connection.go index ac78c12045..14350e3366 100644 --- a/x/mongo/driver/topology/connection.go +++ b/x/mongo/driver/topology/connection.go @@ -74,8 +74,7 @@ type connection struct { // pool related fields pool *pool - // TODO(GODRIVER-2824): change driverConnectionID type to int64. - driverConnectionID uint64 + driverConnectionID int64 generation uint64 } @@ -107,8 +106,7 @@ func newConnection(addr address.Address, opts ...ConnectionOption) *connection { } // DriverConnectionID returns the driver connection ID. -// TODO(GODRIVER-2824): change return type to int64. -func (c *connection) DriverConnectionID() uint64 { +func (c *connection) DriverConnectionID() int64 { return c.driverConnectionID } @@ -802,8 +800,7 @@ func (c *Connection) unpin(reason string) error { } // DriverConnectionID returns the driver connection ID. -// TODO(GODRIVER-2824): change return type to int64. -func (c *Connection) DriverConnectionID() uint64 { +func (c *Connection) DriverConnectionID() int64 { return c.connection.DriverConnectionID() } diff --git a/x/mongo/driver/topology/pool.go b/x/mongo/driver/topology/pool.go index 6e150344db..890a91c596 100644 --- a/x/mongo/driver/topology/pool.go +++ b/x/mongo/driver/topology/pool.go @@ -86,7 +86,7 @@ type pool struct { // - atomic bug: https://pkg.go.dev/sync/atomic#pkg-note-BUG // - suggested layout: https://go101.org/article/memory-layout.html - nextID uint64 // nextID is the next pool ID for a new connection. + nextID int64 // nextID is the next pool ID for a new connection. pinnedCursorConnections uint64 pinnedTransactionConnections uint64 @@ -118,9 +118,9 @@ type pool struct { // to the state of the guarded values must be made while holding the lock to prevent undefined // behavior in the createConnections() waiting logic. createConnectionsCond *sync.Cond - cancelBackgroundCtx context.CancelFunc // cancelBackgroundCtx is called to signal background goroutines to stop. - conns map[uint64]*connection // conns holds all currently open connections. - newConnWait wantConnQueue // newConnWait holds all wantConn requests for new connections. + cancelBackgroundCtx context.CancelFunc // cancelBackgroundCtx is called to signal background goroutines to stop. + conns map[int64]*connection // conns holds all currently open connections. + newConnWait wantConnQueue // newConnWait holds all wantConn requests for new connections. idleMu sync.Mutex // idleMu guards idleConns, idleConnWait idleConns []*connection // idleConns holds all idle connections. @@ -219,7 +219,7 @@ func newPool(config poolConfig, connOpts ...ConnectionOption) *pool { maintainReady: make(chan struct{}, 1), backgroundDone: &sync.WaitGroup{}, createConnectionsCond: sync.NewCond(&sync.Mutex{}), - conns: make(map[uint64]*connection, config.MaxPoolSize), + conns: make(map[int64]*connection, config.MaxPoolSize), idleConns: make([]*connection, 0, config.MaxPoolSize), } // minSize must not exceed maxSize if maxSize is not 0 @@ -1002,7 +1002,7 @@ func (p *pool) createConnections(ctx context.Context, wg *sync.WaitGroup) { conn := newConnection(p.address, p.connOpts...) conn.pool = p - conn.driverConnectionID = atomic.AddUint64(&p.nextID, 1) + conn.driverConnectionID = atomic.AddInt64(&p.nextID, 1) p.conns[conn.driverConnectionID] = conn return w, conn, true diff --git a/x/mongo/driver/topology/server.go b/x/mongo/driver/topology/server.go index a20ad729f1..1f0447250c 100644 --- a/x/mongo/driver/topology/server.go +++ b/x/mongo/driver/topology/server.go @@ -209,7 +209,7 @@ func logServerMessage(srv *Server, msg string, keysAndValues ...interface{}) { serverPort = "" } - var driverConnectionID uint64 + var driverConnectionID int64 var serverConnectionID *int64 if srv.conn != nil {