Alon Bar-Lev has posted comments on this change. Change subject: tools: Replaces log4j with JUL as backend for manage-domains ......................................................................
Patch Set 3: (3 comments) http://gerrit.ovirt.org/#/c/32853/3/backend/manager/modules/builtin-extensions/src/main/java/org/ovirt/engine/extensions/aaa/builtin/tools/ManageDomainsExecutor.java File backend/manager/modules/builtin-extensions/src/main/java/org/ovirt/engine/extensions/aaa/builtin/tools/ManageDomainsExecutor.java: Line 28: Line 29: public static void setupCustomLogging(String configFile, String logFile, String logLevel) { Line 30: if (configFile != null) { Line 31: try (InputStream is = new FileInputStream(configFile)) { Line 32: JavaLoggingUtils.setupLogging(is); > This logging configuration file is entered by used in --log-config tool com I thought of having default embedded property file or user provided file, and not require any code to load this... much simpler especially for debugging. If you like, we can provide alternate logging property files via environment and not cmdline, so it will be easier for us to handle at wrapper level. Line 33: } catch (IOException ex) { Line 34: throw new IllegalArgumentException( Line 35: String.format("Error loading loggging configuration from '%s': %s", configFile, ex.getMessage()), Line 36: ex); http://gerrit.ovirt.org/#/c/32853/3/backend/manager/modules/builtin-extensions/src/main/resources/engine-manage-domains/logging.properties File backend/manager/modules/builtin-extensions/src/main/resources/engine-manage-domains/logging.properties: Line 6: # To change logging properties for engine-manage-domains user command line Line 7: # options --log-file, --log-level and --log-config Line 8: Line 9: # Set root level to ALL, so it can be changed per handler using command line parameter --log-level Line 10: .level=ALL > I just verified a few minutes ago, if I change to .level=OFF and in command but why? something is wrong. as you should be able to override the level, or provide somewhere default sane value. maybe this level is for the appender? and we need to set a level for the logger? Line 11: Line 12: # Default format of log line is: DATE TIME LEVEL [SOURCE] Message Stacktrace Line 9: # Set root level to ALL, so it can be changed per handler using command line parameter --log-level Line 10: .level=ALL Line 11: Line 12: # Default format of log line is: DATE TIME LEVEL [SOURCE] Message Stacktrace Line 13: 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 > In JavaLoggingUtils.addFileHandler(), this formatter is used only for log f this should be here not at code... this is why there are property files, so we can reduce the code. same should have done for the arguments... if you remember... but I gave up then :( -- To view, visit http://gerrit.ovirt.org/32853 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16afc336c26615978392f7d4ff911bf646c42df6 Gerrit-PatchSet: 3 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