Skip to content

feat: rollout total duration tracking#4774

Open
agaudreault wants to merge 50 commits into
argoproj:masterfrom
agaudreault:total-rollout-time
Open

feat: rollout total duration tracking#4774
agaudreault wants to merge 50 commits into
argoproj:masterfrom
agaudreault:total-rollout-time

Conversation

@agaudreault

@agaudreault agaudreault commented Jun 10, 2026

Copy link
Copy Markdown
Member

Requires #4785

Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>
Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>
Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>
Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>
Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>
Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>
Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>
Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>
Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>
Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>
Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>
Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>
Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>
Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>
@github-actions

github-actions Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Published E2E Test Results

  4 files    4 suites   4h 11m 30s ⏱️
148 tests 134 ✅  7 💤 7 ❌
600 runs  564 ✅ 28 💤 8 ❌

For more details on these failures, see this check.

Results for commit 2851889.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Published Unit Test Results

2 553 tests   2 553 ✅  3m 24s ⏱️
  131 suites      0 💤
    1 files        0 ❌

Results for commit 2851889.

♻️ This comment has been updated with latest results.

Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>
Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>
Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>
Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>
Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>
Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>
Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>
Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>
Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>
Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>
Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>
Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>
Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>
Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>
Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>
Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>
Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>
Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>
Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>
Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>
Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>
Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>
Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>
Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>
@codecov

codecov Bot commented Jun 13, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.53465% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.03%. Comparing base (d1c2967) to head (2851889).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
rollout/sync.go 96.24% 3 Missing and 2 partials ⚠️
controller/metrics/metrics.go 95.65% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4774      +/-   ##
==========================================
- Coverage   85.09%   85.03%   -0.06%     
==========================================
  Files         166      166              
  Lines       19136    19315     +179     
==========================================
+ Hits        16283    16424     +141     
- Misses       2005     2024      +19     
- Partials      848      867      +19     
Flag Coverage Δ
e2e 53.21% <94.05%> (+0.24%) ⬆️
unit-tests 81.47% <92.07%> (+0.07%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>
Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>
Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>
Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>
Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>
Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>
Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>
Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>
@agaudreault agaudreault marked this pull request as ready for review July 2, 2026 17:21
@agaudreault agaudreault requested review from a team as code owners July 2, 2026 17:21
Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>
@zachaller zachaller added this to the v1.11 milestone Jul 9, 2026
Comment thread rollout/sync.go
completionStatus = v1alpha1.CompletionStatusPromoted
}
durationStatus.CompleteRollout(now, completionStatus)
c.metricsServer.EmitRolloutDuration(durationStatus)

@zachaller zachaller Jul 9, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

EmitRolloutDuration runs before the status patch in persistRolloutStatus / patchCondition. On conflict/retry, FinishedAt is still unset and the same completion is observed again.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I am not sure there is a way around this. The 2 operations cannot be atomic. I prefer doing the status patch at the absolute end then emitting metrics.

In an ideal scenario, the controller would eventually RetryOnConflict on the status update, but that seems out of scope for this PR since it could have side effects.

Maybe what this code could do is return a callback function to do the metric(s) emission, and this callback is performed after the Patch? It does add a little complexity, but wouldn't affect the logic too much. Wdyt?

Comment thread controller/metrics/metrics.go Outdated
prometheus.HistogramOpts{
Name: "rollout_duration_seconds_progression",
Help: "Active progression time for a rollout (excluding manual pause time)",
Buckets: []float64{30, 60, 120, 300, 600, 900, 1800, 3600},

@zachaller zachaller Jul 9, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Progression buckets stop at 3600s; total goes to 28800s. A long non-paused rollout still has useful total resolution but only hits +Inf for progression.

@agaudreault agaudreault Jul 10, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I did set progression much lower since it does not contains the manually paused steps, which can be very long.

I feel that there are not much difference investigating rollouts that took more than 1h compared to rollouts that took more than 2h+ of active progression time.

Maybe in the future, we can track the active/automated paused time outside this metric or with a label.

Comment thread rollout/sync.go
// CompletionStatusSuperseded indicates the rollout was superseded by a new rollout
CompletionStatusSuperseded CompletionStatus = "superseded"
// CompletionStatusRollbacked indicates the rollout was explicitly rolled back to a previous version
CompletionStatusRollbacked CompletionStatus = "rollbacked"

@zachaller zachaller Jul 9, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

rollbacked / fast-rollbacked are non-standard English. Renaming after release breaks the metrics label API.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

What would be the proper terms for Rollout?

Comment thread pkg/apis/rollouts/v1alpha1/types.go Outdated
Comment thread rollout/sync.go Outdated
Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>
Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>
@sonarqubecloud

Copy link
Copy Markdown

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