From dc7153caa0db4451460a99604d2f1d0eef02fd9b Mon Sep 17 00:00:00 2001 From: Mohamed Bilel Besrour Date: Mon, 25 Nov 2024 19:49:30 +0100 Subject: [PATCH 1/4] limit build logs size --- .../buildagent/service/BuildLogsMap.java | 34 +++++++++++++++++++ .../service/SharedQueueProcessingService.java | 4 +-- .../config/application-buildagent.yml | 9 +++-- 3 files changed, 42 insertions(+), 5 deletions(-) diff --git a/src/main/java/de/tum/cit/aet/artemis/buildagent/service/BuildLogsMap.java b/src/main/java/de/tum/cit/aet/artemis/buildagent/service/BuildLogsMap.java index 5ea22f2911ec..1171848542a3 100644 --- a/src/main/java/de/tum/cit/aet/artemis/buildagent/service/BuildLogsMap.java +++ b/src/main/java/de/tum/cit/aet/artemis/buildagent/service/BuildLogsMap.java @@ -8,6 +8,7 @@ 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; @@ -17,6 +18,12 @@ @Component public class BuildLogsMap { + @Value("${artemis.continuous-integration.build-logs.max-lines-per-job:10000}") + private int maxLogLinesPerBuildJob; + + @Value("${artemis.continuous-integration.build-logs.max-chars-per-line:1024}") + private int maxCharsPerLine; + private final ConcurrentMap> buildLogsMap = new ConcurrentHashMap<>(); public List getBuildLogs(String buildLogId) { @@ -34,4 +41,31 @@ public void appendBuildLogEntry(String buildLogId, BuildLogEntry buildLog) { public void removeBuildLogs(String buildLogId) { buildLogsMap.remove(buildLogId); } + + public List getAndTruncateBuildLogs(String buildLogId) { + List buildLogs = buildLogsMap.get(buildLogId); + + 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 BuildLogEntry(ZonedDateTime.now(), "Truncated build logs...\n")); + buildLogs = truncatedBuildLogs; + } + + // Truncate each line to maxCharsPerLine + for (int i = 0; i < buildLogs.size(); i++) { + BuildLogEntry buildLog = buildLogs.get(i); + String log = buildLog.getLog(); + if (log.length() > maxCharsPerLine) { + String truncatedLog = log.substring(0, maxCharsPerLine) + "\n"; + buildLogs.set(i, new BuildLogEntry(buildLog.getTime(), truncatedLog)); + } + } + + 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..d566a4e7b737 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 @@ -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/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 From 3e6c24a9e0a4aff5899d7cbfd80ea38e5ad1e310 Mon Sep 17 00:00:00 2001 From: Mohamed Bilel Besrour Date: Mon, 25 Nov 2024 19:52:46 +0100 Subject: [PATCH 2/4] java docs --- .../cit/aet/artemis/buildagent/service/BuildLogsMap.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/main/java/de/tum/cit/aet/artemis/buildagent/service/BuildLogsMap.java b/src/main/java/de/tum/cit/aet/artemis/buildagent/service/BuildLogsMap.java index 1171848542a3..285806b2cc8a 100644 --- a/src/main/java/de/tum/cit/aet/artemis/buildagent/service/BuildLogsMap.java +++ b/src/main/java/de/tum/cit/aet/artemis/buildagent/service/BuildLogsMap.java @@ -42,6 +42,12 @@ public void removeBuildLogs(String buildLogId) { buildLogsMap.remove(buildLogId); } + /** + * Retrieves and truncates the build logs for the specified build log ID. + * + * @param buildLogId the ID of the build log 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 buildLogId) { List buildLogs = buildLogsMap.get(buildLogId); From 77a4cd0b8d73bf1d9c2e10319f4a48573213b2a9 Mon Sep 17 00:00:00 2001 From: Stephan Krusche Date: Mon, 25 Nov 2024 20:57:57 +0100 Subject: [PATCH 3/4] truncate build log entries when adding them, convert build log entry to a record --- .../artemis/buildagent/dto/BuildLogDTO.java | 13 ++++ .../artemis/buildagent/dto/BuildResult.java | 8 ++- .../buildagent/dto/ResultQueueItem.java | 5 +- .../service/BuildAgentDockerService.java | 6 +- .../service/BuildJobContainerService.java | 4 +- .../service/BuildJobExecutionService.java | 2 +- .../service/BuildJobManagementService.java | 6 +- .../buildagent/service/BuildLogsMap.java | 63 ++++++++++--------- .../service/SharedQueueProcessingService.java | 6 +- .../service/BuildLogEntryService.java | 9 +-- .../LocalCIResultProcessingService.java | 4 +- .../icl/LocalCIResourceIntegrationTest.java | 4 +- 12 files changed, 74 insertions(+), 56 deletions(-) create mode 100644 src/main/java/de/tum/cit/aet/artemis/buildagent/dto/BuildLogDTO.java 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..8536a69ed2e4 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); + var 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<>(); + // buildJobId --> List of build logs + private final ConcurrentMap> buildLogsMap = new ConcurrentHashMap<>(); - public List getBuildLogs(String buildLogId) { - return buildLogsMap.get(buildLogId); - } - - 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. + * + * @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, BuildLogEntry buildLog) { - buildLogsMap.computeIfAbsent(buildLogId, k -> new ArrayList<>()).add(buildLog); + /** + * 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) { + var 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 removeBuildLogs(String buildLogId) { - buildLogsMap.remove(buildLogId); + public void removeBuildLogs(String buildJobId) { + buildLogsMap.remove(buildJobId); } /** - * Retrieves and truncates the build logs for the specified build log ID. + * Retrieves and truncates the build logs for the specified build job ID. Does not modify the original build logs. * - * @param buildLogId the ID of the build log to retrieve and truncate + * @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 buildLogId) { - List buildLogs = buildLogsMap.get(buildLogId); + public List getAndTruncateBuildLogs(String buildJobId) { + List buildLogs = buildLogsMap.get(buildJobId); if (buildLogs == null) { return null; @@ -57,21 +74,11 @@ public List getAndTruncateBuildLogs(String buildLogId) { // Truncate the build logs to maxLogLinesPerBuildJob if (buildLogs.size() > maxLogLinesPerBuildJob) { - List truncatedBuildLogs = new ArrayList<>(buildLogs.subList(0, maxLogLinesPerBuildJob)); - truncatedBuildLogs.add(new BuildLogEntry(ZonedDateTime.now(), "Truncated build logs...\n")); + List truncatedBuildLogs = new ArrayList<>(buildLogs.subList(0, maxLogLinesPerBuildJob)); + truncatedBuildLogs.add(new BuildLogDTO(ZonedDateTime.now(), "Truncated build logs...\n")); buildLogs = truncatedBuildLogs; } - // Truncate each line to maxCharsPerLine - for (int i = 0; i < buildLogs.size(); i++) { - BuildLogEntry buildLog = buildLogs.get(i); - String log = buildLog.getLog(); - if (log.length() > maxCharsPerLine) { - String truncatedLog = log.substring(0, maxCharsPerLine) + "\n"; - buildLogs.set(i, new BuildLogEntry(buildLog.getTime(), truncatedLog)); - } - } - 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 d566a4e7b737..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.getAndTruncateBuildLogs(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.getAndTruncateBuildLogs(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..fd51c4f7284f 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 (var 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/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"); From ef0ae62b6f9f7e5b7a94992b248baac0a7fb1099 Mon Sep 17 00:00:00 2001 From: Mohamed Bilel Besrour Date: Mon, 25 Nov 2024 22:23:05 +0100 Subject: [PATCH 4/4] feedback --- .../artemis/buildagent/service/BuildJobContainerService.java | 2 +- .../de/tum/cit/aet/artemis/buildagent/service/BuildLogsMap.java | 2 +- .../aet/artemis/programming/service/BuildLogEntryService.java | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) 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 8536a69ed2e4..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 @@ -414,7 +414,7 @@ private void executeDockerCommand(String containerId, String buildJobId, boolean @Override public void onNext(Frame item) { String text = new String(item.getPayload()); - var buildLogEntry = new BuildLogDTO(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/BuildLogsMap.java b/src/main/java/de/tum/cit/aet/artemis/buildagent/service/BuildLogsMap.java index 76e59c479da0..8f9e0a6226a1 100644 --- a/src/main/java/de/tum/cit/aet/artemis/buildagent/service/BuildLogsMap.java +++ b/src/main/java/de/tum/cit/aet/artemis/buildagent/service/BuildLogsMap.java @@ -46,7 +46,7 @@ public void appendBuildLogEntry(String buildJobId, String message) { * @param buildLog the build log entry to append to the build log */ public void appendBuildLogEntry(String buildJobId, BuildLogDTO buildLog) { - var buildLogs = buildLogsMap.computeIfAbsent(buildJobId, k -> new ArrayList<>()); + 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"); 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 fd51c4f7284f..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 @@ -324,7 +324,7 @@ public void saveBuildLogsToFile(List buildLogEntries, String buildJ Path logPath = exerciseLogsPath.resolve(buildJobId + ".log"); StringBuilder logsStringBuilder = new StringBuilder(); - for (var buildLogEntry : buildLogEntries) { + for (BuildLogDTO buildLogEntry : buildLogEntries) { logsStringBuilder.append(buildLogEntry.time()).append("\t").append(buildLogEntry.log()); }