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.

Sign up for GitHub

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

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

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
6b6b4b8
exit code due to out of memory update
zahidblackduck Jan 16, 2025
8fa5fb6
winning exit code method update
zahidblackduck Jan 16, 2025
db09734
add priority field in exit code, update winning exit code method
zahidblackduck Jan 23, 2025
7fe206b
unused imports removed from files, cleanup
zahidblackduck Jan 23, 2025
435951a
refactor out of memory check in separate function
zahidblackduck Jan 30, 2025
d7cfc8a
update winning exit code logic. test exit code and priority uniqueness
zahidblackduck Jan 31, 2025
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
17 changes: 14 additions & 3 deletions src/main/java/com/blackduck/integration/detect/configuration/enumeration/ExitCodeType.java
View file
Open in desktop
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.", 0.5),

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

Expand All @@ -35,25 +36,35 @@ public enum ExitCodeType {

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

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

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

/**
* A failure always beats a success and a failure with a lower exit code beats a failure with a higher exit code.
* A failure always beats a success. Among failures:
* - The one with a lower priority wins.
* - If priorities are equal, the one with a lower exit code wins.
*/
public static ExitCodeType getWinningExitCodeType(ExitCodeType first, ExitCodeType second) {
if (first.isSuccess()) {
return second;
} else if (second.isSuccess()) {
return first;
} else {
if (first.getExitCode() < second.getExitCode()) {
if (first.priority < second.priority) {
return first;
} else {
} else if (second.priority < first.priority) {
return second;
} else {
return (first.getExitCode() < second.getExitCode()) ? first : second;
Copy link
Contributor

@andrian-sevastyanov andrian-sevastyanov Jan 30, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

For your consideration: we could ensure that all exit codes have unique priorities by adding a test that iterates through all enum values and checks that. Then, this last check would not be necessary as comparing priorities would be enough to ensure deterministic results. Additionally, this would ensure that someone does not accidentally add an exit code without considering what priorities are meant for. We could also perform a similar check to ensure that exit code values are also unique.

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 a great suggestion! I've implemented the proposed change and added tests to validate it.

One concern I have is how we determine the priority value for an exit code. Currently, we are assigning an arbitrary lower double value to ensure that a specific exit code takes precedence. For example:

FAILURE_OUT_OF_MEMORY(16, "Detect encountered an Out of Memory error. Please review memory settings and system resources.", 0.5)

I believe we should establish clear rules for setting priority values when deviating from the default (i.e., using the exit code itself as the priority). Assigning priorities arbitrarily doesn't seem ideal to me. What are your thoughts on defining a more structured approach?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, we should try to get to that point eventually and it's on a TODO list. If you have some concrete ideas already, feel free to write them down and share with the team!

}
}
}
Expand Down
11 changes: 11 additions & 0 deletions src/main/java/com/blackduck/integration/detect/tool/detector/DetectorIssuePublisher.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@
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 {
public void publishIssues(StatusEventPublisher statusEventPublisher, List<DetectorDirectoryReport> reports) {
Expand All @@ -25,4 +28,12 @@ public void publishIssues(StatusEventPublisher statusEventPublisher, List<Detect
}
}

public boolean hasOutOfMemoryIssue(List<DetectorDirectoryReport> reports) {
return reports.stream()
.flatMap(report -> report.getNotExtractedDetectors().stream())
.flatMap(notExtracted -> notExtracted.getAttemptedDetectables().stream())
.anyMatch(attemptedDetectableReport -> {
return attemptedDetectableReport.getStatusCode() == DetectorStatusCode.EXECUTABLE_TERMINATED_LIKELY_OUT_OF_MEMORY;
});
}
}
7 changes: 7 additions & 0 deletions src/main/java/com/blackduck/integration/detect/tool/detector/DetectorTool.java
View file
Open in desktop
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