Skip to content

Commit

Permalink
GEODE-8798: Improve output from export logs command (apache#5863)
Browse files Browse the repository at this point in the history 
- 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 <[email protected]>
  • Loading branch information
@DonalEvans
DonalEvans authored Jan 8, 2021
1 parent 089c1ba commit 94c3aea
Show file tree
Hide file tree
Showing 3 changed files with 219 additions and 19 deletions.
13 changes: 10 additions & 3 deletions geode-docs/tools_modules/gfsh/command-pages/export.html.md.erb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand All @@ -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">&#8209;&#8209;merge&#8209;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 date prior to the one specified. | no limit |
| <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 |
Expand All @@ -221,6 +221,13 @@ gfsh>export logs --dir=data/logs
Logs exported to the connected member's file system: /my-locator/data/logs/exportedLogs_1489513007261.zip
```

``` 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.
Expand Down
145 changes: 134 additions & 11 deletions ...rc/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsInterceptor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -39,6 +48,13 @@
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) {

Expand All @@ -54,13 +70,25 @@ public ResultModel preExecution(GfshParseResult parseResult) {
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.");
}
Expand All @@ -79,6 +107,29 @@ public ResultModel preExecution(GfshParseResult parseResult) {
@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;
Expand All @@ -93,7 +144,9 @@ public ResultModel postExecution(GfshParseResult parseResult, ResultModel comman
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());
Expand All @@ -106,8 +159,78 @@ public ResultModel postExecution(GfshParseResult parseResult, ResultModel comman
}

// 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;
}
}
80 changes: 75 additions & 5 deletions ...est/java/org/apache/geode/management/internal/cli/commands/ExportLogsInterceptorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -52,22 +56,88 @@ public void testGroupAndMember() {
.containsOnly("Can't specify both group and member.");
}

@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);
Expand Down

0 comments on commit 94c3aea

Please sign in to comment.