[AMORO-4313] AIP-5 Phase 5: observability metrics for dynamic allocation - #4314
Conversation
…the backlog gauge Signed-off-by: Jiwon Park <jpark92@outlook.kr>
…ked gauges and scale counters Signed-off-by: Jiwon Park <jpark92@outlook.kr>
…tch lifecycle Signed-off-by: Jiwon Park <jpark92@outlook.kr>
…mizer group metrics Signed-off-by: Jiwon Park <jpark92@outlook.kr>
Signed-off-by: Jiwon Park <jpark92@outlook.kr>
|
Hi @zhoujinsong @xxubai @czy006, AIP-5 Phase 5 (the final phase, observability metrics) is ready for review. It includes the config-invalid gauge and alertable scale events suggested in the design review. CI is green. Thanks! |
zhoujinsong
left a comment
There was a problem hiding this comment.
Looks good — the Source-backed gauge pattern, lifecycle management, and defensive error handling in config_invalid are all well done. No issues with the 7 metrics here; they cover the core "is DRA working?" question well.
A few additional metrics worth considering for follow-up — they'd help operators diagnose failures and spot config limits. None of these block merging.
1. scale_up_failed_total (Counter) — high priority
scale_up_total increments unconditionally before the requestResource/createResource loop. The code already distinguishes success vs failure in the catch block, but only logs it — a scale_up_failed_total incremented there would let operators directly see failing scale-ups without correlating against optimizer_instances (which is ambiguous during the pending-registration window).
2. scale_down_failed_total (Counter)
executeRemoval can fail on container release and retry silently. A failure counter would distinguish "drain stuck because tasks won't finish" from "drain stuck because releases keep failing."
3. at_max_capacity / at_min_capacity (Gauge 0/1)
1 when effectiveThreads >= maxParallelism / <= minParallelism, else 0. Directly answers "is this group maxed/floor-bound?" without operators needing to know the configured limits. Combined with backlog_duration_ms > 0, cleanly identifies "needs manual max-parallelism increase" — currently requires inference from "backlog growing but scale_up_total stopped."
Why are the changes needed?
Close #4313.
Final phase of AIP-5. The scaling behavior from #4272 and #4296 is currently observable only through INFO logs; this exposes it as metrics so operators can alert on it — in particular resource thrashing ("N scale-downs in the last hour",
increase(optimizer_group_scale_down_total[1h])) and the config fail-safe fallback that would otherwise be silent.Brief change log
New per-group metrics (all tagged
group), registered in two places by ownership of their source state:OptimizerGroupMetrics(every group, queue lifecycle):optimizer_group_idle_optimizers(gauge) — instances with zero in-flight (SCHEDULED/ACKED) tasksoptimizer_group_config_invalid(gauge) —1while an opted-in group's DRA config is invalid. Registered for every group because an invalid config makes the group not effectively enabled — tying this gauge to the DRA watch would remove it exactly when it should alarm.DynamicAllocationMetrics(DRA groups only, scale-keeper watch lifecycle):optimizer_group_pending_removal_optimizers(gauge) — instances in graceful drainoptimizer_group_effective_threads(gauge) — registered threads + threads of optimizers pending registration, the raw observable as defined in the AIP (the keeper's round arithmetic additionally subtracts draining instances internally)optimizer_group_backlog_duration_ms(gauge) — duration since demand first exceeded capacity, derived at scrape time (keeper rounds are seconds apart, pushing would go stale between scrapes)optimizer_group_scale_up_total/optimizer_group_scale_down_total(counters) — one attempted scale-out round (regardless of instance count or request outcome) and one drain start each count 1; instance-count trends are already covered byoptimizer_group_optimizer_instances, and a rising counter without rising instances sustained beyond the pod boot window signals failing resource requestsNaming follows the Prometheus/OpenMetrics conventions (
_totalcounter suffix,_msunit suffix) as published in the AIP, rather than the pre-reporter_count/_millssuffixes of the existing table metrics.Metric lifecycle is hardened against the failure paths it introduces: registration failures roll back partially registered metrics so the group stays rewatchable, the keeper's dispose unregisters its metrics (the global registry outlives the service across HA leader hand-offs), and a watch arriving after dispose is a no-op (
stoppedguard, the same pattern asOptimizerGroupKeeper.keepInTouch). Watch/unwatch are serialized, and a disable unwatches on the config-entry path itself — the round-driven unwatch runs on the leader only, so followers would otherwise keep exporting the group's DRA metrics until failover.Supporting changes:
DynamicAllocationState.backlogDurationMs()read accessor,DynamicAllocationConfig.isConfigInvalid()(a group that never opted in is never invalid, whatever its leftover properties parse to), and the metrics table indocs/user-guides/metrics.md.Known trade-offs
increase()/rate()are counter-reset tolerant, so alerts survive this.backlog_duration_msgauge wiring is covered by unit tests on the accessor plus the keeper lambda; an end-to-end assertion under real demand would need the heavy real-table demand harness already declared out of scope in the previous phases.Documents & Website changes
docs/user-guides/metrics.md: 2 rows added to the optimizer group table, plus a dynamic-allocation metrics table with the counter semantics.How was this patch tested?
TestDynamicAllocationMetrics(new): gauge/counter registration, source-backed values, unregistration, rollback of partial registration on failure.TestOptimizerScaleKeeper: metrics registered on watch and removed on disable; a floor round counts one scale-up action; a drain start counts one scale-down action; a failed watch leaves the group rewatchable; dispose unregisters the metrics and a watch arriving after dispose registers nothing; an enabled-to-enabled config update re-enters watch idempotently.TestOptimizingQueue: idle-optimizer gauge flips with an in-flight task; config-invalid gauge flips on an invalid runtime update.TestDynamicAllocationConfig/TestComputeScaleUp:isConfigInvalidandbacklogDurationMsunit coverage.All new code was written test-first; full DRA suite green (184 tests, Phase 1–4 tests unmodified).