BLogger icon indicating copy to clipboard operation
BLogger copied to clipboard

Why do you have to call flush all the time ?

Open Mecanik opened this issue 4 years ago • 7 comments

First of all, great work! Second, why do I have to call flush to write to file ?

I`m testing this on Windows with the following code:

auto custom_logger = bl::logger::make_custom(
        "Test",                   // logger tag
        bl::level::info,             // log level filter
        bl::logger::default_pattern, // logger pattern
        false,                       // is asynchronous
        //bl::sink::make_stderr(),     // any number of sinks at the end
        //bl::sink::make_stdlog(),     // any number of sinks at the end
        //bl::sink::make_stdout(),     // any number of sinks at the end
        //bl::sink::make_syslog(),     // any number of sinks at the end
        //bl::sink::make_console(),
        bl::sink::make_file(
            test,                 // path to a directory where you want the log files to be stored
            bl::infinite,            // bytes per file
            bl::infinite,            // maximum log files
            true                     // should rotate logs?
        )
    );
    custom_logger->debug("{0}, {}!", "Hello", "debug");
    custom_logger->info("{0}, {}!", "Hello", "info");
    custom_logger->error("{0}, {}!", "Hello", "error");
    custom_logger->warning("{0}, {}!", "Hello", "warning");
    custom_logger->critical("{0}, {}!", "Hello", "critical");
    custom_logger->flush();

No matter what options you will use (even async) it will not write to file until you called flush();. The idea is to write to file whatever you output to console.

Is this expected ?

Further enhancements:

  • Add an option to customise the file name, eg with time/date ? The program might run 24/7 and you would need logs for each day (preferably)
  • Add an easier option to change the console output colour ?
  • Change the timestamp to include date/time/ms/ns ?
  • Resolve the full path to the desired logs folder automatically ? For example in my test without adding the full path it would not create the log file...

Also I have a couple of questions:

  • Is this TS ? I plan to use it from multiple threads/thread pool
  • Is this really cross platform ? Meaning, Linux, Mac etc ?

Thank you!

Mecanik avatar Jun 03 '20 07:06 Mecanik

Hey! Appreciate the feedback.

Thanks for the enhancement suggestions, they all look interesting, and I would love to implement them some day, probably not any time soon tho because I am focused on something else right now. Feel free to open a pull request if you would want to implement them yourself tho.

No matter what options you will use (even async) it will not write to file until you called flush();

Flush is pretty expensive, especially for files, I would imagine it's OK to have it flush for the asynchronous logger every time, perhaps I should add that feature. (atm it does flush on destruction, and when a file is closed/switched)

Is this TS ? I plan to use it from multiple threads/thread pool

Logging is completely TS, however, changing logger properties (e.g pattern) from multiple threads isn't.

Is this really cross platform ? Meaning, Linux, Mac etc ?

Confirmed on Windows and Linux, cannot say anything about Mac because I don't have one at home to test it on.

d-tatianin avatar Jun 03 '20 07:06 d-tatianin

Flush is pretty expensive, especially for files, I would imagine it's OK to have it flush for the asynchronous logger every time, perhaps I should add that feature. (atm it does flush on destruction, and when a file is closed/switched)

Well I cannot imagine any logger that would not write to file... eventually ? Calling yourself when to write to file is a bit strange. Also when we say "async" it means immediately, so it would need to write immediately to file ? Or maybe I got it wrong how to use your logger ?

Nevertheless, if you use this for example in 50 threads at once and you have important logs, calling flush from each of them would cause issues ?

Logging is completely TS, however, changing logger properties (e.g pattern) from multiple threads isn't.

That sounds great to me!

Confirmed on Windows and Linux, cannot say anything about Mac because I don't have one at home to test it on.

Great stuff! S**** Apple :)

Mecanik avatar Jun 03 '20 08:06 Mecanik

Well I cannot imagine any logger that would not write to file... eventually ?

Yeah it does write eventually like I said, upon destruction or switching files.

Also when we say "async" it means immediately, so it would need to write immediately to file ?

"async" means asynchronous, not immediate. but I do agree that the async version should flush by itself every time.

Nevertheless, if you use this for example in 50 threads at once and you have important logs, calling flush from each of them would cause issues ?

No, it shouldn't, it would simply call flush 50 times atomically.

d-tatianin avatar Jun 03 '20 08:06 d-tatianin

Great stuff then, for now I will use it as is until you can update it for a bit 👍

Mecanik avatar Jun 03 '20 08:06 Mecanik

I got a (dumb) question, but quite crucial... how would one use this across the whole application ? Say in threads, different classes etc ?

Would this work ?

std::shared_ptr<bl::logger> m_BLogger;

m_BLogger = bl::logger::make_custom....

Then just add in each class:

std::shared_ptr<bl::logger> m_BLogger;

Is this safe/ok ?

Mecanik avatar Jun 03 '20 18:06 Mecanik

Yeah, this should work. You could also make it static or something. Also, there is a typedef bl::logger::ptr I suggest you use that instead to be safe in case the actual type changes down the road.

d-tatianin avatar Jun 03 '20 18:06 d-tatianin

Yeah, this should work. You could also make it static or something. Also, there is a typedef bl::logger::ptr I suggest you use that instead to be safe in case the actual type changes down the road.

Yeah well, my idea does not work and neither your bl::logger::ptr for some reason in another class. Weirdly, it was works in threads.

This is most likely because the class I was trying to use it in, was later instantiated using std::make_shared.

The solution for now was, to define bl::logger::ptr m_BLogger; in the main file, and then extern bl::logger::ptr m_BLogger; in the precompiled header :(

Mecanik avatar Jun 03 '20 18:06 Mecanik