Merge #6505: trivial(stats): remove deprecated arguments `-stats{enab…

…led,hostname,ns}`

d673ede stats: add release notes for removal of deprecated arguments (Kittywhiskers Van Gogh)
59593d7 stats: remove deprecated `-statshostname` argument (Kittywhiskers Van Gogh)
c0d5d33 stats: remove deprecated `-statsns` argument (Kittywhiskers Van Gogh)
2d55a9c stats: remove deprecated `-statsenabled` argument (Kittywhiskers Van Gogh)

Pull request description:

  ## Breaking Changes

  - The arguments `-statsenabled`, `-statsns`, `-statshostname` have been removed. They were previously deprecated in v22.0 and will no longer be recognized on runtime.

  ## Checklist

  - [x] I have performed a self-review of my own code
  - [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)**
  - [x] I have added or updated relevant unit/integration/functional/e2e tests **(note: N/A)**
  - [x] I have made corresponding changes to the documentation
  - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  UdjinM6:
    utACK d673ede
  PastaPastaPasta:
    utACK d673ede

Tree-SHA512: b3ee6e210b3a9288bf3016731f7206f8a4c5ae87734f2771780a8693cb0371d49644e2e3b6d8fa7a44bf8cbc710aa0a3bddca0ec0816087109f569bde9dd37f6

doc/release-notes-6505.md

@@ -0,0 +1,5 @@
+Statistics
+-----------
+
+- The arguments `-statsenabled`, `-statsns`, `-statshostname` have been removed. They were
+  previously deprecated in v22.0 and will no longer be recognized on runtime.

src/init.cpp

@@ -754,13 +754,10 @@ 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);
 
-    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);
-    hidden_args.emplace_back("-statshostname");
     argsman.AddArg("-statsport=<port>", strprintf("Specify statsd port (default: %u)", DEFAULT_STATSD_PORT), ArgsManager::ALLOW_ANY, OptionsCategory::STATSD);
-    hidden_args.emplace_back("-statsns");
     argsman.AddArg("-statsperiod=<seconds>", strprintf("Specify the number of seconds between periodic measurements (default: %d)", DEFAULT_STATSD_PERIOD), ArgsManager::ALLOW_ANY, OptionsCategory::STATSD);
     argsman.AddArg("-statsprefix=<string>", strprintf("Specify an optional string prepended to every stats key (default: %s)", DEFAULT_STATSD_PREFIX), ArgsManager::ALLOW_ANY, OptionsCategory::STATSD);
     argsman.AddArg("-statssuffix=<string>", strprintf("Specify an optional string appended to every stats key (default: %s)", DEFAULT_STATSD_SUFFIX), ArgsManager::ALLOW_ANY, OptionsCategory::STATSD);

src/stats/client.cpp

@@ -33,72 +33,30 @@ 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=*/DEFAULT_STATSD_HOST);
-
-    if (is_enabled && host.empty()) {
-        // Stats are enabled but host has not been specified, then use
-        // default legacy host. This is to preserve old behavior.
-        host = "127.0.0.1";
-    } 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;
-    }
-
-    auto sanitize_string = [](std::string& string) {
-        // Remove key delimiters from the front and back as they're added back
+    auto sanitize_string = [](std::string string) {
+        // Remove key delimiters from the front and back as they're added back in
+        // the constructor
         if (!string.empty()) {
             if (string.front() == STATSD_NS_DELIMITER) string.erase(string.begin());
             if (string.back() == STATSD_NS_DELIMITER) string.pop_back();
         }
+        return string;
     };
 
-    // 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);
-    } else {
-        // We restrict sanitization logic to our newly added arguments to
-        // prevent breaking changes.
-        sanitize_string(suffix);
-    }
-
-    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),
-        prefix,
-        suffix,
-        is_enabled
-    );
+    return std::make_unique<StatsdClient>(args.GetArg("-statshost", DEFAULT_STATSD_HOST),
+                                          args.GetArg("-statsport", DEFAULT_STATSD_PORT),
+                                          args.GetArg("-statsbatchsize", DEFAULT_STATSD_BATCH_SIZE),
+                                          args.GetArg("-statsduration", DEFAULT_STATSD_DURATION),
+                                          sanitize_string(args.GetArg("-statsprefix", DEFAULT_STATSD_PREFIX)),
+                                          sanitize_string(args.GetArg("-statssuffix", DEFAULT_STATSD_SUFFIX)));
 }
 
 StatsdClient::StatsdClient(const std::string& host, uint16_t port, uint64_t batch_size, uint64_t interval_ms,
-                           const std::string& prefix, const std::string& suffix, bool enabled) :
-    m_prefix{prefix},
+                           const std::string& prefix, const std::string& suffix) :
+    m_prefix{[prefix]() { return !prefix.empty() ? prefix + STATSD_NS_DELIMITER : prefix; }()},
     m_suffix{[suffix]() { return !suffix.empty() ? STATSD_NS_DELIMITER + suffix : suffix; }()}
 {
-    if (!enabled) {
+    if (host.empty()) {
         LogPrintf("Transmitting stats are disabled, will not init StatsdClient\n");
         return;
     }

src/stats/client.h

@@ -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& prefix, const std::string& suffix, bool enabled);
+                          const std::string& prefix, const std::string& suffix);
     ~StatsdClient();
 
 public: