From 18a2e48eb93f9810d0dff5ea7fe3bb96104a9ce6 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Tue, 10 Sep 2024 18:52:28 +0000 Subject: [PATCH] stats: rename `statsns` to clearer `statsprefix` `statsns`, aside from being an unclear name, in its default behaviour, doesn't use the namespace delimiter contaminates the root namespace of the key. Let's take care of that by deprecating `statsns` with its replacement, `statsprefix`, that behaves properly. --- src/init.cpp | 3 ++- src/stats/client.cpp | 28 +++++++++++++++++++++------- src/stats/client.h | 6 +++--- 3 files changed, 26 insertions(+), 11 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 6b1f895427..b43f04f345 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -777,8 +777,9 @@ void SetupServerArgs(ArgsManager& argsman) argsman.AddArg("-statshost=", strprintf("Specify statsd host (default: %s)", DEFAULT_STATSD_HOST), 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_SUFFIX), ArgsManager::ALLOW_ANY, OptionsCategory::STATSD); + hidden_args.emplace_back("-statsns"); argsman.AddArg("-statsperiod=", strprintf("Specify the number of seconds between periodic measurements (default: %d)", DEFAULT_STATSD_PERIOD), ArgsManager::ALLOW_ANY, OptionsCategory::STATSD); + argsman.AddArg("-statsprefix=", strprintf("Specify an optional string prepended to every stats key (default: %s)", DEFAULT_STATSD_PREFIX), 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); diff --git a/src/stats/client.cpp b/src/stats/client.cpp index 865c05bb41..90bdcbdbee 100644 --- a/src/stats/client.cpp +++ b/src/stats/client.cpp @@ -80,15 +80,29 @@ std::unique_ptr InitStatsClient(const ArgsManager& args) 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. + // Get our prefix and suffix and if we get nothing, try again with the + // deprecated argument. If we still get nothing, that's fine, they're optional. + auto prefix = args.GetArg("-statsprefix", DEFAULT_STATSD_PREFIX); + if (prefix.empty()) { + prefix = args.GetArg("-statsns", DEFAULT_STATSD_PREFIX); + } else { + // We restrict sanitization logic to our newly added arguments to + // prevent breaking changes. + sanitize_string(prefix); + // We need to add the delimiter here for backwards compatibility with + // the deprecated argument. + // + // TODO: Move this step into the constructor when removing deprecated + // args support + prefix += STATSD_NS_DELIMITER; + } + auto suffix = args.GetArg("-statssuffix", DEFAULT_STATSD_SUFFIX); if (suffix.empty()) { suffix = args.GetArg("-statshostname", DEFAULT_STATSD_SUFFIX); @@ -103,15 +117,15 @@ std::unique_ptr InitStatsClient(const ArgsManager& args) args.GetArg("-statsport", DEFAULT_STATSD_PORT), args.GetArg("-statsbatchsize", DEFAULT_STATSD_BATCH_SIZE), args.GetArg("-statsduration", DEFAULT_STATSD_DURATION), - args.GetArg("-statsns", DEFAULT_STATSD_NAMESPACE), + prefix, suffix, is_enabled ); } StatsdClient::StatsdClient(const std::string& host, uint16_t port, uint64_t batch_size, uint64_t interval_ms, - const std::string& ns, const std::string& suffix, bool enabled) : - m_ns{ns}, + const std::string& prefix, const std::string& suffix, bool enabled) : + m_prefix{prefix}, m_suffix{[suffix]() { return !suffix.empty() ? STATSD_NS_DELIMITER + suffix : suffix; }()} { if (!enabled) { @@ -177,7 +191,7 @@ bool StatsdClient::send(const std::string& key, T1 value, const std::string& typ } // Construct the message and if our message isn't always-send, report the sample rate - RawMessage msg{strprintf("%s%s%s:%f|%s", m_ns, key, m_suffix, value, type)}; + RawMessage msg{strprintf("%s%s%s:%f|%s", m_prefix, 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 729d4dc5a8..0fa8538260 100644 --- a/src/stats/client.h +++ b/src/stats/client.h @@ -21,7 +21,7 @@ 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 prefix prepended to Statsd message keys */ -static const std::string DEFAULT_STATSD_NAMESPACE{""}; +static const std::string DEFAULT_STATSD_PREFIX{""}; /** Default suffix appended to Statsd message keys */ static const std::string DEFAULT_STATSD_SUFFIX{""}; @@ -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& ns, const std::string& suffix, bool enabled); + const std::string& prefix, const std::string& suffix, bool enabled); ~StatsdClient(); public: @@ -69,7 +69,7 @@ private: std::unique_ptr m_sender{nullptr}; /* Phrase prepended to keys */ - const std::string m_ns; + const std::string m_prefix{""}; /* Phrase appended to keys */ const std::string m_suffix{""}; };