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.

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

Adding rolled custom logs in support bundle #627

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

nevingeorgesunny
Copy link
Contributor

No description provided.

@nevingeorgesunny nevingeorgesunny requested a review from a team as a code owner February 18, 2025 08:39
@nevingeorgesunny nevingeorgesunny changed the title [BEE-55630] Adding rolled custom logs in support bundle Adding rolled custom logs in support bundle Feb 19, 2025
@jglick
Copy link
Member

jglick commented Feb 20, 2025

Test failure sounds legitimate.

@@ -95,6 +95,30 @@ public Iterable<LogRecord> getLogRecords() {
}
});
}

// Add rotated log files
File[] rotatedLogFiles = customLogs.listFiles((dir, filename) -> filename.matches(name + "\\.log\\.\\d+"));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW I find

Suggested change
File[] rotatedLogFiles = customLogs.listFiles((dir, filename) -> filename.matches(name + "\\.log\\.\\d+"));
File[] rotatedLogFiles = customLogs.listFiles((dir, filename) -> filename.matches(name + "[.]log[.][0-9]+"));

a bit more legible (so I do not need to mentally parse out backslashes).

String logRotationNumber = logNameParts[logNameParts.length - 1];

result.add(
new FileContent(rotatedEntryName, new String[] {name, logRotationNumber}, rotatedLogFile));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logRotationNumber is not actually a “filterable parameter”. name would be.

Comment on lines 96 to 97
try (FileWriter writer = new FileWriter(new File(customLogsDir, "test1.log"))) {
writer.write("test1 one");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is a Files method for this

@@ -44,7 +44,7 @@ public class CustomLogs extends Component {

private static final Logger LOGGER = Logger.getLogger(CustomLogs.class.getName());
private static final int MAX_ROTATE_LOGS = Integer.getInteger(CustomLogs.class.getName() + ".MAX_ROTATE_LOGS", 9);
private static final File customLogs = new File(SafeTimerTask.getLogsRoot(), "custom");
private final File customLogs = new File(SafeTimerTask.getLogsRoot(), "custom");
Copy link
Contributor Author

@nevingeorgesunny nevingeorgesunny Feb 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed static as when I was running a test class, [CustomLogsTest.java], it has 3 test in it for some reason SafeTimerTask.getLogsRoot() was same for all test , due to which test were failing ,

removing static fixed this , i think its similar to #617 (comment)

[edited, please paste raw discussion permalinks]

}
}

private static final class LogFile {
private final RewindableRotatingFileOutputStream stream;
private final Handler handler;
private int count;
private final File customLogs = new File(SafeTimerTask.getLogsRoot(), "custom");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or pass it in the constructor.

try (FileWriter writer = new FileWriter(new File(customLogsDir, "nonRotatedCustomLog.log"))) {
writer.write("nonRotatedCustomLog one");
}
Files.write(Paths.get(customLogsDir.getPath(), "test1.log"), "test1 one".getBytes(), StandardOpenOption.CREATE);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just

Suggested change
Files.write(Paths.get(customLogsDir.getPath(), "test1.log"), "test1 one".getBytes(), StandardOpenOption.CREATE);
Files.write(Paths.get(customLogsDir.getPath(), "test1.log"), "test1 one".getBytes());

suffices (ditto elsewhere)

@jglick
Copy link
Member

jglick commented Feb 20, 2025

CustomLogsTest.testCustomLogsContent failure is reproducible for me in trunk in a Windows 10 VM. Looking into it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants