stats: cleanup error logging, improve code sanity

- Use `uint16_t` instead of `short`, `int64_t` instead of `size_t`
- Get rid of the `errmsg` buffer and use `LogPrintf` to report errors
- Use `strprintf` instead of `snprintf`
- Rephrase networking error logs to allow inclusion of error strings
This commit is contained in:
Kittywhiskers Van Gogh 2024-09-11 14:33:55 +00:00
parent 85890ddb13
commit 840241eefd
No known key found for this signature in database
GPG Key ID: 30CD0C065E5C4AAD
3 changed files with 38 additions and 57 deletions

View File

@ -65,7 +65,7 @@ inline bool should_send(float sample_rate)
return sample_rate > p; return sample_rate > p;
} }
StatsdClient::StatsdClient(const std::string& host, const std::string& nodename, short port, const std::string& ns, StatsdClient::StatsdClient(const std::string& host, const std::string& nodename, uint16_t port, const std::string& ns,
bool enabled) bool enabled)
: m_enabled{enabled}, m_port{port}, m_host{host}, m_nodename{nodename}, m_ns{ns} : m_enabled{enabled}, m_port{port}, m_host{host}, m_nodename{nodename}, m_ns{ns}
{ {
@ -79,11 +79,11 @@ StatsdClient::~StatsdClient()
int StatsdClient::init() int StatsdClient::init()
{ {
if ( m_init ) return 0; if (m_init) return 0;
m_sock = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP); m_sock = ::socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP);
if ( m_sock == INVALID_SOCKET ) { if (m_sock == INVALID_SOCKET) {
snprintf(m_errmsg, sizeof(m_errmsg), "could not create socket, err=%m"); LogPrintf("ERROR: Cannot create socket (socket() returned error %s)\n", NetworkErrorString(WSAGetLastError()));
return -1; return -1;
} }
@ -93,7 +93,7 @@ int StatsdClient::init()
CNetAddr netaddr(m_server.sin_addr); CNetAddr netaddr(m_server.sin_addr);
if (!LookupHost(m_host, netaddr, true) || !netaddr.GetInAddr(&m_server.sin_addr)) { if (!LookupHost(m_host, netaddr, true) || !netaddr.GetInAddr(&m_server.sin_addr)) {
snprintf(m_errmsg, sizeof(m_errmsg), "LookupHost or GetInAddr failed"); LogPrintf("ERROR: LookupHost or GetInAddr failed\n");
return -2; return -2;
} }
@ -104,8 +104,8 @@ int StatsdClient::init()
/* will change the original string */ /* will change the original string */
void StatsdClient::cleanup(std::string& key) void StatsdClient::cleanup(std::string& key)
{ {
size_t pos = key.find_first_of(":|@"); auto pos = key.find_first_of(":|@");
while ( pos != std::string::npos ) while (pos != std::string::npos)
{ {
key[pos] = '_'; key[pos] = '_';
pos = key.find_first_of(":|@"); pos = key.find_first_of(":|@");
@ -122,12 +122,12 @@ int StatsdClient::inc(const std::string& key, float sample_rate)
return count(key, 1, sample_rate); return count(key, 1, sample_rate);
} }
int StatsdClient::count(const std::string& key, size_t value, float sample_rate) int StatsdClient::count(const std::string& key, int64_t value, float sample_rate)
{ {
return send(key, value, "c", sample_rate); return send(key, value, "c", sample_rate);
} }
int StatsdClient::gauge(const std::string& key, size_t value, float sample_rate) int StatsdClient::gauge(const std::string& key, int64_t value, float sample_rate)
{ {
return send(key, value, "g", sample_rate); return send(key, value, "g", sample_rate);
} }
@ -137,12 +137,12 @@ int StatsdClient::gaugeDouble(const std::string& key, double value, float sample
return sendDouble(key, value, "g", sample_rate); return sendDouble(key, value, "g", sample_rate);
} }
int StatsdClient::timing(const std::string& key, size_t ms, float sample_rate) int StatsdClient::timing(const std::string& key, int64_t ms, float sample_rate)
{ {
return send(key, ms, "ms", sample_rate); return send(key, ms, "ms", sample_rate);
} }
int StatsdClient::send(std::string key, size_t value, const std::string& type, 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 (!should_send(sample_rate)) {
return 0; return 0;
@ -154,16 +154,11 @@ int StatsdClient::send(std::string key, size_t value, const std::string& type, f
cleanup(key); cleanup(key);
char buf[256]; std::string buf{""};
if ( fequal( sample_rate, 1.0 ) ) if (fequal(sample_rate, 1.0)) {
{ buf = strprintf("%s%s:%d|%s", m_ns, key, value, type);
snprintf(buf, sizeof(buf), "%s%s:%zd|%s", } else {
m_ns.c_str(), key.c_str(), (ssize_t) value, type.c_str()); buf = strprintf("%s%s:%d|%s|@%.2f", m_ns, key, value, type, sample_rate);
}
else
{
snprintf(buf, sizeof(buf), "%s%s:%zd|%s|@%.2f",
m_ns.c_str(), key.c_str(), (ssize_t) value, type.c_str(), sample_rate);
} }
return send(buf); return send(buf);
@ -181,16 +176,11 @@ int StatsdClient::sendDouble(std::string key, double value, const std::string& t
cleanup(key); cleanup(key);
char buf[256]; std::string buf{""};
if ( fequal( sample_rate, 1.0 ) ) if (fequal(sample_rate, 1.0)) {
{ buf = strprintf("%s%s:%f|%s", m_ns, key, value, type);
snprintf(buf, sizeof(buf), "%s%s:%f|%s", } else {
m_ns.c_str(), key.c_str(), value, type.c_str()); buf = strprintf("%s%s:%f|%s|@%.2f", m_ns, key, value, type, sample_rate);
}
else
{
snprintf(buf, sizeof(buf), "%s%s:%f|%s|@%.2f",
m_ns.c_str(), key.c_str(), value, type.c_str(), sample_rate);
} }
return send(buf); return send(buf);
@ -202,21 +192,18 @@ int StatsdClient::send(const std::string& message)
return -3; return -3;
int ret = init(); int ret = init();
if ( ret ) if (ret) {
{
return ret; return ret;
} }
ret = sendto(m_sock, message.data(), message.size(), 0, reinterpret_cast<const sockaddr*>(&m_server), sizeof(m_server));
if ( ret == -1) { ret = ::sendto(m_sock, message.data(), message.size(), 0, reinterpret_cast<const sockaddr*>(&m_server),
snprintf(m_errmsg, sizeof(m_errmsg), sizeof(m_server));
"sendto server fail, host=%s:%d, err=%m", m_host.c_str(), m_port); if (ret == -1) {
return -1; LogPrintf("ERROR: Unable to send message (sendto() returned error %s), host=%s:%d\n",
NetworkErrorString(WSAGetLastError()), m_host, m_port);
return ret;
} }
return 0; return 0;
} }
const char* StatsdClient::errmsg()
{
return m_errmsg;
}
} // namespace statsd } // namespace statsd

View File

@ -12,7 +12,7 @@
#include <memory> #include <memory>
static const bool DEFAULT_STATSD_ENABLE = false; static const bool DEFAULT_STATSD_ENABLE = false;
static const int DEFAULT_STATSD_PORT = 8125; static const uint16_t DEFAULT_STATSD_PORT = 8125;
static const std::string DEFAULT_STATSD_HOST = "127.0.0.1"; static const std::string DEFAULT_STATSD_HOST = "127.0.0.1";
static const std::string DEFAULT_STATSD_HOSTNAME = ""; static const std::string DEFAULT_STATSD_HOSTNAME = "";
static const std::string DEFAULT_STATSD_NAMESPACE = ""; static const std::string DEFAULT_STATSD_NAMESPACE = "";
@ -25,20 +25,17 @@ static const int MAX_STATSD_PERIOD = 60 * 60;
namespace statsd { namespace statsd {
class StatsdClient { class StatsdClient {
public: public:
explicit StatsdClient(const std::string& host, const std::string& nodename, short port, const std::string& ns, explicit StatsdClient(const std::string& host, const std::string& nodename, uint16_t port, const std::string& ns,
bool enabled); bool enabled);
~StatsdClient(); ~StatsdClient();
public:
const char* errmsg();
public: public:
int inc(const std::string& key, float sample_rate = 1.0); int inc(const std::string& key, float sample_rate = 1.0);
int dec(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, size_t value, 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, size_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 gaugeDouble(const std::string& key, double value, float sample_rate = 1.0);
int timing(const std::string& key, size_t ms, float sample_rate = 1.0); int timing(const std::string& key, int64_t ms, float sample_rate = 1.0);
public: public:
/** /**
@ -50,7 +47,7 @@ class StatsdClient {
/* (Low Level Api) manually send a message /* (Low Level Api) manually send a message
* type = "c", "g" or "ms" * type = "c", "g" or "ms"
*/ */
int send(std::string key, size_t value, int send(std::string key, int64_t value,
const std::string& type, float sample_rate); const std::string& type, float sample_rate);
int sendDouble(std::string key, double value, int sendDouble(std::string key, double value,
const std::string& type, float sample_rate); const std::string& type, float sample_rate);
@ -61,12 +58,11 @@ class StatsdClient {
private: private:
bool m_init{false}; bool m_init{false};
char m_errmsg[1024];
SOCKET m_sock{INVALID_SOCKET}; SOCKET m_sock{INVALID_SOCKET};
struct sockaddr_in m_server; struct sockaddr_in m_server;
const bool m_enabled{false}; const bool m_enabled{false};
const short m_port; const uint16_t m_port;
const std::string m_host; const std::string m_host;
const std::string m_nodename; const std::string m_nodename;
const std::string m_ns; const std::string m_ns;

View File

@ -24,8 +24,6 @@ FALSE_POSITIVES = [
("src/qt/networkstyle.cpp", "strprintf(titleAddText, gArgs.GetDevNetName())"), ("src/qt/networkstyle.cpp", "strprintf(titleAddText, gArgs.GetDevNetName())"),
("src/rpc/evo.cpp", "strprintf(it->second, nParamNum)"), ("src/rpc/evo.cpp", "strprintf(it->second, nParamNum)"),
("src/stacktraces.cpp", "strprintf(fmtStr, i, si.pc, lstr, fstr)"), ("src/stacktraces.cpp", "strprintf(fmtStr, i, si.pc, lstr, fstr)"),
("src/statsd_client.cpp", "snprintf(d->errmsg, sizeof(d->errmsg), \"could not create socket, err=%m\")"),
("src/statsd_client.cpp", "snprintf(d->errmsg, sizeof(d->errmsg), \"sendto server fail, host=%s:%d, err=%m\", d->host.c_str(), d->port)"),
("src/util/system.cpp", "strprintf(_(COPYRIGHT_HOLDERS).translated, COPYRIGHT_HOLDERS_SUBSTITUTION)"), ("src/util/system.cpp", "strprintf(_(COPYRIGHT_HOLDERS).translated, COPYRIGHT_HOLDERS_SUBSTITUTION)"),
("src/validationinterface.cpp", "LogPrint(BCLog::VALIDATION, fmt \"\\n\", __VA_ARGS__)"), ("src/validationinterface.cpp", "LogPrint(BCLog::VALIDATION, fmt \"\\n\", __VA_ARGS__)"),
("src/wallet/wallet.h", "WalletLogPrintf(std::string fmt, Params... parameters)"), ("src/wallet/wallet.h", "WalletLogPrintf(std::string fmt, Params... parameters)"),