SWIFT-360 Allow specification of an event handler instead of NotificationCenter for APM

[patrickfreed]

SWIFT-360

This PR replaces the NotificationCenter based APM event reporting API with a protocol oriented one ("callback based"). Users can now implement CommandEventHandler and SDAMEventHandler protocols on their own types and pass them in via the ClientOptions to receive events, rather than attaching observers to clients' NotificationCenters.

Another key change that occurred as part of this PR is that the event protocols were replaced with enums. Enums seemed like nice fits because all the protocol implementation cases were known and that users would likely have been downcasting the protocols quite a bit.

The mapping from old protocol to enum is as follows:

• MongoEvent -> removed: It didn't seem useful for users to be able to bucket different categories of events together.
• MongoCommandEvent -> CommandEvent: I removed the Mongo prefix since it seemed redundant given that users can always do MongoSwift.CommandEvent to disambiguate, though we do still have a number of Mongo-prefixed types in the driver, so maybe this wasn't prudent...
• MongoSDAMEvent -> removed: "SDAM event" was too broad, so I decided to just get rid of it.
• MongoTopologyUpdateEvent -> TopologyEvent: dropped the "Update" part, since a TopologyOpeningEvent isn't really an "update" to the topology per se.
• MongoServerUpdateEvent -> removed: Seemed too specific. These events were just included with the TopologyEvent enum. They are grouped this way in the SDAM Monitoring spec as well.
• MongoServerHeartbeatEvent -> ServerHeartbeatEvent

[kmahar]

seems like we turn monitoring on/off and clear out events a lot, do you think something like a withMonitoring method that allows you to run a closure would be useful?

[patrickfreed]

Yeah that's a good idea. I added a captureEvents method to TestCommandEventHandler that does as you suggest. I actually was able to just remove the begin/stop methods since everywhere that used them could be replaced with the closure approach.

[kmahar]

lgtm mod the question of splitting up server and topology events, will wait to see what @mbroadst has to say on that.
this is way nicer than our old approach IMO and I'm thrilled to get rid of all the NotificationCenter junk in the tests, great job 👏

[patrickfreed]

The latest changes move the registration of handlers to MongoClient and add a way for a user to just specify a callback instead of implementing their own handlers. It also combines all SDAM monitoring events into a single SDAMEvent enum as discussed.

Some other changes made as part of this PR (inspired by parts 1 and 2 of this article) is that the client now stores the handlers weakly to prevent strong reference cycles. The callbacks are stored with strong references though, since otherwise any closure literals passed in would just immediately go out of scope.

The articles suggest that the callback we accept should accept the client in as one of the parameters to discourage strong capturing (and thus reference cycles), but I didn't see that as a super compelling solution, since it doesn't prevent the user from capturing the client and adds some unneeded complexity to the callback signature. Instead, I just added a warning to the docstring of the callback registration method.

One open question I have is whether we want to add method(s) to deregister event handlers. It is mentioned in the article and is also something that NotificationCenter supports. It would be relatively easy to implement, but I just wasn't sure if it was something we'd expect users to want. Also, it would be an additive change so we can add it later if we want.

[kmahar]

LGTM! nice job with the updates.

I'm fine with deferring adding the capability to remove handlers for now, let's maybe just open a ticket about it as a future improvement. if people need it they have the escape hatch of removing any references to their handler or using some flag in their callback to enable/disable it.

[patrickfreed]

Filed SWIFT-732. Once my evergreen patch passes will merge.