Skip to content

Commit

Permalink
Restrict writing build events to a white-listed set of commands
Browse files Browse the repository at this point in the history
It is not desirable to have a build-event stream for all invocations of blaze;
so restrict the BuildEventServiceModule to be only active if the executed
command is in a white list of commands for which having build events is desirable.

Change-Id: Id4acf9468f67b1af7fc8ea703785df5229c69258
PiperOrigin-RevId: 161207563
  • Loading branch information
aehlig authored and katre committed Jul 7, 2017
1 parent 25ef6a8 commit 93a84bf
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,11 @@

package com.google.devtools.build.lib.buildeventservice;

import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.authandtls.AuthAndTLSOptions;
import com.google.devtools.build.lib.buildeventservice.client.BuildEventServiceClient;
import com.google.devtools.build.lib.buildeventservice.client.BuildEventServiceGrpcClient;
import java.util.Set;

/**
* Bazel's BES module.
Expand All @@ -37,4 +39,9 @@ protected BuildEventServiceClient createBesClient(BuildEventServiceOptions besOp
authAndTLSOptions.tlsAuthorityOverride, authAndTLSOptions.authCredentials,
authAndTLSOptions.authScope);
}

@Override
protected Set<String> whitelistedCommands() {
return ImmutableSet.of("build", "test", "run", "query", "coverage", "mobile-install");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
import com.google.devtools.common.options.OptionsProvider;
import java.util.ArrayList;
import java.util.List;
import java.util.Set;
import java.util.UUID;
import java.util.logging.Logger;
import javax.annotation.Nullable;
Expand All @@ -66,6 +67,7 @@ public abstract class BuildEventServiceModule<T extends BuildEventServiceOptions
private CommandEnvironment commandEnvironment;
private SynchronizedOutputStream out;
private SynchronizedOutputStream err;
private boolean disabled;

private static class BuildEventRecorder {
private final List<BuildEvent> events = new ArrayList<>();
Expand All @@ -90,13 +92,21 @@ public Iterable<Class<? extends OptionsBase>> getCommandOptions(Command command)
@Override
public void beforeCommand(CommandEnvironment commandEnvironment)
throws AbruptExitException {
disabled = false;
if (!whitelistedCommands().contains(commandEnvironment.getCommandName())) {
disabled = true;
return;
}
this.commandEnvironment = commandEnvironment;
this.buildEventRecorder = new BuildEventRecorder();
commandEnvironment.getEventBus().register(buildEventRecorder);
}

@Override
public void handleOptions(OptionsProvider optionsProvider) {
if (disabled) {
return;
}
checkState(commandEnvironment != null, "Methods called out of order");
BuildEventStreamer streamer =
tryCreateStreamer(
Expand Down Expand Up @@ -158,6 +168,9 @@ public String getErr() {

@Override
public OutErr getOutputListener() {
if (disabled) {
return null;
}
this.out = new SynchronizedOutputStream();
this.err = new SynchronizedOutputStream();
return OutErr.create(this.out, this.err);
Expand Down Expand Up @@ -266,4 +279,6 @@ private BuildEventTransport tryCreateBesTransport(T besOptions, AuthAndTLSOption

protected abstract BuildEventServiceClient createBesClient(T besOptions,
AuthAndTLSOptions authAndTLSOptions);

protected abstract Set<String> whitelistedCommands();
}
17 changes: 14 additions & 3 deletions src/test/shell/integration/build_event_stream_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -379,9 +379,6 @@ function test_build_only() {
function test_query() {
# Verify that at least a minimally meaningful event stream is generated
# for non-build. In particular, we expect bazel not to crash.
bazel version --build_event_text_file=$TEST_log \
|| fail "bazel version failed"
expect_log '^started'
bazel query --build_event_text_file=$TEST_log 'tests(//...)' \
|| fail "bazel query failed"
expect_log '^started'
Expand All @@ -397,6 +394,20 @@ function test_query() {
expect_log 'name: "SUCCESS"'
}

function test_command_whitelisting() {
# We expect the "help" command to not generate a build-event stream,
# but the "build" command to do.
rm -f bep.txt
bazel help --build_event_text_file=bep.txt || fail "bazel help failed"
( [ -f bep.txt ] && fail "bazel help generated a build-event file" ) || :
bazel version --build_event_text_file=bep.txt || fail "bazel help failed"
( [ -f bep.txt ] && fail "bazel version generated a build-event file" ) || :
bazel build --build_event_text_file=bep.txt //pkg:true \
|| fail "bazel build failed"
[ -f bep.txt ] || fail "build did not generate requested build-event file"
rm -f bep.txt
}

function test_multiple_transports() {
# Verifies usage of multiple build event transports at the same time
outdir=$(mktemp -d ${TEST_TMPDIR}/bazel.XXXXXXXX)
Expand Down

0 comments on commit 93a84bf

Please sign in to comment.