Skip to content

feat: add agent metadata #6614

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 85 commits into from
Mar 31, 2023
Merged

feat: add agent metadata #6614

merged 85 commits into from
Mar 31, 2023

Conversation

ammario
Copy link
Member

@ammario ammario commented Mar 15, 2023 •
edited
Loading

Resolves #3480
Screen Shot 2023-03-27 at 2 34 46 PM

resource "coder_agent" "main" {
  ...
  metadata {
    display_name = "Process Count"
    key  = "process_count"
    script = "ps aux | wc -l"
    interval = 1
    timeout = 1
  }

  metadata {
    display_name = "Load Average"
    key  = "load"
    script = "awk '{print $1}' /proc/loadavg"
    interval = 1
    timeout = 1
  }

  metadata {
    display_name = "Disk Usage"
    key  = "disk"
    script = "df -h | awk '$6 ~ /^\\/$/ { print $5 }'"
    interval = 1
    timeout = 1
  }
}

Now

  • Publish new version of our terraform provider
  • Create endpoint for streaming metadata updates
  • Write basic, acceptable frontend
  • Show errors correctly in the frontend
  • Write stories for the frontend!
  • Get CollectedAt on the server and send age in seconds to avoid issues caused by time drift between the agent, server, and browser. Also this protects the integrity of the system if coder is ran close to a black hole.
  • Write docs
  • Mark as alpha

Later

  • Update example templates to showcase dynamic metadata
  • Allow showing "expanded view" of metadata of everything passed the newline

@ammario ammario self-assigned this Mar 15, 2023
ammario added 21 commits March 15, 2023 16:12
@matifali
Copy link
Member

What do you mean by dynamic metadata?
For example, indexing the 'options' map can show the currently selected coder_parameter.
Is it something like changing inside the workspace like any ENV_VARIABLE?

@ammario
Copy link
Member Author

ammario commented Mar 23, 2023

@matifali — see screenshot I added to original post.

@ammario ammario requested a review from kylecarbs March 28, 2023 21:02
kylecarbs
kylecarbs requested changes Mar 29, 2023
View reviewed changes
agent/agent.go
Listen(ctx context.Context) (net.Conn, error)
ReportStats(ctx context.Context, log slog.Logger, statsChan <-chan *agentsdk.Stats, setInterval func(time.Duration)) (io.Closer, error)
PostLifecycle(ctx context.Context, state agentsdk.PostLifecycleRequest) error
PostAppHealth(ctx context.Context, req agentsdk.PostAppHealthsRequest) error
PostStartup(ctx context.Context, req agentsdk.PostStartupRequest) error
PostMetadata(ctx context.Context, key string, req agentsdk.PostMetadataRequest) error
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If someone has a lot of metadata, I could see us wanting to debounce some of the requests... e.g. 5 things polling each second * 1000 workspaces = a lot of requests

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I considered this but didn't want to pursue optimizing it immediately because the feature is very optional and there's a clear path to adding a WebSocket endpoint to improve performance when the time comes.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A WebSocket endpoint would add substantial complexity (making this harder to review...) and bug risk.

agent/agent.go
if md.Interval == 0 {
continue
}
if collectedAt.Add(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is this possible?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See new comment

@ammario ammario requested a review from kylecarbs March 30, 2023 23:18
@ammario
Copy link
Member Author

ammario commented Mar 30, 2023

@kylecarbs — made some minor changes, and fixed one concurrency bug... you should take another look at the short diff.

kylecarbs
kylecarbs approved these changes Mar 31, 2023
View reviewed changes
agent/agent.go
// We send the result to the channel in the goroutine to avoid
// sending the same result multiple times. So, we don't care about
// the return values.
flight.DoChan(md.Key, func() (interface{}, error) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why use DoChan instead of Do?

Copy link
Member Author

@ammario ammario Mar 31, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's to not block the loop, and a bit cleaner than wrapping the whole thing in a goroutine.

@ammario ammario merged commit ca4fa81 into main Mar 31, 2023
@ammario ammario deleted the agent-metadata branch March 31, 2023 20:26
@github-actions github-actions bot locked and limited conversation to collaborators Mar 31, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@coadler coadler coadler left review comments

@kylecarbs kylecarbs kylecarbs approved these changes

Assignees

@ammario ammario

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Agent Metadata
4 participants
@ammario @matifali @coadler @kylecarbs