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

stdlib_logger: proposition to add a logger for debug phases and levels among the different logs #254

Closed
jvdp1 opened this issue Nov 29, 2020 · 7 comments
Labels
topic: IO Common input/output related features

Comments

@jvdp1
Copy link
Member

jvdp1 commented Nov 29, 2020

I am playing with stdlib_logger and I think it would be nice to have a subroutine for messages that should be printed only during debug phases.

Proposition

log_debug - Writes the string message to self % log_units

Syntax

call self % log_debug( message [,module, procedure, stat, errmsg] )

Log levels

Log levels are used for filtering the messages based on their level/severity.
Each subroutine log_error, log_information, log_io_error, log_text_error, log_warning, and the additional log_debug is associated with a level.

For example:

  • log_debug associated with debug_level
  • log_information associated with information_level
  • log_warning associated with warning_level
  • log_error, log_io_error, log_text_error associated with error_level

with debug_level < information_level < warning_level < error_level.

Since log_message is called by all other log subroutines, it is probably best to not assign a level to log_message IMO.

The level of the logger should be set with the method configure, e.g. call self %configure( level = information_level), meaning that all calls with a level < information_level would be ignored.

Such log level options are avaible in Julia or Python loggers.

@wclodius2 , @milancurcic , @14NGiestas , @ivan-pi , @certik is that of interests? any thoughs, comments?

@wclodius2
Copy link
Contributor

A few comments. This is really two separate proposals" adding log_debug and adding level, and I think it would be best addressed by two separate PRs. Levels are also used by FLIBS (m_exception). On the other hand, Futility (exception_handler) has separate flags for each "level", so that you can output "debug" while not outputting "information" or "warning". Testing the effects of level will be tricky. How do you verify what is not output? log_text_error is different from the other error levels. It indicates a problem not with the application, but rather with a file being read by the application. I suspect its output should be independent of level. Overall I am in favor of adding log_debug, but worry that adding level will be a bit tricky.

@jvdp1
Copy link
Member Author

jvdp1 commented Nov 29, 2020

A few comments. This is really two separate proposals" adding log_debug and adding level, and I think it would be best addressed by two separate PRs.

I agree with you that it should be two separate PR.

Re: log_debug, its implementation is straighfoward. The main question is about its API. Should it be similar to e.g., log_information,

call self % log_debug( message [, module, procedure ] )

Here is a draft of log_debug.

Levels are also used by FLIBS (m_exception). On the other hand, Futility (exception_handler) has separate flags for each "level", so that you can output "debug" while not outputting "information" or "warning".

The proposed approach in this message is similar to Julia and Python loggers.
However, the approach proposed of Futility (exception_handler) seem to be more flexible, but more difficult to implement and manage (even for the user).

Here is a draft for a proposition similar to Julia and Python loggers.
Note that the different default levels could be set by the user too.

Testing the effects of level will be tricky. How do you verify what is not output?

I don't understand that. For example, we could verify that a message is not printed to output_unit. Here is a proposition for testing the different options.

log_text_error is different from the other error levels. It indicates a problem not with the application, but rather with a file being read by the application. I suspect its output should be independent of level.

Right. Default levels should be discussed first and set accordingly.

Overall I am in favor of adding log_debug, but worry that adding level will be a bit tricky.

I am not sure what is tricky with the option level (at least in the way I imagine it). I might miss something about the 'level' approach.

@wclodius2
Copy link
Contributor

The draft and the proposition for testing look mostly reasonable. A few comments on it:

  1. There is one case of "...be NOT printed." that should be "...NOT be printed."
  2. The levels have the prefix "stdlib_" that I consider wordy and superfluous. I consider the stdlib_ prefix useful for avoiding naming conflicts between modules, but unnecessary for module entities. If you insist on a prefix I would make it "log_".
  3. In the test code you use more vertical white space than I would now. I would not insert four blank lines to separate constructs. Because others objected to my inserting blank lines into my if ... then ... else ... end if, I no longer do that though you may be doing that to be consistent with my earlier style in the test code.
  4. The if ( self % level > stdlib_..._level) return should have a blank between level and ).

@jvdp1
Copy link
Member Author

jvdp1 commented Dec 4, 2020

Thank you for your comments. I integrated them, and submitted a first PR (#256) for adding log_debug.

@wclodius2
Copy link
Contributor

Great!

@awvwgk awvwgk added the topic: IO Common input/output related features label Sep 18, 2021
@14NGiestas
Copy link
Member

This issue was solved 9 Dec 2020 by #261 and probably can be closed.

@14NGiestas
Copy link
Member

I'm closing this issue since it was solved and merged in #261 .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: IO Common input/output related features
Projects
None yet
Development

No branches or pull requests

4 participants