From d76ccc63d6e812ffc7c4dc796f51d699dd667920 Mon Sep 17 00:00:00 2001
From: Daniel Wagner-Hall <dwagnerhall@apple.com>
Date: Mon, 26 Jun 2023 01:30:52 -0700
Subject: [PATCH] Report remote execution messages as events

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
---
 .../build/lib/remote/RemoteSpawnRunner.java      | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java
index 32f60bdb6f79f6..ec7e7d7e86d6e2 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java
@@ -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;
@@ -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());
@@ -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)));
     }
   }
 
@@ -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)