Merge pull request #1888 from newrelic/daily-log-fix

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

newrelic-agent/src/main/java/com/newrelic/agent/logging/ClearExpiredLogsRunnable.java

@@ -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;
@@ -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);
  }
@@ -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];
- }
 }

newrelic-agent/src/main/java/com/newrelic/agent/logging/FileAppenderFactory.java

@@ -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

newrelic-agent/src/test/java/com/newrelic/agent/logging/ClearExpiredLogsRunnableTest.java

@@ -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);
  }
 
@@ -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();
@@ -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;
@@ -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;
@@ -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;
@@ -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"));
@@ -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")));
@@ -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();