Skip to content

Commit

Permalink
GEODE-2644: Cleanup logging related files
Browse files Browse the repository at this point in the history
Cleanup files that helped fix logging related test failures or helped
with reviewing and understanding code while working on GEODE-2644.

* Remove warnings and improve code cleanliness
* Improve testing and failure messages
  • Loading branch information
kirklund committed Nov 16, 2018
1 parent 225833b commit 1452763
Show file tree
Hide file tree
Showing 29 changed files with 345 additions and 449 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
*/
package org.apache.geode.admin.jmx.internal;

import static java.lang.System.lineSeparator;
import static org.apache.geode.admin.jmx.AgentConfig.DEFAULT_PROPERTY_FILE;
import static org.apache.geode.admin.jmx.internal.AgentConfigImpl.AGENT_PROPSFILE_PROPERTY_NAME;
import static org.apache.geode.admin.jmx.internal.AgentLauncher.AGENT_PROPS;
Expand All @@ -23,11 +24,13 @@
import static org.apache.geode.admin.jmx.internal.AgentLauncher.SHUTDOWN;
import static org.apache.geode.admin.jmx.internal.AgentLauncher.STARTING;
import static org.apache.geode.admin.jmx.internal.AgentLauncher.VMARGS;
import static org.apache.geode.test.awaitility.GeodeAwaitility.getTimeout;
import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.Assume.assumeFalse;

import java.io.File;
import java.io.IOException;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.Properties;
Expand All @@ -45,6 +48,8 @@

public class DeprecatedAgentLauncherIntegrationTest {

private static final long TIMEOUT = getTimeout().getValueInMS();

private String classpath;

@Rule
Expand Down Expand Up @@ -294,7 +299,11 @@ private static void runAgent(final String processOutputPattern, final String...
if (processOutputPattern != null) {
agentProcess.waitForOutputToMatch(processOutputPattern);
}
agentProcess.waitFor();
assertThat(agentProcess.waitFor(TIMEOUT)).as("Expecting process started with:" + lineSeparator()
+ " " + Arrays.asList(args) + lineSeparator() + "with output:" + lineSeparator() + " "
+ agentProcess.getOutput(true) + lineSeparator() + "to terminate with exit code: 0"
+ lineSeparator() + "but waitFor is still waiting after timeout: " + TIMEOUT + " seconds.")
.isEqualTo(0);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,12 @@
import org.apache.geode.test.junit.categories.LoggingTest;

/**
* Integration tests for log rolling with cache lifecycle.
* Integration tests for log rolling with {@link DistributedSystem} lifecycle.
*
* @since GemFire 6.5
*/
@Category(LoggingTest.class)
public class CacheLogRollingIntegrationTest {
public class LogRollingWithDistributedSystemIntegrationTest {

private static final int MAX_LOG_STATEMENTS = 100000;
private static final String SECURITY_PREFIX = "security_";
Expand Down Expand Up @@ -84,9 +84,9 @@ public void after() {
@Test
public void testSimpleStartRestartWithRolling() {
Properties config = createConfig();
config.put(LOG_FILE, logFile.getAbsolutePath());
config.put(LOG_FILE_SIZE_LIMIT, "1");
config.put(LOG_DISK_SPACE_LIMIT, "200");
config.setProperty(LOG_FILE, logFile.getAbsolutePath());
config.setProperty(LOG_FILE_SIZE_LIMIT, "1");
config.setProperty(LOG_DISK_SPACE_LIMIT, "200");

system = DistributedSystem.connect(config);
system.disconnect();
Expand All @@ -98,7 +98,8 @@ public void testSimpleStartRestartWithRolling() {
File newRolledLogFile = childFile(mainInt - 1, 1);

assertThat(newMetaFile).doesNotExist();
assertThat(newRolledLogFile).doesNotExist();
assertThat(newRolledLogFile).as("mainInt=" + mainInt + ", newMetaFile=" + newMetaFile
+ ", newRolledLogFile=" + newRolledLogFile).doesNotExist();

system = DistributedSystem.connect(config);

Expand All @@ -112,8 +113,8 @@ public void testSimpleStartRestartWithRolling() {
@Test
public void testStartWithRollingThenRestartWithRolling() throws Exception {
Properties config = createConfig();
config.put(LOG_FILE, logFile.getAbsolutePath());
config.put(LOG_FILE_SIZE_LIMIT, "1");
config.setProperty(LOG_FILE, logFile.getAbsolutePath());
config.setProperty(LOG_FILE_SIZE_LIMIT, "1");

system = DistributedSystem.connect(config);

Expand All @@ -139,8 +140,8 @@ public void testStartWithRollingThenRestartWithRolling() throws Exception {
@Test
public void testLogFileLayoutAndRolling() throws Exception {
Properties config = createConfig();
config.put(LOG_FILE, logFile.getAbsolutePath());
config.put(LOG_FILE_SIZE_LIMIT, "1");
config.setProperty(LOG_FILE, logFile.getAbsolutePath());
config.setProperty(LOG_FILE_SIZE_LIMIT, "1");

system = DistributedSystem.connect(config);

Expand All @@ -150,9 +151,9 @@ public void testLogFileLayoutAndRolling() throws Exception {
@Test
public void testSecurityLogFileLayoutAndRolling() throws Exception {
Properties config = createConfig();
config.put(LOG_FILE, logFile.getAbsolutePath());
config.put(LOG_FILE_SIZE_LIMIT, "1");
config.put(SECURITY_LOG_FILE, securityLogFile.getAbsolutePath());
config.setProperty(LOG_FILE, logFile.getAbsolutePath());
config.setProperty(LOG_FILE_SIZE_LIMIT, "1");
config.setProperty(SECURITY_LOG_FILE, securityLogFile.getAbsolutePath());

system = DistributedSystem.connect(config);

Expand All @@ -162,8 +163,8 @@ public void testSecurityLogFileLayoutAndRolling() throws Exception {
@Test
public void with_logFileSizeLimit_should_createMetaLogFile() {
Properties config = createConfig();
config.put(LOG_FILE, logFile.getAbsolutePath());
config.put(LOG_FILE_SIZE_LIMIT, "1");
config.setProperty(LOG_FILE, logFile.getAbsolutePath());
config.setProperty(LOG_FILE_SIZE_LIMIT, "1");

system = DistributedSystem.connect(config);

Expand All @@ -178,7 +179,7 @@ public void with_logFileSizeLimit_should_createMetaLogFile() {
@Test
public void without_logFileSizeLimit_shouldNot_createMetaLogFile() {
Properties config = createConfig();
config.put(LOG_FILE, logFile.getAbsolutePath());
config.setProperty(LOG_FILE, logFile.getAbsolutePath());

system = DistributedSystem.connect(config);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
import org.apache.geode.test.junit.categories.LoggingTest;

/**
* This class tests the functionality of the (new multi-threaded) {@link MergeLogFiles} utility.
* Integration tests for (new multi-threaded) {@link MergeLogFiles} utility.
*/
@Category(LoggingTest.class)
public class MergeLogFilesIntegrationTest {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,6 @@ public static void main(String args[]) {
}
SystemFailure.loadEmergencyClasses();

// log.info(Banner.getString(args));
final int port = parsePort(args[0]);
InetAddress address = null;
boolean peerLocator = true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@

/**
* Defines the common date format for GemFire and provides DateFormat instances.
*
*/
public class DateFormatter {

Expand All @@ -29,7 +28,7 @@ public class DateFormatter {
*/
public static final String FORMAT_STRING = "yyyy/MM/dd HH:mm:ss.SSS z";

private static final DateFormat timeFormatter = createDateFormat();
private static final DateFormat TIME_FORMATTER = createDateFormat();

/**
* Creates a SimpleDateFormat using {@link #FORMAT_STRING}.
Expand All @@ -39,7 +38,7 @@ public class DateFormatter {
* format concurrently, it must be synchronized externally.
*/
public static DateFormat createDateFormat() {
return new SimpleDateFormat(DateFormatter.FORMAT_STRING);
return new SimpleDateFormat(FORMAT_STRING);
}

/**
Expand All @@ -64,11 +63,11 @@ public static String getTimeStamp() {
* @param d a Date to format as a timestamp String.
* @return a String representation of the current time.
*/
public static String formatDate(Date d) {
public static String formatDate(final Date d) {
try {
synchronized (timeFormatter) {
synchronized (TIME_FORMATTER) {
// Need sync: see bug 21858
return timeFormatter.format(d);
return TIME_FORMATTER.format(d);
}
} catch (Exception e1) {
// Fix bug 21857
Expand All @@ -87,5 +86,7 @@ public static String formatDate(Date d) {
/**
* Do not instantiate this class.
*/
private DateFormatter() {}
private DateFormatter() {
// do not instantiate this class
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,33 +14,28 @@
*/
package org.apache.geode.internal.logging;

import java.io.IOException;
import java.io.PrintWriter;
import java.io.StringWriter;
import java.text.DateFormat;
import java.util.Date;
import java.util.logging.Formatter;
import java.util.logging.LogRecord;

import org.apache.geode.LogWriter;

/**
* Implementation of the standard JDK formatter that formats a message in GemFire's log format.
*/
public class GemFireFormatter extends Formatter {

/**
* Use the log writer to use some of its formatting code.
*/
private final LogWriter logWriter;

private final DateFormat dateFormat = DateFormatter.createDateFormat();

public GemFireFormatter(LogWriter logWriter) {
this.logWriter = logWriter;
public GemFireFormatter() {
// nothing
}

@Override
public String format(LogRecord record) {
java.io.StringWriter sw = new java.io.StringWriter();
public String format(final LogRecord record) {
StringWriter sw = new StringWriter();
PrintWriter pw = new PrintWriter(sw);

pw.println();
Expand All @@ -61,18 +56,6 @@ public String format(LogRecord record) {

pw.print(" msgSN=");
pw.print(record.getSequenceNumber());
// if (record.getLoggerName() != null) {
// pw.print(' ');
// pw.print(record.getLoggerName());
// }
// if (record.getSourceClassName() != null) {
// pw.print(' ');
// pw.print(record.getSourceClassName());
// }
// if (record.getSourceMethodName() != null) {
// pw.print(' ');
// pw.print(record.getSourceMethodName());
// }
pw.print(") ");

String msg = record.getMessage();
Expand All @@ -81,8 +64,7 @@ public String format(LogRecord record) {
LogWriterImpl.formatText(pw, msg, 40);
} catch (RuntimeException e) {
pw.println(msg);
pw.println(
"Ignoring the following exception:");
pw.println("Ignoring the following exception:");
e.printStackTrace(pw);
}
} else {
Expand All @@ -94,9 +76,8 @@ public String format(LogRecord record) {
pw.close();
try {
sw.close();
} catch (java.io.IOException ignore) {
} catch (IOException ignore) {
}
String result = sw.toString();
return result;
return sw.toString();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -35,39 +35,39 @@ public class GemFireHandler extends Handler {

public GemFireHandler(LogWriter logWriter) {
this.logWriter = logWriter;
this.setFormatter(new GemFireFormatter(logWriter));
setFormatter(new GemFireFormatter());
}

@Override
public void close() {
// clear the reference to GFE LogWriter
this.logWriter = null;
logWriter = null;
}

@Override
public void flush() {
// nothing needed
}

private String getMessage(LogRecord record) {
final StringBuilder b = new StringBuilder();
b.append('(').append("tid=" + record.getThreadID())
.append(" msgId=" + record.getSequenceNumber()).append(") ");
private String getMessage(final LogRecord record) {
StringBuilder stringBuilder = new StringBuilder();
stringBuilder.append('(').append("tid=").append(record.getThreadID())
.append(" msgId=").append(record.getSequenceNumber()).append(") ");
if (record.getMessage() != null) {
b.append(getFormatter().formatMessage(record));
stringBuilder.append(getFormatter().formatMessage(record));
}
return b.toString();
return stringBuilder.toString();
}

@Override
public void publish(LogRecord record) {
public void publish(final LogRecord record) {
if (isLoggable(record)) {
try {
if (this.logWriter instanceof LogWriterLogger) {
((LogWriterLogger) this.logWriter).log(record.getLevel().intValue(), getMessage(record),
if (logWriter instanceof LogWriterLogger) {
((LogWriterLogger) logWriter).log(record.getLevel().intValue(), getMessage(record),
record.getThrown());
} else {
((LogWriterImpl) this.logWriter).put(record.getLevel().intValue(), getMessage(record),
((LogWriterImpl) logWriter).put(record.getLevel().intValue(), getMessage(record),
record.getThrown());
}
} catch (GemFireException ex) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,11 @@
import java.io.PrintStream;
import java.io.PrintWriter;

import org.apache.geode.LogWriter;


/**
* Implementation of {@link org.apache.geode.LogWriter} that will write to a local stream.
* Implementation of {@link LogWriter} that will write to a local stream.
* <p>
* Note this class is no longer needed. It can be replaced by PureLogWriter.
*/
Expand Down
Loading

0 comments on commit 1452763

Please sign in to comment.