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
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@

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 final List<LogRecorder> logRecorders = Jenkins.get().getLog().getRecorders();

@NonNull
Expand Down Expand Up @@ -97,14 +97,15 @@
}

// Add rotated log files
File[] rotatedLogFiles = customLogs.listFiles((dir, filename) -> filename.matches(name + "\\.log\\.\\d+"));
File[] rotatedLogFiles =
customLogs.listFiles((dir, filename) -> filename.matches(name + "[.]log[.][0-9]+"));
if (rotatedLogFiles == null) {

Check warning on line 102 in src/main/java/com/cloudbees/jenkins/support/impl/CustomLogs.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line

Line 102 is only partially covered, one branch is missing
LOGGER.fine("No rotated logs found for : " + name);
return;

Check warning on line 104 in src/main/java/com/cloudbees/jenkins/support/impl/CustomLogs.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered lines

Lines 103-104 are not covered by tests
}

for (File rotatedLogFile : rotatedLogFiles) {
String rotatedEntryName = "nodes/master/logs/custom/{0}.log.{1}";
String rotatedEntryName = "nodes/master/logs/custom/{0}.log.";

try {
// Fetch the rotations number of the log
Expand All @@ -113,10 +114,9 @@
String logRotationNumber = logNameParts[logNameParts.length - 1];

result.add(
new FileContent(rotatedEntryName, new String[] {name, logRotationNumber}, rotatedLogFile));
new FileContent(rotatedEntryName + logRotationNumber, new String[] {name}, rotatedLogFile));
} catch (Exception e) {
LOGGER.warning(String.format(
"Error while adding rotated log files for '%s'. Error: %s", name, e.getMessage()));
LOGGER.log(Level.WARNING, "Error while adding rotated log files for " + name, e);

Check warning on line 119 in src/main/java/com/cloudbees/jenkins/support/impl/CustomLogs.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered lines

Lines 118-119 are not covered by tests
}
}
}
Expand All @@ -126,6 +126,7 @@
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.


@SuppressFBWarnings(
value = "RV_RETURN_VALUE_IGNORED_BAD_PRACTICE",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,10 @@
import hudson.security.Permission;
import hudson.triggers.SafeTimerTask;
import java.io.File;
import java.io.FileWriter;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Paths;
import java.nio.file.StandardOpenOption;
import java.util.Collections;
import java.util.Map;
import java.util.Objects;
Expand Down Expand Up @@ -93,30 +95,31 @@ public void testCustomLogRotation() throws IOException {
customLogsDir.mkdirs();

// Create dummy log for test1 log recorder
try (FileWriter writer = new FileWriter(new File(customLogsDir, "test1.log"))) {
writer.write("test1 one");
}
try (FileWriter writer = new FileWriter(new File(customLogsDir, "test1.log.1"))) {
writer.write("test1 two");
}
try (FileWriter writer = new FileWriter(new File(customLogsDir, "test1.log.2"))) {
writer.write("test1 three");
}

// Create dummy log for test2 log recorder
try (FileWriter writer = new FileWriter(new File(customLogsDir, "second.test2.log"))) {
writer.write("second.test2 one");
}
try (FileWriter writer = new FileWriter(new File(customLogsDir, "second.test2.log.1"))) {
writer.write("second.test2 two");
}
try (FileWriter writer = new FileWriter(new File(customLogsDir, "second.test2.log.2"))) {
writer.write("second.test2 three");
}

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)

Files.write(
Paths.get(customLogsDir.getPath(), "test1.log.1"), "test1 two".getBytes(), StandardOpenOption.CREATE);
Files.write(
Paths.get(customLogsDir.getPath(), "test1.log.2"), "test1 three".getBytes(), StandardOpenOption.CREATE);

// Create dummy log for second.test2 log recorder
Files.write(
Paths.get(customLogsDir.getPath(), "second.test2.log"),
"second.test2 one".getBytes(),
StandardOpenOption.CREATE);
Files.write(
Paths.get(customLogsDir.getPath(), "second.test2.log.1"),
"second.test2 two".getBytes(),
StandardOpenOption.CREATE);
Files.write(
Paths.get(customLogsDir.getPath(), "second.test2.log.2"),
"second.test2 three".getBytes(),
StandardOpenOption.CREATE);

// Create dummy log for nonRotatedCustomLog
Files.write(
Paths.get(customLogsDir.getPath(), "nonRotatedCustomLog.log"),
"nonRotatedCustomLog one".getBytes(),
StandardOpenOption.CREATE);

// Invoke the component and get the result map
Map<String, String> resultMap = SupportTestUtils.invokeComponentToMap(
Expand All @@ -134,5 +137,14 @@ public void testCustomLogRotation() throws IOException {

// Assert the result map
assertEquals(expectedMap, resultMap);

// Cleanup all the dummy files
Files.deleteIfExists(Paths.get(customLogsDir.getPath(), "test1.log"));
Files.deleteIfExists(Paths.get(customLogsDir.getPath(), "test1.log.1"));
Files.deleteIfExists(Paths.get(customLogsDir.getPath(), "test1.log.2"));
Files.deleteIfExists(Paths.get(customLogsDir.getPath(), "second.test2.log"));
Files.deleteIfExists(Paths.get(customLogsDir.getPath(), "second.test2.log.1"));
Files.deleteIfExists(Paths.get(customLogsDir.getPath(), "second.test2.log.2"));
Files.deleteIfExists(Paths.get(customLogsDir.getPath(), "nonRotatedCustomLog.log"));
}
}
Loading