Skip to content

Commit f3a4844

Browse files
kwvgkwvg
committed
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.
1 parent 69603a8 commit f3a4844

File tree

5 files changed

+41
-22
lines changed

5 files changed

+41
-22
lines changed

‎src/init.cpp‎

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -771,7 +771,7 @@ void SetupServerArgs(ArgsManager& argsman)
771771
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);
772772
argsman.AddArg("-server", "Accept command line and JSON-RPC commands", ArgsManager::ALLOW_ANY, OptionsCategory::RPC);
773773

774-
argsman.AddArg("-statsenabled", strprintf("Publish internal stats to statsd (default: %u)", DEFAULT_STATSD_ENABLE), ArgsManager::ALLOW_ANY, OptionsCategory::STATSD);
774+
hidden_args.emplace_back("-statsenabled");
775775
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);
776776
argsman.AddArg("-statsduration=<ms>", strprintf("Specify the number of milliseconds between stats messages (default: %d)", DEFAULT_STATSD_DURATION), ArgsManager::ALLOW_ANY, OptionsCategory::STATSD);
777777
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)
838838

839839
static void PeriodicStats(ArgsManager& args, ChainstateManager& chainman, const CTxMemPool& mempool)
840840
{
841-
assert(args.GetBoolArg("-statsenabled", DEFAULT_STATSD_ENABLE));
841+
assert(::g_stats_client->active());
842842
CCoinsStats stats{CoinStatsHashType::NONE};
843843
chainman.ActiveChainstate().ForceFlushStateToDisk();
844844
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)
15411541
// We need to initialize g_stats_client early as currently, g_stats_client is called
15421542
// regardless of whether transmitting stats are desirable or not and if
15431543
// g_stats_client isn't present when that attempt is made, the client will crash.
1544-
::g_stats_client = std::make_unique<StatsdClient>(args.GetArg("-statshost", DEFAULT_STATSD_HOST),
1545-
args.GetArg("-statsport", DEFAULT_STATSD_PORT),
1546-
args.GetArg("-statsbatchsize", DEFAULT_STATSD_BATCH_SIZE),
1547-
args.GetArg("-statsduration", DEFAULT_STATSD_DURATION),
1548-
args.GetArg("-statshostname", DEFAULT_STATSD_HOSTNAME),
1549-
args.GetArg("-statsns", DEFAULT_STATSD_NAMESPACE),
1550-
args.GetBoolArg("-statsenabled", DEFAULT_STATSD_ENABLE));
1544+
::g_stats_client = InitStatsClient(args);
15511545

15521546
{
15531547

@@ -2278,7 +2272,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
22782272
#endif // ENABLE_WALLET
22792273
}
22802274

2281-
if (args.GetBoolArg("-statsenabled", DEFAULT_STATSD_ENABLE)) {
2275+
if (::g_stats_client->active()) {
22822276
int nStatsPeriod = std::min(std::max((int)args.GetArg("-statsperiod", DEFAULT_STATSD_PERIOD), MIN_STATSD_PERIOD), MAX_STATSD_PERIOD);
22832277
node.scheduler->scheduleEvery(std::bind(&PeriodicStats, std::ref(*node.args), std::ref(chainman), std::cref(*node.mempool)), std::chrono::seconds{nStatsPeriod});
22842278
}

‎src/net.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1688,7 +1688,7 @@ void CConnman::NotifyNumConnectionsChanged(CMasternodeSync& mn_sync)
16881688

16891689
void CConnman::CalculateNumConnectionsChangedStats()
16901690
{
1691-
if (!gArgs.GetBoolArg("-statsenabled", DEFAULT_STATSD_ENABLE)) {
1691+
if (!::g_stats_client->active()) {
16921692
return;
16931693
}
16941694

‎src/stats/client.cpp‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,34 @@ static constexpr char STATSD_METRIC_TIMING[]{"ms"};
5959

6060
std::unique_ptr<StatsdClient> g_stats_client;
6161

62+
std::unique_ptr<StatsdClient> InitStatsClient(const ArgsManager& args)
63+
{
64+
auto is_enabled = args.GetBoolArg("-statsenabled", /*fDefault=*/false);
65+
auto host = args.GetArg("-statshost", /*fDefault=*/"");
66+
67+
if (is_enabled && host.empty()) {
68+
// Stats are enabled but host has not been specified, then use
69+
// default host. This is to preserve old behavior.
70+
host = DEFAULT_STATSD_HOST;
71+
} else if (!host.empty()) {
72+
// Host is specified but stats are not explcitly enabled. Assume
73+
// that if a host has been specified, we want stats enabled. This
74+
// is new behaviour and will substitute old behaviour in a future
75+
// release.
76+
is_enabled = true;
77+
}
78+
79+
return std::make_unique<StatsdClient>(
80+
host,
81+
args.GetArg("-statsport", DEFAULT_STATSD_PORT),
82+
args.GetArg("-statsbatchsize", DEFAULT_STATSD_BATCH_SIZE),
83+
args.GetArg("-statsduration", DEFAULT_STATSD_DURATION),
84+
args.GetArg("-statshostname", DEFAULT_STATSD_HOSTNAME),
85+
args.GetArg("-statsns", DEFAULT_STATSD_NAMESPACE),
86+
is_enabled
87+
);
88+
}
89+
6290
StatsdClient::StatsdClient(const std::string& host, uint16_t port, uint64_t batch_size, uint64_t interval_ms,
6391
const std::string& nodename, const std::string& ns, bool enabled) :
6492
m_nodename{nodename},

‎src/stats/client.h‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,9 @@
1313
#include <memory>
1414
#include <string>
1515

16+
class ArgsManager;
1617
class RawSender;
1718

18-
/** Default state of Statsd enablement */
19-
static constexpr bool DEFAULT_STATSD_ENABLE{false};
2019
/** Default port used to connect to a Statsd server */
2120
static constexpr uint16_t DEFAULT_STATSD_PORT{8125};
2221
/** Default host assumed to be running a Statsd server */
@@ -57,6 +56,9 @@ class StatsdClient
5756
template <typename T1>
5857
bool send(const std::string& key, T1 value, const std::string& type, float sample_rate = 1.f);
5958

59+
/* Check if a StatsdClient instance is ready to send messages */
60+
bool active() const { return m_sender != nullptr; }
61+
6062
private:
6163
/* Mutex to protect PRNG */
6264
mutable Mutex cs;
@@ -72,6 +74,9 @@ class StatsdClient
7274
const std::string m_ns;
7375
};
7476

77+
/** Parses arguments and constructs a StatsdClient instance */
78+
std::unique_ptr<StatsdClient> InitStatsClient(const ArgsManager& args);
79+
7580
/** Global smart pointer containing StatsdClient instance */
7681
extern std::unique_ptr<StatsdClient> g_stats_client;
7782

‎src/test/util/setup_common.cpp‎

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -183,15 +183,7 @@ BasicTestingSetup::BasicTestingSetup(const std::string& chainName, const std::ve
183183
SetupNetworking();
184184
InitSignatureCache();
185185
InitScriptExecutionCache();
186-
::g_stats_client = std::make_unique<StatsdClient>(
187-
m_node.args->GetArg("-statshost", DEFAULT_STATSD_HOST),
188-
m_node.args->GetArg("-statsport", DEFAULT_STATSD_PORT),
189-
m_node.args->GetArg("-statsbatchsize", DEFAULT_STATSD_BATCH_SIZE),
190-
m_node.args->GetArg("-statsduration", DEFAULT_STATSD_DURATION),
191-
m_node.args->GetArg("-statshostname", DEFAULT_STATSD_HOSTNAME),
192-
m_node.args->GetArg("-statsns", DEFAULT_STATSD_NAMESPACE),
193-
m_node.args->GetBoolArg("-statsenabled", DEFAULT_STATSD_ENABLE)
194-
);
186+
::g_stats_client = InitStatsClient(*m_node.args);
195187
m_node.chain = interfaces::MakeChain(m_node);
196188

197189
m_node.netgroupman = std::make_unique<NetGroupManager>(/*asmap=*/std::vector<bool>());

0 commit comments

Comments
 (0)