[Serve] Instrument ray_serve_replica_state metric

[petern48]

Description

Instrument ray_serve_replica_state, which will enable us to observe "Replica Status Over Time."

Related issues

Fixes #59699

Additional information

It emits the existing ReplicaStates with the following int values as a gauge.

ray/python/ray/serve/_private/common.py

Lines 185 to 191 in b3ce651

	class ReplicaState(str, Enum):
	STARTING = "STARTING"
	UPDATING = "UPDATING"
	RECOVERING = "RECOVERING"
	RUNNING = "RUNNING"
	STOPPING = "STOPPING"
	PENDING_MIGRATION = "PENDING_MIGRATION"

        1=STARTING, 2=UPDATING, 3=RECOVERING, 4=RUNNING,
        5=STOPPING, 6=PENDING_MIGRATION.

[gemini-code-assist]

Code Review

This pull request introduces a new metric, "serve_replica_state", to track the lifecycle states of Ray Serve deployment replicas. It adds a numeric mapping for states and updates the controller to report these values via a gauge. Feedback focuses on a performance regression where the "is_new_replica" check in "ReplicaStateContainer.add" triggers redundant callbacks on every controller tick due to the internal "pop"-and-"add" pattern. This leads to excessive metric updates and prevents the controller from reaching a steady state. It is recommended to implement a state cache to guard against these redundant calls and ensure the "_in_transition" flag is only set during actual state transitions.

[abrarsheikh]

instead of gating each replica with RAY_SERVE_STATUS_GAUGE_REPORT_INTERVAL_S, i think we should gate this entire block. looping over all replicas is not cheap. Better to short circuit before the loop

[petern48]

Implemented in 3c94770. If we do this, we can no longer short-circuit on if value_changed (emitting the metric before the interval has completed) on a per-replica basis. Any replica state changes could wait up to RAY_SERVE_STATUS_GAUGE_REPORT_INTERVAL_S (10 seconds) before the metric reflects the change. Double-checking, is that really reasonable?

Also dropped the per-replica cache in that commit bc interval_elapsed is essentially always true once we get inside the for loop now, making it pointless to check whether the value changed.

[abrarsheikh]

we can revisit this later

[abrarsheikh]

nice, good attention to detail :)

[abrarsheikh]

i think as a follow up PR we should set RAY_SERVE_STATUS_GAUGE_REPORT_INTERVAL_S=60, 10s is too small and can hurt controller perf

[petern48]

@abrarsheikh Could we add the go label to the PR, since you approved?

[petern48]

CI's passing now

[abrarsheikh]

i have recently learnt about some degradation caused by an explosion in metrics cardinality in the head node. Until that is resolve, let's put this PR on hold. See #63114

[github-actions]

This pull request has been automatically marked as stale because it has not had
any activity for 14 days. It will be closed in another 14 days if no further activity occurs.
Thank you for your contributions.

You can always ask for help on our discussion forum or Ray's public slack channel.

If you'd like to keep this open, just leave any comment, and the stale label will be removed.

[cursor]

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

^{Reviewed by Cursor Bugbot for commit 3a63b4b. Configure here.}

[cursor]

Empty refresh defers replica metrics

Medium Severity

The periodic ray_serve_replica_state export always updates _last_replica_state_gauge_refresh_time after the loop, even when self._replicas.get() is empty. Because check_and_update_replicas() runs before scaling in the same controller tick, a new deployment can start the interval with no replicas and skip reporting until up to RAY_SERVE_STATUS_GAUGE_REPORT_INTERVAL_S after replicas exist.

 

^{Reviewed by Cursor Bugbot for commit 3a63b4b. Configure here.}