GODRIVER-2966 Remove the heartbeat events after a connection is closed / a check is canceled.

[qingyang-hu]

GODRIVER-2966

Summary

Remove the heartbeat events when checking on the server after a connection is closed / a check is canceled.

Background & Motivation

[prestonvasquez]

LGTM! Did you notice any freezing with the lambda update? Which CI tasks would this be tested in?

[qingyang-hu]

Unfortunately, we don't have a CI task to test against the lambda freezing. I can try to add one.

[matthewdale]

Looks good 👍

[prestonvasquez]

@qingyang-hu Currently there is no clear requirement in the SDAM specifications that requires us to publish heartbeat events when attempting to establish a connection. However, many drivers do this including Python, Node, and Rust.

Go Driver only publishes the "started" event when the connection is not nil. So we don't publish this event on the initial connection, per se. Only when we re-establish connections. The reason we do this comes from PR #1133 , where IIUC it was determined that passing an empty value for the connection ID is not appropriate. There is a Drivers ticket open for this right now, DRIVERS-2677 , where it has been suggested that passing a "0" value for the connectionID is a viable solution in this case since connectionID starts at "1".

I suggest we do three things to resolve this PR:

1. Remove the if block here: https://github.com/mongodb/mongo-go-driver/blob/master/x/mongo/driver/topology/server.go#L801 , but leave the publication. When the connection ID is unknown, then publish that values as "0".

2. Remove this entire block: https://github.com/mongodb/mongo-go-driver/blob/master/x/mongo/driver/topology/server.go#L811-L813 . IIUC, it is a duplicate of this publication: https://github.com/mongodb/mongo-go-driver/blob/master/x/mongo/driver/topology/server.go#L865 . Is that correct?

3. Remove this entire block: https://github.com/mongodb/mongo-go-driver/blob/master/x/mongo/driver/topology/server.go#L816-L818 . IIUC, it is a duplicate of this publication: https://github.com/mongodb/mongo-go-driver/blob/master/x/mongo/driver/topology/server.go#L872 . Is that correct?

CC: @matthewdale , @ShaneHarvey

[qingyang-hu]

I think, instead, we need to remove all publications in the else block L820-L874 including L829 if we plan to publish heartbeat events when attempting to establish a connection.

[prestonvasquez]

I think, instead, we need to remove all publications in the else block L820-L874 including L829 if we plan to publish heartbeat events when attempting to establish a connection.

The publications in the else block are required by the specification, i.e. before we send a hello command and whether or not the hello failed/succeeded: https://github.com/mongodb/specifications/blob/master/source/server-discovery-and-monitoring/server-discovery-and-monitoring-logging-and-monitoring.rst#events-api

[matthewdale]

A connection ID of "0" doesn't match the existing pattern of "<addr>[-<number>]" (see here). Using an empty string seems more correct for when the connection is nil.

[matthewdale]

We should do something similar for the subsequent publishServerHeartbeatSucceededEvent and publishServerHeartbeatFailedEvent so they always get published, even if the conn is nil. That's especially important for the failed event so we can report the error.

[prestonvasquez]

LGTM

[matthewdale]

Looks good 👍