runtime: decode BuildKit aux trace records for per-vertex events#51
Conversation
streamBuildOutput now routes moby.buildkit.trace aux records to a hand-rolled wire-format decoder (protowire) that parses the subset of controlapi.StatusResponse we need: Vertex name/cached/started/completed/ error and VertexLog msg. Emits BuildLayerEvent on per-digest start and complete transitions, BuildLogEvent per RUN log line. Avoids pulling github.com/moby/buildkit as a direct dep (+253 transitive modules: containerd, k8s, sigstore, AWS/Azure SDKs). Net cost is +2 modules (golang/protobuf v1 shim, xerrors) from promoting google.golang.org/protobuf to direct. Integration test builds a 2-step Dockerfile with a per-run-unique RUN marker and asserts >=2 BuildLayerEvents + a BuildLogEvent containing the marker, so a future regression in the aux routing goes loud rather than silent. Closes part of #50 (limitation 1). Limitations 2 (extractBaseImages parser blind spots) and 3 (tarDirectory / .dockerignore) remain open. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a BuildKit trace decoder to parse ChangesBuildKit Trace Event Decoding
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@runtime/docker/buildkit_trace.go`:
- Around line 103-108: The file has formatting drift causing lint failures; run
gofmt (or gofmt -w) on the file containing the buildkitTraceDecoder.decodeVertex
function to normalize indentation and spacing (e.g., align the var block and
surrounding code) so the decodedVertex method matches gofmt style; commit the
reformatted file.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e74b1b25-6154-4c88-8f52-9502bc1cb6f0
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (5)
go.modruntime/docker/build.goruntime/docker/buildkit_trace.goruntime/docker/buildkit_trace_test.gotest/integration/buildkit_trace_events_test.go
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
streamBuildOutputnow routesmoby.buildkit.traceaux records to a hand-rolled wire-format decoder (protowire) that parses the subset ofcontrolapi.StatusResponsewe need:Vertex.{name,cached,started,completed,error}andVertexLog.msg.BuildLayerEventon per-digest start/complete transitions andBuildLogEventper RUN log line. Closes consumers'BuildStart → silence → BuildCompletedgap under BuildKit.github.com/moby/buildkitas a direct dep (+253 transitive modules: containerd, k8s, sigstore, AWS/Azure SDKs). Net cost: +2 modules (golang/protobufv1 shim,xerrors) from promotinggoogle.golang.org/protobufto direct.Closes part of #50 (limitation 1, per-vertex events). Limitations 2 (
extractBaseImagesparser blind spots) and 3 (tarDirectory/.dockerignore) remain open.Test plan
go test -race ./...— passesgo vet ./...— cleanmake test-integration(20 tests) — all pass, 230sruntime/docker/buildkit_trace_test.gocover: start/done/cached/error vertex transitions, log-line splitting, malformed input (silently ignored), unknown-field forward-compatTestBuildKit_PerVertexEventsEmittedasserts ≥2BuildLayerEvent+ aBuildLogEventcontaining a per-run unique RUN marker (cache-busts so the RUN vertex actually executes)"got 0 (aux decoder dead?)", confirming the assertion catches a dead decoder🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests