Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Custom Asynchronous Loggers #2742

Closed
pparuzel opened this issue May 26, 2023 · 7 comments
Closed

Custom Asynchronous Loggers #2742

pparuzel opened this issue May 26, 2023 · 7 comments

Comments

@pparuzel
Copy link

As the current implementation stands, the logger class is the only logging class that supports polymorphism, while the async_logger class is marked as final. This limitation poses challenges for creating custom asynchronous loggers.

Could you please clarify if this restriction is intentional? If so, it would be helpful to understand alternative approaches to achieve asynchronous logging without the need to rewrite or wrap the existing implementation functionalities.

@gabime
Copy link
Owner

gabime commented May 26, 2023

It is kind of intentional, for performance reasons. What do you want to achieve specifically?

@pparuzel
Copy link
Author

In short, our objective is to prepend a thread-local message to every log call of asynchronous loggers.

Here's the context:

We successfully replaced our previous logging library with spdlog, which brought us notable performance and readability improvements. However, we encountered an obstacle while converting one specific functionality: spdlog lacks support for Nested Diagnostic Context (NDC). To address this, we created a custom implementation of NDC based on thread-local stacks.

To achieve seamless integration with the thread pool's worker, we need to access the NDC within the log call and forward it as a string from the corresponding thread to the queue, as our previous logging library did. Unfortunately, we haven't found a viable approach to accomplish this in the current setup.

Consequently, we are left with the challenge of manually prepending NDC information to each log message, which presents its own set of difficulties.

FYI, Our first draft was implemented like this:

class NDCFormatter : public spdlog::custom_flag_formatter
{
public:
    void format(const spdlog::details::log_msg& msg, const std::tm& tm_time, spdlog::memory_buf_t& dest) override
    {
        if (!NDC::empty())
        {
            const auto& ndc_context = NDC::get();
            dest.append(ndc_context.data(), ndc_context.data() + ndc_context.size());
        }
    }
    
    std::unique_ptr<custom_flag_formatter> clone() const override;
};

inline std::unique_ptr<spdlog::pattern_formatter> createNDCPatternFormatter(std::string pattern)
{
    auto f = std::make_unique<spdlog::pattern_formatter>();
    f->template add_flag<NDCFormatter>('x').set_pattern(std::move(pattern));
    return f;
}

However, as you know, this kind of formatting happens from within the thread_pool's worker thread. This means that the NDC stack and a call to NDC::get() will always result in an empty string. The only formatting that happens synchronously is the message formatting call to fmt library which is also inextensible.

@gabime
Copy link
Owner

gabime commented May 26, 2023

So how would inheriting the async logger solve this?

@pparuzel
Copy link
Author

It would unlock overriding the sink_it_ protected function member with additional NDC information prepended. I noticed that sink_it_ is called synchronously, while backend_sink_it is async, which suggests that it could work.

While it may not be a perfect solution as it requires rebuilding the log_msg, it still offers advantages over creating a wrapper class or rewriting the entire async logger (including thread_pools due to their coupling) starting from the spdlog::logger base.

My aim is to achieve the same goal regardless of the inheritability of the async_logger. The asynchronous property of a logger feels more like a different logging strategy rather than a final extension. Hence, my initial thought was to build upon this class. There may be another approach that I'm not aware of. I would be grateful to receive any suggestions.

@gabime
Copy link
Owner

gabime commented May 26, 2023

Yes, that might work. If you change the lib code, you can achieve this. Another (a bit dirty, but avoids changing the lib) option would be to use 2 loggers and a custom sink: Synchronous Logger with custom sink that adds MDC as a string to the message and forward to to a second asynchronous logger.

@gabime
Copy link
Owner

gabime commented May 27, 2023

Related: #2595

@gabime gabime closed this as completed May 27, 2023
@F1F88
Copy link
Contributor

F1F88 commented Dec 10, 2024

Hi, I also encountered the problem of custom logger and was stumped on this issue.

For custom asynchronous logger, is it possible to consider abstracting spdlog::async_logger::backend_sink_it_ and spdlog::async_logger::backend_flush_ into another class. (For example: async_logger_backend)

Then spdlog::async_logger needs to inherit and implement the two class spdlog::logger and async_logger_backend.

Finally, modify async_logger_ptr of spdlog::details::thread_pool to async_logger_backend type.

I guess this would allow users to customize async_logger without modifying the source code of the library (just inherit spdlog::logger and async_logger_backend), and also reuse spdlog::thrad_pool.

Although this question has been closed, my problem is similar, so I post it here in the hope of finding a better solution.

I'm learning C++, so please feel free to correct me if I make any stupid mistakes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants