docs: OpAMP config push architecture — supervisor restart semantics (PR F) - #155
Merged
Merged
Conversation
…PR F) Documents the actual control-plane config-push topology that landed in #133 (commit 4b196e1, "feat: OpAMP Supervisor config — push-based config from controller to collector") and corrects an inaccurate claim in the existing control-plane-design.md. ## What's wrong today `docs/control-plane-design.md` §"OpAMP Clients — the Collectors" says: agents reload without restart and report back health and current config hash. This is incorrect. The actual apply path is: controller push → supervisor receives ServerToAgent → supervisor merges with local config → writes effective.yaml → restarts the child collector process with the new config The collector pipeline is **not** hot-reloaded in process. Commit 4b196e1 confirmed the supervisor-restart chain works end-to-end but the design doc was never updated. ## What's in this PR 1. **New doc: `docs/opamp-config-push.md`** — end-to-end architecture with: - Three-process ASCII topology (controller / supervisor / collector) - Controller-side push call graph with file:line references (`controller/src/opamp/mod.rs:77-283`, `controller/src/main.rs:133-404`, `controller/src/replan.rs:123-178`) - Supervisor-side apply flow (WebSocket register → `effective.yaml` merge → child restart → `opampextension` health report) - Rationale for why supervisor restart is used instead of in-process hot-reload (upstream OTel collector doesn't expose a graceful processor-DAG swap primitive; supervisor restart is the only pattern that works with stock processors today) - Known gap: controller has no knowledge of which sketch processors each collector binary has compiled in, so pushing a KLL config to `countminsketchcol` crashes the restarted collector. Documented with mitigation and long-term fix. - Test surface: what the existing 3 mock-WebSocket tests in `main.rs:991-1238` cover, and a proposed `#[ignore]` integration test layout for future real-supervisor-binary coverage. - File:line quick-reference index. 2. **Correction in `docs/control-plane-design.md`** — replaces the "reload without restart" paragraph with an accurate description that links to the new doc. Clarifies that the `opampextension` is a reporting channel, not an apply channel; the supervisor is the apply channel. ## What's NOT in this PR - **Real opampsupervisor binary integration test** — the proposed layout is in §5c of the new doc but not implemented. The test depends on the `opampsupervisor` binary being pinned in CI, which needs a separate Go toolchain decision. Marked as a follow-up that can land independently without touching the architecture. - **Capability-matching fix** — the "push a KLL config to a collector that doesn't have KLL compiled in" gap is documented but not fixed in this PR. That's a controller/supervisor registration change tracked in `controller-optimization-problem.md`. ## Scope rationale PR F was originally scoped as "verify / fix OpAMP config push from controller to agents". The control-plane side is already verified by the 3 existing integration tests in `controller/src/main.rs` and by the manual e2e validation in commit 4b196e1. What was missing was a written contract future contributors could refer to — especially since the existing design doc made a claim that the code no longer matches. Shipping docs + a test layout proposal closes that gap without the flakiness risk of a binary-level integration test. Co-Authored-By: Claude Opus 4.6 (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
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.
PR F — documents the actual control-plane config-push topology that landed in #133 (commit
4b196e1) and corrects an inaccurate claim in the existingdocs/control-plane-design.md.What's wrong today
docs/control-plane-design.md§"OpAMP Clients — the Collectors" currently says:This is incorrect. The actual apply path is:
The collector pipeline is not hot-reloaded in process. Commit
4b196e1confirmed the supervisor-restart chain works end-to-end but the design doc was never updated.What's in this PR
New doc:
docs/opamp-config-push.md— end-to-end architecture with:controller/src/opamp/mod.rs:77-283,controller/src/main.rs:133-404,controller/src/replan.rs:123-178)effective.yamlmerge → child restart →opampextensionhealth report)countminsketchcolcrashes the restarted collector. Documented with mitigation and long-term fix.main.rs:991-1238cover, and a proposed#[ignore]integration test layout for future real-opampsupervisor-binary coverage.Correction in
docs/control-plane-design.md— replaces the "reload without restart" paragraph with an accurate description that links to the new doc. Clarifies that theopampextensionis a reporting channel (health + config_hash), not an apply channel; the supervisor is the apply channel.What's NOT in this PR
opampsupervisorbinary integration test — the proposed layout is in §5c of the new doc but not implemented. Depends on theopampsupervisorbinary being pinned in CI, which needs a separate Go-toolchain decision. Marked as a follow-up that can land independently without touching the architecture.controller-optimization-problem.md.Scope rationale
PR F was originally scoped as "verify / fix OpAMP config push from controller to agents". The control-plane side is already verified by the 3 existing integration tests in
controller/src/main.rs:991-1238and by the manual e2e validation in commit4b196e1. What was missing was a written contract future contributors could refer to — especially since the existing design doc made a claim that the code no longer matches. Shipping docs + a test-layout proposal closes that gap without the flakiness risk of a binary-level integration test.Related
StreamingConfighot-reload on the backend, depends on the controller → backend channel being decided (HTTP POST is the leading option)🤖 Generated with Claude Code