-
Notifications
You must be signed in to change notification settings - Fork 6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[observability][export-api] Env var config to write export events of specific source types #49541
Changes from 8 commits
c5cc79e
092a5e2
230cfdd
2ef5053
46dbbdf
eed72c3
dbf5acd
84e825c
263c95c
e3b6d5d
032ce7a
6cb6a7b
8841f70
c9b2367
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This change wouldn't be backward compatible if |
||
# this is enabled. | ||
RAY_ENABLE_EXPORT_API_WRITE = env_bool("RAY_enable_export_api_write", False) | ||
|
||
# JSON configuration that lists individual resource types to write | ||
# export API events for. This configuration is only used if | ||
# RAY_enable_export_api_write is not enabled. Full list of valid | ||
# 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_SUBMISSION_JOB"]'` | ||
RAY_ENABLE_EXPORT_API_WRITE_CONFIG = os.environ.get( | ||
"RAY_enable_export_api_write_config", "[]" | ||
) | ||
|
||
RAY_EXPORT_EVENT_MAX_FILE_SIZE_BYTES = env_bool( | ||
"RAY_EXPORT_EVENT_MAX_FILE_SIZE_BYTES", 100 * 1e6 | ||
) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -927,5 +927,15 @@ RAY_CONFIG(int, object_manager_client_connection_num, 4) | |
// Update this to overwrite it. | ||
RAY_CONFIG(int, object_manager_rpc_threads_num, 0) | ||
|
||
// Write export API events to file if enabled | ||
// Write export API event of all resource types to file if enabled. | ||
// RAY_enable_export_api_write_config will not be considered if | ||
// this is enabled. | ||
RAY_CONFIG(bool, enable_export_api_write, false) | ||
|
||
// JSON configuration that lists individual resource types to write | ||
// export API events for. This configuration is only used if | ||
// RAY_enable_export_api_write is not enabled. Full list of valid | ||
// 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, "[]") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If the config will always be a list, I'm wondering if it can be converted to a vector here instead of using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sounds good, updated to use |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -509,4 +509,27 @@ void RayEventInit(const std::vector<SourceTypeVariant> source_types, | |
}); | ||
} | ||
|
||
bool IsExportAPIEnabledSourceType(std::string source_type, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is this called once per event or per source? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be once per source now |
||
bool enable_export_api_write_global, | ||
std::string enable_export_api_write_config_str) { | ||
if (enable_export_api_write_global) { | ||
return true; | ||
} | ||
nlohmann::json enable_export_api_write_config = {}; | ||
try { | ||
enable_export_api_write_config = | ||
nlohmann::json::parse(enable_export_api_write_config_str); | ||
} catch (const nlohmann::json::parse_error &e) { | ||
RAY_LOG(ERROR) << "Error parsing JSON for RAY_enable_export_api_write_config: " | ||
<< e.what(); | ||
return false; | ||
} | ||
for (const auto &element : enable_export_api_write_config) { | ||
if (element == source_type) { | ||
return true; | ||
} | ||
} | ||
return false; | ||
} | ||
|
||
} // namespace ray |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we want to parse this every time? Or just once on ray launch / first time this config is read?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, updated to move the JSON parsing to where the env var is read