From 4bc727cd6c235415f63a076f0377043959f10b60 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Thu, 5 Sep 2024 20:42:14 +0000 Subject: [PATCH] stats: clean up randomization code, move `FastRandomContext` inward We can skip the computationally expensive dice-roll if our sample rate is zero as that means we should never send it. Also get rid of the `FastRandomContext::operator()` that isn't used anywhere else in the codebase. --- src/random.h | 4 ---- src/statsd_client.cpp | 46 ++++++++++++++++++------------------------- src/statsd_client.h | 20 +++++++++++++------ 3 files changed, 33 insertions(+), 37 deletions(-) diff --git a/src/random.h b/src/random.h index 71cdbf4a11..7f3b903622 100644 --- a/src/random.h +++ b/src/random.h @@ -207,10 +207,6 @@ public: return rand32() % nMax; } - uint32_t operator()(uint32_t nMax) { - return rand32(nMax); - } - /** Generate random bytes. */ template std::vector randbytes(size_t len); diff --git a/src/statsd_client.cpp b/src/statsd_client.cpp index 81f0aac5d3..81056cd357 100644 --- a/src/statsd_client.cpp +++ b/src/statsd_client.cpp @@ -37,32 +37,28 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include #include -#include #include #include #include +#include std::unique_ptr g_stats_client; namespace statsd { -inline bool fequal(float a, float b) +bool StatsdClient::ShouldSend(float sample_rate) { - const float epsilon = 0.0001; - return ( fabs(a - b) < epsilon ); -} + sample_rate = std::clamp(sample_rate, 0.f, 1.f); -thread_local FastRandomContext insecure_rand; + constexpr float EPSILON{0.0001f}; + /* If sample rate is 1, we should always send */ + if (std::fabs(sample_rate - 1.f) < EPSILON) return true; + /* If sample rate is 0, we should never send */ + if (std::fabs(sample_rate) < EPSILON) return false; -inline bool should_send(float sample_rate) -{ - if ( fequal(sample_rate, 1.0) ) - { - return true; - } - - float p = float(insecure_rand(std::numeric_limits::max())) / float(std::numeric_limits::max()); - return sample_rate > p; + /* Sample rate is >0 and <1, roll the dice */ + LOCK(cs); + return sample_rate > std::uniform_real_distribution(0.f, 1.f)(insecure_rand); } StatsdClient::StatsdClient(const std::string& host, const std::string& nodename, uint16_t port, const std::string& ns, @@ -144,7 +140,7 @@ int StatsdClient::timing(const std::string& key, int64_t ms, float sample_rate) int StatsdClient::send(std::string key, int64_t value, const std::string& type, float sample_rate) { - if (!should_send(sample_rate)) { + if (!ShouldSend(sample_rate)) { return 0; } @@ -154,11 +150,9 @@ int StatsdClient::send(std::string key, int64_t value, const std::string& type, cleanup(key); - std::string buf{""}; - if (fequal(sample_rate, 1.0)) { - buf = strprintf("%s%s:%d|%s", m_ns, key, value, type); - } else { - buf = strprintf("%s%s:%d|%s|@%.2f", m_ns, key, value, type, sample_rate); + std::string buf{strprintf("%s%s:%d|%s", m_ns, key, value, type)}; + if (sample_rate < 1.f) { + buf += strprintf("|@%.2f", sample_rate); } return send(buf); @@ -166,7 +160,7 @@ int StatsdClient::send(std::string key, int64_t value, const std::string& type, int StatsdClient::sendDouble(std::string key, double value, const std::string& type, float sample_rate) { - if (!should_send(sample_rate)) { + if (!ShouldSend(sample_rate)) { return 0; } @@ -176,11 +170,9 @@ int StatsdClient::sendDouble(std::string key, double value, const std::string& t cleanup(key); - std::string buf{""}; - if (fequal(sample_rate, 1.0)) { - buf = strprintf("%s%s:%f|%s", m_ns, key, value, type); - } else { - buf = strprintf("%s%s:%f|%s|@%.2f", m_ns, key, value, type, sample_rate); + std::string buf{strprintf("%s%s:%f|%s", m_ns, key, value, type)}; + if (sample_rate < 1.f) { + buf += strprintf("|@%.2f", sample_rate); } return send(buf); diff --git a/src/statsd_client.h b/src/statsd_client.h index ec24e2f5b1..20d87f6bec 100644 --- a/src/statsd_client.h +++ b/src/statsd_client.h @@ -6,6 +6,8 @@ #ifndef BITCOIN_STATSD_CLIENT_H #define BITCOIN_STATSD_CLIENT_H +#include +#include #include #include @@ -30,12 +32,12 @@ class StatsdClient { ~StatsdClient(); public: - int inc(const std::string& key, float sample_rate = 1.0); - int dec(const std::string& key, float sample_rate = 1.0); - int count(const std::string& key, int64_t value, float sample_rate = 1.0); - int gauge(const std::string& key, int64_t value, float sample_rate = 1.0); - int gaugeDouble(const std::string& key, double value, float sample_rate = 1.0); - int timing(const std::string& key, int64_t ms, float sample_rate = 1.0); + int inc(const std::string& key, float sample_rate = 1.f); + int dec(const std::string& key, float sample_rate = 1.f); + int count(const std::string& key, int64_t value, float sample_rate = 1.f); + int gauge(const std::string& key, int64_t value, float sample_rate = 1.f); + int gaugeDouble(const std::string& key, double value, float sample_rate = 1.f); + int timing(const std::string& key, int64_t ms, float sample_rate = 1.f); public: /** @@ -57,6 +59,12 @@ class StatsdClient { static void cleanup(std::string& key); private: + bool ShouldSend(float sample_rate); + + private: + mutable Mutex cs; + mutable FastRandomContext insecure_rand GUARDED_BY(cs); + bool m_init{false}; SOCKET m_sock{INVALID_SOCKET}; struct sockaddr_in m_server;