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

common: improve logger #257

Merged
merged 16 commits into from
Feb 8, 2022
Merged

common: improve logger #257

merged 16 commits into from
Feb 8, 2022

Conversation

andrea-iob
Copy link
Member

@andrea-iob andrea-iob commented Dec 8, 2021

The improvements are:

  • there are now 5 levels of message severity (DEBUG, INFO, WARNING, ERROR, and CRITICAL);
  • each severity level has an associated function that allows to write messages with the specified severity (log::debug(), log::info(), ...);
  • the function log::cout now accepts an argument that specifies the default severity level of the messages;
  • a new function to disable logging up to a specified severity level has been introduced (void Logger::disable(log::Level level = log::CRITICAL));
  • some functions have been renamed (e.g., setVerbosity renamed to setDefaultVerbosity, setPriority renamed to setDefaultSevierity), old functions are still there but they are marked as deprecated.

Here some examples:

log.cout().disableFile(); // Logging on file will be disabled
log::cout() << "Information message" << std::endl;
log::cout(log::CRITICAL) << "Critical message" << std::endl;
log::info() << "Information message" << std::endl;
log::debug() << "Debug message" << std::endl;

log.cout("logger1") << disableConsole(log::INFO); // Only messages with severity greater than INFO will be printed on the console
log.cout("logger1", log::ERROR) << "Error message" << std::endl;
log.warning("logger1") << "Warning message" << std::endl;

log.cout("logger2") << disable(); // Disable logging (verbosity is overridden in order to suppress all output)
log.error("logger2") << "Error message" << std::endl; // This message will not be logged
log.cout("logger2") << disable(log::NOTSET); // Re-enable the logging (log::NOTSET will remove the verbosity override)
log.error("logger2") << "Error message" << std::endl;

@andrea-iob
Copy link
Member Author

I moved insertion/extraction operators from the operators module to the common module, because those operations need to be defined before the logger. If the operators are defined after the logger, they will not be found by the compiler when selecting the right specialization to be used for the insertion operator in Logger::println (gcc will still accept the code, but this is a gcc bug, https://gcc.gnu.org/bugzilla/show_bug.cgi?id=51577).

@andrea-iob andrea-iob added this to the 1.8 milestone Jan 14, 2022
Copy link
Contributor

@roccoarpa roccoarpa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, new logger interface is fine for me, there are some minor fixes to do, I'm just listing them here:

  • .kdev4 folder, code.kdev4 file and cloc perl script, i think they are accidentally here
  • example: pod_application_example_00002 does not compile and it's still using deprecated logger methods
  • (just a warning for future rebasing) when this branch is rebased on the actual master, newest test_LA_parallel_00002 will trigger the deprecated stuff too.

@andrea-iob
Copy link
Member Author

All fixed, thanks. I've also rebased the branch on top of the current master.

@marcocisternino
Copy link
Member

Could it be useful for users to rely on an "enable" method for the logger? I understand the semantic, but double negation to enable (or re-enable) the logger (at its level( it seems not intuitive to me. Maybe a kind of "enableDefault", even if I do not like "Default".

@roccoarpa
Copy link
Contributor

Hello guys, modifications are fine for me, but honestly I've not completely understood the request from Marco, and i don't want to rush my final approval. I will let you to iterate and elaborate more the concept, first.

@andrea-iob
Copy link
Member Author

Could it be useful for users to rely on an "enable" method for the logger? I understand the semantic, but double negation to enable (or re-enable) the logger (at its level( it seems not intuitive to me. Maybe a kind of "enableDefault", even if I do not like "Default".

The function disable(log:Level level) will provide an overriding verbosity level which takes precedence over the logger’s own level. Its effect is to disable all logging calls of severity level and below. When no arguments are specified, the overriding verbosity will be set to CRITICAL, hence all logger output will be disabled.

Its main purpose is to temporary throttle logging output down. When the logger verbosity can be set again to its defualt value, the function debug is called with the argument log::NOTSET and the verbosity override is removed.

(I've updated the wording of the example in the pull request description to better reflect what the function disable does.)

@marcocisternino
Copy link
Member

Fine. Doc in sources was clear and your final comment too. Thanks for re-wording. I approve.

Copy link
Contributor

@roccoarpa roccoarpa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fine for me too

@andrea-iob andrea-iob merged commit 0db3048 into master Feb 8, 2022
@andrea-iob andrea-iob deleted the common.improve.log branch February 8, 2022 13:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants