RUBY-3241 URI option for monitoring type#2989
RUBY-3241 URI option for monitoring type#2989comandeo-mongo wants to merge 2 commits intomongodb:masterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds support for a new serverMonitoringMode URI/client option to control whether the driver uses SDAM streaming (awaitable hello) vs polling, and updates unified test runner support for SDAM/heartbeat events.
Changes:
- Add
serverMonitoringModeparsing/stringifying viaMongo::URI::OptionsMapper, plus URI spec coverage (valid values, case-insensitivity, invalid value warning). - Gate push monitor (streaming) startup in
Mongo::Server::Monitorbehind a newstreaming_enabled?decision. - Extend unified spec runner/assertions to subscribe to server heartbeat events, filter SDAM events, and assert the
awaitedfield.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| spec/runners/unified/test.rb | Subscribes unified runner to SERVER_HEARTBEAT events when requested. |
| spec/runners/unified/assertions.rb | Adds SDAM event filtering and awaited assertion support. |
| spec/mongo/uri_spec.rb | Adds tests for serverMonitoringMode URI option parsing/roundtripping/warnings. |
| spec/mongo/server/monitor_spec.rb | Adds unit tests for Monitor#streaming_enabled? across modes and environments. |
| lib/mongo/uri/options_mapper.rb | Introduces serverMonitoringMode option mapping and value conversion/stringification. |
| lib/mongo/uri.rb | Defines SERVER_MONITORING_MODES accepted values for URI parsing. |
| lib/mongo/server/monitor.rb | Adds streaming_enabled? and uses it to enable/disable push monitoring. |
| lib/mongo/client.rb | Adds :server_monitoring_mode to VALID_OPTIONS. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| mode = options[:server_monitoring_mode] || :auto | ||
| case mode | ||
| when :poll | ||
| false | ||
| when :stream |
There was a problem hiding this comment.
streaming_enabled? can return nil if :server_monitoring_mode is set to an unexpected value (e.g. a String like "poll" or any other symbol), because the case has no else. This makes the method violate its boolean contract and will implicitly disable streaming without any warning. Consider normalizing string values (e.g. downcase+to_sym) and adding an else branch that logs a warning and defaults to :auto (or raises) so the return value is always true/false.
Implements the serverMonitoringMode URI option per the URI options spec. Accepted values are
stream,poll, orauto(default).