From fe775afc650f79cd3a96424be0589edba3666c38 Mon Sep 17 00:00:00 2001 From: livio Date: Fri, 31 Jul 2020 20:17:51 +0200 Subject: [PATCH 1/8] implement python logger wrapper --- pulsar-client-cpp/lib/ClientImpl.cc | 2 +- pulsar-client-cpp/python/pulsar/__init__.py | 7 ++ pulsar-client-cpp/python/src/config.cc | 99 +++++++++++++++++++++ 3 files changed, 107 insertions(+), 1 deletion(-) diff --git a/pulsar-client-cpp/lib/ClientImpl.cc b/pulsar-client-cpp/lib/ClientImpl.cc index 83db1c44e2a97..2de1e899eb4a3 100644 --- a/pulsar-client-cpp/lib/ClientImpl.cc +++ b/pulsar-client-cpp/lib/ClientImpl.cc @@ -94,7 +94,7 @@ ClientImpl::ClientImpl(const std::string& serviceUrl, const ClientConfiguration& requestIdGenerator_(0), closingError(ResultOk) { std::unique_ptr loggerFactory = clientConfiguration_.impl_->takeLogger(); - if (!loggerFactory) { + if (loggerFactory == nullptr) { #ifdef USE_LOG4CXX if (!clientConfiguration_.getLogConfFilePath().empty()) { // A log4cxx log file was passed through deprecated parameter. Use that to configure Log4CXX diff --git a/pulsar-client-cpp/python/pulsar/__init__.py b/pulsar-client-cpp/python/pulsar/__init__.py index b47c87d32e6e2..a66da4c0ac1f9 100644 --- a/pulsar-client-cpp/python/pulsar/__init__.py +++ b/pulsar-client-cpp/python/pulsar/__init__.py @@ -99,6 +99,7 @@ def send_callback(res, msg_id): client.close() """ +import logging import _pulsar from _pulsar import Result, CompressionType, ConsumerType, InitialPosition, PartitionsRoutingMode, BatchingType # noqa: F401 @@ -360,6 +361,7 @@ def __init__(self, service_url, tls_trust_certs_file_path=None, tls_allow_insecure_connection=False, tls_validate_hostname=False, + logger=None ): """ Create a new Pulsar client instance. @@ -403,6 +405,8 @@ def __init__(self, service_url, Configure whether the Pulsar client validates that the hostname of the endpoint, matches the common name on the TLS certificate presented by the endpoint. + * `logger`: + Set a Python logger for this Pulsar client. """ _check_type(str, service_url, 'service_url') _check_type_or_none(Authentication, authentication, 'authentication') @@ -415,6 +419,7 @@ def __init__(self, service_url, _check_type_or_none(str, tls_trust_certs_file_path, 'tls_trust_certs_file_path') _check_type(bool, tls_allow_insecure_connection, 'tls_allow_insecure_connection') _check_type(bool, tls_validate_hostname, 'tls_validate_hostname') + _check_type_or_none(logging.Logger, logger, 'logger') conf = _pulsar.ClientConfiguration() if authentication: @@ -425,6 +430,8 @@ def __init__(self, service_url, conf.concurrent_lookup_requests(concurrent_lookup_requests) if log_conf_file_path: conf.log_conf_file_path(log_conf_file_path) + if logger: + conf.set_logger(logger) if use_tls or service_url.startswith('pulsar+ssl://') or service_url.startswith('https://'): conf.use_tls(True) if tls_trust_certs_file_path: diff --git a/pulsar-client-cpp/python/src/config.cc b/pulsar-client-cpp/python/src/config.cc index 188aaf546d176..2ff85c08b65ea 100644 --- a/pulsar-client-cpp/python/src/config.cc +++ b/pulsar-client-cpp/python/src/config.cc @@ -88,6 +88,104 @@ static ProducerConfiguration& ProducerConfiguration_setCryptoKeyReader(ProducerC return conf; } +class LoggerWrapper: public Logger { + std::string _logger; + PyObject* _pyLogger; + int _currentPythonLogLevel = 10 + (Logger::LEVEL_INFO*10); + + void _updateCurrentPythonLogLevel() { + PyGILState_STATE state = PyGILState_Ensure(); + + try { + _currentPythonLogLevel = py::call_method(_pyLogger, "getEffectiveLevel"); + } catch (py::error_already_set e) { + PyErr_Print(); + } + + PyGILState_Release(state); + }; + + public: + + LoggerWrapper(const std::string &logger, PyObject* pyLogger) : _logger(logger) { + _pyLogger = pyLogger; + Py_XINCREF(_pyLogger); + + _updateCurrentPythonLogLevel(); + } + + LoggerWrapper(const LoggerWrapper& other) { + _pyLogger = other._pyLogger; + Py_XINCREF(_pyLogger); + } + + LoggerWrapper& operator=(const LoggerWrapper& other) { + _pyLogger = other._pyLogger; + Py_XINCREF(_pyLogger); + return *this; + } + + virtual ~LoggerWrapper() { + Py_XDECREF(_pyLogger); + } + + bool isEnabled(Level level) { + return 10 + (level*10) >= _currentPythonLogLevel; + } + + void log(Level level, int line, const std::string& message) { + PyGILState_STATE state = PyGILState_Ensure(); + + try { + switch (level) { + case Logger::LEVEL_DEBUG: + py::call_method(_pyLogger, "debug", message.c_str()); + break; + case Logger::LEVEL_INFO: + py::call_method(_pyLogger, "info", message.c_str()); + break; + case Logger::LEVEL_WARN: + py::call_method(_pyLogger, "warning", message.c_str()); + break; + case Logger::LEVEL_ERROR: + py::call_method(_pyLogger, "error", message.c_str()); + break; + } + + } catch (py::error_already_set e) { + PyErr_Print(); + } + + PyGILState_Release(state); + } +}; + +class LoggerWrapperFactory : public LoggerFactory { + static LoggerWrapperFactory* _instance; + PyObject* _pyLogger; + + public: + LoggerWrapperFactory(py::object pyLogger) { + _pyLogger = pyLogger.ptr(); + Py_XINCREF(_pyLogger); + } + + virtual ~LoggerWrapperFactory() { + Py_XDECREF(_pyLogger); + } + + Logger* getLogger(const std::string &fileName) { + return new LoggerWrapper(fileName, _pyLogger); + } +}; + +static ClientConfiguration& ClientConfiguration_setLogger(ClientConfiguration& conf, + py::object logger) { + conf.setLogger(new LoggerWrapperFactory(logger)); + return conf; +} + + void export_config() { using namespace boost::python; @@ -110,6 +208,7 @@ void export_config() { .def("tls_allow_insecure_connection", &ClientConfiguration::isTlsAllowInsecureConnection) .def("tls_allow_insecure_connection", &ClientConfiguration::setTlsAllowInsecureConnection, return_self<>()) .def("tls_validate_hostname", &ClientConfiguration::setValidateHostName, return_self<>()) + .def("set_logger", &ClientConfiguration_setLogger, return_self<>()) ; class_("ProducerConfiguration") From 838d6ae815b2117a0e28289e629f866fe3c6744d Mon Sep 17 00:00:00 2001 From: Livio Date: Sat, 1 Aug 2020 10:25:43 +0200 Subject: [PATCH 2/8] rerun tests --- pulsar-client-cpp/python/src/config.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pulsar-client-cpp/python/src/config.cc b/pulsar-client-cpp/python/src/config.cc index 2ff85c08b65ea..05282ee748a51 100644 --- a/pulsar-client-cpp/python/src/config.cc +++ b/pulsar-client-cpp/python/src/config.cc @@ -179,8 +179,7 @@ class LoggerWrapperFactory : public LoggerFactory { } }; -static ClientConfiguration& ClientConfiguration_setLogger(ClientConfiguration& conf, - py::object logger) { +static ClientConfiguration& ClientConfiguration_setLogger(ClientConfiguration& conf, py::object logger) { conf.setLogger(new LoggerWrapperFactory(logger)); return conf; } From ffe130bba8927454c67cc12480bfaabea7acb6bb Mon Sep 17 00:00:00 2001 From: livio Date: Sat, 3 Oct 2020 09:53:24 +0200 Subject: [PATCH 3/8] revert accidental merge change --- pulsar-client-cpp/lib/ClientImpl.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pulsar-client-cpp/lib/ClientImpl.cc b/pulsar-client-cpp/lib/ClientImpl.cc index 2de1e899eb4a3..83db1c44e2a97 100644 --- a/pulsar-client-cpp/lib/ClientImpl.cc +++ b/pulsar-client-cpp/lib/ClientImpl.cc @@ -94,7 +94,7 @@ ClientImpl::ClientImpl(const std::string& serviceUrl, const ClientConfiguration& requestIdGenerator_(0), closingError(ResultOk) { std::unique_ptr loggerFactory = clientConfiguration_.impl_->takeLogger(); - if (loggerFactory == nullptr) { + if (!loggerFactory) { #ifdef USE_LOG4CXX if (!clientConfiguration_.getLogConfFilePath().empty()) { // A log4cxx log file was passed through deprecated parameter. Use that to configure Log4CXX From 2aa9c0279b3703904291f96ee53284008509d27f Mon Sep 17 00:00:00 2001 From: livio Date: Sat, 3 Oct 2020 12:19:21 +0200 Subject: [PATCH 4/8] extract log level equation to a function --- pulsar-client-cpp/python/src/config.cc | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/pulsar-client-cpp/python/src/config.cc b/pulsar-client-cpp/python/src/config.cc index 05282ee748a51..9ae6a9b356718 100644 --- a/pulsar-client-cpp/python/src/config.cc +++ b/pulsar-client-cpp/python/src/config.cc @@ -91,7 +91,7 @@ static ProducerConfiguration& ProducerConfiguration_setCryptoKeyReader(ProducerC class LoggerWrapper: public Logger { std::string _logger; PyObject* _pyLogger; - int _currentPythonLogLevel = 10 + (Logger::LEVEL_INFO*10); + int _currentPythonLogLevel = _getLogLevelValue(Logger::LEVEL_INFO); void _updateCurrentPythonLogLevel() { PyGILState_STATE state = PyGILState_Ensure(); @@ -105,6 +105,10 @@ class LoggerWrapper: public Logger { PyGILState_Release(state); }; + int _getLogLevelValue(Level level) { + return 10 + (level * 10); + } + public: LoggerWrapper(const std::string &logger, PyObject* pyLogger) : _logger(logger) { @@ -130,7 +134,7 @@ class LoggerWrapper: public Logger { } bool isEnabled(Level level) { - return 10 + (level*10) >= _currentPythonLogLevel; + return _getLogLevelValue(level) >= _currentPythonLogLevel; } void log(Level level, int line, const std::string& message) { From 4cb86276d5e1792eb68b24a3897233f9291baab5 Mon Sep 17 00:00:00 2001 From: livio Date: Sat, 3 Oct 2020 12:23:27 +0200 Subject: [PATCH 5/8] add python logger test --- pulsar-client-cpp/python/pulsar_test.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/pulsar-client-cpp/python/pulsar_test.py b/pulsar-client-cpp/python/pulsar_test.py index e7d05f37cd6d6..50456754ac9a7 100755 --- a/pulsar-client-cpp/python/pulsar_test.py +++ b/pulsar-client-cpp/python/pulsar_test.py @@ -17,8 +17,7 @@ # specific language governing permissions and limitations # under the License. # - - +import logging from unittest import TestCase, main import time import os @@ -96,6 +95,10 @@ def test_consumer_config(self): conf.consumer_name("my-name") self.assertEqual(conf.consumer_name(), "my-name") + def test_client_logger(self): + logger = logging.getLogger("pulsar") + Client(self.serviceUrl, logger=logger) + def test_simple_producer(self): client = Client(self.serviceUrl) producer = client.create_producer('my-python-topic') From 20852310c62d14d1b792b9740cf1b7a7676bf01f Mon Sep 17 00:00:00 2001 From: livio Date: Sat, 20 Mar 2021 09:42:59 +0100 Subject: [PATCH 6/8] try to fix the license error --- pulsar-client-cpp/python/pulsar_test.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pulsar-client-cpp/python/pulsar_test.py b/pulsar-client-cpp/python/pulsar_test.py index 50456754ac9a7..11595fafe49b3 100755 --- a/pulsar-client-cpp/python/pulsar_test.py +++ b/pulsar-client-cpp/python/pulsar_test.py @@ -17,6 +17,8 @@ # specific language governing permissions and limitations # under the License. # + + import logging from unittest import TestCase, main import time From 2dd8f746c85189296cb202d3f64339f6099584e9 Mon Sep 17 00:00:00 2001 From: livio Date: Tue, 23 Mar 2021 11:04:25 +0100 Subject: [PATCH 7/8] update documentation --- pulsar-client-cpp/python/pulsar/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pulsar-client-cpp/python/pulsar/__init__.py b/pulsar-client-cpp/python/pulsar/__init__.py index a66da4c0ac1f9..9173dac1d53d5 100644 --- a/pulsar-client-cpp/python/pulsar/__init__.py +++ b/pulsar-client-cpp/python/pulsar/__init__.py @@ -406,7 +406,7 @@ def __init__(self, service_url, endpoint, matches the common name on the TLS certificate presented by the endpoint. * `logger`: - Set a Python logger for this Pulsar client. + Set a Python logger for this Pulsar client. Should be an instance of `logging.Logger`. """ _check_type(str, service_url, 'service_url') _check_type_or_none(Authentication, authentication, 'authentication') From e82dab9c0ac4a569ff80db60949ca0f49d3057ce Mon Sep 17 00:00:00 2001 From: livio Date: Tue, 23 Mar 2021 11:04:36 +0100 Subject: [PATCH 8/8] remove redundant code --- pulsar-client-cpp/python/src/config.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pulsar-client-cpp/python/src/config.cc b/pulsar-client-cpp/python/src/config.cc index 9ae6a9b356718..c21b1d3b4203b 100644 --- a/pulsar-client-cpp/python/src/config.cc +++ b/pulsar-client-cpp/python/src/config.cc @@ -89,7 +89,6 @@ static ProducerConfiguration& ProducerConfiguration_setCryptoKeyReader(ProducerC } class LoggerWrapper: public Logger { - std::string _logger; PyObject* _pyLogger; int _currentPythonLogLevel = _getLogLevelValue(Logger::LEVEL_INFO); @@ -111,7 +110,7 @@ class LoggerWrapper: public Logger { public: - LoggerWrapper(const std::string &logger, PyObject* pyLogger) : _logger(logger) { + LoggerWrapper(const std::string &logger, PyObject* pyLogger) { _pyLogger = pyLogger; Py_XINCREF(_pyLogger);