cleanup: drop dead processors, cmd dirs, backup scripts, v0 aliases - #363
Merged
Merged
Conversation
Removes scaffolding that was unused after the cold-path delivery-modes split (#359) and the asap-otel unification. The user explicitly authorized breaking external workflows (build scripts, paper sweeps); old benchmark scripts that targeted deleted cmd dirs are removed rather than repointed. Removed: - `opentelemetry-collector-contrib-patch/processor/gorillaprocessor/` (non-S3 sibling — never wired into MVP path; gorillas3processor with `delivery_mode={best_effort,durable_raw,durable_fragment}` covers all cold-path use cases now). - 12 old per-sketch + per-collector cmd dirs: `gorillacol`, `sketchcollector`, `countminsketchcol`, `countsketchcol`, `ddsketchcol`, `hllcol`, `intervalcol`, `nopcol`, `otelcontribcol`, `serfagentcol`, `serfbackendcol`, `serfcol`. Only `asap-otel`, `asap-otel-opamp`, `kll`, and `telemetrygen` remain. - 8 root `backup_*.sh` + 8 root `restore_*.sh` patch-overlay helpers; the patch tree is the source of truth now. - 7 `otel_collector_benchmark/bench_*.sh` scripts targeting deleted col binaries. - `build_ddsketchcol.sh` (root); `build_asap_otel.sh` and `build_asap_telegraf.sh` retained but no longer call the deleted restore scripts. - v0 agent-runtime header aliases (`sketchcollector` / `sketchotap` / `sketchtelegraf`) in `controller/src/config/mod.rs` + their unit test. The `TODO(remove-after-2026-Q3)` marker is obsolete; the new aliases (`asap-otap`/`otap`, `asap-telegraf`/`telegraf`, `asap-otel`) cover all in-tree agents. Updated (not deleted): - `opentelemetry-collector-contrib-patch/cmd/asap-otel*/builder-config.yaml`: dropped the `gorillaprocessor` entry. - `opentelemetry-collector-contrib-patch/cmd/asap-otel-opamp/supervisor-config.yaml`: fixed stale `executable: ./countminsketchcol` → `./asap-otel-opamp`. - `setup.sh`, `README.md`: removed Step 5 patch-overlay paragraph + ddsketchcol build instructions; repointed users to `build_asap_otel.sh`. - `tests/{otel_controller_e2e_test.sh, promql_e2e_collector_test.sh}`: repointed `DDSKETCHCOL` → `ASAP_OTEL`. - `docs/e2e-test-guide.md`: 3 references repointed. - `datasets_eval/debs/benchmark/run.py`: `DEFAULT_COLLECTOR_PATHS` routes all sketch types (except `kll`) at the unified `asap-otel` binary. - `otel_collector_benchmark/README.md` + `cardinality_crossover/` + `matched_accuracy/`: stale `countminsketchcol/builder-config.yaml` comments updated. Verification: - `cargo test --manifest-path controller/Cargo.toml -- --test-threads=1` → 620 passed (was 621; -1 from removed v0-alias unit test). - All 11 patched Go processor test suites pass. - `python3 -m pytest deploy/scripts/tests/ -q` → 68 passed. Known follow-ups (deferred to future PRs): - `cmd/asap-otel/{components.go,go.mod,go.sum}` still hold stale `gorillaprocessor` import lines; OCB regenerates them on next `build_asap_otel.sh`. - `cmd/bench.sh` still has dead case branches for the removed per-sketch cmd dirs (only `kll*` branches remain runnable). - `docs/{pipeline-query-catalog,serf-compression-architecture,opamp-config-push}.md` mention deleted col binaries in explanatory prose; rewrite is out of scope for a code-cleanup PR. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2 tasks
zzylol
added a commit
that referenced
this pull request
May 9, 2026
After cleanup PR #363 deleted the per-sketch / per-collector cmd dirs (`countminsketchcol`, `ddsketchcol`, `hllcol`, `serfcol`, `gorillacol`, etc.), only the `kll*` branches in `cmd/bench.sh` had functioning build targets — 23 of 28 case branches pointed at deleted directories. Rather than trim to a kll-only stub of a 922-line script, drop the harness entirely and route bandwidth/CPU/throughput measurements through the MVP demo's `measure_stages.py` + `measure_per_edge_bandwidth.py` (which work against the unified `asap-otel` build). The Serf compression architecture doc had a "Benchmark targets" section listing 11 deleted bench.sh invocations; replaced with a brief note explaining the harness was removed and pointing at the live processor (`serfprocessor` is still in the patched contrib tree) plus the MVP measurement scripts. The opentelemetry-app README's "Running via bench.sh" subsection was a single dangling pointer; removed. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2 tasks
zzylol
added a commit
that referenced
this pull request
May 9, 2026
After PR #363 deleted per-sketch / per-collector cmd dirs (`countminsketchcol`, `ddsketchcol`, `kllcol`, `hllcol`, `countsketchcol`, `gorillacol`, `serfcol`, `sketchcollector`, etc.), three docs still mentioned them. Repoint to the unified `asap-otel` binary; where the prose is about the underlying processor, name the processor instead. - `docs/pipeline-query-catalog.md` — 13 substitutions across the pipeline diagram, Q-C1–Q-C6 query-mapping table, DEBS Q3 reference, §7.3–§7.6 stage bullets, §7.8 ClickHouse + MetricsQL maps, NCCL Inspector YAML, and §8.2 archive-lane prose. - `docs/opamp-config-push.md` — topology diagram + §4 capability- matching section rewritten to reflect single-binary consolidation (kept past-tense historical sentence about per-sketch builders). - `docs/serf-compression-architecture.md` — verified line 302's past-tense historical mention of deleted dirs is correct as written. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
zzylol
added a commit
that referenced
this pull request
May 9, 2026
After PR #363 deleted per-sketch / per-collector cmd dirs (`countminsketchcol`, `ddsketchcol`, `kllcol`, `hllcol`, `countsketchcol`, `gorillacol`, `serfcol`, `sketchcollector`, etc.), three docs still mentioned them. Repoint to the unified `asap-otel` binary; where the prose is about the underlying processor, name the processor instead. - `docs/pipeline-query-catalog.md` — 13 substitutions across the pipeline diagram, Q-C1–Q-C6 query-mapping table, DEBS Q3 reference, §7.3–§7.6 stage bullets, §7.8 ClickHouse + MetricsQL maps, NCCL Inspector YAML, and §8.2 archive-lane prose. - `docs/opamp-config-push.md` — topology diagram + §4 capability- matching section rewritten to reflect single-binary consolidation (kept past-tense historical sentence about per-sketch builders). - `docs/serf-compression-architecture.md` — verified line 302's past-tense historical mention of deleted dirs is correct as written. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
zzylol
added a commit
that referenced
this pull request
May 9, 2026
Three small follow-ons after PR #365 (asap-gorilla symlink delete) + PR #363 (gorillaprocessor / per-sketch cmd dirs delete) made the service rebuild flow brittle: ## (1) `Dockerfile.backend` — repoint asap-gorilla path-dep The backend's `asap-query-engine/Cargo.toml` path-dep was updated to `asap-gorilla = { path = "../../ASAPCollector/asap-gorilla-rust" }` in PR #118 (backend repo) when the symlink was being removed, but the matching Dockerfile still copied to `ASAPCollector/asap-gorilla` (the now-deleted symlink path). Result: cargo couldn't find the crate inside the build container; backend image rebuild failed. Repoints the COPY: - `--from=asap-gorilla → ASAPCollector/asap-gorilla` → - `--from=asap-gorilla-rust → ASAPCollector/asap-gorilla-rust` Caller has to pass the matching `--build-context asap-gorilla-rust=/path/to/asap-gorilla-rust` (mirrors the existing `asap-precompute-rs` build-context pattern). ## (2) `cmd/asap-otel/builder-config.yaml` — add asap-gorilla-go replace `gorillas3processor`'s own go.mod has `replace github.com/ProjectASAP/asap-gorilla-go => ../../../asap-gorilla-go`, but OCB does NOT transitively forward processor-level replaces into the binary's generated `cmd/asap-otel/go.mod`. Result: OCB tried to fetch `asap-gorilla-go` from github.com (private/non-existent public path), build failed with "Repository not found". Adds the replace at the binary level, mirroring the existing `sketchlib-go` and `asap-precompute-go` redirects. Path math documented inline (binary go.mod is 3 dirs deep under repo root). ## (3) `mvp-no-resource-limits.yml` — opt-in override The MVP demo's `mvp-multi-stage.yml` hardcodes `cpus: 1.5` and `memory: 1536M` on agents (and similar caps on gateway / Thanos services). At any meaningful cardinality (≥2k aggregate series), the agent's 5-sketch fan-out + windowState buffer eats 7+ GiB of RSS and gets OOM-killed (SIGKILL/137). Each restart flushes a 1-ms-wide TSDB block — the cold-path-through-thanos returns present-but-degenerate data. Override lifts both `cpus` and `memory` (the whole `limits:` block) for: agent-a, agent-b, gateway, prometheus-b0, thanos-store-gateway, thanos-query, thanos-compact. Apply by appending `-f deploy/docker-compose/mvp-no-resource-limits.yml` to the existing compose chain. Default behaviour unchanged for paper-eval reproducibility. Verified with `docker compose ... config | python3 -c '...'`: all 7 services show `limits=None`. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
5 tasks
zzylol
added a commit
that referenced
this pull request
May 10, 2026
* chore: revive patch-overlay sync for OTel + Telegraf submodules PR #363 retired the patch-overlay restore scripts on the assumption that the patch trees alone were sufficient. They aren't: the patched sketch processors and selfmonitor live under the upstream submodule import paths (e.g. `go.opentelemetry.io/collector/processor/selfmonitor`), and the patched processors' `replace` directives point at `opentelemetry-collector/processor/...` not at the patch tree. A fresh clone or a worktree without overlay-staged submodules fails to build asap-otel because `processor/selfmonitor/` is missing from the v0.141.0 submodule. This revives the overlay step. The patch trees (committed in ASAPCollector main) stay the source of truth; submodules stay clean (we don't push changes upstream). The restore scripts copy `*-patch/` → submodule working tree at build time. Restored: - restore_otel_collector_patches.sh (85 files) - restore_otel_collector_contrib_patches.sh (201 files) - restore_otel_proto_patches.sh (15 files) - restore_opentelemetry_go_patches.sh (80 files) - restore_otel_client_patches.sh (legacy alias → -go) - restore_telegraf_patches.sh (7 files; skips go.mod/go.sum) - restore_otel_patches.sh (wrapper: collector+contrib+proto+go) - restore_all.sh (wrapper: otel + telegraf) Wired into: - build_asap_otel.sh: Step 1 calls restore_otel_patches.sh; --skip-patches is real again. - build_asap_telegraf.sh: Step 1 calls restore_telegraf_patches.sh; --skip-patches is real again. OTAP (build_asap_otap.sh) inlines its own overlay (stages a new crate + edits Cargo.toml/main.rs in place); not factored out, no behavior change there. Verification: - `bash -n` parses all 8 new scripts + the 2 modified build scripts. - Each restore script runs idempotently and reports the expected file counts on the current working tree. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore: revive backup_*.sh — submodule → patch-tree reverse sync Companion to the restore_*.sh revival. Use these when you've edited files directly in a submodule (e.g. while debugging at the build site) and want to fold the changes back into the canonical patch tree before they're blown away by the next restore. Restored: - backup_otel_collector_patches.sh (submodule → -patch) - backup_otel_collector_contrib_patches.sh (submodule → -patch) - backup_otel_proto_patches.sh (submodule → -patch) - backup_opentelemetry_go_patches.sh (submodule → -patch) - backup_otel_client_patches.sh (legacy alias → -go) - backup_telegraf_patches.sh (submodule → -patch) - backup_otel_patches.sh (wrapper) - backup_all.sh (wrapper) Each script uses `git status --porcelain` inside the submodule to discover modified/added/renamed paths and copies only those (plus their parent dirs) back into the patch tree. Deletions are skipped on purpose — patch trees should never delete files relative to the upstream tag. `backup_telegraf_patches.sh` does NOT exclude go.mod/go.sum (unlike its restore counterpart) — if you've genuinely modified those in the submodule, you probably want them captured. The script header notes the footgun: the patch tree's standalone `processors/allsketches/` go.mod is separate from telegraf's mono-module go.mod, so don't run this if you've only added an import inside the submodule's main go.mod. Verification: - `bash -n` parses all 8 scripts. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (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
1 participant
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.
Summary
Aggressive scaffolding cleanup — 139 files changed, +65 / −13,439. User explicitly authorized breaking external workflows; old benchmark scripts targeting deleted cmd dirs removed rather than repointed.
Removed:
gorillaprocessor(non-S3) —gorillas3processorwith 3 cold-path delivery modes covers all use casesgorillacol,sketchcollector,countminsketchcol,countsketchcol,ddsketchcol,hllcol,intervalcol,nopcol,otelcontribcol,serfagentcol,serfbackendcol,serfcol); keptasap-otel,asap-otel-opamp,kll,telemetrygenbackup_*.sh+restore_*.shpatch-overlay helpers (patch tree is source of truth)otel_collector_benchmark/bench_*.shtargeting deleted col binariescontroller/src/config/mod.rsUpdated, not deleted:
setup.sh,README.md,tests/*.sh,docs/e2e-test-guide.md,datasets_eval/debs/benchmark/run.py,otel_collector_benchmark/README.md, builder-configs, supervisor-config, build scripts — all references repointed or pruned.Companion PR
Backend dead-code cleanup: ProjectASAP/ASAPQuery-backend (in flight)
Test plan
cargo test --manifest-path controller/Cargo.toml -- --test-threads=1→ 620 passed (was 621; −1 from removed v0-alias unit test)python3 -m pytest deploy/scripts/tests/ -q→ 68 passedbash build_asap_otel.shto regeneratecmd/asap-otel/{components.go,go.mod,go.sum}(still hold stalegorillaprocessorimport lines that OCB will rewrite)Known follow-ups (out of scope)
cmd/asap-otel/{components.go,go.mod}regen via OCB on next buildcmd/bench.shhas dead case branches for removed per-sketch dirs (onlykll*branches still runnable) — full strip or full delete is a separate decisiondocs/{pipeline-query-catalog,serf-compression-architecture,opamp-config-push}.mdmention deleted col binaries in prose; rewrite is a docs task🤖 Generated with Claude Code