Change rpc and legacy forwarder to take forward filter in the constructor.

7 files changed, 48 insertions, 43 deletions

diff --git a/logd/src/logd/config_subscriber.cpp b/logd/src/logd/config_subscriber.cpp
index 3d87cabc625..0a88db9772f 100644
--- a/logd/src/logd/config_subscriber.cpp
+++ b/logd/src/logd/config_subscriber.cpp
@@ -132,20 +132,17 @@ ConfigSubscriber::~ConfigSubscriber()
 std::unique_ptr<Forwarder>
 ConfigSubscriber::make_forwarder(Metrics& metrics)
 {
+    std::unique_ptr<Forwarder> result;
     if (_logserver_use_rpc) {
-        auto result = std::make_unique<RpcForwarder>(metrics, _supervisor, _logserver_host,
-                                                     _logserver_rpc_port, 60.0, 100);
-        result->set_forward_filter(_forward_filter);
-        _need_new_forwarder = false;
-        return result;
+        result = std::make_unique<RpcForwarder>(metrics, _forward_filter, _supervisor, _logserver_host,
+                                                _logserver_rpc_port, 60.0, 100);
     } else {
-        auto result = _use_logserver ?
-                      LegacyForwarder::to_logserver(metrics, _logserver_host, _logserver_port) :
-                      LegacyForwarder::to_dev_null(metrics);
-        result->setForwardMap(_forward_filter);
-        _need_new_forwarder = false;
-        return result;
+        result = _use_logserver ?
+                 LegacyForwarder::to_logserver(metrics, _forward_filter, _logserver_host, _logserver_port) :
+                 LegacyForwarder::to_dev_null(metrics);
     }
+    _need_new_forwarder = false;
+    return result;
 }
 
 }
diff --git a/logd/src/logd/legacy_forwarder.cpp b/logd/src/logd/legacy_forwarder.cpp
index 32be2bd72c8..c0f74d205e7 100644
--- a/logd/src/logd/legacy_forwarder.cpp
+++ b/logd/src/logd/legacy_forwarder.cpp
@@ -52,19 +52,20 @@ LegacyForwarder::connect_to_dev_null()
     }
 }
 
-LegacyForwarder::LegacyForwarder(Metrics &metrics)
+LegacyForwarder::LegacyForwarder(Metrics &metrics, const ForwardMap& forward_filter)
     :
       _metrics(metrics),
       _logserver_fd(-1),
-      _forwardMap(),
+      _forward_filter(forward_filter),
       _badLines(0)
 {
 }
 
 LegacyForwarder::UP
-LegacyForwarder::to_logserver(Metrics& metrics, const vespalib::string& logserver_host, int logserver_port)
+LegacyForwarder::to_logserver(Metrics& metrics, const ForwardMap& forward_filter,
+                              const vespalib::string& logserver_host, int logserver_port)
 {
-    LegacyForwarder::UP result(new LegacyForwarder(metrics));
+    LegacyForwarder::UP result(new LegacyForwarder(metrics, forward_filter));
     result->connect_to_logserver(logserver_host, logserver_port);
     return result;
 }
@@ -72,15 +73,15 @@ LegacyForwarder::to_logserver(Metrics& metrics, const vespalib::string& logserve
 LegacyForwarder::UP
 LegacyForwarder::to_dev_null(Metrics& metrics)
 {
-    LegacyForwarder::UP result(new LegacyForwarder(metrics));
+    LegacyForwarder::UP result(new LegacyForwarder(metrics, ForwardMap()));
     result->connect_to_dev_null();
     return result;
 }
 
 LegacyForwarder::UP
-LegacyForwarder::to_open_file(Metrics& metrics, int file_desc)
+LegacyForwarder::to_open_file(Metrics& metrics, const ForwardMap& forward_filter, int file_desc)
 {
-    LegacyForwarder::UP result(new LegacyForwarder(metrics));
+    LegacyForwarder::UP result(new LegacyForwarder(metrics, forward_filter));
     result->_logserver_fd = file_desc;
     return result;
 }
@@ -155,8 +156,8 @@ LegacyForwarder::parseLine(std::string_view line)
     _metrics.countLine(logLevelName, message.service());
 
     // Check overrides
-    ForwardMap::iterator found = _forwardMap.find(message.level());
-    if (found != _forwardMap.end()) {
+    auto found = _forward_filter.find(message.level());
+    if (found != _forward_filter.end()) {
         return found->second;
     }
     return false; // Unknown log level
diff --git a/logd/src/logd/legacy_forwarder.h b/logd/src/logd/legacy_forwarder.h
index bbd7ed840ae..52c946754f8 100644
--- a/logd/src/logd/legacy_forwarder.h
+++ b/logd/src/logd/legacy_forwarder.h
@@ -16,7 +16,7 @@ class LegacyForwarder : public Forwarder {
 private:
     Metrics &_metrics;
     int _logserver_fd;
-    ForwardMap _forwardMap;
+    ForwardMap _forward_filter;
     int _badLines;
     const char *copystr(const char *b, const char *e) {
         int len = e - b;
@@ -29,15 +29,15 @@ private:
     void connect_to_dev_null();
     bool parseLine(std::string_view line);
     void forwardText(const char *text, int len);
-    LegacyForwarder(Metrics &metrics);
+    LegacyForwarder(Metrics &metrics, const ForwardMap& forward_filter);
 
 public:
     using UP = std::unique_ptr<LegacyForwarder>;
-    static LegacyForwarder::UP to_logserver(Metrics& metrics, const vespalib::string& logserver_host, int logserver_port);
+    static LegacyForwarder::UP to_logserver(Metrics& metrics, const ForwardMap& forward_filter,
+                                            const vespalib::string& logserver_host, int logserver_port);
     static LegacyForwarder::UP to_dev_null(Metrics& metrics);
-    static LegacyForwarder::UP to_open_file(Metrics& metrics, int file_desc);
+    static LegacyForwarder::UP to_open_file(Metrics& metrics, const ForwardMap& forward_filter, int file_desc);
     ~LegacyForwarder();
-    void setForwardMap(const ForwardMap& forwardMap) { _forwardMap = forwardMap; }
 
     // Implements Forwarder
     void forwardLine(std::string_view line) override;
diff --git a/logd/src/logd/rpc_forwarder.cpp b/logd/src/logd/rpc_forwarder.cpp
index 654266f18af..cf774504bf3 100644
--- a/logd/src/logd/rpc_forwarder.cpp
+++ b/logd/src/logd/rpc_forwarder.cpp
@@ -50,7 +50,7 @@ RpcForwarder::ping_logserver()
     }
 }
 
-RpcForwarder::RpcForwarder(Metrics& metrics, FRT_Supervisor& supervisor,
+RpcForwarder::RpcForwarder(Metrics& metrics, const ForwardMap& forward_filter, FRT_Supervisor& supervisor,
                            const vespalib::string &hostname, int rpc_port,
                            double rpc_timeout_secs, size_t max_messages_per_request)
     : _metrics(metrics),
@@ -60,7 +60,7 @@ RpcForwarder::RpcForwarder(Metrics& metrics, FRT_Supervisor& supervisor,
       _target(),
       _messages(),
       _bad_lines(0),
-      _forward_filter()
+      _forward_filter(forward_filter)
 {
     _target = supervisor.GetTarget(_connection_spec.c_str());
     ping_logserver();
diff --git a/logd/src/logd/rpc_forwarder.h b/logd/src/logd/rpc_forwarder.h
index 0f224561830..7ab1506d881 100644
--- a/logd/src/logd/rpc_forwarder.h
+++ b/logd/src/logd/rpc_forwarder.h
@@ -29,11 +29,10 @@ private:
     void ping_logserver();
 
 public:
-    RpcForwarder(Metrics& metrics, FRT_Supervisor& supervisor,
+    RpcForwarder(Metrics& metrics, const ForwardMap& forward_filter, FRT_Supervisor& supervisor,
                  const vespalib::string& logserver_host, int logserver_rpc_port,
                  double rpc_timeout_secs, size_t max_messages_per_request);
     ~RpcForwarder() override;
-    void set_forward_filter(const ForwardMap& forward_filter) { _forward_filter = forward_filter; }
 
     // Implements Forwarder
     void sendMode() override {}
diff --git a/logd/src/tests/legacy_forwarder/legacy_forwarder_test.cpp b/logd/src/tests/legacy_forwarder/legacy_forwarder_test.cpp
index 83a54a24365..d3339894819 100644
--- a/logd/src/tests/legacy_forwarder/legacy_forwarder_test.cpp
+++ b/logd/src/tests/legacy_forwarder/legacy_forwarder_test.cpp
@@ -27,11 +27,14 @@ struct ForwardFixture {
           logLine(createLogLine())
     {
         fd = open(fileName.c_str(), O_CREAT | O_TRUNC | O_WRONLY, 0777);
-        forwarder = LegacyForwarder::to_open_file(m, fd);
     }
     ~ForwardFixture() {
     }
 
+    void make_forwarder(const ForwardMap& forwarder_filter) {
+        forwarder = LegacyForwarder::to_open_file(m, forwarder_filter, fd);
+    }
+
     const std::string createLogLine() {
         FastOS_Time timer;
         timer.SetNow();
@@ -55,16 +58,16 @@ struct ForwardFixture {
 
 
 TEST_F("require that forwarder forwards if set", ForwardFixture("forward.txt")) {
-    ForwardMap forwardMap;
-    forwardMap[Logger::event] = true;
-    f1.forwarder->setForwardMap(forwardMap);
+    ForwardMap forward_filter;
+    forward_filter[Logger::event] = true;
+    f1.make_forwarder(forward_filter);
     f1.verifyForward(true);
 }
 
 TEST_F("require that forwarder does not forward if not set", ForwardFixture("forward.txt")) {
-    ForwardMap forwardMap;
-    forwardMap[Logger::event] = false;
-    f1.forwarder->setForwardMap(forwardMap);
+    ForwardMap forward_filter;
+    forward_filter[Logger::event] = false;
+    f1.make_forwarder(forward_filter);
     f1.verifyForward(false);
 }
 
diff --git a/logd/src/tests/rpc_forwarder/rpc_forwarder_test.cpp b/logd/src/tests/rpc_forwarder/rpc_forwarder_test.cpp
index 30ca5e19d44..5b70434e793 100644
--- a/logd/src/tests/rpc_forwarder/rpc_forwarder_test.cpp
+++ b/logd/src/tests/rpc_forwarder/rpc_forwarder_test.cpp
@@ -110,6 +110,17 @@ public:
 
 };
 
+ForwardMap
+make_forward_filter()
+{
+    ForwardMap result;
+    result[ns_log::Logger::error] = true;
+    result[ns_log::Logger::warning] = false;
+    result[ns_log::Logger::info] = true;
+    // all other log levels are implicit false
+    return result;
+}
+
 struct RpcForwarderTest : public ::testing::Test {
     RpcServer server;
     std::shared_ptr<MockMetricsManager> metrics_mgr;
@@ -120,14 +131,8 @@ struct RpcForwarderTest : public ::testing::Test {
         : server(),
           metrics_mgr(std::make_shared<MockMetricsManager>()),
           metrics(metrics_mgr),
-          forwarder(metrics, supervisor.get(), "localhost", server.get_listen_port(), 60.0, 3)
+          forwarder(metrics, make_forward_filter(), supervisor.get(), "localhost", server.get_listen_port(), 60.0, 3)
     {
-        ForwardMap forward_filter;
-        forward_filter[ns_log::Logger::error] = true;
-        forward_filter[ns_log::Logger::warning] = false;
-        forward_filter[ns_log::Logger::info] = true;
-        // all other log levels are implicit false
-        forwarder.set_forward_filter(forward_filter);
     }
     void forward_line(const std::string& payload) {
         forwarder.forwardLine(make_log_line("info", payload));