From ccd9194a67d363eeb59c0f87a8599c0101f8c0b4 Mon Sep 17 00:00:00 2001 From: Stepan Mazurov Date: Fri, 28 Aug 2020 14:14:22 -0600 Subject: [PATCH 1/3] Fix for not respecting custom LoggerFactory client config --- pulsar-client-cpp/lib/ClientImpl.cc | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/pulsar-client-cpp/lib/ClientImpl.cc b/pulsar-client-cpp/lib/ClientImpl.cc index 716f372fc3cce..f0ce634a70369 100644 --- a/pulsar-client-cpp/lib/ClientImpl.cc +++ b/pulsar-client-cpp/lib/ClientImpl.cc @@ -97,18 +97,20 @@ ClientImpl::ClientImpl(const std::string& serviceUrl, const ClientConfiguration& requestIdGenerator_(0), closingError(ResultOk) { std::unique_ptr loggerFactory = clientConfiguration_.impl_->takeLogger(); + if (!loggerFactory) { #ifdef USE_LOG4CXX - if (!clientConfiguration_.getLogConfFilePath().empty()) { - // A log4cxx log file was passed through deprecated parameter. Use that to configure Log4CXX - loggerFactory = Log4CxxLoggerFactory::create(clientConfiguration_.getLogConfFilePath()); - } else { + if (!clientConfiguration_.getLogConfFilePath().empty()) { + // A log4cxx log file was passed through deprecated parameter. Use that to configure Log4CXX + loggerFactory = Log4CxxLoggerFactory::create(clientConfiguration_.getLogConfFilePath()); + } else { + // Use default simple console logger + loggerFactory = SimpleLoggerFactory::create(); + } +#else // Use default simple console logger loggerFactory = SimpleLoggerFactory::create(); - } -#else - // Use default simple console logger - loggerFactory = SimpleLoggerFactory::create(); #endif + } LogUtils::setLoggerFactory(std::move(loggerFactory)); if (serviceUrl_.compare(0, 4, "http") == 0) { From d363b73e3fab42fb915dc8fa32fb165b1e20466b Mon Sep 17 00:00:00 2001 From: Stepan Mazurov Date: Mon, 31 Aug 2020 19:21:04 -0600 Subject: [PATCH 2/3] Add Custom Logger Factory Test --- pulsar-client-cpp/docker-build.sh | 2 +- pulsar-client-cpp/lib/LogUtils.cc | 2 + pulsar-client-cpp/lib/LogUtils.h | 2 + pulsar-client-cpp/tests/CustomLoggerTest.cc | 69 +++++++++++++++++++++ 4 files changed, 74 insertions(+), 1 deletion(-) create mode 100644 pulsar-client-cpp/tests/CustomLoggerTest.cc diff --git a/pulsar-client-cpp/docker-build.sh b/pulsar-client-cpp/docker-build.sh index 73ab80dc7924b..883e58a05a9fa 100755 --- a/pulsar-client-cpp/docker-build.sh +++ b/pulsar-client-cpp/docker-build.sh @@ -36,7 +36,7 @@ echo "---- Build Pulsar C++ client using image $IMAGE (pass for inc docker pull $IMAGE VOLUME_OPTION=${VOLUME_OPTION:-"-v $ROOT_DIR:/pulsar"} -COMMAND="cd /pulsar/pulsar-client-cpp && cmake . $CMAKE_ARGS && make check-format && make -j8" +COMMAND="cd /pulsar/pulsar-client-cpp && cmake . $CMAKE_ARGS && make check-format && make -j16" DOCKER_CMD="docker run -i ${VOLUME_OPTION} ${IMAGE}" diff --git a/pulsar-client-cpp/lib/LogUtils.cc b/pulsar-client-cpp/lib/LogUtils.cc index 9de6ff4aae005..1c545fec10dd6 100644 --- a/pulsar-client-cpp/lib/LogUtils.cc +++ b/pulsar-client-cpp/lib/LogUtils.cc @@ -63,4 +63,6 @@ std::string LogUtils::getLoggerName(const std::string& path) { return path.substr(startIdx + 1, endIdx - startIdx - 1); } +void LogUtils::resetLoggerFactory() { s_loggerFactory.exchange(nullptr, std::memory_order_release); } + } // namespace pulsar diff --git a/pulsar-client-cpp/lib/LogUtils.h b/pulsar-client-cpp/lib/LogUtils.h index 61d3e6356797c..67ddf431c2e6f 100644 --- a/pulsar-client-cpp/lib/LogUtils.h +++ b/pulsar-client-cpp/lib/LogUtils.h @@ -88,6 +88,8 @@ class PULSAR_PUBLIC LogUtils { static void setLoggerFactory(std::unique_ptr loggerFactory); + static void resetLoggerFactory(); + static LoggerFactory* getLoggerFactory(); static std::string getLoggerName(const std::string& path); diff --git a/pulsar-client-cpp/tests/CustomLoggerTest.cc b/pulsar-client-cpp/tests/CustomLoggerTest.cc new file mode 100644 index 0000000000000..da2759674a62e --- /dev/null +++ b/pulsar-client-cpp/tests/CustomLoggerTest.cc @@ -0,0 +1,69 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +#include +#include +#include +#include +#include + +using namespace pulsar; + +static std::vector logLines; + +class MyTestLogger : public Logger { + public: + MyTestLogger() = default; + + bool isEnabled(Level level) override { return true; } + + void log(Level level, int line, const std::string &message) override { + std::stringstream ss; + ss << " " << level << ":" << line << " " << message << std::endl; + logLines.emplace_back(ss.str()); + } +}; + +class MyTestLoggerFactory : public LoggerFactory { + public: + Logger *getLogger(const std::string &fileName) override { return logger; } + + private: + MyTestLogger *logger = new MyTestLogger; +}; + +TEST(CustomLoggerTest, testCustomLogger) { + // simulate new client created on a different thread (because logging factory is called once per thread) + auto testThread = std::thread([] { + ClientConfiguration clientConfig; + auto customLogFactory = new MyTestLoggerFactory(); + clientConfig.setLogger(customLogFactory); + // reset to previous log factory + Client client("pulsar://localhost:6650", clientConfig); + client.close(); + ASSERT_EQ(logLines.size(), 2); + LogUtils::resetLoggerFactory(); + }); + testThread.join(); + + ClientConfiguration clientConfig; + Client client("pulsar://localhost:6650", clientConfig); + client.close(); + // custom logger didn't get any new lines + ASSERT_EQ(logLines.size(), 2); +} From 4c33d39e25a8f89da39d016f43898730639a51ec Mon Sep 17 00:00:00 2001 From: Stepan Mazurov Date: Mon, 31 Aug 2020 19:23:40 -0600 Subject: [PATCH 3/3] revert threads used by make in docker-build --- pulsar-client-cpp/docker-build.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pulsar-client-cpp/docker-build.sh b/pulsar-client-cpp/docker-build.sh index 883e58a05a9fa..73ab80dc7924b 100755 --- a/pulsar-client-cpp/docker-build.sh +++ b/pulsar-client-cpp/docker-build.sh @@ -36,7 +36,7 @@ echo "---- Build Pulsar C++ client using image $IMAGE (pass for inc docker pull $IMAGE VOLUME_OPTION=${VOLUME_OPTION:-"-v $ROOT_DIR:/pulsar"} -COMMAND="cd /pulsar/pulsar-client-cpp && cmake . $CMAKE_ARGS && make check-format && make -j16" +COMMAND="cd /pulsar/pulsar-client-cpp && cmake . $CMAKE_ARGS && make check-format && make -j8" DOCKER_CMD="docker run -i ${VOLUME_OPTION} ${IMAGE}"