Skip to content

Commit

Permalink
Report remote execution messages as events (#18780)
Browse files Browse the repository at this point in the history
Currently the message is appended to a spawn's stderr, which pollutes that output. It also means that whether a message is output is gated by the `--output_filter` flag, which is surprising - metadata messages don't feel like "output" in the same way `--output_filter` applies to.

Instead, emit the message as a top-level message to the terminal.

Closes #18757.

PiperOrigin-RevId: 543365674
Change-Id: I9874c8a0946a3156a2c17a2962880184d3aeebe0

Co-authored-by: Daniel Wagner-Hall <[email protected]>
  • Loading branch information
iancha1992 and illicitonion authored Jun 27, 2023
1 parent 362540c commit ad7957a
Showing 1 changed file with 9 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,6 @@
import com.google.devtools.build.lib.server.FailureDetails;
import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
import com.google.devtools.build.lib.util.ExitCode;
import com.google.devtools.build.lib.util.io.FileOutErr;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.longrunning.Operation;
Expand Down Expand Up @@ -274,7 +273,7 @@ public SpawnResult exec(Spawn spawn, SpawnExecutionContext context)
// It's already late at this stage, but we should at least report once.
reporter.reportExecutingIfNot();

maybePrintExecutionMessages(context, result.getMessage(), result.success());
maybePrintExecutionMessages(spawn, result.getMessage(), result.success());

profileAccounting(result.getExecutionMetadata());
spawnMetricsAccounting(spawnMetrics, result.getExecutionMetadata());
Expand Down Expand Up @@ -445,14 +444,16 @@ public boolean handlesCaching() {
return true;
}

private void maybePrintExecutionMessages(
SpawnExecutionContext context, String message, boolean success) {
FileOutErr outErr = context.getFileOutErr();
private void maybePrintExecutionMessages(Spawn spawn, String message, boolean success) {
boolean printMessage =
remoteOptions.remotePrintExecutionMessages.shouldPrintMessages(success)
&& !message.isEmpty();
if (printMessage) {
outErr.printErr(message + "\n");
report(
Event.info(
String.format(
"Remote execution message for %s %s: %s",
spawn.getMnemonic(), spawn.getTargetLabel(), message)));
}
}

Expand Down Expand Up @@ -534,7 +535,8 @@ private SpawnResult handleError(
}
}
if (e.isExecutionTimeout()) {
maybePrintExecutionMessages(context, e.getResponse().getMessage(), /* success = */ false);
maybePrintExecutionMessages(
action.getSpawn(), e.getResponse().getMessage(), /* success= */ false);
return new SpawnResult.Builder()
.setRunnerName(getName())
.setStatus(Status.TIMEOUT)
Expand Down

0 comments on commit ad7957a

Please sign in to comment.