Martin Peřina 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: > uutils is a better place, as we do not want to pull entire util dependencie This requires to be in bootclasspath, because: 1) Logging is configured using properties, configuration is executed prior to any main method 2) The class is contained in different module (utils) than it's needed (tools and builtin) 3) LogManager is using simple classloader incompatible with jboss modules classloader, so the class is needed to be part of bootclasspath Also this formatter converts SEVERE to ERROR, so it's not only about debug level names. So I will move this into its own module 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. If we are not storing instance of "org.ovirt" logger, then findbugs raises an error described above and patch is marked as failed build. 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. AFAIK there's no way to define level name aliases in logging.properties. I will change code to use toLowercase() and equals() 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. I will try, but I'm almost sure you canned define a file handler without a filename, which can be entered as a part of tool's command line options 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. Formatter can be specified only for existing handler, but OK, I will rename it to custom format property name. -- 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
