diff --git a/.github/workflows/bazel.yml b/.github/workflows/bazel.yml index b6c4b3d45..2fce2ac0c 100644 --- a/.github/workflows/bazel.yml +++ b/.github/workflows/bazel.yml @@ -523,6 +523,19 @@ jobs: # CACHE holds only the cache-specific flags. COMMON=() if [ "${{ matrix.subtree.id }}" = "byoo-otel-collector" ]; then + # --jobs=1 is a memory guard, not a throughput setting. The + # collector genrule is analysed in three configurations (host, + # //platforms:linux_x86_64, //platforms:linux_arm64) and each + # `go build` of the otelcol module peaks at ~6 GB RSS, so two + # concurrent instances would not fit a 16 GB runner. Bazel has no + # per-action memory throttle for genrules ("resources:memory:N" + # and "cpu:N" execution requirements are ignored outside tests, + # and so is "exclusive"), so capping Bazel-side concurrency is the + # only lever. Per-build parallelism is set inside the genrule + # instead (go build -p $(nproc)), which is ~3x faster on a 4-vCPU + # runner at the same peak RSS. Nothing else in this module is + # expensive (the row's other ~380 actions are cache hits), so the + # cap costs nothing elsewhere. COMMON+=(--jobs=1) # Persist the collector genrule's Go module + build cache across CI # runs so cold runners restore instead of re-downloading the whole @@ -533,6 +546,18 @@ jobs: COMMON+=(--action_env="GOMODCACHE=$RUNNER_TEMP/byoo-gomodcache" --action_env="GOCACHE=$RUNNER_TEMP/byoo-gocache" --action_env="GOPATH=$RUNNER_TEMP/byoo-gopath") + # Bind the collector genrule's remote-cache key to the host Go + # toolchain. The genrule shells out to the image's `go`, which is + # not a declared Bazel input, so without this a toolchain bump in + # the bazel-ci image would keep serving collector binaries built by + # the previous compiler -- including across a Go security patch. + # --action_env values are part of the action key, so feeding the + # version string through it turns a toolchain change into a cache + # miss and a rebuild. Must match on build AND test. + byoo_go="$(command -v go || echo /usr/local/go/bin/go)" + byoo_go_ver="$("$byoo_go" version 2>/dev/null || echo unknown)" + echo "byoo collector toolchain: $byoo_go_ver" + COMMON+=(--action_env="BYOO_GO_TOOLCHAIN=$byoo_go_ver") fi CACHE=(--remote_cache=) if [ "${CACHE_READY:-0}" = "1" ]; then @@ -629,12 +654,19 @@ jobs: # deferred to that lane (see the bazel-integration job). COMMON=(--flaky_test_attempts=3 --test_tag_filters=-requires-docker) if [ "${{ matrix.subtree.id }}" = "byoo-otel-collector" ]; then + # Memory guard, not throughput. See the build step for the full + # rationale (~6 GB peak RSS per collector build, no per-action + # memory throttle for genrules). COMMON+=(--jobs=1) # Match the build step's --action_env so the collector genrule reuses - # the cached Go module/build cache instead of re-running (see #373). + # the cached Go module/build cache instead of re-running (see #373), + # and so the two steps compute the same action key. COMMON+=(--action_env="GOMODCACHE=$RUNNER_TEMP/byoo-gomodcache" --action_env="GOCACHE=$RUNNER_TEMP/byoo-gocache" --action_env="GOPATH=$RUNNER_TEMP/byoo-gopath") + byoo_go="$(command -v go || echo /usr/local/go/bin/go)" + byoo_go_ver="$("$byoo_go" version 2>/dev/null || echo unknown)" + COMMON+=(--action_env="BYOO_GO_TOOLCHAIN=$byoo_go_ver") fi CACHE=(--remote_cache=) if [ "${CACHE_READY:-0}" = "1" ]; then diff --git a/src/compute-plane-services/byoo-otel-collector/README.md b/src/compute-plane-services/byoo-otel-collector/README.md index 248fb6cfa..15add7831 100644 --- a/src/compute-plane-services/byoo-otel-collector/README.md +++ b/src/compute-plane-services/byoo-otel-collector/README.md @@ -71,12 +71,19 @@ OpenTelemetry Bazel community. The genrule trades Bazel's per-package dep tracking for forward progress: the binary lives inside Bazel's output graph and flows through to `oci_image` + `oci_push` cleanly. -Cache contract: Bazel rebuilds the genrule when any input -(`otelcol/**/*.go`, `go.mod`, `go.sum`) changes. The genrule uses -`local = True` + `tags = ["no-sandbox"]` so it can resolve `go` from -`$PATH` and write to the standard Go module cache. The wrapper +Cache contract: Bazel rebuilds the genrule when any declared input +(`otelcol/**/*.go`, `otelcol/go.mod`, `otelcol/go.sum`) changes; the +glob is `**/*.go` so a new subpackage is picked up automatically. The genrule is tagged +`no-sandbox` + `no-remote-exec` so it can resolve `go` from `$PATH` and +write to the standard Go module cache, while remaining eligible for the +build cache. It is deliberately not `local = True`: that tag also stops +the result being reused from the disk or remote cache, which made the +collector recompile on every CI run. Because the action shells out to a +host `go` that Bazel does not track, CI binds the toolchain into the +action key with `--action_env=BYOO_GO_TOOLCHAIN`, so a Go bump in the CI +image cannot serve binaries built by the previous compiler. The wrapper binary, in contrast, is a regular `go_binary` and benefits from full -Bazel hermeticity + nvcfbarn remote-cache reuse. +Bazel hermeticity + remote-cache reuse. A containerized Go application that provides a complete observability solution by orchestrating three functional components: it generates OpenTelemetry Collector configurations, extracts and manages secrets from ESS (Encrypted Secret Store), and runs a custom-built OpenTelemetry Collector binary. diff --git a/src/compute-plane-services/byoo-otel-collector/VERSION b/src/compute-plane-services/byoo-otel-collector/VERSION index a95410b17..4f58d4880 100644 --- a/src/compute-plane-services/byoo-otel-collector/VERSION +++ b/src/compute-plane-services/byoo-otel-collector/VERSION @@ -1 +1 @@ -0.157.0 +0.157.1 diff --git a/src/compute-plane-services/byoo-otel-collector/otelcol/BUILD.bazel b/src/compute-plane-services/byoo-otel-collector/otelcol/BUILD.bazel index 8b73fb6b5..1fcb05821 100644 --- a/src/compute-plane-services/byoo-otel-collector/otelcol/BUILD.bazel +++ b/src/compute-plane-services/byoo-otel-collector/otelcol/BUILD.bazel @@ -81,11 +81,28 @@ GOMODCACHE="$${GOMODCACHE:-/tmp/byoo-otelcol-gomodcache}" GOPATH="$${GOPATH:-/tmp/byoo-otelcol-gopath}" GOCACHE="$${GOCACHE:-/tmp/byoo-otelcol-gocache}" HOME="$${HOME:-$$(mktemp -d -t byoo-otelcol-home.XXXXXX)}" -# Cap parallelism. Each linker pass of the otelcol module -# holds high RSS, so the Go toolchain's internal worker pool -# can OOM a standard CI runner even when Bazel is serialized. -GOMAXPROCS="$${GOMAXPROCS:-2}" -GO_BUILD_P="$${GO_BUILD_P:-1}" +# Parallelism. Peak RSS of a collector build is set by the `go build` +# driver holding export data for the whole 250+ module graph and by the +# final link. Neither scales with how many compile actions run at once, +# so the previous -p 1 cap traded a lot of wall time for no memory +# headroom. Measured cold (warm module cache, otelcol v0.157.0): +# +# -p 1 490s 5.69 GB peak RSS +# -p 2 211s 6.09 GB +# -p 4 97s 6.08 GB +# -p 8 58s 5.27 GB +# +# What actually protects a 16 GB runner is never running two collector +# builds at once, which CI enforces Bazel-side with --jobs=1 (see +# .github/workflows/bazel.yml); Bazel has no per-action memory throttle +# for genrules. Both knobs stay overridable via env for hosts that need +# a smaller footprint. +BYOO_NPROC="$$(nproc 2>/dev/null || echo 2)" +# Cap at 8: past that, `go build` is link-bound on this module and the +# extra workers only add memory pressure on large dev machines. +if [ "$$BYOO_NPROC" -gt 8 ]; then BYOO_NPROC=8; fi +GOMAXPROCS="$${GOMAXPROCS:-$$BYOO_NPROC}" +GO_BUILD_P="$${GO_BUILD_P:-$$BYOO_NPROC}" export GOMODCACHE GOPATH GOCACHE HOME GOMAXPROCS GO_BUILD_P mkdir -p "$$GOMODCACHE" "$$GOPATH" "$$GOCACHE" WORK_DIR=$$(dirname $$(realpath $(location go.mod))) @@ -108,11 +125,14 @@ GOOS=linux GOARCH=arm64 CGO_ENABLED=0 \\ genrule( name = "otelcol-contrib-bin", + # **/*.go rather than listing each package: the previous glob covered + # root-level files plus logchunkprocessor only, so a new subpackage would + # have been silently excluded from the action key and its edits would not + # have invalidated the build. The resolved file set is unchanged today. srcs = glob([ - "*.go", + "**/*.go", "go.mod", "go.sum", - "logchunkprocessor/**/*.go", ]), outs = ["otelcol-contrib"], # GO_BIN can be overridden via env when the host installs go at a @@ -124,15 +144,38 @@ genrule( "@platforms//cpu:aarch64": _CMD_GO_BUILD_ARM64, "//conditions:default": _CMD_GO_BUILD_AMD64, }), - local = True, tags = [ - # "exclusive" forces Bazel to run this genrule alone, so the - # multi_arch transition does not invoke two collector builds - # in parallel and double-peak the runner's memory. The Go - # toolchain's internal worker pool is bounded separately via - # GOMAXPROCS in the cmd above. - "exclusive", "manual", + # no-sandbox: the action shells out to the host `go`, which is not + # a declared input, so it cannot execute inside Bazel's sandbox. + # no-remote-exec: for the same reason it can never be dispatched to + # a remote worker; it has to run where `go` is installed. + # + # Deliberately NOT `local = True`. Bazel's `local` tag is the union + # of no-sandbox + no-remote-exec + no-cache, and that last component + # is what made every CI run recompile the collector from scratch: + # the result was never written to or read from the remote cache, so + # a warm cache bought nothing. The GHA bazel matrix logged + # "383 processes: 317 remote cache hit, 59 internal, 3 local" with + # those 3 local actions -- this genrule in its three configurations + # -- accounting for ~18 of the row's ~20 minutes. Keeping the first + # two execution requirements and dropping no-cache preserves + # identical execution semantics (unsandboxed, on the local machine) + # while letting an unchanged collector resolve to a cache hit. + # + # Cache-key safety. Two things are not covered by the declared + # inputs, and both are handled: + # - Target architecture: the select() above puts GOOS/GOARCH + # literally in the command line, and the command line is part of + # the action key, so an amd64 result can never be served for an + # arm64 request. + # - Host toolchain: `go` itself is not an input. CI binds it to + # the key out-of-band with + # --action_env=BYOO_GO_TOOLCHAIN="$(go version)" (see + # .github/workflows/bazel.yml), so bumping Go in the bazel-ci + # image invalidates the entry rather than silently serving a + # binary built by the previous compiler. + "no-remote-exec", "no-sandbox", ], visibility = ["//visibility:public"],