Model Host Health Monitoring choice on RabbitContextOptions

Author: Rahmeen14

src/rmq/rmqa/rmqa_rabbitcontextimpl.cpp

@@ -38,6 +38,7 @@
 #include <ball_log.h>
 #include <bdlf_bind.h>
 #include <bdlmt_threadpool.h>
+#include <bsl_variant.h>
 #include <bsls_review.h>
 #include <bsls_timeinterval.h>
 
@@ -191,16 +192,21 @@ RabbitContextImpl::RabbitContextImpl(
 , d_producerTracing(options.producerTracing())
 {
 
-    const bool isHostHealthMonitoringEnabled =
-        options.hostHealthConfig().has_value();
+    // Host health monitoring runs only when a config has been selected. An
+    // explicit opt-out (HostHealthAwarenessOff) and an unset selection both
+    // leave no config, so no monitor is created -- preserving the behaviour
+    // from before an opt-out could be expressed: monitoring ran whenever a
+    // config was attached. get_if returns a pointer into the selection owned by
+    // `options` (which outlives this constructor), or null for the opt-out and
+    // unset alternatives.
+    const rmqt::HostHealthConfig* hostHealthConfig =
+        bsl::get_if<rmqt::HostHealthConfig>(&options.hostHealthSelection());
+    const bool isHostHealthMonitoringEnabled = hostHealthConfig != 0;
 
     // Host health monitoring enabled
     if (isHostHealthMonitoringEnabled) {
-        const rmqt::HostHealthConfig& hostHealthConfig =
-            *options.hostHealthConfig();
-
         d_hostHealthMonitor = bsl::make_shared<rmqamqp::HostHealthMonitor>(
-            hostHealthConfig, d_metricPublisher.get());
+            *hostHealthConfig, d_metricPublisher.get());
         d_hostHealthMonitor->start(d_eventLoop->timerFactory());
     }
 

src/rmq/rmqa/rmqa_rabbitcontextoptions.cpp

@@ -62,18 +62,18 @@ void defaultErrorCallback(const bsl::string& err, int rc)
 } // namespace
 
 RabbitContextOptions::RabbitContextOptions()
-: d_threadpool()
-, d_onError(bdlf::BindUtil::bind(&defaultErrorCallback,
+: d_onError(bdlf::BindUtil::bind(&defaultErrorCallback,
                                  bdlf::PlaceHolders::_1,
                                  bdlf::PlaceHolders::_2))
+, d_hostHealthSelection(HostHealthAwarenessUnset())
+, d_threadpool()
 , d_metricPublisher()
-, d_clientProperties()
 , d_messageProcessingTimeout(DEFAULT_MESSAGE_PROCESSING_TIMEOUT)
-, d_tunables()
 , d_connectionErrorThreshold()
 , d_connectionEstablishmentTimeout()
+, d_clientProperties()
+, d_tunables()
 , d_shuffleConnectionEndpoints()
-, d_hostHealthConfig()
 {
     populateUsefulInformation(&d_clientProperties);
 }
@@ -174,7 +174,15 @@ RabbitContextOptions& RabbitContextOptions::setShuffleConnectionEndpoints(
 RabbitContextOptions& RabbitContextOptions::setHostHealthConfig(
     const rmqt::HostHealthConfig& hostHealthConfig)
 {
-    d_hostHealthConfig = hostHealthConfig;
+    // A config is one alternative of the selection; delegate so the variant
+    // setter remains the single writer of d_hostHealthSelection.
+    return setHostHealthSelection(hostHealthConfig);
+}
+
+RabbitContextOptions& RabbitContextOptions::setHostHealthSelection(
+    const HostHealthSelection& selection)
+{
+    d_hostHealthSelection = selection;
     return *this;
 }
 

src/rmq/rmqa/rmqa_rabbitcontextoptions.h

@@ -28,7 +28,9 @@
 
 #include <bdlmt_threadpool.h>
 #include <bsl_memory.h>
+#include <bsl_optional.h>
 #include <bsl_set.h>
+#include <bsl_variant.h>
 #include <bsls_timeinterval.h>
 
 namespace BloombergLP {
@@ -44,6 +46,30 @@ class RabbitContextOptions {
   public:
     typedef bsl::set<bsl::string> Tunables;
 
+    /// \brief Tag type expressing no host health preference: it neither opts in
+    /// nor opts out, leaving the choice to enclosing configuration layers. This
+    /// is the alternative held by a default-constructed \c HostHealthSelection
+    /// (the default). See \c setHostHealthSelection.
+    struct HostHealthAwarenessUnset {};
+
+    /// \brief Tag type selecting an explicit opt-out from host health
+    /// awareness; see \c setHostHealthSelection. Selecting it is distinct from
+    /// leaving the host health selection unset (\c HostHealthAwarenessUnset),
+    /// which expresses no preference and lets enclosing configuration layers
+    /// apply their own policy.
+    struct HostHealthAwarenessOff {};
+
+    /// \brief A caller's host health selection: exactly one of three mutually
+    /// exclusive alternatives -- \c HostHealthAwarenessUnset (no preference;
+    /// the default), \c rmqt::HostHealthConfig (opt-in; monitoring runs), or
+    /// \c HostHealthAwarenessOff (explicit opt-out). A default-constructed
+    /// selection holds \c HostHealthAwarenessUnset. See
+    /// \c setHostHealthSelection.
+    typedef bsl::variant<HostHealthAwarenessUnset,
+                         rmqt::HostHealthConfig,
+                         HostHealthAwarenessOff>
+        HostHealthSelection;
+
     /// \brief By Default RabbitContext will
     /// 1) Create it's own threadpool for
     /// calling back to client code e.g. consuming messages, confirming
@@ -177,9 +203,25 @@ class RabbitContextOptions {
     /// config is not set, \c consumeOnlyFromHealthyHost has no effect.
     ///
     /// \param hostHealthConfig configuration for host health monitoring
+    ///
+    /// \note This is a convenience equivalent to \c setHostHealthSelection with
+    /// a \c HostHealthConfig: selecting a config is what causes the monitor to
+    /// run, and it replaces any prior opt-out selection. Leaving the selection
+    /// unset (the default) expresses no preference.
     RabbitContextOptions&
     setHostHealthConfig(const rmqt::HostHealthConfig& hostHealthConfig);
 
+    /// \brief Set the caller's host health selection: a \c HostHealthConfig to
+    /// opt in (monitoring runs) or \c HostHealthAwarenessOff to explicitly
+    /// opt out (no monitor is created even if a config could otherwise be
+    /// attached). The alternatives are mutually exclusive -- this replaces any
+    /// previously set config or opt-out. Not calling this at all leaves the
+    /// selection unset (the default), which expresses no preference and lets
+    /// enclosing configuration layers apply their own policy.
+    /// \param selection the host health selection
+    RabbitContextOptions&
+    setHostHealthSelection(const HostHealthSelection& selection);
+
     bdlmt::ThreadPool* threadpool() const { return d_threadpool; }
 
     const bsl::shared_ptr<rmqp::MetricPublisher>& metricPublisher() const
@@ -227,32 +269,38 @@ class RabbitContextOptions {
         return d_shuffleConnectionEndpoints;
     }
 
-    /// \brief Get the host health config. If not set, host health monitoring is
-    /// disabled. By default, host health monitoring is disabled.
-    /// \return The host health config
-    const bsl::optional<rmqt::HostHealthConfig>& hostHealthConfig() const
+    /// \brief Get the caller's host health selection. The returned variant
+    /// holds \c HostHealthAwarenessUnset when no preference has been expressed
+    /// (the default), a \c rmqt::HostHealthConfig when opted in (via
+    /// \c setHostHealthSelection or \c setHostHealthConfig), or a
+    /// \c HostHealthAwarenessOff when explicitly opted out.
+    /// \return The host health selection
+    const HostHealthSelection& hostHealthSelection() const
     {
-        return d_hostHealthConfig;
+        return d_hostHealthSelection;
     }
 
 #ifdef USES_LIBRMQ_EXPERIMENTAL_FEATURES
     RabbitContextOptions& setTunable(const bsl::string& tunable);
 #endif
 
   private:
+    // Members are ordered to minimize padding (see
+    // clang-analyzer-optin.performance.Padding), not by logical grouping. The
+    // constructor's initializer list mirrors this order to avoid -Wreorder.
     static const int DEFAULT_MESSAGE_PROCESSING_TIMEOUT = 60;
-    bdlmt::ThreadPool* d_threadpool;
     rmqt::ErrorCallback d_onError;
+    HostHealthSelection d_hostHealthSelection;
+    bdlmt::ThreadPool* d_threadpool;
     bsl::shared_ptr<rmqp::MetricPublisher> d_metricPublisher;
-    rmqt::FieldTable d_clientProperties;
     bsls::TimeInterval d_messageProcessingTimeout;
-    rmqt::Tunables d_tunables;
-    bsl::optional<bsls::TimeInterval> d_connectionErrorThreshold;
-    bsl::optional<bsls::TimeInterval> d_connectionEstablishmentTimeout;
     bsl::shared_ptr<rmqp::ConsumerTracing> d_consumerTracing;
     bsl::shared_ptr<rmqp::ProducerTracing> d_producerTracing;
+    bsl::optional<bsls::TimeInterval> d_connectionErrorThreshold;
+    bsl::optional<bsls::TimeInterval> d_connectionEstablishmentTimeout;
+    rmqt::FieldTable d_clientProperties;
+    rmqt::Tunables d_tunables;
     bsl::optional<bool> d_shuffleConnectionEndpoints;
-    bsl::optional<rmqt::HostHealthConfig> d_hostHealthConfig;
 };
 
 } // namespace rmqa

src/tests/rmqa/rmqa_rabbitcontextimpl.t.cpp

@@ -31,6 +31,7 @@
 
 #include <bdlmt_threadpool.h>
 #include <bsl_memory.h>
+#include <bsl_variant.h>
 #include <bslma_managedptr.h>
 
 using namespace BloombergLP;
@@ -97,10 +98,47 @@ class RabbitContextImplTests : public Test {
 
 TEST_F(RabbitContextImplTests, ItsAlive)
 {
+    // d_options has a host health config selected, so the monitor is created --
+    // timerFactory() is called three times (monitor start, connection factory,
+    // connection watchdog).
     createExpectations();
     rmqa::RabbitContextImpl context(getMockEventLoop(), d_options);
 }
 
+TEST_F(RabbitContextImplTests, LegacyUserWithConfigStillMonitors)
+{
+    // Backward-compatibility guarantee: a caller who enabled monitoring before
+    // an explicit opt-out could be expressed did so by attaching a host health
+    // config, and did not opt out. They must keep being monitored -- the
+    // monitor is still created (three timerFactory() calls).
+    ASSERT_TRUE(bsl::holds_alternative<rmqt::HostHealthConfig>(
+        d_options.hostHealthSelection()));
+
+    createExpectations();
+    rmqa::RabbitContextImpl context(getMockEventLoop(), d_options);
+}
+
+TEST_F(RabbitContextImplTests, HostHealthMonitoringOptOutSuppressesMonitor)
+{
+    // Even though d_options had a host health config attached, an explicit
+    // opt-out selection replaces it and prevents the monitor from being
+    // created. The monitor is what makes the third timerFactory() call (its
+    // start()), so without it timerFactory() is called only twice (connection
+    // factory + watchdog).
+    d_options.setHostHealthSelection(
+        rmqa::RabbitContextOptions::HostHealthAwarenessOff());
+    ASSERT_FALSE(bsl::holds_alternative<rmqt::HostHealthConfig>(
+        d_options.hostHealthSelection()));
+
+    EXPECT_CALL(*d_mockEventLoop, resolver(false)).Times(1);
+    EXPECT_CALL(*d_mockEventLoop, timerFactory())
+        .Times(2)
+        .WillRepeatedly(Return(d_mockTimerFactory));
+    EXPECT_CALL(*d_mockEventLoop, waitForEventLoopExit(_)).Times(1);
+
+    rmqa::RabbitContextImpl context(getMockEventLoop(), d_options);
+}
+
 TEST_F(RabbitContextImplTests, ErrorWhenProvideNullEndpoint)
 {
     createExpectations();

src/tests/rmqa/rmqa_rabbitcontextoptions.t.cpp

@@ -21,7 +21,9 @@
 #include <bsls_timeinterval.h>
 
 #include <bsl_optional.h>
+#include <bsl_variant.h>
 
+#include <gmock/gmock.h>
 #include <gtest/gtest.h>
 
 using namespace BloombergLP;
@@ -31,6 +33,33 @@ using namespace ::testing;
 namespace {
 bool alwaysHealthy() { return true; }
 
+// gmock matchers over a RabbitContextOptions' host health selection, so
+// assertions read EXPECT_THAT(options, isOptOut()) / Not(isConfig()) etc. `arg`
+// is the matched RabbitContextOptions.
+
+/// Matches when the selection is an explicit opt-out.
+MATCHER(isOptOut, "holds an explicit host health opt-out")
+{
+    return bsl::holds_alternative<
+        rmqa::RabbitContextOptions::HostHealthAwarenessOff>(
+        arg.hostHealthSelection());
+}
+
+/// Matches when no host health preference is expressed (the default).
+MATCHER(isUnset, "expresses no host health preference")
+{
+    return bsl::holds_alternative<
+        rmqa::RabbitContextOptions::HostHealthAwarenessUnset>(
+        arg.hostHealthSelection());
+}
+
+/// Matches when a host health config is selected (opt-in).
+MATCHER(isConfig, "holds a host health config")
+{
+    return bsl::holds_alternative<rmqt::HostHealthConfig>(
+        arg.hostHealthSelection());
+}
+
 /// `bsls_asserttest`'s `BSLS_ASSERTTEST_ASSERT_{PASS,FAIL}` macros report their
 /// outcome by calling a driver-supplied `ASSERT(bool)` (which its header allows
 /// to be a function, not just a macro). Route that into a gtest expectation.
@@ -43,7 +72,10 @@ TEST(RabbitContextOptions, Defaults)
     rmqa::RabbitContextOptions t;
     EXPECT_FALSE(t.metricPublisher());
     EXPECT_FALSE(t.threadpool());
-    EXPECT_FALSE(t.hostHealthConfig().has_value());
+    EXPECT_THAT(t, Not(isConfig()));
+    // No host health selection is expressed by default: neither a config nor
+    // an explicit opt-out.
+    EXPECT_THAT(t, isUnset());
     t.errorCallback()("heres an error", -1);
 }
 
@@ -52,7 +84,7 @@ TEST(RabbitContextOptions, SetHostHealthConfig)
     rmqa::RabbitContextOptions options;
 
     // Initially not set
-    EXPECT_FALSE(options.hostHealthConfig().has_value());
+    EXPECT_THAT(options, Not(isConfig()));
 
     // Create a health checker function
     rmqt::HostHealthConfig config(alwaysHealthy);
@@ -61,7 +93,55 @@ TEST(RabbitContextOptions, SetHostHealthConfig)
     options.setHostHealthConfig(config);
 
     // Now it should be set
-    EXPECT_TRUE(options.hostHealthConfig().has_value());
+    EXPECT_THAT(options, isConfig());
+}
+
+TEST(RabbitContextOptions, SetHostHealthSelectionOptOut)
+{
+    rmqa::RabbitContextOptions options;
+
+    // Initially no selection is expressed (distinct from an explicit opt-out).
+    EXPECT_THAT(options, Not(isConfig()));
+    EXPECT_THAT(options, isUnset());
+
+    // Explicit opt-out.
+    options.setHostHealthSelection(
+        RabbitContextOptions::HostHealthAwarenessOff());
+    EXPECT_THAT(options, isOptOut());
+    EXPECT_THAT(options, Not(isConfig()));
+}
+
+TEST(RabbitContextOptions, SetHostHealthSelectionConfig)
+{
+    rmqa::RabbitContextOptions options;
+
+    // Selecting a config via the variant setter is equivalent to
+    // setHostHealthConfig: the config is retrievable and it is not an opt-out.
+    options.setHostHealthSelection(rmqt::HostHealthConfig(alwaysHealthy));
+    EXPECT_THAT(options, isConfig());
+    EXPECT_THAT(options, Not(isOptOut()));
+}
+
+TEST(RabbitContextOptions, HostHealthSelectionIsMutuallyExclusive)
+{
+    // A config and an explicit opt-out are two alternatives of a single
+    // selection, so setting one replaces the other -- the contradictory
+    // "config attached AND opted out" state cannot be expressed.
+    rmqa::RabbitContextOptions options;
+
+    // Opt-out then attach a config: the config wins.
+    options.setHostHealthSelection(
+        RabbitContextOptions::HostHealthAwarenessOff());
+    options.setHostHealthConfig(rmqt::HostHealthConfig(alwaysHealthy));
+    EXPECT_THAT(options, isConfig());
+    EXPECT_THAT(options, Not(isOptOut()));
+
+    // Attach a config then opt out: the opt-out wins.
+    options.setHostHealthConfig(rmqt::HostHealthConfig(alwaysHealthy));
+    options.setHostHealthSelection(
+        RabbitContextOptions::HostHealthAwarenessOff());
+    EXPECT_THAT(options, isOptOut());
+    EXPECT_THAT(options, Not(isConfig()));
 }
 
 TEST(RabbitContextOptions, SetConnectionEstablishmentTimeoutAcceptsValid)