From 7c84615738d1503182bd94d92529f9f1ba6b53e1 Mon Sep 17 00:00:00 2001 From: Otto Moerbeek Date: Wed, 28 Sep 2022 15:28:02 +0200 Subject: [PATCH] Process review commment from @fredmorcos, thanks! --- pdns/fstrm_logger.hh | 9 +++++---- pdns/rec_channel_rec.cc | 30 +++++++++++++++--------------- pdns/remote_logger.hh | 14 +++++++------- 3 files changed, 27 insertions(+), 26 deletions(-) diff --git a/pdns/fstrm_logger.hh b/pdns/fstrm_logger.hh index 7147a471c0..3eafb3d997 100644 --- a/pdns/fstrm_logger.hh +++ b/pdns/fstrm_logger.hh @@ -40,21 +40,22 @@ public: ~FrameStreamLogger(); [[nodiscard]] RemoteLoggerInterface::Result queueData(const std::string& data) override; - std::string address() const override + [[nodiscard]] std::string address() const override { return d_address; } - const std::string name() const override + [[nodiscard]] std::string name() const override { return "dnstap"; } - std::string toString() override + + [[nodiscard]] std::string toString() override { return "FrameStreamLogger to " + d_address + " (" + std::to_string(d_framesSent) + " frames sent, " + std::to_string(d_queueFullDrops) + " dropped, " + std::to_string(d_permanentFailures) + " permanent failures)"; } - RemoteLoggerInterface::Stats getStats() override + [[nodiscard]] RemoteLoggerInterface::Stats getStats() override { return Stats{.d_queued = d_framesSent, .d_pipeFull = d_queueFullDrops, diff --git a/pdns/rec_channel_rec.cc b/pdns/rec_channel_rec.cc index 149f68606b..cff6e03508 100644 --- a/pdns/rec_channel_rec.cc +++ b/pdns/rec_channel_rec.cc @@ -944,8 +944,8 @@ static RemoteLoggerStats_t* pleaseGetRemoteLoggerStats() auto ret = make_unique(); if (t_protobufServers) { - for (const auto& s : *t_protobufServers) { - ret->emplace(std::make_pair(s->address(), s->getStats())); + for (const auto& server : *t_protobufServers) { + ret->emplace(std::make_pair(server->address(), server->getStats())); } } return ret.release(); @@ -967,8 +967,8 @@ static RemoteLoggerStats_t* pleaseGetOutgoingRemoteLoggerStats() auto ret = make_unique(); if (t_outgoingProtobufServers) { - for (const auto& s : *t_outgoingProtobufServers) { - ret->emplace(std::make_pair(s->address(), s->getStats())); + for (const auto& server : *t_outgoingProtobufServers) { + ret->emplace(std::make_pair(server->address(), server->getStats())); } } return ret.release(); @@ -980,38 +980,38 @@ static RemoteLoggerStats_t* pleaseGetFramestreamLoggerStats() auto ret = make_unique(); if (t_frameStreamServersInfo.servers) { - for (const auto& s : *t_frameStreamServersInfo.servers) { - ret->emplace(std::make_pair(s->address(), s->getStats())); + for (const auto& server : *t_frameStreamServersInfo.servers) { + ret->emplace(std::make_pair(server->address(), server->getStats())); } } return ret.release(); } #endif -static void remoteLoggerStats(const string& type, const RemoteLoggerStats_t& stats, ostringstream& os) +static void remoteLoggerStats(const string& type, const RemoteLoggerStats_t& stats, ostringstream& outpustStream) { if (stats.empty()) { return; } for (const auto& [key, entry] : stats) { - os << entry.d_queued << '\t' << entry.d_pipeFull << '\t' << entry.d_tooLarge << '\t' << entry.d_otherError << '\t' << key << '\t' << type << endl; + outpustStream << entry.d_queued << '\t' << entry.d_pipeFull << '\t' << entry.d_tooLarge << '\t' << entry.d_otherError << '\t' << key << '\t' << type << endl; } } static string getRemoteLoggerStats() { - ostringstream os; - os << "Queued\tPipe-\tToo-\tOther-\tAddress\tType" << endl; - os << "\tFull\tLarge\terror" << endl; + ostringstream outputStream; + outputStream << "Queued\tPipe-\tToo-\tOther-\tAddress\tType" << endl; + outputStream << "\tFull\tLarge\terror" << endl; auto stats = broadcastAccFunction(pleaseGetRemoteLoggerStats); - remoteLoggerStats("protobuf", stats, os); + remoteLoggerStats("protobuf", stats, outputStream); stats = broadcastAccFunction(pleaseGetOutgoingRemoteLoggerStats); - remoteLoggerStats("outgoingProtobuf", stats, os); + remoteLoggerStats("outgoingProtobuf", stats, outputStream); #ifdef HAVE_FSTRM stats = broadcastAccFunction(pleaseGetFramestreamLoggerStats); - remoteLoggerStats("dnstapFrameStream", stats, os); + remoteLoggerStats("dnstapFrameStream", stats, outputStream); #endif - return os.str(); + return outputStream.str(); } static uint64_t calculateUptime() diff --git a/pdns/remote_logger.hh b/pdns/remote_logger.hh index 058b0c6c96..a5390f27ee 100644 --- a/pdns/remote_logger.hh +++ b/pdns/remote_logger.hh @@ -66,9 +66,9 @@ public: virtual ~RemoteLoggerInterface() {}; virtual Result queueData(const std::string& data) = 0; - virtual std::string address() const = 0; - virtual std::string toString() = 0; - virtual const std::string name() const = 0; + [[nodiscard]] virtual std::string address() const = 0; + [[nodiscard]] virtual std::string toString() = 0; + [[nodiscard]] virtual std::string name() const = 0; bool logQueries(void) const { return d_logQueries; } bool logResponses(void) const { return d_logResponses; } void setLogQueries(bool flag) { d_logQueries = flag; } @@ -91,7 +91,7 @@ public: } }; - virtual Stats getStats() = 0; + [[nodiscard]] virtual Stats getStats() = 0; private: bool d_logQueries{true}; @@ -118,17 +118,17 @@ public: } [[nodiscard]] Result queueData(const std::string& data) override; - const std::string name() const override + [[nodiscard]] std::string name() const override { return "protobuf"; } - std::string toString() override + [[nodiscard]] std::string toString() override { auto runtime = d_runtime.lock(); return d_remote.toStringWithPort() + " (" + std::to_string(runtime->d_stats.d_queued) + " processed, " + std::to_string(runtime->d_stats.d_pipeFull + runtime->d_stats.d_tooLarge + runtime->d_stats.d_otherError) + " dropped)"; } - virtual RemoteLoggerInterface::Stats getStats() override + [[nodiscard]] RemoteLoggerInterface::Stats getStats() override { return d_runtime.lock()->d_stats; } -- 2.47.2