-
Notifications
You must be signed in to change notification settings - Fork 1.1k
feat: add initial API for boundary log forwarding to coderd #21293
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
base: main
Are you sure you want to change the base?
Conversation
| rpc CreateSubAgent(CreateSubAgentRequest) returns (CreateSubAgentResponse); | ||
| rpc DeleteSubAgent(DeleteSubAgentRequest) returns (DeleteSubAgentResponse); | ||
| rpc ListSubAgents(ListSubAgentsRequest) returns (ListSubAgentsResponse); | ||
| rpc ReportBoundaryLogs(ReportBoundaryLogsRequest) returns (ReportBoundaryLogsResponse); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any opinions on this RPC name?
go.mod
Outdated
| // by boundary, which lives in a separate repository. | ||
| require github.com/coder/coder/v2/agent/proto/boundary_logs v0.0.0-incompatible | ||
|
|
||
| replace github.com/coder/coder/v2/agent/proto/boundary_logs => ./agent/proto/boundary_logs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand why this is needed and the comment is not really helping.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was a essentially the way to use a nested Go module in a way that doesn't need to refer to a specific version of said module. I was planning to import the nested module in boundary.
I nuked this approach in favor of keeping AgentAPI independent, and separately defining the boundary <-> agent API (future PRs). I realized there’s little benefit from tightly coupling everything now other than re-using generated types. cc @johnstcn
johnstcn
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Proto changes look OK to me but I'm less certain about having them in the coder/coder repo. What would you think about inverting the direction of the import?
effad7c to
92de60c
Compare
Add the API changes to support the feature that transmits boundary logs from workspaces to coderd via the agent API, then re-emits them to stderr. Architecture: - Boundary process connects to Unix socket - Boundary batches logs and sends TLV prefixed protobuf messages to Agent - Agent proxies messages to coderd via DRPC - coderd re-emits to stderr
92de60c to
9635d1c
Compare
Add the API changes to support the feature that transmits boundary logs from workspaces to coderd via the agent API for eventual re-emission to stderr. The API handlers are stubs for now because I'm trying to land this feature from multiple smaller PRs.
High level architecture:
RFC: https://www.notion.so/coderhq/Agent-Boundary-Logs-2afd579be59280f29629fc9823ac41ba