Fix job execution duration when runner assign time is not set#4472
Fix job execution duration when runner assign time is not set#4472nikola-jokic wants to merge 2 commits intomasterfrom
Conversation
nikola-jokic
requested review from
a team,
Steve-Glass,
mumoshu,
rentziass and
toast-gear
as code owners
nikola-jokic
added
the
gha-runner-scale-set
|
Hello! Thank you for your contribution. Please review our contribution guidelines to understand the project's testing and code conventions. |
There was a problem hiding this comment.
Pull request overview
Fixes an edge case in gha_job_execution_duration_seconds where jobs completed before runner assignment could yield an epoch-sized duration.
Changes:
- Add a
RunnerAssignTime.IsZero()check when computing job execution duration. - Record an execution duration of
0when the runner was never assigned.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| var executionDuration int64 | ||
| if !msg.RunnerAssignTime.IsZero() { | ||
| executionDuration = msg.FinishTime.Unix() - msg.RunnerAssignTime.Unix() | ||
| } | ||
|
|
||
| e.observeHistogram(MetricJobExecutionDurationSeconds, l, float64(executionDuration)) |
There was a problem hiding this comment.
This change fixes a production-facing metric edge case, but there are currently no unit tests covering RecordJobCompleted. Please add a test for the zero RunnerAssignTime scenario to ensure we don’t regress back to recording an epoch-sized duration (and to lock in the intended behavior: either skipping the observation or recording a specific value).
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
nikola-jokic
force-pushed
the
nikola-jokic/no-runner-assigned-time-job-completed
branch
from
83f6619 to
74bce34
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Fixes #4444
Fixes #3731