You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by do...@apache.org on 2021/01/16 00:44:44 UTC
[geode] branch support/1.13 updated: GEODE-8798: Improve output
from export logs command (#5863)
This is an automated email from the ASF dual-hosted git repository.
donalevans pushed a commit to branch support/1.13
in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/support/1.13 by this push:
new ac11ccf GEODE-8798: Improve output from export logs command (#5863)
ac11ccf is described below
commit ac11ccf995642e1efc2a30dcd7dfa410e342bf44
Author: Donal Evans <do...@pivotal.io>
AuthorDate: Fri Jan 8 15:13:35 2021 -0800
GEODE-8798: Improve output from export logs command (#5863)
- Validate the format of the --start-time and --end-time arguments
- Validate that the --start-time and --end-time arguments can be
correctly parsed
- Output the values for --start-time and --end-time as used by the
function to allow users to check that they're as expected when specified
- Update documentation to reflect the new output and better explain the
behaviour when the arguments are provided
Authored-by: Donal Evans <do...@vmware.com>
(cherry picked from commit 94c3aea4821eed49ab2420333cd2544d81b0094f)
---
.../gfsh/command-pages/export.html.md.erb | 13 +-
.../cli/commands/ExportLogsInterceptor.java | 145 +++++++++++++++++++--
.../cli/commands/ExportLogsInterceptorTest.java | 80 +++++++++++-
3 files changed, 219 insertions(+), 19 deletions(-)
diff --git a/geode-docs/tools_modules/gfsh/command-pages/export.html.md.erb b/geode-docs/tools_modules/gfsh/command-pages/export.html.md.erb
index 65c8423..100e69d 100644
--- a/geode-docs/tools_modules/gfsh/command-pages/export.html.md.erb
+++ b/geode-docs/tools_modules/gfsh/command-pages/export.html.md.erb
@@ -185,7 +185,7 @@ Export logs to a given directory.
All files that have logs in the specified time range will be exported. If no time range is specified, all logs will be exported.
-The `--dir` parameter specifies a local directory to which log files will be written. This is used only when you are exporting logs using an http connection. If executed over http, the zip archive will be saved in the specified directory on the user's client machine. If not specified, logs are written to the location specified by the `user.dir` system property. When the command is executed over JMX, logs will be saved as `exportedlogs_xxx.zip` in the connected locator's working directory.
+The `--dir` parameter specifies a local directory to which log files will be written. This is used only when you are exporting logs using an http connection. If executed over http, the zip archive will be saved in the specified directory on the user's client machine. If not specified, logs are written to the location specified by the `user.dir` system property. When the command is executed over JMX, logs will be saved as `exportedLogs_xxx.zip` in the connected locator's working directory.
**Availability:** Online. You must be connected in `gfsh` to a JMX Manager member to use this command.
@@ -208,8 +208,8 @@ export logs [--dir=value] [--groups=value(,value)*] [--members=value(,value)*]
| <span class="keyword parmname">\\-\\-log-level</span> | Minimum level of log entries to export. Valid values are: `OFF`, `FATAL`, `ERROR`, `WARN`, `INFO`, `DEBUG`, `TRACE`, and `ALL`. | `INFO` |
| <span class="keyword parmname">\\-\\-only-log-level</span> | Whether to only include only entries that exactly match the <span class="keyword parmname">\\-\\-log-level</span> specified. | false |
| <span class="keyword parmname">‑‑merge‑log</span> | Whether to merge logs after exporting to the target directory (deprecated). | false |
-| <span class="keyword parmname">\\-\\-start-time</span> | Log entries that occurred after this time will be exported. Format: yyyy/MM/dd/HH/mm/ss/SSS/z OR yyyy/MM/dd | no limit |
-| <span class="keyword parmname">\\-\\-end-time</span> | Log entries that occurred before this time will be exported. Format: yyyy/MM/dd/HH/mm/ss/SSS/z OR yyyy/MM/dd | no limit |
+| <span class="keyword parmname">\\-\\-start-time</span> | Log entries that occurred after this time will be exported. Format: `yyyy/MM/dd/HH/mm/ss/SSS/z` OR `yyyy/MM/dd`. When using the `yyyy/MM/dd/HH/mm/ss/SSS/z` format, the time zone, denoted `z`, should be specified as either a 3-letter time zone such as `PST` or as an offset to GMT/UTC such as `GMT+08:00`. | no limit |
+| <span class="keyword parmname">\\-\\-end-time</span> | Log entries that occurred before this time will be exported. Format: `yyyy/MM/dd/HH/mm/ss/SSS/z` OR `yyyy/MM/dd`. When using the `yyyy/MM/dd/HH/mm/ss/SSS/z` format, the time zone, denoted `z`, should be specified as either a 3-letter time zone such as `PST` or as an offset to GMT/UTC such as `GMT+08:00`. An end time specified by only a date implements a time of `00:00`. This exports logs written up until `23:59:59.999` on the [...]
| <span class="keyword parmname">\\-\\-logs-only</span> | Whether to export only logs (not statistics) | If parameter not specified: false. If parameter specified without a value: true |
| <span class="keyword parmname">\\-\\-stats-only</span> | Whether to export only statistics (not logs) | If parameter not specified: false. If parameter specified without a value: true |
| <span class="keyword parmname">\\-\\-file-size-limit</span> | Limits total unzipped size of the exported files. Specify 0 (zero) for no limit. Value is in megabytes by default or [k,m,g,t] may be specified. | If parameter not specified: 100m. If parameter specified without a value: 0 |
@@ -222,6 +222,13 @@ Logs exported to the connected member's file system: /my-locator/data/logs/expor
```
``` pre
+gfsh>export logs --start-time=2020/12/14/12/00/00/000/GMT-08:00 --end-time=2020/12/27 --dir=data/logs
+Start time parsed as 2020-12-14T12:00 PST
+End time parsed as 2020-12-27T00:00 PST
+Logs exported to the connected member's file system: /my-locator/data/logs/exportedLogs_1608165308676.zip
+```
+
+``` pre
gfsh>export logs --dir=data/logs --file-size-limit=1k
Estimated exported logs expanded file size = 95599, file-size-limit = 1024.
To disable exported logs file size check use option "--file-size-limit=0".
diff --git a/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsInterceptor.java b/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsInterceptor.java
index 8aa749a..5ddd010 100644
--- a/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsInterceptor.java
+++ b/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsInterceptor.java
@@ -14,11 +14,20 @@
*/
package org.apache.geode.management.internal.cli.commands;
+import static org.apache.geode.management.internal.cli.commands.ExportLogsCommand.FORMAT;
+import static org.apache.geode.management.internal.cli.commands.ExportLogsCommand.ONLY_DATE_FORMAT;
+import static org.apache.geode.management.internal.i18n.CliStrings.EXPORT_LOGS__ENDTIME;
+import static org.apache.geode.management.internal.i18n.CliStrings.EXPORT_LOGS__STARTTIME;
+
import java.io.File;
import java.io.IOException;
import java.nio.file.Path;
import java.nio.file.Paths;
+import java.text.ParseException;
+import java.text.SimpleDateFormat;
import java.time.LocalDateTime;
+import java.util.TimeZone;
+import java.util.regex.Pattern;
import org.apache.commons.io.FileUtils;
import org.apache.commons.lang3.StringUtils;
@@ -39,6 +48,13 @@ import org.apache.geode.management.internal.cli.result.model.ResultModel;
public class ExportLogsInterceptor extends AbstractCliAroundInterceptor {
private static final Logger logger = LogService.getLogger();
+ /** To match {@link ExportLogsCommand#FORMAT} */
+ private static final Pattern DATE_AND_TIME_PATTERN =
+ Pattern.compile("^\\d{4}(/\\d{2}){5}/\\d{3}/.{3,}");
+
+ /** To match {@link ExportLogsCommand#ONLY_DATE_FORMAT} */
+ private static final Pattern DATE_ONLY_PATTERN = Pattern.compile("^\\d{4}(/\\d{2}){2}$");
+
@Override
public ResultModel preExecution(GfshParseResult parseResult) {
@@ -54,13 +70,25 @@ public class ExportLogsInterceptor extends AbstractCliAroundInterceptor {
return ResultModel.createError("Invalid log level: " + logLevel);
}
- // validate start date and end date
- String start = parseResult.getParamValueAsString("start-time");
- String end = parseResult.getParamValueAsString("end-time");
- if (start != null && end != null) {
- // need to make sure end is later than start
- LocalDateTime startTime = ExportLogsFunction.parseTime(start);
- LocalDateTime endTime = ExportLogsFunction.parseTime(end);
+ String start = parseResult.getParamValueAsString(EXPORT_LOGS__STARTTIME);
+ String end = parseResult.getParamValueAsString(EXPORT_LOGS__ENDTIME);
+
+ // First check start date and end date format
+ ResultModel formatErrorResult = checkStartAndEndFormat(start, end);
+ if (formatErrorResult != null) {
+ return formatErrorResult;
+ }
+
+ // Check if start date and end date are able to be parsed correctly
+ ResultModel parseErrorResult = checkStartAndEndParsing(start, end);
+ if (parseErrorResult != null) {
+ return parseErrorResult;
+ }
+
+ // Check that end is later than start
+ LocalDateTime startTime = ExportLogsFunction.parseTime(start);
+ LocalDateTime endTime = ExportLogsFunction.parseTime(end);
+ if (startTime != null && endTime != null) {
if (startTime.isAfter(endTime)) {
return ResultModel.createError("start-time has to be earlier than end-time.");
}
@@ -79,6 +107,29 @@ public class ExportLogsInterceptor extends AbstractCliAroundInterceptor {
@Override
public ResultModel postExecution(GfshParseResult parseResult, ResultModel commandResult,
Path tempFile) {
+
+ StringBuilder output = new StringBuilder();
+
+ // Output start time as used by ExportLogsFunction, if specified
+ String startTime = parseResult.getParamValueAsString(EXPORT_LOGS__STARTTIME);
+ if (startTime != null && !startTime.isEmpty()) {
+ output.append("Start time parsed as ")
+ .append(ExportLogsFunction.parseTime(startTime))
+ .append(" ")
+ .append(TimeZone.getDefault().getDisplayName(false, TimeZone.SHORT))
+ .append("\n");
+ }
+
+ // Output end time as used by ExportLogsFunction, if specified
+ String endTime = parseResult.getParamValueAsString(EXPORT_LOGS__ENDTIME);
+ if (endTime != null && !endTime.isEmpty()) {
+ output.append("End time parsed as ")
+ .append(ExportLogsFunction.parseTime(endTime))
+ .append(" ")
+ .append(TimeZone.getDefault().getDisplayName(false, TimeZone.SHORT))
+ .append("\n");
+ }
+
if (tempFile != null) {
// in the command over http case, the command result is in the downloaded temp file
Path dirPath;
@@ -93,7 +144,9 @@ public class ExportLogsInterceptor extends AbstractCliAroundInterceptor {
try {
FileUtils.copyFile(tempFile.toFile(), exportedLogFile);
FileUtils.deleteQuietly(tempFile.toFile());
- return ResultModel.createInfo("Logs exported to: " + exportedLogFile.getAbsolutePath());
+ output.append("Logs exported to: ")
+ .append(exportedLogFile.getAbsolutePath());
+ return ResultModel.createInfo(output.toString());
} catch (IOException e) {
logger.error(e.getMessage(), e);
return ResultModel.createError(e.getMessage());
@@ -106,8 +159,78 @@ public class ExportLogsInterceptor extends AbstractCliAroundInterceptor {
}
// if there is no downloaded file. File is saved on the locator/manager.
- return ResultModel.createInfo(
- "Logs exported to the connected member's file system: "
- + commandResult.getFileToDownload().toString());
+ output.append("Logs exported to the connected member's file system: ")
+ .append(commandResult.getFileToDownload().toString());
+ return ResultModel.createInfo(output.toString());
+ }
+
+ private ResultModel checkStartAndEndFormat(String start, String end) {
+ StringBuilder formatErrorMessage = new StringBuilder();
+ boolean formatError = false;
+ if (start != null && !DATE_AND_TIME_PATTERN.matcher(start).matches()
+ && !DATE_ONLY_PATTERN.matcher(start).matches()) {
+ formatErrorMessage.append(EXPORT_LOGS__STARTTIME);
+ formatError = true;
+ }
+
+ if (end != null && !DATE_AND_TIME_PATTERN.matcher(end).matches()
+ && !DATE_ONLY_PATTERN.matcher(end).matches()) {
+ if (formatError) {
+ formatErrorMessage.append(" and ");
+ }
+ formatErrorMessage.append(EXPORT_LOGS__ENDTIME);
+ formatError = true;
+ }
+
+ if (formatError) {
+ formatErrorMessage.append(" had incorrect format. Valid formats are ")
+ .append(ONLY_DATE_FORMAT).append(" and ").append(FORMAT);
+ return ResultModel.createError(formatErrorMessage.toString());
+ }
+ return null;
+ }
+
+ private ResultModel checkStartAndEndParsing(String start, String end) {
+ StringBuilder parseErrorMessage = new StringBuilder();
+ boolean parseError = false;
+
+ SimpleDateFormat dateAndTimeFormat = new SimpleDateFormat(FORMAT);
+ SimpleDateFormat dateOnlyFormat = new SimpleDateFormat(ONLY_DATE_FORMAT);
+ if (start != null) {
+ try {
+ // If the input is intended to be parsed as date and time, use the date and time format
+ if (start.length() > 10) {
+ dateAndTimeFormat.parse(start);
+ } else {
+ dateOnlyFormat.parse(start);
+ }
+ } catch (ParseException e) {
+ parseErrorMessage.append(EXPORT_LOGS__STARTTIME);
+ parseError = true;
+ }
+ }
+
+ if (end != null) {
+ try {
+ // If the input is intended to be parsed as date and time, use the date and time format
+ if (end.length() > 10) {
+ dateAndTimeFormat.parse(end);
+ } else {
+ dateOnlyFormat.parse(end);
+ }
+ } catch (ParseException e) {
+ if (parseError) {
+ parseErrorMessage.append(" and ");
+ }
+ parseErrorMessage.append(EXPORT_LOGS__ENDTIME);
+ parseError = true;
+ }
+ }
+
+ if (parseError) {
+ parseErrorMessage.append(" could not be parsed to valid date/time.");
+ return ResultModel.createError(parseErrorMessage.toString());
+ }
+ return null;
}
}
diff --git a/geode-gfsh/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsInterceptorTest.java b/geode-gfsh/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsInterceptorTest.java
index 969f40b..b9af77b 100644
--- a/geode-gfsh/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsInterceptorTest.java
+++ b/geode-gfsh/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsInterceptorTest.java
@@ -14,6 +14,10 @@
*/
package org.apache.geode.management.internal.cli.commands;
+import static org.apache.geode.management.internal.cli.commands.ExportLogsCommand.FORMAT;
+import static org.apache.geode.management.internal.cli.commands.ExportLogsCommand.ONLY_DATE_FORMAT;
+import static org.apache.geode.management.internal.i18n.CliStrings.EXPORT_LOGS__ENDTIME;
+import static org.apache.geode.management.internal.i18n.CliStrings.EXPORT_LOGS__STARTTIME;
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.when;
@@ -53,21 +57,87 @@ public class ExportLogsInterceptorTest {
}
@Test
+ public void testStartEndFormat() {
+ // Correct date only format
+ when(parseResult.getParamValueAsString(EXPORT_LOGS__STARTTIME)).thenReturn("2000/01/01");
+ when(parseResult.getParamValueAsString(EXPORT_LOGS__ENDTIME)).thenReturn("2000/01/02");
+ result = interceptor.preExecution(parseResult);
+ assertThat(result.getInfoSection("info").getContent()).containsOnly("");
+
+ // Correct date and time format
+ when(parseResult.getParamValueAsString(EXPORT_LOGS__STARTTIME))
+ .thenReturn("2000/01/01/00/00/00/001/PST");
+ when(parseResult.getParamValueAsString(EXPORT_LOGS__ENDTIME))
+ .thenReturn("2000/01/02/00/00/00/001/GMT");
+ result = interceptor.preExecution(parseResult);
+ assertThat(result.getInfoSection("info").getContent()).containsOnly("");
+
+ // Incorrect date only format
+ when(parseResult.getParamValueAsString(EXPORT_LOGS__STARTTIME)).thenReturn("2000/123/01");
+ when(parseResult.getParamValueAsString(EXPORT_LOGS__ENDTIME)).thenReturn(null);
+ result = interceptor.preExecution(parseResult);
+ assertThat(result.getInfoSection("info").getContent())
+ .containsOnly("start-time had incorrect format. Valid formats are " + ONLY_DATE_FORMAT
+ + " and " + FORMAT);
+
+ // Incorrect date and time format
+ when(parseResult.getParamValueAsString(EXPORT_LOGS__STARTTIME)).thenReturn(null);
+ when(parseResult.getParamValueAsString(EXPORT_LOGS__ENDTIME))
+ .thenReturn("2000/01/02/00/00/00/001/0");
+ result = interceptor.preExecution(parseResult);
+ assertThat(result.getInfoSection("info").getContent()).containsOnly(
+ "end-time had incorrect format. Valid formats are " + ONLY_DATE_FORMAT + " and " + FORMAT);
+
+ // Empty string
+ when(parseResult.getParamValueAsString(EXPORT_LOGS__STARTTIME)).thenReturn("");
+ when(parseResult.getParamValueAsString(EXPORT_LOGS__ENDTIME)).thenReturn("");
+ result = interceptor.preExecution(parseResult);
+ assertThat(result.getInfoSection("info").getContent())
+ .containsOnly("start-time and end-time had incorrect format. Valid formats are "
+ + ONLY_DATE_FORMAT + " and " + FORMAT);
+ }
+
+ @Test
+ public void testStartEndParsing() {
+ // Parsable date only input
+ when(parseResult.getParamValueAsString(EXPORT_LOGS__STARTTIME)).thenReturn("2000/01/01");
+ when(parseResult.getParamValueAsString(EXPORT_LOGS__ENDTIME)).thenReturn("2000/01/02");
+ result = interceptor.preExecution(parseResult);
+ assertThat(result.getInfoSection("info").getContent()).containsOnly("");
+
+ // Parsable date and time input
+ when(parseResult.getParamValueAsString(EXPORT_LOGS__STARTTIME))
+ .thenReturn("2000/01/01/00/00/00/001/PST");
+ when(parseResult.getParamValueAsString(EXPORT_LOGS__ENDTIME))
+ .thenReturn("2000/01/02/00/00/00/001/GMT+01:00");
+ result = interceptor.preExecution(parseResult);
+ assertThat(result.getInfoSection("info").getContent()).containsOnly("");
+
+ // Non-parsable input
+ when(parseResult.getParamValueAsString(EXPORT_LOGS__STARTTIME)).thenReturn(null);
+ when(parseResult.getParamValueAsString(EXPORT_LOGS__ENDTIME))
+ .thenReturn("2000/01/02/00/00/00/001/NOT_A_TIMEZONE");
+ result = interceptor.preExecution(parseResult);
+ assertThat(result.getInfoSection("info").getContent())
+ .containsOnly("end-time could not be parsed to valid date/time.");
+ }
+
+ @Test
public void testStartEnd() {
- when(parseResult.getParamValueAsString("start-time")).thenReturn("2000/01/01");
- when(parseResult.getParamValueAsString("end-time")).thenReturn("2000/01/02");
+ when(parseResult.getParamValueAsString(EXPORT_LOGS__STARTTIME)).thenReturn("2000/01/01");
+ when(parseResult.getParamValueAsString(EXPORT_LOGS__ENDTIME)).thenReturn("2000/01/02");
result = interceptor.preExecution(parseResult);
assertThat(result.getInfoSection("info").getContent()).containsOnly("");
- when(parseResult.getParamValueAsString("start-time")).thenReturn("2000/01/02");
- when(parseResult.getParamValueAsString("end-time")).thenReturn("2000/01/01");
+ when(parseResult.getParamValueAsString(EXPORT_LOGS__STARTTIME)).thenReturn("2000/01/02");
+ when(parseResult.getParamValueAsString(EXPORT_LOGS__ENDTIME)).thenReturn("2000/01/01");
result = interceptor.preExecution(parseResult);
assertThat(result.getInfoSection("info").getContent())
.containsOnly("start-time has to be earlier than end-time.");
}
@Test
- public void testInclideStats() {
+ public void testIncludeStats() {
when(parseResult.getParamValue("logs-only")).thenReturn(true);
when(parseResult.getParamValue("stats-only")).thenReturn(false);
result = interceptor.preExecution(parseResult);