[Core][TaskEvents] Add Integration Tests for Task Event Generation #57636

MengjinYan · 2025-10-11T00:27:26Z

Why are these changes needed?

This PR add integration tests for task status update events.

Related issue number

N/A

Checks

I've signed off every commit(by using the -s flag, i.e., git commit -s) in this PR.
I've run pre-commit jobs to lint the changes in this PR. (pre-commit setup)
I've included any doc changes needed for https://docs.ray.io/en/master/.
- I've added any new APIs to the API Reference. For example, if I added a
  method in Tune, I've added it in doc/source/tune/api/ under the
  corresponding .rst file.
I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
Testing Strategy
- Unit tests
- Release tests
- This PR is not tested :(

Signed-off-by: Mengjin Yan <mengjinyan3@gmail.com>

jjyao · 2025-10-21T04:57:56Z

python/ray/tests/test_ray_event_export_task_events.py

+                event["taskDefinitionEvent"]["taskType"] == "DRIVER_TASK"
+                and event["taskDefinitionEvent"]["jobId"] != test_job_id
+            ):
+                driver_task_id = event["taskDefinitionEvent"]["taskId"]


I think we should just test it with the field renaming turned off

Good point. The PR was written before the field renaming. Will the add the test for both and when we remove the env var, we can remove the field renaming in the test.

I think just testing the renaming disabled is fine since you have other tests to check to renaming but up to you.

Signed-off-by: Mengjin Yan <mengjinyan3@gmail.com>

jjyao

Let's figure out why it takes a long time to run. Ideally CI tests should be fast.

Signed-off-by: Mengjin Yan <mengjinyan3@gmail.com>

MengjinYan · 2025-10-22T21:36:27Z

Let's figure out why it takes a long time to run. Ideally CI tests should be fast.

I dig deeper into the tests and did some fixes to make the tests more stable. At the same time, I had the following findings from the tests and created followup issues for them:

[Core] Dashboard agent log file conflict when having multiple nodes on the same machine using Cluster class #58003: When we add multiple worker nodes to the Cluster object in the tests, we won't generate corresponding log files for dashboard agent and the runtime env agent. Instead, some of them share the same file name file name and cause conflict
[Core] Improve the start up sequence of the worker and dashboard agent #58007: During the start up process, the worker can start to connect to the dashboard agent before the agent is ready to receive grpc connection. This leads to the backoff retry strategy to kick in and the task event requests to fail even if the agent grpc server is ready. Currently, I add a time.sleep() in the test to mitigate the issue.
[Core] Improve the shutdown sequence between dashboard agent and core workers #58016: During the node graceful shutdown case, the dashboard agent can stop before the core workers, so even if during core worker shutdown, the events are exported, the requests will fail because of server unavailable. This issue causes the node fail test to be a little flaky when I run the test locally.

Signed-off-by: Mengjin Yan <mengjinyan3@gmail.com>

jjyao · 2025-10-23T05:39:11Z

python/ray/tests/test_ray_event_export_task_events.py

+def run_driver_script_and_wait_for_events(script, httpserver, cluster, validation_func):
+    httpserver.expect_request("/", method="POST").respond_with_data("", status=200)
+    node_ids = [node.node_id for node in cluster.list_all_nodes()]
+    assert wait_until_grpc_channel_ready(cluster.gcs_address, node_ids)


I think you can use the existing wait_until_server_available util?

Yeah I looked at the wait_until_server_available before but from what I understand, it only checks whether a port is ready but not the GRPC server is ready for connection for not.

jjyao · 2025-10-23T05:39:47Z

python/ray/tests/test_ray_event_export_task_events.py

+    # before start the driver script. A longer term fix is to improve the start up
+    # sequence of the dashboard agent and the workers.
+    # Followup issue: https://github.com/ray-project/ray/issues/58007
+    time.sleep(3)


since we already wait for the dashboard agent server to be up with wait_until_grpc_channel_ready, why do we need to sleep again?

My understanding what happened is:

When the test cluster started, we do ray.init() right away. This creates the driver process along with a core worker process.

The processes starts to send the events to the aggregator agent every 100ms, even if there is no events (this is fixed in the latest commit.)

At the time when the core worker starts, the aggregator agent might not ready to receive grpc connection, so the first sends failed. At the same time, the GRPC client's backoff retry strategy kicks in, so in the background, the connection will be retried only after the retry interval completes.

So it could happen that the aggregator agent server is ready to receive connection but the retry hasn't happened yet. And the wait here was mainly to wait for the retry to happen and the core worker has successfully create the connection.

At the same time, as mentioned in 2, I task buffer's updated to logic to only send grpc requests to aggregator agent when there are events to send. This should eliminate the issue. So I removed the sleep in the latest commit.

Signed-off-by: Mengjin Yan <mengjinyan3@gmail.com>

can-anyscale · 2025-10-24T01:39:33Z

just add myself so the PR shown up on my github review todo list, not blocking

jjyao

LGTM

python/ray/tests/test_ray_event_export_task_events.py

Co-authored-by: Jiajun Yao <jeromeyjj@gmail.com> Signed-off-by: Mengjin Yan <mengjinyan3@gmail.com>

Signed-off-by: Mengjin Yan <mengjinyan3@gmail.com>

…ay-project#57636) Signed-off-by: Mengjin Yan <mengjinyan3@gmail.com> Co-authored-by: Jiajun Yao <jeromeyjj@gmail.com> Signed-off-by: xgui <xgui@anyscale.com>

…ay-project#57636) Signed-off-by: Mengjin Yan <mengjinyan3@gmail.com> Co-authored-by: Jiajun Yao <jeromeyjj@gmail.com>

…ay-project#57636) Signed-off-by: Mengjin Yan <mengjinyan3@gmail.com> Co-authored-by: Jiajun Yao <jeromeyjj@gmail.com> Signed-off-by: Aydin Abiar <aydin@anyscale.com>

…ay-project#57636) Signed-off-by: Mengjin Yan <mengjinyan3@gmail.com> Co-authored-by: Jiajun Yao <jeromeyjj@gmail.com> Signed-off-by: Future-Outlier <eric901201@gmail.com>

MengjinYan added 2 commits October 10, 2025 16:50

add integration tests for task events

098ba29

Signed-off-by: Mengjin Yan <mengjinyan3@gmail.com>

Merge branch 'master' into core-703-11

e5a4744

MengjinYan added the go add ONLY when ready to merge, run all tests label Oct 15, 2025

MengjinYan added 6 commits October 15, 2025 13:01

Merge branch 'master' into core-703-11

3ac1a07

Merge branch 'master' into core-703-11

15be6be

Merge branch 'master' into core-703-11

5d9f73a

fix the tests based on the task event schema update

83a1a20

Signed-off-by: Mengjin Yan <mengjinyan3@gmail.com>

move the test to medium size test list

c232920

Signed-off-by: Mengjin Yan <mengjinyan3@gmail.com>

Merge branch 'master' into core-703-11

d869717

MengjinYan marked this pull request as ready for review October 20, 2025 19:23

MengjinYan requested a review from a team as a code owner October 20, 2025 19:23

This comment was marked as outdated.

Sign in to view

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 21, 2025

jjyao reviewed Oct 21, 2025

View reviewed changes

MengjinYan added 4 commits October 20, 2025 22:16

Merge branch 'master' into core-703-11

8791de0

add logic to test the preserving proto field option

1239b26

Signed-off-by: Mengjin Yan <mengjinyan3@gmail.com>

Merge branch 'master' into core-703-11

5176f7c

move the test to large

ee2869b

Signed-off-by: Mengjin Yan <mengjinyan3@gmail.com>

This comment was marked as outdated.

Sign in to view

jjyao approved these changes Oct 21, 2025

View reviewed changes

fix cursor comment

855b9e4

Signed-off-by: Mengjin Yan <mengjinyan3@gmail.com>

jjyao reviewed Oct 21, 2025

View reviewed changes

improve the test logic

0024526

Signed-off-by: Mengjin Yan <mengjinyan3@gmail.com>

MengjinYan added 3 commits October 22, 2025 14:43

Merge branch 'master' into core-703-11

bfe1fd0

add followup issues

83544c0

Signed-off-by: Mengjin Yan <mengjinyan3@gmail.com>

Merge branch 'master' into core-703-11

d7ee0a1

This comment was marked as outdated.

Sign in to view

move the test to medium size

7ef3bed

Signed-off-by: Mengjin Yan <mengjinyan3@gmail.com>

jjyao reviewed Oct 23, 2025

View reviewed changes

MengjinYan added 2 commits October 23, 2025 18:23

fix tests

a53c986

Signed-off-by: Mengjin Yan <mengjinyan3@gmail.com>

Merge branch 'master' into core-703-11

a037979

can-anyscale self-requested a review October 24, 2025 01:38

jjyao approved these changes Oct 24, 2025

View reviewed changes

python/ray/tests/test_ray_event_export_task_events.py Outdated Show resolved Hide resolved

python/ray/tests/test_ray_event_export_task_events.py Outdated Show resolved Hide resolved

MengjinYan and others added 4 commits October 24, 2025 12:26

Update python/ray/tests/test_ray_event_export_task_events.py

18693ac

Co-authored-by: Jiajun Yao <jeromeyjj@gmail.com> Signed-off-by: Mengjin Yan <mengjinyan3@gmail.com>

Update python/ray/tests/test_ray_event_export_task_events.py

88648c1

Co-authored-by: Jiajun Yao <jeromeyjj@gmail.com> Signed-off-by: Mengjin Yan <mengjinyan3@gmail.com>

fix lint failure

de4d664

Signed-off-by: Mengjin Yan <mengjinyan3@gmail.com>

Merge branch 'master' into core-703-11

e55ca1b

jjyao merged commit 9cbe131 into ray-project:master Oct 26, 2025
6 checks passed

[Core][TaskEvents] Add Integration Tests for Task Event Generation #57636

[Core][TaskEvents] Add Integration Tests for Task Event Generation #57636

Uh oh!

Conversation

MengjinYan commented Oct 11, 2025

Why are these changes needed?

Related issue number

Checks

Uh oh!

This comment was marked as outdated.

Uh oh!

jjyao Oct 21, 2025

Choose a reason for hiding this comment

Uh oh!

MengjinYan Oct 21, 2025

Choose a reason for hiding this comment

Uh oh!

jjyao Oct 21, 2025

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

Uh oh!

jjyao left a comment

Choose a reason for hiding this comment

Uh oh!

MengjinYan commented Oct 22, 2025

Uh oh!

This comment was marked as outdated.

Uh oh!

jjyao Oct 23, 2025

Choose a reason for hiding this comment

Uh oh!

MengjinYan Oct 23, 2025

Choose a reason for hiding this comment

Uh oh!

jjyao Oct 23, 2025

Choose a reason for hiding this comment

Uh oh!

MengjinYan Oct 24, 2025

Choose a reason for hiding this comment

Uh oh!

can-anyscale commented Oct 24, 2025

Uh oh!

jjyao left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants