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

Why do you have to call flush all the time ? #4

Open
Mecanik opened this issue Jun 3, 2020 · 7 comments
Open

Why do you have to call flush all the time ? #4

Mecanik opened this issue Jun 3, 2020 · 7 comments
Labels
question Further information is requested

Comments

@Mecanik
Copy link

Mecanik commented Jun 3, 2020

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!

@d-tatianin
Copy link
Owner

d-tatianin commented Jun 3, 2020

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 d-tatianin added the question Further information is requested label Jun 3, 2020
@Mecanik
Copy link
Author

Mecanik commented Jun 3, 2020

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 :)

@d-tatianin
Copy link
Owner

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.

@Mecanik
Copy link
Author

Mecanik commented Jun 3, 2020

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

@Mecanik
Copy link
Author

Mecanik commented Jun 3, 2020

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 ?

@d-tatianin
Copy link
Owner

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.

@Mecanik
Copy link
Author

Mecanik commented Jun 3, 2020

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 :(

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

No branches or pull requests

2 participants