se-edu / addressbook-level3

:ab::three: Address Book sample application (Level 3)
https://se-education.org/addressbook-level3
MIT License
28 stars 415 forks source link

Changing the log level in config.json does not work #126

Closed damithc closed 1 year ago

damithc commented 2 years ago

See https://github.com/nus-tic4002-AY2122s2/forum/issues/46

damithc commented 2 years ago

@se-edu/tech-team-level1 can confirm if this is a bug?

vimuthm commented 2 years ago

I seem to face the same issue, but only with FINE and further depth log levels. (WARNING seems to work)

Issue seems to be stemming from the fact that in MainApp.java, a logger is initialized as a class attribute first before the config.json file is read within the init() method of the same file, therefore defaulting the log level to INFO and there is no subsequent overwriting because of the null check here in LogsCenter.java;

private static void addConsoleHandler(Logger logger) {
        if (consoleHandler == null) {
            consoleHandler = createConsoleHandler();
        }
        logger.addHandler(consoleHandler);
}

So only log levels higher than INFO seem to work. I'm unsure of the best practices concerned here with how and when to set the log level.

edit: Manually changing the default log level inside LogsCenter.java doesn't fix the issue. So something else might be causing the issue

LimJunxue commented 2 years ago

For me, changing the log level in config.json to all 4 logging levels as stated in the logging conventions does not limit the logged messages at all and remains at INFO.

I agree with @VimuthM on the source of the issue. Calling setLevel of the consoleHandler and fileHandler in addConsoleHandler and addFileHandler after the null check like so will result in the correct outcome shown below when setting the log level to WARNING in config.json.

private static void addConsoleHandler(Logger logger) {
        if (consoleHandler == null) {
            consoleHandler = createConsoleHandler();
        }
        consoleHandler.setLevel(currentLogLevel);
        logger.addHandler(consoleHandler);
}

image All subsequent log messages after that last line lower than WARNING will be discarded, which is the expected outcome. Also, not sure if the intent is to show the currentLogLevel with an INFO message before applying the log level of the config.json.

Py0000 commented 2 years ago

I seem to be facing the same issues as mentioned and similar to @VimuthM, only FINE seems to have the issue.

WARNING and SEVERE seems to be working as intended after changing the logLevel in config.json as well as the currentLogLevel field in LogsCenter.java.

Changing logLevel in config.json as well as the currentLogLevel field in LogsCenter.java does not seem to fix the issue with FINE. I have also tried adding the line as suggested by @LimJunxue to set the log level of consoleHandler to currentLogLevel (FINE in this case) doesn't seem to fix the issue for me. But the log messages printed out on my device before and after adding in the suggested line seems to be the same.

Log Message output before adding consoleHandler.setLevel(currentLogLevel);: image

Log Message output after adding consoleHandler.setLevel(currentLogLevel);: image

LimJunxue commented 2 years ago

@Py0000 the current log level defines that the messages to be written out should have log levels higher or equal to the current log level