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

Return a Unique Exit Code for Out of Memory Incidents #1339

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 2 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 @@ -25,6 +25,7 @@ public enum ExitCodeType {
),

FAILURE_ACCURACY_NOT_MET(15, "Detect was unable to meet the required accuracy."),
FAILURE_OUT_OF_MEMORY(16, "Detect encountered an Out of Memory error. Please review memory settings and system resources."),
Copy link
Contributor

@andrian-sevastyanov andrian-sevastyanov Jan 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An idea for your consideration: we could modify the enum to have a new double priority field.

    private final int exitCode;
    private final String description;
    private final double priority;

    ExitCodeType(int exitCode, String description) {
        ExitCodeType(exitCode, description, (double) exitCode);
    }

    ExitCodeType(int exitCode, String description, double priority) {
        this.exitCode = exitCode;
        this.description = description;
        this.priority = priority;
    }

Then you could define the new exit code as FAILURE_OUT_OF_MEMORY(16, "...", 0.5), and start sorting exit codes not by their value but by their priority. This would also allow us to insert any priority we want anywhere going forward.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's an interesting insight. Surely it can be done. Let's have team's opinion on it whether we should go with this approach.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated the ExitCodeType enum as per your suggestion and it worked as expected in my end. As this is planned for 10.4.0 release, the team might have their opinions whether we should go with this approach.


FAILURE_IMAGE_NOT_AVAILABLE(20, "Image scan attempted but no return data available."),

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,12 @@ public ExitCodeType getWinningExitCode() {
for (ExitCodeRequest exitCodeRequest : exitCodeRequests) {
winningExitCodeType = ExitCodeType.getWinningExitCodeType(winningExitCodeType, exitCodeRequest.getExitCodeType());
}

if (!winningExitCodeType.isSuccess() &&
exitCodeRequests.stream().anyMatch(request -> request.getExitCodeType() == ExitCodeType.FAILURE_OUT_OF_MEMORY)) {
return ExitCodeType.FAILURE_OUT_OF_MEMORY;
}

return winningExitCodeType;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,13 @@
import com.blackduck.integration.detect.workflow.status.DetectIssue;
import com.blackduck.integration.detect.workflow.status.DetectIssueType;
import com.blackduck.integration.detect.workflow.status.StatusEventPublisher;
import com.blackduck.integration.detector.base.DetectorStatusCode;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public class DetectorIssuePublisher {
private final Logger logger = LoggerFactory.getLogger(this.getClass());

public void publishIssues(StatusEventPublisher statusEventPublisher, List<DetectorDirectoryReport> reports) {
//TODO (detectors): just verify we don't want to publish 'attempted' when successfully extracted, right now publishing all attempted in not-extracted.
String spacer = "\t";
Expand All @@ -24,5 +29,16 @@ public void publishIssues(StatusEventPublisher statusEventPublisher, List<Detect

}
}
public boolean hasOutOfMemoryIssue(List<DetectorDirectoryReport> reports) {
logger.info("Checking for Out of Memory (OOM) Error.");

return reports.stream()
.flatMap(report -> report.getNotExtractedDetectors().stream())
.flatMap(notExtracted -> notExtracted.getAttemptedDetectables().stream())
.anyMatch(attemptedDetectableReport -> {
logger.debug("Attempted Detectable Status Code: {}", attemptedDetectableReport.getStatusCode());
return attemptedDetectableReport.getStatusCode() == DetectorStatusCode.EXECUTABLE_TERMINATED_LIKELY_OUT_OF_MEMORY;
});
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,13 @@ public DetectorToolResult performDetectors(
List<DetectorDirectoryReport> reports = new DetectorReporter().generateReport(evaluation);
DetectorToolResult toolResult = publishAllResults(reports, directory, projectDetector, requiredDetectors, requiredAccuracyTypes);

boolean outOfMemoryIssueFound = detectorIssuePublisher.hasOutOfMemoryIssue(reports);

if (outOfMemoryIssueFound) {
logger.error("Detected an issue: EXECUTABLE_TERMINATED_LIKELY_OUT_OF_MEMORY.");
exitCodePublisher.publishExitCode(ExitCodeType.FAILURE_OUT_OF_MEMORY, "Executable terminated likely due to out of memory.");
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might just be me but I find this error checking a bit distracting in the overall flow of the performDetectors method. Perhaps consider refactoring these lines to a small method you can call.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refactored this error checking into a function called checkAndHandleOutOfMemoryIssue

//Completed.
logger.debug("Finished running detectors.");
detectorEventPublisher.publishDetectorsComplete(toolResult);
Expand Down