Add support for serializing Any types to BEP's JSON transport.
Followup from issue #19471 to include details specific to execution strategy
for the select actions included in BEP.
PiperOrigin-RevId: 577279639
Change-Id: I112182134f1ce798c22f8f81998a14040344c191
diff --git a/src/main/java/com/google/devtools/build/lib/buildeventservice/BUILD b/src/main/java/com/google/devtools/build/lib/buildeventservice/BUILD
index f627aca..db79ef5 100644
--- a/src/main/java/com/google/devtools/build/lib/buildeventservice/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/buildeventservice/BUILD
@@ -37,7 +37,6 @@
),
deps = [
":buildeventservice-options",
- "//src/main/java/com/google/devtools/build/lib:build-request-options",
"//src/main/java/com/google/devtools/build/lib:runtime",
"//src/main/java/com/google/devtools/build/lib/analysis:blaze_directories",
"//src/main/java/com/google/devtools/build/lib/analysis:test/test_configuration",
@@ -62,6 +61,7 @@
"//src/main/java/com/google/devtools/build/lib/util/io:out-err",
"//src/main/java/com/google/devtools/common/options",
"//src/main/protobuf:failure_details_java_proto",
+ "//src/main/protobuf:spawn_java_proto",
"//third_party:auth",
"//third_party:auto_value",
"//third_party:flogger",
diff --git a/src/main/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceModule.java b/src/main/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceModule.java
index 6b2afdf..8e8db9c 100644
--- a/src/main/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceModule.java
+++ b/src/main/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceModule.java
@@ -50,6 +50,7 @@
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.EventHandler;
import com.google.devtools.build.lib.events.Reporter;
+import com.google.devtools.build.lib.exec.Protos.SpawnExec;
import com.google.devtools.build.lib.network.ConnectivityStatus;
import com.google.devtools.build.lib.network.ConnectivityStatus.Status;
import com.google.devtools.build.lib.network.ConnectivityStatusProvider;
@@ -72,6 +73,7 @@
import com.google.devtools.common.options.OptionsBase;
import com.google.devtools.common.options.OptionsParsingException;
import com.google.devtools.common.options.OptionsParsingResult;
+import com.google.protobuf.util.JsonFormat.TypeRegistry;
import com.google.protobuf.util.Timestamps;
import java.io.BufferedOutputStream;
import java.io.IOException;
@@ -748,6 +750,16 @@
.build();
}
+ /**
+ * Returns the JSON type registry, used to resolve {@code Any} type names at serialization time.
+ *
+ * <p>Intended to be overridden by custom build tools with a subclassed {@link
+ * BuildEventServiceModule} to add additional Any types to be produced.
+ */
+ protected TypeRegistry makeJsonTypeRegistry() {
+ return TypeRegistry.newBuilder().add(SpawnExec.getDescriptor()).build();
+ }
+
private ImmutableSet<BuildEventTransport> createBepTransports(
CommandEnvironment cmdEnv,
ThrowingBuildEventArtifactUploaderSupplier uploaderSupplier,
@@ -820,7 +832,11 @@
: new LocalFilesArtifactUploader();
bepTransportsBuilder.add(
new JsonFormatFileTransport(
- bepJsonOutputStream, bepOptions, localFileUploader, artifactGroupNamer));
+ bepJsonOutputStream,
+ bepOptions,
+ localFileUploader,
+ artifactGroupNamer,
+ makeJsonTypeRegistry()));
} catch (IOException exception) {
// TODO(b/125216340): Consider making this a warning instead of an error once the
// associated bug has been resolved.
diff --git a/src/main/java/com/google/devtools/build/lib/buildeventstream/proto/build_event_stream.proto b/src/main/java/com/google/devtools/build/lib/buildeventstream/proto/build_event_stream.proto
index f70bbbe..8520be2 100644
--- a/src/main/java/com/google/devtools/build/lib/buildeventstream/proto/build_event_stream.proto
+++ b/src/main/java/com/google/devtools/build/lib/buildeventstream/proto/build_event_stream.proto
@@ -585,7 +585,11 @@
// End of action execution, after all attempted execution completes.
google.protobuf.Timestamp end_time = 13;
- // Additional details about action execution supplied by any/all strategies.
+ // Additional details about action execution supplied by strategies. Bazel
+ // options will determine which strategy details are included when multiple
+ // strategies are involved in a single action's execution.
+ //
+ // The default type will be `tools.proto.SpawnExec` found in `spawn.proto`.
repeated google.protobuf.Any strategy_details = 14;
}
diff --git a/src/main/java/com/google/devtools/build/lib/buildeventstream/transports/BUILD b/src/main/java/com/google/devtools/build/lib/buildeventstream/transports/BUILD
index 1dacd15..b74731e 100644
--- a/src/main/java/com/google/devtools/build/lib/buildeventstream/transports/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/buildeventstream/transports/BUILD
@@ -22,6 +22,7 @@
"//src/main/java/com/google/devtools/common/options",
"//src/main/protobuf:failure_details_java_proto",
"//third_party:flogger",
+ "//third_party:gson",
"//third_party:guava",
"//third_party:jsr305",
"//third_party/protobuf:protobuf_java",
diff --git a/src/main/java/com/google/devtools/build/lib/buildeventstream/transports/JsonFormatFileTransport.java b/src/main/java/com/google/devtools/build/lib/buildeventstream/transports/JsonFormatFileTransport.java
index a250741..5d7af3a 100644
--- a/src/main/java/com/google/devtools/build/lib/buildeventstream/transports/JsonFormatFileTransport.java
+++ b/src/main/java/com/google/devtools/build/lib/buildeventstream/transports/JsonFormatFileTransport.java
@@ -16,13 +16,18 @@
import static java.nio.charset.StandardCharsets.UTF_8;
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.flogger.GoogleLogger;
import com.google.devtools.build.lib.buildeventstream.ArtifactGroupNamer;
import com.google.devtools.build.lib.buildeventstream.BuildEventArtifactUploader;
import com.google.devtools.build.lib.buildeventstream.BuildEventProtocolOptions;
import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos;
import com.google.devtools.build.lib.buildeventstream.BuildEventTransport;
+import com.google.gson.GsonBuilder;
import com.google.protobuf.InvalidProtocolBufferException;
import com.google.protobuf.util.JsonFormat;
+import com.google.protobuf.util.JsonFormat.Printer;
+import com.google.protobuf.util.JsonFormat.TypeRegistry;
import java.io.BufferedOutputStream;
/**
@@ -30,12 +35,22 @@
* representation of the events to a file.
*/
public final class JsonFormatFileTransport extends FileTransport {
+ private static final GoogleLogger logger = GoogleLogger.forEnclosingClass();
+
+ private static final String UNKNOWN_ANY_TYPE_ERROR_EVENT =
+ new GsonBuilder().create().toJson(new UnknownAnyProtoError());
+
+ private final Printer jsonPrinter;
+
public JsonFormatFileTransport(
BufferedOutputStream outputStream,
BuildEventProtocolOptions options,
BuildEventArtifactUploader uploader,
- ArtifactGroupNamer namer) {
+ ArtifactGroupNamer namer,
+ TypeRegistry typeRegistry) {
super(outputStream, options, uploader, namer);
+ jsonPrinter =
+ JsonFormat.printer().usingTypeRegistry(typeRegistry).omittingInsignificantWhitespace();
}
@Override
@@ -47,14 +62,24 @@
protected byte[] serializeEvent(BuildEventStreamProtos.BuildEvent buildEvent) {
String protoJsonRepresentation;
try {
- protoJsonRepresentation =
- JsonFormat.printer().omittingInsignificantWhitespace().print(buildEvent) + "\n";
+ protoJsonRepresentation = jsonPrinter.print(buildEvent);
} catch (InvalidProtocolBufferException e) {
// We don't expect any unknown Any fields in our protocol buffer. Nevertheless, handle
// the exception gracefully and, at least, return valid JSON with an id field.
- protoJsonRepresentation =
- "{\"id\" : \"unknown\", \"exception\" : \"InvalidProtocolBufferException\"}\n";
+ logger.atWarning().withCause(e).log(
+ "Failed to serialize to JSON due to Any type resolution failure: %s", buildEvent);
+ protoJsonRepresentation = UNKNOWN_ANY_TYPE_ERROR_EVENT;
}
- return protoJsonRepresentation.getBytes(UTF_8);
+ return (protoJsonRepresentation + "\n").getBytes(UTF_8);
+ }
+
+ /** Error produced when serializing an {@code Any} protobuf whose contained type is unknown. */
+ @VisibleForTesting
+ static class UnknownAnyProtoError {
+ @SuppressWarnings({"FieldCanBeStatic", "unused"}) // Used by Gson formatting; cannot be static
+ private final String id = "unknown";
+
+ @SuppressWarnings({"FieldCanBeStatic", "unused"}) // Used by Gson formatting; cannot be static
+ private final String exception = "InvalidProtocolBufferException";
}
}
diff --git a/src/test/java/com/google/devtools/build/lib/buildeventstream/transports/BUILD b/src/test/java/com/google/devtools/build/lib/buildeventstream/transports/BUILD
index f0ccefc..3caaa10 100644
--- a/src/test/java/com/google/devtools/build/lib/buildeventstream/transports/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/buildeventstream/transports/BUILD
@@ -24,6 +24,8 @@
"//src/main/java/com/google/devtools/build/lib/buildeventstream/transports",
"//src/main/java/com/google/devtools/build/lib/vfs",
"//src/main/java/com/google/devtools/common/options",
+ "//src/main/protobuf:spawn_java_proto",
+ "//third_party:gson",
"//third_party:guava",
"//third_party:junit4",
"//third_party:mockito",
diff --git a/src/test/java/com/google/devtools/build/lib/buildeventstream/transports/JsonFormatFileTransportTest.java b/src/test/java/com/google/devtools/build/lib/buildeventstream/transports/JsonFormatFileTransportTest.java
index 3635efb..fe9dd01 100644
--- a/src/test/java/com/google/devtools/build/lib/buildeventstream/transports/JsonFormatFileTransportTest.java
+++ b/src/test/java/com/google/devtools/build/lib/buildeventstream/transports/JsonFormatFileTransportTest.java
@@ -15,28 +15,37 @@
package com.google.devtools.build.lib.buildeventstream.transports;
import static com.google.common.truth.Truth.assertThat;
+import static java.nio.charset.StandardCharsets.UTF_8;
import static org.mockito.Mockito.when;
import com.google.devtools.build.lib.buildeventstream.ArtifactGroupNamer;
import com.google.devtools.build.lib.buildeventstream.BuildEvent;
-import com.google.devtools.build.lib.buildeventstream.BuildEventContext;
import com.google.devtools.build.lib.buildeventstream.BuildEventProtocolOptions;
import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos;
+import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos.ActionExecuted;
+import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos.BuildEventId;
+import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos.BuildEventId.ActionCompletedId;
import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos.BuildStarted;
import com.google.devtools.build.lib.buildeventstream.LocalFilesArtifactUploader;
import com.google.devtools.build.lib.buildeventstream.PathConverter;
+import com.google.devtools.build.lib.buildeventstream.transports.JsonFormatFileTransport.UnknownAnyProtoError;
+import com.google.devtools.build.lib.exec.Protos.SpawnExec;
import com.google.devtools.common.options.Options;
+import com.google.gson.GsonBuilder;
+import com.google.protobuf.Any;
import com.google.protobuf.util.JsonFormat;
+import com.google.protobuf.util.JsonFormat.TypeRegistry;
import java.io.BufferedOutputStream;
+import java.io.BufferedReader;
import java.io.File;
import java.io.FileInputStream;
+import java.io.FileNotFoundException;
import java.io.IOException;
import java.io.InputStreamReader;
import java.io.OutputStream;
import java.io.Reader;
import java.nio.file.Files;
import java.nio.file.Paths;
-import org.junit.After;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
@@ -45,53 +54,60 @@
import org.junit.runners.JUnit4;
import org.mockito.ArgumentMatchers;
import org.mockito.Mock;
-import org.mockito.Mockito;
-import org.mockito.MockitoAnnotations;
+import org.mockito.junit.MockitoJUnit;
+import org.mockito.junit.MockitoRule;
+import org.mockito.quality.Strictness;
-/** Tests {@link TextFormatFileTransport}. * */
+/** Tests {@link JsonFormatFileTransport}. */
@RunWith(JUnit4.class)
public class JsonFormatFileTransportTest {
+
+ private static final TypeRegistry SPAWN_EXEC_TYPE_REGISTRY =
+ TypeRegistry.newBuilder().add(SpawnExec.getDescriptor()).build();
private final BuildEventProtocolOptions defaultOpts =
Options.getDefaults(BuildEventProtocolOptions.class);
@Rule public TemporaryFolder tmp = new TemporaryFolder();
+ @Rule public MockitoRule mocks = MockitoJUnit.rule().strictness(Strictness.STRICT_STUBS);
@Mock public BuildEvent buildEvent;
@Mock public PathConverter pathConverter;
@Mock public ArtifactGroupNamer artifactGroupNamer;
+ private File output = null;
+ private BufferedOutputStream outputStream = null;
+ private JsonFormatFileTransport underTest = null;
+
@Before
- public void setUp() {
- MockitoAnnotations.initMocks(this);
- }
-
- @After
- public void tearDown() {
- Mockito.validateMockitoUsage();
- }
-
- @Test
- public void testCreatesFileAndWritesProtoJsonFormat() throws Exception {
- File output = tmp.newFile();
- BufferedOutputStream outputStream =
+ public void setUp() throws IOException {
+ output = tmp.newFile();
+ outputStream =
new BufferedOutputStream(Files.newOutputStream(Paths.get(output.getAbsolutePath())));
-
- BuildEventStreamProtos.BuildEvent started =
- BuildEventStreamProtos.BuildEvent.newBuilder()
- .setStarted(BuildStarted.newBuilder().setCommand("build"))
- .build();
- when(buildEvent.asStreamProto(ArgumentMatchers.<BuildEventContext>any())).thenReturn(started);
- JsonFormatFileTransport transport =
+ underTest =
new JsonFormatFileTransport(
outputStream,
defaultOpts,
new LocalFilesArtifactUploader(),
- artifactGroupNamer);
- transport.sendBuildEvent(buildEvent);
+ artifactGroupNamer,
+ SPAWN_EXEC_TYPE_REGISTRY);
+ }
- transport.close().get();
- try (Reader reader = new InputStreamReader(new FileInputStream(output))) {
+ @Test
+ public void testCreatesFileAndWritesProtoJsonFormat() throws Exception {
+ // Arrange: Prepare a simple BEP event that can be round-tripped.
+ BuildEventStreamProtos.BuildEvent started =
+ BuildEventStreamProtos.BuildEvent.newBuilder()
+ .setStarted(BuildStarted.newBuilder().setCommand("build"))
+ .build();
+ when(buildEvent.asStreamProto(ArgumentMatchers.any())).thenReturn(started);
+
+ // Act: Send the simple BuildStarted event.
+ underTest.sendBuildEvent(buildEvent);
+ underTest.close().get();
+
+ // Assert: Read back the BEP event and confirm it round-tripped.
+ try (Reader reader = openOutputReader()) {
JsonFormat.Parser parser = JsonFormat.parser();
BuildEventStreamProtos.BuildEvent.Builder builder =
BuildEventStreamProtos.BuildEvent.newBuilder();
@@ -100,11 +116,104 @@
}
}
+ @Test
+ public void expandsKnownAnyType() throws Exception {
+ // Arrange: Prepare an Any event that is recognized by the JSON formatter.
+ Any spawnExecAny = Any.pack(SpawnExec.newBuilder().setExitCode(1).setMnemonic("Javac").build());
+ BuildEventStreamProtos.BuildEvent action = makeActionEventWithAnyDetails(spawnExecAny);
+ when(buildEvent.asStreamProto(ArgumentMatchers.any())).thenReturn(action);
+
+ // Act: Write the event and close the transport to force flushing.
+ underTest.sendBuildEvent(buildEvent);
+ underTest.close().get();
+
+ // Assert: Confirm we get the SpawnExec back.
+ try (Reader reader = openOutputReader()) {
+ JsonFormat.Parser parser = JsonFormat.parser().usingTypeRegistry(SPAWN_EXEC_TYPE_REGISTRY);
+ BuildEventStreamProtos.BuildEvent.Builder builder =
+ BuildEventStreamProtos.BuildEvent.newBuilder();
+ parser.merge(reader, builder);
+ assertThat(builder.build()).isEqualTo(action);
+ }
+ }
+
+ @Test
+ public void rejectsUnknownAnyType() throws Exception {
+ // Arrange: Prepare a BuildEvent that cannot be serialized due to an unrecognized Any type.
+ Any bogusAnyProto =
+ Any.pack(
+ BuildEventId.newBuilder()
+ .setActionCompleted(ActionCompletedId.newBuilder().setLabel("//:foo"))
+ .build());
+ BuildEventStreamProtos.BuildEvent action = makeActionEventWithAnyDetails(bogusAnyProto);
+ when(buildEvent.asStreamProto(ArgumentMatchers.any())).thenReturn(action);
+
+ // Act: Send the event with a bogus Any value.
+ underTest.sendBuildEvent(buildEvent);
+ underTest.close().get();
+
+ // Assert: Special invalid event message is written in JSON.
+ try (BufferedReader reader = new BufferedReader(openOutputReader())) {
+ String jsonLine = reader.readLine();
+ UnknownAnyProtoError error =
+ new GsonBuilder().create().fromJson(jsonLine, UnknownAnyProtoError.class);
+ assertThat(error).isNotNull();
+ }
+ }
+
+ @Test
+ public void testFlushesStreamAfterSmallWrites() throws Exception {
+ // Arrange: JsonFormatFileTransport writes to a wrapped output stream to verify flushing.
+ WrappedOutputStream wrappedOutputStream = new WrappedOutputStream(outputStream);
+ underTest =
+ new JsonFormatFileTransport(
+ wrappedOutputStream,
+ defaultOpts,
+ new LocalFilesArtifactUploader(),
+ artifactGroupNamer,
+ SPAWN_EXEC_TYPE_REGISTRY);
+
+ BuildEventStreamProtos.BuildEvent started =
+ BuildEventStreamProtos.BuildEvent.newBuilder()
+ .setStarted(BuildStarted.newBuilder().setCommand("build"))
+ .build();
+ when(buildEvent.asStreamProto(ArgumentMatchers.any())).thenReturn(started);
+
+ // Act: Write an event, then wait for three flush intervals.
+ underTest.sendBuildEvent(buildEvent);
+ Thread.sleep(underTest.getFlushInterval().toMillis() * 3);
+
+ // Assert: Confirm BEP events were written even though the file transport is not closed.
+
+ // Some users, e.g. Tulsi, use JSON build event output for interactive use and expect the stream
+ // to be flushed at regular short intervals.
+ assertThat(wrappedOutputStream.flushCount).isGreaterThan(0);
+
+ // We know that large writes get flushed; test is valuable only if we check small writes,
+ // meaning smaller than 8192, the default buffer size used by BufferedOutputStream.
+ assertThat(wrappedOutputStream.byteCount).isLessThan(8192L);
+ assertThat(wrappedOutputStream.byteCount).isGreaterThan(0L);
+
+ underTest.close().get();
+ }
+
+ private InputStreamReader openOutputReader() throws FileNotFoundException {
+ return new InputStreamReader(new FileInputStream(output), UTF_8);
+ }
+
+ private static BuildEventStreamProtos.BuildEvent makeActionEventWithAnyDetails(
+ Any strategyDetails) {
+ return BuildEventStreamProtos.BuildEvent.newBuilder()
+ .setAction(
+ ActionExecuted.newBuilder().setExitCode(1).addStrategyDetails(strategyDetails).build())
+ .build();
+ }
+
/**
* A thin wrapper around an OutputStream that counts number of bytes written and verifies flushes.
*
* <p>The methods below need to be synchronized because they override methods from {@link
- * BufferedOutputStream} *not* because there's a concurrent access to the stream.
+ * BufferedOutputStream} *not* because there is concurrent access to the stream.
*/
private static final class WrappedOutputStream extends BufferedOutputStream {
private long byteCount;
@@ -139,36 +248,4 @@
flushCount++;
}
}
-
- @Test
- public void testFlushesStreamAfterSmallWrites() throws Exception {
- File output = tmp.newFile();
- BufferedOutputStream outputStream =
- new BufferedOutputStream(Files.newOutputStream(Paths.get(output.getAbsolutePath())));
- WrappedOutputStream wrappedOutputStream = new WrappedOutputStream(outputStream);
-
- BuildEventStreamProtos.BuildEvent started =
- BuildEventStreamProtos.BuildEvent.newBuilder()
- .setStarted(BuildStarted.newBuilder().setCommand("build"))
- .build();
- when(buildEvent.asStreamProto(ArgumentMatchers.<BuildEventContext>any())).thenReturn(started);
-
- JsonFormatFileTransport transport =
- new JsonFormatFileTransport(
- wrappedOutputStream, defaultOpts, new LocalFilesArtifactUploader(), artifactGroupNamer);
-
- transport.sendBuildEvent(buildEvent);
- Thread.sleep(transport.getFlushInterval().toMillis() * 3);
-
- // Some users, e.g. Tulsi, use JSON build event output for interactive use and expect the stream
- // to be flushed at regular short intervals.
- assertThat(wrappedOutputStream.flushCount).isGreaterThan(0);
-
- // We know that large writes get flushed; test is valuable only if we check small writes,
- // meaning smaller than 8192, the default buffer size used by BufferedOutputStream.
- assertThat(wrappedOutputStream.byteCount).isLessThan(8192L);
- assertThat(wrappedOutputStream.byteCount).isGreaterThan(0L);
-
- transport.close().get();
- }
}
