BES export support#19
Draft
9999years wants to merge 56 commits into
Draft
Conversation
I have a highly trivial Manifold implementation internally at Mercury (https://github.com/mercurytechnologies/locally-euclidean) for which I would like to set up for buck2 to receive rage and all that at. This allows for fully implementing the build logs feature mentioned in facebook#441. There's still a buck2_core::facebook_only() or two that need inspected/removed and some more tidying.
> Prefer using the short name when referring to an eponymous target (`//x` instead of `//x:x`). If > you are in the same package, prefer the local reference (`:x` instead of `//x`). See: https://bazel.build/build/style-guide#target-naming This syntax is supported on the command-line, but not in BUCK files. Unfortunately, the only(?) Starlark formatter (buildifier: https://github.com/bazelbuild/buildtools) automatically abbreviates targets on the assumption that this syntax is valid, which causes errors when using Buck2: ... 4: Error coercing "//src/Foo" 5: Invalid absolute target pattern `//src/Foo` is not allowed 6: Expected a `:`, a trailing `/...` or the literal `...`. We can adjust the parsing rules to allow this syntax in more places.
Only show test output if errors.
Can be overridden with:
buck test --test-output=(all|none|error) ...
Avoids re-running passed tests. Only compatible with DICE (local only).
…r action debugging. facebook#1227
…e-commit-due-to-buck-starlark-lint-failure-concurrency make buck external cells more reliable facebook#1259
Avoids needing to rebase the entire patch of facebook#1100, since we don't care about supporting cell segmentation at Mercury anyway.
Note that this has been merged upstream and can be removed the next time we update buck2. See: facebook#1247
Note that this has been merged upstream and can be removed the next time we update buck2. See: facebook@8c2fdd9
facebook#1221 Note: modified during rebase to reconcile with prior patches (pr1076) which partially overlapped with this PR. Also adds execution_concurrency_limit and grpc_timeout config fields which were assumed present by the upstream diff.
experimental patch by joseph which we may try to upstream later if it works
…t-crash dont have soft errors promote to hard errors
I was surprised to learn that my code which the target exprs were used as a list no longer worked after a buck2 upgrade. These are documented as being a list, yet they were not actually parsed as such! https://buck2.build/docs/api/bxl/cli_args/#target_expr ``` def _collect_nix_deps(ctx): print(ctx.cli_args) pass main = bxl.main( impl = _collect_nix_deps, cli_args = { "target": bxl.cli_args.target_expr("Target(s) to build and import"), }, doc = """ Extracts the direct/buck2-transitive Nix dependency list of a buck2 target into a messy json file. """, ) ``` Before: ``` $ cargo r -p buck2 -- bxl test.bxl:main -- --target //:foo --target //:bar Compiling buck2_bxl v0.1.0 (/Users/jade/co/buck2/app/buck2_bxl) Compiling buck2 v0.1.0 (/Users/jade/co/buck2/app/buck2) Finished `dev` profile [optimized + debuginfo] target(s) in 5.84s Running `target/debug/buck2 bxl 'test.bxl:main' -- --target '//:foo' --target '//:bar'` buck2 daemon constraint mismatch: Version mismatch; killing daemon... Starting new buck2 daemon... Connected to new buck2 daemon. error: the argument '--target <target>' cannot be used multiple times ``` After: ``` Compiling buck2_bxl v0.1.0 (/Users/jade/co/buck2/app/buck2_bxl) Compiling buck2 v0.1.0 (/Users/jade/co/buck2/app/buck2) Finished `dev` profile [optimized + debuginfo] target(s) in 9.08s Running `target/debug/buck2 bxl 'test.bxl:main' -- --target '//:buck2' --target '//:buck2_bundle'` buck2 daemon constraint mismatch: Version mismatch; killing daemon... Starting new buck2 daemon... Connected to new buck2 daemon. struct(target=[gh_facebook_buck2//:buck2, gh_facebook_buck2//:buck2_bundle]) ``` I have no way to run the tests (facebook#1027), so you will have to forgive my attempt to add a test case here; try getting Claude to fix the bits I missed. The OSS build is down due to April Fools, so while I've tested this locally with some build fix hacks, I don't think the external CI will say much useful.
fix: bxl target patterns didn't accept multiple args
use BlockingExecuter = BuckBlockingExecutor::default_concurrency(fs.dupe()) for macos
The signature of `compile` changed from Tonic 0.12 to 0.14. The new signature requires the type of `protos` and `includes` to be the same. This prepares for the tonic 0.14 upgrade by passing both `protos` and `includes` as `&[&str]` instead of one being `&[String]`. Upstream: facebook/buck2@1a14d2b
Patch hyper with arianvp's fork and tonic/tonic-prost/tonic-prost-build with edef1c's fork until these changes make it upstream.
) * Add debugging output for RE actions * Print method for RPC retries * Track and retry message received from stream During stream reading, there was no retry mechanism in place, so seeing many `ResourceExhausted` errors would lead h2 issuing a GOWAWY to the server, effectively closing the connection. Now, stream responses are also retried by wrapping both, stream establishment and stream reading in a retry loop. * Do not set a timeout on the gRPC message itself This is otherwise enforced by the server which cancels the action, which wo don't want. * Do not enforce a timeout for Execute requests
Summary: `DefaultConfigParserFileOps::read_dir` uses `std::fs::read_dir` which returns entries in platform-dependent, unspecified order. The DICE-based `ReadDirOutput` is already documented as returning sorted entries, but `DefaultConfigParserFileOps` (used during initial daemon startup) did not match this contract. This means when buckconfig directories contain multiple files defining the same config key, which value wins could vary across runs depending on filesystem traversal order. Sort entries by name in `DefaultConfigParserFileOps::read_dir` to match the DICE path's contract and ensure deterministic config resolution. Reviewed By: AishwaryaSivaraman Differential Revision: D101209619 fbshipit-source-id: e47dedb2530640e4cd0e96112b757e28969e591c Co-authored-by: Neil Mitchell <ndmitchell@meta.com>
If you run `buck2` in a `pre-commit` hook, then the inherited `GIT_DIR` and similar can cause `buck2` to run destructive commands like `git reset --hard` in the wrong repository!
…wyzskk Allow source labels to omit eponymous target names
* Merge pull request #13 from MercuryTechnologies/hdgarrood/push-rlpkyxwyzskk Allow source labels to omit eponymous target names * buck2_server: Implement Git revision data What it says on the tin, closes a long-standing TODO and helps with our telemetry. TODO: Should we just reuse the `hg_revision` field for Git revisions? They _are_ also 40-character hashes... Upstream: - facebook#1346 --------- Co-authored-by: Harry Garrood <harry@garrood.me>
Unnecessary given validation in TransitiveSet:new. Already upstreamed.
The OpenTelemetry exporter we're using was added with `default-features = false`, which adds `reqwest` without a TLS stack, which ends up breaking OpenTelemetry export in our current CI setup. Follow-up to #14 `cargo tree` confirms that we're using the system store: ``` $ cargo tree --edges=normal --invert=rustls-platform-verifier --package=buck2_core rustls-platform-verifier v0.7.0 └── reqwest v0.13.4 ├── opentelemetry-http v0.32.0 │ └── opentelemetry-otlp v0.32.0 │ └── buck2_core v0.1.0 (/Users/wiggles/buck2-2/app/buck2_core) └── opentelemetry-otlp v0.32.0 (*) ``` See: - https://github.com/rustls/rustls-platform-verifier - "A certificate verification library for rustls that uses the operating system's verifier" <details><summary><code>Cargo.lock</code> diff</summary> ```diff --- /Users/wiggles/mwb/nix/packages/buck2-source/Cargo.lock 2026-07-06 10:44:58.618882289 -0700 +++ Cargo.lock 2026-07-06 11:50:41.817123538 -0700 @@ -3270,6 +3270,17 @@ checksum = "613afe47fcd5fac7ccf1db93babcb082c5994d996f20b8b159f2ad1658eb5724" [[package]] +name = "chacha20" +version = "0.10.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d524456ba66e72eb8b115ff89e01e497f8e6d11d78b70b1aa13c0fbd97540a81" +dependencies = [ + "cfg-if", + "cpufeatures 0.3.0", + "rand_core 0.10.1", +] + +[[package]] name = "chrono" version = "0.4.44" source = "registry+https://github.com/rust-lang/crates.io-index" @@ -3376,6 +3387,16 @@ checksum = "1d07550c9036bf2ae0c684c4297d503f838287c83c53686d05370d0e139ae570" [[package]] +name = "combine" +version = "4.6.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ba5a308b75df32fe02788e748662718f03fde005016435c444eea572398219fd" +dependencies = [ + "bytes", + "memchr", +] + +[[package]] name = "common-path" version = "1.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" @@ -4642,8 +4663,10 @@ checksum = "ff2abc00be7fca6ebc474524697ae276ad847ad0a6b3faa4bcb027e9a4614ad0" dependencies = [ "cfg-if", + "js-sys", "libc", "wasi", + "wasm-bindgen", ] [[package]] @@ -4665,11 +4688,13 @@ checksum = "0de51e6874e94e7bf76d726fc5d13ba782deca734ff60d5bb2fb2607c7406555" dependencies = [ "cfg-if", + "js-sys", "libc", "r-efi 6.0.0", "rand_core 0.10.1", "wasip2", "wasip3", + "wasm-bindgen", ] [[package]] @@ -5379,6 +5404,55 @@ ] [[package]] +name = "jni" +version = "0.22.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5efd9a482cf3a427f00d6b35f14332adc7902ce91efb778580e180ff90fa3498" +dependencies = [ + "cfg-if", + "combine", + "jni-macros", + "jni-sys", + "log", + "simd_cesu8", + "thiserror 2.0.18", + "walkdir", + "windows-link 0.2.1", +] + +[[package]] +name = "jni-macros" +version = "0.22.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a00109accc170f0bdb141fed3e393c565b6f5e072365c3bd58f5b062591560a3" +dependencies = [ + "proc-macro2", + "quote", + "rustc_version", + "simd_cesu8", + "syn 2.0.117", +] + +[[package]] +name = "jni-sys" +version = "0.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c6377a88cb3910bee9b0fa88d4f42e1d2da8e79915598f65fb0c7ee14c878af2" +dependencies = [ + "jni-sys-macros", +] + +[[package]] +name = "jni-sys-macros" +version = "0.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "38c0b942f458fe50cdac086d2f946512305e5631e720728f2a61aabcd47a6264" +dependencies = [ + "quote", + "syn 2.0.117", +] + +[[package]] name = "jobserver" version = "0.1.34" source = "registry+https://github.com/rust-lang/crates.io-index" @@ -5677,6 +5751,12 @@ ] [[package]] +name = "lru-slab" +version = "0.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "112b39cec0b298b6c1999fee3e31427f74f676e4cb9879ed1a121b43661a4154" + +[[package]] name = "lsp-server" version = "0.7.9" source = "registry+https://github.com/rust-lang/crates.io-index" @@ -6935,6 +7015,63 @@ ] [[package]] +name = "quinn" +version = "0.11.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0c1a41e437b6bbd489372cd4971de128e85c855f56c57f283d20ff016cf7c0a8" +dependencies = [ + "bytes", + "cfg_aliases 0.2.1", + "pin-project-lite", + "quinn-proto", + "quinn-udp", + "rustc-hash", + "rustls", + "socket2", + "thiserror 2.0.18", + "tokio", + "tracing", + "web-time", +] + +[[package]] +name = "quinn-proto" +version = "0.11.16" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2f4bfc015262b9df63c8845072ce59068853ff5872180c2ce2f13038b970e560" +dependencies = [ + "aws-lc-rs", + "bytes", + "getrandom 0.4.2", + "lru-slab", + "rand 0.10.1", + "rand_pcg", + "ring", + "rustc-hash", + "rustls", + "rustls-pki-types", + "slab", + "thiserror 2.0.18", + "tinyvec", + "tracing", + "web-time", +] + +[[package]] +name = "quinn-udp" +version = "0.5.15" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "35a133f956daabe89a61a685c2649f13d82d5aa4bd5d12d1277e1072a21c0694" +dependencies = [ + "cfg_aliases 0.2.1", + "libc", + "once_cell", + "socket2", + "tracing", + "windows-sys 0.52.0", +] + +[[package]] name = "quote" version = "1.0.45" source = "registry+https://github.com/rust-lang/crates.io-index" @@ -6992,6 +7129,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d2e8e8bcc7961af1fdac401278c6a831614941f6164ee3bf4ce61b7edb162207" dependencies = [ + "chacha20", "getrandom 0.4.2", "rand_core 0.10.1", ] @@ -7061,6 +7199,15 @@ ] [[package]] +name = "rand_pcg" +version = "0.10.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "caa0f4137e1c0a72f4c651489402276c8e8e1cf081f3b0ba156d2cbeef09e86a" +dependencies = [ + "rand_core 0.10.1", +] + +[[package]] name = "rand_xorshift" version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" @@ -7280,13 +7427,19 @@ "http-body", "http-body-util", "hyper", + "hyper-rustls", "hyper-util", "js-sys", "log", "percent-encoding", "pin-project-lite", + "quinn", + "rustls", + "rustls-pki-types", + "rustls-platform-verifier", "sync_wrapper 1.0.2", "tokio", + "tokio-rustls", "tower 0.5.3", "tower-http", "tower-service", @@ -7475,10 +7628,38 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "be040f8b0a225e40375822a563fa9524378b9d63112f53e19ffff34df5d33fdd" dependencies = [ + "web-time", "zeroize", ] [[package]] +name = "rustls-platform-verifier" +version = "0.7.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "26d1e2536ce4f35f4846aa13bff16bd0ff40157cdb14cc056c7b14ba41233ba0" +dependencies = [ + "core-foundation 0.10.1", + "core-foundation-sys", + "jni", + "log", + "once_cell", + "rustls", + "rustls-native-certs 0.8.3", + "rustls-platform-verifier-android", + "rustls-webpki", + "security-framework 3.7.0", + "security-framework-sys", + "webpki-root-certs", + "windows-sys 0.52.0", +] + +[[package]] +name = "rustls-platform-verifier-android" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f87165f0995f63a9fbeea62b64d10b4d9d8e78ec6d7d51fb2125fda7bb36788f" + +[[package]] name = "rustls-webpki" version = "0.103.12" source = "registry+https://github.com/rust-lang/crates.io-index" @@ -7834,6 +8015,22 @@ checksum = "703d5c7ef118737c72f1af64ad2f6f8c5e1921f818cdcb97b8fe6fc69bf66214" [[package]] +name = "simd_cesu8" +version = "1.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "94f90157bb87cddf702797c5dadfa0be7d266cdf49e22da2fcaa32eff75b2c33" +dependencies = [ + "rustc_version", + "simdutf8", +] + +[[package]] +name = "simdutf8" +version = "0.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e3a9fe34e3e7a50316060351f37187a3f546bce95496156754b601a5fa71b76e" + +[[package]] name = "siphasher" version = "0.3.11" source = "registry+https://github.com/rust-lang/crates.io-index" @@ -8551,6 +8748,21 @@ ] [[package]] +name = "tinyvec" +version = "1.11.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3e61e67053d25a4e82c844e8424039d9745781b3fc4f32b8d55ed50f5f667ef3" +dependencies = [ + "tinyvec_macros", +] + +[[package]] +name = "tinyvec_macros" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1f3ccbac311fea05f86f61904b462b55fb3df8837a366dfc601a0161d0532f20" + +[[package]] name = "tokio" version = "1.52.0" source = "registry+https://github.com/rust-lang/crates.io-index" @@ -9207,6 +9419,25 @@ ] [[package]] +name = "web-time" +version = "1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5a6580f308b1fad9207618087a65c04e7a10bc77e02c8e84e9b00dd4b12fa0bb" +dependencies = [ + "js-sys", + "wasm-bindgen", +] + +[[package]] +name = "webpki-root-certs" +version = "1.0.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0d46a5a140e6f7afeccd8eae97eff335163939eac8b929834875168b29b3d267" +dependencies = [ + "rustls-pki-types", +] + +[[package]] name = "webpki-roots" version = "1.0.6" source = "registry+https://github.com/rust-lang/crates.io-index" ``` </details>
This adds support for more OpenTelemetry environment variables: - `$OTEL_EXPORTER_OTLP_*` - `$OTEL_SDK_DISABLED` - `$OTEL_SERVICE_NAME` - `$OTEL_RESOURCE_ATTRIBUTES` - `$TRACEPARENT` - `TRACESTATE`
Fill the previously-stubbed test metadata fields that were left as empty defaults during the BES squash: TestSuite / TestDiscoveryStart / TestDiscoveryEnd `labels` and TestRunEnd `timeout`. Adds test_labels() (adapted to mercury's external-only FrozenExternalRunnerTestInfo, collapsing sluongng's OwnedTestInfo enum) and test_timeout_proto() helpers, threads test_labels through execute_request, and populates timeout from request.timeout(). Reconciles the dropped portion of sluongng's: 828089e [bep parity 6/10]: report test metrics Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Wire up the emitter that was dropped during the BES squash: emit ReLogStreamAvailable once per action when RE stdout/stderr stream names first become available. The BEP converter arm and its unit tests already landed; only the emitter was missing. Populates the mercury-specific `key: ActionKey` field -- sourced from the previously-unused proto_action_key param -- which is what the converter actually keys off of (the string action_key / use_case fields are ignored by the consumer). Reconciles the dropped portion of sluongng's: 0685af3 Emit ReLogStreamAvailable event for RE log stream handles Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Populate the previously-stubbed CreateOutputSymlinksEnd.symlinks field that was left empty during the BES squash. create_unhashed_outputs now returns a CreateUnhashedOutputsResult carrying each created link (path + target) instead of a bare count, and the emitter maps those into buck2_data::OutputSymlink. The BEP converter arm (push_convenience_symlinks_identified) already landed, so this closes the loop and convenience symlinks now appear in BEP. Adapted to mercury's std HashSet (sluongng used StdBuckHashSet). Reconciles the dropped portion of sluongng's: 9819e8b [bep parity 7/10]: emit external events Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Emit the Bazel-compatible "Streaming build results to:" line at command start and end in both simpleconsole and superconsole, so BuildBuddy workflows can discover Buck2 child invocations by scanning command output. The line is one-shot per lifecycle event via per-subscriber latches. The eager-send infrastructure (session_info::streaming_results_line, the bes_client wait_for_acks priority lane, and scribe's send-before-client path) already landed during the squash; this wires up the console output that was reset to mercury-head. Adaptations from sluongng: mercury's BuckRootComponent has no games_overlay field, and we use .buck_error_context() instead of internal_error! (no new import). mercury's SimpleConsole has no ConsoleOutputLimit field. Reconciles the dropped portion of sluongng's: 732a681 bes: publish invocation links eagerly Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
download_impl gained a leading `opts: &RERuntimeOpts` parameter, but the top-level test_download_compressed (which sits outside `mod tests`, so it can't see the test_re_runtime_opts helper) was never updated and failed to compile under `--tests`. Construct RERuntimeOpts inline to restore a green test build. Pre-existing breakage in mercury-head, fixed as prep for reconciling the dropped RE-profiling work into this file. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
First slice of the dropped RE-profiling commit: plumb REAPI ExecutedActionMetadata server-side phase timestamps (queue / worker / input-fetch / execution / output-upload) from executed ActionResults into CommandExecutionMetadata, so they flow into the event log. - data.proto: add RemoteExecutionTiming message + CommandExecutionMetadata .remote_execution_timing (field 12), and RemoteRequestStart/End span events (field 91 in both span oneofs) for the emitter that follows. - build.rs: serde annotations for the timestamp fields. - result.rs: RemoteExecutionTimestamp / RemoteExecutionTiming Rust types + to_proto, threaded through CommandExecutionMetadata constructors. - remote_action_result.rs: convert TExecutedActionMetadata timestamps into RemoteExecutionTiming (None when all zero; None for action-cache hits). - local.rs: local executor reports no remote timing. NOTE: the RemoteRequest span emitter (re_grpc), the BEP converter, and scribe/display support are follow-up slices. This commit is inert-but- correct: timing is captured and logged, nothing consumes it yet. This commit's ActionDigestTrace has nothing to do with it -- that dead proto message is unrelated G5 telemetry; the dropped commit is RemoteRequest / RemoteExecutionTiming based. Reconciles part of sluongng's: 7aa4930 [bes buildbuddy]: profile remote execution Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Teach the Bazel/BES command-profile builder to surface remote execution work in the uploaded profile: - bazel_converter.rs: convert RemoteRequestStart/End spans into profile spans, synthesize RE server-phase child spans (queue / input-fetch / execution / output-upload) from CommandExecutionMetadata.remote_execution _timing -- parented to the action lane and clamped to the action interval so client/worker clock skew can't break the trace layout. Adds executor-stage / RE-stage / RE-queue / RE-platform profile args. - scribe.rs: keep RemoteRequest / ReUpload / ExecutorStage / Materialization / cache-upload span pairs in the BuildBuddy (preserve_bazel_logs) stream so the profile can be reconstructed. Synthetic profile span ids use a high base (1<<63) to avoid colliding with real span ids. NOTE: RemoteRequest spans require the re_grpc emitter (follow-up slice) to be produced; the RemoteExecutionTiming phase-span synthesis works off the already-landed foundation. Two pre-existing invocation_record_* test failures in this file predate this work (verified on the parent). Reconciles part of sluongng's: 7aa4930 [bes buildbuddy]: profile remote execution Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Final slice of the dropped RE-profiling commit: wrap each REAPI request the
OSS re_grpc client makes in a RemoteRequestStart/End span, so the BEP
command profile (converter, prior commit) can show CAS / ActionCache /
ByteStream work under each action.
Adds buck2_data + buck2_events deps to re_grpc (Cargo.toml + BUCK) so the
client can call span_async -- a deliberate layering addition matching
upstream; cargo confirms it introduces no dependency cycle.
Helpers (remote_request_start/end/span, grpc digest/stat helpers) plus
per-request spans on:
- ActionCache: GetActionResult, UpdateActionResult
- CAS: FindMissingBlobs, BatchUpdateBlobs, BatchReadBlobs
- ByteStream: Write, Read
IMPORTANT -- this is a native re-implementation, NOT a port. Mercury's
re_grpc client has diverged from sluongng's base: it uses retry(...) +
with_re_metadata(...) rather than retry_grpc_request_with_client_reconnect
+ with_re_metadata_timeout, so the upstream hunks did not apply and each
call site was wrapped by hand against mercury's structure.
Scope notes:
- Execute (streaming) is intentionally not wrapped: server-side execution
timing is already surfaced by the RemoteExecutionTiming phase-span
synthesis (foundation commit); a span around stream creation would not
map to execution duration.
- QueryWriteStatus has no mercury equivalent, so no span there.
Reconciles the remaining part of sluongng's:
7aa4930 [bes buildbuddy]: profile remote execution
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The Bazel-profile span whitelist (preserve_bazel_logs) sent build/action spans but not TestStart / TestDiscovery starts, so for `buck2 test` the entire test-execution phase was blank below the command span in BuildBuddy's timeline (and its Timing Breakdown had almost nothing to categorize across that stretch). Add TestDiscovery + TestStart to the span-start whitelist; their ends (TestDiscovery / TestEnd) were already sent unconditionally, and the profile builder already renders them on the Tests lane, so complete test spans now land in command.profile.gz. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A real `buck2 test //tools/...` produced ~177k profile-eligible spans, but MAX_PROFILE_SPANS was 50k -- and push_completed drops in completion order, so the cap filled at t+132s of a 578s run and silently dropped the entire back ~7.5 minutes (including all test spans). BuildBuddy's timeline showed activity only in the first ~2 min and its Timing Breakdown was starved. ExecutorStage is ~59% of those spans (104k here) -- fine-grained per-action sub-stages that both blow the budget and clutter the timeline. Exclude them from the command profile (action-level spans and the synthesized RE-phase spans carry the useful timing) and raise MAX_PROFILE_SPANS to 150k. This build now yields ~73k spans (~2MB gzip, well under the 10MB upload ceiling) covering the full timeline. Remove the guard in record_span_start to re-enable ExecutorStage detail. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
BuildBuddy's Timing Breakdown card is hardcoded to look up specific Bazel
profiler event names (invocation_breakdown_card.tsx): it reads
`buildTargets`, `runAnalysisPhase`, and `evaluateTargetPatterns` out of
the by-name duration map and renders nothing when they're all absent -- so
buck2 profiles, which use their own span names, showed an empty breakdown.
Synthesize those three "build phase marker" spans in the command profile,
on a new "Build phases" lane:
- evaluateTargetPatterns: command start .. first analysis
- runAnalysisPhase: first analysis .. first action
- buildTargets: whole command (BuildBuddy derives execution as
buildTargets - runAnalysisPhase - evaluateTargetPatterns)
buck2's phases interleave via DICE (unlike Bazel's sequential phases), so
these are a documented approximation, but they light up the phase breakdown.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
BuildBuddy's Timing Breakdown "Execution breakdown" reads specific Bazel
category/name strings. Map the buck2 equivalents so the meaningful slices
populate:
- locally-executed actions -> category "local action execution"
("Executing locally") -- the main slice for local builds
- RE execution phase span -> name "execute remotely"
("Executing remotely")
- CAS/ByteStream reads -> category "remote output download"
("Downloading outputs")
These are approximations: buck2's action span covers more than just
execution, and buck2 has no equivalent for several Bazel-specific slices
(sandbox setup/teardown, merkle-tree building, input mapping,
detectModifiedOutputFiles, stable-status), which stay unpopulated. The
flame chart is unaffected -- only categories (not display names) change,
except the RE execution phase which is renamed to Bazel's "execute
remotely".
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
buck2 already computes critical-path data and sends it as the BuildGraphInfo instant event (BuildGraphExecutionInfo.critical_path2). Capture those entries in the command profile builder and emit them as a dedicated "Critical Path" thread -- matching Bazel's profile convention, which BuildBuddy renders as its own row. Each entry is placed at its start_offset_ns from the command start (falling back to sequential placement if offsets are absent), named from the typed entry (action name, load/listing package, analysis, test, waiting, etc.), and annotated with total_duration / potential_improvement. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
8 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This series of patches is cherry-picked from https://github.com/sluongng/buck2/tree/stack, with merge conflicts resolved by Claude with little oversight. I can confirm that the patches work and give us lots of nice info in our BuildBuddy UI, but it's a lot of code, and (IIRC) a fair amount if it is diverged from upstream. Not sure what we'll end up doing with this, but here it is as a demo.