Conditionally download stdout and stderr of remotely executed actions

[Silic0nS0ldier]

Description

Adds a new flag --remote_download_stdouterr (stdouterr is to match --experimental_ui_max_stdouterr_bytes) to control downloading of action stdout and stderr.

To suite the various use cases, multiple options are available.

• failed: This is functionally a none option, as it matches the behavior of other action outputs under all configurations (barring usage of --remote_download_symlink_template) where outputs of failed actions are always downloaded.
  • Fewest possible downloads when combined with --remote_download_outputs=minimal.
  • Implicitly reduces action log noise.
• uncached: Downloads stdout and stderr of all actions that actually ran. Implicitly includes failed actions (since failures are not cached).
  • An arbitrary split similar to --test_summary=short_uncached|detailed_uncached.
  • Number of downloads drops proportional to cache hit rate.
  • Implicitly reduces action log noise. May be useful for local development to put the focus on affected actions.
• all: The default and current behavior. All actions have their stdout and stderr downloaded.
  • Beyond being the default, keeping support for this is useful for integrations that require logs to be downloaded (none that I know of, but as the default I'm sure something is reliant on this).

A toplevel option was considered, but is more complicated to implement. Best implemented in a separate PR, if there is demand.

This does not affect test outputs (test.xml and test.log).

Motivation

Ideally actions are silent, in practice however many log warnings (very common for third party and even first-party C and C++).

In a CI setting this output is seldom viewed (mostly being regarded as log noise, which can be suppressed with UI filter flags), which makes the networking overhead hard to justify. This overhead can amount to significant load as the number of concurrent invocations increases (especially when combined with bazel test //...).

Closes #19782

Note that a different take on this which modifies the behavior based on --remote_download_outputs has also been raised. #23223 This PR borrows the unit test implementation.

Build API Changes

New command line flag --remote_download_stdouterr with options;

• failed
• uncached
• all

The flag itself has not been discussed, although the behavior (not downloading stdout and stderr of actions) has been discussed at #19782 and #23223.

Since the flag is not usable within builds (as part of a transition) it is backward compatible, although using the flag with older Bazel versions may result in an error (unrecognized flag).

Checklist

• I have added tests for the new use cases (if any).
• I have updated the documentation (if applicable).

Release Notes

RELNOTES[NEW]: Downloading of stdout and stderr of remotely executed actions can now be disabled with --remote_download_stdouterr. Available options are all (default), uncached and failed.

[gemini-code-assist]

Code Review

This pull request introduces a new flag --remote_download_stdouterr to conditionally download stdout and stderr for remotely executed actions, which can reduce network overhead. The changes include the new flag definition, the logic to control downloads based on the flag's value, and shell tests to verify the new behavior. The implementation is correct and the new feature is well-tested. I have no critical feedback on these changes.

[fmeum]

There are actions that always require stdout/stderr, most importantly C++ actions using MSVC's /showIncludes feature. Test actions also act on the local output. Have you tested those with the flag?

[Silic0nS0ldier]

/showIncludes does rely on stdout/stderr, that is just where the outputs are sent. In that respect, it is no different to another other action which writes to standard output streams (via a flag or otherwise).

Confusing? Potentially. But this (not downloading logs) is an opt-in behaviour.

While working on this I did attempt implement printing of the BES action result URL (for RE services where logs are accessible) so there can be a fallback. Attempted and failed, so I opted to exclude it from this PR, with the hopes of implementing it later.

[Silic0nS0ldier]

As for test actions, I'll do some testing.

[Silic0nS0ldier]

I've done some digging. test.log (and it's permutations for sharding and retries) is the standard output destination, so in it's current state this does break them (and means the default test.xml is generated with an empty test.log).

It also cannot be made a regular test output, as the streaming output mode for tests relies on this.

I don't think this is impossible to solve, but it is likely to require changing the test machinery (much more effort, greater risk). What I'll do is scope this to just "normal" actions (i.e. non-test actions) and work on test action support in a follow up.

[Silic0nS0ldier]

I stumbled across --remote_print_execution_messages. Noting here so I can follow up if and how the scope overlaps with this change.