Alon Bar-Lev has posted comments on this change.

Change subject: tools: Adds helpers to configure java logging in tools
......................................................................


Patch Set 5:

(5 comments)

http://gerrit.ovirt.org/#/c/32852/5/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/log/JavaLoggingFormatter.java
File 
backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/log/JavaLoggingFormatter.java:

I will keep note that this class is not required per my opinion.
Line 1: package org.ovirt.engine.core.utils.log;
Line 2: 
Line 3: import java.io.PrintWriter;
Line 4: import java.io.StringWriter;


http://gerrit.ovirt.org/#/c/32852/5/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/log/JavaLoggingUtils.java
File 
backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/log/JavaLoggingUtils.java:

Line 12:     /**
Line 13:      * Instance of org.ovirt logger. We need to keep instance of it to 
prevent OpenJDK incompatibility described at
Line 14:      * 
http://findbugs.sourceforge.net/bugDescriptions.html#LG_LOST_LOGGER_DUE_TO_WEAK_REFERENCE
Line 15:      */
Line 16:     private static Logger ovirtLogger;
not sure why you need this.
Line 17: 
Line 18:     /**
Line 19:      * Parses logging level from case insensitive string. Level name 
can be specified in log4j or java.util.logging
Line 20:      * format:


Line 36:      * @exception java.lang.IllegalArgumentException
Line 37:      *                if unknown level is specified
Line 38:      */
Line 39:     public static Level parseLevel(String levelName) {
Line 40:         if ("ALL".equalsIgnoreCase(levelName)) {
this should be in properties file resource or any data element not in code.

 map.level = alias

there is also no need to ignore case all over, just convert both arguments to 
lower case once.
Line 41:             return Level.ALL;
Line 42:         } else if ("FINEST".equalsIgnoreCase(levelName)) {
Line 43:             return Level.FINEST;
Line 44:         } else if ("TRACE".equalsIgnoreCase(levelName) || 
"FINER".equalsIgnoreCase(levelName)) {


Line 92:      */
Line 93:     public static void addFileHandler(String fileName, String 
levelName) {
Line 94:         try {
Line 95:             FileHandler fh = new FileHandler(fileName);
Line 96:             fh.setFormatter(new JavaLoggingFormatter());
this should be in logging properties if we can, and I almost sure we can.
Line 97:             getOvirtLogger().addHandler(fh);
Line 98:             getOvirtLogger().setLevel(parseLevel(levelName));
Line 99:         } catch (SecurityException | IOException ex) {
Line 100:             throw new IllegalArgumentException(


http://gerrit.ovirt.org/#/c/32852/5/packaging/conf/tools-logging.properties
File packaging/conf/tools-logging.properties:

Line 10: # Default level for org.ovirt logger
Line 11: org.ovirt.level=INFO
Line 12: 
Line 13: # Default format of log line is: DATE TIME LEVEL [SOURCE] Message 
Stacktrace
Line 14: java.util.logging.SimpleFormatter.format=%1$tY-%1$tm-%1$td 
%1$tH:%1$tM:%1$tS,%1$tL %4$-5s [%2$s] %5$s%6$s%n
I almost sure you can specify your own formatter here.


-- 
To view, visit http://gerrit.ovirt.org/32852
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie1c46ca552888b347a4ff810003e0fcc818f832f
Gerrit-PatchSet: 5
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Martin Peřina <mper...@redhat.com>
Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com>
Gerrit-Reviewer: Martin Peřina <mper...@redhat.com>
Gerrit-Reviewer: Oved Ourfali <oourf...@redhat.com>
Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com>
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to