Skip to content

Commit

Permalink
stats: rename statsns to clearer statsprefix
Browse files Browse the repository at this point in the history
`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.
  • Loading branch information
kwvg committed Sep 13, 2024
1 parent c2ff5a2 commit abf6e2a
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 11 deletions.
3 changes: 2 additions & 1 deletion src/init.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -777,8 +777,9 @@ void SetupServerArgs(ArgsManager& argsman)
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);
argsman.AddArg("-statsns=<ns>", strprintf("Specify additional namespace prefix (default: %s)", DEFAULT_STATSD_SUFFIX), 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);
#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);
Expand Down
28 changes: 21 additions & 7 deletions src/stats/client.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -79,15 +79,29 @@ std::unique_ptr<StatsdClient> 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);
Expand All @@ -102,15 +116,15 @@ std::unique_ptr<StatsdClient> 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) {
Expand Down Expand Up @@ -174,7 +188,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);
}
Expand Down
6 changes: 3 additions & 3 deletions src/stats/client.h
Original file line number Diff line number Diff line change
Expand Up @@ -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{""};

Expand All @@ -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);

public:
/* Statsd-defined APIs */
Expand Down Expand Up @@ -68,7 +68,7 @@ class StatsdClient
std::unique_ptr<RawSender> 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{""};
};
Expand Down

0 comments on commit abf6e2a

Please sign in to comment.