Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Minor refactoring of daily logfile rolling job to support all platforms. #1888

Merged
merged 2 commits into from
May 13, 2024
Merged

Minor refactoring of daily logfile rolling job to support all platforms. #1888

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a9b8d5f
support for windows hosts; minor refactoring
jtduffy May 8, 2024
ea1ae83
Handle file ref with no parent folder
jtduffy May 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 7 additions & 12 deletions newrelic-agent/src/main/java/com/newrelic/agent/logging/ClearExpiredLogsRunnable.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import com.newrelic.api.agent.NewRelic;

import java.io.File;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
Expand Down Expand Up @@ -36,15 +37,15 @@ public class ClearExpiredLogsRunnable implements Runnable {
/**
* Constructs a ClearExpiredLogFilesRunnable object.
*
* @param logDirectoryPath the directory path where log files are located
* @param fileCount the number of days to keep log files before deleting them
* @param filePrefixPath the file prefix used to filter log files
* @param fileCount the number of days to keep log files before deleting them
* @param logfile the full path and filename of the agent logfile
*/
public ClearExpiredLogsRunnable(Path logDirectoryPath, int fileCount, String filePrefixPath) {
this.logDirectoryPath = logDirectoryPath;
public ClearExpiredLogsRunnable(int fileCount, String logfile) {
File absoluteLogFilename = new File(logfile);
this.daysToKeepFiles = fileCount;
this.logDirectoryPath = absoluteLogFilename.getParent() == null ? Paths.get("./") : Paths.get(absoluteLogFilename.getParent());

String fileNamePrefix = extractFileNamePrefix(filePrefixPath);
String fileNamePrefix = absoluteLogFilename.getName();
pattern = Pattern.compile(fileNamePrefix.replace(".", "\\.")
+ DATE_REGEX);
}
Expand Down Expand Up @@ -88,10 +89,4 @@ private String extractDateString(String fileName) {
Matcher matcher = pattern.matcher(fileName);
return matcher.find() ? matcher.group(1) : null;
}

private String extractFileNamePrefix(String fileName) {
// Extract the file name prefix from the given file path
String[] parts = fileName.split("/");
return parts[parts.length - 1];
}
}
2 changes: 1 addition & 1 deletion newrelic-agent/src/main/java/com/newrelic/agent/logging/FileAppenderFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ private RollingFileAppender buildDailyRollingAppender() {
return thread;
});
executorService.scheduleWithFixedDelay(
new ClearExpiredLogsRunnable(directory, fileCount, fileName),
new ClearExpiredLogsRunnable(fileCount, fileName),
INITIAL_DELAY_SECONDS,
REPEAT_INTERVAL_SECONDS,
TimeUnit.SECONDS
Expand Down
35 changes: 19 additions & 16 deletions newrelic-agent/src/test/java/com/newrelic/agent/logging/ClearExpiredLogsRunnableTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,14 @@ public class ClearExpiredLogsRunnableTest {

private ClearExpiredLogsRunnable clearExpiredLogsRunnable;
private Path tempDir;
private String absoluteLogFilePath;
private final static String LOG_FILENAME = "newrelic.log";

@Before
public void setUp() throws IOException {
String fileNamePrefix = "testLogDir";
String fileNamePrefix = "nr_log_test";
tempDir = Files.createTempDirectory(fileNamePrefix);
absoluteLogFilePath = tempDir.toString() + "/" + LOG_FILENAME;
assertNotNull(tempDir);
}

Expand All @@ -43,7 +46,7 @@ public void tearDown() throws IOException {

@Test
public void testRunnableAgainstEmptyLogDirectory() throws IOException {
clearExpiredLogsRunnable = new ClearExpiredLogsRunnable(tempDir, 3, "testLogDir");
clearExpiredLogsRunnable = new ClearExpiredLogsRunnable(3, absoluteLogFilePath);
clearExpiredLogsRunnable.run();

long actualLogFileCount = Files.list(tempDir).count();
Expand All @@ -52,14 +55,14 @@ public void testRunnableAgainstEmptyLogDirectory() throws IOException {

@Test
public void testDeleteOldFilesWithLogExtensions() throws Exception {
addOldLogFilesWithExtensions(10, "testLogDir");
addOldLogFilesWithExtensions(10);

long expectedLogFileCount = 5;
long actualLogFileCount = Files.list(tempDir).count();

assertEquals(expectedLogFileCount, actualLogFileCount);

clearExpiredLogsRunnable = new ClearExpiredLogsRunnable(tempDir, 3, "testLogDir");
clearExpiredLogsRunnable = new ClearExpiredLogsRunnable(3, absoluteLogFilePath);
clearExpiredLogsRunnable.run();

expectedLogFileCount = 0;
Expand All @@ -70,14 +73,14 @@ public void testDeleteOldFilesWithLogExtensions() throws Exception {

@Test
public void testDeleteOldFilesWithoutLogExtensions() throws IOException {
addOldLogFilesWithoutExtensions(10, "testLogDir");
addOldLogFilesWithoutExtensions(10);

long expectedLogFileCount = 5;
long actualLogFileCount = Files.list(tempDir).count();

assertEquals(expectedLogFileCount, actualLogFileCount);

clearExpiredLogsRunnable = new ClearExpiredLogsRunnable(tempDir, 3, "testLogDir");
clearExpiredLogsRunnable = new ClearExpiredLogsRunnable(3, absoluteLogFilePath);
clearExpiredLogsRunnable.run();

expectedLogFileCount = 0;
Expand All @@ -91,13 +94,13 @@ public void testSkipInvalidLogFiles() throws Exception {
Files.createFile(tempDir.resolve("invalid_log_file.txt"));
Files.createFile(tempDir.resolve("invalid.txt"));
Files.createFile(tempDir.resolve("2024-02-04"));
addOldLogFilesWithExtensions(10, "testLogDir");
addOldLogFilesWithExtensions(10);

long expectedLogFileCount = 8;
long actualLogFileCount = Files.list(tempDir).count();
assertEquals(expectedLogFileCount, actualLogFileCount);

clearExpiredLogsRunnable = new ClearExpiredLogsRunnable(tempDir, 1, "testLogDir");
clearExpiredLogsRunnable = new ClearExpiredLogsRunnable(1, absoluteLogFilePath);
clearExpiredLogsRunnable.run();

expectedLogFileCount = 3;
Expand All @@ -114,16 +117,16 @@ public void testNoPermissionToDeleteFiles() throws IOException {
Path readOnlyFile = Files.createFile(tempDir.resolve("readonly.log"));
readOnlyFile.toFile().setReadOnly();

clearExpiredLogsRunnable = new ClearExpiredLogsRunnable(tempDir, 3, "testLogDir");
clearExpiredLogsRunnable = new ClearExpiredLogsRunnable(3, absoluteLogFilePath);
clearExpiredLogsRunnable.run();

assertTrue("Read-only file should still exist", Files.exists(readOnlyFile));
}

@Test
public void testDirectoryWithMixedFiles() throws IOException {
addOldLogFilesWithExtensions(5, "testLogDir");
addOldLogFilesWithoutExtensions(5, "testLogDir");
addOldLogFilesWithExtensions(5);
addOldLogFilesWithoutExtensions(5);

// non-log files
Files.createFile(tempDir.resolve("testFile1.txt"));
Expand All @@ -135,7 +138,7 @@ public void testDirectoryWithMixedFiles() throws IOException {
long actualLogFileCount = Files.list(tempDir).count();
assertEquals(expectedLogFileCount, actualLogFileCount);

clearExpiredLogsRunnable = new ClearExpiredLogsRunnable(tempDir, 3, "testLogDir");
clearExpiredLogsRunnable = new ClearExpiredLogsRunnable(3, absoluteLogFilePath);
clearExpiredLogsRunnable.run();

assertTrue("Non-log file should exist", Files.exists(tempDir.resolve("testFile1.txt")));
Expand All @@ -146,28 +149,28 @@ public void testDirectoryWithMixedFiles() throws IOException {
assertEquals(expectedLogFileCount, actualLogFileCount);
}

private void addOldLogFilesWithExtensions(int daysAgo, String fileNamePrefix) throws IOException {
private void addOldLogFilesWithExtensions(int daysAgo) throws IOException {
Calendar calendar = Calendar.getInstance();
calendar.add(Calendar.DAY_OF_YEAR, -daysAgo);
Date date = calendar.getTime();
SimpleDateFormat dateFormat = new SimpleDateFormat("yyyy-MM-dd");

for (int i = 0; i < 5; i++) {
String fileName = fileNamePrefix + "." + dateFormat.format(date) + "." + (i + 1);
String fileName = LOG_FILENAME + "." + dateFormat.format(date) + "." + (i + 1);
Files.createFile(tempDir.resolve(fileName));
calendar.add(Calendar.MINUTE, 1);
date = calendar.getTime();
}
}

private void addOldLogFilesWithoutExtensions(int daysAgo, String fileNamePrefix) throws IOException {
private void addOldLogFilesWithoutExtensions(int daysAgo) throws IOException {
Calendar calendar = Calendar.getInstance();
calendar.add(Calendar.DAY_OF_YEAR, -daysAgo);
Date date = calendar.getTime();
SimpleDateFormat dateFormat = new SimpleDateFormat("yyyy-MM-dd");

for (int i = 0; i < 5; i++) {
String fileName = fileNamePrefix + "." + dateFormat.format(date);
String fileName = LOG_FILENAME + "." + dateFormat.format(date);
Files.createFile(tempDir.resolve(fileName));
calendar.add(Calendar.DAY_OF_YEAR, 1);
date = calendar.getTime();
Expand Down
Loading