stats: implicitly treat stats as enabled if statshost is specified

This is to avoid the odd circumstance where stats don't work because you
specified everything but didn't explicitly enable it. Using
`-statsenabled` has been deprecated and will be removed in the next
major release.

src/init.cpp

@@ -771,7 +771,7 @@ void SetupServerArgs(ArgsManager& argsman)
     argsman.AddArg("-rpcworkqueue=<n>", strprintf("Set the depth of the work queue to service RPC calls (default: %d)", DEFAULT_HTTP_WORKQUEUE), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::RPC);
     argsman.AddArg("-server", "Accept command line and JSON-RPC commands", ArgsManager::ALLOW_ANY, OptionsCategory::RPC);
 
-    argsman.AddArg("-statsenabled", strprintf("Publish internal stats to statsd (default: %u)", DEFAULT_STATSD_ENABLE), ArgsManager::ALLOW_ANY, OptionsCategory::STATSD);
+    hidden_args.emplace_back("-statsenabled");
     argsman.AddArg("-statsbatchsize=<bytes>", strprintf("Specify the size of each batch of stats messages (default: %d)", DEFAULT_STATSD_BATCH_SIZE), ArgsManager::ALLOW_ANY, OptionsCategory::STATSD);
     argsman.AddArg("-statsduration=<ms>", strprintf("Specify the number of milliseconds between stats messages (default: %d)", DEFAULT_STATSD_DURATION), ArgsManager::ALLOW_ANY, OptionsCategory::STATSD);
     argsman.AddArg("-statshost=<ip>", strprintf("Specify statsd host (default: %s)", DEFAULT_STATSD_HOST), ArgsManager::ALLOW_ANY, OptionsCategory::STATSD);
@@ -838,7 +838,7 @@ static void StartupNotify(const ArgsManager& args)
 
 static void PeriodicStats(ArgsManager& args, ChainstateManager& chainman, const CTxMemPool& mempool)
 {
-    assert(args.GetBoolArg("-statsenabled", DEFAULT_STATSD_ENABLE));
+    assert(::g_stats_client->active());
     CCoinsStats stats{CoinStatsHashType::NONE};
     chainman.ActiveChainstate().ForceFlushStateToDisk();
     if (WITH_LOCK(cs_main, return GetUTXOStats(&chainman.ActiveChainstate().CoinsDB(), std::ref(chainman.m_blockman), stats, RpcInterruptionPoint, chainman.ActiveChain().Tip()))) {
@@ -1541,13 +1541,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
     // We need to initialize g_stats_client early as currently, g_stats_client is called
     // regardless of whether transmitting stats are desirable or not and if
     // g_stats_client isn't present when that attempt is made, the client will crash.
-    ::g_stats_client = std::make_unique<StatsdClient>(args.GetArg("-statshost", DEFAULT_STATSD_HOST),
+    ::g_stats_client = InitStatsClient(args);
-                                                      args.GetArg("-statsport", DEFAULT_STATSD_PORT),
-                                                      args.GetArg("-statsbatchsize", DEFAULT_STATSD_BATCH_SIZE),
-                                                      args.GetArg("-statsduration", DEFAULT_STATSD_DURATION),
-                                                      args.GetArg("-statshostname", DEFAULT_STATSD_HOSTNAME),
-                                                      args.GetArg("-statsns", DEFAULT_STATSD_NAMESPACE),
-                                                      args.GetBoolArg("-statsenabled", DEFAULT_STATSD_ENABLE));
 
     {
 
@@ -2278,7 +2272,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
 #endif // ENABLE_WALLET
     }
 
-    if (args.GetBoolArg("-statsenabled", DEFAULT_STATSD_ENABLE)) {
+    if (::g_stats_client->active()) {
         int nStatsPeriod = std::min(std::max((int)args.GetArg("-statsperiod", DEFAULT_STATSD_PERIOD), MIN_STATSD_PERIOD), MAX_STATSD_PERIOD);
         node.scheduler->scheduleEvery(std::bind(&PeriodicStats, std::ref(*node.args), std::ref(chainman), std::cref(*node.mempool)), std::chrono::seconds{nStatsPeriod});
     }

src/net.cpp

@@ -1688,7 +1688,7 @@ void CConnman::NotifyNumConnectionsChanged(CMasternodeSync& mn_sync)
 
 void CConnman::CalculateNumConnectionsChangedStats()
 {
-    if (!gArgs.GetBoolArg("-statsenabled", DEFAULT_STATSD_ENABLE)) {
+    if (!::g_stats_client->active()) {
         return;
     }
 

src/stats/client.cpp

@@ -59,6 +59,34 @@ static constexpr char STATSD_METRIC_TIMING[]{"ms"};
 
 std::unique_ptr<StatsdClient> g_stats_client;
 
+std::unique_ptr<StatsdClient> InitStatsClient(const ArgsManager& args)
+{
+    auto is_enabled = args.GetBoolArg("-statsenabled", /*fDefault=*/false);
+    auto host = args.GetArg("-statshost", /*fDefault=*/"");
+
+    if (is_enabled && host.empty()) {
+        // Stats are enabled but host has not been specified, then use
+        // default host. This is to preserve old behavior.
+        host = DEFAULT_STATSD_HOST;
+    } else if (!host.empty()) {
+        // Host is specified but stats are not explcitly enabled. Assume
+        // that if a host has been specified, we want stats enabled. This
+        // is new behaviour and will substitute old behaviour in a future
+        // release.
+        is_enabled = true;
+    }
+
+    return std::make_unique<StatsdClient>(
+        host,
+        args.GetArg("-statsport", DEFAULT_STATSD_PORT),
+        args.GetArg("-statsbatchsize", DEFAULT_STATSD_BATCH_SIZE),
+        args.GetArg("-statsduration", DEFAULT_STATSD_DURATION),
+        args.GetArg("-statshostname", DEFAULT_STATSD_HOSTNAME),
+        args.GetArg("-statsns", DEFAULT_STATSD_NAMESPACE),
+        is_enabled
+    );
+}
+
 StatsdClient::StatsdClient(const std::string& host, uint16_t port, uint64_t batch_size, uint64_t interval_ms,
                            const std::string& nodename, const std::string& ns, bool enabled) :
     m_nodename{nodename},

src/stats/client.h

@@ -13,10 +13,9 @@
 #include <memory>
 #include <string>
 
+class ArgsManager;
 class RawSender;
 
-/** Default state of Statsd enablement */
-static constexpr bool DEFAULT_STATSD_ENABLE{false};
 /** Default port used to connect to a Statsd server */
 static constexpr uint16_t DEFAULT_STATSD_PORT{8125};
 /** Default host assumed to be running a Statsd server */
@@ -57,6 +56,9 @@ public:
     template <typename T1>
     bool send(const std::string& key, T1 value, const std::string& type, float sample_rate = 1.f);
 
+    /* Check if a StatsdClient instance is ready to send messages */
+    bool active() const { return m_sender != nullptr; }
+
 private:
     /* Mutex to protect PRNG */
     mutable Mutex cs;
@@ -72,6 +74,9 @@ private:
     const std::string m_ns;
 };
 
+/** Parses arguments and constructs a StatsdClient instance */
+std::unique_ptr<StatsdClient> InitStatsClient(const ArgsManager& args);
+
 /** Global smart pointer containing StatsdClient instance */
 extern std::unique_ptr<StatsdClient> g_stats_client;
 

src/test/util/setup_common.cpp

@@ -183,15 +183,7 @@ BasicTestingSetup::BasicTestingSetup(const std::string& chainName, const std::ve
     SetupNetworking();
     InitSignatureCache();
     InitScriptExecutionCache();
-    ::g_stats_client = std::make_unique<StatsdClient>(
+    ::g_stats_client = InitStatsClient(*m_node.args);
-        m_node.args->GetArg("-statshost", DEFAULT_STATSD_HOST),
-        m_node.args->GetArg("-statsport", DEFAULT_STATSD_PORT),
-        m_node.args->GetArg("-statsbatchsize", DEFAULT_STATSD_BATCH_SIZE),
-        m_node.args->GetArg("-statsduration", DEFAULT_STATSD_DURATION),
-        m_node.args->GetArg("-statshostname", DEFAULT_STATSD_HOSTNAME),
-        m_node.args->GetArg("-statsns", DEFAULT_STATSD_NAMESPACE),
-        m_node.args->GetBoolArg("-statsenabled", DEFAULT_STATSD_ENABLE)
-    );
     m_node.chain = interfaces::MakeChain(m_node);
 
     m_node.netgroupman = std::make_unique<NetGroupManager>(/*asmap=*/std::vector<bool>());