From: Otto Moerbeek <otto.moerbeek@open-xchange.com>
Date: Mon, 29 Aug 2022 10:23:18 +0000 (+0200)
Subject: Implement a common interface to get stats for both RemoteLogger and FrameStreamLogger
X-Git-Tag: rec-4.9.0-alpha0~16^2~10
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=2b7a8236247e496d82326e2114b4ad4316267894;p=thirdparty%2Fpdns.git

Implement a common interface to get stats for both RemoteLogger and FrameStreamLogger

These two classes do not use the same mechanism to protect the stats from mult-threaded
access. Should maybe be revisited. Recursor has an instance per pthread, don't know
about dnsdist.
---

diff --git a/pdns/fstrm_logger.cc b/pdns/fstrm_logger.cc
index 1261953a80..0de4632036 100644
--- a/pdns/fstrm_logger.cc
+++ b/pdns/fstrm_logger.cc
@@ -161,10 +161,12 @@ FrameStreamLogger::~FrameStreamLogger()
 RemoteLoggerInterface::Result FrameStreamLogger::queueData(const std::string& data)
 {
   if (!d_ioqueue || !d_iothr) {
+    ++d_permanentFailures;
     return Result::OtherError;
   }
   uint8_t *frame = (uint8_t*)malloc(data.length());
   if (!frame) {
+    ++d_queueFullDrops; // XXX separate count?
     return Result::TooLarge;
   }
   memcpy(frame, data.c_str(), data.length());
diff --git a/pdns/fstrm_logger.hh b/pdns/fstrm_logger.hh
index 21ec298906..c9890fc52d 100644
--- a/pdns/fstrm_logger.hh
+++ b/pdns/fstrm_logger.hh
@@ -41,13 +41,22 @@ public:
   [[nodiscard]] RemoteLoggerInterface::Result queueData(const std::string& data) override;
   const std::string name() const override
   {
-    return "framestream";
+    return "dnstap";
   }
   std::string toString() const 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() const override
+  {
+    return Stats{.d_queued = d_framesSent,
+                 .d_pipeFull = d_queueFullDrops,
+                 .d_otherError = d_permanentFailures,
+                 .d_tooLarge = 0
+    };
+  }
+
 private:
 
   const int d_family;
diff --git a/pdns/remote_logger.cc b/pdns/remote_logger.cc
index 9616511699..e9e2efe39d 100644
--- a/pdns/remote_logger.cc
+++ b/pdns/remote_logger.cc
@@ -146,43 +146,44 @@ bool RemoteLogger::reconnect()
 
 RemoteLoggerInterface::Result RemoteLogger::queueData(const std::string& data)
 {
+  auto runtime = d_runtime.lock();
+
   if (data.size() > std::numeric_limits<uint16_t>::max()) {
+    ++runtime->d_stats.d_tooLarge;
     return Result::TooLarge;
   }
 
-  auto runtime = d_runtime.lock();
-
   if (!runtime->d_writer.hasRoomFor(data)) {
     /* not connected, queue is full, just drop */
     if (!runtime->d_socket) {
-      ++d_drops;
+      ++runtime->d_stats.d_pipeFull;
       return Result::PipeFull;
     }
     try {
       /* we try to flush some data */
       if (!runtime->d_writer.flush(runtime->d_socket->getHandle())) {
         /* but failed, let's just drop */
-        ++d_drops;
+        ++runtime->d_stats.d_pipeFull;
         return Result::PipeFull;
       }
 
       /* see if we freed enough data */
       if (!runtime->d_writer.hasRoomFor(data)) {
         /* we didn't */
-        ++d_drops;
+        ++runtime->d_stats.d_pipeFull;
         return Result::PipeFull;
       }
     }
     catch(const std::exception& e) {
       //      cout << "Got exception writing: "<<e.what()<<endl;
-      ++d_drops;
       runtime->d_socket.reset();
-      return Result::PipeFull;
+      ++runtime->d_stats.d_otherError;
+      return Result::OtherError;
     }
   }
 
   runtime->d_writer.write(data);
-  ++d_processed;
+  ++runtime->d_stats.d_queued;
   return Result::Queued;
 }
 
diff --git a/pdns/remote_logger.hh b/pdns/remote_logger.hh
index 72b1ec5e31..98caa91af3 100644
--- a/pdns/remote_logger.hh
+++ b/pdns/remote_logger.hh
@@ -63,6 +63,7 @@ public:
   enum class Result : uint8_t { Queued, PipeFull, TooLarge, OtherError };
   static const std::string& toErrorString(Result r);
 
+
   virtual ~RemoteLoggerInterface() {};
   virtual Result queueData(const std::string& data) = 0;
   virtual std::string toString() const = 0;
@@ -72,6 +73,16 @@ public:
   void setLogQueries(bool flag) { d_logQueries = flag; }
   void setLogResponses(bool flag) { d_logResponses = flag; }
 
+  struct Stats
+  {
+    uint64_t d_queued{};
+    uint64_t d_pipeFull{};
+    uint64_t d_tooLarge{};
+    uint64_t d_otherError{};
+  };
+
+  virtual Stats getStats() const = 0;
+
 private:
   bool d_logQueries{true};
   bool d_logResponses{true};
@@ -98,8 +109,15 @@ public:
   }
   std::string toString() const override
   {
-    return d_remote.toStringWithPort() + " (" + std::to_string(d_processed) + " processed, " + std::to_string(d_drops) + " dropped)";
+    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() const override
+  {
+    return d_runtime.lock()->d_stats;
+  }
+
   void stop()
   {
     d_exiting = true;
@@ -113,17 +131,16 @@ private:
   {
     CircularWriteBuffer d_writer;
     std::unique_ptr<Socket> d_socket{nullptr};
+    RemoteLoggerInterface::Stats d_stats{};
   };
 
   ComboAddress d_remote;
-  std::atomic<uint64_t> d_drops{0};
-  std::atomic<uint64_t> d_processed{0};
   uint16_t d_timeout;
   uint8_t d_reconnectWaitTime;
   std::atomic<bool> d_exiting{false};
   bool d_asyncConnect{false};
 
-  LockGuarded<RuntimeData> d_runtime;
+  mutable LockGuarded<RuntimeData> d_runtime;
   std::thread d_thread;
 };