bazel: restore cmd/serverless-init/cloudservice to the build - #53337
Conversation
💡 Codex ReviewOn slow or heavily scheduled CI workers this fixed sleep can expire before the goroutine inside ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
🎯 Code Coverage (details) 🔗 Commit SHA: 810ffe1 | Docs | Datadog PR Page | Give us feedback! |
Files inventory check summaryFile checks results against ancestor 1f54e07e: Results for datadog-agent_7.83.0~devel.git.332.810ffe1.pipeline.126317511-1_amd64.deb:No change detected |
Static quality checks✅ Please find below the results from static quality gates 33 successful checks with minimal change (< 2 KiB)
|
Regression DetectorRegression Detector ResultsMetrics dashboard Baseline: 6537db0 ❌ Experiments with retried target crashesThis is a critical error. One or more replicates failed with a non-zero exit code. These replicates may have been retried. See Replicate Execution Details for more information.
Optimization Goals: ✅ No significant changes detected
|
| perf | experiment | goal | Δ mean % | Δ mean % CI | trials | links |
|---|---|---|---|---|---|---|
| ➖ | quality_gate_logs | % cpu utilization | +1.13 | [+0.15, +2.11] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_security_mean_fs_load | memory utilization | +0.70 | [+0.66, +0.74] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_metrics_logs | memory utilization | +0.11 | [-0.15, +0.36] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_idle | memory utilization | +0.07 | [+0.02, +0.12] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_idle_all_features | memory utilization | -0.17 | [-0.22, -0.12] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_security_idle | memory utilization | -0.18 | [-0.24, -0.12] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_security_no_fs_load | memory utilization | -0.30 | [-0.39, -0.20] | 1 | Logs bounds checks dashboard |
Bounds Checks: ✅ Passed
| perf | experiment | bounds_check_name | replicates_passed | observed_value | links |
|---|---|---|---|---|---|
| ✅ | quality_gate_idle | intake_connections | 10/10 | 3 ≤ 4 | bounds checks dashboard |
| ✅ | quality_gate_idle | memory_usage | 10/10 | 146.28MiB ≤ 154MiB | bounds checks dashboard |
| ✅ | quality_gate_idle | total_bytes_received | 10/10 | 734.55KiB ≤ 819.20KiB | bounds checks dashboard |
| ✅ | quality_gate_idle_all_features | intake_connections | 10/10 | 3 ≤ 4 | bounds checks dashboard |
| ✅ | quality_gate_idle_all_features | memory_usage | 10/10 | 490.38MiB ≤ 495MiB | bounds checks dashboard |
| ✅ | quality_gate_idle_all_features | total_bytes_received | 10/10 | 1.12MiB ≤ 1.25MiB | bounds checks dashboard |
| ✅ | quality_gate_logs | intake_connections | 10/10 | 4 ≤ 6 | bounds checks dashboard |
| ✅ | quality_gate_logs | memory_usage | 10/10 | 184.38MiB ≤ 195MiB | bounds checks dashboard |
| ✅ | quality_gate_logs | missed_bytes | 10/10 | 0B = 0B | bounds checks dashboard |
| ✅ | quality_gate_logs | total_bytes_received | 10/10 | 263.75MiB ≤ 292MiB | bounds checks dashboard |
| ✅ | quality_gate_metrics_logs | cpu_usage | 10/10 | 365.40 ≤ 2000 | bounds checks dashboard |
| ✅ | quality_gate_metrics_logs | intake_connections | 10/10 | 3 ≤ 6 | bounds checks dashboard |
| ✅ | quality_gate_metrics_logs | memory_usage | 10/10 | 407.93MiB ≤ 430MiB | bounds checks dashboard |
| ✅ | quality_gate_metrics_logs | missed_bytes | 10/10 | 0B = 0B | bounds checks dashboard |
| ✅ | quality_gate_metrics_logs | total_bytes_received | 10/10 | 0.93GiB ≤ 1.04GiB | bounds checks dashboard |
| ✅ | quality_gate_security_idle | cpu_usage | 10/10 | 29.84 ≤ 40 | bounds checks dashboard |
| ✅ | quality_gate_security_idle | memory_usage | 10/10 | 297.32MiB ≤ 330MiB | bounds checks dashboard |
| ✅ | quality_gate_security_mean_fs_load | cpu_usage | 10/10 | 62.38 ≤ 80 | bounds checks dashboard |
| ✅ | quality_gate_security_mean_fs_load | memory_usage | 10/10 | 278.50MiB ≤ 310MiB | bounds checks dashboard |
| ✅ | quality_gate_security_no_fs_load | cpu_usage | 10/10 | 21.00 ≤ 40 | bounds checks dashboard |
| ✅ | quality_gate_security_no_fs_load | memory_usage | 10/10 | 284.34MiB ≤ 320MiB | bounds checks dashboard |
Explanation
Confidence level: 90.00%
Effect size tolerance: |Δ mean %| ≥ 5.00%
Performance changes are noted in the perf column of each table:
- ✅ = significantly better comparison variant performance
- ❌ = significantly worse comparison variant performance
- ➖ = no significant change in performance
A regression test is an A/B test of target performance in a repeatable rig, where "performance" is measured as "comparison variant minus baseline variant" for an optimization goal (e.g., ingress throughput). Due to intrinsic variability in measuring that goal, we can only estimate its mean value for each experiment; we report uncertainty in that value as a 90.00% confidence interval denoted "Δ mean % CI".
For each experiment, we decide whether a change in performance is a "regression" -- a change worth investigating further -- if all of the following criteria are true:
-
Its estimated |Δ mean %| ≥ 5.00%, indicating the change is big enough to merit a closer look.
-
Its 90.00% confidence interval "Δ mean % CI" does not contain zero, indicating that if our statistical model is accurate, there is at least a 90.00% chance there is a difference in performance between baseline and comparison variants.
-
Its configuration does not mark it "erratic".
Replicate Execution Details
We run multiple replicates for each experiment/variant. However, we allow replicates to be automatically retried if there are any failures, up to 8 times, at which point the replicate is marked dead and we are unable to run analysis for the entire experiment. We call each of these attempts at running replicates a replicate execution. This section lists all replicate executions that failed due to the target crashing or being oom killed.
Note: In the below tables we bucket failures by experiment, variant, and failure type. For each of these buckets we list out the replicate indexes that failed with an annotation signifying how many times said replicate failed with the given failure mode. In the below example the baseline variant of the experiment named experiment_with_failures had two replicates that failed by oom kills. Replicate 0, which failed 8 executions, and replicate 1 which failed 6 executions, all with the same failure mode.
| Experiment | Variant | Replicates | Failure | Logs | Debug Dashboard |
|---|---|---|---|---|---|
| experiment_with_failures | baseline | 0 (x8) 1 (x6) | Oom killed | Debug Dashboard |
The debug dashboard links will take you to a debugging dashboard specifically designed to investigate replicate execution failures.
❌ Retried Normal Replicate Execution Failures (non-profiling)
| Experiment | Variant | Replicates | Failure | Debug Dashboard |
|---|---|---|---|---|
| quality_gate_idle_all_features | baseline | 2 | Oom killed | Debug Dashboard |
❌ Retried Profiling Replicate Execution Failures (ddprof)
Note: Profiling replicas may still be executing. See the debug dashboard for up to date status.
| Experiment | Variant | Replicates | Failure | Debug Dashboard |
|---|---|---|---|---|
| quality_gate_idle | baseline | 10 | Oom killed | Debug Dashboard |
| quality_gate_idle | comparison | 10 | Oom killed | Debug Dashboard |
| quality_gate_idle_all_features | baseline | 10 | Oom killed | Debug Dashboard |
| quality_gate_idle_all_features | comparison | 10 | Oom killed | Debug Dashboard |
| quality_gate_logs | baseline | 10 | Oom killed | Debug Dashboard |
| quality_gate_logs | comparison | 10 | Oom killed | Debug Dashboard |
| quality_gate_metrics_logs | baseline | 10 | Oom killed | Debug Dashboard |
| quality_gate_metrics_logs | comparison | 10 | Oom killed | Debug Dashboard |
| quality_gate_security_idle | baseline | 10 | Crashed (exit code: 134) | Debug Dashboard |
| quality_gate_security_idle | comparison | 10 | Crashed (exit code: 134) | Debug Dashboard |
| quality_gate_security_mean_fs_load | baseline | 10 | Crashed (exit code: 134) | Debug Dashboard |
| quality_gate_security_no_fs_load | baseline | 10 | Crashed (exit code: 134) | Debug Dashboard |
| quality_gate_security_no_fs_load | comparison | 10 | Crashed (exit code: 134) | Debug Dashboard |
CI Pass/Fail Decision
✅ Passed. All Quality Gates passed.
- quality_gate_idle, bounds check memory_usage: 10/10 replicas passed. Gate passed.
- quality_gate_idle, bounds check intake_connections: 10/10 replicas passed. Gate passed.
- quality_gate_idle, bounds check total_bytes_received: 10/10 replicas passed. Gate passed.
- quality_gate_security_mean_fs_load, bounds check cpu_usage: 10/10 replicas passed. Gate passed.
- quality_gate_security_mean_fs_load, bounds check memory_usage: 10/10 replicas passed. Gate passed.
- quality_gate_metrics_logs, bounds check cpu_usage: 10/10 replicas passed. Gate passed.
- quality_gate_metrics_logs, bounds check total_bytes_received: 10/10 replicas passed. Gate passed.
- quality_gate_metrics_logs, bounds check intake_connections: 10/10 replicas passed. Gate passed.
- quality_gate_metrics_logs, bounds check memory_usage: 10/10 replicas passed. Gate passed.
- quality_gate_metrics_logs, bounds check missed_bytes: 10/10 replicas passed. Gate passed.
- quality_gate_security_no_fs_load, bounds check memory_usage: 10/10 replicas passed. Gate passed.
- quality_gate_security_no_fs_load, bounds check cpu_usage: 10/10 replicas passed. Gate passed.
- quality_gate_idle_all_features, bounds check memory_usage: 10/10 replicas passed. Gate passed.
- quality_gate_idle_all_features, bounds check total_bytes_received: 10/10 replicas passed. Gate passed.
- quality_gate_idle_all_features, bounds check intake_connections: 10/10 replicas passed. Gate passed.
- quality_gate_security_idle, bounds check cpu_usage: 10/10 replicas passed. Gate passed.
- quality_gate_security_idle, bounds check memory_usage: 10/10 replicas passed. Gate passed.
- quality_gate_logs, bounds check missed_bytes: 10/10 replicas passed. Gate passed.
- quality_gate_logs, bounds check intake_connections: 10/10 replicas passed. Gate passed.
- quality_gate_logs, bounds check memory_usage: 10/10 replicas passed. Gate passed.
- quality_gate_logs, bounds check total_bytes_received: 10/10 replicas passed. Gate passed.
876f736 to
8e514fb
Compare
4cc1aba to
f93cd7c
Compare
8e514fb to
e6aa81a
Compare
f93cd7c to
782d34f
Compare
@chatgpt-codex-connector This spotted issue was not related to MicroVM related work. A separate PR would be required to address this if we see the flaky tests failure |
|
Summary
Testing
Committed changes on the current branch: |
782d34f to
b26f433
Compare
e6aa81a to
797f7a4
Compare
3525db4 to
bfd2f48
Compare
797f7a4 to
936b03a
Compare
bfd2f48 to
b11ee51
Compare
936b03a to
2831b81
Compare
b11ee51 to
b669604
Compare
2831b81 to
ff280be
Compare
b669604 to
3d49316
Compare
ff280be to
31fa830
Compare
5181866 to
6fb46b4
Compare
31fa830 to
3747439
Compare
6fb46b4 to
50161a6
Compare
There was a problem hiding this comment.
Pull request overview
This PR restores Bazel build support for cmd/serverless-init/cloudservice by reintroducing Bazel targets for the packages it directly depends on and removing the root-level Gazelle exclusion that previously kept cloudservice (and mode) out of the Bazel build graph.
Changes:
- Add Bazel
go_library+dd_agent_go_testtargets forcmd/serverless-init/mode. - Add Bazel
go_library+dd_agent_go_testtargets forcmd/serverless-init/cloudservice. - Remove the
gazelle:excludedirectives (and associated comment) forcmd/serverless-init/cloudserviceandcmd/serverless-init/modefrom the repo rootBUILD.bazel.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
cmd/serverless-init/mode/BUILD.bazel |
Introduces Bazel targets for the mode package and its tests so it can be built/tested under Bazel. |
cmd/serverless-init/cloudservice/BUILD.bazel |
Re-adds Bazel targets for the cloudservice package and its unit tests, restoring it to the Bazel build graph. |
BUILD.bazel |
Drops Gazelle exclusions that prevented cloudservice/mode from being considered by Bazel builds. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
672ffa9 to
3d06be2
Compare
50161a6 to
072a8c0
Compare
Adds `cloudservice.MicroVM`, a `CloudService` implementation for AWS Lambda MicroVMs, and a `LifecycleContext` type carried on `TracingContext.LifecycleCtx`: - `GetTags` parses `DD_AWS_MICROVM_IMAGE_ARN` for `region`/`account_id`/`image_name` (falling back to `"unknown"` for any field it can't parse). - `GetEnhancedMetricTags` derives base/usage tag sets from those tags. - `Init` reads a `LifecycleContext` (metric/log flushers, trace-tag/log-tag setters, flush timeout, sidecar flag) and constructs + starts the lifecycle hook server. - `Run` spawns the user process via `mode.RunInit`, binding `ProcessHooks.OnAlive`/`OnDead` to the lifecycle server's child so its `/ready` check reflects real liveness. Sidecar mode is fatal for MicroVM — there's no child process to track, so `/ready` would silently return 503 forever instead of surfacing the misconfiguration. - `Shutdown` stops the lifecycle server within a bounded timeout so in-flight `/suspend`/`/terminate` requests can complete before the metric/trace agents tear down. - MicroVM supports both amd64 and arm64 (every other cloud service here is amd64-only). `MicroVM` is a complete, self-contained type in this PR but is **not yet reachable**: `GetCloudServiceType` still doesn't know about it, so nothing in the running agent changes yet. This is the core piece of the MicroVM integration: everything needed to answer the lifecycle server's HTTP hooks and track the user process's liveness, in one type that satisfies `CloudService` end-to-end. Landing it as its own PR — before wiring it into `main.go` or registering it in `GetCloudServiceType` (PR 5) — lets it be reviewed as a unit without also having to reason about the wiring changes, and avoids any intermediate state where `MicroVM` could be selected by `GetCloudServiceType` before `main.go` populates its `LifecycleContext` (which would leave `Init` a no-op and `Run` dereferencing a nil child). This is PR 2 of 5 in the split of #53036 (`tianning.li/microvm-07-microvm-service-wiring`). See PR 1 (#53092) for the full stack list. ``` dda inv test --targets=./cmd/serverless-init/... ``` All 242 tests pass (3 platform-skips) — this PR adds no new tests of its own; `MicroVM`'s tests land in PRs 3 and 4 so each stays under ~300 lines. Existing tests confirm nothing else regresses with the new type and field present but unused. Amended to address Copilot review feedback on PR #53092: - `mode/initcontainer_mode.go`: `hooks.OnDead`'s defer was registered only inside the `hooks.OnAlive != nil` branch, and after `OnAlive()` ran. An `OnDead`-only hook never fired, and `OnDead` wouldn't fire if `OnAlive` panicked. Now deferred unconditionally right after `cmd.Start()` succeeds. - `main.go`: fixed a non-gofmt-compliant import grouping (`comp/logs-library/processor` was sandwiched into the stdlib group). - `cloudservice/microvm_test.go`: renamed `Test...InvokedOnLaunch`/`launchPath` and reworded assertion messages that said `/launch` to match the actual `/run` route under test. - `mode/initcontainer_mode_test.go`: added `TestExecute_OnDeadOnly_OnAliveNil_StillFires` and `TestExecute_OnAlivePanics_OnDeadStillFires`, regression tests that fail against the pre-fix code and pass with the fix above. --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
… tags Codex flagged a data race on PR #53036: MicroVM's /run lifecycle hook calls serverlessTraceAgent.SetTags from an async goroutine, which mutates Agent.GlobalTags via SetGlobalTagsUnsafe concurrently with the trace agent's span-processing loop reading that same field unsynchronized. GlobalTags is computed once at config-build time and every reader in pkg/trace/agent and pkg/trace/api assumes it is frozen thereafter; MicroVM's dynamic lambda_microvm_id update breaks that invariant. Route the async update through the span modifier instead, which already runs at the identical point in the span-processing loop and is exclusively serverless-owned: - pkg/serverless/trace/span_modifier.go: tags field is now atomic.Pointer[map[string]string]; ModifySpan reads it lock-free instead of reading a field written by a separate mutator with no synchronization. - pkg/serverless/trace/trace.go: new UpdateRuntimeTags method that only updates the span modifier, never GlobalTags. Existing SetTags (used once synchronously at startup, before the trace agent runs) is untouched. - cmd/serverless-init/main.go: the two async TraceTagSetterFunc closures now call UpdateRuntimeTags instead of SetTags. This is scoped entirely to MicroVM: every other cloud service ignores LifecycleCtx in Init, so they never exercise the async path and keep calling the original SetTags at startup unchanged. Also fixes an issue flagged by Codex review: ModifySpan's new tag-apply loop unconditionally overwrote _dd.origin whenever the tags map contained it, undoing the "only fill _dd.origin if absent" guard immediately above it. Every CloudService.GetTags() sets _dd.origin (not just MicroVM's), and that value flows into the tags applied here via SetTags/UpdateRuntimeTags at startup for every cloud service — so this would have silently overwritten a tracer-supplied span origin (e.g. _dd.origin:rum) for all of them, not just MicroVM. Fixed by skipping _dd.origin in the loop. Covered by TestSpanModifierModifySpanPreservesExistingOrigin. A second Codex finding on this PR — UpdateRuntimeTags not reaching spans processed via the V1 payload path (ProcessV1 skips SpanModifier) — is left as a known follow-up; not fixed here. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ge metric Codex flagged on PR #53093 (review comment #53093 (comment)) that MicroVM's enhanced usage metric (aws.lambda.microvm.instance) is emitted with the same static tag set for every MicroVM booted from a given image. MicroVM.GetEnhancedMetricTags returns Usage tags at startup, before the MicroVM's instance ID is known (it only becomes available once the /run lifecycle hook fires), and nothing ever adds it afterward. Under normal autoscaling, with multiple concurrent MicroVMs from the same image, their usage samples become indistinguishable from each other. Rather than mutating shared, periodically-read state on ServerlessMetricAgent (which would need a lock/atomic and broke go vet's copylocks check, since CloudService.Shutdown takes ServerlessMetricAgent by value across every cloud service), this attaches the tag at the point of emission instead: - lifecycle.Server already tracks the instance ID race-free (instanceID *atomic.String, captured in handleRun). Added InstanceID(), a nil-safe accessor, so callers outside the lifecycle package can read it. - MicroVM.CurrentUsageMetricTags() turns that into an "instance:<id>" tag, or nil before /run has fired. - enhanced-metrics.Collector gained an optional usageMetricTagsFunc, invoked on every collection tick and passed through the existing AddEnhancedUsageMetric(..., extraTags ...string) parameter — no new shared mutable state required. - main.go duck-types cloudService against a local usageMetricTagProvider interface to wire this hook; every other cloud service doesn't implement it, so usageMetricTagsFunc stays nil and their usage metrics are unaffected. This keeps the change scoped to MicroVM and the (nil-safe, additive) collector plumbing — no other cloud service, ServerlessMetricAgent, or the Shutdown interface needed to change. Unit tests: dda inv test --targets=./cmd/serverless-init/... Amended to address Copilot review feedback on this PR: - `cloudservice/microvm_test.go`: the new TestMicroVM_CurrentUsageMetricTags_AfterRun_ReturnsInstanceTag dialed l.Addr().String() directly, which resolves to the unspecified address (e.g. [::]:port) since the lifecycle server binds with no host — not a guaranteed-valid dial target on every platform. Now dials 127.0.0.1:<port> explicitly. Also reworded the leftover launchPath/ "/launch" naming to runPath/"/run" to match the route under test. - `cloudservice/microvm.go`: CurrentUsageMetricTags' doc comment cited exact file:line locations in appservice.go/cloudrun.go, which drift as those files change. Now references the method names (AppService.GetEnhancedMetricTags, CloudRun.GetEnhancedMetricTags) instead. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Update MicroVM lifecycle hook handling to match AWS's documented behavior for image-build and runtime hooks. For /ready and /validate, keep the platform-owned retry loop in control by returning fast 503s when the user app is not reachable or when forwarding times out. Set the default /ready and /validate forward budgets to 30s each so build-time smoke tests are not capped by the runtime hook default, and include the reachability dial budget in the lifecycle server WriteTimeout. For /run, fail the hook when the platform request body cannot be read instead of parsing or forwarding a partial runHookPayload. Validated with dda inv test --targets=./cmd/serverless-init/lifecycle/... and dda inv test --targets=./cmd/serverless-init. Amended to address Copilot review feedback on this PR: - lifecycle/forwarder.go: the Forwarder struct's readyTimeout/validateTimeout field comments still said "default 60s"/"default 10s" -- stale relative to the actual defaultReadyTimeout/defaultValidateTimeout constants (30s each, per the const block's own comment). Updated both to say "default 30s". (Copilot's TestNewForwarder_Defaults comment was based on this same stale 60s/10s reading -- no test change needed there, since the test already asserts the correct 30s/30s.) - lifecycle/forwarder_test.go: TestForwarder_PassThroughWaiting_UnboundPort_FailsFastWith503 passed a 150ms timeout and asserted elapsed < 100ms, conflating that parameter with dialCheckTimeout (the actual internal bound used by the reachability check) and leaving little margin under load. Now passes a 2s timeout -- large enough that a passing assertion can only be explained by the fast dial-check path -- and asserts elapsed < dialCheckTimeout+100ms. Verified stable across 5 repeated runs. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
3d06be2 to
2fae807
Compare
cloudservice was re-excluded from Gazelle in PR #53031 as a stopgap after adding a Run() dependency on mode/log, which pulled in an unmigrated chain: mode -> log -> pkg/serverless/logs -> comp/logs/agent/impl -> pkg/logs/launchers/{file,integration,journald, container/tailerfactory}. Migrate the whole chain instead of leaving cloudservice excluded: remove the gazelle:exclude directives for all of the above and add the generated BUILD.bazel files (container's stub gazelle:ignore is replaced with a real target now that tailerfactory is migrated). Un-excluding pkg/logs/launchers/file exposed one more gap: its file_test target has a linux/android-only source, launcher_privileged_logs_test.go, that imports pkg/privileged-logs/test. That package is gated behind the custom linux_bpf build tag and remains gazelle-excluded (pre-existing, unrelated to this PR), so Gazelle emitted an unresolvable '@//pkg/privileged-logs/test' label in file_test's deps, breaking 'bazel build //pkg/logs/launchers/file/...' with 'No BUILD file found for package pkg/privileged-logs/test'. Un-excluding pkg/privileged-logs/test would only move the problem further down the chain: its handler.go imports cmd/system-probe/modules, itself deliberately gazelle-excluded and unrelated to serverless-init. Rather than pull the entire system-probe module registry into this PR's scope, exclude just the one offending file (pkg/logs/launchers/file/launcher_privileged_logs_test.go) from Gazelle and regenerate. It still runs fine under 'dda inv test'/plain 'go test'; it's just not covered by this package's Bazel test target, matching its pre-existing lack of Bazel coverage. Verified with bazel build //... and a scoped bazel test of the restored chain; the only skips are pre-existing platform-incompatible targets (journald's cgo/systemd dep is Linux-only), which already exit 0 under CI's --keep_going.
072a8c0 to
810ffe1
Compare
2fae807 to
0c09e1a
Compare
Why
PR #53031 added
Run(modeConf mode.Conf, logConfig *serverlessInitLog.Config) errorto the
CloudServiceinterface, which madecmd/serverless-init/cloudserviceimport
cmd/serverless-init/modeandcmd/serverless-init/logfor the firsttime. Both packages were still
gazelle:excluded (never migrated to Bazel),so
bazel build //...broke:As a stopgap to unblock CI, that PR re-excluded
cloudserviceitselfalongside
mode/log, reverting it to its pre-Bazel-migration state. Thatunblocked CI but is a step backward for the repo-wide Bazel migration
(
cloudservicehad already been migrated once, in #52842) and leaves itwithout Bazel test coverage.
This PR does the migration properly instead of leaving the exclusion in place.
Why removing just those 3 lines wasn't enough
Bazel requires every transitive Go import to resolve to a buildable target —
unlike
go build, which only needs the packages actually reached by theactive build tags. Un-excluding only these three doesn't stop at
mode/log:following the real (verified via direct
importinspection, not assumption)dependency chain,
every one of those packages was also still gazelle-excluded. Removing only
the first three would have just moved the "no such package" error one hop
further down the chain (confirmed this is exactly what happened when this was
first investigated: un-excluding
log/modealone reproduced the identicalfailure at
pkg/serverless/logs, then atcomp/logs/agent/impl). So all 8gazelle:excludelines had to come out together in one PR, innermostdependency first, with Gazelle re-run and each generated
BUILD.bazelreviewed by hand.
Every other internal dependency of this chain was already migrated — no
further cascading exclusions were needed beyond these 8.
Summary of changes
gazelle:excludedirectives (plus their explanatory comment)from the root
BUILD.bazel, for:cmd/serverless-init/{cloudservice,log,mode},comp/logs/agent/impl,pkg/serverless/logs, andpkg/logs/launchers/{container/tailerfactory,file/*.go,integration,journald}.BUILD.bazelfor each of those packages.pkg/logs/launchers/container/BUILD.bazelwas previously a# gazelle:ignorestub (it couldn't be built without
tailerfactory); replaced with a realtarget now that
tailerfactoryis migrated..gosource files changed — this is a pure Bazel build-graph change.Test plan
bazel build //...— no errors on any of the restored packages (therun did hit unrelated disk-space-exhaustion failures in ~340 other,
pre-existing packages; none touch this change).
bazel build/bazel testof the full restored chain — buildsclean; 5/5 runnable tests pass. The remaining test variants
(
cloudservice/log/mode/impl/journald) reportSKIPPEDonmacOS specifically because
pkg/logs/launchers/journaldlinks aLinux-only cgo/systemd dependency — verified this exact
unconditional dependency is what Gazelle has always generated for this
package (byte-identical to the last time it was live in
//..., pergit show 7559a83ec93:pkg/logs/launchers/journald/BUILD.bazel), andconfirmed CI's
bazel test --keep_going //...on macOS treats theseskips as non-fatal (exit 0), so this isn't a regression.
bazel run //bazel/buildifier— clean, no formatting changes needed.