Skip to content

Commit

Permalink
[26] Use util.logging's logger and update logging documentation
Browse files Browse the repository at this point in the history
  • Loading branch information
m133225 authored Sep 9, 2016
1 parent 62fa368 commit 6d8c91f
Show file tree
Hide file tree
Showing 20 changed files with 136 additions and 319 deletions.
4 changes: 4 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
@@ -1,2 +1,6 @@
# addressbook-level4
Address Book sample application (Level 4)

- [Gradle](docs/devops/gradle/Introduction to Gradle.md)
- [Configuration](docs/addressbook/Configuration.md)
- [Logging](docs/addressbook/Logging.md)
8 changes: 1 addition & 7 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

plugins {
id "com.github.kt3k.coveralls" version "2.4.0"
id 'com.github.johnrengelman.shadow' version '1.2.3'
id "com.github.johnrengelman.shadow" version '1.2.3'
}

allprojects {
Expand Down Expand Up @@ -44,12 +44,9 @@ allprojects {
guavaVersion = '19.0'
jacksonVersion = '2.7.0'
jacksonDataTypeVersion = '2.7.4'
jKeyMasterVersion = '1.2'
junitVersion = '4.12'
log4jVersion = '2.6'
testFxVersion = '3.1.0'
monocleVersion = '1.8.0_20'
slf4jSimpleVersion = '1.6.4'

libDir = 'lib'
}
Expand All @@ -63,9 +60,6 @@ allprojects {
}

dependencies {
compile "org.apache.logging.log4j:log4j-api:$log4jVersion"
compile "org.apache.logging.log4j:log4j-core:$log4jVersion"
compile "org.slf4j:slf4j-simple:$slf4jSimpleVersion" // Required to suppress warning, for jkeymaster, see http://www.slf4j.org/codes.html#StaticLoggerBinder
compile "org.controlsfx:controlsfx:$controlsFxVersion"
compile "com.fasterxml.jackson.core:jackson-databind:$jacksonVersion"
compile "com.fasterxml.jackson.datatype:jackson-datatype-jsr310:$jacksonDataTypeVersion"
Expand Down
33 changes: 4 additions & 29 deletions docs/addressbook/Configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,40 +2,15 @@

Certain properties of the application can be customized through the configuration file (default: `config.json`):

- Application title
- Logging level
- Update interval
- Number/Type of browser(s) used
- Local Data file
- Cloud data file
- Preferences file
- Name of addressbook
- Application title

Most of the variable names are rather straightforward. However, the logging section is slightly more complex and will be elaborated on.

Behaviours to note:
- The application creates a config file with default values if no existing config file can be found.
- Configuration is reset to default if file cannot be read (including if the file contains unexpected or has missing parameters)

# Logging
There are 2 configuration variables meant for logging:
- `currentLogLevel`
- Specifies the application-wide logging level
- `specialLogLevel`
- Adding `className` : `loggingLevel` pairs to this variable will impose a special logging level for that class (priority over the application-wide logging level)

Here is a typical example of the logging parameters in `config.json`:
```
...
"currentLogLevel" : "DEBUG",
"specialLogLevels" : {
"ModelManager" : "TRACE",
"SyncManager" : "TRACE"
},
...
```
Such a configuration cause the loggers to log messages at the `DEBUG` level throughout the application, except `ModelManager`'s and `SyncManager`'s loggers which will log messages at the `TRACE` level.
Variable names in this application are meant to be straightforward, to allow students to understand its idea more easily.

**Note that these settings do not apply to most of `Config`, and also `StorageManager`'s code to read the configuration file since they are not yet in effect. Therefore the logging will usually be at the default level for `Config` and for the affected parts of `StorageManager`. This default logging level is specified in `resources/log4j2.json`.**
**Note that setting of custom the logging level is done only after reading of the configuration file, thus any logging done before it will be at the default `INFO` level**

# Introducing new configuration variables
(For developers)
Expand Down
46 changes: 25 additions & 21 deletions docs/addressbook/Logging.md
Original file line number Diff line number Diff line change
@@ -1,51 +1,42 @@
# Logging

We are using [log4j2](http://logging.apache.org/log4j/2.x/) as our logger, and `AppLogger` is used as a wraparound of the logger to control the logging levels programmatically according to our requirements.
We are using `java.util.logging.Logger` as our logger, and `LoggerManager` is used to manage the logging levels of loggers and handlers (for output of log messages)

**Note to developers**
- The logging level can be controlled using the `loggingLevel` setting in the configuration file (See [Configuration](../docs/Configuration.md)).
- The `AppLogger` for a class can be obtained using `LoggerManager.getLogger(Class)` which will log messages according to `src/main/resources/log4j2.json`'s specified patterns
- Currently log messages are output through 3 methods: `Console`, `.log` and `.csv`. Since the CSV file is using `;` as its delimiter, developers should avoid using `;` which may lead to incorrect interpretation of log data.


**log4j2 information**
- Supposedly optimized version of `LogBack`
- Can also act as an API for other logger implementations such as `slf4j`
- `log4j2` has its internal implementation, but can be substituted as needed
- Our application uses the default internal implementation
- The logging level can be controlled using the `currentLogLevel` setting in the configuration file (See [Configuration](../docs/Configuration.md)).
- The `Logger` for a class can be obtained using `LoggerManager.getLogger(Class)` which will log messages according to the specified logging level
- Currently log messages are output through: `Console` and `.log`

# Guidelines

## Logging Levels

- FATAL
- SEVERE
- Critical use case affected, which may possibly cause the termination of the application

- ERROR **(NOT USED)**

- WARN
- WARNING
- Can continue, but with caution

- INFO
- Information important for the application's purpose
- e.g. update to local model/request sent to cloud
- Information that the layman user can understand

- DEBUG
- CONFIG
- Usually used to output information regarding the system's configuration

- FINE
- Used for superficial debugging purposes to pinpoint components that the fault/bug is likely to arise from
- Should include more detailed information as compared to `INFO` i.e. log useful information!
- e.g. print the actual list instead of just its size

- TRACE
- Everywhere, e.g. entry and exit of ALL methods, including the parameters being given/returned
- Will not be using log4j2 for this, will explore AOP at a future date
- FINER/FINEST
- Almost everywhere, to allow more specific pinpointing of errors in methods

## General

- Possible guidelines to adhere to:
- Include version of code being run in EVERY file
- Don't manually concatenate strings, which may worsen performance
- Instead, use log4j2's `{}` to log parameters
- `Catch` blocks that re-throw the exception should not log the exception as this may lead to repeated logging of the same exception
- Avoid `NullPointerException`s, which may occur if we try to log some return value of an object's method, when the object itself may be null
- Ensure that the object being printed has `toString()` implemented, if not the printout will be uninformative such as `Object@31942`
Expand All @@ -54,3 +45,16 @@ We are using [log4j2](http://logging.apache.org/log4j/2.x/) as our logger, and `
- [Apache Commons Logging guide](http://commons.apache.org/proper/commons-logging/guide.html#Message_PrioritiesLevels)
- [10 Tips for Proper Application Logging](https://www.javacodegeeks.com/2011/01/10-tips-proper-application-logging.html)
- [Base 22 Java Logging Standards and Guidelines](https://wiki.base22.com/display/btg/Java+Logging+Standards+and+Guidelines)

# Technical Information
`Logger`s create the log records, and `Handler`s publish these log records to some output destination
- We can specify the log level for both the `Logger`s and the `Handler`s
- By default, `java.util.logging.Logger` has a root logger of level `INFO` which has a `ConsoleHandler` of level `INFO` attached.

## LoggerManager
- Named `Logger`s can be obtained from this class
- These loggers will output messages to the console and a `.log` file by default, at the `INFO` level
- Can be initialized with a `Config` object which contains a custom logging level
- Loggers obtained *AFTER* this initialization will have their logging level changed
- Logging levels for existing loggers will only be updated if the logger with the same name is requested again from `LoggerManager`

6 changes: 3 additions & 3 deletions src/main/java/seedu/address/MainApp.java
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,17 @@
import seedu.address.model.ModelManager;
import seedu.address.model.UserPrefs;
import seedu.address.storage.StorageManager;
import seedu.address.util.AppLogger;
import seedu.address.util.Config;
import seedu.address.util.LoggerManager;

import java.util.Map;
import java.util.logging.Logger;

/**
* The main entry point to the application.
*/
public class MainApp extends Application {
private static final AppLogger logger = LoggerManager.getLogger(MainApp.class);
private static final Logger logger = LoggerManager.getLogger(MainApp.class);

private static final int VERSION_MAJOR = 1;
private static final int VERSION_MINOR = 6;
Expand Down Expand Up @@ -88,7 +88,7 @@ protected ModelManager initModelManager(Config config) {

@Override
public void start(Stage primaryStage) {
logger.info("Starting application: {}", MainApp.VERSION);
logger.info("Starting application: " + MainApp.VERSION);
ui.start(primaryStage, this);
storageManager.start();
}
Expand Down
5 changes: 3 additions & 2 deletions src/main/java/seedu/address/browser/BrowserManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,10 @@
import javafx.scene.web.WebView;
import seedu.address.commons.FxViewUtil;
import seedu.address.model.person.ReadOnlyPerson;
import seedu.address.util.AppLogger;
import seedu.address.util.LoggerManager;

import java.util.logging.Logger;

/**
* Manages the AddressBook browser.
* To begin using this class: call start().
Expand All @@ -15,7 +16,7 @@ public class BrowserManager {

private static final String GITHUB_ROOT_URL = "https://github.com/";
private static final String INVALID_GITHUB_USERNAME_MESSAGE = "Unparsable GitHub Username.";
private static AppLogger logger = LoggerManager.getLogger(BrowserManager.class);
private static Logger logger = LoggerManager.getLogger(BrowserManager.class);
private WebView browser;

public BrowserManager() {
Expand Down
9 changes: 2 additions & 7 deletions src/main/java/seedu/address/commons/JsonUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,11 @@
import com.fasterxml.jackson.databind.module.SimpleModule;
import com.fasterxml.jackson.databind.ser.std.ToStringSerializer;
import com.fasterxml.jackson.databind.type.TypeFactory;
import org.apache.logging.log4j.Level;

import java.io.IOException;
import java.util.HashMap;
import java.util.List;
import java.util.logging.Level;

/**
* Converts a Java object instance to JSON and vice versa
Expand All @@ -41,12 +41,7 @@ protected Level _deserialize(String value, DeserializationContext ctxt) throws I
* @return
*/
private Level getLoggingLevel(String loggingLevelString) {
for (Level level : Level.values()) {
if (level.toString().equals(loggingLevelString)) {
return level;
}
}
return null;
return Level.parse(loggingLevelString);
}

@Override
Expand Down
7 changes: 4 additions & 3 deletions src/main/java/seedu/address/controller/HelpWindow.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,16 @@
import javafx.stage.Stage;
import seedu.address.MainApp;
import seedu.address.commons.FxViewUtil;
import seedu.address.util.AppLogger;
import seedu.address.util.LoggerManager;

import java.util.logging.Logger;

/**
* Controller for a help page
*/
public class HelpWindow extends BaseUiPart {

private static final AppLogger logger = LoggerManager.getLogger(HelpWindow.class);
private static final Logger logger = LoggerManager.getLogger(HelpWindow.class);
private static final String ICON = "/images/help_icon.png";
public static final String FXML = "HelpWindow.fxml";
public static final String TITLE = "Help";
Expand All @@ -26,7 +27,7 @@ public class HelpWindow extends BaseUiPart {
private Stage dialogStage;

public static HelpWindow load(Stage primaryStage) {
logger.debug("Showing help page about the application.");
logger.fine("Showing help page about the application.");
HelpWindow helpWindow = UiPartLoader.loadUiPart(primaryStage, new HelpWindow());
helpWindow.configure();
return helpWindow;
Expand Down
11 changes: 6 additions & 5 deletions src/main/java/seedu/address/controller/MainWindow.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,17 +21,18 @@
import seedu.address.model.ModelManager;
import seedu.address.model.UserPrefs;
import seedu.address.parser.Parser;
import seedu.address.util.AppLogger;
import seedu.address.util.Config;
import seedu.address.util.GuiSettings;
import seedu.address.util.LoggerManager;

import java.util.logging.Logger;

/**
* The Main Window. Provides the basic application layout containing
* a menu bar and space where other JavaFX elements can be placed.
*/
public class MainWindow extends BaseUiPart {
private static AppLogger logger = LoggerManager.getLogger(MainWindow.class);
private static Logger logger = LoggerManager.getLogger(MainWindow.class);
private static final String ICON = "/images/address_book_32.png";
private static final String FXML = "MainWindow.fxml";
private static final String HEADER_STATUSBAR_PLACEHOLDER_FIELD_ID = "#headerStatusbarPlaceholder";
Expand Down Expand Up @@ -87,7 +88,7 @@ public String getFxmlPath() {

public static MainWindow load(Stage primaryStage, Config config, UserPrefs prefs, Ui ui,
ModelManager modelManager, BrowserManager browserManager) {
logger.debug("Initializing main window.");
logger.fine("Initializing main window.");
MainWindow mainWindow = UiPartLoader.loadUiPart(primaryStage, new MainWindow());
mainWindow.configure(config.getAppTitle(), config.getAddressBookName(), prefs, ui, modelManager,
browserManager);
Expand Down Expand Up @@ -209,7 +210,7 @@ private void handleCommandInputChanged() {

logger.info("Result: " + command.getClass().getSimpleName());
logger.info("Result: " + result.feedbackToUser);
logger.debug("Invalid command: {}", commandInput.getText());
logger.fine("Invalid command: " + commandInput.getText());
}

@FXML
Expand All @@ -227,7 +228,7 @@ public void show() {
*/
@FXML
private void handleAbout() {//TODO: refactor to be similar to handleHelp and remove the dependency to ui
logger.debug("Showing information about the application.");
logger.fine("Showing information about the application.");
ui.showAlertDialogAndWait(AlertType.INFORMATION, "AddressApp", "About",
"Version " + MainApp.VERSION.toString() + "\nSome code adapted from http://code.makery.ch");
}
Expand Down
8 changes: 4 additions & 4 deletions src/main/java/seedu/address/controller/PersonListPanel.java
Original file line number Diff line number Diff line change
Expand Up @@ -14,19 +14,19 @@
import seedu.address.model.ModelManager;
import seedu.address.model.person.ReadOnlyPerson;
import seedu.address.ui.PersonListViewCell;
import seedu.address.util.AppLogger;
import seedu.address.util.LoggerManager;

import java.util.ArrayList;
import java.util.List;
import java.util.logging.Logger;

/**
* Dialog to view the list of persons and their details
*
* setConnections should be set before showing stage
*/
public class PersonListPanel extends BaseUiPart {
private static AppLogger logger = LoggerManager.getLogger(PersonListPanel.class);
private static Logger logger = LoggerManager.getLogger(PersonListPanel.class);
public static final String FXML = "PersonListPanel.fxml";
private VBox panel;
private AnchorPane placeHolderPane;
Expand All @@ -40,7 +40,7 @@ public PersonListPanel() {

public static PersonListPanel load(Stage primaryStage, AnchorPane personListPlaceholder,
ModelManager modelManager, BrowserManager browserManager) {
logger.debug("Loading person list panel.");
logger.fine("Loading person list panel.");
PersonListPanel personListPanel =
UiPartLoader.loadUiPart(primaryStage, personListPlaceholder, new PersonListPanel());
personListPanel.configure(modelManager, browserManager,
Expand Down Expand Up @@ -99,7 +99,7 @@ private void setupListviewSelectionModelSettings() {
private void loadGithubProfilePageWhenPersonIsSelected(BrowserManager browserManager) {
personListView.getSelectionModel().selectedItemProperty().addListener((observable, oldValue, newValue) -> {
if (newValue != null) {
logger.debug("Person in list view clicked. Loading GitHub profile page: '{}'", newValue);
logger.fine("Person in list view clicked. Loading GitHub profile page: '" + newValue + "'");
browserManager.loadPersonPage(newValue);
}
});
Expand Down
Loading

0 comments on commit 6d8c91f

Please sign in to comment.