Add an option for printing selected server logs on failure. by EdSchouten · Pull Request #6591 · bazelbuild/bazel

EdSchouten · 2018-11-04T09:34:14Z

Bazel Buildbarn recently gained a web frontend called bbb_browser that
can be used to explore the CAS/AC. It allows users to get better insight
in how remote execution works under the hood. It also makes it possible
for users to more easily share information on build failures with their
peers.

In order to make it easy for people to access this service, we'd like
build failures to automatically print links to the corresponding page in
bbb_browser to the terminal. RE's server_logs feature is ideally suited
for this. Unfortunately, there exists no mechanism to print the output
of these files to the terminal yet.

This change adds a new command line flag, --print_server_log_on_failure,
that can print selected server logs to the terminal automatically. Logs
are identified by the key that was used to store them in the map.

EdSchouten · 2018-11-04T09:59:21Z

Obligatory screenshot. Notice the "Build failure details" line, which was stored in a server log.

buchgr · 2018-11-05T11:51:51Z

Hi Ed,

have you considered implementing a BES service in Buildbarn? I think that might be a more idiomatic way to do what you want to do. We also have the --build_results_url= flag in Bazel to display a URL where a user can look up details.

See e.g. https://source.cloud.google.com/results/invocations/48d7938b-2f43-4585-b51d-19fd7113c776/targets and
https://docs.bazel.build/versions/master/build-event-protocol.html

EdSchouten · 2018-11-05T13:41:38Z

Hi Jakob. Hope you're doing well! \o/

The Build Event Service looks pretty neat as well, but I think it fulfills a different purpose. The BES acts as a high-level logging system for full Bazel build/test runs (the 'top half' of Bazel). The page I built is intended for inspecting the literal data structures used by remote execution (the 'bottom half' of Bazel). It can, for example, be used to browse the input root of a build action stored in the CAS. This is quite valuable when diagnosing issues in the remote build implementation or self-made build rules.

For those who are curious, below is a screenshot of bbb_browser showing information about a failed build action:

buchgr · 2018-11-05T14:18:41Z

Thanks for explaining. You are indeed correct. This looks great!

I have two questions:

How would this look like with multiple failing targets?
Wouldn't ExecuteResponse.status.message be a better fit than putting this into a server log file? I would be more comfortable with unconditionally printing the message for failed executions than printing a server log.

EdSchouten · 2018-11-05T15:05:21Z

Below is a screenshot of what this looks like for a couple of errors in a row. Notice that they are all printed right above the corresponding ERROR statements.

With regards to using ExecuteResponse.status.message: in case of simple build failures (i.e., the builder as a whole functioned properly, but it's just that we saw a non-zero exit code), ExecuteResponse.status is intended to be nil/OK, right? That likely means that there's no room for returning any info.

buchgr · 2018-11-05T15:32:06Z

You are right with regard to status - I always mix this up :-).

We can do this. I am not convinced that using server_log is the right approach in the long term but it's an acceptable solution for now:

This seems like a feature all remote execution systems will want to have eventually. So this should just work by default without having to specify a flag and agreeing on a log name.
INFO events are expected to be short (ideally < 80 characters) and thus a generic mechanism to dump a log file content as an INFO event doesn't seem ideal.

I think a better approach would be to add an additional_info_url field to the ExecuteResponse message. What do you think? Would you mind opening an issue for discussion here: https://github.com/bazelbuild/remote-apis?

As mentioned in bazelbuild/bazel#6591, Bazel Buildbarn now ships with a web service that can be used to explore data stored in the CAS and AC. By adding an extra field to ExecuteResponse, we can let Bazel automatically report URLs to the user pointing to the correct page. These URLs could, for example, be printed for build steps that fail.

EdSchouten · 2018-11-05T15:55:30Z

Having an additional_info_url as part of the protocol would be a very good idea:

As Bazel only downloads server logs upon failure, this patch doesn't allow me to get the URL for actions that do build properly.
It reduces the amount of stuff that needs to be written in the CAS.

I just opened this pull request: bazelbuild/remote-apis#36. Let's close this one and continue discussion there. Thanks!

buchgr

I wonder if it would make sense to simply append this information to the stderr instead of posting it as an info event. That has the additional advantage of this information being printed after the error message, which seems nicer to me.

src/main/java/com/google/devtools/build/lib/exec/ExecutionOptions.java

src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java

buchgr · 2018-11-05T16:01:49Z

@EdSchouten I wasn't suggesting for you to close this one. I think we could certainly merge this as-is and iterate.

EdSchouten · 2018-11-23T14:03:28Z

Hi Jakob,

I've just altered this pull request to no longer use server logs. Instead, it now uses the ExecuteResponse's message field. The change is equivalent featurewise; it prints the information upon build failures.

That said, it would also be interesting to show this information for successful targets when not too noisy. Being able to obtains links to those would allow people to share build output (e.g., firmware images) with each other.

I was thinking that BuildResultPrinter's showBuildResult() would be the right spot for printing that. The only thing that isn't clear to me is how I can get that information to trickle all the way up to the BuildResultPrinter. Do you have any idea what would be the cleanest way to realise that?

buchgr · 2018-11-23T14:57:33Z

That said, it would also be interesting to show this information for successful targets when not too noisy. Being able to obtains links to those would allow people to share build output (e.g., firmware images) with each other.

I think this is the classic BEP/BES use case and best implemented there. The BEP will reference all outputs of top level targets and Bazel can upload this information via the BES protocol and also print a link (--bes_results_url) to a user readable page. The BEP can reference output files as a link to the CAS of the remote execution system. So I think that would be perfect for what you want to do.

buchgr · 2018-11-23T15:01:01Z

src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java

+              String message = reply.getMessage();
+              if ((reply.getResult().getExitCode() != 0 ||
+                   reply.getStatus().getCode() != Code.OK.value()) && !message.isEmpty()) {
+                outErr.printErr(message + "\n");


Just to clarify:

For build failures Bazel will print this to the UI

For test failures this will be the last line in the test.log

Correct?

Exactly. It's printed in those two conditions.

buchgr · 2018-12-13T10:38:18Z

Ping @EdSchouten :-)

This includes the new ExecuteResponse's message field that is dedicated to containing log entries that may be printed to the user.

The message may contain additional information that the user may use to diagnose failures. Bazel Buildbarn will emit messages containing links to its web frontend that can be used to inspect actions (command, input root, etc.) in more detail.

EdSchouten · 2018-12-23T16:37:31Z

Ping @EdSchouten :-)

Pong! Sorry for letting you folks wait.

…uild#6591 Bazel Buildbarn recently gained a web frontend called bbb_browser that can be used to explore the CAS/AC. It allows users to get better insight in how remote execution works under the hood. It also makes it possible for users to more easily share information on build failures with their peers. In order to make it easy for people to access this service, we'd like build failures to automatically print links to the corresponding page in bbb_browser to the terminal. We want to use the remote execution APIs message field for that.

…uild#6591 Bazel Buildbarn recently gained a web frontend called bbb_browser that can be used to explore the CAS/AC. It allows users to get better insight in how remote execution works under the hood. It also makes it possible for users to more easily share information on build failures with their peers. In order to make it easy for people to access this service, we'd like build failures to automatically print links to the corresponding page in bbb_browser to the terminal. We want to use the remote execution APIs message field for that. Closes bazelbuild#7234. PiperOrigin-RevId: 230695190

EdSchouten requested review from buchgr, ola-rozenfeld and philwo as code owners November 4, 2018 09:34

googlebot added the cla: yes label Nov 4, 2018

EdSchouten mentioned this pull request Nov 5, 2018

Add an additional_info_url fields to ExecuteResponse. bazelbuild/remote-apis#36

Merged

EdSchouten closed this Nov 5, 2018

buchgr reviewed Nov 5, 2018

View reviewed changes

EdSchouten reopened this Nov 5, 2018

EdSchouten force-pushed the print-server-log-on-failure branch 2 times, most recently from fd0698e to 915fdfd Compare November 5, 2018 20:03

EdSchouten force-pushed the print-server-log-on-failure branch 2 times, most recently from b9bdc82 to 249aace Compare November 23, 2018 13:58

buchgr reviewed Nov 23, 2018

View reviewed changes

jin assigned buchgr Dec 10, 2018

jin added the team-Execution label Dec 10, 2018

Ed Schouten added 2 commits December 23, 2018 15:23

Upgrade to the latest Remote Execution protocol definition.

7f58bba

This includes the new ExecuteResponse's message field that is dedicated to containing log entries that may be printed to the user.

EdSchouten force-pushed the print-server-log-on-failure branch from 249aace to b5afa24 Compare December 23, 2018 14:24

jin added team-Remote-Exec Issues and PRs for the Execution (Remote) team and removed team-Execution labels Jan 14, 2019

buchgr mentioned this pull request Jan 24, 2019

remote: print server provided message on failed action. Closes #6591 #7234

Closed

bazel-io closed this in e213269 Jan 24, 2019

Conversation

EdSchouten commented Nov 4, 2018

Uh oh!

EdSchouten commented Nov 4, 2018

Uh oh!

buchgr commented Nov 5, 2018

Uh oh!

EdSchouten commented Nov 5, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

buchgr commented Nov 5, 2018

Uh oh!

EdSchouten commented Nov 5, 2018

Uh oh!

buchgr commented Nov 5, 2018

Uh oh!

EdSchouten commented Nov 5, 2018

Uh oh!

buchgr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

buchgr commented Nov 5, 2018

Uh oh!

EdSchouten commented Nov 23, 2018

Uh oh!

buchgr commented Nov 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

buchgr Nov 23, 2018

Choose a reason for hiding this comment

Uh oh!

EdSchouten Dec 23, 2018

Choose a reason for hiding this comment

Uh oh!

buchgr commented Dec 13, 2018

Uh oh!

EdSchouten commented Dec 23, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

EdSchouten commented Nov 5, 2018 •

edited

Loading

buchgr commented Nov 23, 2018 •

edited

Loading