RUBY-3456 Suppress no-op SDAM debug log lines

lib/mongo/monitoring/server_description_changed_log_subscriber.rb

@@ -23,6 +23,8 @@ class ServerDescriptionChangedLogSubscriber < SDAMLogSubscriber
       private
 
       def log_event(event)
+        return if event.previous_description == event.new_description
+
         log_debug(
           "Server description for #{event.address} changed from " +
           "'#{event.previous_description.server_type}' to '#{event.new_description.server_type}'#{awaited_indicator(event)}."

lib/mongo/monitoring/topology_changed_log_subscriber.rb

@@ -24,6 +24,9 @@ class TopologyChangedLogSubscriber < SDAMLogSubscriber
 
       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
+
           log_debug(
             "There was a change in the members of the '#{event.new_topology.display_name}' " +
               'topology.'

lib/mongo/server/description.rb

@@ -185,14 +185,35 @@ class Description
       # @api private
       CONNECTION_ID = 'connectionId'
 
+      # Constant for reading the modern primary flag from config.
+      #
+      # @api private
+      IS_WRITABLE_PRIMARY = 'isWritablePrimary'
+
+      # Constant for reading the helloOk capability flag from config.
+      #
+      # @api private
+      HELLO_OK = 'helloOk'
+
       # 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
 
       # Instantiate the new server description from the result of the hello
       # command or fabricate a placeholder description for Unknown and
@@ -866,6 +887,7 @@ def service_id
       def ==(other)
         return false if self.class != other.class
         return false if unknown? || other.unknown?
+        return false if server_type != other.server_type
 
         (config.keys + other.config.keys).uniq.all? do |k|
           config[k] == other.config[k] || EXCLUDE_FOR_COMPARISON.include?(k)

spec/mongo/monitoring/server_description_changed_log_subscriber_spec.rb

@@ -0,0 +1,119 @@
+# frozen_string_literal: true
+
+require 'lite_spec_helper'
+
+describe Mongo::Monitoring::ServerDescriptionChangedLogSubscriber do
+  let(:subscriber) { described_class.new }
+  let(:address) { Mongo::Address.new('127.0.0.1:27017') }
+
+  let(:primary_config) do
+    {
+      'isWritablePrimary' => true,
+      'secondary' => false,
+      'setName' => 'rs0',
+      'hosts' => [ '127.0.0.1:27017' ],
+      'me' => '127.0.0.1:27017',
+      'minWireVersion' => 0,
+      'maxWireVersion' => 21,
+      'ok' => 1,
+    }
+  end
+
+  let(:topology) { double('topology') }
+
+  before { Mongo::Logger.level = Logger::DEBUG }
+  after { Mongo::Logger.level = Logger::INFO }
+
+  def make_event(prev_desc, new_desc, awaited: false)
+    Mongo::Monitoring::Event::ServerDescriptionChanged.new(
+      address, topology, prev_desc, new_desc, awaited: awaited
+    )
+  end
+
+  context 'when previous and new descriptions are equal' do
+    let(:prev_desc) { Mongo::Server::Description.new(address, primary_config.dup) }
+    let(:new_desc)  { Mongo::Server::Description.new(address, primary_config.dup) }
+
+    it 'does not log' do
+      expect(subscriber).not_to receive(:log_debug)
+      subscriber.succeeded(make_event(prev_desc, new_desc))
+    end
+
+    it 'does not log when only excluded heartbeat-noise fields differ' do
+      old_config = primary_config.merge('localTime' => Time.at(0))
+      new_config = primary_config.merge('localTime' => Time.at(1000))
+      prev = Mongo::Server::Description.new(address, old_config)
+      new_d = Mongo::Server::Description.new(address, new_config)
+
+      expect(subscriber).not_to receive(:log_debug)
+      subscriber.succeeded(make_event(prev, new_d, awaited: true))
+    end
+  end
+
+  context 'when server_type transitions from unknown to primary' do
+    let(:prev_desc) { Mongo::Server::Description.new(address, {}) }
+    let(:new_desc) { Mongo::Server::Description.new(address, primary_config) }
+
+    it 'logs the change' do
+      expect(subscriber).to receive(:log_debug).with(
+        "Server description for #{address} changed from 'unknown' to 'primary'."
+      )
+      subscriber.succeeded(make_event(prev_desc, new_desc))
+    end
+
+    it 'includes the [awaited] suffix when the event is awaited' do
+      expect(subscriber).to receive(:log_debug).with(
+        "Server description for #{address} changed from 'unknown' to 'primary' [awaited]."
+      )
+      subscriber.succeeded(make_event(prev_desc, new_desc, awaited: true))
+    end
+  end
+
+  context 'when server_type is identical but a non-excluded field differs' do
+    let(:prev_desc) do
+      Mongo::Server::Description.new(address, primary_config.merge('setVersion' => 1))
+    end
+    let(:new_desc) do
+      Mongo::Server::Description.new(address, primary_config.merge('setVersion' => 2))
+    end
+
+    it 'logs the change' do
+      expect(subscriber).to receive(:log_debug)
+      subscriber.succeeded(make_event(prev_desc, new_desc))
+    end
+  end
+
+  context 'when both descriptions are unknown' do
+    let(:prev_desc) { Mongo::Server::Description.new(address, {}) }
+    let(:new_desc) { Mongo::Server::Description.new(address, {}) }
+
+    it 'still logs (Description#== returns false when either side is unknown)' do
+      expect(subscriber).to receive(:log_debug).with(
+        "Server description for #{address} changed from 'unknown' to 'unknown'."
+      )
+      subscriber.succeeded(make_event(prev_desc, new_desc))
+    end
+  end
+
+  context 'when descriptions differ only by legacy hello vs modern hello reply shape' do
+    let(:prev_desc) do
+      Mongo::Server::Description.new(address, primary_config.merge(
+                                                'ismaster' => true,
+                                                'helloOk' => true,
+                                                'isWritablePrimary' => nil
+                                              ))
+    end
+    let(:new_desc) do
+      Mongo::Server::Description.new(address, primary_config.merge(
+                                                'isWritablePrimary' => true,
+                                                'ismaster' => nil,
+                                                'helloOk' => nil
+                                              ))
+    end
+
+    it 'does not log (one-time protocol switch is not interesting)' do
+      expect(subscriber).not_to receive(:log_debug)
+      subscriber.succeeded(make_event(prev_desc, new_desc, awaited: true))
+    end
+  end
+end

spec/mongo/monitoring/topology_changed_log_subscriber_spec.rb

@@ -0,0 +1,96 @@
+# frozen_string_literal: true
+
+require 'lite_spec_helper'
+
+describe Mongo::Monitoring::TopologyChangedLogSubscriber do
+  let(:subscriber) { described_class.new }
+  let(:address) { Mongo::Address.new('127.0.0.1:27017') }
+
+  let(:primary_config) do
+    {
+      'isWritablePrimary' => true,
+      'secondary' => false,
+      'setName' => 'rs0',
+      'hosts' => [ '127.0.0.1:27017' ],
+      'me' => '127.0.0.1:27017',
+      'minWireVersion' => 0,
+      'maxWireVersion' => 21,
+      'ok' => 1,
+    }
+  end
+
+  let(:primary_desc) { Mongo::Server::Description.new(address, primary_config) }
+
+  before { Mongo::Logger.level = Logger::DEBUG }
+  after { Mongo::Logger.level = Logger::INFO }
+
+  # Build a topology instance without invoking its initializer (which requires
+  # a real Cluster). The subscriber only reads `class`, `display_name`, and
+  # `server_descriptions` on the topology, so this is sufficient.
+  def fabricate_topology(klass, server_descriptions)
+    topology = klass.allocate
+    topology.instance_variable_set(:@server_descriptions, server_descriptions)
+    topology
+  end
+
+  def make_event(prev_top, new_top)
+    Mongo::Monitoring::Event::TopologyChanged.new(prev_top, new_top)
+  end
+
+  context 'when topologies have the same class and equal server_descriptions' do
+    let(:server_descriptions) { { address.to_s => primary_desc } }
+    let(:prev_top) do
+      fabricate_topology(Mongo::Cluster::Topology::ReplicaSetWithPrimary, server_descriptions)
+    end
+    let(:new_top) do
+      fabricate_topology(Mongo::Cluster::Topology::ReplicaSetWithPrimary, server_descriptions)
+    end
+
+    it 'does not log' do
+      expect(subscriber).not_to receive(:log_debug)
+      subscriber.succeeded(make_event(prev_top, new_top))
+    end
+  end
+
+  context 'when topologies have the same class but different server_descriptions' do
+    let(:other_address) { Mongo::Address.new('127.0.0.1:27018') }
+    let(:other_desc) { Mongo::Server::Description.new(other_address, primary_config) }
+
+    let(:prev_top) do
+      fabricate_topology(
+        Mongo::Cluster::Topology::ReplicaSetWithPrimary,
+        { address.to_s => primary_desc }
+      )
+    end
+    let(:new_top) do
+      fabricate_topology(
+        Mongo::Cluster::Topology::ReplicaSetWithPrimary,
+        { address.to_s => primary_desc, other_address.to_s => other_desc }
+      )
+    end
+
+    it 'logs the members-changed message' do
+      expect(subscriber).to receive(:log_debug).with(
+        a_string_matching(/There was a change in the members of the '.*' topology\./)
+      )
+      subscriber.succeeded(make_event(prev_top, new_top))
+    end
+  end
+
+  context 'when topology class changes' do
+    let(:server_descriptions) { { address.to_s => primary_desc } }
+    let(:prev_top) do
+      fabricate_topology(Mongo::Cluster::Topology::Unknown, server_descriptions)
+    end
+    let(:new_top) do
+      fabricate_topology(Mongo::Cluster::Topology::ReplicaSetWithPrimary, server_descriptions)
+    end
+
+    it 'logs the type-change message' do
+      expect(subscriber).to receive(:log_debug).with(
+        a_string_matching(/Topology type '.*' changed to type '.*'\./)
+      )
+      subscriber.succeeded(make_event(prev_top, new_top))
+    end
+  end
+end

spec/mongo/server/description_spec.rb

@@ -712,6 +712,52 @@
           end
         end
 
+        context 'when one description is from a legacy isMaster reply and the other from a modern hello reply' do
+          let(:legacy) do
+            described_class.new(address, replica.merge(
+                                           'ismaster' => true,
+                                           'helloOk' => true,
+                                           'isWritablePrimary' => nil
+                                         ))
+          end
+
+          let(:modern) do
+            described_class.new(address, replica.merge(
+                                           'isWritablePrimary' => true,
+                                           'ismaster' => nil,
+                                           'helloOk' => nil
+                                         ))
+          end
+
+          it 'treats them as equal' do
+            expect(legacy == modern).to be(true)
+          end
+        end
+
+        context 'when configs differ only in isWritablePrimary, causing server_type to flip' do
+          let(:rs_other) do
+            described_class.new(address, replica.merge(
+                                           'ismaster' => nil,
+                                           'isWritablePrimary' => false,
+                                           'secondary' => false
+                                         ))
+          end
+
+          let(:promoted) do
+            described_class.new(address, replica.merge(
+                                           'ismaster' => nil,
+                                           'isWritablePrimary' => true,
+                                           'secondary' => false
+                                         ))
+          end
+
+          it 'returns false because the server_type changed' do
+            expect(rs_other.server_type).to eq(:other)
+            expect(promoted.server_type).to eq(:primary)
+            expect(rs_other == promoted).to be false
+          end
+        end
+
         context 'when one config is a subset of the other' do
           let(:one) do
             described_class.new(address, { primary_param => true,