GEODE-2716: export logs default behavior changed from filtering at log level INFO to ALL.
* Removed reliance on LogService.DEFAULT_LOG_LEVEL, added ExportLogCommand.DEFAULT_EXPORT_LOG_LEVEL. * Added DUnit test that fails under previous behavior. * this closes #439 Project: http://git-wip-us.apache.org/repos/asf/geode/repo Commit: http://git-wip-us.apache.org/repos/asf/geode/commit/5430c91f Tree: http://git-wip-us.apache.org/repos/asf/geode/tree/5430c91f Diff: http://git-wip-us.apache.org/repos/asf/geode/diff/5430c91f Branch: refs/heads/feature/GEODE-2632 Commit: 5430c91f61fdb9ae4b0d11758d2b8e5a18f52163 Parents: 669d3ed Author: Patrick Rhomberg <prhomb...@pivotal.io> Authored: Tue Mar 28 15:12:02 2017 -0700 Committer: Jinmei Liao <jil...@pivotal.io> Committed: Thu Apr 6 10:17:36 2017 -0700 ---------------------------------------------------------------------- .../geode/internal/logging/LogService.java | 1 - .../cli/commands/ExportLogsCommand.java | 4 +- .../cli/functions/ExportLogsFunction.java | 2 +- .../internal/cli/i18n/CliStrings.java | 4 +- .../cli/commands/ExportLogsDUnitTest.java | 7 +++ .../cli/functions/ExportLogsFunctionTest.java | 45 ++++++++++++++++++++ .../cli/commands/golden-help-offline.properties | 11 ++--- 7 files changed, 64 insertions(+), 10 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/geode/blob/5430c91f/geode-core/src/main/java/org/apache/geode/internal/logging/LogService.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/internal/logging/LogService.java b/geode-core/src/main/java/org/apache/geode/internal/logging/LogService.java index 1f8a564..5a229d3 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/logging/LogService.java +++ b/geode-core/src/main/java/org/apache/geode/internal/logging/LogService.java @@ -46,7 +46,6 @@ public class LogService extends LogManager { public static final String BASE_LOGGER_NAME = "org.apache.geode"; public static final String MAIN_LOGGER_NAME = "org.apache.geode"; public static final String SECURITY_LOGGER_NAME = "org.apache.geode.security"; - public static final String DEFAULT_LOG_LEVEL = "INFO"; public static final String GEODE_VERBOSE_FILTER = "{GEODE_VERBOSE}"; public static final String GEMFIRE_VERBOSE_FILTER = "{GEMFIRE_VERBOSE}"; http://git-wip-us.apache.org/repos/asf/geode/blob/5430c91f/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommand.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommand.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommand.java index fe9cecd..1d5c412 100644 --- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommand.java +++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommand.java @@ -57,6 +57,8 @@ public class ExportLogsCommand implements CommandMarker { public static final String FORMAT = "yyyy/MM/dd/HH/mm/ss/SSS/z"; public static final String ONLY_DATE_FORMAT = "yyyy/MM/dd"; + public final static String DEFAULT_EXPORT_LOG_LEVEL = "ALL"; + private static final Pattern DISK_SPACE_LIMIT_PATTERN = Pattern.compile("(\\d+)([mgtMGT]?)"); @CliCommand(value = CliStrings.EXPORT_LOGS, help = CliStrings.EXPORT_LOGS__HELP) @@ -77,7 +79,7 @@ public class ExportLogsCommand implements CommandMarker { optionContext = ConverterHint.ALL_MEMBER_IDNAME, help = CliStrings.EXPORT_LOGS__MEMBER__HELP) String[] memberIds, @CliOption(key = CliStrings.EXPORT_LOGS__LOGLEVEL, - unspecifiedDefaultValue = LogService.DEFAULT_LOG_LEVEL, + unspecifiedDefaultValue = DEFAULT_EXPORT_LOG_LEVEL, optionContext = ConverterHint.LOG_LEVEL, help = CliStrings.EXPORT_LOGS__LOGLEVEL__HELP) String logLevel, @CliOption(key = CliStrings.EXPORT_LOGS__UPTO_LOGLEVEL, unspecifiedDefaultValue = "false", http://git-wip-us.apache.org/repos/asf/geode/blob/5430c91f/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ExportLogsFunction.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ExportLogsFunction.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ExportLogsFunction.java index 13124c5..3ce1721 100644 --- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ExportLogsFunction.java +++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ExportLogsFunction.java @@ -168,7 +168,7 @@ public class ExportLogsFunction implements Function, InternalEntity { this.endTime = parseTime(endTime); if (StringUtils.isBlank(logLevel)) { - this.logLevel = Level.INFO; + this.logLevel = LogLevel.getLevel(ExportLogsCommand.DEFAULT_EXPORT_LOG_LEVEL); } else { this.logLevel = LogLevel.getLevel(logLevel); } http://git-wip-us.apache.org/repos/asf/geode/blob/5430c91f/geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java index dfc4cf1..67955dc 100644 --- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java +++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java @@ -1391,7 +1391,7 @@ public class CliStrings { public static final String EXPORT_LOGS__HELP = "Export the log files for a member or members."; public static final String EXPORT_LOGS__DIR = "dir"; public static final String EXPORT_LOGS__DIR__HELP = - "Local directory to which logs will be written. This is used only when you are exporting logs using an http connection. If not specified, logs are written to the location specified by the user.dir system property."; + "Directory to which logs will be written. This refers to a local directory when exporting logs using an http connection, but refers to the filesystem of the manager when connected via JMX. If not specified, logs are written to the location specified by the user.dir system property."; public static final String EXPORT_LOGS__MEMBER = "member"; public static final String EXPORT_LOGS__MEMBER__HELP = "Name/Id of the member whose log files will be exported."; @@ -1401,7 +1401,7 @@ public class CliStrings { public static final String EXPORT_LOGS__MSG__CANNOT_EXECUTE = "Cannot execute"; public static final String EXPORT_LOGS__LOGLEVEL = LOG_LEVEL; public static final String EXPORT_LOGS__LOGLEVEL__HELP = - "Minimum level of log entries to export. Valid values are: fatal, error, warn, info, debug, trace and all. The default is \"INFO\"."; + "Minimum level of log entries to export. Valid values are: fatal, error, warn, info, debug, trace and all. The default is \"ALL\"."; public static final String EXPORT_LOGS__UPTO_LOGLEVEL = "only-log-level"; public static final String EXPORT_LOGS__UPTO_LOGLEVEL__HELP = "Whether to only include those entries that exactly match the --log-level specified."; http://git-wip-us.apache.org/repos/asf/geode/blob/5430c91f/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsDUnitTest.java ---------------------------------------------------------------------- diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsDUnitTest.java b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsDUnitTest.java index d0180d0..c960c8f 100644 --- a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsDUnitTest.java +++ b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsDUnitTest.java @@ -177,6 +177,13 @@ public class ExportLogsDUnitTest { } @Test + public void testExportWithNoOptionsGiven() throws Exception { + CommandResult result = gfshConnector.executeAndVerifyCommand("export logs"); + Set<String> acceptedLogLevels = Stream.of("info", "error", "debug").collect(toSet()); + verifyZipFileContents(acceptedLogLevels); + } + + @Test public void testExportWithNoFilters() throws Exception { gfshConnector.executeAndVerifyCommand("export logs --log-level=all"); http://git-wip-us.apache.org/repos/asf/geode/blob/5430c91f/geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/ExportLogsFunctionTest.java ---------------------------------------------------------------------- diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/ExportLogsFunctionTest.java b/geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/ExportLogsFunctionTest.java new file mode 100644 index 0000000..4e72444 --- /dev/null +++ b/geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/ExportLogsFunctionTest.java @@ -0,0 +1,45 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more contributor license + * agreements. See the NOTICE file distributed with this work for additional information regarding + * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the License. You may obtain a + * copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + * + */ + +package org.apache.geode.management.internal.cli.functions; + +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.assertEquals; + +import org.apache.geode.internal.logging.log4j.LogLevel; +import org.apache.geode.management.internal.cli.commands.ExportLogsCommand; +import org.apache.geode.test.junit.categories.UnitTest; +import org.junit.Test; +import org.apache.logging.log4j.Level; +import org.junit.experimental.categories.Category; + +@Category(UnitTest.class) +public class ExportLogsFunctionTest { + @Test + public void defaultExportLogLevelShouldBeAll() throws Exception { + assertTrue(ExportLogsCommand.DEFAULT_EXPORT_LOG_LEVEL.equals("ALL")); + assertEquals(LogLevel.getLevel(ExportLogsCommand.DEFAULT_EXPORT_LOG_LEVEL), Level.ALL); + } + + @Test + public void defaultExportLogLevelShouldBeAllViaArgs() throws Exception { + ExportLogsFunction.Args args = new ExportLogsFunction.Args("", "", "", false, false, false); + assertEquals(args.getLogLevel(), Level.ALL); + ExportLogsFunction.Args args2 = new ExportLogsFunction.Args("", "", null, false, false, false); + assertEquals(args2.getLogLevel(), Level.ALL); + } + +} http://git-wip-us.apache.org/repos/asf/geode/blob/5430c91f/geode-core/src/test/resources/org/apache/geode/management/internal/cli/commands/golden-help-offline.properties ---------------------------------------------------------------------- diff --git a/geode-core/src/test/resources/org/apache/geode/management/internal/cli/commands/golden-help-offline.properties b/geode-core/src/test/resources/org/apache/geode/management/internal/cli/commands/golden-help-offline.properties index 3c56def..af6b4bf 100644 --- a/geode-core/src/test/resources/org/apache/geode/management/internal/cli/commands/golden-help-offline.properties +++ b/geode-core/src/test/resources/org/apache/geode/management/internal/cli/commands/golden-help-offline.properties @@ -1473,9 +1473,10 @@ SYNTAX\n\ \ \ \ \ [--end-time=value] [--logs-only(=value)?] [--stats-only(=value)?]\n\ PARAMETERS\n\ \ \ \ \ dir\n\ -\ \ \ \ \ \ \ \ Local directory to which logs will be written. This is used only when you are exporting\n\ -\ \ \ \ \ \ \ \ logs using an http connection. If not specified, logs are written to the location specified\n\ -\ \ \ \ \ \ \ \ by the user.dir system property.\n\ +\ \ \ \ \ \ \ \ Directory to which logs will be written. This refers to a local directory when exporting\n\ +\ \ \ \ \ \ \ \ logs using an http connection, but refers to the filesystem of the manager when connected\n\ +\ \ \ \ \ \ \ \ via JMX. If not specified, logs are written to the location specified by the user.dir\n\ +\ \ \ \ \ \ \ \ system property.\n\ \ \ \ \ \ \ \ \ Required: false\n\ \ \ \ \ group\n\ \ \ \ \ \ \ \ \ Group of members whose log files will be exported.\n\ @@ -1485,9 +1486,9 @@ PARAMETERS\n\ \ \ \ \ \ \ \ \ Required: false\n\ \ \ \ \ log-level\n\ \ \ \ \ \ \ \ \ Minimum level of log entries to export. Valid values are: fatal, error, warn, info, debug,\n\ -\ \ \ \ \ \ \ \ trace and all. The default is "INFO".\n\ +\ \ \ \ \ \ \ \ trace and all. The default is "ALL".\n\ \ \ \ \ \ \ \ \ Required: false\n\ -\ \ \ \ \ \ \ \ Default (if the parameter is not specified): INFO\n\ +\ \ \ \ \ \ \ \ Default (if the parameter is not specified): ALL\n\ \ \ \ \ only-log-level\n\ \ \ \ \ \ \ \ \ Whether to only include those entries that exactly match the --log-level specified.\n\ \ \ \ \ \ \ \ \ Required: false\n\
