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

Integrated code lifecycle: Limit build logs size #9861

Merged
Merged
Show file tree
Hide file tree
Changes from all 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
@@ -0,0 +1,13 @@
package de.tum.cit.aet.artemis.buildagent.dto;

import java.io.Serializable;
import java.time.ZonedDateTime;

import jakarta.validation.constraints.NotNull;

import com.fasterxml.jackson.annotation.JsonInclude;

@JsonInclude(JsonInclude.Include.NON_EMPTY)
public record BuildLogDTO(@NotNull ZonedDateTime time, @NotNull String log) implements Serializable {
BBesrour marked this conversation as resolved.
Show resolved Hide resolved

}
BBesrour marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
// in the future are migrated or cleared. Changes should be communicated in release notes as potentially breaking changes.
@JsonIgnoreProperties(ignoreUnknown = true)
@JsonInclude(JsonInclude.Include.NON_EMPTY)
// TODO: this should be a record in the future
public class BuildResult extends AbstractBuildResultNotificationDTO implements Serializable {

private final String assignmentRepoBranchName;
Expand All @@ -41,7 +42,7 @@ public class BuildResult extends AbstractBuildResultNotificationDTO implements S

private final List<LocalCIJobDTO> jobs;

private List<BuildLogEntry> buildLogEntries = new ArrayList<>();
private List<BuildLogDTO> buildLogEntries = new ArrayList<>();
BBesrour marked this conversation as resolved.
Show resolved Hide resolved

private final List<StaticCodeAnalysisReportDTO> staticCodeAnalysisReports;

Expand Down Expand Up @@ -123,15 +124,16 @@ public boolean hasLogs() {

@Override
public List<BuildLogEntry> extractBuildLogs() {
return buildLogEntries;
// convert the buildLogEntry DTOs to BuildLogEntry objects
return buildLogEntries.stream().map(log -> new BuildLogEntry(log.time(), log.log())).toList();
BBesrour marked this conversation as resolved.
Show resolved Hide resolved
}

/**
* Setter for the buildLogEntries
*
* @param buildLogEntries the buildLogEntries to be set
*/
public void setBuildLogEntries(List<BuildLogEntry> buildLogEntries) {
public void setBuildLogEntries(List<BuildLogDTO> buildLogEntries) {
this.buildLogEntries = buildLogEntries;
hasLogs = true;
}
BBesrour marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,8 @@

import com.fasterxml.jackson.annotation.JsonInclude;

import de.tum.cit.aet.artemis.programming.domain.build.BuildLogEntry;

// NOTE: this data structure is used in shared code between core and build agent nodes. Changing it requires that the shared data structures in Hazelcast (or potentially Redis)
// in the future are migrated or cleared. Changes should be communicated in release notes as potentially breaking changes.
@JsonInclude(JsonInclude.Include.NON_EMPTY)
// TODO: this data structure should not use BuildLogEntry because it's an entity class (and not a DTO)
public record ResultQueueItem(BuildResult buildResult, BuildJobQueueItem buildJobQueueItem, List<BuildLogEntry> buildLogs, Throwable exception) implements Serializable {
public record ResultQueueItem(BuildResult buildResult, BuildJobQueueItem buildJobQueueItem, List<BuildLogDTO> buildLogs, Throwable exception) implements Serializable {
}
Original file line number Diff line number Diff line change
Expand Up @@ -180,16 +180,14 @@ public static class MyPullImageResultCallback extends PullImageResultCallback {
@Override
public void onNext(PullResponseItem item) {
String msg = "~~~~~~~~~~~~~~~~~~~~ Pull image progress: " + item.getStatus() + " ~~~~~~~~~~~~~~~~~~~~";
log.info(msg);
buildLogsMap.appendBuildLogEntry(buildJobId, msg);
log.debug(msg);
super.onNext(item);
}

@Override
public void onComplete() {
String msg = "~~~~~~~~~~~~~~~~~~~~ Pull image complete ~~~~~~~~~~~~~~~~~~~~";
log.info(msg);
buildLogsMap.appendBuildLogEntry(buildJobId, msg);
log.debug(msg);
super.onComplete();
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,9 @@
import com.github.dockerjava.api.model.Frame;
import com.github.dockerjava.api.model.HostConfig;

import de.tum.cit.aet.artemis.buildagent.dto.BuildLogDTO;
import de.tum.cit.aet.artemis.core.exception.LocalCIException;
import de.tum.cit.aet.artemis.programming.domain.ProgrammingLanguage;
import de.tum.cit.aet.artemis.programming.domain.build.BuildLogEntry;
import de.tum.cit.aet.artemis.programming.service.ci.ContinuousIntegrationService.RepositoryCheckoutPath;

/**
Expand Down Expand Up @@ -414,7 +414,7 @@ private void executeDockerCommand(String containerId, String buildJobId, boolean
@Override
public void onNext(Frame item) {
String text = new String(item.getPayload());
BuildLogEntry buildLogEntry = new BuildLogEntry(ZonedDateTime.now(), text);
BuildLogDTO buildLogEntry = new BuildLogDTO(ZonedDateTime.now(), text);
if (buildJobId != null) {
buildLogsMap.appendBuildLogEntry(buildJobId, buildLogEntry);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,7 @@ private BuildResult runScriptAndParseResults(BuildJobQueueItem buildJob, String
try {
buildResult = parseTestResults(testResultsTarInputStream, buildJob.buildConfig().branch(), assignmentRepoCommitHash, testRepoCommitHash, buildCompletedDate,
buildJob.id());
buildResult.setBuildLogEntries(buildLogsMap.getBuildLogs(buildJob.id()));
buildResult.setBuildLogEntries(buildLogsMap.getAndTruncateBuildLogs(buildJob.id()));
}
catch (IOException | IllegalStateException e) {
msg = "Error while parsing test results";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,9 @@
import com.hazelcast.topic.ITopic;

import de.tum.cit.aet.artemis.buildagent.dto.BuildJobQueueItem;
import de.tum.cit.aet.artemis.buildagent.dto.BuildLogDTO;
import de.tum.cit.aet.artemis.buildagent.dto.BuildResult;
import de.tum.cit.aet.artemis.core.exception.LocalCIException;
import de.tum.cit.aet.artemis.programming.domain.build.BuildLogEntry;

/**
* This service is responsible for adding build jobs to the Integrated Code Lifecycle executor service.
Expand Down Expand Up @@ -171,7 +171,7 @@ public CompletableFuture<BuildResult> executeBuildJob(BuildJobQueueItem buildJob
finishCancelledBuildJob(buildJobItem.repositoryInfo().assignmentRepositoryUri(), buildJobItem.id(), containerName);
String msg = "Build job with id " + buildJobItem.id() + " was cancelled.";
String stackTrace = stackTraceToString(e);
buildLogsMap.appendBuildLogEntry(buildJobItem.id(), new BuildLogEntry(ZonedDateTime.now(), msg + "\n" + stackTrace));
buildLogsMap.appendBuildLogEntry(buildJobItem.id(), new BuildLogDTO(ZonedDateTime.now(), msg + "\n" + stackTrace));
throw new CompletionException(msg, e);
}
else {
Expand Down Expand Up @@ -232,7 +232,7 @@ private CompletableFuture<BuildResult> createCompletableFuture(Supplier<BuildRes
private void finishBuildJobExceptionally(String buildJobId, String containerName, Exception exception) {
String msg = "Error while executing build job " + buildJobId + ": " + exception.getMessage();
String stackTrace = stackTraceToString(exception);
buildLogsMap.appendBuildLogEntry(buildJobId, new BuildLogEntry(ZonedDateTime.now(), msg + "\n" + stackTrace));
buildLogsMap.appendBuildLogEntry(buildJobId, new BuildLogDTO(ZonedDateTime.now(), msg + "\n" + stackTrace));
log.error(msg);

log.info("Getting ID of running container {}", containerName);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,30 +8,77 @@
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;

import org.springframework.beans.factory.annotation.Value;
import org.springframework.context.annotation.Profile;
import org.springframework.stereotype.Component;

import de.tum.cit.aet.artemis.programming.domain.build.BuildLogEntry;
import de.tum.cit.aet.artemis.buildagent.dto.BuildLogDTO;

@Profile(PROFILE_BUILDAGENT)
@Component
public class BuildLogsMap {

private final ConcurrentMap<String, List<BuildLogEntry>> buildLogsMap = new ConcurrentHashMap<>();
@Value("${artemis.continuous-integration.build-logs.max-lines-per-job:10000}")
private int maxLogLinesPerBuildJob;

public List<BuildLogEntry> getBuildLogs(String buildLogId) {
return buildLogsMap.get(buildLogId);
@Value("${artemis.continuous-integration.build-logs.max-chars-per-line:1024}")
private int maxCharsPerLine;

// buildJobId --> List of build logs
private final ConcurrentMap<String, List<BuildLogDTO>> buildLogsMap = new ConcurrentHashMap<>();

/**
* Appends a new build log entry to the build logs for the specified build job ID.
*
* @param buildJobId the ID of the build job to append a log message to
* @param message the message to append to the build log
*/
public void appendBuildLogEntry(String buildJobId, String message) {
appendBuildLogEntry(buildJobId, new BuildLogDTO(ZonedDateTime.now(), message + "\n"));
BBesrour marked this conversation as resolved.
Show resolved Hide resolved
}

public void appendBuildLogEntry(String buildLogId, String message) {
appendBuildLogEntry(buildLogId, new BuildLogEntry(ZonedDateTime.now(), message + "\n"));
/**
* Appends a new build log entry to the build logs for the specified build job ID.
* Only the first maxCharsPerLine characters of the log message will be appended. Longer characters will be truncated to avoid memory issues.
* Only the first maxLogLinesPerBuildJob log entries will be stored. Newer logs will be ignored to avoid memory issues
*
* @param buildJobId the ID of the build job to append a log message to
* @param buildLog the build log entry to append to the build log
*/
public void appendBuildLogEntry(String buildJobId, BuildLogDTO buildLog) {
List<BuildLogDTO> buildLogs = buildLogsMap.computeIfAbsent(buildJobId, k -> new ArrayList<>());
if (buildLogs.size() < maxLogLinesPerBuildJob) {
if (buildLog.log() != null && buildLog.log().length() > maxCharsPerLine) {
buildLog = new BuildLogDTO(buildLog.time(), buildLog.log().substring(0, maxCharsPerLine) + "\n");
}
buildLogs.add(buildLog);
}
}

public void appendBuildLogEntry(String buildLogId, BuildLogEntry buildLog) {
buildLogsMap.computeIfAbsent(buildLogId, k -> new ArrayList<>()).add(buildLog);
public void removeBuildLogs(String buildJobId) {
buildLogsMap.remove(buildJobId);
BBesrour marked this conversation as resolved.
Show resolved Hide resolved
}

public void removeBuildLogs(String buildLogId) {
buildLogsMap.remove(buildLogId);
/**
* Retrieves and truncates the build logs for the specified build job ID. Does not modify the original build logs.
*
BBesrour marked this conversation as resolved.
Show resolved Hide resolved
* @param buildJobId the ID of the build job to retrieve and truncate
* @return a list of truncated build log entries, or null if no logs are found for the specified ID
*/
public List<BuildLogDTO> getAndTruncateBuildLogs(String buildJobId) {
List<BuildLogDTO> buildLogs = buildLogsMap.get(buildJobId);

if (buildLogs == null) {
return null;
}
BBesrour marked this conversation as resolved.
Show resolved Hide resolved

// Truncate the build logs to maxLogLinesPerBuildJob
if (buildLogs.size() > maxLogLinesPerBuildJob) {
List<BuildLogDTO> truncatedBuildLogs = new ArrayList<>(buildLogs.subList(0, maxLogLinesPerBuildJob));
truncatedBuildLogs.add(new BuildLogDTO(ZonedDateTime.now(), "Truncated build logs...\n"));
buildLogs = truncatedBuildLogs;
}

return buildLogs;
BBesrour marked this conversation as resolved.
Show resolved Hide resolved
BBesrour marked this conversation as resolved.
Show resolved Hide resolved
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,11 @@
import de.tum.cit.aet.artemis.buildagent.dto.BuildAgentDTO;
import de.tum.cit.aet.artemis.buildagent.dto.BuildAgentInformation;
import de.tum.cit.aet.artemis.buildagent.dto.BuildJobQueueItem;
import de.tum.cit.aet.artemis.buildagent.dto.BuildLogDTO;
import de.tum.cit.aet.artemis.buildagent.dto.BuildResult;
import de.tum.cit.aet.artemis.buildagent.dto.JobTimingInfo;
import de.tum.cit.aet.artemis.buildagent.dto.ResultQueueItem;
import de.tum.cit.aet.artemis.core.security.SecurityUtils;
import de.tum.cit.aet.artemis.programming.domain.build.BuildLogEntry;
import de.tum.cit.aet.artemis.programming.domain.build.BuildStatus;

/**
Expand Down Expand Up @@ -397,7 +397,7 @@ private void processBuild(BuildJobQueueItem buildJob) {
buildJob.exerciseId(), buildJob.retryCount(), buildJob.priority(), BuildStatus.SUCCESSFUL, buildJob.repositoryInfo(), jobTimingInfo, buildJob.buildConfig(),
null);

List<BuildLogEntry> buildLogs = buildLogsMap.getBuildLogs(buildJob.id());
List<BuildLogDTO> buildLogs = buildLogsMap.getAndTruncateBuildLogs(buildJob.id());
buildLogsMap.removeBuildLogs(buildJob.id());

ResultQueueItem resultQueueItem = new ResultQueueItem(buildResult, finishedJob, buildLogs, null);
Expand Down Expand Up @@ -435,7 +435,7 @@ private void processBuild(BuildJobQueueItem buildJob) {

job = new BuildJobQueueItem(buildJob, completionDate, status);

List<BuildLogEntry> buildLogs = buildLogsMap.getBuildLogs(buildJob.id());
List<BuildLogDTO> buildLogs = buildLogsMap.getAndTruncateBuildLogs(buildJob.id());
buildLogsMap.removeBuildLogs(buildJob.id());

BuildResult failedResult = new BuildResult(buildJob.buildConfig().branch(), buildJob.buildConfig().assignmentCommitHash(), buildJob.buildConfig().testCommitHash(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import org.springframework.scheduling.annotation.Scheduled;
import org.springframework.stereotype.Service;

import de.tum.cit.aet.artemis.buildagent.dto.BuildLogDTO;
import de.tum.cit.aet.artemis.core.service.ProfileService;
import de.tum.cit.aet.artemis.programming.domain.ProgrammingExercise;
import de.tum.cit.aet.artemis.programming.domain.ProgrammingLanguage;
Expand Down Expand Up @@ -300,14 +301,14 @@ public void deleteBuildLogEntriesForProgrammingSubmission(ProgrammingSubmission
* and the build job ID. If the directory structure for the logs does not already exist, it is created.
* Each log entry is written to the log file in the format of "time\tlog message".
*
* @param buildLogEntries A list of {@link BuildLogEntry} objects containing the build log information to be saved.
* @param buildLogEntries A list of {@link BuildLogDTO} objects containing the build log information to be saved.
* @param buildJobId The unique identifier of the build job whose logs are being saved.
* @param programmingExercise The programming exercise associated with the build job, used to
* retrieve the course and exercise short names.
* @throws IllegalStateException If the directory for storing the logs could not be created.
* @throws RuntimeException If an I/O error occurs while writing the log file.
*/
public void saveBuildLogsToFile(List<BuildLogEntry> buildLogEntries, String buildJobId, ProgrammingExercise programmingExercise) {
public void saveBuildLogsToFile(List<BuildLogDTO> buildLogEntries, String buildJobId, ProgrammingExercise programmingExercise) {
BBesrour marked this conversation as resolved.
Show resolved Hide resolved
String courseShortName = programmingExercise.getCourseViaExerciseGroupOrCourseMember().getShortName();
String exerciseShortName = programmingExercise.getShortName();
Path exerciseLogsPath = buildLogsPath.resolve(courseShortName).resolve(exerciseShortName);
Expand All @@ -323,8 +324,8 @@ public void saveBuildLogsToFile(List<BuildLogEntry> buildLogEntries, String buil
Path logPath = exerciseLogsPath.resolve(buildJobId + ".log");

StringBuilder logsStringBuilder = new StringBuilder();
for (BuildLogEntry buildLogEntry : buildLogEntries) {
logsStringBuilder.append(buildLogEntry.getTime()).append("\t").append(buildLogEntry.getLog());
for (BuildLogDTO buildLogEntry : buildLogEntries) {
logsStringBuilder.append(buildLogEntry.time()).append("\t").append(buildLogEntry.log());
}

try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import de.tum.cit.aet.artemis.assessment.domain.Result;
import de.tum.cit.aet.artemis.buildagent.dto.BuildAgentInformation;
import de.tum.cit.aet.artemis.buildagent.dto.BuildJobQueueItem;
import de.tum.cit.aet.artemis.buildagent.dto.BuildLogDTO;
import de.tum.cit.aet.artemis.buildagent.dto.BuildResult;
import de.tum.cit.aet.artemis.buildagent.dto.ResultQueueItem;
import de.tum.cit.aet.artemis.core.exception.EntityNotFoundException;
Expand All @@ -37,7 +38,6 @@
import de.tum.cit.aet.artemis.programming.domain.ProgrammingExerciseParticipation;
import de.tum.cit.aet.artemis.programming.domain.RepositoryType;
import de.tum.cit.aet.artemis.programming.domain.build.BuildJob;
import de.tum.cit.aet.artemis.programming.domain.build.BuildLogEntry;
import de.tum.cit.aet.artemis.programming.domain.build.BuildStatus;
import de.tum.cit.aet.artemis.programming.dto.ResultDTO;
import de.tum.cit.aet.artemis.programming.exception.BuildTriggerWebsocketError;
Expand Down Expand Up @@ -133,7 +133,7 @@ public void processResult() {

BuildJobQueueItem buildJob = resultQueueItem.buildJobQueueItem();
BuildResult buildResult = resultQueueItem.buildResult();
List<BuildLogEntry> buildLogs = resultQueueItem.buildLogs();
List<BuildLogDTO> buildLogs = resultQueueItem.buildLogs();
Throwable ex = resultQueueItem.exception();

BuildJob savedBuildJob;
Expand Down
9 changes: 6 additions & 3 deletions src/main/resources/config/application-buildagent.yml
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,12 @@ artemis:
cleanup-schedule-minutes: 60
pause-grace-period-seconds: 60
build-timeout-seconds:
min: 10
default: 120
max: 240
min: 10
default: 120
max: 240
build-logs:
max-lines-per-job: 10000
max-chars-per-line: 1024
git:
name: Artemis
email: [email protected]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import de.tum.cit.aet.artemis.buildagent.dto.BuildConfig;
import de.tum.cit.aet.artemis.buildagent.dto.BuildJobQueueItem;
import de.tum.cit.aet.artemis.buildagent.dto.BuildJobsStatisticsDTO;
import de.tum.cit.aet.artemis.buildagent.dto.BuildLogDTO;
import de.tum.cit.aet.artemis.buildagent.dto.FinishedBuildJobDTO;
import de.tum.cit.aet.artemis.buildagent.dto.JobTimingInfo;
import de.tum.cit.aet.artemis.buildagent.dto.RepositoryInfo;
Expand All @@ -37,7 +38,6 @@
import de.tum.cit.aet.artemis.programming.AbstractProgrammingIntegrationLocalCILocalVCTestBase;
import de.tum.cit.aet.artemis.programming.domain.RepositoryType;
import de.tum.cit.aet.artemis.programming.domain.build.BuildJob;
import de.tum.cit.aet.artemis.programming.domain.build.BuildLogEntry;
import de.tum.cit.aet.artemis.programming.domain.build.BuildStatus;

class LocalCIResourceIntegrationTest extends AbstractProgrammingIntegrationLocalCILocalVCTestBase {
Expand Down Expand Up @@ -332,7 +332,7 @@ void testGetBuildAgents_instructorAccessForbidden() throws Exception {
void testGetBuildLogsForResult() throws Exception {
try {
buildJobRepository.save(finishedJobForLogs);
BuildLogEntry buildLogEntry = new BuildLogEntry(ZonedDateTime.now(), "Dummy log");
BuildLogDTO buildLogEntry = new BuildLogDTO(ZonedDateTime.now(), "Dummy log");
buildLogEntryService.saveBuildLogsToFile(List.of(buildLogEntry), "6", programmingExercise);
var response = request.get("/api/build-log/6", HttpStatus.OK, String.class);
assertThat(response).contains("Dummy log");
Expand Down
Loading