Skip to content

[DEP-4265] Add depot ci metrics#503

Merged
121watts merged 6 commits intomainfrom
watts/dep-4265-ci-metrics-cli
May 7, 2026
Merged

[DEP-4265] Add depot ci metrics#503
121watts merged 6 commits intomainfrom
watts/dep-4265-ci-metrics-cli

Conversation

@121watts
Copy link
Copy Markdown
Contributor

@121watts 121watts commented May 6, 2026

Summary

depot ci metrics gives humans and scripts the metrics view the app already had.

What was happening

The CLI could inspect CI status and logs, but it had no command for CPU and memory metrics. Customers writing scripts or agents had no clean way to ask for attempt samples or job/run metric summaries.

What happens now

  • Adds depot ci metrics <attempt-id> plus explicit --attempt, --job, and --run selectors.
  • Adds --output text|json, with deterministic snake_case JSON for automation.
  • Prints exact attempt drill-down commands in job/run human output.
  • Maps availability codes to readable JSON strings like available, no_sandbox, no_time_range, and no_samples.
  • Updates the generated CI protobuf/connect client for the new metrics RPCs.

Anything else?

This is the CLI half of DEP-4265 and depends on the API draft: https://github.com/depot/api/pull/3638


Note

Medium Risk
Adds new CLI surface area and new CI RPC wrappers for metrics retrieval; main risk is output/flag behavior changes and handling of large metric responses via new error paths.

Overview
Adds depot ci metrics to fetch CI CPU/memory metrics at the attempt, job, or run level (positional attempt ID or mutually-exclusive --attempt|--job|--run), with --output text|json and deterministic snake_case JSON payloads.

Extends the CI API client wrappers with CIGetJobAttemptMetrics, CIGetJobMetrics, and CIGetRunMetrics, and adds targeted error handling for ResourceExhausted (actionable “narrow your request” guidance) and NotFound (suggests using job/run selectors).

Reviewed by Cursor Bugbot for commit d124c87. Bugbot is set up for automated code reviews on this repo. Configure here.

@linear-code
Copy link
Copy Markdown

linear-code Bot commented May 6, 2026

@121watts 121watts marked this pull request as ready for review May 6, 2026 16:20
Comment thread pkg/cmd/ci/metrics.go
Comment thread pkg/cmd/ci/metrics.go
Comment on lines +35 to +39
Example: ` depot ci metrics att_123
depot ci metrics --attempt att_123 --output json
depot ci metrics --job job_123 --output json
depot ci metrics --run run_123 --output json`,
RunE: func(cmd *cobra.Command, args []string) error {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

will workflowid as a filter come later?

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.

Yes, workflow filtering is a later pass. This CLI change mirrors the current public run/job/attempt metrics RPCs and keeps workflow selection out of this already-large PR. -watts[bot]

Comment thread pkg/cmd/ci/metrics.go
}
}

func metricStatsJSON(stats *civ1.CIMetricsStats) statsJSON {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

metricStatsJSON handles scalar fields with nil-safe getters, but then reads optional pointer fields directly from stats. Since this proto message field is nullable on the Go client, an omitted stats message would panic here instead of emitting an empty stats block. Can we guard stats == nil or normalize it to an empty CIMetricsStats before reading the optional fields?

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.

Yep, fixed in 4f9d544. metricStatsJSON now returns an empty stats object when the message is omitted, so the optional pointer fields are never read through nil. -watts[bot]

Comment thread pkg/cmd/ci/metrics.go
if stats.GetObservedStartedAt() != "" || stats.GetObservedFinishedAt() != "" {
fmt.Printf(" Observed: %s - %s\n", stats.GetObservedStartedAt(), stats.GetObservedFinishedAt())
}
if stats.PeakCpuUtilization != nil || stats.AverageCpuUtilization != nil {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same nil-stats issue on the text path: the code calls nil-safe getters first, but then directly checks stats.PeakCpuUtilization / stats.AverageCpuUtilization. A response with stats omitted will panic while rendering human output. A small helper that returns empty stats for nil would cover both JSON and text formatting.

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.

Fixed in 4f9d544 as well. The text renderer now normalizes missing stats to an empty CIMetricsStats before reading optional CPU or memory pointers. -watts[bot]

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

Bugbot Autofix is ON. A cloud agent has been kicked off to fix the reported issue. You can view the agent here.

Reviewed by Cursor Bugbot for commit 4f9d544. Configure here.

Comment thread pkg/cmd/ci/ci.go
cmd.AddCommand(NewCmdCancel())
cmd.AddCommand(NewCmdDispatch())
cmd.AddCommand(NewCmdLogs())
cmd.AddCommand(NewCmdMetrics())
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

New "metrics" command missing from registration guard test

Low Severity

The new metrics subcommand is registered in NewCmdCI but not added to the wanted slice in TestCICommandRegistration in ci_test.go. That test's comment explicitly says "New verbs should be added to the wanted slice below." Without the entry, a future refactor that accidentally drops cmd.AddCommand(NewCmdMetrics()) won't be caught by the guard test.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 4f9d544. Configure here.

@121watts 121watts merged commit 56044fc into main May 7, 2026
10 checks passed
@121watts 121watts deleted the watts/dep-4265-ci-metrics-cli branch May 7, 2026 12:36
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

Successfully merging this pull request may close these issues.

2 participants