diff --git a/include/cpp-statsd-client/StatsdClient.hpp b/include/cpp-statsd-client/StatsdClient.hpp index 9c2e933..4c3909e 100644 --- a/include/cpp-statsd-client/StatsdClient.hpp +++ b/include/cpp-statsd-client/StatsdClient.hpp @@ -90,6 +90,9 @@ class StatsdClient { //! The random number generator for handling sampling mutable std::mt19937 m_randomEngine; + + //! The buffer string format our stats before sending them + mutable std::string m_buffer; }; namespace detail { @@ -109,6 +112,8 @@ inline StatsdClient::StatsdClient(const std::string& host, : m_prefix(detail::sanitizePrefix(prefix)), m_sender(new UDPSender{host, port, batchsize}) { // Initialize the random generator to be used for sampling seed(); + // Avoid re-allocations by reserving a generous buffer + m_buffer.reserve(256); } inline void StatsdClient::setConfig(const std::string& host, @@ -165,39 +170,27 @@ inline void StatsdClient::send(const std::string& key, return; } - // Prepare the buffer and include the sampling rate if it's not 1.f - const bool needs_dot = !m_prefix.empty() && !key.empty(); - std::string buffer(256, '\0'); - int string_len = -1; - if (isFrequencyOne) { - // Sampling rate is 1.0f, no need to specify it - string_len = std::snprintf(&buffer.front(), - buffer.size(), - "%s%s%s:%d|%s", - m_prefix.c_str(), - needs_dot ? "." : "", - key.c_str(), - value, - type.c_str()); - } else { - // Sampling rate is different from 1.0f, hence specify it - string_len = std::snprintf(&buffer.front(), - buffer.size(), - "%s%s%s:%d|%s|@%.2f", - m_prefix.c_str(), - needs_dot ? "." : "", - key.c_str(), - value, - type.c_str(), - frequency); + // Format the stat message + m_buffer.clear(); + + m_buffer.append(m_prefix); + if (!m_prefix.empty() && !key.empty()) { + m_buffer.push_back('.'); } - // Trim the trailing null chars - // TODO: perf improvement: send the len along and keep the buffer around as a member variable - buffer.resize(std::min(string_len, static_cast(buffer.size()))); + m_buffer.append(key); + m_buffer.push_back(':'); + m_buffer.append(std::to_string(value)); + m_buffer.push_back('|'); + m_buffer.append(type); + + if (frequency < 1.f) { + m_buffer.append("|@0."); + m_buffer.append(std::to_string(static_cast(frequency * 100))); + } // Send the message via the UDP sender - m_sender->send(buffer); + m_sender->send(m_buffer); } inline void StatsdClient::seed(unsigned int seed) noexcept { diff --git a/include/cpp-statsd-client/UDPSender.hpp b/include/cpp-statsd-client/UDPSender.hpp index 7c0b6da..d511960 100644 --- a/include/cpp-statsd-client/UDPSender.hpp +++ b/include/cpp-statsd-client/UDPSender.hpp @@ -41,7 +41,7 @@ class UDPSender final { //! Send a message void send(const std::string& message) noexcept; - //! Returns the error message as an string + //! Returns the error message as a string const std::string& errorMessage() const noexcept; //! Returns true if the sender is initialized @@ -56,6 +56,9 @@ class UDPSender final { //! Initialize the sender and returns true when it is initialized bool initialize() noexcept; + //! Queue a message to be sent to the daemon later + inline void queueMessage(const std::string& message) noexcept; + //! Send a message to the daemon void sendToDaemon(const std::string& message) noexcept; @@ -170,15 +173,25 @@ inline void UDPSender::send(const std::string& message) noexcept { m_errorMessage.clear(); // If batching is on, accumulate messages in the queue if (m_batching) { - std::unique_lock batchingLock(m_batchingMutex); - if (m_batchingMessageQueue.empty() || m_batchingMessageQueue.back().length() > m_batchsize) { - m_batchingMessageQueue.push_back(message); - } else { - m_batchingMessageQueue.rbegin()->append("\n").append(message); - } - } else { - sendToDaemon(message); + queueMessage(message); + return; + } + + sendToDaemon(message); +} + +inline void UDPSender::queueMessage(const std::string& message) noexcept { + std::unique_lock batchingLock(m_batchingMutex); + // Either we don't have a place to batch our message or we exceeded the batch size, so make a new batch + if (m_batchingMessageQueue.empty() || m_batchingMessageQueue.back().length() > m_batchsize) { + m_batchingMessageQueue.emplace_back(); + m_batchingMessageQueue.back().reserve(m_batchsize + 256); + } // When there is already a batch open we need a separator when its not empty + else if (!m_batchingMessageQueue.back().empty()) { + m_batchingMessageQueue.back().push_back('\n'); } + // Add the new message to the batch + m_batchingMessageQueue.back().append(message); } inline const std::string& UDPSender::errorMessage() const noexcept { @@ -189,7 +202,7 @@ inline bool UDPSender::initialize() noexcept { // Connect the socket m_socket = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP); if (m_socket == k_invalidFd) { - m_errorMessage = std::string("Could not create socket, err=") + std::strerror(errno); + m_errorMessage = std::string("socket creation failed: err=") + std::strerror(errno); return false; } @@ -213,7 +226,7 @@ inline bool UDPSender::initialize() noexcept { // An error code has been returned by getaddrinfo close(m_socket); m_socket = k_invalidFd; - m_errorMessage = "getaddrinfo failed: error=" + std::to_string(ret) + ", msg=" + gai_strerror(ret); + m_errorMessage = "getaddrinfo failed: err=" + std::to_string(ret) + ", msg=" + gai_strerror(ret); return false; } diff --git a/tests/testStatsdClient.cpp b/tests/testStatsdClient.cpp index 08f3975..0e40518 100644 --- a/tests/testStatsdClient.cpp +++ b/tests/testStatsdClient.cpp @@ -31,21 +31,17 @@ void mock(StatsdServer& server, std::vector& messages) { } while (server.errorMessage().empty() && !messages.back().empty()); } -// TODO: would it be ok if we change the API of the client to throw on error instead of having to check a string? -void throwOnError(const StatsdClient& client) { - if (!client.errorMessage().empty()) { - std::cerr << client.errorMessage() << std::endl; - throw std::runtime_error(client.errorMessage()); +void throwOnError(const StatsdClient& client, bool expectEmpty = true, const std::string& extraMessage = "") { + if (client.errorMessage().empty() != expectEmpty) { + std::cerr << (expectEmpty ? client.errorMessage() : extraMessage) << std::endl; + throw std::runtime_error(expectEmpty ? client.errorMessage() : extraMessage); } } void testErrorConditions() { - // Connect to a rubbish ip and make sure initialization failed - StatsdClient client{"256.256.256.256", 8125, "myPrefix.", 20}; - if (client.errorMessage().empty()) { - std::cerr << "Should not be able to connect to ridiculous ip" << std::endl; - throw std::runtime_error("Should not be able to connect to ridiculous ip"); - } + // Resolve a rubbish ip and make sure initialization failed + StatsdClient client{"256.256.256.256", 8125, "myPrefix", 20}; + throwOnError(client, false, "Should not be able to resolve a ridiculous ip"); } void testReconfigure() {