Skip to content

RUBY-3456 Suppress no-op SDAM debug log lines#3030

Merged
comandeo-mongo merged 7 commits intomongodb:masterfrom
comandeo-mongo:3456
Apr 28, 2026
Merged

RUBY-3456 Suppress no-op SDAM debug log lines#3030
comandeo-mongo merged 7 commits intomongodb:masterfrom
comandeo-mongo:3456

Conversation

@comandeo-mongo
Copy link
Copy Markdown
Contributor

@comandeo-mongo comandeo-mongo commented Apr 28, 2026

  • Add early-return guards to ServerDescriptionChangedLogSubscriber#log_event
    and TopologyChangedLogSubscriber#log_event so steady-state heartbeats
    no longer produce Server description for ... changed from 'primary' to 'primary'
    / "There was a change in the members of the 'ReplicaSetWithPrimary' topology"
    debug lines when nothing the message conveys has actually changed.
  • Add ismaster, isWritablePrimary, helloOk to
    Server::Description::EXCLUDE_FOR_COMPARISON so the one-time legacy-isMaster
    → modern-hello protocol switch (mandated by the SDAM Server Monitoring spec)
    also stops producing transient 'X' → 'X' lines on connection startup. The
    role itself is still differentiated by the secondary flag and the rest of
    the replica-set metadata.
  • Underlying Monitoring::Event::ServerDescriptionChanged /
    Monitoring::Event::TopologyChanged events are unchanged and continue to
    fire — only the production log subscribers suppress no-op output. SDAM
    unified test runners (which use a separate fixture in
    spec/support/sdam_formatter_integration.rb) and any user-installed
    subscribers are unaffected.

While investigating, I also noticed that in MongoDB 4.4+ streaming mode the
regular Monitor continues to publish ServerDescriptionChanged events while
PushMonitor is also publishing them, contrary to the SDAM spec's "MUST NOT
publish any events when running an RTT command" rule. That's a separate issue,
filed as RUBY-3822.

Resolves https://jira.mongodb.org/browse/RUBY-3456.

Test Plan

  • bundle exec rspec spec/mongo/monitoring/server_description_changed_log_subscriber_spec.rb spec/mongo/monitoring/topology_changed_log_subscriber_spec.rb spec/mongo/server/description_spec.rb — 126 examples, 0 failures locally.
  • bundle exec rubocop clean on all four changed files.
  • 35-second idle smoke against mongodb://localhost:27017,27018,27019/ at debug log level: 3 lines for initial discovery, then zero 'X' → 'X' lines for the rest of the run.
  • CI green on Evergreen / Atlas test matrices.
  • Reviewer to verify SDAM spec runner still passes (no production-log-subscriber dependence in spec/runners/sdam.rb).

The driver does a one-time legacy-isMaster to modern-hello protocol
switch on the initial handshake, per the SDAM Server Monitoring spec.
The two responses populate different config keys (ismaster + helloOk
vs isWritablePrimary), so descriptions for the same logical role were
not considered equal. Add the three protocol-flip keys to
EXCLUDE_FOR_COMPARISON so descriptions that differ only in protocol
shape compare as equal. The role itself remains differentiated by the
SECONDARY flag and the rest of the replica-set metadata.
Copilot AI review requested due to automatic review settings April 28, 2026 08:50
@comandeo-mongo comandeo-mongo requested a review from a team as a code owner April 28, 2026 08:50
@comandeo-mongo comandeo-mongo requested a review from jamis April 28, 2026 08:50
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR reduces steady-state SDAM debug log noise by suppressing no-op output from the built-in SDAM log subscribers, while keeping SDAM monitoring events unchanged.

Changes:

  • Add early-return guards in ServerDescriptionChangedLogSubscriber and TopologyChangedLogSubscriber to avoid emitting debug lines when the logged information would be unchanged.
  • Extend Mongo::Server::Description comparison exclusions to treat legacy isMaster vs modern hello reply shape differences as equivalent.
  • Add unit coverage for the new subscriber suppression behavior and the legacy-vs-modern equality case; remove an obsolete commented Evergreen buildvariant block.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
lib/mongo/monitoring/server_description_changed_log_subscriber.rb Suppresses debug output when descriptions compare equal.
lib/mongo/monitoring/topology_changed_log_subscriber.rb Suppresses debug output when topology class and server descriptions are unchanged.
lib/mongo/server/description.rb Expands Description#== exclusions to ignore ismaster/isWritablePrimary/helloOk differences.
spec/mongo/monitoring/server_description_changed_log_subscriber_spec.rb New unit coverage for no-op suppression and awaited suffix behavior.
spec/mongo/monitoring/topology_changed_log_subscriber_spec.rb New unit coverage for topology no-op suppression vs member/type change logging.
spec/mongo/server/description_spec.rb Adds a focused equality test for legacy isMaster vs modern hello replies.
.evergreen/config/standard.yml.erb Removes a commented-out JRuby buildvariant block.
.evergreen/config.yml Removes the same commented-out JRuby buildvariant block.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

expect(legacy == modern).to be(true)
end
end

Comment on lines 25 to +29
def log_event(event)
if event.previous_topology.class == event.new_topology.class
return if event.previous_topology.server_descriptions ==
event.new_topology.server_descriptions

Comment on lines 198 to +216
# Fields to exclude when comparing two descriptions.
#
# The PRIMARY (legacy `ismaster`), IS_WRITABLE_PRIMARY (modern `hello`),
# and HELLO_OK keys are excluded because the driver does a one-time
# legacy-`isMaster` to modern-`hello` protocol switch on the initial
# handshake (per the SDAM Server Monitoring spec). Two responses for the
# same logical role differ only in which of these keys is populated.
# The role itself is still differentiated by the SECONDARY flag and the
# remaining replica-set metadata.
#
# @since 2.0.6
EXCLUDE_FOR_COMPARISON = [ LOCAL_TIME,
LAST_WRITE,
OPERATION_TIME,
Operation::CLUSTER_TIME,
CONNECTION_ID, ].freeze
CONNECTION_ID,
PRIMARY,
IS_WRITABLE_PRIMARY,
HELLO_OK, ].freeze
private

def log_event(event)
return if event.previous_description == event.new_description
Adding ismaster/isWritablePrimary to EXCLUDE_FOR_COMPARISON made it
possible for two descriptions with different server_type (e.g. RSOther
vs Primary, where only isWritablePrimary flips) to compare equal,
which would let the new SDAM log subscriber early-returns suppress
real role transitions. Guard == with a server_type check and add a
spec covering the RSOther -> Primary boundary.
@comandeo-mongo comandeo-mongo merged commit 2925bac into mongodb:master Apr 28, 2026
198 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants