diff --git a/src/main/java/de/tum/cit/aet/artemis/buildagent/dto/BuildLogDTO.java b/src/main/java/de/tum/cit/aet/artemis/buildagent/dto/BuildLogDTO.java new file mode 100644 index 000000000000..62e9e2229a88 --- /dev/null +++ b/src/main/java/de/tum/cit/aet/artemis/buildagent/dto/BuildLogDTO.java @@ -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 { + +} diff --git a/src/main/java/de/tum/cit/aet/artemis/buildagent/dto/BuildResult.java b/src/main/java/de/tum/cit/aet/artemis/buildagent/dto/BuildResult.java index fbb9bbbdbee1..f8fec009f4b9 100644 --- a/src/main/java/de/tum/cit/aet/artemis/buildagent/dto/BuildResult.java +++ b/src/main/java/de/tum/cit/aet/artemis/buildagent/dto/BuildResult.java @@ -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; @@ -41,7 +42,7 @@ public class BuildResult extends AbstractBuildResultNotificationDTO implements S private final List jobs; - private List buildLogEntries = new ArrayList<>(); + private List buildLogEntries = new ArrayList<>(); private final List staticCodeAnalysisReports; @@ -123,7 +124,8 @@ public boolean hasLogs() { @Override public List extractBuildLogs() { - return buildLogEntries; + // convert the buildLogEntry DTOs to BuildLogEntry objects + return buildLogEntries.stream().map(log -> new BuildLogEntry(log.time(), log.log())).toList(); } /** @@ -131,7 +133,7 @@ public List extractBuildLogs() { * * @param buildLogEntries the buildLogEntries to be set */ - public void setBuildLogEntries(List buildLogEntries) { + public void setBuildLogEntries(List buildLogEntries) { this.buildLogEntries = buildLogEntries; hasLogs = true; } diff --git a/src/main/java/de/tum/cit/aet/artemis/buildagent/dto/ResultQueueItem.java b/src/main/java/de/tum/cit/aet/artemis/buildagent/dto/ResultQueueItem.java index 9ed86f7306f7..6964d2cd58a8 100644 --- a/src/main/java/de/tum/cit/aet/artemis/buildagent/dto/ResultQueueItem.java +++ b/src/main/java/de/tum/cit/aet/artemis/buildagent/dto/ResultQueueItem.java @@ -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 buildLogs, Throwable exception) implements Serializable { +public record ResultQueueItem(BuildResult buildResult, BuildJobQueueItem buildJobQueueItem, List buildLogs, Throwable exception) implements Serializable { } diff --git a/src/main/java/de/tum/cit/aet/artemis/buildagent/service/BuildAgentDockerService.java b/src/main/java/de/tum/cit/aet/artemis/buildagent/service/BuildAgentDockerService.java index 378bd12ed247..c7aca819eb91 100644 --- a/src/main/java/de/tum/cit/aet/artemis/buildagent/service/BuildAgentDockerService.java +++ b/src/main/java/de/tum/cit/aet/artemis/buildagent/service/BuildAgentDockerService.java @@ -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(); } } diff --git a/src/main/java/de/tum/cit/aet/artemis/buildagent/service/BuildJobContainerService.java b/src/main/java/de/tum/cit/aet/artemis/buildagent/service/BuildJobContainerService.java index 5d4c72d5b491..b6f4c63019da 100644 --- a/src/main/java/de/tum/cit/aet/artemis/buildagent/service/BuildJobContainerService.java +++ b/src/main/java/de/tum/cit/aet/artemis/buildagent/service/BuildJobContainerService.java @@ -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; /** @@ -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); } diff --git a/src/main/java/de/tum/cit/aet/artemis/buildagent/service/BuildJobExecutionService.java b/src/main/java/de/tum/cit/aet/artemis/buildagent/service/BuildJobExecutionService.java index 75bbaf826b00..fd06f24e09f7 100644 --- a/src/main/java/de/tum/cit/aet/artemis/buildagent/service/BuildJobExecutionService.java +++ b/src/main/java/de/tum/cit/aet/artemis/buildagent/service/BuildJobExecutionService.java @@ -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"; diff --git a/src/main/java/de/tum/cit/aet/artemis/buildagent/service/BuildJobManagementService.java b/src/main/java/de/tum/cit/aet/artemis/buildagent/service/BuildJobManagementService.java index 551b20f8bdd9..be480892b8e3 100644 --- a/src/main/java/de/tum/cit/aet/artemis/buildagent/service/BuildJobManagementService.java +++ b/src/main/java/de/tum/cit/aet/artemis/buildagent/service/BuildJobManagementService.java @@ -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. @@ -171,7 +171,7 @@ public CompletableFuture 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 { @@ -232,7 +232,7 @@ private CompletableFuture createCompletableFuture(Supplier> buildLogsMap = new ConcurrentHashMap<>(); + @Value("${artemis.continuous-integration.build-logs.max-lines-per-job:10000}") + private int maxLogLinesPerBuildJob; - public List 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> 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 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 getAndTruncateBuildLogs(String buildJobId) { + List buildLogs = buildLogsMap.get(buildJobId); + + if (buildLogs == null) { + return null; + } + + // Truncate the build logs to maxLogLinesPerBuildJob + if (buildLogs.size() > maxLogLinesPerBuildJob) { + List truncatedBuildLogs = new ArrayList<>(buildLogs.subList(0, maxLogLinesPerBuildJob)); + truncatedBuildLogs.add(new BuildLogDTO(ZonedDateTime.now(), "Truncated build logs...\n")); + buildLogs = truncatedBuildLogs; + } + + return buildLogs; } } diff --git a/src/main/java/de/tum/cit/aet/artemis/buildagent/service/SharedQueueProcessingService.java b/src/main/java/de/tum/cit/aet/artemis/buildagent/service/SharedQueueProcessingService.java index 0823ec5a4f9b..2c2e8b8e16bb 100644 --- a/src/main/java/de/tum/cit/aet/artemis/buildagent/service/SharedQueueProcessingService.java +++ b/src/main/java/de/tum/cit/aet/artemis/buildagent/service/SharedQueueProcessingService.java @@ -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; /** @@ -397,7 +397,7 @@ private void processBuild(BuildJobQueueItem buildJob) { buildJob.exerciseId(), buildJob.retryCount(), buildJob.priority(), BuildStatus.SUCCESSFUL, buildJob.repositoryInfo(), jobTimingInfo, buildJob.buildConfig(), null); - List buildLogs = buildLogsMap.getBuildLogs(buildJob.id()); + List buildLogs = buildLogsMap.getAndTruncateBuildLogs(buildJob.id()); buildLogsMap.removeBuildLogs(buildJob.id()); ResultQueueItem resultQueueItem = new ResultQueueItem(buildResult, finishedJob, buildLogs, null); @@ -435,7 +435,7 @@ private void processBuild(BuildJobQueueItem buildJob) { job = new BuildJobQueueItem(buildJob, completionDate, status); - List buildLogs = buildLogsMap.getBuildLogs(buildJob.id()); + List buildLogs = buildLogsMap.getAndTruncateBuildLogs(buildJob.id()); buildLogsMap.removeBuildLogs(buildJob.id()); BuildResult failedResult = new BuildResult(buildJob.buildConfig().branch(), buildJob.buildConfig().assignmentCommitHash(), buildJob.buildConfig().testCommitHash(), diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/service/BuildLogEntryService.java b/src/main/java/de/tum/cit/aet/artemis/programming/service/BuildLogEntryService.java index f6143a43561c..039fce2c76bc 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/service/BuildLogEntryService.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/service/BuildLogEntryService.java @@ -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; @@ -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 buildLogEntries, String buildJobId, ProgrammingExercise programmingExercise) { + public void saveBuildLogsToFile(List buildLogEntries, String buildJobId, ProgrammingExercise programmingExercise) { String courseShortName = programmingExercise.getCourseViaExerciseGroupOrCourseMember().getShortName(); String exerciseShortName = programmingExercise.getShortName(); Path exerciseLogsPath = buildLogsPath.resolve(courseShortName).resolve(exerciseShortName); @@ -323,8 +324,8 @@ public void saveBuildLogsToFile(List 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 { diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/service/localci/LocalCIResultProcessingService.java b/src/main/java/de/tum/cit/aet/artemis/programming/service/localci/LocalCIResultProcessingService.java index 71edf64a3fa8..417ee5bdc630 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/service/localci/LocalCIResultProcessingService.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/service/localci/LocalCIResultProcessingService.java @@ -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; @@ -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; @@ -133,7 +133,7 @@ public void processResult() { BuildJobQueueItem buildJob = resultQueueItem.buildJobQueueItem(); BuildResult buildResult = resultQueueItem.buildResult(); - List buildLogs = resultQueueItem.buildLogs(); + List buildLogs = resultQueueItem.buildLogs(); Throwable ex = resultQueueItem.exception(); BuildJob savedBuildJob; diff --git a/src/main/resources/config/application-buildagent.yml b/src/main/resources/config/application-buildagent.yml index 2872d91575cc..e4c1d3013357 100644 --- a/src/main/resources/config/application-buildagent.yml +++ b/src/main/resources/config/application-buildagent.yml @@ -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 diff --git a/src/test/java/de/tum/cit/aet/artemis/programming/icl/LocalCIResourceIntegrationTest.java b/src/test/java/de/tum/cit/aet/artemis/programming/icl/LocalCIResourceIntegrationTest.java index a4d157d1a690..28a919289e61 100644 --- a/src/test/java/de/tum/cit/aet/artemis/programming/icl/LocalCIResourceIntegrationTest.java +++ b/src/test/java/de/tum/cit/aet/artemis/programming/icl/LocalCIResourceIntegrationTest.java @@ -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; @@ -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 { @@ -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");