Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support multiple agents per WebSocket connection #252

Open
tigrannajaryan opened this issue Feb 21, 2024 · 5 comments
Open

Support multiple agents per WebSocket connection #252

tigrannajaryan opened this issue Feb 21, 2024 · 5 comments

Comments

@tigrannajaryan
Copy link
Member

tigrannajaryan commented Feb 21, 2024

OpAMP spec allows multiple agent instances per WebSocket but opamp-go WebSocket Client implementation currently doesn't.

Use Case

The particular use case I have is to manage 2 agents running side-by-side on the same host using one Supervisor. It is of course possible to use 2 Client instances in the Supervisor, one Client per Agent, each Client using one WebSocket. However WebSockets connections have non-zero cost. When we talk about millions of agents 2x connections become significant from OpAMP Server's point of view. Ideally the Supervisor would use just one WebSocket connection to the Server and still be able to supervise 2 (or more) local agents.

Solution Options

What do we want to do about it?

I see a few options.

Option 1

Change Client API to make it multi-agent. We likely need to add instance_id as a parameter to Client interface methods and to the callbacks.

Cons

  • This will be a significant breaking change of Client API.
  • Also only applicable to WebSocket but will complicate HTTP version unnecessarily.

Option 2

Change Client API to allow instantiating multiple Clients over one WebSocket. This requires separating connection establishing logic. NewWebSocket() can than accept the connection as a parameter. Multiple NewWebSocket() calls over the same connection will be allowed. The connection will be responsible for multiplexing messages to the correct client.

Perhaps we make passing connection optional

Pros

  • A lot less breaking changes to the Client API compared to Option 1. Perhaps doable with zero breaking changes (e.g. introduce NewConnectedWebSocket version).
  • Seems to be a cleaner design with better separation of concerns.
  • Conceptually interesting analogy with what we do on the server-side with Start/Attach.

Cons

  • Possibly requires more work, more code needs to be refactored.

Option 3

Do nothing now. Introduce v2 API later. If necessary can break the API.

@tigrannajaryan tigrannajaryan transferred this issue from open-telemetry/opamp-spec Feb 21, 2024
@tigrannajaryan
Copy link
Member Author

@open-telemetry/opamp-go-approvers @open-telemetry/opamp-go-maintainers please comment.

@andykellr
Copy link
Contributor

I think I need to understand the use case better to form an opinion. I'm leaning toward something like option 2, but I don't think I see the case where you have multiple Client instances representing different Agents that are sending/receiving messages over the same websocket.

The proxy case (an OpAMP gateway) makes a lot of sense to me, but this sounds different than that.

@tigrannajaryan
Copy link
Member Author

@andykellr I added the particular use case I have to the description above.

@andykellr
Copy link
Contributor

Ok, thank you. That makes sense. I'll spend some time thinking about it tomorrow. I'm not too worried about breaking changes to the client at this point, but I think decomposing the transport and the client makes sense and letting the websocket code multiplex between different clients seems appropriate.

@zhanbei1
Copy link

Very good advice, I also need this feature now, I wonder when it can be released

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

No branches or pull requests

3 participants