feat: add opamp controller extension#3410
Conversation
There was a problem hiding this comment.
Pull request overview
Adds an initial Rust implementation of an OpAMP Agent Controller Extension for the OTAP Dataflow engine, enabling remote engine config updates and status reporting over WebSocket (with future transport/security follow-ups noted in the PR description).
Changes:
- Introduces a new
opampcontroller extension module with config validation, WebSocket session loop, and status/config exchange logic. - Adds OpAMP protobuf generation and commits the generated Rust bindings, backed by a new
proto/opamp-specsubmodule. - Adds test utilities and integration-style tests (mock OpAMP WebSocket server) plus required Rust dependencies.
Reviewed changes
Copilot reviewed 15 out of 16 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
rust/otap-dataflow/xtask/src/genproto.rs |
Extends proto generation to compile OpAMP protos into the controller extension’s proto module. |
rust/otap-dataflow/crates/controller/src/lib.rs |
Exposes the new controller extension module publicly. |
rust/otap-dataflow/crates/controller/src/extension.rs |
Introduces the controller extension module entry point (pub mod opamp). |
rust/otap-dataflow/crates/controller/src/extension/opamp/mod.rs |
Implements the OpAMP controller extension runtime (session state, message handling, reconciliation, tests). |
rust/otap-dataflow/crates/controller/src/extension/opamp/config.rs |
Adds extension configuration types and endpoint/backoff validation. |
rust/otap-dataflow/crates/controller/src/extension/opamp/consts.rs |
Defines component health status string constants used in OpAMP health reporting. |
rust/otap-dataflow/crates/controller/src/extension/opamp/error.rs |
Defines error types for OpAMP config validation failures. |
rust/otap-dataflow/crates/controller/src/extension/opamp/util.rs |
Adds an exponential backoff utility (with jitter) used for reconnect/retry behavior. |
rust/otap-dataflow/crates/controller/src/extension/opamp/test/util.rs |
Adds a mock ControlPlane and mock OpAMP WebSocket server helpers for tests. |
rust/otap-dataflow/crates/controller/src/extension/opamp/proto/mod.rs |
Adds module wiring for the generated OpAMP protobuf bindings. |
rust/otap-dataflow/crates/controller/src/extension/opamp/proto/opamp.proto.v1.rs |
Adds generated Rust types for OpAMP protobuf messages. |
rust/otap-dataflow/crates/controller/Cargo.toml |
Adds dependencies needed by the extension and its tests (WebSocket, prost, axum, etc.). |
rust/otap-dataflow/Cargo.toml |
Updates workspace dependency versions/features for axum websockets and adds tungstenite-related crates. |
rust/otap-dataflow/Cargo.lock |
Locks new dependency additions (including multiple tokio-tungstenite/tungstenite versions). |
.gitmodules |
Adds proto/opamp-spec as a git submodule to source OpAMP protobuf definitions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report❌ Patch coverage is ❌ Your patch check has failed because the patch coverage (70.04%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #3410 +/- ##
==========================================
- Coverage 86.23% 86.15% -0.09%
==========================================
Files 741 746 +5
Lines 292447 294595 +2148
==========================================
+ Hits 252203 253819 +1616
- Misses 39720 40252 +532
Partials 524 524
🚀 New features to boost your workflow:
|
…README - Frame WebSocket messages with the varint header required by the OpAMP spec, on both send and receive; previously bare protobuf bytes were sent, and no message from a conforming server could be decoded. - Encode instance_uid in standard RFC 4122 byte order (into_bytes) instead of the little-endian mixed encoding (to_bytes_le). - Advertise AcceptsRemoteConfig and AcceptsRestartCommand capabilities; without them a conforming server never offers config or commands. - Accept ServerToAgent messages with an empty instance_uid, which the opamp-go server sends on server-initiated pushes. - Add README.md with step-by-step instructions for testing against the opamp-go example server.
d32026d to
86a2aaa
Compare
|
|
||
| false | ||
| } | ||
| HandledMessageAction::Restart => { |
There was a problem hiding this comment.
Now that we advertise AcceptsRestartCommand, should this be wired to a real agent/process restart?
| Phase::ExchangeMessages => { | ||
| tokio::select! { | ||
| _ = cancellation_token.cancelled() => { | ||
| try_send_close_message(&mut ws_sender, config.shutdown_timeout).await; |
There was a problem hiding this comment.
Thanks for adding the WebSocket close frame. I think this still misses the OpAMP part though: before the close frame, the client should send an AgentToServer message with agent_disconnect set. Right now this only sends the WebSocket Close frame, so the server still does not receive the OpAMP disconnect message.
There was a problem hiding this comment.
I'm sorry about that Lalit, you're right. I should have read your previous feedback more carefully!
This has been corrected here: 7f7a5c3
|
|
||
| ### 2. Configure and run the engine | ||
|
|
||
| Save as `opamp.yaml`: |
There was a problem hiding this comment.
lets have a working minimal file in the configs directory.
Change Summary
Adds initial implementation of OpAMP Agent Controller Extension.
Design doc can be found in #3388
Adds:
Currently only works with websocket.
Followups will need to include:
Currently this lives in the controller crate. I wasn't sure where was the right place for this, and hesitated between controller or core-nodes. Happy to move if anyone has feelings/suggestions
What issue does this PR close?
How are these changes tested?
Are there any user-facing changes?
Changelog
.chloggen/*.yamlentrychore(indicated in title)