Report remote execution messages as events #18757
Conversation
Currently the message is appended to a spawn's stderr, which pollutes that output. It also means that whether a message is output is gated by the `--output_filter` flag, which is surprising - metadata messages don't feel like "output" in the same way `--output_filter` applies to. Instead, emit the message as a top-level message to the terminal.
github-actions
bot
added
awaiting-review
|
cc @exoson who originally implemented this feature. |
coeuvre
added
awaiting-PR-merge
github-actions
bot
removed
the
awaiting-PR-merge
|
@bazel-io flag |
bazel-io
added
the
potential release blocker
|
I think I'll need these in the build event stream such that I can easily get the messages for a specific build/test target. With this change, the messages will only be available somewhere within the full log in the build events, right? Otherwise this change seems to be a really good one. |
If I am not mistaken, it's still in the progress message. |
|
Yea, that's my understanding too. My worry is that it doesn't seem to be trivial to map the re messages from the progress messages to individual build/test targets. |
|
@bazel-io fork 6.3.0 |
bazel-io
removed
the
potential release blocker
Currently the message is appended to a spawn's stderr, which pollutes that output. It also means that whether a message is output is gated by the `--output_filter` flag, which is surprising - metadata messages don't feel like "output" in the same way `--output_filter` applies to. Instead, emit the message as a top-level message to the terminal. Closes bazelbuild#18757. PiperOrigin-RevId: 543365674 Change-Id: I9874c8a0946a3156a2c17a2962880184d3aeebe0
A workaround is to let server embed the label/action id/whatever in the message. Or Bazel could prepend the label to the message. |
Currently the message is appended to a spawn's stderr, which pollutes that output. It also means that whether a message is output is gated by the `--output_filter` flag, which is surprising - metadata messages don't feel like "output" in the same way `--output_filter` applies to. Instead, emit the message as a top-level message to the terminal. Closes #18757. PiperOrigin-RevId: 543365674 Change-Id: I9874c8a0946a3156a2c17a2962880184d3aeebe0 Co-authored-by: Daniel Wagner-Hall <dwagnerhall@apple.com>
|
Is anything being done to address @exoson 's request? It is unpleasant that this data is no longer part of the Build Event Streams in a somewhat structured way. Out of curiosity, why did this get merged into 6.3.0, even after concerns were raised? Wouldn't it have been better if consensus was reached first? |
Nope. I am not sure what is missing for the server to display these messages in a structured way given that it already has the target label.
I thought not including the message in spawn's stderr is more correct because technically they are not spawn's output. I didn't reject the cherry-pick into 6.3 because the change looked innocent to me. I still believe the concern can be addressed on server side. I am sorry if this change causes trouble for you. If you think it's better to address that from Bazel side, please fire an issue and we can discuss there. |
Currently the message is appended to a spawn's stderr, which pollutes that output. It also means that whether a message is output is gated by the
--output_filterflag, which is surprising - metadata messages don't feel like "output" in the same way--output_filterapplies to.Instead, emit the message as a top-level message to the terminal.