Skip to content

Create a new metric for the main scheduler loop - #5134

Open
JamesMurkin wants to merge 2 commits into
masterfrom
scheduler_main_loop_metric
Open

Create a new metric for the main scheduler loop#5134
JamesMurkin wants to merge 2 commits into
masterfrom
scheduler_main_loop_metric

Conversation

@JamesMurkin

Copy link
Copy Markdown
Contributor

New metric - 'armada_scheduler_main_loop_cycle_time'

  • Labels type (reconciliation/scheduling) and outcome (success/failure)

This will supersede existing metrics:

  • armada_scheduler_schedule_loop_cycle_times
  • armada_scheduler_schedule_loop_outcome
  • armada_scheduler_reconciliation_loop_cycle_times

The reasons to do this are:

  • Simpler in code
  • No risk of the metric being inconsistent with itself. I.e if we set cycle time but not outcome, they could get out sync
  • Allow more flexible querying
    • Can now see duration of failing cycles separately to successful ones
    • Can get an average of all cycles regardless of type, without needing to manaully merge 2 metrics
  • Less confusing naming. armada_scheduler_schedule_* and armada_scheduler_scheduling_* mean different things but are very similar. Now the main loop is explicitly named that in the metrics, avoiding confusion with the scheduling duration
  • Reconciliation loop didn't have an outcome metric so we couldn't see how often it was failing

If this pattern works, we'll likely decom the old metrics + move other metrics to the same pattern

New metric - 'armada_scheduler_main_loop_cycle_time'
 - Labels type (reconciliation/scheduling) and outcome (success/failure)

This will supersede existing metrics:
 - armada_scheduler_schedule_loop_cycle_times
 - armada_scheduler_schedule_loop_outcome
 - armada_scheduler_reconciliation_loop_cycle_times

The reasons to do this are:
 - Simpler in code
 - No risk of the metric being inconsistent with itself. I.e if we set cycle time but not outcome, they could get out sync
 - Allow more flexible querying
   - Can now see duration of failing cycles separately to successful ones
   - Can get an average of all cycles regardless of type, without needing to manaully merge 2 metrics
 - Less confusing naming. armada_scheduler_schedule_* and armada_scheduler_scheduling_* mean different things but are very similar. Now the main loop is explicitly named that in the metrics, avoiding confusion with the scheduling duration

Signed-off-by: JamesMurkin <jamesmurkin@hotmail.com>
@JamesMurkin
JamesMurkin marked this pull request as ready for review August 27, 2026 10:36
@greptile-apps

greptile-apps Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR adds a unified scheduler main-loop cycle-time histogram labeled by loop type and outcome.

  • Adds scheduling and reconciliation loop-type labels.
  • Records success and failure durations after each scheduler cycle.
  • Adds metric collection and focused histogram tests.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
internal/scheduler/metrics/cycle_metrics.go Adds and exports the labeled main-loop duration histogram and its reporting method.
internal/scheduler/metrics/cycle_metrics_test.go Tests independent observations for each loop-type and outcome combination and updates collection expectations.
internal/scheduler/scheduler.go Reports each completed scheduler cycle using its selected loop type and error-derived outcome.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Scheduler main-loop cycle] --> B{Scheduling attempted?}
    B -->|Yes| C[type=scheduling]
    B -->|No| D[type=reconciliation]
    C --> E{Cycle error?}
    D --> E
    E -->|No| F[outcome=success]
    E -->|Yes| G[outcome=failure]
    F --> H[Observe main_loop_cycle_time]
    G --> H
Loading

Reviews (2): Last reviewed commit: "Merge branch 'master' into scheduler_mai..." | Re-trigger Greptile

@mergify

mergify Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

  • Queue this pull request

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