From 904dc56fdf0a3314f6490a7afacefb35335a88f5 Mon Sep 17 00:00:00 2001 From: Geir Storli Date: Tue, 2 Apr 2019 07:15:47 +0000 Subject: Change rpc and legacy forwarder to take forward filter in the constructor. --- logd/src/logd/config_subscriber.cpp | 19 ++++++++----------- logd/src/logd/legacy_forwarder.cpp | 19 ++++++++++--------- logd/src/logd/legacy_forwarder.h | 10 +++++----- logd/src/logd/rpc_forwarder.cpp | 4 ++-- logd/src/logd/rpc_forwarder.h | 3 +-- .../tests/legacy_forwarder/legacy_forwarder_test.cpp | 17 ++++++++++------- logd/src/tests/rpc_forwarder/rpc_forwarder_test.cpp | 19 ++++++++++++------- 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 ConfigSubscriber::make_forwarder(Metrics& metrics) { + std::unique_ptr result; if (_logserver_use_rpc) { - auto result = std::make_unique(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(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; - 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 metrics_mgr; @@ -120,14 +131,8 @@ struct RpcForwarderTest : public ::testing::Test { : server(), metrics_mgr(std::make_shared()), 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)); -- cgit v1.2.3