From 42918c2cdcb42cacaef90ebc4321037169c69e6a Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Thu, 12 Sep 2024 15:18:22 +0000 Subject: [PATCH] stats: rename `statshostname` to more appropriate `statssuffix` `statshostname` implies that alongside specifying the host, the user needs to specify a separate "hostname" field in order to connect when in fact, it is an entirely optional suffix field applied to stats keys. Rename it to `statssuffix` and deprecate `statshostname`, the latter will be removed in a subsequent major release. --- src/init.cpp | 5 +++-- src/stats/client.cpp | 35 +++++++++++++++++++++++++++-------- src/stats/client.h | 10 +++++----- 3 files changed, 35 insertions(+), 15 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 4ee582dde6..6b1f895427 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -775,10 +775,11 @@ void SetupServerArgs(ArgsManager& argsman) argsman.AddArg("-statsbatchsize=", strprintf("Specify the size of each batch of stats messages (default: %d)", DEFAULT_STATSD_BATCH_SIZE), ArgsManager::ALLOW_ANY, OptionsCategory::STATSD); argsman.AddArg("-statsduration=", strprintf("Specify the number of milliseconds between stats messages (default: %d)", DEFAULT_STATSD_DURATION), ArgsManager::ALLOW_ANY, OptionsCategory::STATSD); argsman.AddArg("-statshost=", strprintf("Specify statsd host (default: %s)", DEFAULT_STATSD_HOST), ArgsManager::ALLOW_ANY, OptionsCategory::STATSD); - argsman.AddArg("-statshostname=", strprintf("Specify statsd host name (default: %s)", DEFAULT_STATSD_HOSTNAME), ArgsManager::ALLOW_ANY, OptionsCategory::STATSD); + hidden_args.emplace_back("-statshostname"); argsman.AddArg("-statsport=", strprintf("Specify statsd port (default: %u)", DEFAULT_STATSD_PORT), ArgsManager::ALLOW_ANY, OptionsCategory::STATSD); - argsman.AddArg("-statsns=", strprintf("Specify additional namespace prefix (default: %s)", DEFAULT_STATSD_NAMESPACE), ArgsManager::ALLOW_ANY, OptionsCategory::STATSD); + argsman.AddArg("-statsns=", strprintf("Specify additional namespace prefix (default: %s)", DEFAULT_STATSD_SUFFIX), ArgsManager::ALLOW_ANY, OptionsCategory::STATSD); argsman.AddArg("-statsperiod=", strprintf("Specify the number of seconds between periodic measurements (default: %d)", DEFAULT_STATSD_PERIOD), ArgsManager::ALLOW_ANY, OptionsCategory::STATSD); + argsman.AddArg("-statssuffix=", strprintf("Specify an optional string appended to every stats key (default: %s)", DEFAULT_STATSD_SUFFIX), ArgsManager::ALLOW_ANY, OptionsCategory::STATSD); #if HAVE_DECL_FORK argsman.AddArg("-daemon", strprintf("Run in the background as a daemon and accept commands (default: %d)", DEFAULT_DAEMON), ArgsManager::ALLOW_BOOL, OptionsCategory::OPTIONS); argsman.AddArg("-daemonwait", strprintf("Wait for initialization to be finished before exiting. This implies -daemon (default: %d)", DEFAULT_DAEMONWAIT), ArgsManager::ALLOW_BOOL, OptionsCategory::OPTIONS); diff --git a/src/stats/client.cpp b/src/stats/client.cpp index f9c1e66355..865c05bb41 100644 --- a/src/stats/client.cpp +++ b/src/stats/client.cpp @@ -49,6 +49,8 @@ static constexpr float EPSILON{0.0001f}; /** Delimiter segmenting two fully formed Statsd messages */ static constexpr char STATSD_MSG_DELIMITER{'\n'}; +/** Delimiter segmenting namespaces in a Statsd key */ +static constexpr char STATSD_NS_DELIMITER{'.'}; /** Character used to denote Statsd message type as count */ static constexpr char STATSD_METRIC_COUNT[]{"c"}; /** Character used to denote Statsd message type as gauge */ @@ -76,21 +78,41 @@ std::unique_ptr InitStatsClient(const ArgsManager& args) is_enabled = true; } + auto sanitize_string = [](std::string& string) { + // Remove key delimiters from the front and back as they're added back + // in formatting + if (!string.empty()) { + if (string.front() == STATSD_NS_DELIMITER) string.erase(string.begin()); + if (string.back() == STATSD_NS_DELIMITER) string.pop_back(); + } + }; + + // Get our suffix and if we get nothing, try again with the deprecated + // argument. If we still get nothing, that's fine, suffixes are optional. + auto suffix = args.GetArg("-statssuffix", DEFAULT_STATSD_SUFFIX); + if (suffix.empty()) { + suffix = args.GetArg("-statshostname", DEFAULT_STATSD_SUFFIX); + } else { + // We restrict sanitization logic to our newly added arguments to + // prevent breaking changes. + sanitize_string(suffix); + } + return std::make_unique( 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), + suffix, 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}, - m_ns{ns} + const std::string& ns, const std::string& suffix, bool enabled) : + m_ns{ns}, + m_suffix{[suffix]() { return !suffix.empty() ? STATSD_NS_DELIMITER + suffix : suffix; }()} { if (!enabled) { LogPrintf("Transmitting stats are disabled, will not init StatsdClient\n"); @@ -154,11 +176,8 @@ bool StatsdClient::send(const std::string& key, T1 value, const std::string& typ return true; } - // partition stats by node name if set - if (!m_nodename.empty()) key = key + "." + m_nodename; - // Construct the message and if our message isn't always-send, report the sample rate - RawMessage msg{strprintf("%s%s:%f|%s", m_ns, key, value, type)}; + RawMessage msg{strprintf("%s%s%s:%f|%s", m_ns, key, m_suffix, value, type)}; if (!always_send) { msg += strprintf("|@%.2f", sample_rate); } diff --git a/src/stats/client.h b/src/stats/client.h index c06c8dda05..729d4dc5a8 100644 --- a/src/stats/client.h +++ b/src/stats/client.h @@ -20,10 +20,10 @@ class RawSender; static constexpr uint16_t DEFAULT_STATSD_PORT{8125}; /** Default host assumed to be running a Statsd server */ static const std::string DEFAULT_STATSD_HOST{"127.0.0.1"}; -/** Default suffix appended to Statsd message keys */ -static const std::string DEFAULT_STATSD_HOSTNAME{""}; /** Default prefix prepended to Statsd message keys */ static const std::string DEFAULT_STATSD_NAMESPACE{""}; +/** Default suffix appended to Statsd message keys */ +static const std::string DEFAULT_STATSD_SUFFIX{""}; /** Default number of milliseconds between flushing a queue of messages */ static constexpr int DEFAULT_STATSD_DURATION{1000}; @@ -40,7 +40,7 @@ class StatsdClient { public: explicit 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); + const std::string& ns, const std::string& suffix, bool enabled); ~StatsdClient(); public: @@ -69,9 +69,9 @@ private: std::unique_ptr m_sender{nullptr}; /* Phrase prepended to keys */ - const std::string m_nodename; - /* Phrase appended to keys */ const std::string m_ns; + /* Phrase appended to keys */ + const std::string m_suffix{""}; }; /** Parses arguments and constructs a StatsdClient instance */