massakam opened a new pull request, #377: URL: https://github.com/apache/pulsar-client-cpp/pull/377
### Motivation I noticed that there are cases where the specified logger factory is not used. For example, if we compile and run the following code, it expects `FileLoggerFactory` to be used, but it actually uses the default `ConsoleLoggerFactory`. ```cpp ClientConfiguration config = ClientConfiguration(); config.setLogger(new FileLoggerFactory(Logger::LEVEL_INFO, "pulsar-client-cpp.log")); ParamMap params; params["providerDomain"] = "provider.foo.bar"; params["tenantDomain"] = "tenant.foo.bar"; params["tenantService"] = "test-service"; params["privateKey"] = "file:///path/to/private.key"; params["keyId"] = "0"; params["ztsUrl"] = "https://zts.athenz.example.com:443";; AuthenticationPtr auth = AuthAthenz::create(params); config.setAuth(auth); Client client("pulsar://localhost:6650", config); ``` This happens if logging is performed before `LogUtils::setLoggerFactory` is executed in the constructor of `ClientImpl`. https://github.com/apache/pulsar-client-cpp/blob/25ea451eb3a79d48966689ec64460ae03d5d57da/lib/ClientImpl.cc#L94-L99 When logging is performed, `LogUtils::getLoggerFactory` is called. 1. https://github.com/apache/pulsar-client-cpp/blob/25ea451eb3a79d48966689ec64460ae03d5d57da/lib/LogUtils.h#L60 1. https://github.com/apache/pulsar-client-cpp/blob/25ea451eb3a79d48966689ec64460ae03d5d57da/lib/LogUtils.h#L43 If the constructor of `ClientImpl` has not been executed at this point, the default `ConsoleLoggerFactory` is instantiated, set to the static variable `s_loggerFactory`, and returned to the caller. https://github.com/apache/pulsar-client-cpp/blob/25ea451eb3a79d48966689ec64460ae03d5d57da/lib/LogUtils.cc#L38-L44 Loggers generated from the returned `LoggerFactory` will be set to the static and thread_local variable `threadSpecificLogPtr`, and reused from now on. https://github.com/apache/pulsar-client-cpp/blob/25ea451eb3a79d48966689ec64460ae03d5d57da/lib/LogUtils.h#L38-L47 After that, `LogUtils::setLoggerFactory` is executed again in the constructor of `ClientImpl`, but the value can only be set to `s_loggerFactory` once, and it will be ignored from the second time. This is clearly stated in [the comments](https://github.com/apache/pulsar-client-cpp/blob/25ea451eb3a79d48966689ec64460ae03d5d57da/include/pulsar/ClientConfiguration.h#L183-L186). https://github.com/apache/pulsar-client-cpp/blob/25ea451eb3a79d48966689ec64460ae03d5d57da/lib/LogUtils.cc#L30-L36 Even if we can set it again, if a logger is already set to `threadSpecificLogPtr`, a new logger will not be generated from the new `LoggerFactory`. ### Modifications Keep an instance of the default `ConsoleLoggerFactory` in `s_defaultLoggerFactory`, a variable separate from `s_loggerFactory`, and return it if `LoggerFactory` is not set yet. In addition, keep the pointer value of the `LoggerFactory` that generated the logger cached in `threadSpecificLogPtr`, and regenerate the logger if the `LoggerFactory` pointer changes. ### Verifying this change - [ ] Make sure that the change passes the CI checks. ### Documentation - [ ] `doc-not-needed` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
