Async logging using MDC about spdlog HOT 6 OPEN

An MWE which unfortunately, doesn't compile on godbolt is this:

#include <spdlog/spdlog.h>
#include <spdlog/sinks/stdout_sinks.h>
#include <spdlog/mdc.h>

int main()
{
    auto create_logger = [](auto name) {
        auto logger = std::make_shared<spdlog::logger>(
            name,
            std::make_shared<spdlog::sinks::stdout_sink_st>()
        );
        logger->set_pattern("[%H:%M:%S %z] [%^%L%$] [%&] %v");
        logger->info("Logger created");
        return logger;
    };

    auto log_a = create_logger("log_a");
    auto log_b = create_logger("log_b");

    spdlog::mdc::put("my_key", "my_value");

    log_a->info("Hello World from log_a");
    log_b->info("Hello World from log_b");
}

It produces the following output:

[15:02:46 +02:00] [I] [] Logger created
[15:02:46 +02:00] [I] [] Logger created
[15:02:46 +02:00] [I] [my_key:my_value] Hello World from log_a
[15:02:46 +02:00] [I] [my_key:my_value] Hello World from log_b

from spdlog.

You should decide if MDC will be copied when another thread is spawned from one thread.
When a new thread is spawned, the MDC of the new thread will be in one of the following states:

• MDC is copied from the original thread.
• MDC is empty It is not a copy of the original thread.

It should be necessary to decide whether to implement both of these two behaviors or only one of them.

from spdlog.

I don't think MDC should be linked to the thread at all.

Maybe, I am misunderstanding the feature but I would argue it's much more desirable to have attributes associated to logger objects than to threads.

If I were to implement it from scratch, I would ditch thread_local storage and just go for regular members. When a new thread is spawned it's accessing the same variables as the original thread. Obviously, these variables would have to be locked behind a mutex.

Having thread specific variables loses it's gain as soon as you create a thread-pool.
When using async loggers, I need to allocate a thread pool for them with spdlog::init_thread_pool(...).

I haven't looked into the implementation but it feels like none of the contextual attributes I would set within the business logic would ever make it to any of the sinks when using async loggers and thread_local storage.

So my approach for a re-implementation would be:

• make the MDC map containing the key-values a regular member variable of spdlog::mdc
• make spdlog::mdc a member of the spdlog::logger
• provide functions like add_mdc/remove_mdc/get_mdc to spdlog::logger
• have the logger format these during logging if the pattern requires it

It's difficult however, to keep the current behavior of using thread local contexts while implementing it this way.

I am not sure yet but I would maybe try to find some runtime function that switches from current behavior to the one, I am trying to describe. I want to keep backwards compatibility and have the current behavior be the default to not break user-code.

from spdlog.

First, I am not a native speaker and may write in hard-to-read English. Please point out any parts that do not make sense to you.

I understand your point of view.
The implementation of MDCs being owned by threads has been found to be problematic in frameworks that sequentially execute dependent tasks in a thread pool, where MDCs tied to threads cannot propagate data correctly.
This is because the tasks executed by the thread pool are interchanged, and if the MDC of the immediately preceding task is inherited, the wrong MDC will be referenced.

The best way to solve this problem is not yet available to my knowledge, but many tools and frameworks often take the solution of providing a context that is tied to a task or thread, and the MDC is owned by the context.
Many developers want to share loggers with threads and tasks, but MDC is independent data in units of tasks and threads.
So there are many frameworks that provide contexts as independent data repositories on a per-thread or per-task basis.
This is a more developer-friendly implementation than having MDCs owned by something like a logger that is shared by multiple threads or tasks.
However, this is possible because the framework provides a mechanism for transparently switching the MDC that the logger refers to when switching threads or tasks.

I did not initially offer this as an opinion because I thought it would be very difficult to provide this implementation and would be well beyond the scope of the logging functionality provided by spdlog.

from spdlog.

Current implementation is just the simplest we could think of without breaking the v1.x API and without hurting the performance.

The intention is to explore this approach and revise it in the v2.x branch once we have a deeper understanding and gather user feedback.

@felixjulianheitmann you can open a pr in branch v2.x if you feel you have better solution. Please make sure to make it is as simple as possible as it is easy to over complicate this.

from spdlog.

Hi, thanks for the blessing, I will give it a try!

from spdlog.