Skip to content

docs: OpAMP config push architecture — supervisor restart semantics (PR F) - #155

Merged
zzylol merged 1 commit into
mainfrom
docs/opamp-config-push
Apr 15, 2026
Merged

zzylol merged 1 commit into
mainfrom
docs/opamp-config-push

Conversation

@zzylol

@zzylol zzylol commented Apr 14, 2026

Copy link
Copy Markdown
Contributor

PR F — documents the actual control-plane config-push topology that landed in #133 (commit 4b196e1) and corrects an inaccurate claim in the existing docs/control-plane-design.md.

What's wrong today

docs/control-plane-design.md §"OpAMP Clients — the Collectors" currently 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.yamlrestarts 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 supervisor restart vs 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-opampsupervisor-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 (health + config_hash), 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. 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-attributes 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:991-1238 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.

Related

  • ASAPQuery PR #9 — modified-OTLP sketch data path (Rust consumer)
  • sketchlib-go PR #50 (merged) — Go producer for cross-language MessagePack
  • DataCollector PR #154 — proto enum names for the data path
  • Next: ASAPQuery "PR E" — StreamingConfig hot-reload on the backend, depends on the controller → backend channel being decided (HTTP POST is the leading option)

🤖 Generated with Claude Code

…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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant