From edc1b20132755b1f69e0e6f51b5cbdca4dc80786 Mon Sep 17 00:00:00 2001 From: Ron Levine Date: Thu, 17 Mar 2016 17:38:52 -0400 Subject: [PATCH] Output a summary of WARN messages --- .../VariantAnnotatorIntegrationTest.java | 1 - .../utils/commandline/CommandLineProgram.java | 48 ++++-- .../gatk/utils/commandline/ListAppender.java | 146 ++++++++++++++++++ .../commandline/ListAppenderUnitTest.java | 95 ++++++++++++ 4 files changed, 276 insertions(+), 14 deletions(-) create mode 100644 public/gatk-utils/src/main/java/org/broadinstitute/gatk/utils/commandline/ListAppender.java create mode 100644 public/gatk-utils/src/test/java/org/broadinstitute/gatk/utils/commandline/ListAppenderUnitTest.java diff --git a/protected/gatk-tools-protected/src/test/java/org/broadinstitute/gatk/tools/walkers/annotator/VariantAnnotatorIntegrationTest.java b/protected/gatk-tools-protected/src/test/java/org/broadinstitute/gatk/tools/walkers/annotator/VariantAnnotatorIntegrationTest.java index b0f1ddc4e..ca310e4ab 100644 --- a/protected/gatk-tools-protected/src/test/java/org/broadinstitute/gatk/tools/walkers/annotator/VariantAnnotatorIntegrationTest.java +++ b/protected/gatk-tools-protected/src/test/java/org/broadinstitute/gatk/tools/walkers/annotator/VariantAnnotatorIntegrationTest.java @@ -53,7 +53,6 @@ package org.broadinstitute.gatk.tools.walkers.annotator; import htsjdk.tribble.readers.LineIterator; import htsjdk.tribble.readers.PositionalBufferedStream; -import htsjdk.variant.variantcontext.Genotype; import htsjdk.variant.vcf.VCFHeader; import htsjdk.variant.vcf.VCFHeaderLine; import org.broadinstitute.gatk.engine.walkers.WalkerTest; diff --git a/public/gatk-utils/src/main/java/org/broadinstitute/gatk/utils/commandline/CommandLineProgram.java b/public/gatk-utils/src/main/java/org/broadinstitute/gatk/utils/commandline/CommandLineProgram.java index e77aefe54..4f5afcc08 100644 --- a/public/gatk-utils/src/main/java/org/broadinstitute/gatk/utils/commandline/CommandLineProgram.java +++ b/public/gatk-utils/src/main/java/org/broadinstitute/gatk/utils/commandline/CommandLineProgram.java @@ -40,6 +40,8 @@ import java.util.*; public abstract class CommandLineProgram { + private static final int NUM_WARN_MESSAGES = 10; + /** The command-line program and the arguments it returned. */ public ParsingEngine parser = null; @@ -169,15 +171,21 @@ public abstract class CommandLineProgram { try { // setup our log layout - PatternLayout layout = new PatternLayout(); + PatternLayout layout = new PatternLayout(patternString); Logger logger = CommandLineUtils.getStingLogger(); + // Appender for saving logging messages to a list + final ListAppender listAppender = new ListAppender(layout, NUM_WARN_MESSAGES, Level.WARN); + + // add appender for saving logging messages to a list + logger.addAppender(listAppender); + // now set the layout of all the loggers to our layout CommandLineUtils.setLayout(logger, layout); // Initialize the logger using the defaults. - clp.setupLoggerLevel(layout); + clp.setupLoggerLevel(); // setup the parser ParsingEngine parser = clp.parser = new ParsingEngine(clp); @@ -200,7 +208,7 @@ public abstract class CommandLineProgram { parser.loadArgumentsIntoObject(clp); // Initialize the logger using the loaded command line. - clp.setupLoggerLevel(layout); + clp.setupLoggerLevel(); Class[] argumentSources = clp.getArgumentSources(); for (Class argumentSource : argumentSources) @@ -226,15 +234,14 @@ public abstract class CommandLineProgram { parser.loadArgumentsIntoObject(clp); // Initialize the logger using the loaded command line. - clp.setupLoggerLevel(layout); + clp.setupLoggerLevel(); } if ( ! dryRun ) { // if they specify a log location, output our data there if (clp.toFile != null) { - FileAppender appender; try { - appender = new FileAppender(layout, clp.toFile, false); + final FileAppender appender = new FileAppender(layout, clp.toFile, false); logger.addAppender(appender); } catch (IOException e) { throw new RuntimeException("Unable to re-route log output to " + clp.toFile + " make sure the destination exists"); @@ -246,6 +253,12 @@ public abstract class CommandLineProgram { // call the execute CommandLineProgram.result = clp.execute(); + + // write logging events + printDoneAndLogMessages(listAppender); + + // clear saved logging events + listAppender.close(); } } catch (ArgumentException e) { @@ -266,13 +279,10 @@ public abstract class CommandLineProgram { } /** - * this function checks the logger level passed in on the command line, taking the lowest - * level that was provided. - * @param layout Pattern layout to format based on the logger level. + * this function takes the logger level passed in on the command line and uses it to set the level of the logger + * @throws ArgumentException if the logging level is not valid (DEBUG, INFO, WARN, ERROR, FATAL, OFF) */ - private void setupLoggerLevel(PatternLayout layout) { - layout.setConversionPattern(patternString); - + private void setupLoggerLevel() { // set the default logger level Level par; if (logging_level.toUpperCase().equals("DEBUG")) { @@ -347,6 +357,18 @@ public abstract class CommandLineProgram { return parser.isArgumentPresent("version"); } + /** + * Print done and log messages saved in a list. + * + * @param listAppender Appender for saving logging messages to a list + */ + private static void printDoneAndLogMessages(final ListAppender listAppender) { + System.out.println("------------------------------------------------------------------------------------------"); + System.out.print("Done. "); + listAppender.write(); + System.out.println("------------------------------------------------------------------------------------------"); + } + /** * Print help and exit. */ @@ -376,7 +398,7 @@ public abstract class CommandLineProgram { * @param t the error */ public static void exitSystemWithError(String msg, final Throwable t) { - errorPrintf("------------------------------------------------------------------------------------------%n"); + errorPrintf("--%n"); errorPrintf("stack trace %n"); t.printStackTrace(); diff --git a/public/gatk-utils/src/main/java/org/broadinstitute/gatk/utils/commandline/ListAppender.java b/public/gatk-utils/src/main/java/org/broadinstitute/gatk/utils/commandline/ListAppender.java new file mode 100644 index 000000000..fe5a903d9 --- /dev/null +++ b/public/gatk-utils/src/main/java/org/broadinstitute/gatk/utils/commandline/ListAppender.java @@ -0,0 +1,146 @@ +/* +* Copyright 2012-2016 Broad Institute, Inc. +* +* Permission is hereby granted, free of charge, to any person +* obtaining a copy of this software and associated documentation +* files (the "Software"), to deal in the Software without +* restriction, including without limitation the rights to use, +* copy, modify, merge, publish, distribute, sublicense, and/or sell +* copies of the Software, and to permit persons to whom the +* Software is furnished to do so, subject to the following +* conditions: +* +* The above copyright notice and this permission notice shall be +* included in all copies or substantial portions of the Software. +* +* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, +* EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES +* OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND +* NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT +* HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, +* WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING +* FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR +* THE USE OR OTHER DEALINGS IN THE SOFTWARE. +*/ + +package org.broadinstitute.gatk.utils.commandline; + +import org.apache.log4j.AppenderSkeleton; +import org.apache.log4j.Layout; +import org.apache.log4j.Level; +import org.apache.log4j.spi.LoggingEvent; +import org.broadinstitute.gatk.utils.exceptions.ReviewedGATKException; + +import java.util.ArrayList; +import java.util.List; + +/** + * Appender for saving logging loggingEvents to a list + */ +public class ListAppender extends AppenderSkeleton { + /** + * List of logging events + * + */ + protected final List loggingEvents = new ArrayList<>(); + + /** + * number of logging events to keep + */ + protected int loggingEventsToKeep = 0; + + /** + * number of logging events to keep + */ + protected int numLoggingEvents = 0; + + /** + * Log warning level to keep + */ + protected Level logLevelToKeep = Level.WARN; + + /** + * Constructor + * + * @param layout output layout + * @param loggingEventsToKeep number of warnings to keep + * @param logLevelToKeep logging logLevelToKeep ( TRACE, DEBUG, INFO, WARN, ERROR, FATAL, OFF ) + */ + public ListAppender(final Layout layout, final int loggingEventsToKeep, final Level logLevelToKeep) { + this.layout = layout; + this.loggingEventsToKeep = loggingEventsToKeep; + this.logLevelToKeep = logLevelToKeep; + } + + /** + * Constructor + * + * Hide from use + */ + private ListAppender() {} + + /** + * Close the appender + * + * Clear out the logging events + */ + @Override + public synchronized void close() { + loggingEvents.clear(); + numLoggingEvents = 0; + } + + /** + * Is a message layout required? + * + * @return true + */ + @Override + public boolean requiresLayout() { + return true; + } + + /** + * Append log event to the list of logging events + * + * @param loggingEvent The logging events + */ + @Override + protected synchronized void append(final LoggingEvent loggingEvent) { + if ( loggingEvent.getLevel().equals(logLevelToKeep) ) { + numLoggingEvents++; + if ( numLoggingEvents <= loggingEventsToKeep ) + loggingEvents.add(loggingEvent); + } + } + + /** + * The string representation for the class data + * + * @return string containing the logging events + * @throws ReviewedGATKException if layout is null + */ + public synchronized String toString() { + final String msgType = logLevelToKeep.toString() + " messages"; + if ( loggingEvents.isEmpty() ) { + return "There were no " + new String(msgType).toLowerCase() + ".\n"; + } else { + String out = "There were " + Integer.toString(numLoggingEvents) + " " + msgType; + if ( layout == null ) + throw new ReviewedGATKException("layout cannot be null"); + out += ", the first " + loggingEvents.size() + " are repeated below.\n"; + for ( LoggingEvent event : loggingEvents ) { + out += layout.format(event); + } + return out; + } + } + + /** + * Write class data to standard out + */ + public void write() { + System.out.print(this); + } + +} diff --git a/public/gatk-utils/src/test/java/org/broadinstitute/gatk/utils/commandline/ListAppenderUnitTest.java b/public/gatk-utils/src/test/java/org/broadinstitute/gatk/utils/commandline/ListAppenderUnitTest.java new file mode 100644 index 000000000..124bea323 --- /dev/null +++ b/public/gatk-utils/src/test/java/org/broadinstitute/gatk/utils/commandline/ListAppenderUnitTest.java @@ -0,0 +1,95 @@ +/* +* Copyright 2012-2016 Broad Institute, Inc. +* +* Permission is hereby granted, free of charge, to any person +* obtaining a copy of this software and associated documentation +* files (the "Software"), to deal in the Software without +* restriction, including without limitation the rights to use, +* copy, modify, merge, publish, distribute, sublicense, and/or sell +* copies of the Software, and to permit persons to whom the +* Software is furnished to do so, subject to the following +* conditions: +* +* The above copyright notice and this permission notice shall be +* included in all copies or substantial portions of the Software. +* +* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, +* EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES +* OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND +* NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT +* HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, +* WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING +* FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR +* THE USE OR OTHER DEALINGS IN THE SOFTWARE. +*/ + +package org.broadinstitute.gatk.utils.commandline; + +import org.apache.log4j.Level; +import org.apache.log4j.Logger; +import org.apache.log4j.PatternLayout; +import org.apache.commons.lang.StringUtils; +import org.broadinstitute.gatk.utils.exceptions.ReviewedGATKException; +import org.testng.Assert; +import org.testng.annotations.Test; + +public class ListAppenderUnitTest { + + private final static int EVENTS_TO_KEEP = 10; + private final static int EVENTS = 200; + private final static String MSG_TO_LOG = ": All work and no play makes Jack a dull boy."; + private final static String MSG_TOO_LONG = ": Don't write this one."; + private final static Logger LOGGER = Logger.getLogger(ListAppenderUnitTest.class); + + @Test + public void testListAppender() { + + LOGGER.removeAllAppenders(); + final ListAppender listAppender = new ListAppender(new PatternLayout(PatternLayout.DEFAULT_CONVERSION_PATTERN), EVENTS_TO_KEEP, Level.WARN); + LOGGER.addAppender(listAppender); + + for ( int i = 0; i < EVENTS; i++ ) { + LOGGER.warn(i + MSG_TO_LOG); + LOGGER.info(i + MSG_TOO_LONG); + } + + final String listAppenderString = listAppender.toString(); + listAppender.write(); + + Assert.assertEquals(StringUtils.countMatches(listAppenderString, MSG_TO_LOG), EVENTS_TO_KEEP); + Assert.assertFalse(listAppenderString.contains(MSG_TOO_LONG)); + Assert.assertTrue(listAppenderString.contains(Integer.toString(EVENTS_TO_KEEP))); + Assert.assertTrue(listAppenderString.contains(listAppender.logLevelToKeep.toString())); + Assert.assertEquals(listAppender.numLoggingEvents, EVENTS); + } + + @Test + public void testListAppenderNoWarnMsgs() { + + LOGGER.removeAllAppenders(); + final ListAppender listAppender = new ListAppender(new PatternLayout(PatternLayout.DEFAULT_CONVERSION_PATTERN), EVENTS_TO_KEEP, Level.WARN); + LOGGER.addAppender(listAppender); + + for ( int i = 0; i < EVENTS; i++ ) { + LOGGER.info(i + MSG_TOO_LONG); + } + + final String listAppenderString = listAppender.toString(); + Assert.assertEquals(StringUtils.countMatches(listAppenderString, MSG_TO_LOG), 0); + Assert.assertFalse(listAppenderString.contains(MSG_TOO_LONG)); + Assert.assertFalse(listAppenderString.contains(Integer.toString(EVENTS_TO_KEEP))); + Assert.assertTrue(listAppenderString.contains(listAppender.logLevelToKeep.toString().toLowerCase())); + Assert.assertEquals(listAppender.numLoggingEvents, 0); + } + + @Test(expectedExceptions = ReviewedGATKException.class) + public void testListAppenderMissingLayout() { + + LOGGER.removeAllAppenders(); + final ListAppender listAppender = new ListAppender(null, 1, Level.WARN); + LOGGER.addAppender(listAppender); + LOGGER.warn("Warning"); + + final String listAppenderString = listAppender.toString(); + } +}