[core][stats-die/04] kill STATS in the common component #58299

can-anyscale · 2025-10-30T01:01:25Z

This PR replace STATS with Metric as a way to define metric inside ray (as a unification effort) in all common components. Normally, metrics are defined at the top-level component and passed down to sub-components. However, in this case, because the common component is used as an API across, doing so would feel unnecessarily cumbersome. I decided to define the metrics inline within each client and server class instead.

Note that the metric classes (Metric, Gauge, Sum, etc.) are simply wrappers around static OpenCensus/OpenTelemetry entities.

Details
Full context of this refactoring work.

Each component (e.g., gcs, raylet, core_worker, etc.) now has a metrics.h file located in its top-level directory. This file defines all metrics for that component.
In most cases, metrics are defined once in the main entry point of each component (gcs/gcs_server_main.cc for GCS, raylet/main.cc for Raylet, core_worker/core_worker_process.cc for the Core Worker, etc.). These metrics are then passed down to subcomponents via the ray::observability::MetricInterface.
This approach significantly reduces rebuild time when metric infrastructure changes. Previously, a change would trigger a full Ray rebuild; now, only the top-level entry points of each component need rebuilding.
There are a few exceptions where metrics are tracked inside object libraries (e.g., task_specification). In these cases, metrics are defined within the library itself, since there is no corresponding top-level entry point.

Test:

CI

gemini-code-assist

Code Review

This pull request continues the effort to unify metric definitions by replacing the legacy STATS macro system with the new Metric class-based system in Ray's common components. The changes are logical and follow the intended direction of refactoring.

I have two main points of feedback:

A critical issue regarding duplicate metric definitions for operation_* metrics, which could cause runtime problems. The old definitions need to be removed.
A medium-severity performance concern about creating metric objects on a hot path within StatsHandle. It would be more efficient to create them once in EventTracker.

Overall, this is a good step forward in the metrics refactoring. Addressing these points will improve the correctness and performance of the implementation.

src/ray/common/metrics.h

gemini-code-assist · 2025-10-30T01:03:23Z

src/ray/common/event_stats.h

+  ray::stats::Count operation_count_metric_{ray::GetOperationCountCounterMetric()};
+  ray::stats::Gauge operation_active_gauge_metric_{
+      ray::GetOperationActiveCountGaugeMetric()};
+  ray::stats::Histogram operation_run_time_ms_histogram_metric_{
+      ray::GetOperationRunTimeMsHistogramMetric()};
+  ray::stats::Histogram operation_queue_time_ms_histogram_metric_{
+      ray::GetOperationQueueTimeMsHistogramMetric()};


These metric objects are created as members of StatsHandle. A new StatsHandle is created for every event via EventTracker::RecordStart, which can be very frequent. This means these four metric objects are constructed and likely go through registration logic on every event, which could be a performance concern.

The PR description mentions that metrics are defined inline within client and server classes. Following that pattern, it would be more efficient to define these metrics once as members of EventTracker. Then, StatsHandle could hold pointers or references to them, passed from EventTracker during its construction. This would avoid repeated object creation and registration overhead on a hot path.

these metric objects are just wrapper of static object, no re-registration happens

what are the static objects? I agree with Gemini here that these metric objects shouldn't be members of StatsHandle. They should be part of EventTracker.

oh i didn't read gemini's comments fully; part of EventTracker makes sense, let see how to do that

can-anyscale · 2025-10-30T18:09:33Z

Move the operation metrics to be members of EventTracker instead of StatsHandle. Note that this requires RecordExecution and RecordEnd to be non-static, since they need to access instance members. Making the metric members static is unsafe, as they wrap OpenCensus/OpenTelemetry static objects.

CC: @jjyao

can-anyscale · 2025-11-14T19:19:55Z

ignore buildkite/microcheck failure, buildkite/premerge is a superset and it passed

jjyao · 2025-11-24T20:58:16Z

src/ray/raylet_ipc_client/client_connection.cc

         stats_handle = std::move(stats_handle)](const boost::system::error_code &error,
                                                 size_t bytes_transferred) {
-          EventTracker::RecordExecution(
+          event_stats->RecordExecution(


We don't need to capture event_stats. We have this so we can get static_cast<instrumented_io_context &>(socket_.get_executor().context()); again to get the event_stats. Trying to minimizing the things we capture if possible.

jjyao · 2025-11-24T20:58:30Z

src/ray/raylet_ipc_client/client_connection.cc

+        [this, this_ptr, event_stats, stats_handle = std::move(stats_handle)](
            const boost::system::error_code &ec, size_t bytes_transferred) mutable {
-          EventTracker::RecordExecution(
+          event_stats->RecordExecution(


can-anyscale · 2025-11-24T22:26:34Z

@jjyao's comments

Signed-off-by: Cuong Nguyen <[email protected]>

…58299) This PR replace STATS with Metric as a way to define metric inside ray (as a unification effort) in all common components. Normally, metrics are defined at the top-level component and passed down to sub-components. However, in this case, because the common component is used as an API across, doing so would feel unnecessarily cumbersome. I decided to define the metrics inline within each client and server class instead. Note that the metric classes (Metric, Gauge, Sum, etc.) are simply wrappers around static OpenCensus/OpenTelemetry entities. **Details** Full context of this refactoring work. - Each component (e.g., gcs, raylet, core_worker, etc.) now has a metrics.h file located in its top-level directory. This file defines all metrics for that component. - In most cases, metrics are defined once in the main entry point of each component (gcs/gcs_server_main.cc for GCS, raylet/main.cc for Raylet, core_worker/core_worker_process.cc for the Core Worker, etc.). These metrics are then passed down to subcomponents via the ray::observability::MetricInterface. - This approach significantly reduces rebuild time when metric infrastructure changes. Previously, a change would trigger a full Ray rebuild; now, only the top-level entry points of each component need rebuilding. - There are a few exceptions where metrics are tracked inside object libraries (e.g., task_specification). In these cases, metrics are defined within the library itself, since there is no corresponding top-level entry point. Test: - CI Signed-off-by: Cuong Nguyen <[email protected]> Signed-off-by: YK <[email protected]>

…58299) This PR replace STATS with Metric as a way to define metric inside ray (as a unification effort) in all common components. Normally, metrics are defined at the top-level component and passed down to sub-components. However, in this case, because the common component is used as an API across, doing so would feel unnecessarily cumbersome. I decided to define the metrics inline within each client and server class instead. Note that the metric classes (Metric, Gauge, Sum, etc.) are simply wrappers around static OpenCensus/OpenTelemetry entities. **Details** Full context of this refactoring work. - Each component (e.g., gcs, raylet, core_worker, etc.) now has a metrics.h file located in its top-level directory. This file defines all metrics for that component. - In most cases, metrics are defined once in the main entry point of each component (gcs/gcs_server_main.cc for GCS, raylet/main.cc for Raylet, core_worker/core_worker_process.cc for the Core Worker, etc.). These metrics are then passed down to subcomponents via the ray::observability::MetricInterface. - This approach significantly reduces rebuild time when metric infrastructure changes. Previously, a change would trigger a full Ray rebuild; now, only the top-level entry points of each component need rebuilding. - There are a few exceptions where metrics are tracked inside object libraries (e.g., task_specification). In these cases, metrics are defined within the library itself, since there is no corresponding top-level entry point. Test: - CI Signed-off-by: Cuong Nguyen <[email protected]>

can-anyscale requested a review from a team as a code owner October 30, 2025 01:01

can-anyscale force-pushed the can-statdie04 branch from 7b7adef to dce9219 Compare October 30, 2025 01:02

gemini-code-assist bot reviewed Oct 30, 2025

View reviewed changes

This comment was marked as outdated.

Sign in to view

can-anyscale force-pushed the can-statdie04 branch from dce9219 to b5c88e0 Compare October 30, 2025 01:14

ray-gardener bot added core Issues that should be addressed in Ray Core observability Issues related to the Ray Dashboard, Logging, Metrics, Tracing, and/or Profiling labels Oct 30, 2025

can-anyscale force-pushed the can-statdie04 branch from b5c88e0 to fb3013d Compare October 30, 2025 18:07

can-anyscale requested a review from jjyao October 30, 2025 18:09

This comment was marked as outdated.

Sign in to view

can-anyscale force-pushed the can-statdie04 branch from fb3013d to 393118e Compare October 30, 2025 18:11

can-anyscale force-pushed the can-statdie04 branch from 393118e to eb809b1 Compare November 13, 2025 18:09

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

jjyao approved these changes Nov 24, 2025

View reviewed changes

can-anyscale force-pushed the can-statdie04 branch from eb809b1 to 47dbeda Compare November 24, 2025 22:25

[core][stats-die/04] kill STATS in the common component

43c2382

Signed-off-by: Cuong Nguyen <[email protected]>

can-anyscale force-pushed the can-statdie04 branch from 47dbeda to 43c2382 Compare November 24, 2025 23:04

can-anyscale enabled auto-merge (squash) November 25, 2025 00:56

can-anyscale merged commit eb28037 into master Nov 25, 2025
7 checks passed

can-anyscale deleted the can-statdie04 branch November 25, 2025 01:12

[core][stats-die/04] kill STATS in the common component #58299

[core][stats-die/04] kill STATS in the common component #58299

Uh oh!

Conversation

can-anyscale commented Oct 30, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

gemini-code-assist bot Oct 30, 2025

Choose a reason for hiding this comment

Uh oh!

can-anyscale Oct 30, 2025

Choose a reason for hiding this comment

Uh oh!

jjyao Oct 30, 2025

Choose a reason for hiding this comment

Uh oh!

can-anyscale Oct 30, 2025

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

Uh oh!

can-anyscale commented Oct 30, 2025

Uh oh!

This comment was marked as outdated.

Uh oh!

can-anyscale commented Nov 14, 2025

Uh oh!

jjyao Nov 24, 2025

Choose a reason for hiding this comment

Uh oh!

jjyao Nov 24, 2025

Choose a reason for hiding this comment

Uh oh!

can-anyscale commented Nov 24, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants