Skip to content

[AMORO-4313] AIP-5 Phase 5: observability metrics for dynamic allocation - #4314

Merged
zhoujinsong merged 6 commits into
apache:masterfrom
j1wonpark:dra-phase5-metrics
Aug 17, 2026
Merged

[AMORO-4313] AIP-5 Phase 5: observability metrics for dynamic allocation#4314
zhoujinsong merged 6 commits into
apache:masterfrom
j1wonpark:dra-phase5-metrics

Conversation

@j1wonpark

Copy link
Copy Markdown
Contributor

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) tasks
    • optimizer_group_config_invalid (gauge) — 1 while 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.
  • New DynamicAllocationMetrics (DRA groups only, scale-keeper watch lifecycle):
    • optimizer_group_pending_removal_optimizers (gauge) — instances in graceful drain
    • optimizer_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 by optimizer_group_optimizer_instances, and a rising counter without rising instances sustained beyond the pod boot window signals failing resource requests

Naming follows the Prometheus/OpenMetrics conventions (_total counter suffix, _ms unit suffix) as published in the AIP, rather than the pre-reporter _count/_mills suffixes 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 (stopped guard, the same pattern as OptimizerGroupKeeper.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 in docs/user-guides/metrics.md.

Known trade-offs

  • Disabling and re-enabling dynamic allocation re-registers the keeper-scoped metrics, resetting the counters to 0. Prometheus increase()/rate() are counter-reset tolerant, so alerts survive this.
  • On HA standby nodes the gauges are registered (as the existing group metrics are) but the counters stay 0, since only the leader evaluates scaling.
  • The backlog_duration_ms gauge 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: isConfigInvalid and backlogDurationMs unit coverage.

All new code was written test-first; full DRA suite green (184 tests, Phase 1–4 tests unmodified).

…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>
@github-actions github-actions Bot added type:docs Improvements or additions to documentation module:ams-server Ams server module labels Aug 13, 2026
@j1wonpark
j1wonpark marked this pull request as ready for review August 14, 2026 04:38
@j1wonpark

Copy link
Copy Markdown
Contributor Author

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 zhoujinsong left a comment

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.

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."

@zhoujinsong
zhoujinsong merged commit 56c34f6 into apache:master Aug 17, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

module:ams-server Ams server module type:docs Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Subtask]: AIP-5 Phase 5 — Observability metrics for dynamic allocation

2 participants