Skip to content

feat: add agent timings#14713

Merged
DanielleMaywood merged 57 commits into
mainfrom
dm-add-agent-timings
Sep 24, 2024
Merged

feat: add agent timings#14713
DanielleMaywood merged 57 commits into
mainfrom
dm-add-agent-timings

Conversation

@DanielleMaywood
Copy link
Copy Markdown
Contributor

@DanielleMaywood DanielleMaywood commented Sep 18, 2024

Closes #14630

Adds timing information for workspace agent scripts.

This change adds:

  • display_name, id columns to workspace_agent_scripts table
  • workspace_agent_script_timings table

Demo

Extend the default docker template with the following scripts.

resource "coder_script" "say_hi" {
  agent_id           = coder_agent.main.id
  display_name       = "say hi"
  run_on_start       = true
  start_blocks_login = true

  script = <<EOF
    #!/bin/sh

    echo "Hi!"
  EOF
}

resource "coder_script" "say_cron" {
  agent_id     = coder_agent.main.id
  display_name = "say cron"
  cron         = "*/5 * * * *"

  script = <<EOF
    #!/bin/sh

    echo "Cron!"
  EOF
}

resource "coder_script" "say_bye" {
  agent_id     = coder_agent.main.id
  display_name = "say bye"
  run_on_stop  = true

  script = <<EOF
    #!/bin/sh

    echo "Bye!"
  EOF
}

resource "coder_script" "timeout" {
  agent_id           = coder_agent.main.id
  display_name       = "timeout"
  run_on_start       = true
  start_blocks_login = true
  timeout            = 1

  script = <<EOF
    #!/bin/sh

    sleep 10
  EOF
}

resource "coder_script" "exit_1" {
  agent_id           = coder_agent.main.id
  display_name       = "exit 1"
  run_on_start       = true
  start_blocks_login = true

  script = <<EOF
    #!/bin/sh

    exit 1
  EOF
}

Create a workspace, start it, then stop it. This then gives us the following data in our database.

coder=# select * from workspace_agent_script_timings;

              script_id               |          started_at           |           ended_at            | exit_code | stage |    status
--------------------------------------+-------------------------------+-------------------------------+-----------+-------+--------------
 b3887bc1-3d0b-4eb1-bafb-bc4af3dc19eb | 2024-09-23 16:18:18.370221+00 | 2024-09-23 16:18:18.372466+00 |         0 | start | ok
 1ed547c8-65af-43fd-aa72-47d8866c475c | 2024-09-23 16:18:18.372342+00 | 2024-09-23 16:18:18.373628+00 |         1 | start | exit_failure
 9b8f3503-dd25-4f31-9c47-e715336b9cca | 2024-09-23 16:18:18.372369+00 | 2024-09-23 16:18:19.371824+00 |       255 | start | timed_out
 6e75ef8a-7957-41c7-8cf8-622ed2998c35 | 2024-09-23 16:18:18.371779+00 | 2024-09-23 16:18:21.862226+00 |         0 | start | ok
 11238666-8b11-476b-a6d2-110be27acc9d | 2024-09-23 16:18:25.001315+00 | 2024-09-23 16:18:25.002812+00 |         0 | cron  | ok
 a985fc12-cf2a-49a9-ac65-1c3f98d4bea9 | 2024-09-23 16:18:28.861707+00 | 2024-09-23 16:18:28.862898+00 |         0 | stop  | ok

Copy link
Copy Markdown
Contributor

@dannykopping dannykopping left a comment

Choose a reason for hiding this comment

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

Excellent work!

Comment thread agent/agentscripts/agentscripts.go Outdated
Comment thread agent/agentscripts/agentscripts.go Outdated
Comment thread agent/proto/agent.proto
Comment thread coderd/agentapi/scripts.go Outdated
Comment thread coderd/agentapi/scripts.go Outdated
Comment thread coderd/provisionerdserver/provisionerdserver.go
Comment thread agent/agentscripts/agentscripts.go Outdated
Comment thread agent/agentscripts/agentscripts_test.go
Comment thread coderd/database/dbauthz/dbauthz_test.go Outdated
Comment thread coderd/database/dbmem/dbmem.go Outdated
Comment thread agent/agentscripts/agentscripts_test.go Outdated
Comment thread agent/agenttest/client.go Outdated
Comment thread agent/proto/agent_drpc.pb.go
Comment thread cli/organizationsettings.go Outdated
Comment thread coderd/database/dump.sql Outdated
Copy link
Copy Markdown
Member

@mafredri mafredri left a comment

Choose a reason for hiding this comment

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

Nice job piping this through! And sorry for the amount of comments. 😅

Comment thread agent/agentscripts/agentscripts.go
Comment thread agent/agentscripts/agentscripts.go
Comment thread agent/agentscripts/agentscripts.go Outdated
Comment thread agent/agentscripts/agentscripts.go Outdated
Comment thread agent/proto/agent.proto
enum Stage {
START = 0;
STOP = 1;
CRON = 2;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I feel there's a slight mismatch in naming here as I have trouble fitting cron under stage, but I also don't have a better suggestion 😅, just an observation.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah it does feel a little strange. Couldn't think of a better way of handling this though.

'cron'
);

CREATE TABLE workspace_agent_script_timings
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think calling this timings could work but I feel it's primarily a exec_status or exec_result table, it contains timing information which feels like more of a side-effect.

Renaming it would give us more flexibility to store more data in it if needed.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Renaming a table isn't too difficult; we could do this as a follow-up if needed?

Comment thread coderd/database/migrations/000256_workspace_agent_script_timings.up.sql Outdated
Comment thread coderd/database/queries/workspaceagents.sql Outdated
Comment thread coderd/database/queries/workspacescripts.sql Outdated
Comment on lines +1 to +2
DROP TYPE IF EXISTS workspace_agent_script_timing_status CASCADE;
DROP TYPE IF EXISTS workspace_agent_script_timing_stage CASCADE;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

note: DROP TYPE ... CASCADE is already done for 000246_provisioner_job_timings

Copy link
Copy Markdown
Member

@johnstcn johnstcn left a comment

Choose a reason for hiding this comment

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

I don't have any further blocking comments.

Copy link
Copy Markdown
Member

@mafredri mafredri left a comment

Choose a reason for hiding this comment

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

Looking great, thanks for your hard work on this PR!

Comment thread agent/agentscripts/agentscripts.go Outdated
Comment thread coderd/database/migrations/000257_workspace_agent_script_timings.up.sql Outdated
Comment thread coderd/database/migrations/000257_workspace_agent_script_timings.up.sql Outdated
@DanielleMaywood DanielleMaywood merged commit ae522c5 into main Sep 24, 2024
@DanielleMaywood DanielleMaywood deleted the dm-add-agent-timings branch September 24, 2024 09:51
@github-actions github-actions Bot locked and limited conversation to collaborators Sep 24, 2024
Copy link
Copy Markdown
Contributor

@dannykopping dannykopping left a comment

Choose a reason for hiding this comment

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

Post-merge approval, excellent work! ✅

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add agent script timings

4 participants