Use the Builder pattern for BuildEventService{Transport,ProtoUtil} and BuildEventStreamer.
By using builders we can simplify the creation of those objects in
the BuildEventServiceModule, but most importantly we can better handle the
possible exceptions thrown at Object creation instead of catching generic
Exceptions (as we currently do in tryCreateBesStreamer) which
may hide unchecked exceptions from being surfaced at the right place.
PiperOrigin-RevId: 234964615
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 5d347c7..85ad819 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
@@ -22,9 +22,7 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.authandtls.AuthAndTLSOptions;
-import com.google.devtools.build.lib.buildeventservice.BuildEventServiceTransport.ExitFunction;
import com.google.devtools.build.lib.buildeventservice.client.BuildEventServiceClient;
-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.Aborted.AbortReason;
@@ -99,46 +97,76 @@
}
@Override
- public void beforeCommand(CommandEnvironment commandEnvironment) {
- OptionsParsingResult parsingResult = commandEnvironment.getOptions();
- besOptions = Preconditions.checkNotNull(parsingResult.getOptions(optionsClass()));
- bepOptions =
+ public void beforeCommand(CommandEnvironment cmdEnv) {
+ // Reset to null in case afterCommand was not called.
+ // TODO(lpino): Remove this statement once {@link BlazeModule#afterCommmand()} is guaranteed
+ // to be executed for every invocation.
+ this.outErr = null;
+
+ OptionsParsingResult parsingResult = cmdEnv.getOptions();
+ this.besOptions = Preconditions.checkNotNull(parsingResult.getOptions(optionsClass()));
+ this.bepOptions =
Preconditions.checkNotNull(parsingResult.getOptions(BuildEventProtocolOptions.class));
- authTlsOptions = Preconditions.checkNotNull(parsingResult.getOptions(AuthAndTLSOptions.class));
- besStreamOptions =
+ this.authTlsOptions =
+ Preconditions.checkNotNull(parsingResult.getOptions(AuthAndTLSOptions.class));
+ this.besStreamOptions =
Preconditions.checkNotNull(parsingResult.getOptions(BuildEventStreamOptions.class));
- // Reset to null in case afterCommand was not called.
- this.outErr = null;
- if (!whitelistedCommands(besOptions).contains(commandEnvironment.getCommandName())) {
+ CountingArtifactGroupNamer artifactGroupNamer = new CountingArtifactGroupNamer();
+ Supplier<BuildEventArtifactUploader> uploaderSupplier =
+ Suppliers.memoize(
+ () ->
+ cmdEnv
+ .getRuntime()
+ .getBuildEventArtifactUploaderFactoryMap()
+ .select(bepOptions.buildEventUploadStrategy)
+ .create(cmdEnv));
+
+ if (!whitelistedCommands(besOptions).contains(cmdEnv.getCommandName())) {
+ // Exit early if the running command isn't supported.
return;
}
- streamer = tryCreateStreamer(commandEnvironment);
- if (streamer != null) {
- commandEnvironment.getReporter().addHandler(streamer);
- commandEnvironment.getEventBus().register(streamer);
- int bufferSize = besOptions.besOuterrBufferSize;
- int chunkSize = besOptions.besOuterrChunkSize;
-
- final SynchronizedOutputStream out = new SynchronizedOutputStream(bufferSize, chunkSize);
- final SynchronizedOutputStream err = new SynchronizedOutputStream(bufferSize, chunkSize);
- this.outErr = OutErr.create(out, err);
- streamer.registerOutErrProvider(
- new BuildEventStreamer.OutErrProvider() {
- @Override
- public Iterable<String> getOut() {
- return out.readAndReset();
- }
-
- @Override
- public Iterable<String> getErr() {
- return err.readAndReset();
- }
- });
- err.registerStreamer(streamer);
- out.registerStreamer(streamer);
+ bepTransports = createBepTransports(cmdEnv, uploaderSupplier, artifactGroupNamer);
+ if (bepTransports.isEmpty()) {
+ // Exit early if there are no transports to stream to.
+ return;
}
+
+ streamer =
+ new BuildEventStreamer.Builder()
+ .buildEventTransports(bepTransports)
+ .cmdLineReporter(cmdEnv.getReporter())
+ .besStreamOptions(besStreamOptions)
+ .artifactGroupNamer(artifactGroupNamer)
+ .build();
+
+ cmdEnv.getReporter().addHandler(streamer);
+ cmdEnv.getEventBus().register(streamer);
+ registerOutAndErrOutputStreams();
+ }
+
+ private void registerOutAndErrOutputStreams() {
+ int bufferSize = besOptions.besOuterrBufferSize;
+ int chunkSize = besOptions.besOuterrChunkSize;
+ final SynchronizedOutputStream out = new SynchronizedOutputStream(bufferSize, chunkSize);
+ final SynchronizedOutputStream err = new SynchronizedOutputStream(bufferSize, chunkSize);
+
+ this.outErr = OutErr.create(out, err);
+ streamer.registerOutErrProvider(
+ new BuildEventStreamer.OutErrProvider() {
+ @Override
+ public Iterable<String> getOut() {
+ return out.readAndReset();
+ }
+
+ @Override
+ public Iterable<String> getErr() {
+ return err.readAndReset();
+ }
+ });
+ err.registerStreamer(streamer);
+ out.registerStreamer(streamer);
}
@Override
@@ -175,121 +203,106 @@
this.bepTransports = null;
}
- /** Returns {@code null} if no stream could be created. */
+ // TODO(b/115961387): Remove the @Nullable and print one line for the IDs and another optional
+ // line for the results URL instead. Currently it's not straightforward since
+ // {@link ExitFunction} (accidentally) depends on the nullability of besResultsUrl.
@Nullable
- @VisibleForTesting
- BuildEventStreamer tryCreateStreamer(CommandEnvironment env) {
- Supplier<BuildEventArtifactUploader> uploaderSupplier =
- Suppliers.memoize(
- () ->
- env.getRuntime()
- .getBuildEventArtifactUploaderFactoryMap()
- .select(bepOptions.buildEventUploadStrategy)
- .create(env));
-
- CountingArtifactGroupNamer namer = new CountingArtifactGroupNamer();
- try {
- BuildEventTransport besTransport;
- try {
- besTransport = tryCreateBesTransport(env, uploaderSupplier, namer);
- } catch (IOException | OptionsParsingException e) {
- reportError(
- env.getReporter(),
- env.getBlazeModuleEnvironment(),
- "Failed to create BuildEventTransport: " + e,
- e,
- (e instanceof OptionsParsingException)
- ? ExitCode.COMMAND_LINE_ERROR
- : ExitCode.TRANSIENT_BUILD_EVENT_SERVICE_UPLOAD_ERROR);
- clearBesClient();
- return null;
- }
-
- ImmutableSet<BuildEventTransport> bepFileTransports =
- BuildEventTransportFactory.createFromOptions(
- env, env.getBlazeModuleEnvironment()::exit, bepOptions, uploaderSupplier, namer);
-
- ImmutableSet.Builder<BuildEventTransport> transportsBuilder =
- ImmutableSet.<BuildEventTransport>builder().addAll(bepFileTransports);
- if (besTransport != null) {
- transportsBuilder.add(besTransport);
- }
-
- bepTransports = transportsBuilder.build();
- if (!bepTransports.isEmpty()) {
- return new BuildEventStreamer(bepTransports, env.getReporter(), besStreamOptions, namer);
- }
- } catch (Exception e) {
- // TODO(lpino): This generic catch with Exception shouldn't exist, remove it once the code
- // above is re-structured.
- reportError(
- env.getReporter(),
- env.getBlazeModuleEnvironment(),
- e.getMessage(),
- e,
- ExitCode.LOCAL_ENVIRONMENTAL_ERROR);
+ private String constructAndReportBesResultsMessage(
+ String invocationId, String buildRequestId, EventHandler reporter) {
+ final String besResultsUrl;
+ if (!Strings.isNullOrEmpty(besOptions.besResultsUrl)) {
+ besResultsUrl =
+ besOptions.besResultsUrl.endsWith("/")
+ ? besOptions.besResultsUrl + invocationId
+ : besOptions.besResultsUrl + "/" + invocationId;
+ reporter.handle(Event.info("Streaming Build Event Protocol to " + besResultsUrl));
+ } else {
+ besResultsUrl = null;
+ reporter.handle(
+ Event.info(
+ String.format(
+ "Streaming Build Event Protocol to %s build_request_id: %s "
+ + "invocation_id: %s",
+ besOptions.besBackend, buildRequestId, invocationId)));
}
- return null;
+ return besResultsUrl;
}
@Nullable
- private BuildEventTransport tryCreateBesTransport(
- CommandEnvironment env,
+ private BuildEventServiceTransport createBesTransport(
+ CommandEnvironment cmdEnv,
Supplier<BuildEventArtifactUploader> uploaderSupplier,
- ArtifactGroupNamer namer)
- throws IOException, OptionsParsingException {
- OptionsParsingResult optionsProvider = env.getOptions();
-
+ CountingArtifactGroupNamer artifactGroupNamer) {
if (Strings.isNullOrEmpty(besOptions.besBackend)) {
clearBesClient();
return null;
- } else {
- String invocationId = env.getCommandId().toString();
- final String besResultsUrl;
- if (!Strings.isNullOrEmpty(besOptions.besResultsUrl)) {
- besResultsUrl =
- besOptions.besResultsUrl.endsWith("/")
- ? besOptions.besResultsUrl + invocationId
- : besOptions.besResultsUrl + "/" + invocationId;
- env.getReporter().handle(Event.info("Streaming Build Event Protocol to " + besResultsUrl));
- } else {
- besResultsUrl = null;
- env.getReporter()
- .handle(
- Event.info(
- String.format(
- "Streaming Build Event Protocol to %s build_request_id: %s "
- + "invocation_id: %s",
- besOptions.besBackend, env.getBuildRequestId(), invocationId)));
- }
-
- BuildEventServiceClient client = getBesClient(besOptions, authTlsOptions);
- BuildEventArtifactUploader artifactUploader = uploaderSupplier.get();
-
- BuildEventServiceProtoUtil besProtoUtil =
- new BuildEventServiceProtoUtil(
- env.getBuildRequestId(),
- invocationId,
- besOptions.projectId,
- env.getCommandName(),
- keywords(besOptions, env.getRuntime().getStartupOptionsProvider()));
-
- BuildEventTransport besTransport =
- new BuildEventServiceTransport.Builder()
- .closeTimeout(besOptions.besTimeout)
- .publishLifecycleEvents(besOptions.besLifecycleEvents)
- .setEventBus(env.getEventBus())
- .build(
- client,
- artifactUploader,
- bepOptions,
- besProtoUtil,
- env.getRuntime().getClock(),
- bazelExitFunction(
- env.getReporter(), env.getBlazeModuleEnvironment(), besResultsUrl),
- namer);
- return besTransport;
}
+
+ String besResultsUrl =
+ constructAndReportBesResultsMessage(
+ cmdEnv.getCommandId().toString(), cmdEnv.getBuildRequestId(), cmdEnv.getReporter());
+
+ final BuildEventServiceClient besClient;
+ try {
+ besClient = getBesClient(besOptions, authTlsOptions);
+ } catch (IOException | OptionsParsingException e) {
+ reportError(
+ cmdEnv.getReporter(),
+ cmdEnv.getBlazeModuleEnvironment(),
+ e.getMessage(),
+ e,
+ ExitCode.LOCAL_ENVIRONMENTAL_ERROR);
+ return null;
+ }
+
+ BuildEventServiceProtoUtil besProtoUtil =
+ new BuildEventServiceProtoUtil.Builder()
+ .buildRequestId(cmdEnv.getBuildRequestId())
+ .invocationId(cmdEnv.getCommandId().toString())
+ .projectId(besOptions.projectId)
+ .commandName(cmdEnv.getCommandName())
+ .keywords(getBesKeywords(besOptions, cmdEnv.getRuntime().getStartupOptionsProvider()))
+ .build();
+
+ return new BuildEventServiceTransport.Builder()
+ .localFileUploader(uploaderSupplier.get())
+ .besClient(besClient)
+ .besOptions(besOptions)
+ .besProtoUtil(besProtoUtil)
+ .artifactGroupNamer(artifactGroupNamer)
+ .bepOptions(bepOptions)
+ .clock(cmdEnv.getRuntime().getClock())
+ .exitFunction(
+ ExitFunction.standardExitFunction(
+ cmdEnv.getReporter(),
+ cmdEnv.getBlazeModuleEnvironment(),
+ besResultsUrl,
+ errorsShouldFailTheBuild()))
+ .eventBus(cmdEnv.getEventBus())
+ .build();
+ }
+
+ private ImmutableSet<BuildEventTransport> createBepTransports(
+ CommandEnvironment cmdEnv,
+ Supplier<BuildEventArtifactUploader> uploaderSupplier,
+ CountingArtifactGroupNamer artifactGroupNamer) {
+ // TODO(lpino): Rewrite the BuildEventTransportFactory using the Builder pattern too.
+ ImmutableSet<BuildEventTransport> bepFileTransports =
+ BuildEventTransportFactory.createFromOptions(
+ cmdEnv,
+ cmdEnv.getBlazeModuleEnvironment()::exit,
+ bepOptions,
+ uploaderSupplier,
+ artifactGroupNamer);
+ BuildEventServiceTransport besTransport =
+ createBesTransport(cmdEnv, uploaderSupplier, artifactGroupNamer);
+
+ ImmutableSet.Builder<BuildEventTransport> transportsBuilder =
+ ImmutableSet.<BuildEventTransport>builder().addAll(bepFileTransports);
+ if (besTransport != null) {
+ transportsBuilder.add(besTransport);
+ }
+ return transportsBuilder.build();
}
protected abstract Class<BESOptionsT> optionsClass();
@@ -302,42 +315,13 @@
protected abstract Set<String> whitelistedCommands(BESOptionsT besOptions);
- protected Set<String> keywords(
+ protected Set<String> getBesKeywords(
BESOptionsT besOptions, @Nullable OptionsParsingResult startupOptionsProvider) {
return besOptions.besKeywords.stream()
.map(keyword -> "user_keyword=" + keyword)
.collect(ImmutableSet.toImmutableSet());
}
- private ExitFunction bazelExitFunction(
- EventHandler commandLineReporter, ModuleEnvironment moduleEnvironment, String besResultsUrl) {
- return (String message, Throwable cause, ExitCode exitCode) -> {
- if (exitCode == ExitCode.SUCCESS) {
- Preconditions.checkState(cause == null, cause);
- commandLineReporter.handle(Event.info("Build Event Protocol upload finished successfully"));
- if (besResultsUrl != null) {
- commandLineReporter.handle(
- Event.info("Build Event Protocol results available at " + besResultsUrl));
- }
- } else {
- Preconditions.checkState(cause != null, cause);
- if (errorsShouldFailTheBuild()) {
- commandLineReporter.handle(Event.error(message));
- moduleEnvironment.exit(new AbruptExitException(exitCode, cause));
- } else {
- commandLineReporter.handle(Event.warn(message));
- }
- if (besResultsUrl != null) {
- if (!Strings.isNullOrEmpty(besResultsUrl)) {
- commandLineReporter.handle(
- Event.info(
- "Partial Build Event Protocol results may be available at " + besResultsUrl));
- }
- }
- }
- };
- }
-
// TODO(lpino): This method shouldn exist. It only does because some tests are relying on the
// transport creation logic of this module directly.
@VisibleForTesting
diff --git a/src/main/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceProtoUtil.java b/src/main/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceProtoUtil.java
index f398183..9e4f39b 100644
--- a/src/main/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceProtoUtil.java
+++ b/src/main/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceProtoUtil.java
@@ -14,6 +14,7 @@
package com.google.devtools.build.lib.buildeventservice;
+import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.devtools.build.v1.BuildEvent.BuildComponentStreamFinished.FinishType.FINISHED;
import com.google.common.annotations.VisibleForTesting;
@@ -46,7 +47,7 @@
private final String commandName;
private final Set<String> additionalKeywords;
- public BuildEventServiceProtoUtil(
+ private BuildEventServiceProtoUtil(
String buildRequestId,
String buildInvocationId,
@Nullable String projectId,
@@ -195,4 +196,47 @@
.addAll(additionalKeywords)
.build();
}
+
+ /** A builder for {@link BuildEventServiceProtoUtil}. */
+ public static class Builder {
+ private String invocationId;
+ private String buildRequestId;
+ private String commandName;
+ private Set<String> keywords;
+ private @Nullable String projectId;
+
+ public Builder buildRequestId(String value) {
+ this.buildRequestId = value;
+ return this;
+ }
+
+ public Builder invocationId(String value) {
+ this.invocationId = value;
+ return this;
+ }
+
+ public Builder projectId(String value) {
+ this.projectId = value;
+ return this;
+ }
+
+ public Builder commandName(String value) {
+ this.commandName = value;
+ return this;
+ }
+
+ public Builder keywords(Set<String> value) {
+ this.keywords = value;
+ return this;
+ }
+
+ public BuildEventServiceProtoUtil build() {
+ return new BuildEventServiceProtoUtil(
+ checkNotNull(buildRequestId),
+ checkNotNull(invocationId),
+ projectId,
+ checkNotNull(commandName),
+ checkNotNull(keywords));
+ }
+ }
}
diff --git a/src/main/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceTransport.java b/src/main/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceTransport.java
index 2c38546..7c684de 100644
--- a/src/main/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceTransport.java
+++ b/src/main/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceTransport.java
@@ -14,9 +14,9 @@
package com.google.devtools.build.lib.buildeventservice;
+import static com.google.common.base.Preconditions.checkNotNull;
import com.google.common.annotations.VisibleForTesting;
-import com.google.common.base.Preconditions;
import com.google.common.eventbus.EventBus;
import com.google.common.util.concurrent.ListenableFuture;
import com.google.common.util.concurrent.MoreExecutors;
@@ -28,69 +28,15 @@
import com.google.devtools.build.lib.buildeventstream.BuildEventProtocolOptions;
import com.google.devtools.build.lib.buildeventstream.BuildEventTransport;
import com.google.devtools.build.lib.clock.Clock;
-import com.google.devtools.build.lib.util.ExitCode;
import com.google.devtools.build.lib.util.JavaSleeper;
import com.google.devtools.build.lib.util.Sleeper;
import java.time.Duration;
+import javax.annotation.Nullable;
/** A {@link BuildEventTransport} that streams {@link BuildEvent}s to BuildEventService. */
public class BuildEventServiceTransport implements BuildEventTransport {
private final BuildEventServiceUploader besUploader;
- /** A builder for {@link BuildEventServiceTransport}. */
- public static class Builder {
- private boolean publishLifecycleEvents;
- private Duration closeTimeout;
- private Sleeper sleeper;
- private EventBus eventBus;
-
- /** Whether to publish lifecycle events. */
- public Builder publishLifecycleEvents(boolean publishLifecycleEvents) {
- this.publishLifecycleEvents = publishLifecycleEvents;
- return this;
- }
-
- /** The time to wait for the build event upload after the build has completed. */
- public Builder closeTimeout(Duration closeTimeout) {
- this.closeTimeout = closeTimeout;
- return this;
- }
-
- @VisibleForTesting
- public Builder sleeper(Sleeper sleeper) {
- this.sleeper = sleeper;
- return this;
- }
-
- public Builder setEventBus(EventBus eventBus) {
- this.eventBus = eventBus;
- return this;
- }
-
- public BuildEventServiceTransport build(
- BuildEventServiceClient besClient,
- BuildEventArtifactUploader localFileUploader,
- BuildEventProtocolOptions bepOptions,
- BuildEventServiceProtoUtil besProtoUtil,
- Clock clock,
- ExitFunction exitFunction,
- ArtifactGroupNamer namer) {
- Preconditions.checkNotNull(eventBus);
- return new BuildEventServiceTransport(
- besClient,
- localFileUploader,
- bepOptions,
- besProtoUtil,
- clock,
- exitFunction,
- publishLifecycleEvents,
- closeTimeout != null ? closeTimeout : Duration.ZERO,
- sleeper != null ? sleeper : new JavaSleeper(),
- namer,
- eventBus);
- }
- }
-
private BuildEventServiceTransport(
BuildEventServiceClient besClient,
BuildEventArtifactUploader localFileUploader,
@@ -99,23 +45,24 @@
Clock clock,
ExitFunction exitFunc,
boolean publishLifecycleEvents,
+ ArtifactGroupNamer artifactGroupNamer,
+ EventBus eventBus,
Duration closeTimeout,
- Sleeper sleeper,
- ArtifactGroupNamer namer,
- EventBus eventBus) {
+ Sleeper sleeper) {
this.besUploader =
- new BuildEventServiceUploader(
- besClient,
- localFileUploader,
- besProtoUtil,
- bepOptions,
- publishLifecycleEvents,
- closeTimeout,
- exitFunc,
- sleeper,
- clock,
- namer,
- eventBus);
+ new BuildEventServiceUploader.Builder()
+ .besClient(besClient)
+ .localFileUploader(localFileUploader)
+ .bepOptions(bepOptions)
+ .besProtoUtil(besProtoUtil)
+ .clock(clock)
+ .exitFunc(exitFunc)
+ .publishLifecycleEvents(publishLifecycleEvents)
+ .sleeper(sleeper)
+ .artifactGroupNamer(artifactGroupNamer)
+ .eventBus(eventBus)
+ .closeTimeout(closeTimeout)
+ .build();
}
@Override
@@ -148,12 +95,84 @@
return besUploader;
}
- /**
- * Called by the {@link BuildEventServiceUploader} in case of error to asynchronously notify Bazel
- * of an error.
- */
- @FunctionalInterface
- public interface ExitFunction {
- void accept(String message, Throwable cause, ExitCode code);
+ /** A builder for {@link BuildEventServiceTransport}. */
+ public static class Builder {
+ private BuildEventServiceClient besClient;
+ private BuildEventArtifactUploader localFileUploader;
+ private BuildEventServiceOptions besOptions;
+ private BuildEventProtocolOptions bepOptions;
+ private Clock clock;
+ private ArtifactGroupNamer artifactGroupNamer;
+ private BuildEventServiceProtoUtil besProtoUtil;
+ private EventBus eventBus;
+ private ExitFunction exitFunction;
+ private @Nullable Sleeper sleeper;
+
+ public Builder besClient(BuildEventServiceClient value) {
+ this.besClient = value;
+ return this;
+ }
+
+ public Builder localFileUploader(BuildEventArtifactUploader value) {
+ this.localFileUploader = value;
+ return this;
+ }
+
+ public Builder besProtoUtil(BuildEventServiceProtoUtil value) {
+ this.besProtoUtil = value;
+ return this;
+ }
+
+ public Builder bepOptions(BuildEventProtocolOptions value) {
+ this.bepOptions = value;
+ return this;
+ }
+
+ public Builder besOptions(BuildEventServiceOptions value) {
+ this.besOptions = value;
+ return this;
+ }
+
+ public Builder clock(Clock value) {
+ this.clock = value;
+ return this;
+ }
+
+ public Builder artifactGroupNamer(ArtifactGroupNamer value) {
+ this.artifactGroupNamer = value;
+ return this;
+ }
+
+ public Builder eventBus(EventBus value) {
+ this.eventBus = value;
+ return this;
+ }
+
+ public Builder exitFunction(ExitFunction value) {
+ this.exitFunction = value;
+ return this;
+ }
+
+ @VisibleForTesting
+ public Builder sleeper(Sleeper value) {
+ this.sleeper = value;
+ return this;
+ }
+
+ public BuildEventServiceTransport build() {
+ checkNotNull(besOptions);
+ return new BuildEventServiceTransport(
+ checkNotNull(besClient),
+ checkNotNull(localFileUploader),
+ checkNotNull(bepOptions),
+ checkNotNull(besProtoUtil),
+ checkNotNull(clock),
+ checkNotNull(exitFunction),
+ besOptions.besLifecycleEvents,
+ checkNotNull(artifactGroupNamer),
+ checkNotNull(eventBus),
+ (besOptions.besTimeout != null) ? besOptions.besTimeout : Duration.ZERO,
+ sleeper != null ? sleeper : new JavaSleeper());
+ }
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceUploader.java b/src/main/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceUploader.java
index 140f2c8..3c48db7f 100644
--- a/src/main/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceUploader.java
+++ b/src/main/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceUploader.java
@@ -13,6 +13,7 @@
// limitations under the License.
package com.google.devtools.build.lib.buildeventservice;
+import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.base.Preconditions.checkState;
import static com.google.common.util.concurrent.Uninterruptibles.getUninterruptibly;
import static com.google.devtools.build.v1.BuildStatus.Result.COMMAND_FAILED;
@@ -29,7 +30,6 @@
import com.google.common.util.concurrent.ListenableFuture;
import com.google.common.util.concurrent.MoreExecutors;
import com.google.common.util.concurrent.SettableFuture;
-import com.google.devtools.build.lib.buildeventservice.BuildEventServiceTransport.ExitFunction;
import com.google.devtools.build.lib.buildeventservice.BuildEventServiceUploaderCommands.AckReceivedCommand;
import com.google.devtools.build.lib.buildeventservice.BuildEventServiceUploaderCommands.EventLoopCommand;
import com.google.devtools.build.lib.buildeventservice.BuildEventServiceUploaderCommands.OpenStreamCommand;
@@ -148,7 +148,7 @@
private StreamContext streamContext;
- BuildEventServiceUploader(
+ private BuildEventServiceUploader(
BuildEventServiceClient besClient,
BuildEventArtifactUploader localFileUploader,
BuildEventServiceProtoUtil besProtoUtil,
@@ -160,15 +160,15 @@
Clock clock,
ArtifactGroupNamer namer,
EventBus eventBus) {
- this.besClient = Preconditions.checkNotNull(besClient);
- this.localFileUploader = Preconditions.checkNotNull(localFileUploader);
- this.besProtoUtil = Preconditions.checkNotNull(besProtoUtil);
+ this.besClient = besClient;
+ this.localFileUploader = localFileUploader;
+ this.besProtoUtil = besProtoUtil;
this.buildEventProtocolOptions = buildEventProtocolOptions;
this.publishLifecycleEvents = publishLifecycleEvents;
- this.closeTimeout = Preconditions.checkNotNull(closeTimeout);
- this.exitFunc = Preconditions.checkNotNull(exitFunc);
- this.sleeper = Preconditions.checkNotNull(sleeper);
- this.clock = Preconditions.checkNotNull(clock);
+ this.closeTimeout = closeTimeout;
+ this.exitFunc = exitFunc;
+ this.sleeper = sleeper;
+ this.clock = clock;
this.namer = namer;
this.eventBus = eventBus;
}
@@ -750,5 +750,89 @@
super(cause);
}
}
+
+ static class Builder {
+ private BuildEventServiceClient besClient;
+ private BuildEventArtifactUploader localFileUploader;
+ private BuildEventServiceProtoUtil besProtoUtil;
+ private BuildEventProtocolOptions bepOptions;
+ private boolean publishLifecycleEvents;
+ private Duration closeTimeout;
+ private ExitFunction exitFunc;
+ private Sleeper sleeper;
+ private Clock clock;
+ private ArtifactGroupNamer artifactGroupNamer;
+ private EventBus eventBus;
+
+ Builder besClient(BuildEventServiceClient value) {
+ this.besClient = value;
+ return this;
+ }
+
+ Builder localFileUploader(BuildEventArtifactUploader value) {
+ this.localFileUploader = value;
+ return this;
+ }
+
+ Builder besProtoUtil(BuildEventServiceProtoUtil value) {
+ this.besProtoUtil = value;
+ return this;
+ }
+
+ Builder bepOptions(BuildEventProtocolOptions value) {
+ this.bepOptions = value;
+ return this;
+ }
+
+ Builder publishLifecycleEvents(boolean value) {
+ this.publishLifecycleEvents = value;
+ return this;
+ }
+
+ Builder closeTimeout(Duration value) {
+ this.closeTimeout = value;
+ return this;
+ }
+
+ Builder clock(Clock value) {
+ this.clock = value;
+ return this;
+ }
+
+ Builder exitFunc(ExitFunction value) {
+ this.exitFunc = value;
+ return this;
+ }
+
+ Builder sleeper(Sleeper value) {
+ this.sleeper = value;
+ return this;
+ }
+
+ Builder artifactGroupNamer(ArtifactGroupNamer value) {
+ this.artifactGroupNamer = value;
+ return this;
+ }
+
+ Builder eventBus(EventBus value) {
+ this.eventBus = value;
+ return this;
+ }
+
+ BuildEventServiceUploader build() {
+ return new BuildEventServiceUploader(
+ checkNotNull(besClient),
+ checkNotNull(localFileUploader),
+ checkNotNull(besProtoUtil),
+ checkNotNull(bepOptions),
+ publishLifecycleEvents,
+ checkNotNull(closeTimeout),
+ checkNotNull(exitFunc),
+ checkNotNull(sleeper),
+ checkNotNull(clock),
+ checkNotNull(artifactGroupNamer),
+ checkNotNull(eventBus));
+ }
+ }
}
diff --git a/src/main/java/com/google/devtools/build/lib/buildeventservice/ExitFunction.java b/src/main/java/com/google/devtools/build/lib/buildeventservice/ExitFunction.java
new file mode 100644
index 0000000..01ea842
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/buildeventservice/ExitFunction.java
@@ -0,0 +1,67 @@
+// Copyright 2019 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.lib.buildeventservice;
+
+import com.google.common.base.Preconditions;
+import com.google.common.base.Strings;
+import com.google.devtools.build.lib.buildeventstream.BuildEventTransport;
+import com.google.devtools.build.lib.events.Event;
+import com.google.devtools.build.lib.events.EventHandler;
+import com.google.devtools.build.lib.runtime.BlazeModule.ModuleEnvironment;
+import com.google.devtools.build.lib.util.AbruptExitException;
+import com.google.devtools.build.lib.util.ExitCode;
+import javax.annotation.Nullable;
+
+/**
+ * Function used by a {@link BuildEventTransport} to report errors using the provided {@link
+ * EventHandler} and maybe exit abruptly.
+ */
+@FunctionalInterface
+public interface ExitFunction {
+ void accept(String message, Throwable cause, ExitCode code);
+
+ // TODO(lpino): The error handling should in the BES module but we can't do that yet because
+ // we use the exit function inside the {@link BuildEventServiceUploader}.
+ static ExitFunction standardExitFunction(
+ EventHandler commandLineReporter,
+ ModuleEnvironment moduleEnvironment,
+ @Nullable String besResultsUrl,
+ boolean errorsFailBuild) {
+ return (String message, Throwable cause, ExitCode exitCode) -> {
+ if (exitCode == ExitCode.SUCCESS) {
+ Preconditions.checkState(cause == null, cause);
+ commandLineReporter.handle(Event.info("Build Event Protocol upload finished successfully"));
+ if (besResultsUrl != null) {
+ commandLineReporter.handle(
+ Event.info("Build Event Protocol results available at " + besResultsUrl));
+ }
+ } else {
+ Preconditions.checkState(cause != null, cause);
+ if (errorsFailBuild) {
+ commandLineReporter.handle(Event.error(message));
+ moduleEnvironment.exit(new AbruptExitException(exitCode, cause));
+ } else {
+ commandLineReporter.handle(Event.warn(message));
+ }
+ if (besResultsUrl != null) {
+ if (!Strings.isNullOrEmpty(besResultsUrl)) {
+ commandLineReporter.handle(
+ Event.info(
+ "Partial Build Event Protocol results may be available at " + besResultsUrl));
+ }
+ }
+ }
+ };
+ }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/buildeventstream/transports/BinaryFormatFileTransport.java b/src/main/java/com/google/devtools/build/lib/buildeventstream/transports/BinaryFormatFileTransport.java
index 29bbdef..53e8a12 100644
--- a/src/main/java/com/google/devtools/build/lib/buildeventstream/transports/BinaryFormatFileTransport.java
+++ b/src/main/java/com/google/devtools/build/lib/buildeventstream/transports/BinaryFormatFileTransport.java
@@ -37,8 +37,7 @@
BuildEventProtocolOptions options,
BuildEventArtifactUploader uploader,
Consumer<AbruptExitException> exitFunc,
- ArtifactGroupNamer namer)
- throws IOException {
+ ArtifactGroupNamer namer) {
super(path, options, uploader, exitFunc, namer);
}
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 be9880c..ec9cb63 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
@@ -23,7 +23,6 @@
import com.google.devtools.build.lib.util.AbruptExitException;
import com.google.protobuf.InvalidProtocolBufferException;
import com.google.protobuf.util.JsonFormat;
-import java.io.IOException;
import java.util.function.Consumer;
/**
@@ -36,8 +35,7 @@
BuildEventProtocolOptions options,
BuildEventArtifactUploader uploader,
Consumer<AbruptExitException> exitFunc,
- ArtifactGroupNamer namer)
- throws IOException {
+ ArtifactGroupNamer namer) {
super(path, options, uploader, exitFunc, namer);
}
diff --git a/src/main/java/com/google/devtools/build/lib/buildeventstream/transports/TextFormatFileTransport.java b/src/main/java/com/google/devtools/build/lib/buildeventstream/transports/TextFormatFileTransport.java
index c7cd181..36a5f95 100644
--- a/src/main/java/com/google/devtools/build/lib/buildeventstream/transports/TextFormatFileTransport.java
+++ b/src/main/java/com/google/devtools/build/lib/buildeventstream/transports/TextFormatFileTransport.java
@@ -22,7 +22,6 @@
import com.google.devtools.build.lib.buildeventstream.BuildEventTransport;
import com.google.devtools.build.lib.util.AbruptExitException;
import com.google.protobuf.TextFormat;
-import java.io.IOException;
import java.util.function.Consumer;
/**
@@ -37,8 +36,7 @@
BuildEventProtocolOptions options,
BuildEventArtifactUploader uploader,
Consumer<AbruptExitException> exitFunc,
- ArtifactGroupNamer namer)
- throws IOException {
+ ArtifactGroupNamer namer) {
super(path, options, uploader, exitFunc, namer);
}
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BuildEventStreamer.java b/src/main/java/com/google/devtools/build/lib/runtime/BuildEventStreamer.java
index bf1d062..87fb0f6 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BuildEventStreamer.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BuildEventStreamer.java
@@ -14,7 +14,6 @@
package com.google.devtools.build.lib.runtime;
-import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.base.Strings.nullToEmpty;
import static com.google.devtools.build.lib.events.Event.of;
@@ -142,13 +141,11 @@
}
/** Creates a new build event streamer. */
- public BuildEventStreamer(
+ private BuildEventStreamer(
Collection<BuildEventTransport> transports,
@Nullable Reporter reporter,
BuildEventStreamOptions options,
CountingArtifactGroupNamer artifactGroupNamer) {
- checkArgument(transports.size() > 0);
- checkNotNull(options);
this.transports = transports;
this.reporter = reporter;
this.options = options;
@@ -728,4 +725,40 @@
private boolean isVacuousTestSummary(BuildEvent event) {
return event instanceof TestSummary && (((TestSummary) event).totalRuns() == 0);
}
+
+ /** A builder for {@link BuildEventStreamer}. */
+ public static class Builder {
+ private Set<BuildEventTransport> buildEventTransports;
+ private Reporter cmdLineReporter;
+ private BuildEventStreamOptions besStreamOptions;
+ private CountingArtifactGroupNamer artifactGroupNamer;
+
+ public Builder buildEventTransports(Set<BuildEventTransport> value) {
+ this.buildEventTransports = value;
+ return this;
+ }
+
+ public Builder cmdLineReporter(Reporter value) {
+ this.cmdLineReporter = value;
+ return this;
+ }
+
+ public Builder besStreamOptions(BuildEventStreamOptions value) {
+ this.besStreamOptions = value;
+ return this;
+ }
+
+ public Builder artifactGroupNamer(CountingArtifactGroupNamer value) {
+ this.artifactGroupNamer = value;
+ return this;
+ }
+
+ public BuildEventStreamer build() {
+ return new BuildEventStreamer(
+ checkNotNull(buildEventTransports),
+ checkNotNull(cmdLineReporter),
+ checkNotNull(besStreamOptions),
+ checkNotNull(artifactGroupNamer));
+ }
+ }
}
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BuildEventTransportFactory.java b/src/main/java/com/google/devtools/build/lib/runtime/BuildEventTransportFactory.java
index f5c8ab8..2c48b5b 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BuildEventTransportFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BuildEventTransportFactory.java
@@ -47,8 +47,7 @@
BuildEventProtocolOptions protocolOptions,
BuildEventArtifactUploader uploader,
Consumer<AbruptExitException> exitFunc,
- ArtifactGroupNamer namer)
- throws IOException {
+ ArtifactGroupNamer namer) {
return new TextFormatFileTransport(
options.getBuildEventTextFile(), protocolOptions, uploader, exitFunc, namer);
}
@@ -71,8 +70,7 @@
BuildEventProtocolOptions protocolOptions,
BuildEventArtifactUploader uploader,
Consumer<AbruptExitException> exitFunc,
- ArtifactGroupNamer namer)
- throws IOException {
+ ArtifactGroupNamer namer) {
return new BinaryFormatFileTransport(
options.getBuildEventBinaryFile(), protocolOptions, uploader, exitFunc, namer);
}
@@ -95,8 +93,7 @@
BuildEventProtocolOptions protocolOptions,
BuildEventArtifactUploader uploader,
Consumer<AbruptExitException> exitFunc,
- ArtifactGroupNamer namer)
- throws IOException {
+ ArtifactGroupNamer namer) {
return new JsonFormatFileTransport(
options.getBuildEventJsonFile(), protocolOptions, uploader, exitFunc, namer);
}
@@ -135,8 +132,7 @@
Consumer<AbruptExitException> exitFunc,
BuildEventProtocolOptions protocolOptions,
Supplier<BuildEventArtifactUploader> uploaderSupplier,
- ArtifactGroupNamer namer)
- throws IOException {
+ ArtifactGroupNamer namer) {
BuildEventStreamOptions bepOptions =
checkNotNull(
env.getOptions().getOptions(BuildEventStreamOptions.class),
@@ -164,8 +160,7 @@
BuildEventProtocolOptions protocolOptions,
BuildEventArtifactUploader uploader,
Consumer<AbruptExitException> exitFunc,
- ArtifactGroupNamer namer)
- throws IOException;
+ ArtifactGroupNamer namer);
protected abstract boolean usePathConverter(BuildEventStreamOptions options);
}
diff --git a/src/test/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceProtoUtilTest.java b/src/test/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceProtoUtilTest.java
index d435140..2107b66 100644
--- a/src/test/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceProtoUtilTest.java
+++ b/src/test/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceProtoUtilTest.java
@@ -52,19 +52,20 @@
private static final String ADDITIONAL_KEYWORD = "keyword=foo";
private static final ImmutableList<String> EXPECTED_KEYWORDS =
ImmutableList.of("command_name=" + COMMAND_NAME, "protocol_name=BEP", ADDITIONAL_KEYWORD);
+ private static final BuildEventServiceProtoUtil BES_PROTO_UTIL =
+ new BuildEventServiceProtoUtil.Builder()
+ .buildRequestId(BUILD_REQUEST_ID)
+ .invocationId(BUILD_INVOCATION_ID)
+ .projectId(PROJECT_ID)
+ .commandName(COMMAND_NAME)
+ .keywords(ImmutableSet.of(ADDITIONAL_KEYWORD))
+ .build();
private final ManualClock clock = new ManualClock();
- private final BuildEventServiceProtoUtil besProtocol =
- new BuildEventServiceProtoUtil(
- BUILD_REQUEST_ID,
- BUILD_INVOCATION_ID,
- PROJECT_ID,
- COMMAND_NAME,
- ImmutableSet.of(ADDITIONAL_KEYWORD));
@Test
public void testBuildEnqueued() {
Timestamp expected = Timestamps.fromMillis(clock.advanceMillis(100));
- assertThat(besProtocol.buildEnqueued(expected))
+ assertThat(BES_PROTO_UTIL.buildEnqueued(expected))
.isEqualTo(
PublishLifecycleEventRequest.newBuilder()
.setServiceLevel(ServiceLevel.INTERACTIVE)
@@ -86,7 +87,7 @@
@Test
public void testInvocationAttemptStarted() {
Timestamp expected = Timestamps.fromMillis(clock.advanceMillis(100));
- assertThat(besProtocol.invocationStarted(expected))
+ assertThat(BES_PROTO_UTIL.invocationStarted(expected))
.isEqualTo(
PublishLifecycleEventRequest.newBuilder()
.setServiceLevel(ServiceLevel.INTERACTIVE)
@@ -110,7 +111,7 @@
@Test
public void testInvocationAttemptFinished() {
Timestamp expected = Timestamps.fromMillis(clock.advanceMillis(100));
- assertThat(besProtocol.invocationFinished(expected, Result.COMMAND_SUCCEEDED))
+ assertThat(BES_PROTO_UTIL.invocationFinished(expected, Result.COMMAND_SUCCEEDED))
.isEqualTo(
PublishLifecycleEventRequest.newBuilder()
.setServiceLevel(ServiceLevel.INTERACTIVE)
@@ -137,7 +138,7 @@
@Test
public void testBuildFinished() {
Timestamp expected = Timestamps.fromMillis(clock.advanceMillis(100));
- assertThat(besProtocol.buildFinished(expected, Result.COMMAND_SUCCEEDED))
+ assertThat(BES_PROTO_UTIL.buildFinished(expected, Result.COMMAND_SUCCEEDED))
.isEqualTo(
PublishLifecycleEventRequest.newBuilder()
.setServiceLevel(ServiceLevel.INTERACTIVE)
@@ -164,7 +165,7 @@
public void testStreamEvents() {
Timestamp firstEventTimestamp = Timestamps.fromMillis(clock.advanceMillis(100));
Any anything = Any.getDefaultInstance();
- assertThat(besProtocol.bazelEvent(1, firstEventTimestamp, anything))
+ assertThat(BES_PROTO_UTIL.bazelEvent(1, firstEventTimestamp, anything))
.isEqualTo(
PublishBuildToolEventStreamRequest.newBuilder()
.addAllNotificationKeywords(EXPECTED_KEYWORDS)
@@ -185,7 +186,7 @@
.build());
Timestamp secondEventTimestamp = Timestamps.fromMillis(clock.advanceMillis(100));
- assertThat(besProtocol.bazelEvent(2, secondEventTimestamp, anything))
+ assertThat(BES_PROTO_UTIL.bazelEvent(2, secondEventTimestamp, anything))
.isEqualTo(
PublishBuildToolEventStreamRequest.newBuilder()
.setProjectId(PROJECT_ID)
@@ -205,7 +206,7 @@
.build());
Timestamp thirdEventTimestamp = Timestamps.fromMillis(clock.advanceMillis(100));
- assertThat(besProtocol.streamFinished(3, thirdEventTimestamp))
+ assertThat(BES_PROTO_UTIL.streamFinished(3, thirdEventTimestamp))
.isEqualTo(
PublishBuildToolEventStreamRequest.newBuilder()
.setProjectId(PROJECT_ID)
diff --git a/src/test/java/com/google/devtools/build/lib/runtime/BuildEventStreamerTest.java b/src/test/java/com/google/devtools/build/lib/runtime/BuildEventStreamerTest.java
index 5be246d..fef7e48 100644
--- a/src/test/java/com/google/devtools/build/lib/runtime/BuildEventStreamerTest.java
+++ b/src/test/java/com/google/devtools/build/lib/runtime/BuildEventStreamerTest.java
@@ -1028,7 +1028,12 @@
options.publishAllActions = true;
BuildEventStreamer streamer =
- new BuildEventStreamer(ImmutableSet.of(transport), reporter, options, artifactGroupNamer);
+ new BuildEventStreamer.Builder()
+ .artifactGroupNamer(artifactGroupNamer)
+ .besStreamOptions(options)
+ .cmdLineReporter(reporter)
+ .buildEventTransports(ImmutableSet.of(transport))
+ .build();
ActionExecutedEvent failedActionExecutedEvent =
new ActionExecutedEvent(
