[core][metric] Redefine gcs STATS using metric interface by can-anyscale · Pull Request #56201 · ray-project/ray

can-anyscale · 2025-09-03T18:27:06Z

This PR is in the series of unifying all metric definition infra.

This PR migrates all GCS metrics to use the metric interface. It does that by creating the metric object inside gcs_server and pass them down as interfaces to sub-components.

Purely refactoring code and repetitive patterns, easier to review than the number of file changed tells you.

Test:

CI

Note

Refactors GCS and core worker to use injected MetricInterface objects for all metrics, adding new metric helpers and rewiring constructors, server startup, storage client, and tests accordingly.

Metrics Infrastructure:
- Introduce metric helpers in src/ray/common/metrics.h and src/ray/gcs/metrics.h (gauges/histograms/counters for actors, jobs, placement groups, task events, and GCS storage).
- Replace direct stats usage with MetricInterface across GCS and core worker; rename helpers (e.g., GetTaskMetric -> GetTaskByStateGaugeMetric, GetRayEventRecorderDroppedEventsMetric -> GetRayEventRecorderDroppedEventsCounterMetric).
GCS Server Refactor:
- GcsServer now constructs/accepts metric instances and passes them to subcomponents via Start/DoStart and init methods.
- GcsActorManager, GcsJobManager, GcsPlacementGroupManager, and GcsTaskManager constructors updated to receive and record via MetricInterface.
- ObservableStoreClient wraps delegate and records storage metrics via injected interfaces.
Core Worker:
- TaskCounter and CoreWorker updated to use task/actor state gauges via injected MetricInterface.
Tests/Mocks/Build:
- Update mocks and tests to use FakeGauge/FakeCounter/FakeHistogram; validate metric tags/values.
- Add Bazel targets/deps for new metric headers and fakes; minor BUILD wiring adjustments.

^{Written by Cursor Bugbot for commit bd5ff5a. This will update automatically on new commits. Configure here.}

src/ray/gcs/gcs_server/gcs_server_main.cc

ZacAttack · 2025-09-24T16:58:04Z

src/ray/common/metrics.h

+inline ray::stats::Gauge GetActorMetric() {
+  /// Tracks actors by state, including pending, running, and idle actors.
+  ///
+  /// To avoid metric collection conflicts between components reporting on the same actor,


I'm a little confused by this. Right now we have component labels for things like raylet and gcs and we have a Name label for a metric name. Now we're adding a source label. Why would two components necessarily conflict in the current set up? Are they within the same component? I'm unclear why we need an additional label now.

Good point about Source vs. Component. I’m also not sure of the original purpose of each, these tags predate both my time and this PR (which is purely a refactoring and doesn’t add the Source tag). I’m open to revisiting or merging the two tags in a follow-up PR.

ZacAttack · 2025-09-24T17:06:35Z

src/ray/core_worker/core_worker.cc


-TaskCounter::TaskCounter(ray::observability::MetricInterface &task_by_state_counter)
-    : task_by_state_counter_(task_by_state_counter) {
+TaskCounter::TaskCounter(ray::observability::MetricInterface &task_by_state_gauge,


Maybe we should change the name of the type since it's now a gauge?

The internal Prometheus metric has always been a gauge, while the wrapper has always been TaskCounter. I fixed the naming in this PR from task_by_state_counter to task_by_state_gauge to correct a regression I introduced earlier, but underneath, it has always been a gauge. I can see the merit in naming the wrapper Counter, since the concept of a gauge might feel unfamiliar or like an implementation detail, at least to me. But I’m open to using either name for the wrapper, though elsewhere in the codebase Gauge and Counter are used interchangeably for wrapper names (like in [1]).

can-anyscale · 2025-09-24T18:22:22Z

src/ray/gcs/gcs_task_manager.cc

  auto counters = stats_counter_.GetAll();
-  ray::stats::STATS_gcs_task_manager_task_events_reported.Record(
-      counters[kTotalNumTaskEventsReported]);
+  task_events_reported_gauge_.Record(counters[kTotalNumTaskEventsReported]);


can-anyscale · 2025-10-03T19:47:44Z

@jjyao's comments

jjyao · 2025-10-03T20:30:39Z

src/ray/gcs/gcs_server.cc

+      metrics_.placement_group_gauge,
+      metrics_.placement_group_creation_latency_in_ms_histogram,
+      metrics_.placement_group_scheduling_latency_in_ms_histogram,
+      metrics_.placement_group_count_gauge);


We can pass in the entire GcsSeverMetric and let the impl unpacks it.

i just pass in what's needed so the subcomponent doesn't need to be rebuilt when a new metric is added

I mean something like

void InitGcsXXXManager(GcsServerMetric& metrics) { GcsXXXManager(metrics.a, metrics.b, metrics.c); }

jjyao · 2025-10-03T20:35:45Z

src/ray/gcs/metrics.h

+namespace ray {
+namespace gcs {
+
+struct GcsServerMetrics {


nit: ideally we should let this struct own all the metrics by strong them as unique_ptr<MetricInterface>

sure let's do that

The code gets messy when I try to initialize these metrics using unique_ptr, so I'll revert to having gcs_server_main own everything and pass them down by reference instead.

Signed-off-by: Cuong Nguyen <can@anyscale.com>

…#56201) This PR is in the series of unifying all metric definition infra. This PR migrates all GCS metrics to use the metric interface. It does that by creating the metric object inside gcs_server and pass them down as interfaces to sub-components. Purely refactoring code and repetitive patterns, easier to review than the number of file changed tells you. Test: - CI  --- > [!NOTE] > Refactors GCS and core worker to use injected MetricInterface objects for all metrics, adding new metric helpers and rewiring constructors, server startup, storage client, and tests accordingly. > > - **Metrics Infrastructure**: > - Introduce metric helpers in `src/ray/common/metrics.h` and `src/ray/gcs/metrics.h` (gauges/histograms/counters for actors, jobs, placement groups, task events, and GCS storage). > - Replace direct `stats` usage with `MetricInterface` across GCS and core worker; rename helpers (e.g., `GetTaskMetric` -> `GetTaskByStateGaugeMetric`, `GetRayEventRecorderDroppedEventsMetric` -> `GetRayEventRecorderDroppedEventsCounterMetric`). > - **GCS Server Refactor**: > - `GcsServer` now constructs/accepts metric instances and passes them to subcomponents via `Start`/`DoStart` and init methods. > - `GcsActorManager`, `GcsJobManager`, `GcsPlacementGroupManager`, and `GcsTaskManager` constructors updated to receive and record via `MetricInterface`. > - `ObservableStoreClient` wraps delegate and records storage metrics via injected interfaces. > - **Core Worker**: > - `TaskCounter` and `CoreWorker` updated to use task/actor state gauges via injected `MetricInterface`. > - **Tests/Mocks/Build**: > - Update mocks and tests to use `FakeGauge`/`FakeCounter`/`FakeHistogram`; validate metric tags/values. > - Add Bazel targets/deps for new metric headers and fakes; minor BUILD wiring adjustments. > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit bd5ff5a. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).  --------- Signed-off-by: Cuong Nguyen <can@anyscale.com> Signed-off-by: Douglas Strodtman <douglas@anyscale.com>

…#56201) This PR is in the series of unifying all metric definition infra. This PR migrates all GCS metrics to use the metric interface. It does that by creating the metric object inside gcs_server and pass them down as interfaces to sub-components. Purely refactoring code and repetitive patterns, easier to review than the number of file changed tells you. Test: - CI  --- > [!NOTE] > Refactors GCS and core worker to use injected MetricInterface objects for all metrics, adding new metric helpers and rewiring constructors, server startup, storage client, and tests accordingly. > > - **Metrics Infrastructure**: > - Introduce metric helpers in `src/ray/common/metrics.h` and `src/ray/gcs/metrics.h` (gauges/histograms/counters for actors, jobs, placement groups, task events, and GCS storage). > - Replace direct `stats` usage with `MetricInterface` across GCS and core worker; rename helpers (e.g., `GetTaskMetric` -> `GetTaskByStateGaugeMetric`, `GetRayEventRecorderDroppedEventsMetric` -> `GetRayEventRecorderDroppedEventsCounterMetric`). > - **GCS Server Refactor**: > - `GcsServer` now constructs/accepts metric instances and passes them to subcomponents via `Start`/`DoStart` and init methods. > - `GcsActorManager`, `GcsJobManager`, `GcsPlacementGroupManager`, and `GcsTaskManager` constructors updated to receive and record via `MetricInterface`. > - `ObservableStoreClient` wraps delegate and records storage metrics via injected interfaces. > - **Core Worker**: > - `TaskCounter` and `CoreWorker` updated to use task/actor state gauges via injected `MetricInterface`. > - **Tests/Mocks/Build**: > - Update mocks and tests to use `FakeGauge`/`FakeCounter`/`FakeHistogram`; validate metric tags/values. > - Add Bazel targets/deps for new metric headers and fakes; minor BUILD wiring adjustments. > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit bd5ff5a. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).  --------- Signed-off-by: Cuong Nguyen <can@anyscale.com> Signed-off-by: Seiji Eicher <seiji@anyscale.com>

…#56201) This PR is in the series of unifying all metric definition infra. This PR migrates all GCS metrics to use the metric interface. It does that by creating the metric object inside gcs_server and pass them down as interfaces to sub-components. Purely refactoring code and repetitive patterns, easier to review than the number of file changed tells you. Test: - CI  --- > [!NOTE] > Refactors GCS and core worker to use injected MetricInterface objects for all metrics, adding new metric helpers and rewiring constructors, server startup, storage client, and tests accordingly. > > - **Metrics Infrastructure**: > - Introduce metric helpers in `src/ray/common/metrics.h` and `src/ray/gcs/metrics.h` (gauges/histograms/counters for actors, jobs, placement groups, task events, and GCS storage). > - Replace direct `stats` usage with `MetricInterface` across GCS and core worker; rename helpers (e.g., `GetTaskMetric` -> `GetTaskByStateGaugeMetric`, `GetRayEventRecorderDroppedEventsMetric` -> `GetRayEventRecorderDroppedEventsCounterMetric`). > - **GCS Server Refactor**: > - `GcsServer` now constructs/accepts metric instances and passes them to subcomponents via `Start`/`DoStart` and init methods. > - `GcsActorManager`, `GcsJobManager`, `GcsPlacementGroupManager`, and `GcsTaskManager` constructors updated to receive and record via `MetricInterface`. > - `ObservableStoreClient` wraps delegate and records storage metrics via injected interfaces. > - **Core Worker**: > - `TaskCounter` and `CoreWorker` updated to use task/actor state gauges via injected `MetricInterface`. > - **Tests/Mocks/Build**: > - Update mocks and tests to use `FakeGauge`/`FakeCounter`/`FakeHistogram`; validate metric tags/values. > - Add Bazel targets/deps for new metric headers and fakes; minor BUILD wiring adjustments. > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit bd5ff5a. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).  --------- Signed-off-by: Cuong Nguyen <can@anyscale.com>

)" This reverts commit 534b0e4.

…7248) Reverts #56201

…#56201) This PR is in the series of unifying all metric definition infra. This PR migrates all GCS metrics to use the metric interface. It does that by creating the metric object inside gcs_server and pass them down as interfaces to sub-components. Purely refactoring code and repetitive patterns, easier to review than the number of file changed tells you. Test: - CI  --- > [!NOTE] > Refactors GCS and core worker to use injected MetricInterface objects for all metrics, adding new metric helpers and rewiring constructors, server startup, storage client, and tests accordingly. > > - **Metrics Infrastructure**: > - Introduce metric helpers in `src/ray/common/metrics.h` and `src/ray/gcs/metrics.h` (gauges/histograms/counters for actors, jobs, placement groups, task events, and GCS storage). > - Replace direct `stats` usage with `MetricInterface` across GCS and core worker; rename helpers (e.g., `GetTaskMetric` -> `GetTaskByStateGaugeMetric`, `GetRayEventRecorderDroppedEventsMetric` -> `GetRayEventRecorderDroppedEventsCounterMetric`). > - **GCS Server Refactor**: > - `GcsServer` now constructs/accepts metric instances and passes them to subcomponents via `Start`/`DoStart` and init methods. > - `GcsActorManager`, `GcsJobManager`, `GcsPlacementGroupManager`, and `GcsTaskManager` constructors updated to receive and record via `MetricInterface`. > - `ObservableStoreClient` wraps delegate and records storage metrics via injected interfaces. > - **Core Worker**: > - `TaskCounter` and `CoreWorker` updated to use task/actor state gauges via injected `MetricInterface`. > - **Tests/Mocks/Build**: > - Update mocks and tests to use `FakeGauge`/`FakeCounter`/`FakeHistogram`; validate metric tags/values. > - Add Bazel targets/deps for new metric headers and fakes; minor BUILD wiring adjustments. > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit bd5ff5a. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).  --------- Signed-off-by: Cuong Nguyen <can@anyscale.com>

…y-project#57248) Reverts ray-project#56201

…#56201) This PR is in the series of unifying all metric definition infra. This PR migrates all GCS metrics to use the metric interface. It does that by creating the metric object inside gcs_server and pass them down as interfaces to sub-components. Purely refactoring code and repetitive patterns, easier to review than the number of file changed tells you. Test: - CI  --- > [!NOTE] > Refactors GCS and core worker to use injected MetricInterface objects for all metrics, adding new metric helpers and rewiring constructors, server startup, storage client, and tests accordingly. > > - **Metrics Infrastructure**: > - Introduce metric helpers in `src/ray/common/metrics.h` and `src/ray/gcs/metrics.h` (gauges/histograms/counters for actors, jobs, placement groups, task events, and GCS storage). > - Replace direct `stats` usage with `MetricInterface` across GCS and core worker; rename helpers (e.g., `GetTaskMetric` -> `GetTaskByStateGaugeMetric`, `GetRayEventRecorderDroppedEventsMetric` -> `GetRayEventRecorderDroppedEventsCounterMetric`). > - **GCS Server Refactor**: > - `GcsServer` now constructs/accepts metric instances and passes them to subcomponents via `Start`/`DoStart` and init methods. > - `GcsActorManager`, `GcsJobManager`, `GcsPlacementGroupManager`, and `GcsTaskManager` constructors updated to receive and record via `MetricInterface`. > - `ObservableStoreClient` wraps delegate and records storage metrics via injected interfaces. > - **Core Worker**: > - `TaskCounter` and `CoreWorker` updated to use task/actor state gauges via injected `MetricInterface`. > - **Tests/Mocks/Build**: > - Update mocks and tests to use `FakeGauge`/`FakeCounter`/`FakeHistogram`; validate metric tags/values. > - Add Bazel targets/deps for new metric headers and fakes; minor BUILD wiring adjustments. > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit bd5ff5a. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).  --------- Signed-off-by: Cuong Nguyen <can@anyscale.com> Signed-off-by: Josh Kodi <joshkodi@gmail.com>

…y-project#57248) Reverts ray-project#56201 Signed-off-by: Josh Kodi <joshkodi@gmail.com>

…#56201) This PR is in the series of unifying all metric definition infra. This PR migrates all GCS metrics to use the metric interface. It does that by creating the metric object inside gcs_server and pass them down as interfaces to sub-components. Purely refactoring code and repetitive patterns, easier to review than the number of file changed tells you. Test: - CI  --- > [!NOTE] > Refactors GCS and core worker to use injected MetricInterface objects for all metrics, adding new metric helpers and rewiring constructors, server startup, storage client, and tests accordingly. > > - **Metrics Infrastructure**: > - Introduce metric helpers in `src/ray/common/metrics.h` and `src/ray/gcs/metrics.h` (gauges/histograms/counters for actors, jobs, placement groups, task events, and GCS storage). > - Replace direct `stats` usage with `MetricInterface` across GCS and core worker; rename helpers (e.g., `GetTaskMetric` -> `GetTaskByStateGaugeMetric`, `GetRayEventRecorderDroppedEventsMetric` -> `GetRayEventRecorderDroppedEventsCounterMetric`). > - **GCS Server Refactor**: > - `GcsServer` now constructs/accepts metric instances and passes them to subcomponents via `Start`/`DoStart` and init methods. > - `GcsActorManager`, `GcsJobManager`, `GcsPlacementGroupManager`, and `GcsTaskManager` constructors updated to receive and record via `MetricInterface`. > - `ObservableStoreClient` wraps delegate and records storage metrics via injected interfaces. > - **Core Worker**: > - `TaskCounter` and `CoreWorker` updated to use task/actor state gauges via injected `MetricInterface`. > - **Tests/Mocks/Build**: > - Update mocks and tests to use `FakeGauge`/`FakeCounter`/`FakeHistogram`; validate metric tags/values. > - Add Bazel targets/deps for new metric headers and fakes; minor BUILD wiring adjustments. > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit bd5ff5a. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).  --------- Signed-off-by: Cuong Nguyen <can@anyscale.com>

…y-project#57248) Reverts ray-project#56201

can-anyscale force-pushed the can-metric02 branch from 9833161 to 1c6e1ea Compare September 3, 2025 19:54

can-anyscale force-pushed the can-metric01 branch 4 times, most recently from aa8a215 to 85e420f Compare September 5, 2025 21:53

Base automatically changed from can-metric01 to master September 8, 2025 17:37

can-anyscale force-pushed the can-metric02 branch 2 times, most recently from 587ed3b to bf0b3ff Compare September 11, 2025 19:07

can-anyscale changed the title ~~[core][metric] Redefine more STATS using metric interface~~ [core][metric] Redefine gcs STATS using metric interface Sep 11, 2025

can-anyscale marked this pull request as ready for review September 11, 2025 19:08

can-anyscale requested a review from a team as a code owner September 11, 2025 19:08

can-anyscale added the go add ONLY when ready to merge, run all tests label Sep 11, 2025

jjyao reviewed Sep 11, 2025

View reviewed changes

src/ray/gcs/gcs_server/gcs_server_main.cc Outdated Show resolved Hide resolved

can-anyscale marked this pull request as draft September 11, 2025 23:11

can-anyscale force-pushed the can-metric02 branch 7 times, most recently from ef046b6 to b22516d Compare September 23, 2025 21:56

can-anyscale force-pushed the can-metric02 branch 3 times, most recently from e53cd65 to be370f9 Compare September 24, 2025 16:19

ZacAttack reviewed Sep 24, 2025

View reviewed changes

can-anyscale force-pushed the can-metric02 branch from be370f9 to 752db28 Compare September 24, 2025 18:07

can-anyscale commented Sep 24, 2025

View reviewed changes

can-anyscale force-pushed the can-metric02 branch 2 times, most recently from a56c143 to cabfde9 Compare September 24, 2025 22:08

can-anyscale force-pushed the can-metric02 branch from 99892ac to 526b745 Compare October 3, 2025 01:02

can-anyscale requested a review from jjyao October 3, 2025 19:47

jjyao approved these changes Oct 3, 2025

View reviewed changes

can-anyscale force-pushed the can-metric02 branch 2 times, most recently from 2afc068 to 526b745 Compare October 3, 2025 22:26

Cuong Nguyen added 6 commits October 3, 2025 15:29

w00t

6fdf8c3

Signed-off-by: Cuong Nguyen <can@anyscale.com>

[core][metric/04] redefine STATS_tasks using Metric class

49848e7

Signed-off-by: Cuong Nguyen <can@anyscale.com>

w00t

acd38ed

Signed-off-by: Cuong Nguyen <can@anyscale.com>

[core][metric/04] redefine STATS_tasks using Metric class

b245af5

Signed-off-by: Cuong Nguyen <can@anyscale.com>

[core][metric] Redefine more STATS using metric interface

b230390

Signed-off-by: Cuong Nguyen <can@anyscale.com>

something

67cd514

Signed-off-by: Cuong Nguyen <can@anyscale.com>

can-anyscale force-pushed the can-metric02 branch from 526b745 to 67cd514 Compare October 3, 2025 22:31

can-anyscale merged commit 534b0e4 into master Oct 6, 2025
6 checks passed

can-anyscale deleted the can-metric02 branch October 6, 2025 18:37

aslonnie added a commit that referenced this pull request Oct 7, 2025

Revert "[core][metric] Redefine gcs STATS using metric interface (#56201

85f345a

)" This reverts commit 534b0e4.

aslonnie mentioned this pull request Oct 7, 2025

Revert "[core][metric] Redefine gcs STATS using metric interface" #57248

Merged

aslonnie added a commit that referenced this pull request Oct 7, 2025

Revert "[core][metric] Redefine gcs STATS using metric interface" (#5…

23aad02

…7248) Reverts #56201

liulehui pushed a commit to liulehui/ray that referenced this pull request Oct 9, 2025

Revert "[core][metric] Redefine gcs STATS using metric interface" (ra…

1bab17c

…y-project#57248) Reverts ray-project#56201

joshkodi pushed a commit to joshkodi/ray that referenced this pull request Oct 13, 2025

Revert "[core][metric] Redefine gcs STATS using metric interface" (ra…

b4512fe

…y-project#57248) Reverts ray-project#56201 Signed-off-by: Josh Kodi <joshkodi@gmail.com>

justinyeh1995 pushed a commit to justinyeh1995/ray that referenced this pull request Oct 20, 2025

Revert "[core][metric] Redefine gcs STATS using metric interface" (ra…

1d56f47

…y-project#57248) Reverts ray-project#56201

Conversation

can-anyscale commented Sep 3, 2025 • edited by cursor bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

ZacAttack Sep 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

can-anyscale commented Oct 3, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

can-anyscale commented Sep 3, 2025 •

edited by cursor bot

Loading

ZacAttack Sep 24, 2025 •

edited

Loading