Skip to content

Commit

Permalink
Integrated code lifecycle: Limit build logs size (#9861)
Browse files Browse the repository at this point in the history
  • Loading branch information
BBesrour authored Nov 25, 2024
1 parent 37bcf8e commit ce8be25
Show file tree
Hide file tree
Showing 13 changed files with 102 additions and 41 deletions.
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 {

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

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

/**
* 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;
}
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"));
}

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

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.
*
* @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;
}

// 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;
}
}
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) {
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: artemis@xcit.tum.de
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

0 comments on commit ce8be25

Please sign in to comment.