[observability][export-api] Env var config to write export events of specific source types #49541

nikitavemuri · 2025-01-02T16:59:39Z

Why are these changes needed?

Add RAY_enable_export_api_write_config environment variable which can be set to a string containing a comma separated list of source types that export events should be written for. This is only considered if RAY_enable_export_api_write is not set, so this change is backward compatible.
This will allow users to select which export events they need to be written based on their use case or performance implications. Eg: A user can list all export events except EXPORT_TASK which we saw impacted task scheduling latency.
This feature is in alpha stage currently, so there are no public docs yet. We will send individual updates to users who have expressed interest in this feature.

Related issue number

Checks

I've signed off every commit(by using the -s flag, i.e., git commit -s) in this PR.
I've run scripts/format.sh to lint the changes in this PR.
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: Nikita Vemuri <[email protected]>

…ita/export_api_enable_config

Signed-off-by: Nikita Vemuri <[email protected]>

…ita/export_api_enable_config

Signed-off-by: Nikita Vemuri <[email protected]>

alanwguo · 2025-01-03T19:13:47Z

python/ray/_private/ray_constants.py

@@ -534,8 +534,22 @@ def gcs_actor_scheduling_enabled():

 RAY_BACKEND_LOG_JSON_ENV_VAR = "RAY_BACKEND_LOG_JSON"

+# Write export API event of all resource types to file if enabled.
+# RAY_enable_export_api_write_config will not be considered if


maybe we should have this flag always be considered. This can be used as a hard shut off if this is not set or set to False.

This change wouldn't be backward compatible if RAY_enable_export_api_write_config is always considered, right? I was thinking we can eventually deprecate RAY_enable_export_api_write, which should have the same effect.

alanwguo · 2025-01-03T19:15:45Z

python/ray/_private/event/export_event_logger.py

+        ray_export_api_config_json = json.loads(
+            ray_constants.RAY_ENABLE_EXPORT_API_WRITE_CONFIG
+        )


do we want to parse this every time? Or just once on ray launch / first time this config is read?

Good point, updated to move the JSON parsing to where the env var is read

Signed-off-by: Nikita Vemuri <[email protected]>

alanwguo · 2025-01-03T23:29:01Z

src/ray/util/event.cc

@@ -509,4 +509,27 @@ void RayEventInit(const std::vector<SourceTypeVariant> source_types,
      });
 }

+bool IsExportAPIEnabledSourceType(std::string source_type,


is this called once per event or per source?

This should be once per source now

Signed-off-by: Nikita Vemuri <[email protected]>

alanwguo

I think in the future we may want to support an RAY_enable_export_api_write_config=all config for convenience. Do we expect to be able to re-use this config for the new obs stack export path?

nikitavemuri · 2025-01-14T17:28:33Z

I think in the future we may want to support an RAY_enable_export_api_write_config=all config for convenience.

Sounds good, we can add this as a followup

Do we expect to be able to re-use this config for the new obs stack export path?

Yes, as long as we are ok with the naming

Signed-off-by: Nikita Vemuri <[email protected]>

MengjinYan · 2025-01-16T06:53:15Z

python/ray/_private/ray_constants.py

+    "RAY_enable_export_api_write_config", "[]"
+)
+try:
+    RAY_ENABLE_EXPORT_API_WRITE_CONFIG = json.loads(


Just to clarify, will RAY_enable_export_api_write_config always be a list of resources or we are thinking about extending it to more complex json objects in the future?

This should only be a flat list of resource types, so updated to a comma separated string so we can convert to a vector

MengjinYan · 2025-01-16T06:55:06Z

python/ray/_private/ray_constants.py

+    )
+except Exception:
+    RAY_ENABLE_EXPORT_API_WRITE_CONFIG = None
+    logger.exception("Error parsing JSON for RAY_enable_export_api_write_config. ")


Suggested change

logger.exception("Error parsing JSON for RAY_enable_export_api_write_config. ")

logger.exception(f"Error parsing JSON for RAY_enable_export_api_write_config. RAY_enable_export_api_write_config={RAY_ENABLE_EXPORT_API_WRITE_CONFIG_STR}")

MengjinYan · 2025-01-16T07:31:40Z

src/ray/common/ray_config_def.h

+// resource types in ExportEvent.SourceType enum in
+// src/ray/protobuf/export_api/export_event.proto
+// Example config: `export RAY_enable_export_api_write_config='["EXPORT_ACTOR"]'`
+RAY_CONFIG(std::string, enable_export_api_write_config, "[]")


If the config will always be a list, I'm wondering if it can be converted to a vector here instead of using std::string type. In this case, the conversion will only be done once.

Sounds good, updated to use std::vector<std::string> and have the config be a comma separated string instead of JSON

Signed-off-by: Nikita Vemuri <[email protected]>

MengjinYan · 2025-01-21T18:49:13Z

src/ray/common/ray_config_def.h

@@ -932,10 +932,10 @@ RAY_CONFIG(int, object_manager_rpc_threads_num, 0)
 // this is enabled.
 RAY_CONFIG(bool, enable_export_api_write, false)

-// JSON configuration that lists individual resource types to write


Wondering is there a particular reason we need a space in the separator of the resource string? If not, for better maintenance of the environment variable, wondering if we can just use , to separate the resources?

Yes, we can separate the resources with only , (the ConvertValue function already removes extra spaces so either one doesn't require changing the implementation)

MengjinYan · 2025-01-21T18:55:03Z

python/ray/dashboard/modules/event/tests/test_generate_export_events.py

@@ -9,7 +9,7 @@
 # RAY_enable_export_api_write_config env var must be set before importing
 # `ray` so the correct value is set for RAY_ENABLE_EXPORT_API_WRITE_CONFIG
 # even outside a Ray driver.
-os.environ["RAY_enable_export_api_write_config"] = '["EXPORT_SUBMISSION_JOB"]'


Wondering should we also add python test with multiple resources specified in the config?

I updated the test_check_export_api_enabled on line 48 to test multiple resources in the config.

The config on line 12 is needed for test_submission_job_export_events where submission jobs use the env var outside a Ray driver. Probably we won't need to add other resources to this line unless there is another resource that export events outside a driver in the future.

Signed-off-by: Nikita Vemuri <[email protected]>

MengjinYan

Looks good! @jjyao can you take a look and help merge the PR? Thanks!!

Nikita Vemuri added 2 commits December 15, 2024 22:21

add config env var c++ events

c5cc79e

Signed-off-by: Nikita Vemuri <[email protected]>

update config for submission job

092a5e2

Signed-off-by: Nikita Vemuri <[email protected]>

nikitavemuri added the go add ONLY when ready to merge, run all tests label Jan 2, 2025

Nikita Vemuri added 6 commits January 2, 2025 09:14

comments and update test

230cfdd

Signed-off-by: Nikita Vemuri <[email protected]>

Merge branch 'master' of https://github.com/nikitavemuri/ray into nik…

2ef5053

…ita/export_api_enable_config

fix import

46dbbdf

Signed-off-by: Nikita Vemuri <[email protected]>

update

eed72c3

Signed-off-by: Nikita Vemuri <[email protected]>

Merge branch 'master' of https://github.com/nikitavemuri/ray into nik…

dbf5acd

…ita/export_api_enable_config

update

84e825c

Signed-off-by: Nikita Vemuri <[email protected]>

nikitavemuri marked this pull request as ready for review January 3, 2025 18:05

nikitavemuri requested a review from a team as a code owner January 3, 2025 18:05

nikitavemuri requested a review from alanwguo January 3, 2025 18:06

alanwguo reviewed Jan 3, 2025

View reviewed changes

parse json once

263c95c

Signed-off-by: Nikita Vemuri <[email protected]>

nikitavemuri requested a review from alanwguo January 3, 2025 22:59

alanwguo reviewed Jan 3, 2025

View reviewed changes

update

e3b6d5d

Signed-off-by: Nikita Vemuri <[email protected]>

nikitavemuri requested a review from alanwguo January 13, 2025 20:54

alanwguo approved these changes Jan 14, 2025

View reviewed changes

merge

032ce7a

Signed-off-by: Nikita Vemuri <[email protected]>

MengjinYan reviewed Jan 16, 2025

View reviewed changes

Nikita Vemuri added 2 commits January 21, 2025 10:34

convert to vector string in RAY_CONFIG

6cb6a7b

Signed-off-by: Nikita Vemuri <[email protected]>

merge

8841f70

Signed-off-by: Nikita Vemuri <[email protected]>

nikitavemuri requested a review from MengjinYan January 21, 2025 18:41

MengjinYan reviewed Jan 21, 2025

View reviewed changes

only comma separated

c9b2367

Signed-off-by: Nikita Vemuri <[email protected]>

MengjinYan approved these changes Jan 21, 2025

View reviewed changes

MengjinYan assigned jjyao Jan 21, 2025

jjyao merged commit 67f9490 into ray-project:master Jan 22, 2025
5 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[observability][export-api] Env var config to write export events of specific source types #49541

[observability][export-api] Env var config to write export events of specific source types #49541

nikitavemuri commented Jan 2, 2025 •

edited

Loading

alanwguo Jan 3, 2025

nikitavemuri Jan 3, 2025

alanwguo Jan 3, 2025

nikitavemuri Jan 3, 2025

alanwguo Jan 3, 2025 •

edited

Loading

nikitavemuri Jan 13, 2025

alanwguo left a comment

nikitavemuri commented Jan 14, 2025

MengjinYan Jan 16, 2025

nikitavemuri Jan 21, 2025

MengjinYan Jan 16, 2025

MengjinYan Jan 16, 2025

nikitavemuri Jan 21, 2025

MengjinYan Jan 21, 2025

nikitavemuri Jan 21, 2025

MengjinYan Jan 21, 2025

nikitavemuri Jan 21, 2025

MengjinYan left a comment

	logger.exception("Error parsing JSON for RAY_enable_export_api_write_config. ")
	logger.exception(f"Error parsing JSON for RAY_enable_export_api_write_config. RAY_enable_export_api_write_config={RAY_ENABLE_EXPORT_API_WRITE_CONFIG_STR}")

[observability][export-api] Env var config to write export events of specific source types #49541

[observability][export-api] Env var config to write export events of specific source types #49541

Conversation

nikitavemuri commented Jan 2, 2025 • edited Loading

Why are these changes needed?

Related issue number

Checks

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alanwguo Jan 3, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alanwguo left a comment

Choose a reason for hiding this comment

nikitavemuri commented Jan 14, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MengjinYan left a comment

Choose a reason for hiding this comment

nikitavemuri commented Jan 2, 2025 •

edited

Loading

alanwguo Jan 3, 2025 •

edited

Loading