GODRIVER-2464 Add timeout for RTT monitor hello operations.

[matthewdale]

GODRIVER-2464

Currently the RTT monitor uses a context without timeout to run the "hello" operation (see here). As a result, it's possible for RTT monitor "hello" operations to get stuck indefinitely if there is a network problem, preventing the monitor from recording more RTT samples. The motivation for this ticket comes from troubleshooting GODRIVER-2438.

Add a timeout to the RTT monitor "hello" operation to prevent network issues from causing the RTT monitor to get stuck. Add a test that confirms that the RTT monitor can recover from a stuck operation.

Also includes proposed SDAM spec test fixes from mongodb/specifications#1272.

[benjirewis]

According to the server monitoring spec, I think we should be using connectTimeoutMS as the socket timeout for hello operations run by the RTT monitor. Do we want to do that instead of adding a 1 minute timeout via context to the hello operation?

[matthewdale]

@benjirewis good find in the SDAM spec! I initially tried to use connectTimeoutMS as the RTT monitor timeout, but it's a little wonky to get access to that information when the RTT monitor is initialized, so I decided to go with a flat 1-minute timeout. However, since the SDAM spec specifically calls for that value, I'll give it another shot.

[benjirewis]

Interesting design! I have some questions, but seems like very elegant test design. It does look like the TestUnifiedSpecs/server-discovery-and-monitoring/integration/hello-network-error.json/Network_error_on_Monitor_check spec test has become flaky due to these changes, though.

[qingyang-hu]

Excellent job! It looks good to me.

[benjirewis]

Looks good to me, and thank you for all the explanations 😄

Any idea if the flakiness of TestUnifiedSpecs/server-discovery-and-monitoring/integration/hello-network-error.json/Network_error_on_Monitor_check is related to these changes, though?

[matthewdale]

Added the proposed changes from mongodb/specifications#1272 here to test them.

[benjirewis]

Thank you for investigating that test flakiness! I think I understand your diagnosis and the spec changes make sense to me (apart from that comment I made). I think we can merge this for now and use the GODRIVER ticket derived from DRIVERS-2386 to revise the tests with any potential changes.

[benjirewis]

Can leave this up to the spec PR, but if this is not an assertion we plan on uncommenting eventually, I'm not sure I see why we don't just remove it entirely.